fix(search): keep trashed and live folders at the same path apart

A trashed folder keeps its path in the index, so a new folder can take
the same one. The descendant lookup behind move, delete, restore and
purge matched on space and path only: purging the trashed folder took
the live folder and its descendants out of the index, and moving the
live folder carried the trashed tree along.

Both engines now also match the root's trash state; purge with
onlyDeleted keeps matching the trashed ones only.

services/search/pkg/bleve/batch.go

@@ -56,7 +56,7 @@ func (b *Batch) Move(id, parentID, location string) error {
 	return b.withSizeLimit(func() error {
 		nextPath := utils.MakeRelativePath(location)
 		var currentPath string
-		return b.forSelfAndDescendants(id, func(resource *search.Resource) error {
+		return b.forSelfAndDescendants(id, false, func(resource *search.Resource) error {
 			if resource.ID == id {
 				currentPath = resource.Path
 				resource.Path = nextPath
@@ -84,7 +84,7 @@ func (b *Batch) Restore(id string) error {
 }
 
 func (b *Batch) setDeleted(id string, deleted bool) error {
-	return b.forSelfAndDescendants(id, func(resource *search.Resource) error {
+	return b.forSelfAndDescendants(id, false, func(resource *search.Resource) error {
 		resource.Deleted = deleted
 		return b.indexResource(resource.ID, *resource)
 	})
@@ -92,23 +92,23 @@ func (b *Batch) setDeleted(id string, deleted bool) error {
 
 func (b *Batch) Purge(id string, onlyDeleted bool) error {
 	return b.withSizeLimit(func() error {
-		return b.forSelfAndDescendants(id, func(resource *search.Resource) error {
-			if onlyDeleted && !resource.Deleted {
-				return nil
-			}
+		return b.forSelfAndDescendants(id, onlyDeleted, func(resource *search.Resource) error {
 			b.batch.Delete(resource.ID)
 			return nil
 		})
 	})
 }
 
-// fn sees the root first; the root's original path drives the descendant lookup
-func (b *Batch) forSelfAndDescendants(id string, fn func(*search.Resource) error) error {
+// fn sees the root first; the root's original path and trash state drive the
+// descendant lookup. 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, or the
+// trashed ones with onlyDeleted, which skips a live root as well.
+func (b *Batch) forSelfAndDescendants(id string, onlyDeleted bool, fn func(*search.Resource) error) error {
 	root, err := searchResourceByID(id, b.index)
 	if err != nil {
 		return err
 	}
-	rootID, rootPath := root.RootID, root.Path
+	rootID, rootPath, deleted := root.RootID, root.Path, root.Deleted || onlyDeleted
 	isContainer := root.Type == uint64(storageProvider.ResourceType_RESOURCE_TYPE_CONTAINER)
 
 	apply := func(resource *search.Resource) error {
@@ -121,13 +121,15 @@ func (b *Batch) forSelfAndDescendants(id string, fn func(*search.Resource) error
 		return nil
 	}
 
-	if err := apply(root); err != nil {
-		return err
+	if root.Deleted || !onlyDeleted {
+		if err := apply(root); err != nil {
+			return err
+		}
 	}
 	if !isContainer {
 		return nil
 	}
-	return forEachResourceByPath(rootID, rootPath, b.index, func(resource *search.Resource) error {
+	return forEachResourceByPath(rootID, rootPath, deleted, b.index, func(resource *search.Resource) error {
 		if resource.ID == id {
 			return nil
 		}

services/search/pkg/bleve/index.go

@@ -266,16 +266,19 @@ func searchResourceByID(id string, index bleve.Index) (*search.Resource, error)
 var descendantPageSize = 20_000
 
 // forEachResourceByPath streams the folder at lookupPath and its descendants
-// (the folder term matches both, see PathAnalyzer); paged by id so memory is
-// bounded by the page and fn may write to the index between pages
-func forEachResourceByPath(rootID string, lookupPath string, index bleve.Index, fn func(*search.Resource) error) error {
+// (the folder term matches both, see PathAnalyzer) whose Deleted flag equals
+// deleted; paged by id so memory is bounded by the page and fn may write to the
+// index between pages
+func forEachResourceByPath(rootID string, lookupPath string, deleted bool, index bleve.Index, fn func(*search.Resource) error) error {
 	rootQuery := bleve.NewTermQuery(rootID)
 	rootQuery.SetField("RootID")
 	pathQuery := bleve.NewTermQuery(lookupPath)
 	pathQuery.SetField("Path")
+	deletedQuery := bleve.NewBoolFieldQuery(deleted)
+	deletedQuery.SetField("Deleted")
 
 	pageSize := descendantPageSize
-	bleveReq := bleve.NewSearchRequest(bleve.NewConjunctionQuery(rootQuery, pathQuery))
+	bleveReq := bleve.NewSearchRequest(bleve.NewConjunctionQuery(rootQuery, pathQuery, deletedQuery))
 	bleveReq.Size = pageSize
 	bleveReq.Fields = []string{"*"}
 	bleveReq.SortBy([]string{"_id"})

services/search/pkg/opensearch/batch.go

@@ -168,14 +168,14 @@ func (b *Batch) Purge(id string, onlyDeleted bool) error {
 			return fmt.Errorf("failed to get resource: %w", err)
 		}
 
-		// scope to the resource's space: the same path exists in other spaces
+		// 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),
 		)
-		if onlyDeleted {
-			query.Must(osu.NewTermQuery[bool]("Deleted").Value(true))
-		}
 
 		req, err := osu.BuildDocumentDeleteByQueryReq(
 			opensearchgoAPI.DocumentDeleteByQueryReq{

services/search/pkg/opensearch/opensearch.go

@@ -57,9 +57,12 @@ func updateSelfAndDescendants(ctx context.Context, client *opensearchgoAPI.Clien
 				Refresh:           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),
 		},

services/search/pkg/parity/README.md

@@ -488,6 +488,10 @@ Fixtures:
 
 - `parent`, ID = 1$1!2, folder
 - `child.pdf`, ID = 1$1!3, Path = ./parent/child.pdf
+- `a`, ID = 1$1!trashed, folder, deleted
+- `x.txt`, ID = 1$1!trashed-x, Path = ./a/x.txt, deleted
+- `a`, ID = 1$1!live, folder
+- `y.txt`, ID = 1$1!live-y, Path = ./a/y.txt
 
 | Case | Query | expected | bleve | OpenSearch | same? |
 |---|---|---|---|---|---|
@@ -499,6 +503,8 @@ Fixtures:
 | PURGE-02 | removes the tree, then `DocCount()` | 0 | 0 | 0 | ✅ |
 | PURGE-03 | takes only the deleted ones when it is told to, then `name:"*parent*"` | parent | parent | parent | ✅ |
 | PURGE-03 | takes only the deleted ones when it is told to, then `name:"*child*"` | no match | no match | no match | ✅ |
+| PURGE-04 | leaves the live folder that took the trashed one's path alone, then `path:"./a"` | a, y.txt | a, y.txt | a, y.txt | ✅ |
+| PURGE-04 | leaves the live folder that took the trashed one's path alone, then `DocCount()` | 2 | 2 | 2 | ✅ |
 
 ### purgespace
 
@@ -535,6 +541,10 @@ Fixtures:
 - `x.txt`, ID = 1$1!6, Path = ./big2/x.txt
 - `odd name (1)`, ID = 1$1!7, folder
 - `f:x+y.txt`, ID = 1$1!8, Path = ./odd name (1)/f:x+y.txt
+- `a`, ID = 1$1!trashed, folder, deleted
+- `x.txt`, ID = 1$1!trashed-x, Path = ./a/x.txt, deleted
+- `a`, ID = 1$1!live, folder
+- `y.txt`, ID = 1$1!live-y, Path = ./a/y.txt
 
 | Case | Query | expected | bleve | OpenSearch | same? |
 |---|---|---|---|---|---|
@@ -546,6 +556,8 @@ Fixtures:
 | MOVE-03 | leaves a sibling folder that shares the prefix alone, then `path:"./big2"` | x.txt | x.txt | x.txt | ✅ |
 | MOVE-04 | carries the descendants of a path with special characters, then `path:"./odd name (2)"` | f:x+y.txt, odd name (2) | f:x+y.txt, odd name (2) | f:x+y.txt, odd name (2) | ✅ |
 | MOVE-04 | carries the descendants of a path with special characters, then `path:"./odd name (1)"` | no match | no match | no match | ✅ |
+| MOVE-05 | leaves a trashed folder that shares the path where it is, then `path:"./a"` | a, x.txt | a, x.txt | a, x.txt | ✅ |
+| MOVE-05 | leaves a trashed folder that shares the path where it is, then `path:"./b"` | b, y.txt | b, y.txt | b, y.txt | ✅ |
 
 ### rootscope
 

services/search/pkg/parity/fixtures_test.go

@@ -101,6 +101,20 @@ func fixtureTree() (parent, child search.Resource) {
 	return parent, child
 }
 
+// fixtureSamePath is a trashed folder and the live folder that took its path,
+// with a file in each.
+func fixtureSamePath() (trashed, live search.Resource, all []search.Resource) {
+	trashed = fixtureFolder("a", withID("1$1!trashed"), isDeleted())
+	live = fixtureFolder("a", withID("1$1!live"))
+
+	return trashed, live, []search.Resource{
+		trashed,
+		fixtureDoc("x.txt", withID("1$1!trashed-x"), withParent(trashed.ID), withPath("./a/x.txt"), isDeleted()),
+		live,
+		fixtureDoc("y.txt", withID("1$1!live-y"), withParent(live.ID), withPath("./a/y.txt")),
+	}
+}
+
 func treeIsLeft(names ...string) []expectation {
 	left := func(name string) []string {
 		if slices.Contains(names, name) {

services/search/pkg/parity/lifecycle_move_test.go

@@ -11,6 +11,7 @@ func moveLifecycle() lifecycleGroup {
 	inSibling := fixtureDoc("x.txt", withID("1$1!6"), withParent("1$1!big2"), withPath("./big2/x.txt"))
 	odd := fixtureFolder("odd name (1)", withID("1$1!7"))
 	inOdd := fixtureDoc("f:x+y.txt", withID("1$1!8"), withParent(odd.ID), withPath("./odd name (1)/f:x+y.txt"))
+	trashed, live, samePath := fixtureSamePath()
 
 	return lifecycleGroup{
 		name:     "move",
@@ -56,6 +57,22 @@ func moveLifecycle() lifecycleGroup {
 					{`path:"./odd name (1)"`, nil},
 				},
 			},
+			{
+				id: 5, title: "leaves a trashed folder that shares the path where it is",
+				fixtures: samePath,
+				do: func(e search.Engine) error {
+					if err := e.Move(live.ID, live.ParentID, "./b"); err != nil {
+						return err
+					}
+
+					// results only show the trashed folder once it is back
+					return e.Restore(trashed.ID)
+				},
+				expect: []expectation{
+					{`path:"./a"`, []string{"a", "x.txt"}},
+					{`path:"./b"`, []string{"b", "y.txt"}},
+				},
+			},
 		},
 	}
 }

services/search/pkg/parity/lifecycle_purge_test.go

@@ -7,6 +7,7 @@ import (
 
 func purgeLifecycle() lifecycleGroup {
 	parent, child := fixtureTree()
+	trashed, _, samePath := fixtureSamePath()
 
 	return lifecycleGroup{
 		name:     "purge",
@@ -35,6 +36,13 @@ func purgeLifecycle() lifecycleGroup {
 				},
 				expect: treeIsLeft("parent"),
 			},
+			{
+				id: 4, title: "leaves the live folder that took the trashed one's path alone",
+				fixtures:     samePath,
+				do:           func(e search.Engine) error { return e.Purge(trashed.ID, false) },
+				expect:       []expectation{{`path:"./a"`, []string{"a", "y.txt"}}},
+				wantDocCount: conversions.ToPointer(uint64(2)),
+			},
 		},
 	}
 }