correctly render response when accepting merged shares (#2830)

* correctly render response when accepting merged shares Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * update expected failures Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · May 6, 2022 · 7ff3d93 · 7ff3d93
1 parent 62b6715
commit 7ff3d93
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 19 deletions.
diff --git a/changelog/unreleased/fix-accepting-merged-shares.md b/changelog/unreleased/fix-accepting-merged-shares.md
@@ -0,0 +1,5 @@
+Bugfix: correctly render response when accepting merged shares
+
+We now only return the data for the accepted share instead of concatenating data for all affected shares.
+
+https://github.com/cs3org/reva/pull/2830
diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go
@@ -103,24 +103,31 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) {
 	*/
 
 	for id := range sharesToAccept {
-		h.updateReceivedShare(w, r, id, false, mount)
+		data := h.updateReceivedShare(w, r, id, false, mount)
+		// only render the data for the changed share
+		if id == shareID && data != nil {
+			response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
+		}
 	}
 }
 
 // RejectReceivedShare handles DELETE Requests on /apps/files_sharing/api/v1/shares/{shareid}
 func (h *Handler) RejectReceivedShare(w http.ResponseWriter, r *http.Request) {
 	shareID := chi.URLParam(r, "shareid")
-	h.updateReceivedShare(w, r, shareID, true, "")
+	data := h.updateReceivedShare(w, r, shareID, true, "")
+	if data != nil {
+		response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
+	}
 }
 
-func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, shareID string, rejectShare bool, mountPoint string) {
+func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, shareID string, rejectShare bool, mountPoint string) *conversions.ShareData {
 	ctx := r.Context()
 	logger := appctx.GetLogger(ctx)
 
 	client, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
-		return
+		return nil
 	}
 
 	// we need to add a path to the share
@@ -143,16 +150,16 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 	shareRes, err := client.UpdateReceivedShare(ctx, shareRequest)
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update received share request failed", err)
-		return
+		return nil
 	}
 
 	if shareRes.Status.Code != rpc.Code_CODE_OK {
 		if shareRes.Status.Code == rpc.Code_CODE_NOT_FOUND {
 			response.WriteOCSError(w, r, response.MetaNotFound.StatusCode, "not found", nil)
-			return
+			return nil
 		}
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update received share request failed", errors.Errorf("code: %d, message: %s", shareRes.Status.Code, shareRes.Status.Message))
-		return
+		return nil
 	}
 
 	rs := shareRes.GetShare()
@@ -161,7 +168,7 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 	if err != nil || status.Code != rpc.Code_CODE_OK {
 		h.logProblems(status, err, "could not stat, skipping")
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc get resource info failed", errors.Errorf("code: %d, message: %s", status.Code, status.Message))
-		return
+		return nil
 	}
 
 	data, err := conversions.CS3Share2ShareData(r.Context(), rs.Share)
@@ -181,7 +188,7 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 		data.Path = path.Join(h.sharePrefix, path.Base(info.Path))
 	}
 
-	response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
+	return data
 }
 
 // getReceivedShareFromID uses a client to the gateway to fetch a share based on its ID.

diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md
@@ -1547,11 +1547,6 @@ _ocs: api compatibility, return correct status code_
 - [apiShareCreateSpecialToShares2/createShareWithInvalidPermissions.feature:29](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares2/createShareWithInvalidPermissions.feature#L29)
 
 #### [OCS status code zero](https://github.com/owncloud/ocis/issues/3621)
-- [apiShareManagementToShares/mergeShare.feature:16](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L16)
-- [apiShareManagementToShares/mergeShare.feature:26](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L26)
-- [apiShareManagementToShares/mergeShare.feature:36](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L36)
-- [apiShareManagementToShares/mergeShare.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L48)
-- [apiShareManagementToShares/mergeShare.feature:60](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L60)
 - [apiShareManagementToShares/moveReceivedShare.feature:32](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveReceivedShare.feature#L32)
 
 #### [HTTP status code differ while listing the contents of another user's trash bin](https://github.com/owncloud/ocis/issues/3561)

diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md
@@ -1546,11 +1546,6 @@ _ocs: api compatibility, return correct status code_
 - [apiShareCreateSpecialToShares2/createShareWithInvalidPermissions.feature:29](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareCreateSpecialToShares2/createShareWithInvalidPermissions.feature#L29)
 
 #### [OCS status code zero](https://github.com/owncloud/ocis/issues/3621)
-- [apiShareManagementToShares/mergeShare.feature:16](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L16)
-- [apiShareManagementToShares/mergeShare.feature:26](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L26)
-- [apiShareManagementToShares/mergeShare.feature:36](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L36)
-- [apiShareManagementToShares/mergeShare.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L48)
-- [apiShareManagementToShares/mergeShare.feature:60](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/mergeShare.feature#L60)
 - [apiShareManagementToShares/moveReceivedShare.feature:32](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveReceivedShare.feature#L32)
 
 #### [HTTP status code differ while listing the contents of another user's trash bin](https://github.com/owncloud/ocis/issues/3561)