Revert "[cbox-commit-5] concurrently resolve list with me shares"

This reverts commit 773c42d.
cernbox · Oct 25, 2021 · d34c557 · d34c557
1 parent 28d3a89
commit d34c557
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 90 deletions.
diff --git a/docs/content/en/docs/config/packages/auth/manager/nextcloud/_index.md b/docs/content/en/docs/config/packages/auth/manager/nextcloud/_index.md
@@ -9,7 +9,7 @@ description: >
 # _struct: AuthManagerConfig_
 
 {{% dir name="endpoint" type="string" default="" %}}
-The Nextcloud backend endpoint for user check [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/auth/manager/nextcloud/nextcloud.go#L51)
+The Nextcloud backend endpoint for user check [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/auth/manager/nextcloud/nextcloud.go#L49)
 {{< highlight toml >}}
 [auth.manager.nextcloud]
 endpoint = ""

diff --git a/docs/content/en/docs/config/packages/share/manager/nextcloud/_index.md b/docs/content/en/docs/config/packages/share/manager/nextcloud/_index.md
@@ -9,7 +9,7 @@ description: >
 # _struct: ShareManagerConfig_
 
 {{% dir name="endpoint" type="string" default="" %}}
-The Nextcloud backend endpoint for user check [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/share/manager/nextcloud/nextcloud.go#L58)
+The Nextcloud backend endpoint for user check [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/share/manager/nextcloud/nextcloud.go#L56)
 {{< highlight toml >}}
 [share.manager.nextcloud]
 endpoint = ""

diff --git a/docs/content/en/docs/config/packages/user/manager/nextcloud/_index.md b/docs/content/en/docs/config/packages/user/manager/nextcloud/_index.md
@@ -9,7 +9,7 @@ description: >
 # _struct: UserManagerConfig_
 
 {{% dir name="endpoint" type="string" default="" %}}
-The Nextcloud backend endpoint for user management [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/user/manager/nextcloud/nextcloud.go#L53)
+The Nextcloud backend endpoint for user management [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/user/manager/nextcloud/nextcloud.go#L51)
 {{< highlight toml >}}
 [user.manager.nextcloud]
 endpoint = ""

diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go
@@ -28,8 +28,8 @@ import (
 	"sync"
 	"time"
 
-	"github.com/cs3org/reva/pkg/utils"
 	"google.golang.org/protobuf/types/known/fieldmaskpb"
+	"github.com/cs3org/reva/pkg/utils"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"

diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go
@@ -29,7 +29,6 @@ import (
 	"path"
 	"strconv"
 	"strings"
-	"sync"
 	"text/template"
 	"time"
 
@@ -611,102 +610,86 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	shares := make([]*conversions.ShareData, len(lrsRes.GetShares()))
-
-	const maxConcurrency = 50 // for example
-	var throttle = make(chan int, maxConcurrency)
-	var wg sync.WaitGroup
+	shares := make([]*conversions.ShareData, 0, len(lrsRes.GetShares()))
 
 	// TODO(refs) filter out "invalid" shares
-	for i, rs := range lrsRes.GetShares() {
-		throttle <- 1
-		wg.Add(1)
-		go func(ctx context.Context, client gateway.GatewayAPIClient, share *collaboration.ReceivedShare, wg *sync.WaitGroup, data *conversions.ShareData) {
-			defer wg.Done()
-			defer func() {
-				<-throttle
-			}()
-
-			if stateFilter != ocsStateUnknown && rs.GetState() != stateFilter {
-				return
+	for _, rs := range lrsRes.GetShares() {
+		if stateFilter != ocsStateUnknown && rs.GetState() != stateFilter {
+			continue
+		}
+		var info *provider.ResourceInfo
+		if pinfo != nil {
+			// check if the shared resource matches the path resource
+			if !utils.ResourceIDEqual(rs.Share.ResourceId, pinfo.Id) {
+				// try next share
+				continue
 			}
-			var info *provider.ResourceInfo
-			if pinfo != nil {
-				// check if the shared resource matches the path resource
-				if !utils.ResourceIDEqual(rs.Share.ResourceId, pinfo.Id) {
-					// try next share
-					return
-				}
-				// we can reuse the stat info
-				info = pinfo
-			} else {
-				var status *rpc.Status
-				info, status, err = h.getResourceInfoByID(ctx, client, rs.Share.ResourceId)
-				if err != nil || status.Code != rpc.Code_CODE_OK {
-					h.logProblems(status, err, "could not stat, skipping")
-					return
-				}
-
+			// we can reuse the stat info
+			info = pinfo
+		} else {
+			var status *rpc.Status
+			info, status, err = h.getResourceInfoByID(ctx, client, rs.Share.ResourceId)
+			if err != nil || status.Code != rpc.Code_CODE_OK {
+				h.logProblems(status, err, "could not stat, skipping")
+				continue
 			}
+		}
 
-			dataS, err := conversions.CS3Share2ShareData(r.Context(), rs.Share)
-			if err != nil {
-				log.Debug().Interface("share", rs.Share).Interface("shareData", data).Err(err).Msg("could not CS3Share2ShareData, skipping")
-				return
-			}
-			data = dataS
+		data, err := conversions.CS3Share2ShareData(r.Context(), rs.Share)
+		if err != nil {
+			log.Debug().Interface("share", rs.Share).Interface("shareData", data).Err(err).Msg("could not CS3Share2ShareData, skipping")
+			continue
+		}
 
-			data.State = mapState(rs.GetState())
+		data.State = mapState(rs.GetState())
 
-			if err := h.addFileInfo(ctx, data, info); err != nil {
-				log.Debug().Interface("received_share", rs).Interface("info", info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping")
-				return
-			}
-			h.mapUserIds(r.Context(), client, data)
-
-			if data.State == ocsStateAccepted {
-				// only accepted shares can be accessed when jailing users into their home.
-				// in this case we cannot stat shared resources that are outside the users home (/home),
-				// the path (/users/u-u-i-d/foo) will not be accessible
-
-				// in a global namespace we can access the share using the full path
-				// in a jailed namespace we have to point to the mount point in the users /Shares jail
-				// - needed for oc10 hot migration
-				// or use the /dav/spaces/<space id> endpoint?
-
-				// list /Shares and match fileids with list of received shares
-				// - only works for a /Shares folder jail
-				// - does not work for freely mountable shares as in oc10 because we would need to iterate over the whole tree, there is no listing of mountpoints, yet
-
-				// can we return the mountpoint when the gateway resolves the listing of shares?
-				// - no, the gateway only sees the same list any has the same options as the ocs service
-				// - we would need to have a list of mountpoints for the shares -> owncloudstorageprovider for hot migration migration
-
-				// best we can do for now is stat the /Shares jail if it is set and return those paths
-
-				// if we are in a jail and the current share has been accepted use the stat from the share jail
-				// Needed because received shares can be jailed in a folder in the users home
-
-				if h.sharePrefix != "/" {
-					// if we have share jail infos use them to build the path
-					if sji := findMatch(shareJailInfos, rs.Share.ResourceId); sji != nil {
-						// override path with info from share jail
-						data.FileTarget = path.Join(h.sharePrefix, path.Base(sji.Path))
-						data.Path = path.Join(h.sharePrefix, path.Base(sji.Path))
-					} else {
-						data.FileTarget = path.Join(h.sharePrefix, path.Base(info.Path))
-						data.Path = path.Join(h.sharePrefix, path.Base(info.Path))
-					}
+		if err := h.addFileInfo(ctx, data, info); err != nil {
+			log.Debug().Interface("received_share", rs).Interface("info", info).Interface("shareData", data).Err(err).Msg("could not add file info, skipping")
+			continue
+		}
+		h.mapUserIds(r.Context(), client, data)
+
+		if data.State == ocsStateAccepted {
+			// only accepted shares can be accessed when jailing users into their home.
+			// in this case we cannot stat shared resources that are outside the users home (/home),
+			// the path (/users/u-u-i-d/foo) will not be accessible
+
+			// in a global namespace we can access the share using the full path
+			// in a jailed namespace we have to point to the mount point in the users /Shares jail
+			// - needed for oc10 hot migration
+			// or use the /dav/spaces/<space id> endpoint?
+
+			// list /Shares and match fileids with list of received shares
+			// - only works for a /Shares folder jail
+			// - does not work for freely mountable shares as in oc10 because we would need to iterate over the whole tree, there is no listing of mountpoints, yet
+
+			// can we return the mountpoint when the gateway resolves the listing of shares?
+			// - no, the gateway only sees the same list any has the same options as the ocs service
+			// - we would need to have a list of mountpoints for the shares -> owncloudstorageprovider for hot migration migration
+
+			// best we can do for now is stat the /Shares jail if it is set and return those paths
+
+			// if we are in a jail and the current share has been accepted use the stat from the share jail
+			// Needed because received shares can be jailed in a folder in the users home
+
+			if h.sharePrefix != "/" {
+				// if we have share jail infos use them to build the path
+				if sji := findMatch(shareJailInfos, rs.Share.ResourceId); sji != nil {
+					// override path with info from share jail
+					data.FileTarget = path.Join(h.sharePrefix, path.Base(sji.Path))
+					data.Path = path.Join(h.sharePrefix, path.Base(sji.Path))
 				} else {
-					data.FileTarget = info.Path
-					data.Path = info.Path
+					data.FileTarget = path.Join(h.sharePrefix, path.Base(info.Path))
+					data.Path = path.Join(h.sharePrefix, path.Base(info.Path))
 				}
+			} else {
+				data.FileTarget = info.Path
+				data.Path = info.Path
 			}
+		}
 
-			log.Debug().Msgf("share: %+v", data)
-		}(ctx, client, rs, &wg, shares[i])
-		wg.Wait()
-
+		shares = append(shares, data)
+		log.Debug().Msgf("share: %+v", *data)
 	}
 
 	response.WriteOCSSuccess(w, r, shares)