From 1e5d75e3661df2750c9cb30149e597bb04a12b31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 1 Jul 2022 10:23:16 +0000 Subject: [PATCH 1/5] indexer: introduce LookupCtx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer lint Signed-off-by: Jörn Friedrich Dreyer add changelog Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/lookupctx.md | 6 ++ .../owncloud/ocdav/propfind/propfind.go | 32 ++++++----- pkg/publicshare/manager/cs3/cs3.go | 36 ++++++------ .../utils/indexer/index/autoincrement.go | 50 ++++++++++++++--- pkg/storage/utils/indexer/index/index.go | 7 ++- pkg/storage/utils/indexer/index/non_unique.go | 55 ++++++++++++++++--- pkg/storage/utils/indexer/index/unique.go | 54 +++++++++++++++--- pkg/storage/utils/indexer/indexer.go | 20 ++++++- 8 files changed, 200 insertions(+), 60 deletions(-) create mode 100644 changelog/unreleased/lookupctx.md diff --git a/changelog/unreleased/lookupctx.md b/changelog/unreleased/lookupctx.md new file mode 100644 index 0000000000..bd084b60d1 --- /dev/null +++ b/changelog/unreleased/lookupctx.md @@ -0,0 +1,6 @@ +Enhancement: introduce LookupCtx for index interface + +The index interface now has a new LookupCtx that can look up multiple values so we can more efficiently look up multiple shares by id. +It also takes a context so we can pass on the trace context to the CS3 backend + +https://github.com/cs3org/reva/pull/3043 \ No newline at end of file diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go index df90d295d4..10c29c017f 100644 --- a/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -366,26 +366,28 @@ func (p *Handler) propfindResponse(ctx context.Context, w http.ResponseWriter, r ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(ctx, "propfind_response") defer span.End() - filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) - for i := range resourceInfos { - // the list of filters grows with every public link in a folder - filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) - } - - client, err := p.getClient() - if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return - } - var linkshares map[string]struct{} // public link access does not show share-types // oc:share-type is not part of an allprops response if namespace != "/public" { // only fetch this if property was queried - for _, p := range pf.Prop { - if p.Space == net.NsOwncloud && (p.Local == "share-types" || p.Local == "permissions") { + for _, prop := range pf.Prop { + if prop.Space == net.NsOwncloud && (prop.Local == "share-types" || prop.Local == "permissions") { + filters := make([]*link.ListPublicSharesRequest_Filter, 0, len(resourceInfos)) + for i := range resourceInfos { + // FIXME this is expensive + // the filters array grow by one for every file in a folder + // TODO store public links as grants on the storage, reassembling them here is too costly + // we can then add the filter if the file has share-types=3 in the opaque, + // same as user / group shares for share indicators + filters = append(filters, publicshare.ResourceIDFilter(resourceInfos[i].Id)) + } + client, err := p.getClient() + if err != nil { + log.Error().Err(err).Msg("error getting grpc client") + w.WriteHeader(http.StatusInternalServerError) + return + } listResp, err := client.ListPublicShares(ctx, &link.ListPublicSharesRequest{Filters: filters}) if err == nil { linkshares = make(map[string]struct{}, len(listResp.Share)) diff --git a/pkg/publicshare/manager/cs3/cs3.go b/pkg/publicshare/manager/cs3/cs3.go index f021389d2f..eea63ec730 100644 --- a/pkg/publicshare/manager/cs3/cs3.go +++ b/pkg/publicshare/manager/cs3/cs3.go @@ -397,33 +397,37 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return result, nil } - tokensByResourceID := make(map[string]*provider.ResourceId) - for _, filter := range idFilter { - resourceID := filter.GetResourceId() - tokens, err := m.indexer.FindBy(&link.PublicShare{}, - indexer.NewField("ResourceId", resourceIDToIndex(resourceID)), - ) - if err != nil { - continue + tokens := []string{} + if len(idFilter) > 0 { + idFilters := []indexer.Field{} + for _, filter := range idFilter { + resourceID := filter.GetResourceId() + idFilters = append(idFilters, indexer.NewField("ResourceId", resourceIDToIndex(resourceID))) } - for _, token := range tokens { - tokensByResourceID[token] = resourceID + tokens, err = m.indexer.FindBy(&link.PublicShare{}, idFilters...) + if err != nil { + return nil, err } } // statMem is used as a local cache to prevent statting resources which // already have been checked. statMem := make(map[string]struct{}) - for token, resourceID := range tokensByResourceID { + for _, token := range tokens { if _, handled := shareMem[token]; handled { // We don't want to add a share multiple times when we added it // already. continue } - if _, checked := statMem[resourceIDToIndex(resourceID)]; !checked { + s, err := m.getByToken(ctx, token) + if err != nil { + return nil, err + } + + if _, checked := statMem[resourceIDToIndex(s.PublicShare.GetResourceId())]; !checked { sReq := &provider.StatRequest{ - Ref: &provider.Reference{ResourceId: resourceID}, + Ref: &provider.Reference{ResourceId: s.PublicShare.GetResourceId()}, } sRes, err := m.gatewayClient.Stat(ctx, sReq) if err != nil { @@ -435,13 +439,9 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] if !sRes.Info.PermissionSet.ListGrants { continue } - statMem[resourceIDToIndex(resourceID)] = struct{}{} + statMem[resourceIDToIndex(s.PublicShare.GetResourceId())] = struct{}{} } - s, err := m.getByToken(ctx, token) - if err != nil { - return nil, err - } if publicshare.MatchesFilters(s.PublicShare, filters) { result = append(result, &s.PublicShare) shareMem[s.PublicShare.Token] = struct{}{} diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index b02dc3df79..02c0077a45 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -75,17 +75,53 @@ func (idx *Autoincrement) Init() error { // Lookup exact lookup by value. func (idx *Autoincrement) Lookup(v string) ([]string, error) { - searchPath := path.Join(idx.indexRootDir, v) - oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) - if err != nil { - if os.IsNotExist(err) { - err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + return idx.LookupCtx(context.Background(), v) +} + +// LookupCtx retieves multiple exact values and allows passing in a context +func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + var allValues []string + var err error + if len(values) != 1 { + allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + if err != nil { + return nil, err + } + } else { + allValues = values + } + + var matches = []string{} + for _, av := range allValues { + // TODO check if av can contain more than the base path + av = path.Base(av) + for i, v := range values { + if av == v { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, av)) + if err != nil { + break + } + matches = append(matches, oldname) + values = remove(values, i) + break + } } - return nil, err + } + if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } + return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return []string{oldname}, nil + return matches, nil } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/index.go b/pkg/storage/utils/indexer/index/index.go index 6b593ff97f..15b8a94704 100644 --- a/pkg/storage/utils/indexer/index/index.go +++ b/pkg/storage/utils/indexer/index/index.go @@ -18,13 +18,18 @@ package index -import "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" +import ( + "context" + + "github.com/cs3org/reva/v2/pkg/storage/utils/indexer/option" +) // Index can be implemented to create new indexer-strategies. See Unique for example. // Each indexer implementation is bound to one data-column (IndexBy) and a data-type (TypeName) type Index interface { Init() error Lookup(v string) ([]string, error) + LookupCtx(ctx context.Context, v ...string) ([]string, error) Add(id, v string) (string, error) Remove(id string, v string) error Update(id, oldV, newV string) error diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index f40e992a68..85eca10268 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -79,24 +79,63 @@ func (idx *NonUnique) Init() error { // Lookup exact lookup by value. func (idx *NonUnique) Lookup(v string) ([]string, error) { - if idx.caseInsensitive { - v = strings.ToLower(v) - } - paths, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) + return idx.LookupCtx(context.Background(), v) +} + +// LookupCtx retieves multiple exact values and allows passing in a context +func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + allValues, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { return nil, err } - var matches = make([]string, 0) - for _, p := range paths { - matches = append(matches, path.Base(p)) + if idx.caseInsensitive { + for i := range values { + values[i] = strings.ToLower(values[i]) + } + } + + var matches = map[string]struct{}{} + for _, av := range allValues { + av = path.Base(av) + for i, v := range values { + if av == v { + children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, av)) + if err != nil { + break + } + for _, c := range children { + matches[path.Base(c)] = struct{}{} + } + values = remove(values, i) + break + } + } } if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return matches, nil + ret := make([]string, 0, len(matches)) + for m := range matches { + ret = append(ret, m) + } + return ret, nil +} + +func remove(s []string, i int) []string { + s[i] = s[len(s)-1] + return s[:len(s)-1] } // Add a new value to the index. diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index fccf092f3a..843ece137c 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -74,20 +74,58 @@ func (idx *Unique) Init() error { // Lookup exact lookup by value. func (idx *Unique) Lookup(v string) ([]string, error) { + return idx.LookupCtx(context.Background(), v) +} + +// LookupCtx retieves multiple exact values and allows passing in a context +func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { + var allValues []string + var err error + if len(values) != 1 { + allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + if err != nil { + return nil, err + } + } else { + allValues = values + } if idx.caseInsensitive { - v = strings.ToLower(v) + for i := range allValues { + allValues[i] = strings.ToLower(allValues[i]) + } } - searchPath := path.Join(idx.indexRootDir, v) - oldname, err := idx.storage.ResolveSymlink(context.Background(), searchPath) - if err != nil { - if os.IsNotExist(err) { - err = &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} + + var matches = make([]string, 0) + for _, av := range allValues { + // TODO check if av can contain more than the base path + av = path.Base(av) + for i, v := range values { + if av == v { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, av)) + if err != nil { + break + } + matches = append(matches, oldname) + values = remove(values, i) + break + } } - return nil, err + } + if len(matches) == 0 { + var v string + switch len(values) { + case 0: + v = "none" + case 1: + v = values[0] + default: + v = "multiple" + } + return nil, &idxerrs.NotFoundErr{TypeName: idx.typeName, IndexBy: idx.indexBy, Value: v} } - return []string{oldname}, nil + return matches, nil } // Add adds a value to the index, returns the path to the root-document diff --git a/pkg/storage/utils/indexer/indexer.go b/pkg/storage/utils/indexer/indexer.go index 1a9837533b..a3a7ace296 100644 --- a/pkg/storage/utils/indexer/indexer.go +++ b/pkg/storage/utils/indexer/indexer.go @@ -166,9 +166,14 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, resultPaths := make(map[string]struct{}) if fields, ok := i.indices[typeName]; ok { - for _, field := range queryFields { - for _, idx := range fields.IndicesByField[strcase.ToCamel(field.Name)] { - res, err := idx.Lookup(field.Value) + for fieldName, queryFields := range groupFieldsByName(queryFields) { + idxes := fields.IndicesByField[strcase.ToCamel(fieldName)] + values := make([]string, 0, len(queryFields)) + for _, f := range queryFields { + values = append(values, f.Value) + } + for _, idx := range idxes { + res, err := idx.LookupCtx(context.Background(), values...) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { continue @@ -193,6 +198,15 @@ func (i *StorageIndexer) FindBy(t interface{}, queryFields ...Field) ([]string, return result, nil } +// groupFieldsByName groups the given filters and returns a map using the filter type as the key. +func groupFieldsByName(queryFields []Field) map[string][]Field { + grouped := make(map[string][]Field) + for _, f := range queryFields { + grouped[f.Name] = append(grouped[f.Name], f) + } + return grouped +} + // Delete deletes all indexed fields of a given type t on the Indexer. func (i *StorageIndexer) Delete(t interface{}) error { typeName := getTypeFQN(t) From 3314f584069087382e9ab7d2ca2237130c47b78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 7 Jul 2022 15:45:17 +0000 Subject: [PATCH 2/5] use sets instead af arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/publicshare/manager/cs3/cs3.go | 4 +-- .../utils/indexer/index/autoincrement.go | 26 ++++++++-------- pkg/storage/utils/indexer/index/non_unique.go | 30 ++++++++----------- pkg/storage/utils/indexer/index/unique.go | 30 ++++++++----------- 4 files changed, 40 insertions(+), 50 deletions(-) diff --git a/pkg/publicshare/manager/cs3/cs3.go b/pkg/publicshare/manager/cs3/cs3.go index eea63ec730..3b24d8088e 100644 --- a/pkg/publicshare/manager/cs3/cs3.go +++ b/pkg/publicshare/manager/cs3/cs3.go @@ -397,9 +397,9 @@ func (m *Manager) ListPublicShares(ctx context.Context, u *user.User, filters [] return result, nil } - tokens := []string{} + var tokens []string if len(idFilter) > 0 { - idFilters := []indexer.Field{} + idFilters := make([]indexer.Field, 0, len(idFilter)) for _, filter := range idFilter { resourceID := filter.GetResourceId() idFilters = append(idFilters, indexer.NewField("ResourceId", resourceIDToIndex(resourceID))) diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index 02c0077a45..2b52393019 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -91,23 +91,21 @@ func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]st allValues = values } + valueSet := make(map[string]struct{}, len(allValues)) + for _, v := range allValues { + valueSet[path.Base(v)] = struct{}{} + } + var matches = []string{} - for _, av := range allValues { - // TODO check if av can contain more than the base path - av = path.Base(av) - for i, v := range values { - if av == v { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, av)) - if err != nil { - break - } - matches = append(matches, oldname) - values = remove(values, i) - break - } - } + for v := range valueSet { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, v)) + if err != nil { + continue + } + matches = append(matches, oldname) } + if len(matches) == 0 { var v string switch len(values) { diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index 85eca10268..a1c4ee4ba8 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -89,27 +89,23 @@ func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string return nil, err } - if idx.caseInsensitive { - for i := range values { - values[i] = strings.ToLower(values[i]) + valueSet := make(map[string]struct{}, len(allValues)) + for _, v := range allValues { + if idx.caseInsensitive { + valueSet[strings.ToLower(path.Base(v))] = struct{}{} + } else { + valueSet[path.Base(v)] = struct{}{} } } var matches = map[string]struct{}{} - for _, av := range allValues { - av = path.Base(av) - for i, v := range values { - if av == v { - children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, av)) - if err != nil { - break - } - for _, c := range children { - matches[path.Base(c)] = struct{}{} - } - values = remove(values, i) - break - } + for v := range valueSet { + children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) + if err != nil { + continue + } + for _, c := range children { + matches[path.Base(c)] = struct{}{} } } diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index 843ece137c..3e3bc3f96c 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -89,29 +89,25 @@ func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, e } else { allValues = values } - if idx.caseInsensitive { - for i := range allValues { - allValues[i] = strings.ToLower(allValues[i]) + + valueSet := make(map[string]struct{}, len(allValues)) + for _, v := range allValues { + if idx.caseInsensitive { + valueSet[strings.ToLower(path.Base(v))] = struct{}{} + } else { + valueSet[path.Base(v)] = struct{}{} } } var matches = make([]string, 0) - for _, av := range allValues { - // TODO check if av can contain more than the base path - av = path.Base(av) - for i, v := range values { - if av == v { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, av)) - if err != nil { - break - } - matches = append(matches, oldname) - values = remove(values, i) - break - } + for v := range valueSet { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, v)) + if err != nil { + continue } - + matches = append(matches, oldname) } + if len(matches) == 0 { var v string switch len(values) { From 76266497d38e730aa4c9a0036897978d2488ba49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 8 Jul 2022 09:53:47 +0200 Subject: [PATCH 3/5] Update pkg/storage/utils/indexer/index/autoincrement.go Co-authored-by: David Christofas --- pkg/storage/utils/indexer/index/autoincrement.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index 2b52393019..1dc170cc44 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -98,13 +98,15 @@ func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]st var matches = []string{} - for v := range valueSet { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, v)) - if err != nil { - continue - } - matches = append(matches, oldname) +for _, v := range values { + if _, ok := valueSet[v]; ok { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, v)) + if err != nil { + continue + } + matches = append(matches, oldname) } +} if len(matches) == 0 { var v string From c4b70b6f98eb161d21b344b76497bc7198d7a77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 8 Jul 2022 09:01:19 +0000 Subject: [PATCH 4/5] fix logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../utils/indexer/index/autoincrement.go | 37 ++++++++-------- pkg/storage/utils/indexer/index/non_unique.go | 42 +++++++++++-------- pkg/storage/utils/indexer/index/unique.go | 38 ++++++++++------- 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/pkg/storage/utils/indexer/index/autoincrement.go b/pkg/storage/utils/indexer/index/autoincrement.go index 1dc170cc44..f1009c16e1 100644 --- a/pkg/storage/utils/indexer/index/autoincrement.go +++ b/pkg/storage/utils/indexer/index/autoincrement.go @@ -80,33 +80,36 @@ func (idx *Autoincrement) Lookup(v string) ([]string, error) { // LookupCtx retieves multiple exact values and allows passing in a context func (idx *Autoincrement) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - var allValues []string - var err error + var allValues map[string]struct{} if len(values) != 1 { - allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + // prefetch all values with one request + entries, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { return nil, err } - } else { - allValues = values + // convert known values to set + allValues = make(map[string]struct{}, len(entries)) + for _, e := range entries { + allValues[path.Base(e)] = struct{}{} + } } - valueSet := make(map[string]struct{}, len(allValues)) - for _, v := range allValues { - valueSet[path.Base(v)] = struct{}{} + // convert requested values to set + valueSet := make(map[string]struct{}, len(values)) + for _, v := range values { + valueSet[v] = struct{}{} } var matches = []string{} - -for _, v := range values { - if _, ok := valueSet[v]; ok { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, v)) - if err != nil { - continue - } - matches = append(matches, oldname) + for v := range valueSet { + if _, ok := allValues[v]; ok || len(allValues) == 0 { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join("/", idx.indexRootDir, v)) + if err != nil { + continue + } + matches = append(matches, oldname) + } } -} if len(matches) == 0 { var v string diff --git a/pkg/storage/utils/indexer/index/non_unique.go b/pkg/storage/utils/indexer/index/non_unique.go index a1c4ee4ba8..b96f17e6cc 100644 --- a/pkg/storage/utils/indexer/index/non_unique.go +++ b/pkg/storage/utils/indexer/index/non_unique.go @@ -84,28 +84,39 @@ func (idx *NonUnique) Lookup(v string) ([]string, error) { // LookupCtx retieves multiple exact values and allows passing in a context func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - allValues, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + // prefetch all values with one request + entries, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { return nil, err } + // convert known values to set + allValues := make(map[string]struct{}, len(entries)) + for _, e := range entries { + allValues[path.Base(e)] = struct{}{} + } - valueSet := make(map[string]struct{}, len(allValues)) - for _, v := range allValues { - if idx.caseInsensitive { - valueSet[strings.ToLower(path.Base(v))] = struct{}{} - } else { - valueSet[path.Base(v)] = struct{}{} + // convert requested values to set + valueSet := make(map[string]struct{}, len(values)) + if idx.caseInsensitive { + for _, v := range values { + valueSet[strings.ToLower(v)] = struct{}{} + } + } else { + for _, v := range values { + valueSet[v] = struct{}{} } } var matches = map[string]struct{}{} for v := range valueSet { - children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) - if err != nil { - continue - } - for _, c := range children { - matches[path.Base(c)] = struct{}{} + if _, ok := allValues[v]; ok { + children, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir, v)) + if err != nil { + continue + } + for _, c := range children { + matches[path.Base(c)] = struct{}{} + } } } @@ -129,11 +140,6 @@ func (idx *NonUnique) LookupCtx(ctx context.Context, values ...string) ([]string return ret, nil } -func remove(s []string, i int) []string { - s[i] = s[len(s)-1] - return s[:len(s)-1] -} - // Add a new value to the index. func (idx *NonUnique) Add(id, v string) (string, error) { if v == "" { diff --git a/pkg/storage/utils/indexer/index/unique.go b/pkg/storage/utils/indexer/index/unique.go index 3e3bc3f96c..e92894d01d 100644 --- a/pkg/storage/utils/indexer/index/unique.go +++ b/pkg/storage/utils/indexer/index/unique.go @@ -79,33 +79,41 @@ func (idx *Unique) Lookup(v string) ([]string, error) { // LookupCtx retieves multiple exact values and allows passing in a context func (idx *Unique) LookupCtx(ctx context.Context, values ...string) ([]string, error) { - var allValues []string - var err error + var allValues map[string]struct{} if len(values) != 1 { - allValues, err = idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) + // prefetch all values with one request + entries, err := idx.storage.ReadDir(context.Background(), path.Join("/", idx.indexRootDir)) if err != nil { return nil, err } - } else { - allValues = values + // convert known values to set + allValues = make(map[string]struct{}, len(entries)) + for _, e := range entries { + allValues[path.Base(e)] = struct{}{} + } } - valueSet := make(map[string]struct{}, len(allValues)) - for _, v := range allValues { - if idx.caseInsensitive { - valueSet[strings.ToLower(path.Base(v))] = struct{}{} - } else { - valueSet[path.Base(v)] = struct{}{} + // convert requested values to set + valueSet := make(map[string]struct{}, len(values)) + if idx.caseInsensitive { + for _, v := range values { + valueSet[strings.ToLower(v)] = struct{}{} + } + } else { + for _, v := range values { + valueSet[v] = struct{}{} } } var matches = make([]string, 0) for v := range valueSet { - oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, v)) - if err != nil { - continue + if _, ok := allValues[v]; ok || len(allValues) == 0 { + oldname, err := idx.storage.ResolveSymlink(context.Background(), path.Join(idx.indexRootDir, v)) + if err != nil { + continue + } + matches = append(matches, oldname) } - matches = append(matches, oldname) } if len(matches) == 0 { From ba61d5800398c31b6a7476f930cb2167b139cf69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 8 Jul 2022 14:32:15 +0000 Subject: [PATCH 5/5] ignore result order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/utils/indexer/index/non_unique_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/storage/utils/indexer/index/non_unique_test.go b/pkg/storage/utils/indexer/index/non_unique_test.go index 7d3ebefd8f..69ebcfb8c7 100644 --- a/pkg/storage/utils/indexer/index/non_unique_test.go +++ b/pkg/storage/utils/indexer/index/non_unique_test.go @@ -37,7 +37,9 @@ func TestNonUniqueIndexAdd(t *testing.T) { ids, err := sut.Lookup("Green") assert.NoError(t, err) - assert.EqualValues(t, []string{"goefe-789", "xadaf-189"}, ids) + assert.Len(t, ids, 2) + assert.Contains(t, ids, "goefe-789") + assert.Contains(t, ids, "xadaf-189") ids, err = sut.Lookup("White") assert.NoError(t, err)