diff --git a/services/search/pkg/search/service.go b/services/search/pkg/search/service.go index c77a3746a0..47c9161d33 100644 --- a/services/search/pkg/search/service.go +++ b/services/search/pkg/search/service.go @@ -46,6 +46,10 @@ const ( _spaceTypeProject = "project" _spaceTypeGrant = "grant" _slowQueryDuration = 500 * time.Millisecond + + // facetKeyPrefix is the arbitrary metadata namespace the facets are stored + // under. The indexer owns all of it and removes keys it no longer writes. + facetKeyPrefix = "libre.graph." ) // Searcher is the interface to the SearchService @@ -720,34 +724,57 @@ func (s *Service) doUpsertItem(ctx context.Context, ref *provider.Reference, bat // determine if metadata needs to be stored in storage as well metadata := map[string]string{} - facetToMetadata(metadata, doc.Audio, "libre.graph.audio.") - facetToMetadata(metadata, doc.Image, "libre.graph.image.") - facetToMetadata(metadata, doc.Location, "libre.graph.location.") - facetToMetadata(metadata, doc.Photo, "libre.graph.photo.") - facetToMetadata(metadata, doc.Video, "libre.graph.video.") - facetToMetadata(metadata, doc.MotionPhoto, "libre.graph.motionPhoto.") - facetToMetadata(metadata, doc.LivePhoto, "libre.graph.livePhoto.") - if len(metadata) == 0 { + facetToMetadata(metadata, doc.Audio, facetKeyPrefix+"audio.") + facetToMetadata(metadata, doc.Image, facetKeyPrefix+"image.") + facetToMetadata(metadata, doc.Location, facetKeyPrefix+"location.") + facetToMetadata(metadata, doc.Photo, facetKeyPrefix+"photo.") + facetToMetadata(metadata, doc.Video, facetKeyPrefix+"video.") + facetToMetadata(metadata, doc.MotionPhoto, facetKeyPrefix+"motionPhoto.") + facetToMetadata(metadata, doc.LivePhoto, facetKeyPrefix+"livePhoto.") + + // the facet keys are ours, so the ones this extraction did not produce + // describe content the resource no longer has + var stale []string + for k := range stat.GetInfo().GetArbitraryMetadata().GetMetadata() { + if _, ok := metadata[k]; !ok && strings.HasPrefix(k, facetKeyPrefix) { + stale = append(stale, k) + } + } + if len(metadata) == 0 && len(stale) == 0 { return } - s.logger.Trace().Str("name", doc.Name).Interface("metadata", metadata).Msg("Storing metadata") - gatewayClient, err := s.gatewaySelector.Next() if err != nil { s.logger.Error().Err(err).Msg("could not retrieve client to store metadata") return } - resp, err := gatewayClient.SetArbitraryMetadata(ctx, &provider.SetArbitraryMetadataRequest{ - Ref: ref, - ArbitraryMetadata: &provider.ArbitraryMetadata{ - Metadata: metadata, - }, - }) - if err != nil || resp.GetStatus().GetCode() != rpc.Code_CODE_OK { - s.logger.Error().Err(err).Int32("status", int32(resp.GetStatus().GetCode())).Msg("error storing metadata") - return + if len(metadata) > 0 { + s.logger.Trace().Str("name", doc.Name).Interface("metadata", metadata).Msg("Storing metadata") + + resp, err := gatewayClient.SetArbitraryMetadata(ctx, &provider.SetArbitraryMetadataRequest{ + Ref: ref, + ArbitraryMetadata: &provider.ArbitraryMetadata{ + Metadata: metadata, + }, + }) + if err != nil || resp.GetStatus().GetCode() != rpc.Code_CODE_OK { + s.logger.Error().Err(err).Int32("status", int32(resp.GetStatus().GetCode())).Msg("error storing metadata") + return + } + } + + if len(stale) > 0 { + s.logger.Trace().Str("name", doc.Name).Strs("keys", stale).Msg("Removing stale metadata") + + resp, err := gatewayClient.UnsetArbitraryMetadata(ctx, &provider.UnsetArbitraryMetadataRequest{ + Ref: ref, + ArbitraryMetadataKeys: stale, + }) + if err != nil || resp.GetStatus().GetCode() != rpc.Code_CODE_OK { + s.logger.Error().Err(err).Int32("status", int32(resp.GetStatus().GetCode())).Msg("error removing stale metadata") + } } } diff --git a/services/search/pkg/search/service_test.go b/services/search/pkg/search/service_test.go index 9a4ad01307..eecf3f986e 100644 --- a/services/search/pkg/search/service_test.go +++ b/services/search/pkg/search/service_test.go @@ -10,6 +10,7 @@ import ( typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + libregraph "github.com/opencloud-eu/libre-graph-api-go" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -175,6 +176,92 @@ var _ = Describe("Searchprovider", func() { Entry("./", "./", false), Entry("/", "/", false), ) + + Context("with facets stored on the resource", func() { + var ( + info *sprovider.ResourceInfo + calls []string + set map[string]string + unset []string + ) + + BeforeEach(func() { + calls, set, unset = nil, nil, nil + info = &sprovider.ResourceInfo{ + Id: &sprovider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "photo-opaqueid"}, + ParentId: &sprovider.ResourceId{StorageId: "storageid", OpaqueId: "parentopaqueid"}, + Path: "./photo.jpg", + ArbitraryMetadata: &sprovider.ArbitraryMetadata{ + Metadata: map[string]string{ + "libre.graph.photo.cameraMake": "Canon", + "libre.graph.photo.iso": "100", + "libre.graph.location.altitude": "12", + "http://owncloud.org/ns/favorite": "1", + }, + }, + } + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&sprovider.StatResponse{ + Status: status.NewOK(context.Background()), + Info: info, + }, nil) + gatewayClient.On("GetPath", mock.Anything, mock.MatchedBy(func(req *sprovider.GetPathRequest) bool { + return req.GetResourceId().GetOpaqueId() == info.GetId().GetOpaqueId() + })).Return(&sprovider.GetPathResponse{ + Status: status.NewOK(context.Background()), + Path: info.GetPath(), + }, nil) + indexClient.On("Upsert", mock.Anything, mock.Anything).Return(nil) + gatewayClient.On("SetArbitraryMetadata", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + calls = append(calls, "set") + set = args.Get(1).(*sprovider.SetArbitraryMetadataRequest).GetArbitraryMetadata().GetMetadata() + }).Return(&sprovider.SetArbitraryMetadataResponse{Status: status.NewOK(context.Background())}, nil) + gatewayClient.On("UnsetArbitraryMetadata", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + calls = append(calls, "unset") + unset = args.Get(1).(*sprovider.UnsetArbitraryMetadataRequest).GetArbitraryMetadataKeys() + }).Return(&sprovider.UnsetArbitraryMetadataResponse{Status: status.NewOK(context.Background())}, nil) + }) + + It("removes the facets the extraction no longer produces after storing the new ones", func() { + extractor.On("Extract", mock.Anything, mock.Anything, mock.Anything).Return(content.Document{ + Photo: &libregraph.Photo{CameraMake: libregraph.PtrString("Nikon")}, + }, nil) + + s.UpsertItem(&sprovider.Reference{ResourceId: info.GetId()}) + Expect(calls).To(Equal([]string{"set", "unset"})) + Expect(set).To(Equal(map[string]string{"libre.graph.photo.cameraMake": "Nikon"})) + Expect(unset).To(ConsistOf("libre.graph.photo.iso", "libre.graph.location.altitude")) + }) + + It("removes all facets when the extraction produces none", func() { + extractor.On("Extract", mock.Anything, mock.Anything, mock.Anything).Return(content.Document{}, nil) + + s.UpsertItem(&sprovider.Reference{ResourceId: info.GetId()}) + Expect(calls).To(Equal([]string{"unset"})) + Expect(unset).To(ConsistOf("libre.graph.photo.cameraMake", "libre.graph.photo.iso", "libre.graph.location.altitude")) + }) + + It("removes keys of facets the indexer no longer knows", func() { + info.ArbitraryMetadata.Metadata["libre.graph.retiredFacet.field"] = "value" + extractor.On("Extract", mock.Anything, mock.Anything, mock.Anything).Return(content.Document{ + Photo: &libregraph.Photo{CameraMake: libregraph.PtrString("Canon"), Iso: libregraph.PtrInt32(100)}, + Location: &libregraph.GeoCoordinates{Altitude: libregraph.PtrFloat64(12)}, + }, nil) + + s.UpsertItem(&sprovider.Reference{ResourceId: info.GetId()}) + Expect(calls).To(Equal([]string{"set", "unset"})) + Expect(unset).To(ConsistOf("libre.graph.retiredFacet.field")) + }) + + It("removes nothing when the extraction still produces every stored facet", func() { + extractor.On("Extract", mock.Anything, mock.Anything, mock.Anything).Return(content.Document{ + Photo: &libregraph.Photo{CameraMake: libregraph.PtrString("Canon"), Iso: libregraph.PtrInt32(100)}, + Location: &libregraph.GeoCoordinates{Altitude: libregraph.PtrFloat64(12)}, + }, nil) + + s.UpsertItem(&sprovider.Reference{ResourceId: info.GetId()}) + Expect(calls).To(Equal([]string{"set"})) + }) + }) }) DescribeTable("ExpandQueryTemplate",