Move the children as well when a directory is moved

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)

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"))
 		})
 	})
 })

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)

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,

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

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