remove unused gateway config "commit_share_to_storage_ref" (#3017)

* remove unused gateway config "commit_share_to_storage_ref" * Update changelog/unreleased/fix-remove-unused-commit-share-to-storage-ref.md Co-authored-by: Phil Davis <[email protected]> Co-authored-by: Phil Davis <[email protected]>
cs3org · Jul 5, 2022 · a5318e5 · a5318e5
1 parent 20c332e
commit a5318e5
Show file tree

Hide file tree

Showing 17 changed files with 9 additions and 48 deletions.
diff --git a/changelog/unreleased/fix-remove-unused-commit-share-to-storage-ref.md b/changelog/unreleased/fix-remove-unused-commit-share-to-storage-ref.md
@@ -0,0 +1,5 @@
+Bugfix: removed unused gateway config "commit_share_to_storage_ref"
+
+We've removed the unused gateway configuration option "commit_share_to_storage_ref".
+
+https://github.com/cs3org/reva/pull/3017
diff --git a/examples/ocmd/ocmd-server-1.toml b/examples/ocmd/ocmd-server-1.toml
@@ -31,7 +31,6 @@ ocmproviderauthorizersvc = "localhost:19000"
 datagateway = "http://localhost:19001/data"
 transfer_expires = 6 # give it a moment
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.authregistry]
 driver = "static"

diff --git a/examples/ocmd/ocmd-server-2.toml b/examples/ocmd/ocmd-server-2.toml
@@ -18,7 +18,6 @@ ocmproviderauthorizersvc = "localhost:17000"
 datagateway = "http://localhost:17001/data"
 transfer_expires = 6 # give it a moment
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.authregistry]
 driver = "static"

diff --git a/examples/oidc-mapping-tpc/server-1.toml b/examples/oidc-mapping-tpc/server-1.toml
@@ -8,7 +8,6 @@ gatewaysvc = "localhost:19000"
 # services to enable
 [grpc.services.gateway]
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.authprovider]
 [grpc.services.authprovider.auth_managers.json]

diff --git a/examples/oidc-mapping-tpc/server-2.toml b/examples/oidc-mapping-tpc/server-2.toml
@@ -8,7 +8,6 @@ gatewaysvc = "localhost:17000"
 # services to enable
 [grpc.services.gateway]
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.authprovider]
 [grpc.services.authprovider.auth_managers.json]

diff --git a/examples/storage-references/gateway.toml b/examples/storage-references/gateway.toml
@@ -1,7 +1,6 @@
 # services to enable
 [grpc.services.gateway]
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.storageregistry]
 [grpc.services.storageregistry.drivers.static]

diff --git a/examples/two-server-setup/gateway-1.toml b/examples/two-server-setup/gateway-1.toml
@@ -4,7 +4,6 @@ address = "0.0.0.0:19000"
 # services to enable
 [grpc.services.gateway]
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.storageregistry]
 [grpc.services.storageregistry.drivers.static]

diff --git a/examples/two-server-setup/gateway-2.toml b/examples/two-server-setup/gateway-2.toml
@@ -19,7 +19,6 @@ ocmshareprovidersvc = "localhost:29000"
 ocminvitemanagersvc = "localhost:29000"
 ocmproviderauthorizersvc = "localhost:29000"
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 
 [grpc.services.storageregistry]
 [grpc.services.storageregistry.drivers.static]

diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go
@@ -56,7 +56,6 @@ type config struct {
 	DataGatewayEndpoint           string `mapstructure:"datagateway"`
 	PermissionsEndpoint           string `mapstructure:"permissionssvc"`
 	CommitShareToStorageGrant     bool   `mapstructure:"commit_share_to_storage_grant"`
-	CommitShareToStorageRef       bool   `mapstructure:"commit_share_to_storage_ref"`
 	DisableHomeCreationOnLogin    bool   `mapstructure:"disable_home_creation_on_login"`
 	TransferSharedSecret          string `mapstructure:"transfer_shared_secret"`
 	TransferExpires               int64  `mapstructure:"transfer_expires"`

diff --git a/internal/grpc/services/gateway/ocmshareprovider.go b/internal/grpc/services/gateway/ocmshareprovider.go
@@ -53,12 +53,6 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest
 		return nil, errors.Wrap(err, "gateway: error calling CreateShare")
 	}
 
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
-	}
-
-	// TODO(labkode): if both commits are enabled they could be done concurrently.
 	if s.c.CommitShareToStorageGrant {
 		addGrantStatus, err := s.addGrant(ctx, req.ResourceId, req.Grant.Grantee, req.Grant.Permissions.Permissions, nil)
 		if err != nil {
@@ -108,12 +102,6 @@ func (s *svc) RemoveOCMShare(ctx context.Context, req *ocm.RemoveOCMShareRequest
 		return nil, errors.Wrap(err, "gateway: error calling RemoveShare")
 	}
 
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
-	}
-
-	// TODO(labkode): if both commits are enabled they could be done concurrently.
 	if s.c.CommitShareToStorageGrant {
 		removeGrantStatus, err := s.removeGrant(ctx, share.ResourceId, share.Grantee, share.Permissions.Permissions)
 		if err != nil {
@@ -221,7 +209,7 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive
 	}
 
 	// if we don't need to create/delete references then we return early.
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
+	if !s.c.CommitShareToStorageGrant {
 		return res, nil
 	}
 

diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go
@@ -115,13 +115,6 @@ func (s *svc) UpdateShare(ctx context.Context, req *collaboration.UpdateShareReq
 		return nil, errors.Wrap(err, "gateway: error calling UpdateShare")
 	}
 
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
-	}
-
-	// TODO(labkode): if both commits are enabled they could be done concurrently.
-
 	if s.c.CommitShareToStorageGrant {
 		updateGrantStatus, err := s.updateGrant(ctx, res.GetShare().GetResourceId(),
 			res.GetShare().GetGrantee(),
@@ -559,12 +552,7 @@ func (s *svc) addShare(ctx context.Context, req *collaboration.CreateShareReques
 	if res.Status.Code != rpc.Code_CODE_OK {
 		return res, nil
 	}
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
-	}
 
-	// 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.
 		var status *rpc.Status
@@ -652,7 +640,7 @@ func (s *svc) removeShare(ctx context.Context, req *collaboration.RemoveShareReq
 	// if we need to commit the share, we need the resource it points to.
 	var share *collaboration.Share
 	// FIXME: I will cause a panic as share will be nil when I'm false
-	if s.c.CommitShareToStorageGrant || s.c.CommitShareToStorageRef {
+	if s.c.CommitShareToStorageGrant {
 		getShareReq := &collaboration.GetShareRequest{
 			Ref: req.Ref,
 		}
@@ -681,12 +669,6 @@ func (s *svc) removeShare(ctx context.Context, req *collaboration.RemoveShareReq
 		s.removeReference(ctx, share.ResourceId)
 	}
 
-	// if we don't need to commit we return earlier
-	if !s.c.CommitShareToStorageGrant && !s.c.CommitShareToStorageRef {
-		return res, nil
-	}
-
-	// TODO(labkode): if both commits are enabled they could be done concurrently.
 	if s.c.CommitShareToStorageGrant {
 		removeGrantStatus, err := s.removeGrant(ctx, share.ResourceId, share.Grantee, share.Permissions.Permissions)
 		if err != nil {

diff --git a/tests/oc-integration-tests/drone/gateway-virtual.toml b/tests/oc-integration-tests/drone/gateway-virtual.toml
@@ -32,7 +32,6 @@ ocminvitemanagersvc = "localhost:14000"
 ocmproviderauthorizersvc = "localhost:14000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 share_folder = "Shares"
 datagateway = "http://localhost:19001/data"
 transfer_shared_secret = "replace-me-with-a-transfer-secret" # for direct uploads

diff --git a/tests/oc-integration-tests/drone/gateway.toml b/tests/oc-integration-tests/drone/gateway.toml
@@ -34,7 +34,6 @@ ocmproviderauthorizersvc = "localhost:14000"
 permissionssvc = "localhost:10000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 share_folder = "Shares"
 datagateway = "http://localhost:19001/data"
 transfer_shared_secret = "replace-me-with-a-transfer-secret" # for direct uploads

diff --git a/tests/oc-integration-tests/local-mesh/gateway.toml b/tests/oc-integration-tests/local-mesh/gateway.toml
@@ -32,7 +32,6 @@ ocminvitemanagersvc = "localhost:34000"
 ocmproviderauthorizersvc = "localhost:34000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 share_folder = "Shares"
 datagateway = "http://localhost:39001/data"
 transfer_shared_secret = "replace-me-with-a-transfer-secret" # for direct uploads

diff --git a/tests/oc-integration-tests/local/combined.toml b/tests/oc-integration-tests/local/combined.toml
@@ -22,7 +22,6 @@ jwt_secret = "Pive-Fumkiu4"
 #groupprovidersvc = "localhost:18000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 #share_folder = "Shares"
 transfer_expires = 6 # give it a moment
 #disable_home_creation_on_login = true
@@ -75,7 +74,7 @@ users = "users.demo.json"
 [grpc.services.sharesstorageprovider]
 usershareprovidersvc = "0.0.0.0:19000"
 
-# FIXME start as a separate service ... collides with the storageprovider: 
+# FIXME start as a separate service ... collides with the storageprovider:
 # Server.RegisterService found duplicate service registration for "cs3.storage.provider.v1beta1.ProviderAPI"
 #[grpc.services.sharesstorageprovider]
 
@@ -186,4 +185,4 @@ temp_folder = "/var/tmp/reva/tmp"
 root = "/var/tmp/reva/data"
 enable_home = false
 treetime_accounting = true
-treesize_accounting = true
+treesize_accounting = true
diff --git a/tests/oc-integration-tests/local/gateway-virtual.toml b/tests/oc-integration-tests/local/gateway-virtual.toml
@@ -36,7 +36,6 @@ ocminvitemanagersvc = "localhost:14000"
 ocmproviderauthorizersvc = "localhost:14000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 share_folder = "Shares"
 datagateway = "http://localhost:19001/data"
 transfer_shared_secret = "replace-me-with-a-transfer-secret" # for direct uploads

diff --git a/tests/oc-integration-tests/local/gateway.toml b/tests/oc-integration-tests/local/gateway.toml
@@ -38,7 +38,6 @@ ocmproviderauthorizersvc = "localhost:14000"
 permissionssvc = "localhost:10000"
 # other
 commit_share_to_storage_grant = true
-commit_share_to_storage_ref = true
 share_folder = "Shares"
 datagateway = "http://localhost:19001/data"
 transfer_shared_secret = "replace-me-with-a-transfer-secret" # for direct uploads