From eaba39639433e895936f03ad095d9a0289a97534 Mon Sep 17 00:00:00 2001 From: Andre Duffeck Date: Tue, 27 Sep 2022 10:39:42 +0200 Subject: [PATCH] Fix unfindable entities (#4651) * Fix unfindable entities from shares/publicshares Directories or empty files weren't added to the search index properly because they were indexed relative to the share instead of the space root. Fixes https://github.com/owncloud/ocis/issues/4489 * Add changelog * Fix tests * Reuse existing authorized context --- .../unreleased/fix-unfindable-entities.md | 8 ++++ services/search/pkg/search/provider/events.go | 44 +++++++++++-------- .../search/pkg/search/provider/events_test.go | 2 + .../pkg/search/provider/searchprovider.go | 26 +++++++++++ .../search/provider/searchprovider_test.go | 2 +- 5 files changed, 63 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/fix-unfindable-entities.md diff --git a/changelog/unreleased/fix-unfindable-entities.md b/changelog/unreleased/fix-unfindable-entities.md new file mode 100644 index 000000000..772cf5c47 --- /dev/null +++ b/changelog/unreleased/fix-unfindable-entities.md @@ -0,0 +1,8 @@ +Bugfix: Fix unfindable entities from shares/publicshares + +We fixed a problem where directories or empty files weren't findable because +they were to the search index improperly when created through a share or +publicshare. + +https://github.com/owncloud/ocis/pull/4651 +https://github.com/owncloud/ocis/issues/4489 diff --git a/services/search/pkg/search/provider/events.go b/services/search/pkg/search/provider/events.go index b68a13a41..525daf3e8 100644 --- a/services/search/pkg/search/provider/events.go +++ b/services/search/pkg/search/provider/events.go @@ -32,7 +32,11 @@ func (p *Provider) handleEvent(ev interface{}) { Id: e.Executant, } - statRes, err := p.statResource(ref, owner) + ownerCtx, err := p.getAuthContext(owner) + if err != nil { + return + } + statRes, err := p.statResource(ownerCtx, ref, owner) if err != nil { p.logger.Error().Err(err).Msg("failed to stat the changed resource") return @@ -56,7 +60,11 @@ func (p *Provider) handleEvent(ev interface{}) { Id: e.Executant, } - statRes, err := p.statResource(ref, owner) + ownerCtx, err := p.getAuthContext(owner) + if err != nil { + return + } + statRes, err := p.statResource(ownerCtx, ref, owner) if err != nil { p.logger.Error().Err(err).Msg("failed to stat the moved resource") return @@ -66,7 +74,7 @@ func (p *Provider) handleEvent(ev interface{}) { return } - gpRes, err := p.getPath(statRes.Info.Id, owner) + gpRes, err := p.getPath(ownerCtx, statRes.Info.Id, owner) if err != nil { p.logger.Error().Err(err).Interface("ref", ref).Msg("failed to get path for moved resource") return @@ -107,7 +115,12 @@ func (p *Provider) handleEvent(ev interface{}) { } p.logger.Debug().Interface("event", ev).Msg("resource has been changed, updating the document") - statRes, err := p.statResource(ref, owner) + ownerCtx, err := p.getAuthContext(owner) + if err != nil { + return + } + + statRes, err := p.statResource(ownerCtx, ref, owner) if err != nil { p.logger.Error().Err(err).Msg("failed to stat the changed resource") return @@ -117,6 +130,11 @@ func (p *Provider) handleEvent(ev interface{}) { return } + ref, err = p.resolveReference(ownerCtx, ref, statRes.Info) + if err != nil { + p.logger.Error().Err(err).Msg("error resolving reference") + return + } err = p.indexClient.Add(ref, statRes.Info) if err != nil { p.logger.Error().Err(err).Msg("error adding updating the resource in the index") @@ -125,22 +143,12 @@ func (p *Provider) handleEvent(ev interface{}) { } } -func (p *Provider) statResource(ref *provider.Reference, owner *user.User) (*provider.StatResponse, error) { - ownerCtx, err := p.getAuthContext(owner) - if err != nil { - return nil, err - } - - return p.gwClient.Stat(ownerCtx, &provider.StatRequest{Ref: ref}) +func (p *Provider) statResource(ctx context.Context, ref *provider.Reference, owner *user.User) (*provider.StatResponse, error) { + return p.gwClient.Stat(ctx, &provider.StatRequest{Ref: ref}) } -func (p *Provider) getPath(id *provider.ResourceId, owner *user.User) (*provider.GetPathResponse, error) { - ownerCtx, err := p.getAuthContext(owner) - if err != nil { - return nil, err - } - - return p.gwClient.GetPath(ownerCtx, &provider.GetPathRequest{ResourceId: id}) +func (p *Provider) getPath(ctx context.Context, id *provider.ResourceId, owner *user.User) (*provider.GetPathResponse, error) { + return p.gwClient.GetPath(ctx, &provider.GetPathRequest{ResourceId: id}) } func (p *Provider) getAuthContext(owner *user.User) (context.Context, error) { diff --git a/services/search/pkg/search/provider/events_test.go b/services/search/pkg/search/provider/events_test.go index 77f644ef4..7ee7f45f2 100644 --- a/services/search/pkg/search/provider/events_test.go +++ b/services/search/pkg/search/provider/events_test.go @@ -37,6 +37,7 @@ var _ = Describe("Searchprovider", func() { ref = &sprovider.Reference{ ResourceId: &sprovider.ResourceId{ StorageId: "storageid", + SpaceId: "rootopaqueid", OpaqueId: "rootopaqueid", }, Path: "./foo.pdf", @@ -44,6 +45,7 @@ var _ = Describe("Searchprovider", func() { ri = &sprovider.ResourceInfo{ Id: &sprovider.ResourceId{ StorageId: "storageid", + SpaceId: "rootopaqueid", OpaqueId: "opaqueid", }, Path: "foo.pdf", diff --git a/services/search/pkg/search/provider/searchprovider.go b/services/search/pkg/search/provider/searchprovider.go index bcdaab4cd..9e314a97f 100644 --- a/services/search/pkg/search/provider/searchprovider.go +++ b/services/search/pkg/search/provider/searchprovider.go @@ -282,6 +282,11 @@ func (p *Provider) IndexSpace(ctx context.Context, req *searchsvc.IndexSpaceRequ Path: utils.MakeRelativePath(filepath.Join(wd, info.Path)), ResourceId: &rootId, } + ref, err = p.resolveReference(ownerCtx, ref, info) + if err != nil { + p.logger.Error().Err(err).Msg("error resolving reference") + return nil + } err = p.indexClient.Add(ref, info) if err != nil { p.logger.Error().Err(err).Msg("error adding resource to the index") @@ -298,6 +303,27 @@ func (p *Provider) IndexSpace(ctx context.Context, req *searchsvc.IndexSpaceRequ return &searchsvc.IndexSpaceResponse{}, nil } +func (p *Provider) resolveReference(ctx context.Context, ref *provider.Reference, ri *provider.ResourceInfo) (*provider.Reference, error) { + if ref.GetResourceId().GetOpaqueId() == ref.GetResourceId().GetSpaceId() { + return ref, nil + } + + gpRes, err := p.gwClient.GetPath(ctx, &provider.GetPathRequest{ + ResourceId: ri.Id, + }) + if err != nil || gpRes.Status.Code != rpcv1beta1.Code_CODE_OK { + return nil, err + } + return &provider.Reference{ + ResourceId: &provider.ResourceId{ + StorageId: ref.GetResourceId().GetStorageId(), + SpaceId: ref.GetResourceId().GetSpaceId(), + OpaqueId: ref.GetResourceId().GetSpaceId(), + }, + Path: utils.MakeRelativePath(gpRes.Path), + }, nil +} + func (p *Provider) logDocCount() { c, err := p.indexClient.DocCount() if err != nil { diff --git a/services/search/pkg/search/provider/searchprovider_test.go b/services/search/pkg/search/provider/searchprovider_test.go index 8caa5ea72..933200ab4 100644 --- a/services/search/pkg/search/provider/searchprovider_test.go +++ b/services/search/pkg/search/provider/searchprovider_test.go @@ -107,7 +107,7 @@ var _ = Describe("Searchprovider", func() { })).Return(nil) res, err := p.IndexSpace(ctx, &searchsvc.IndexSpaceRequest{ - SpaceId: "storageid", + SpaceId: "storageid$spaceid!spaceid", UserId: "user", }) Expect(err).ToNot(HaveOccurred())