From 4e100c471c8f7f1a8b0ce89209ac23ade5aa79e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= <jfd@butonic.de>
Date: Thu, 9 Dec 2021 13:52:54 +0000
Subject: [PATCH] introduce dumb findAllProviders
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
---
 pkg/storage/registry/spaces/spaces.go | 72 ++++++++++++++++-----------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go
index 88904155979..56a43b912a5 100644
--- a/pkg/storage/registry/spaces/spaces.go
+++ b/pkg/storage/registry/spaces/spaces.go
@@ -276,6 +276,9 @@ func (r *registry) ListProviders(ctx context.Context, filters map[string]string)
 	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
 }
@@ -285,7 +288,6 @@ func (r *registry) ListProviders(ctx context.Context, filters map[string]string)
 // for share spaces the res.StorageId tells the registry the spaceid and res.OpaqueId is a node in that space
 func (r *registry) findProvidersForResource(ctx context.Context, id string) []*registrypb.ProviderInfo {
 	currentUser := ctxpkg.ContextMustGetUser(ctx)
-	providerInfos := []*registrypb.ProviderInfo{}
 	for address, provider := range r.c.Providers {
 		p := &registrypb.ProviderInfo{
 			Address:    address,
@@ -314,38 +316,40 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r
 			continue
 		}
 
-		// build the correct path for every space
-		spacePaths := map[string]string{}
-		for _, space := range spaces {
-			var sc *spaceConfig
-			var ok bool
-			if sc, ok = provider.Spaces[space.SpaceType]; !ok {
-				continue
-			}
-			spacePaths[space.Id.OpaqueId], err = sc.SpacePath(currentUser, space)
-			if err != nil {
-				appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("space", space).Msg("failed to execute template, continuing")
-				continue
+		switch len(spaces) {
+		case 0:
+			// nothing to do, will continue with next provider
+		case 1:
+			space := spaces[0]
+			for spaceType, sc := range provider.Spaces {
+				if spaceType == space.SpaceType {
+					providerPath, err := sc.SpacePath(currentUser, space)
+					if err != nil {
+						appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("space", space).Msg("failed to execute template, continuing")
+						continue
+					}
+					spacePaths := map[string]string{
+						space.Id.OpaqueId: providerPath,
+					}
+					p.Opaque, err = spacePathsToOpaque(spacePaths)
+					if err != nil {
+						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")
 		}
-
-		if len(spacePaths) == 0 {
-			// do not add provider to result
-			continue
-		}
-
-		// add spaces to providerinfo
-		p.Opaque, err = spacePathsToOpaque(spacePaths)
-		if err != nil {
-			appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing")
-			continue
-		}
-		providerInfos = append(providerInfos, p)
 	}
-	return providerInfos
+	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)
@@ -453,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 {