From ac1fc44c2b36f8c4cd16c555a72499ff4900e1b7 Mon Sep 17 00:00:00 2001
From: Jesse Geens <jgeens@cern.ch>
Date: Fri, 20 Dec 2024 10:09:50 +0100
Subject: [PATCH] Add db ping to check for liveness in CreateShare

---
 changelog/unreleased/pseudo-tx-share.md       |  8 +++++
 .../services/gateway/usershareprovider.go     | 31 ++++++++++++-------
 .../usershareprovider/usershareprovider.go    |  2 +-
 3 files changed, 29 insertions(+), 12 deletions(-)
 create mode 100644 changelog/unreleased/pseudo-tx-share.md

diff --git a/changelog/unreleased/pseudo-tx-share.md b/changelog/unreleased/pseudo-tx-share.md
new file mode 100644
index 0000000000..59372d060b
--- /dev/null
+++ b/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
\ No newline at end of file
diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go
index 75cef8d223..5b8ef7a04f 100644
--- a/internal/grpc/services/gateway/usershareprovider.go
+++ b/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"
@@ -44,6 +45,8 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
 		return nil, errtypes.AlreadyExists("gateway: can't share the share folder itself")
 	}
 
+	log := appctx.GetLogger(ctx)
+
 	shareClient, err := pool.GetUserShareProviderClient(pool.Endpoint(s.c.UserShareProviderEndpoint))
 	if err != nil {
 		return &collaboration.CreateShareResponse{
@@ -53,19 +56,25 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
 
 	// First we ping the db
 	// --------------------
-
-	// Then we set ACLs on the storage layer
-	// -------------------------------------
-
-	// First, we keep a copy of the original permissions, so we can revert if necessary
-	statResp, err := s.Stat(ctx, &provider.StatRequest{
-		Ref: &provider.Reference{
-			ResourceId: req.ResourceInfo.Id,
-			Path:       req.ResourceInfo.Path,
+	// See ADR-REVA-003
+	_, err = shareClient.GetShare(ctx, &collaboration.GetShareRequest{
+		Ref: &collaboration.ShareReference{
+			Spec: &collaboration.ShareReference_Id{
+				Id: &collaboration.ShareId{
+					OpaqueId: "0",
+				},
+			},
 		},
 	})
 
-	originalPermissions := statResp.GetInfo().PermissionSet
+	// 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 {
@@ -101,8 +110,8 @@ func (s *svc) CreateShare(ctx context.Context, req *collaboration.CreateShareReq
 	// 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 := shareClient.CreateShare(ctx, req)
 
-	// If this fails, we undo updating the storage provider
 	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 {
diff --git a/internal/grpc/services/usershareprovider/usershareprovider.go b/internal/grpc/services/usershareprovider/usershareprovider.go
index 6df38cb029..1bd566c289 100644
--- a/internal/grpc/services/usershareprovider/usershareprovider.go
+++ b/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{