From badeb2e8417af659834529bed58a4e0b90444794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 12 Oct 2026 13:58:58 +0200 Subject: [PATCH 1/2] Keep trashed and live folders at the same path apart in the index A live folder can take the path of a trashed one, so only match the descendants in the root's trash state. Backport of #3602. --- services/search/pkg/bleve/backend_test.go | 81 +++++++++++++++++++ services/search/pkg/bleve/batch.go | 7 +- services/search/pkg/bleve/index.go | 10 ++- .../search/pkg/opensearch/backend_test.go | 51 ++++++++++++ services/search/pkg/opensearch/batch.go | 10 ++- services/search/pkg/opensearch/opensearch.go | 5 +- 6 files changed, 155 insertions(+), 9 deletions(-) diff --git a/services/search/pkg/bleve/backend_test.go b/services/search/pkg/bleve/backend_test.go index d530c5eeff..d6973a0ef5 100644 --- a/services/search/pkg/bleve/backend_test.go +++ b/services/search/pkg/bleve/backend_test.go @@ -56,6 +56,37 @@ var _ = Describe("Bleve", func() { parentResource search.Resource childResource search.Resource childResource2 search.Resource + + // samePath returns a trashed folder and the live folder that took its + // path, with a file in each + samePath = func() (trashed, trashedFile, live, liveFile search.Resource) { + folder := func(id string, deleted bool) search.Resource { + return search.Resource{ + ID: id, + ParentID: rootResource.ID, + RootID: rootResource.ID, + Path: "./a", + Type: uint64(sprovider.ResourceType_RESOURCE_TYPE_CONTAINER), + Deleted: deleted, + Document: content.Document{Name: "a"}, + } + } + file := func(id string, parent search.Resource, name string) search.Resource { + return search.Resource{ + ID: id, + ParentID: parent.ID, + RootID: rootResource.ID, + Path: parent.Path + "/" + name, + Type: uint64(sprovider.ResourceType_RESOURCE_TYPE_FILE), + Deleted: parent.Deleted, + Document: content.Document{Name: name}, + } + } + + trashed = folder("1$2!trashed", true) + live = folder("1$2!live", false) + return trashed, file("1$2!trashed-x", trashed, "x.txt"), live, file("1$2!live-y", live, "y.txt") + } ) BeforeEach(func() { @@ -557,6 +588,36 @@ var _ = Describe("Bleve", func() { assertDocCount(rootResource.ID, `"`+parentResource.Document.Name+`"`, 0) assertDocCount(rootResource.ID, `"`+childResource.Document.Name+`"`, 1) }) + It("removes only the deleted children of a live resource when told to", func() { + for _, resource := range []search.Resource{parentResource, childResource, childResource2} { + Expect(eng.Upsert(resource.ID, resource)).To(Succeed()) + } + Expect(eng.Delete(childResource.ID)).To(Succeed()) + + Expect(eng.Purge(parentResource.ID, true)).To(Succeed()) + + count, err := idx.DocCount() + Expect(err).ToNot(HaveOccurred()) + Expect(count).To(Equal(uint64(2))) + assertDocCount(rootResource.ID, `"`+parentResource.Document.Name+`"`, 1) + assertDocCount(rootResource.ID, "Name:child2.pdf", 1) + }) + It("leaves the live folder that took the trashed one's path alone", func() { + trashed, trashedFile, live, liveFile := samePath() + for _, resource := range []search.Resource{trashed, trashedFile, live, liveFile} { + Expect(eng.Upsert(resource.ID, resource)).To(Succeed()) + } + + Expect(eng.Purge(trashed.ID, false)).To(Succeed()) + + count, err := idx.DocCount() + Expect(err).ToNot(HaveOccurred()) + Expect(count).To(Equal(uint64(2))) + matches := assertDocCount(rootResource.ID, "Name:a", 1) + Expect(matches[0].Entity.Id.OpaqueId).To(Equal("live")) + matches = assertDocCount(rootResource.ID, "Name:y.txt", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal(liveFile.Path)) + }) }) Describe("Move", func() { @@ -601,6 +662,26 @@ var _ = Describe("Bleve", func() { Expect(matches[0].Entity.Ref.Path).To(Equal("./somewhere/else/newname")) }) + + It("leaves a trashed folder that shares the path where it is", func() { + trashed, trashedFile, live, liveFile := samePath() + for _, resource := range []search.Resource{trashed, trashedFile, live, liveFile} { + Expect(eng.Upsert(resource.ID, resource)).To(Succeed()) + } + + Expect(eng.Move(live.ID, live.ParentID, "./b")).To(Succeed()) + // results only show the trashed folder once it is back + Expect(eng.Restore(trashed.ID)).To(Succeed()) + + matches := assertDocCount(rootResource.ID, "Name:a", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal("./a")) + matches = assertDocCount(rootResource.ID, "Name:x.txt", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal(trashedFile.Path)) + matches = assertDocCount(rootResource.ID, "Name:b", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal("./b")) + matches = assertDocCount(rootResource.ID, "Name:y.txt", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal("./b/y.txt")) + }) }) Describe("StartBatch", func() { diff --git a/services/search/pkg/bleve/batch.go b/services/search/pkg/bleve/batch.go index 1db8c7640c..8d6977ae5f 100644 --- a/services/search/pkg/bleve/batch.go +++ b/services/search/pkg/bleve/batch.go @@ -56,7 +56,7 @@ func (b *Batch) Move(id string, parentID string, targetPath string) error { resources := []*search.Resource{rootResource} if rootResource.Type == uint64(storageProvider.ResourceType_RESOURCE_TYPE_CONTAINER) { - descendantResources, err := searchResourcesByPath(rootResource.RootID, currentPath, b.index) + descendantResources, err := searchResourcesByPath(rootResource.RootID, currentPath, rootResource.Deleted, b.index) if err != nil { return err } @@ -145,7 +145,10 @@ func (b *Batch) Purge(id string, onlyDeleted bool) error { add(rootResource) if rootResource.Type == uint64(storageProvider.ResourceType_RESOURCE_TYPE_CONTAINER) { - descendantResources, err := searchResourcesByPath(rootResource.RootID, rootResource.Path, b.index) + // a trashed folder keeps its path, so a live folder can take the same one: + // the descendants are the ones in the root's trash state (onlyDeleted takes + // the trashed ones only) + descendantResources, err := searchResourcesByPath(rootResource.RootID, rootResource.Path, rootResource.Deleted || onlyDeleted, b.index) if err != nil { return err } diff --git a/services/search/pkg/bleve/index.go b/services/search/pkg/bleve/index.go index fea6986ae0..465f96ae51 100644 --- a/services/search/pkg/bleve/index.go +++ b/services/search/pkg/bleve/index.go @@ -13,6 +13,7 @@ import ( "github.com/blevesearch/bleve/v2/analysis/tokenizer/single" "github.com/blevesearch/bleve/v2/analysis/tokenizer/unicode" "github.com/blevesearch/bleve/v2/mapping" + "github.com/blevesearch/bleve/v2/search/query" storageProvider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/opencloud-eu/opencloud/services/search/pkg/search" @@ -102,10 +103,13 @@ func searchResourceByID(id string, index bleve.Index) (*search.Resource, error) return matchToResource(res.Hits[0]), nil } -func searchResourcesByPath(rootID string, lookupPath string, index bleve.Index) ([]*search.Resource, error) { +// searchResourcesByPath returns the descendants of lookupPath in the given trash +// state: a trashed folder keeps its path, so a live folder can take the same one +func searchResourcesByPath(rootID string, lookupPath string, deleted bool, index bleve.Index) ([]*search.Resource, error) { q := bleve.NewConjunctionQuery( bleve.NewQueryStringQuery("RootID:"+rootID), bleve.NewQueryStringQuery("Path:"+escapeQuery(lookupPath+"/*")), + &query.BoolFieldQuery{FieldVal: "Deleted", Bool: deleted}, ) bleveReq := bleve.NewSearchRequest(q) bleveReq.Size = math.MaxInt @@ -128,12 +132,14 @@ func searchAndUpdateResourcesDeletionState(id string, state bool, index bleve.In if err != nil { return nil, err } + // the descendants are the ones in the root's trash state from before the change + wasDeleted := rootResource.Deleted rootResource.Deleted = state resources := []*search.Resource{rootResource} if rootResource.Type == uint64(storageProvider.ResourceType_RESOURCE_TYPE_CONTAINER) { - descendantResources, err := searchResourcesByPath(rootResource.RootID, rootResource.Path, index) + descendantResources, err := searchResourcesByPath(rootResource.RootID, rootResource.Path, wasDeleted, index) if err != nil { return nil, err } diff --git a/services/search/pkg/opensearch/backend_test.go b/services/search/pkg/opensearch/backend_test.go index a8579180ac..e22e7d787e 100644 --- a/services/search/pkg/opensearch/backend_test.go +++ b/services/search/pkg/opensearch/backend_test.go @@ -56,6 +56,25 @@ func otherRoot(r search.Resource) search.Resource { return r } +// samePath returns a trashed folder and the live folder that took its path, +// with a file in each. +func samePath() (trashed, trashedFile, live, liveFile search.Resource) { + folder := opensearchtest.Testdata.Resources.Folder + file := opensearchtest.Testdata.Resources.File + + trashed = folder + trashed.ID, trashed.Path, trashed.Name, trashed.Deleted = "1$1!trashed", "./a", "a", true + trashedFile = file + trashedFile.ID, trashedFile.ParentID, trashedFile.Path, trashedFile.Name, trashedFile.Deleted = "1$1!trashed-x", trashed.ID, "./a/x.txt", "x.txt", true + + live = folder + live.ID, live.Path, live.Name = "1$1!live", "./a", "a" + liveFile = file + liveFile.ID, liveFile.ParentID, liveFile.Path, liveFile.Name = "1$1!live-y", live.ID, "./a/y.txt", "y.txt" + + return trashed, trashedFile, live, liveFile +} + func newBackend(indexName string, resources ...search.Resource) (*opensearch.Backend, *opensearchtest.TestClient) { GinkgoHelper() @@ -212,6 +231,24 @@ var _ = Describe("Backend", func() { Expect(resources).To(HaveLen(1)) Expect(resources[0].Path).To(Equal(document.Path)) }) + + It("leaves a trashed folder that shares the path where it is", func() { + trashed, trashedFile, live, liveFile := samePath() + for _, r := range []search.Resource{trashed, trashedFile, live, liveFile} { + tc.Require.DocumentCreate(indexName, r.ID, strings.NewReader(opensearchtest.JSONMustMarshal(GinkgoTB(), r))) + } + tc.Require.IndicesCount([]string{indexName}, nil, 4) + + Expect(backend.Move(live.ID, live.ParentID, "./b")).To(Succeed()) + tc.Require.IndicesRefresh([]string{indexName}, nil) + Expect(backend.Restore(trashed.ID)).To(Succeed()) + + Expect(resourceByID(tc, indexName, trashed.ID).Path).To(Equal("./a")) + Expect(resourceByID(tc, indexName, trashedFile.ID).Path).To(Equal("./a/x.txt")) + Expect(resourceByID(tc, indexName, trashedFile.ID).Deleted).To(BeFalse(), "the restore takes the trashed folder's tree along") + Expect(resourceByID(tc, indexName, live.ID).Path).To(Equal("./b")) + Expect(resourceByID(tc, indexName, liveFile.ID).Path).To(Equal("./b/y.txt")) + }) }) Describe("Delete", func() { @@ -355,6 +392,20 @@ var _ = Describe("Backend", func() { tc.Require.IndicesCount([]string{indexName}, nil, 1) }) + + It("leaves the live folder that took the trashed one's path alone", func() { + trashed, trashedFile, live, liveFile := samePath() + for _, r := range []search.Resource{trashed, trashedFile, live, liveFile} { + tc.Require.DocumentCreate(indexName, r.ID, strings.NewReader(opensearchtest.JSONMustMarshal(GinkgoTB(), r))) + } + tc.Require.IndicesCount([]string{indexName}, nil, 4) + + Expect(backend.Purge(trashed.ID, false)).To(Succeed()) + + tc.Require.IndicesCount([]string{indexName}, nil, 2) + Expect(resourceByID(tc, indexName, live.ID).Path).To(Equal("./a")) + Expect(resourceByID(tc, indexName, liveFile.ID).Path).To(Equal("./a/y.txt")) + }) }) Describe("PurgeSpace", func() { diff --git a/services/search/pkg/opensearch/batch.go b/services/search/pkg/opensearch/batch.go index 250142f60f..143940fe54 100644 --- a/services/search/pkg/opensearch/batch.go +++ b/services/search/pkg/opensearch/batch.go @@ -143,10 +143,12 @@ func (b *Batch) Purge(id string, onlyDeleted bool) error { return fmt.Errorf("failed to get resource: %w", err) } - query := osu.NewBoolQuery().Must(osu.NewTermQuery[string]("Path").Value(resource.Path)) - if onlyDeleted { - query.Must(osu.NewTermQuery[bool]("Deleted").Value(true)) - } + // a trashed folder keeps its path, so a live folder can take the same one: + // scope to the resource's trash state (onlyDeleted takes the trashed ones only) + query := osu.NewBoolQuery().Must( + osu.NewTermQuery[string]("Path").Value(resource.Path), + osu.NewTermQuery[bool]("Deleted").Value(resource.Deleted || onlyDeleted), + ) req, err := osu.BuildDocumentDeleteByQueryReq( opensearchgoAPI.DocumentDeleteByQueryReq{ diff --git a/services/search/pkg/opensearch/opensearch.go b/services/search/pkg/opensearch/opensearch.go index c2dd7c1e6b..59bc11e02a 100644 --- a/services/search/pkg/opensearch/opensearch.go +++ b/services/search/pkg/opensearch/opensearch.go @@ -56,9 +56,12 @@ func updateSelfAndDescendants(ctx context.Context, client *opensearchgoAPI.Clien WaitForCompletion: conversions.ToPointer(true), }, }, + // a trashed folder keeps its path, so a live folder can take the same one: + // the descendants are the ones in the resource's trash state osu.NewBoolQuery(). Must(osu.NewTermQuery[string]("RootID").Value(resource.RootID)). - Must(osu.NewTermQuery[string]("Path").Value(resource.Path)), + Must(osu.NewTermQuery[string]("Path").Value(resource.Path)). + Must(osu.NewTermQuery[bool]("Deleted").Value(resource.Deleted)), osu.UpdateByQueryBodyParams{ Script: scriptProvider(resource), }, From 1b059af5c6ef15987da74faaf252ec2e092163be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Mon, 12 Oct 2026 14:11:55 +0200 Subject: [PATCH 2/2] Scope the opensearch purge to the resource's space Backport of d4f1ae910c. --- services/search/pkg/opensearch/backend_test.go | 14 ++++++++++++++ services/search/pkg/opensearch/batch.go | 6 ++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/services/search/pkg/opensearch/backend_test.go b/services/search/pkg/opensearch/backend_test.go index e22e7d787e..a22da152b8 100644 --- a/services/search/pkg/opensearch/backend_test.go +++ b/services/search/pkg/opensearch/backend_test.go @@ -406,6 +406,20 @@ var _ = Describe("Backend", func() { Expect(resourceByID(tc, indexName, live.ID).Path).To(Equal("./a")) Expect(resourceByID(tc, indexName, liveFile.ID).Path).To(Equal("./a/y.txt")) }) + + It("purges only the resource in the target root", func() { + target := opensearchtest.Testdata.Resources.File + other := otherRoot(target) + for _, r := range []search.Resource{target, other} { + tc.Require.DocumentCreate(indexName, r.ID, strings.NewReader(opensearchtest.JSONMustMarshal(GinkgoTB(), r))) + } + tc.Require.IndicesCount([]string{indexName}, nil, 2) + + Expect(backend.Purge(target.ID, false)).To(Succeed()) + + tc.Require.IndicesCount([]string{indexName}, nil, 1) + Expect(resourceByID(tc, indexName, other.ID).Path).To(Equal(other.Path), "resource in a different root must not be purged") + }) }) Describe("PurgeSpace", func() { diff --git a/services/search/pkg/opensearch/batch.go b/services/search/pkg/opensearch/batch.go index 143940fe54..60364001f4 100644 --- a/services/search/pkg/opensearch/batch.go +++ b/services/search/pkg/opensearch/batch.go @@ -143,9 +143,11 @@ func (b *Batch) Purge(id string, onlyDeleted bool) error { return fmt.Errorf("failed to get resource: %w", err) } - // a trashed folder keeps its path, so a live folder can take the same one: - // scope to the resource's trash state (onlyDeleted takes the trashed ones only) + // scope to the resource's space: the same path exists in other spaces; + // and to its trash state: a trashed folder keeps its path, so a live + // folder can take the same one (onlyDeleted takes the trashed ones only) query := osu.NewBoolQuery().Must( + osu.NewTermQuery[string]("RootID").Value(resource.RootID), osu.NewTermQuery[string]("Path").Value(resource.Path), osu.NewTermQuery[bool]("Deleted").Value(resource.Deleted || onlyDeleted), )