Pseudo-transactionalize sharing

changelog/unreleased/pseudo-tx-share.md

@@ -0,0 +1,8 @@
+Enhancement: pseudo-transactionalize sharing
+
+Currently, sharing is not transactionalized: setting ACLs and writing the share to the db is completely independent. Because of this, sometimes a share is written to the database even though the ACLs have not been set. This enhancement fixes this by
+a) first pinging the db
+b) writing the ACLs
+c) writing to the db
+
+https://github.com/cs3org/reva/pull/5029

internal/grpc/services/gateway/usershareprovider.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"fmt"
 	"path"
+	"strings"
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
@@ -37,34 +38,42 @@ import (
 	"github.com/pkg/errors"
 )
 
-// TODO(labkode): add multi-phase commit logic when commit share or commit ref is enabled.
 func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareRequest) (*collaboration.CreateShareResponse, error) {
 	if s.isSharedFolder(ctx, req.ResourceInfo.GetPath()) {
 		return nil, errtypes.AlreadyExists("gateway: can't share the share folder itself")
 	}
 
-	c, err := pool.GetUserShareProviderClient(pool.Endpoint(s.c.UserShareProviderEndpoint))
+	log := appctx.GetLogger(ctx)
+
+	shareClient, err := pool.GetUserShareProviderClient(pool.Endpoint(s.c.UserShareProviderEndpoint))
 	if err != nil {
 		return &collaboration.CreateShareResponse{
 			Status: status.NewInternal(ctx, err, "error getting user share provider client"),
 		}, nil
 	}
 
-	// TODO the user share manager needs to be able to decide if the current user is allowed to create that share (and not eg. incerase permissions)
-	// jfd: AFAICT this can only be determined by a storage driver - either the storage provider is queried first or the share manager needs to access the storage using a storage driver
-	res, err := c.CreateShare(ctx, req)
-	if err != nil {
-		return nil, errors.Wrap(err, "gateway: error calling CreateShare")
-	}
-	if res.Status.Code != rpc.Code_CODE_OK {
-		return res, nil
-	}
+	// First we ping the db
+	// --------------------
+	// See ADR-REVA-003
+	_, err = shareClient.GetShare(ctx, &collaboration.GetShareRequest{
+		Ref: &collaboration.ShareReference{
+			Spec: &collaboration.ShareReference_Id{
+				Id: &collaboration.ShareId{
+					OpaqueId: "0",
+				},
+			},
+		},
+	})
 
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
+	// We expect a "not found" error when querying ID 0
+	// error checking is kind of ugly, because we lose the original error object over grpc
+	if !strings.HasSuffix(err.Error(), errtypes.NotFound("0").Error()) {
+		return nil, errtypes.InternalError("ShareManager is not online")
 	}
 
+	// Then we set ACLs on the storage layer
+	// -------------------------------------
+
 	// TODO(labkode): if both commits are enabled they could be done concurrently.
 	if s.c.CommitShareToStorageGrant {
 		// If the share is a denial we call  denyGrant instead.
@@ -78,18 +87,29 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
 					Status: denyGrantStatus,
 				}, err
 			}
-			return res, nil
+		} else {
+			addGrantStatus, err := s.addGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, req.Grant.Permissions.Permissions)
+			if err != nil {
+				return nil, errors.Wrap(err, "gateway: error adding grant to storage")
+			}
+			if addGrantStatus.Code != rpc.Code_CODE_OK {
+				return &collaboration.CreateShareResponse{
+					Status: addGrantStatus,
+				}, err
+			}
 		}
+	}
 
-		addGrantStatus, err := s.addGrant(ctx, req.ResourceInfo.Id, req.Grant.Grantee, req.Grant.Permissions.Permissions)
-		if err != nil {
-			return nil, errors.Wrap(err, "gateway: error adding grant to storage")
-		}
-		if addGrantStatus.Code != rpc.Code_CODE_OK {
-			return &collaboration.CreateShareResponse{
-				Status: addGrantStatus,
-			}, err
-		}
+	// Then we commit to the db
+	// ------------------------
+	res, err := shareClient.CreateShare(ctx, req)
+
+	if err != nil {
+		log.Error().Msg("Failed to Create Share but ACLs are already set")
+		return nil, errors.Wrap(err, "gateway: error calling CreateShare")
+	}
+	if res.Status.Code != rpc.Code_CODE_OK {
+		return res, nil
 	}
 
 	return res, nil

internal/grpc/services/usershareprovider/usershareprovider.go

@@ -172,7 +172,7 @@ func (s *service) GetShare(ctx context.Context, req *collaboration.GetShareReque
 	if err != nil {
 		return &collaboration.GetShareResponse{
 			Status: status.NewInternal(ctx, err, "error getting share"),
-		}, nil
+		}, err
 	}
 
 	return &collaboration.GetShareResponse{