allow listing all storage spaces (#2344)

* allow listing all storage spaces Signed-off-by: Jörn Friedrich Dreyer <[email protected]> * introduce dumb findAllProviders Signed-off-by: Jörn Friedrich Dreyer <[email protected]> * update spaces registry test Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
cs3org · Dec 9, 2021 · b403441 · b403441
1 parent 4f19dd8
commit b403441
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 5 deletions.
diff --git a/changelog/unreleased/spaces-registry-allow-list-all.md b/changelog/unreleased/spaces-registry-allow-list-all.md
@@ -0,0 +1,5 @@
+Enhancement: allow listing all storage spaces
+
+To implement the drives api we now list all spaces when no filter is given. The Provider info will not contain any spaces as the client is responsible for looking up the spaces.
+
+https://github.com/cs3org/reva/pull/2344
diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go
@@ -275,6 +275,10 @@ func (r *registry) ListProviders(ctx context.Context, filters map[string]string)
 		return r.findProvidersForResource(ctx, filters["storage_id"]+"!"+filters["opaque_id"]), nil
 	case filters["path"] != "":
 		return r.findProvidersForAbsolutePathReference(ctx, filters["path"]), nil
+		// TODO add filter for all spaces the user can manage?
+	case len(filters) == 0:
+		// return all providers
+		return r.findAllProviders(ctx), nil
 	}
 	return []*registrypb.ProviderInfo{}, nil
 }
@@ -332,19 +336,20 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r
 						appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing")
 						continue
 					}
+					// we can stop after we found the first space
+					// TODO to improve lookup time the registry could cache which provider last was responsible for a space? could be invalidated by simple ttl? would that work for shares?
 					return []*registrypb.ProviderInfo{p}
 				}
 			}
 		default:
 			// there should not be multiple spaces with the same id per provider
 			appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("spaces", spaces).Msg("multiple spaces returned, ignoring")
 		}
-
 	}
 	return []*registrypb.ProviderInfo{}
 }
 
-// findProvidersForAbsolutePathReference takes a path and ruturns the storage provider with the longest matching path prefix
+// findProvidersForAbsolutePathReference takes a path and returns the storage provider with the longest matching path prefix
 // FIXME use regex to return the correct provider when multiple are configured
 func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, path string) []*registrypb.ProviderInfo {
 	currentUser := ctxpkg.ContextMustGetUser(ctx)
@@ -452,6 +457,18 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa
 	return pis
 }
 
+// findAllProviders returns a list of all storage providers
+// This is a dumb call that does not call ListStorageSpaces() on the providers: ListStorageSpaces() in the gateway can cache that better.
+func (r *registry) findAllProviders(ctx context.Context) []*registrypb.ProviderInfo {
+	pis := make([]*registrypb.ProviderInfo, 0, len(r.c.Providers))
+	for address := range r.c.Providers {
+		pis = append(pis, &registrypb.ProviderInfo{
+			Address: address,
+		})
+	}
+	return pis
+}
+
 func spacePathsToOpaque(spacePaths map[string]string) (*typesv1beta1.Opaque, error) {
 	spacePathsJSON, err := json.Marshal(spacePaths)
 	if err != nil {

diff --git a/pkg/storage/registry/spaces/spaces_test.go b/pkg/storage/registry/spaces/spaces_test.go
@@ -37,7 +37,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("Static", func() {
+var _ = Describe("Spaces", func() {
 	var (
 		handler   storage.Registry
 		ctxAlice  context.Context
@@ -278,11 +278,11 @@ var _ = Describe("Static", func() {
 		})
 
 		Describe("ListProviders", func() {
-			It("returns an empty list when no filters are set", func() {
+			It("returns all providers when no filter is set", func() {
 				filters := map[string]string{}
 				providers, err := handler.ListProviders(ctxAlice, filters)
 				Expect(err).ToNot(HaveOccurred())
-				Expect(len(providers)).To(Equal(0))
+				Expect(len(providers)).To(Equal(3))
 			})
 
 			It("filters by path", func() {