From 18d0fa24162aaa67e1663e3bc1ad0b771fe4e488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Fri, 29 Apr 2022 12:47:34 +0200 Subject: [PATCH] Move the children as well when a directory is moved --- extensions/search/pkg/search/index/index.go | 97 +++++++++++-- .../search/pkg/search/index/index_test.go | 137 ++++++++---------- .../search/pkg/search/mocks/IndexClient.go | 14 ++ .../pkg/search/provider/searchprovider.go | 21 ++- .../search/provider/searchprovider_test.go | 2 +- extensions/search/pkg/search/search.go | 1 + 6 files changed, 181 insertions(+), 91 deletions(-) diff --git a/extensions/search/pkg/search/index/index.go b/extensions/search/pkg/search/index/index.go index 5a8984370..8cba7f7a6 100644 --- a/extensions/search/pkg/search/index/index.go +++ b/extensions/search/pkg/search/index/index.go @@ -21,6 +21,7 @@ package index import ( "context" "errors" + "path" "strings" "time" @@ -30,6 +31,7 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" sprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/v2/pkg/utils" searchmsg "github.com/owncloud/ocis/protogen/gen/ocis/messages/search/v0" searchsvc "github.com/owncloud/ocis/protogen/gen/ocis/services/search/v0" ) @@ -91,22 +93,17 @@ func (i *Index) Restore(id *sprovider.ResourceId) error { } func (i *Index) markAsDeleted(id string, deleted bool) error { - req := bleve.NewSearchRequest(bleve.NewDocIDQuery([]string{id})) - req.Fields = []string{"*"} - res, err := i.bleveIndex.Search(req) + doc, err := i.updateEntity(id, func(doc *indexDocument) { + doc.Deleted = deleted + }) if err != nil { return err } - if res.Hits.Len() == 0 { - return errors.New("entity not found") - } - entity := fieldsToEntity(res.Hits[0].Fields) - entity.Deleted = deleted - if entity.Type == uint64(sprovider.ResourceType_RESOURCE_TYPE_CONTAINER) { + if doc.Type == uint64(sprovider.ResourceType_RESOURCE_TYPE_CONTAINER) { query := bleve.NewConjunctionQuery( - bleve.NewQueryStringQuery("RootID:"+entity.RootID), - bleve.NewQueryStringQuery("Path:"+entity.Path+"/*"), + bleve.NewQueryStringQuery("RootID:"+doc.RootID), + bleve.NewQueryStringQuery("Path:"+doc.Path+"/*"), ) bleveReq := bleve.NewSearchRequest(query) bleveReq.Fields = []string{"*"} @@ -116,11 +113,43 @@ func (i *Index) markAsDeleted(id string, deleted bool) error { } for _, h := range res.Hits { - i.markAsDeleted(h.ID, deleted) + _, err := i.updateEntity(h.ID, func(doc *indexDocument) { + doc.Deleted = deleted + }) + if err != nil { + return err + } } } - return i.bleveIndex.Index(entity.ID, entity) + return nil +} + +func (i *Index) updateEntity(id string, mutateFunc func(doc *indexDocument)) (*indexDocument, error) { + doc, err := i.getEntity(id) + if err != nil { + return nil, err + } + mutateFunc(doc) + err = i.bleveIndex.Index(doc.ID, doc) + if err != nil { + return nil, err + } + + return doc, nil +} + +func (i *Index) getEntity(id string) (*indexDocument, error) { + req := bleve.NewSearchRequest(bleve.NewDocIDQuery([]string{id})) + req.Fields = []string{"*"} + res, err := i.bleveIndex.Search(req) + if err != nil { + return nil, err + } + if res.Hits.Len() == 0 { + return nil, errors.New("entity not found") + } + return fieldsToEntity(res.Hits[0].Fields), nil } // Purge removes an entity from the index @@ -128,6 +157,48 @@ func (i *Index) Purge(id *sprovider.ResourceId) error { return i.bleveIndex.Delete(idToBleveId(id)) } +// Purge removes an entity from the index +func (i *Index) Move(ri *sprovider.ResourceInfo) error { + doc, err := i.getEntity(idToBleveId(ri.Id)) + if err != nil { + return err + } + oldName := doc.Path + newName := utils.MakeRelativePath(ri.Path) + + doc, err = i.updateEntity(idToBleveId(ri.Id), func(doc *indexDocument) { + doc.Path = newName + doc.Name = path.Base(newName) + }) + if err != nil { + return err + } + + if doc.Type == uint64(sprovider.ResourceType_RESOURCE_TYPE_CONTAINER) { + query := bleve.NewConjunctionQuery( + bleve.NewQueryStringQuery("RootID:"+doc.RootID), + bleve.NewQueryStringQuery("Path:"+oldName+"/*"), + ) + bleveReq := bleve.NewSearchRequest(query) + bleveReq.Fields = []string{"*"} + res, err := i.bleveIndex.Search(bleveReq) + if err != nil { + return err + } + + for _, h := range res.Hits { + _, err := i.updateEntity(h.ID, func(doc *indexDocument) { + doc.Path = strings.Replace(doc.Path, oldName, newName, 1) + }) + if err != nil { + return err + } + } + } + + return nil +} + // Search searches the index according to the criteria specified in the given SearchIndexRequest func (i *Index) Search(ctx context.Context, req *searchsvc.SearchIndexRequest) (*searchsvc.SearchIndexResponse, error) { deletedQuery := bleve.NewBoolFieldQuery(false) diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index 503ea0675..8624e55a8 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -70,11 +70,25 @@ var _ = Describe("Index", func() { StorageId: "storageid", OpaqueId: "parentopaqueid", }, - Path: "subdir", + Path: "child.pdf", Size: 12345, Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, Mtime: &typesv1beta1.Timestamp{Seconds: 4000}, } + + assertDocCount = func(rootId *sprovider.ResourceId, query string, expectedCount int) { + res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ + Query: query, + Ref: &searchmsg.Reference{ + ResourceId: &searchmsg.ResourceID{ + StorageId: rootId.StorageId, + OpaqueId: rootId.OpaqueId, + }, + }, + }) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) + ExpectWithOffset(1, len(res.Matches)).To(Equal(expectedCount)) + } ) BeforeEach(func() { @@ -278,50 +292,70 @@ var _ = Describe("Index", func() { }) }) - Describe("Restore", func() { + Describe("Delete", func() { + It("marks a resource as deleted", func() { + err := i.Add(parentRef, parentRi) + Expect(err).ToNot(HaveOccurred()) + assertDocCount(rootId, "subdir", 1) + + err = i.Delete(parentRi.Id) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(rootId, "subdir", 0) + }) + It("also marks child resources as deleted", func() { err := i.Add(parentRef, parentRi) Expect(err).ToNot(HaveOccurred()) err = i.Add(childRef, childRi) Expect(err).ToNot(HaveOccurred()) + + assertDocCount(rootId, "subdir", 1) + assertDocCount(rootId, "child.pdf", 1) + + err = i.Delete(parentRi.Id) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(rootId, "subdir", 0) + assertDocCount(rootId, "child.pdf", 0) + }) + }) + + Describe("Restore", func() { + It("also marks child resources as restored", func() { + err := i.Add(parentRef, parentRi) + Expect(err).ToNot(HaveOccurred()) + err = i.Add(childRef, childRi) + Expect(err).ToNot(HaveOccurred()) err = i.Delete(parentRi.Id) Expect(err).ToNot(HaveOccurred()) - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(0)) + assertDocCount(rootId, "subdir", 0) + assertDocCount(rootId, "child.pdf", 0) err = i.Restore(parentRi.Id) Expect(err).ToNot(HaveOccurred()) - res, err = i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(2)) + assertDocCount(rootId, "subdir", 1) + assertDocCount(rootId, "child.pdf", 1) }) }) - Describe("Delete", func() { - It("marks a resource as deleted", func() { + Describe("Move", func() { + It("moves the parent and its child resources", func() { err := i.Add(parentRef, parentRi) Expect(err).ToNot(HaveOccurred()) + err = i.Add(childRef, childRi) + Expect(err).ToNot(HaveOccurred()) + + parentRi.Path = "newname" + err = i.Move(parentRi) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(rootId, "subdir", 0) + res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", + Query: "child.pdf", Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ StorageId: rootId.StorageId, @@ -331,54 +365,7 @@ var _ = Describe("Index", func() { }) Expect(err).ToNot(HaveOccurred()) Expect(len(res.Matches)).To(Equal(1)) - - err = i.Delete(parentRi.Id) - Expect(err).ToNot(HaveOccurred()) - - res, err = i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(0)) - }) - - It("also marks child resources as deleted", func() { - err := i.Add(parentRef, parentRi) - Expect(err).ToNot(HaveOccurred()) - err = i.Add(childRef, childRi) - Expect(err).ToNot(HaveOccurred()) - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(2)) - - err = i.Delete(parentRi.Id) - Expect(err).ToNot(HaveOccurred()) - - res, err = i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "subdir", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(0)) + Expect(res.Matches[0].Entity.Ref.Path).To(Equal("./newname/child.pdf")) }) }) }) diff --git a/extensions/search/pkg/search/mocks/IndexClient.go b/extensions/search/pkg/search/mocks/IndexClient.go index e33db661e..f87fd7264 100644 --- a/extensions/search/pkg/search/mocks/IndexClient.go +++ b/extensions/search/pkg/search/mocks/IndexClient.go @@ -65,6 +65,20 @@ func (_m *IndexClient) DocCount() (uint64, error) { return r0, r1 } +// Move provides a mock function with given fields: ri +func (_m *IndexClient) Move(ri *providerv1beta1.ResourceInfo) error { + ret := _m.Called(ri) + + var r0 error + if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceInfo) error); ok { + r0 = rf(ri) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Purge provides a mock function with given fields: ri func (_m *IndexClient) Purge(ri *providerv1beta1.ResourceId) error { ret := _m.Called(ri) diff --git a/extensions/search/pkg/search/provider/searchprovider.go b/extensions/search/pkg/search/provider/searchprovider.go index 4bb74342f..c7d05ef20 100644 --- a/extensions/search/pkg/search/provider/searchprovider.go +++ b/extensions/search/pkg/search/provider/searchprovider.go @@ -74,12 +74,29 @@ func New(gwClient gateway.GatewayAPIClient, indexClient search.IndexClient, mach } continue - case events.FileUploaded: + case events.ItemMoved: ref = e.Ref owner = &user.User{ Id: e.Executant, } - case events.ItemMoved: + + statRes, err := p.statResource(ref, owner) + if err != nil { + p.logger.Error().Err(err).Msg("failed to stat the changed resource") + } + + switch statRes.Status.Code { + case rpc.Code_CODE_OK: + err = p.indexClient.Move(statRes.Info) + if err != nil { + p.logger.Error().Err(err).Msg("failed to restore the changed resource in the index") + } + default: + p.logger.Error().Interface("statRes", statRes).Msg("failed to stat the changed resource") + } + + continue + case events.FileUploaded: ref = e.Ref owner = &user.User{ Id: e.Executant, diff --git a/extensions/search/pkg/search/provider/searchprovider_test.go b/extensions/search/pkg/search/provider/searchprovider_test.go index b989f1ce0..f6ea0de39 100644 --- a/extensions/search/pkg/search/provider/searchprovider_test.go +++ b/extensions/search/pkg/search/provider/searchprovider_test.go @@ -174,7 +174,7 @@ var _ = Describe("Searchprovider", func() { It("indexes items when they are being moved", func() { called := false - indexClient.On("Add", mock.Anything, mock.MatchedBy(func(riToIndex *sprovider.ResourceInfo) bool { + indexClient.On("Move", mock.Anything, mock.MatchedBy(func(riToIndex *sprovider.ResourceInfo) bool { return riToIndex.Id.OpaqueId == ri.Id.OpaqueId })).Return(nil).Run(func(args mock.Arguments) { called = true diff --git a/extensions/search/pkg/search/search.go b/extensions/search/pkg/search/search.go index cb9bcbbb8..14f0baf8c 100644 --- a/extensions/search/pkg/search/search.go +++ b/extensions/search/pkg/search/search.go @@ -38,6 +38,7 @@ type ProviderClient interface { type IndexClient interface { Search(ctx context.Context, req *searchsvc.SearchIndexRequest) (*searchsvc.SearchIndexResponse, error) Add(ref *providerv1beta1.Reference, ri *providerv1beta1.ResourceInfo) error + Move(ri *providerv1beta1.ResourceInfo) error Delete(ri *providerv1beta1.ResourceId) error Restore(ri *providerv1beta1.ResourceId) error Purge(ri *providerv1beta1.ResourceId) error