From 4e378be0203a529a17d28bf798bda7e8fb0a5808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 11 May 2022 11:14:10 +0200 Subject: [PATCH 1/9] Improve signature of index.Move --- extensions/search/pkg/search/index/index.go | 7 ++-- .../search/pkg/search/index/index_test.go | 2 +- .../search/pkg/search/mocks/IndexClient.go | 34 +++++++++---------- .../search/pkg/search/provider/events.go | 2 +- .../search/pkg/search/provider/events_test.go | 4 +-- extensions/search/pkg/search/search.go | 8 ++--- 6 files changed, 29 insertions(+), 28 deletions(-) diff --git a/extensions/search/pkg/search/index/index.go b/extensions/search/pkg/search/index/index.go index bc06ba466..338742f19 100644 --- a/extensions/search/pkg/search/index/index.go +++ b/extensions/search/pkg/search/index/index.go @@ -165,15 +165,16 @@ func (i *Index) Purge(id *sprovider.ResourceId) error { } // Move update the path of an entry and all its children -func (i *Index) Move(ri *sprovider.ResourceInfo, fullPath string) error { - doc, err := i.getEntity(idToBleveId(ri.Id)) +func (i *Index) Move(id *sprovider.ResourceId, fullPath string) error { + bleveId := idToBleveId(id) + doc, err := i.getEntity(bleveId) if err != nil { return err } oldName := doc.Path newName := utils.MakeRelativePath(fullPath) - doc, err = i.updateEntity(idToBleveId(ri.Id), func(doc *indexDocument) { + doc, err = i.updateEntity(bleveId, func(doc *indexDocument) { doc.Path = newName doc.Name = path.Base(newName) }) diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index 7c46c64b2..e3e4b43e6 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -366,7 +366,7 @@ var _ = Describe("Index", func() { Expect(err).ToNot(HaveOccurred()) parentRi.Path = "newname" - err = i.Move(parentRi, "./somewhere/else/newname") + err = i.Move(parentRi.Id, "./somewhere/else/newname") Expect(err).ToNot(HaveOccurred()) assertDocCount(rootId, "subdir", 0) diff --git a/extensions/search/pkg/search/mocks/IndexClient.go b/extensions/search/pkg/search/mocks/IndexClient.go index f43f9f3a9..d2286517a 100644 --- a/extensions/search/pkg/search/mocks/IndexClient.go +++ b/extensions/search/pkg/search/mocks/IndexClient.go @@ -30,13 +30,13 @@ func (_m *IndexClient) Add(ref *providerv1beta1.Reference, ri *providerv1beta1.R return r0 } -// Delete provides a mock function with given fields: ri -func (_m *IndexClient) Delete(ri *providerv1beta1.ResourceId) error { - ret := _m.Called(ri) +// Delete provides a mock function with given fields: id +func (_m *IndexClient) Delete(id *providerv1beta1.ResourceId) error { + ret := _m.Called(id) var r0 error if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceId) error); ok { - r0 = rf(ri) + r0 = rf(id) } else { r0 = ret.Error(0) } @@ -65,13 +65,13 @@ func (_m *IndexClient) DocCount() (uint64, error) { return r0, r1 } -// Move provides a mock function with given fields: ri, path -func (_m *IndexClient) Move(ri *providerv1beta1.ResourceInfo, path string) error { - ret := _m.Called(ri, path) +// Move provides a mock function with given fields: id, fullPath +func (_m *IndexClient) Move(id *providerv1beta1.ResourceId, fullPath string) error { + ret := _m.Called(id, fullPath) var r0 error - if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceInfo, string) error); ok { - r0 = rf(ri, path) + if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceId, string) error); ok { + r0 = rf(id, fullPath) } else { r0 = ret.Error(0) } @@ -79,13 +79,13 @@ func (_m *IndexClient) Move(ri *providerv1beta1.ResourceInfo, path string) error return r0 } -// Purge provides a mock function with given fields: ri -func (_m *IndexClient) Purge(ri *providerv1beta1.ResourceId) error { - ret := _m.Called(ri) +// Purge provides a mock function with given fields: id +func (_m *IndexClient) Purge(id *providerv1beta1.ResourceId) error { + ret := _m.Called(id) var r0 error if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceId) error); ok { - r0 = rf(ri) + r0 = rf(id) } else { r0 = ret.Error(0) } @@ -93,13 +93,13 @@ func (_m *IndexClient) Purge(ri *providerv1beta1.ResourceId) error { return r0 } -// Restore provides a mock function with given fields: ri -func (_m *IndexClient) Restore(ri *providerv1beta1.ResourceId) error { - ret := _m.Called(ri) +// Restore provides a mock function with given fields: id +func (_m *IndexClient) Restore(id *providerv1beta1.ResourceId) error { + ret := _m.Called(id) var r0 error if rf, ok := ret.Get(0).(func(*providerv1beta1.ResourceId) error); ok { - r0 = rf(ri) + r0 = rf(id) } else { r0 = ret.Error(0) } diff --git a/extensions/search/pkg/search/provider/events.go b/extensions/search/pkg/search/provider/events.go index d243c5237..2c9e5ec47 100644 --- a/extensions/search/pkg/search/provider/events.go +++ b/extensions/search/pkg/search/provider/events.go @@ -76,7 +76,7 @@ func (p *Provider) handleEvent(ev interface{}) { return } - err = p.indexClient.Move(statRes.Info, gpRes.Path) + err = p.indexClient.Move(statRes.Info.Id, gpRes.Path) if err != nil { p.logger.Error().Err(err).Msg("failed to restore the changed resource in the index") } diff --git a/extensions/search/pkg/search/provider/events_test.go b/extensions/search/pkg/search/provider/events_test.go index 9de17c5df..a36b36aef 100644 --- a/extensions/search/pkg/search/provider/events_test.go +++ b/extensions/search/pkg/search/provider/events_test.go @@ -156,8 +156,8 @@ var _ = Describe("Searchprovider", func() { Status: status.NewOK(ctx), Path: "./new/path.pdf", }, nil) - indexClient.On("Move", mock.MatchedBy(func(riToIndex *sprovider.ResourceInfo) bool { - return riToIndex.Id.OpaqueId == ri.Id.OpaqueId + indexClient.On("Move", mock.MatchedBy(func(id *sprovider.ResourceId) bool { + return id.OpaqueId == ri.Id.OpaqueId }), "./new/path.pdf").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 12df20342..0a8bedd26 100644 --- a/extensions/search/pkg/search/search.go +++ b/extensions/search/pkg/search/search.go @@ -38,9 +38,9 @@ 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, path string) error - Delete(ri *providerv1beta1.ResourceId) error - Restore(ri *providerv1beta1.ResourceId) error - Purge(ri *providerv1beta1.ResourceId) error + Move(id *providerv1beta1.ResourceId, fullPath string) error + Delete(id *providerv1beta1.ResourceId) error + Restore(id *providerv1beta1.ResourceId) error + Purge(id *providerv1beta1.ResourceId) error DocCount() (uint64, error) } From 08e242f1646ce262b7a26bb3765b64d420153892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Wed, 11 May 2022 11:14:59 +0200 Subject: [PATCH 2/9] Remove superfluous comments --- extensions/search/pkg/search/provider/events.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/search/pkg/search/provider/events.go b/extensions/search/pkg/search/provider/events.go index 2c9e5ec47..8229dd5ac 100644 --- a/extensions/search/pkg/search/provider/events.go +++ b/extensions/search/pkg/search/provider/events.go @@ -126,7 +126,6 @@ func (p *Provider) statResource(ref *provider.Reference, owner *user.User) (*pro return nil, err } - // Stat changed resource resource return p.gwClient.Stat(ownerCtx, &provider.StatRequest{Ref: ref}) } @@ -136,7 +135,6 @@ func (p *Provider) getPath(id *provider.ResourceId, owner *user.User) (*provider return nil, err } - // Stat changed resource resource return p.gwClient.GetPath(ownerCtx, &provider.GetPathRequest{ResourceId: id}) } From ae68d5d52209dd20edf33ff7daf7199c73dc920d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 09:11:11 +0200 Subject: [PATCH 3/9] Improve logging --- extensions/search/pkg/search/provider/events.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/search/pkg/search/provider/events.go b/extensions/search/pkg/search/provider/events.go index 8229dd5ac..641c3010a 100644 --- a/extensions/search/pkg/search/provider/events.go +++ b/extensions/search/pkg/search/provider/events.go @@ -58,11 +58,11 @@ func (p *Provider) handleEvent(ev interface{}) { statRes, err := p.statResource(ref, owner) if err != nil { - p.logger.Error().Err(err).Msg("failed to stat the changed resource") + p.logger.Error().Err(err).Msg("failed to stat the moved resource") return } if statRes.Status.Code != rpc.Code_CODE_OK { - p.logger.Error().Interface("statRes", statRes).Msg("failed to stat the changed resource") + p.logger.Error().Interface("statRes", statRes).Msg("failed to stat the moved resource") return } @@ -78,7 +78,7 @@ func (p *Provider) handleEvent(ev interface{}) { err = p.indexClient.Move(statRes.Info.Id, gpRes.Path) if err != nil { - p.logger.Error().Err(err).Msg("failed to restore the changed resource in the index") + p.logger.Error().Err(err).Msg("failed to move the changed resource in the index") } return case events.ContainerCreated: From 22f5e471bda9405fcb90beadc89d5f6ca311e083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 09:27:02 +0200 Subject: [PATCH 4/9] Change the index query string to contain the field name This increases flexibility when searching the index --- extensions/search/pkg/search/index/index.go | 2 +- .../search/pkg/search/index/index_test.go | 31 +++++++++++++------ .../pkg/search/provider/searchprovider.go | 2 +- .../search/provider/searchprovider_test.go | 6 ++-- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/extensions/search/pkg/search/index/index.go b/extensions/search/pkg/search/index/index.go index 338742f19..07797f7d4 100644 --- a/extensions/search/pkg/search/index/index.go +++ b/extensions/search/pkg/search/index/index.go @@ -212,7 +212,7 @@ func (i *Index) Search(ctx context.Context, req *searchsvc.SearchIndexRequest) ( deletedQuery := bleve.NewBoolFieldQuery(false) deletedQuery.SetField("Deleted") query := bleve.NewConjunctionQuery( - bleve.NewQueryStringQuery("Name:"+strings.ToLower(req.Query)), + bleve.NewQueryStringQuery(req.Query), deletedQuery, // Skip documents that have been marked as deleted bleve.NewQueryStringQuery("RootID:"+req.Ref.ResourceId.StorageId+"!"+req.Ref.ResourceId.OpaqueId), // Limit search to the space bleve.NewQueryStringQuery("Path:"+utils.MakeRelativePath(path.Join(req.Ref.Path, "/"))+"*"), // Limit search to this directory in the space diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index e3e4b43e6..34f50257d 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -26,7 +26,7 @@ var _ = Describe("Index", func() { } ref = &sprovider.Reference{ ResourceId: rootId, - Path: "./foo.pdf", + Path: "./Foo.pdf", } ri = &sprovider.ResourceInfo{ Id: &sprovider.ResourceId{ @@ -133,14 +133,14 @@ var _ = Describe("Index", func() { OpaqueId: "differentopaqueid", }, }, - Query: "foo.pdf", + Query: "Name:foo.pdf", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) Expect(len(res.Matches)).To(Equal(0)) }) - It("limits the search to the relevant fields", func() { + It("limits the search to the specified fields", func() { res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ @@ -148,7 +148,7 @@ var _ = Describe("Index", func() { OpaqueId: ref.ResourceId.OpaqueId, }, }, - Query: "*" + ref.ResourceId.OpaqueId + "*", + Query: "Name:*" + ref.ResourceId.OpaqueId + "*", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) @@ -163,7 +163,7 @@ var _ = Describe("Index", func() { OpaqueId: ref.ResourceId.OpaqueId, }, }, - Query: "foo.pdf", + Query: "Name:foo.pdf", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) @@ -203,7 +203,7 @@ var _ = Describe("Index", func() { } }) - It("is case-insensitive", func() { + It("uses a lower-case index", func() { res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ @@ -211,11 +211,24 @@ var _ = Describe("Index", func() { OpaqueId: ref.ResourceId.OpaqueId, }, }, - Query: "Foo*", + Query: "Name:foo*", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) Expect(len(res.Matches)).To(Equal(1)) + + res, err = i.Search(ctx, &searchsvc.SearchIndexRequest{ + Ref: &searchmsg.Reference{ + ResourceId: &searchmsg.ResourceID{ + StorageId: ref.ResourceId.StorageId, + OpaqueId: ref.ResourceId.OpaqueId, + }, + }, + Query: "Name:Foo*", + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(0)) }) Context("and an additional file in a subdirectory", func() { @@ -271,7 +284,7 @@ var _ = Describe("Index", func() { }, Path: "./nested/", }, - Query: "foo.pdf", + Query: "Name:foo.pdf", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) @@ -372,7 +385,7 @@ var _ = Describe("Index", func() { assertDocCount(rootId, "subdir", 0) res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "child.pdf", + Query: "Name:child.pdf", Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ StorageId: rootId.StorageId, diff --git a/extensions/search/pkg/search/provider/searchprovider.go b/extensions/search/pkg/search/provider/searchprovider.go index 940405635..a9014aa87 100644 --- a/extensions/search/pkg/search/provider/searchprovider.go +++ b/extensions/search/pkg/search/provider/searchprovider.go @@ -125,7 +125,7 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s _, rootStorageID := storagespace.SplitStorageID(space.Root.StorageId) res, err := p.indexClient.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: req.Query, + Query: "Name:" + strings.ToLower(req.Query), Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ StorageId: space.Root.StorageId, diff --git a/extensions/search/pkg/search/provider/searchprovider_test.go b/extensions/search/pkg/search/provider/searchprovider_test.go index 37f59ff1c..562a4ee06 100644 --- a/extensions/search/pkg/search/provider/searchprovider_test.go +++ b/extensions/search/pkg/search/provider/searchprovider_test.go @@ -160,7 +160,7 @@ var _ = Describe("Searchprovider", func() { Expect(match.Entity.Ref.Path).To(Equal("./path/to/Foo.pdf")) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == "foo" && req.Ref.ResourceId.OpaqueId == personalSpace.Root.OpaqueId && req.Ref.Path == "" + return req.Query == "Name:foo" && req.Ref.ResourceId.OpaqueId == personalSpace.Root.OpaqueId && req.Ref.Path == "" })) }) }) @@ -225,7 +225,7 @@ var _ = Describe("Searchprovider", func() { }, nil) res, err := p.Search(ctx, &searchsvc.SearchRequest{ - Query: "foo", + Query: "Foo", }) Expect(err).ToNot(HaveOccurred()) Expect(res).ToNot(BeNil()) @@ -237,7 +237,7 @@ var _ = Describe("Searchprovider", func() { Expect(match.Entity.Ref.Path).To(Equal("./to/Shared.pdf")) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == "foo" && req.Ref.ResourceId.StorageId == grantSpace.Root.StorageId && req.Ref.Path == "./grant/path" + return req.Query == "Name:foo" && req.Ref.ResourceId.StorageId == grantSpace.Root.StorageId && req.Ref.Path == "./grant/path" })) }) From 49da927f253cc6eb2861350c15571cfccd1d50ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 09:47:30 +0200 Subject: [PATCH 5/9] Fix search for files with spaces --- .../search/pkg/search/index/index_test.go | 72 +++++++++++++------ .../pkg/search/provider/searchprovider.go | 2 +- .../search/provider/searchprovider_test.go | 18 +++++ 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index 34f50257d..1549c2488 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -24,25 +24,9 @@ var _ = Describe("Index", func() { StorageId: "storageid", OpaqueId: "rootopaqueid", } - ref = &sprovider.Reference{ - ResourceId: rootId, - Path: "./Foo.pdf", - } - ri = &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{ - StorageId: "storageid", - OpaqueId: "opaqueid", - }, - ParentId: &sprovider.ResourceId{ - StorageId: "storageid", - OpaqueId: "someopaqueid", - }, - Path: "Foo.pdf", - Size: 12345, - Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, - MimeType: "application/pdf", - Mtime: &typesv1beta1.Timestamp{Seconds: 4000}, - } + filename string + ref *sprovider.Reference + ri *sprovider.ResourceInfo parentRef = &sprovider.Reference{ ResourceId: rootId, Path: "./my/sudbir", @@ -92,6 +76,8 @@ var _ = Describe("Index", func() { ) BeforeEach(func() { + filename = "Foo.pdf" + mapping, err := index.BuildMapping() Expect(err).ToNot(HaveOccurred()) @@ -102,6 +88,28 @@ var _ = Describe("Index", func() { Expect(err).ToNot(HaveOccurred()) }) + JustBeforeEach(func() { + ref = &sprovider.Reference{ + ResourceId: rootId, + Path: "./" + filename, + } + ri = &sprovider.ResourceInfo{ + Id: &sprovider.ResourceId{ + StorageId: "storageid", + OpaqueId: "opaqueid", + }, + ParentId: &sprovider.ResourceId{ + StorageId: "storageid", + OpaqueId: "someopaqueid", + }, + Path: filename, + Size: 12345, + Type: sprovider.ResourceType_RESOURCE_TYPE_FILE, + MimeType: "application/pdf", + Mtime: &typesv1beta1.Timestamp{Seconds: 4000}, + } + }) + Describe("New", func() { It("returns a new index instance", func() { i, err := index.New(bleveIndex) @@ -119,8 +127,32 @@ var _ = Describe("Index", func() { }) Describe("Search", func() { - Context("with a file in the root of the space", func() { + Context("with a filename with spaces", func() { BeforeEach(func() { + filename = "Foo oo.pdf" + }) + It("finds the file", func() { + err := i.Add(ref, ri) + Expect(err).ToNot(HaveOccurred()) + + res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ + Ref: &searchmsg.Reference{ + ResourceId: &searchmsg.ResourceID{ + StorageId: ref.ResourceId.StorageId, + OpaqueId: ref.ResourceId.OpaqueId, + }, + }, + Query: `Name:foo\ o*`, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(1)) + }) + + }) + + Context("with a file in the root of the space", func() { + JustBeforeEach(func() { err := i.Add(ref, ri) Expect(err).ToNot(HaveOccurred()) }) diff --git a/extensions/search/pkg/search/provider/searchprovider.go b/extensions/search/pkg/search/provider/searchprovider.go index a9014aa87..2c6fc4607 100644 --- a/extensions/search/pkg/search/provider/searchprovider.go +++ b/extensions/search/pkg/search/provider/searchprovider.go @@ -125,7 +125,7 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s _, rootStorageID := storagespace.SplitStorageID(space.Root.StorageId) res, err := p.indexClient.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "Name:" + strings.ToLower(req.Query), + Query: "Name:" + strings.ReplaceAll(strings.ToLower(req.Query), " ", `\ `), Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ StorageId: space.Root.StorageId, diff --git a/extensions/search/pkg/search/provider/searchprovider_test.go b/extensions/search/pkg/search/provider/searchprovider_test.go index 562a4ee06..2f6e2a2ab 100644 --- a/extensions/search/pkg/search/provider/searchprovider_test.go +++ b/extensions/search/pkg/search/provider/searchprovider_test.go @@ -146,6 +146,24 @@ var _ = Describe("Searchprovider", func() { }, nil) }) + It("lowercases the filename", func() { + p.Search(ctx, &searchsvc.SearchRequest{ + Query: "Foo.pdf", + }) + indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { + return req.Query == "Name:foo.pdf" + })) + }) + + It("escapes special characters", func() { + p.Search(ctx, &searchsvc.SearchRequest{ + Query: "Foo oo.pdf", + }) + indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { + return req.Query == `Name:foo\ oo.pdf` + })) + }) + It("searches the personal user space", func() { res, err := p.Search(ctx, &searchsvc.SearchRequest{ Query: "foo", From 89d2939fab7a6f60ff738cdfa0b1b30eebe67b33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 10:01:01 +0200 Subject: [PATCH 6/9] Add test for filename search using digits, cleanup tests --- .../search/pkg/search/index/index_test.go | 161 ++++-------------- 1 file changed, 36 insertions(+), 125 deletions(-) diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index 1549c2488..af7a3fe50 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -60,7 +60,7 @@ var _ = Describe("Index", func() { Mtime: &typesv1beta1.Timestamp{Seconds: 4000}, } - assertDocCount = func(rootId *sprovider.ResourceId, query string, expectedCount int) { + assertDocCount = func(rootId *sprovider.ResourceId, query string, expectedCount int) []*searchmsg.Match { res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ Query: query, Ref: &searchmsg.Reference{ @@ -71,7 +71,8 @@ var _ = Describe("Index", func() { }, }) ExpectWithOffset(1, err).ToNot(HaveOccurred()) - ExpectWithOffset(1, len(res.Matches)).To(Equal(expectedCount)) + ExpectWithOffset(1, len(res.Matches)).To(Equal(expectedCount), "query returned unexpected number of results: "+query) + return res.Matches } ) @@ -127,28 +128,22 @@ var _ = Describe("Index", func() { }) Describe("Search", func() { - Context("with a filename with spaces", func() { - BeforeEach(func() { - filename = "Foo oo.pdf" - }) - It("finds the file", func() { - err := i.Add(ref, ri) - Expect(err).ToNot(HaveOccurred()) + It("finds files with spaces in the filename", func() { + ri.Path = "Foo oo.pdf" + ref.Path = "./" + ri.Path + err := i.Add(ref, ri) + Expect(err).ToNot(HaveOccurred()) - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: `Name:foo\ o*`, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(1)) - }) + assertDocCount(ref.ResourceId, `Name:foo\ o*`, 1) + }) + It("finds files by digits in the filename", func() { + ri.Path = "12345.pdf" + ref.Path = "./" + ri.Path + err := i.Add(ref, ri) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(ref.ResourceId, `Name:1234*`, 1) }) Context("with a file in the root of the space", func() { @@ -158,49 +153,20 @@ var _ = Describe("Index", func() { }) It("scopes the search to the specified space", func() { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: "differentstorageid", - OpaqueId: "differentopaqueid", - }, - }, - Query: "Name:foo.pdf", - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(0)) + resourceId := &sprovider.ResourceId{ + StorageId: "differentstorageid", + OpaqueId: "differentopaqueid", + } + assertDocCount(resourceId, `Name:foo.pdf`, 0) }) It("limits the search to the specified fields", func() { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: "Name:*" + ref.ResourceId.OpaqueId + "*", - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(0)) + assertDocCount(ref.ResourceId, "Name:*"+ref.ResourceId.OpaqueId+"*", 0) }) It("returns all desired fields", func() { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: "Name:foo.pdf", - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(1)) - match := res.Matches[0] + matches := assertDocCount(ref.ResourceId, "Name:foo.pdf", 1) + match := matches[0] Expect(match.Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) Expect(match.Entity.Ref.Path).To(Equal(ref.Path)) Expect(match.Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) @@ -215,52 +181,18 @@ var _ = Describe("Index", func() { It("finds files by name, prefix or substring match", func() { queries := []string{"foo.pdf", "foo*", "*oo.p*"} for _, query := range queries { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: query, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(1), "query returned no result: "+query) - Expect(res.Matches[0].Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) - Expect(res.Matches[0].Entity.Ref.Path).To(Equal(ref.Path)) - Expect(res.Matches[0].Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) - Expect(res.Matches[0].Entity.Name).To(Equal(ri.Path)) - Expect(res.Matches[0].Entity.Size).To(Equal(ri.Size)) + matches := assertDocCount(ref.ResourceId, query, 1) + Expect(matches[0].Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) + Expect(matches[0].Entity.Ref.Path).To(Equal(ref.Path)) + Expect(matches[0].Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) + Expect(matches[0].Entity.Name).To(Equal(ri.Path)) + Expect(matches[0].Entity.Size).To(Equal(ri.Size)) } }) It("uses a lower-case index", func() { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: "Name:foo*", - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(1)) - - res, err = i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: "Name:Foo*", - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(0)) + assertDocCount(ref.ResourceId, "Name:foo*", 1) + assertDocCount(ref.ResourceId, "Name:Foo*", 0) }) Context("and an additional file in a subdirectory", func() { @@ -292,18 +224,7 @@ var _ = Describe("Index", func() { It("finds files living deeper in the tree by filename, prefix or substring match", func() { queries := []string{"nestedpdf.pdf", "nested*", "*tedpdf.*"} for _, query := range queries { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, - }, - }, - Query: query, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(1), "query returned no result: "+query) + assertDocCount(ref.ResourceId, query, 1) } }) @@ -416,18 +337,8 @@ var _ = Describe("Index", func() { assertDocCount(rootId, "subdir", 0) - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "Name:child.pdf", - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: rootId.StorageId, - OpaqueId: rootId.OpaqueId, - }, - }, - }) - Expect(err).ToNot(HaveOccurred()) - Expect(len(res.Matches)).To(Equal(1)) - Expect(res.Matches[0].Entity.Ref.Path).To(Equal("./somewhere/else/newname/child.pdf")) + matches := assertDocCount(rootId, "Name:child.pdf", 1) + Expect(matches[0].Entity.Ref.Path).To(Equal("./somewhere/else/newname/child.pdf")) }) }) }) From c6c718eb419b67fd6c5d3b19cddb7d3a00a66a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 10:57:06 +0200 Subject: [PATCH 7/9] Allow for searching by metadata like "Size:<100" --- .../search/pkg/search/index/index_test.go | 211 ++++++++++-------- .../pkg/search/provider/searchprovider.go | 12 +- .../search/provider/searchprovider_test.go | 9 + extensions/webdav/pkg/service/v0/search.go | 2 +- 4 files changed, 136 insertions(+), 98 deletions(-) diff --git a/extensions/search/pkg/search/index/index_test.go b/extensions/search/pkg/search/index/index_test.go index af7a3fe50..582df6f4f 100644 --- a/extensions/search/pkg/search/index/index_test.go +++ b/extensions/search/pkg/search/index/index_test.go @@ -128,120 +128,139 @@ var _ = Describe("Index", func() { }) Describe("Search", func() { - It("finds files with spaces in the filename", func() { - ri.Path = "Foo oo.pdf" - ref.Path = "./" + ri.Path - err := i.Add(ref, ri) - Expect(err).ToNot(HaveOccurred()) - - assertDocCount(ref.ResourceId, `Name:foo\ o*`, 1) - }) - - It("finds files by digits in the filename", func() { - ri.Path = "12345.pdf" - ref.Path = "./" + ri.Path - err := i.Add(ref, ri) - Expect(err).ToNot(HaveOccurred()) - - assertDocCount(ref.ResourceId, `Name:1234*`, 1) - }) - - Context("with a file in the root of the space", func() { + Context("by other fields than filename", func() { JustBeforeEach(func() { err := i.Add(ref, ri) Expect(err).ToNot(HaveOccurred()) }) - It("scopes the search to the specified space", func() { - resourceId := &sprovider.ResourceId{ - StorageId: "differentstorageid", - OpaqueId: "differentopaqueid", - } - assertDocCount(resourceId, `Name:foo.pdf`, 0) + It("finds files by size", func() { + assertDocCount(ref.ResourceId, `Size:12345`, 1) + assertDocCount(ref.ResourceId, `Size:>1000`, 1) + assertDocCount(ref.ResourceId, `Size:<100000`, 1) + + assertDocCount(ref.ResourceId, `Size:12344`, 0) + assertDocCount(ref.ResourceId, `Size:<1000`, 0) + assertDocCount(ref.ResourceId, `Size:>100000`, 0) + }) + }) + + Context("by filename", func() { + It("finds files with spaces in the filename", func() { + ri.Path = "Foo oo.pdf" + ref.Path = "./" + ri.Path + err := i.Add(ref, ri) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(ref.ResourceId, `Name:foo\ o*`, 1) }) - It("limits the search to the specified fields", func() { - assertDocCount(ref.ResourceId, "Name:*"+ref.ResourceId.OpaqueId+"*", 0) + It("finds files by digits in the filename", func() { + ri.Path = "12345.pdf" + ref.Path = "./" + ri.Path + err := i.Add(ref, ri) + Expect(err).ToNot(HaveOccurred()) + + assertDocCount(ref.ResourceId, `Name:1234*`, 1) }) - It("returns all desired fields", func() { - matches := assertDocCount(ref.ResourceId, "Name:foo.pdf", 1) - match := matches[0] - Expect(match.Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) - Expect(match.Entity.Ref.Path).To(Equal(ref.Path)) - Expect(match.Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) - Expect(match.Entity.Name).To(Equal(ri.Path)) - Expect(match.Entity.Size).To(Equal(ri.Size)) - Expect(match.Entity.Type).To(Equal(uint64(ri.Type))) - Expect(match.Entity.MimeType).To(Equal(ri.MimeType)) - Expect(match.Entity.Deleted).To(BeFalse()) - Expect(uint64(match.Entity.LastModifiedTime.AsTime().Unix())).To(Equal(ri.Mtime.Seconds)) - }) - - It("finds files by name, prefix or substring match", func() { - queries := []string{"foo.pdf", "foo*", "*oo.p*"} - for _, query := range queries { - matches := assertDocCount(ref.ResourceId, query, 1) - Expect(matches[0].Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) - Expect(matches[0].Entity.Ref.Path).To(Equal(ref.Path)) - Expect(matches[0].Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) - Expect(matches[0].Entity.Name).To(Equal(ri.Path)) - Expect(matches[0].Entity.Size).To(Equal(ri.Size)) - } - }) - - It("uses a lower-case index", func() { - assertDocCount(ref.ResourceId, "Name:foo*", 1) - assertDocCount(ref.ResourceId, "Name:Foo*", 0) - }) - - Context("and an additional file in a subdirectory", func() { - var ( - nestedRef *sprovider.Reference - nestedRI *sprovider.ResourceInfo - ) - - BeforeEach(func() { - nestedRef = &sprovider.Reference{ - ResourceId: &sprovider.ResourceId{ - StorageId: "storageid", - OpaqueId: "rootopaqueid", - }, - Path: "./nested/nestedpdf.pdf", - } - nestedRI = &sprovider.ResourceInfo{ - Id: &sprovider.ResourceId{ - StorageId: "storageid", - OpaqueId: "nestedopaqueid", - }, - Path: "nestedpdf.pdf", - Size: 12345, - } - err := i.Add(nestedRef, nestedRI) + Context("with a file in the root of the space", func() { + JustBeforeEach(func() { + err := i.Add(ref, ri) Expect(err).ToNot(HaveOccurred()) }) - It("finds files living deeper in the tree by filename, prefix or substring match", func() { - queries := []string{"nestedpdf.pdf", "nested*", "*tedpdf.*"} + It("scopes the search to the specified space", func() { + resourceId := &sprovider.ResourceId{ + StorageId: "differentstorageid", + OpaqueId: "differentopaqueid", + } + assertDocCount(resourceId, `Name:foo.pdf`, 0) + }) + + It("limits the search to the specified fields", func() { + assertDocCount(ref.ResourceId, "Name:*"+ref.ResourceId.OpaqueId+"*", 0) + }) + + It("returns all desired fields", func() { + matches := assertDocCount(ref.ResourceId, "Name:foo.pdf", 1) + match := matches[0] + Expect(match.Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) + Expect(match.Entity.Ref.Path).To(Equal(ref.Path)) + Expect(match.Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) + Expect(match.Entity.Name).To(Equal(ri.Path)) + Expect(match.Entity.Size).To(Equal(ri.Size)) + Expect(match.Entity.Type).To(Equal(uint64(ri.Type))) + Expect(match.Entity.MimeType).To(Equal(ri.MimeType)) + Expect(match.Entity.Deleted).To(BeFalse()) + Expect(uint64(match.Entity.LastModifiedTime.AsTime().Unix())).To(Equal(ri.Mtime.Seconds)) + }) + + It("finds files by name, prefix or substring match", func() { + queries := []string{"foo.pdf", "foo*", "*oo.p*"} for _, query := range queries { - assertDocCount(ref.ResourceId, query, 1) + matches := assertDocCount(ref.ResourceId, query, 1) + Expect(matches[0].Entity.Ref.ResourceId.OpaqueId).To(Equal(ref.ResourceId.OpaqueId)) + Expect(matches[0].Entity.Ref.Path).To(Equal(ref.Path)) + Expect(matches[0].Entity.Id.OpaqueId).To(Equal(ri.Id.OpaqueId)) + Expect(matches[0].Entity.Name).To(Equal(ri.Path)) + Expect(matches[0].Entity.Size).To(Equal(ri.Size)) } }) - It("does not find the higher levels when limiting the searched directory", func() { - res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ - Ref: &searchmsg.Reference{ - ResourceId: &searchmsg.ResourceID{ - StorageId: ref.ResourceId.StorageId, - OpaqueId: ref.ResourceId.OpaqueId, + It("uses a lower-case index", func() { + assertDocCount(ref.ResourceId, "Name:foo*", 1) + assertDocCount(ref.ResourceId, "Name:Foo*", 0) + }) + + Context("and an additional file in a subdirectory", func() { + var ( + nestedRef *sprovider.Reference + nestedRI *sprovider.ResourceInfo + ) + + BeforeEach(func() { + nestedRef = &sprovider.Reference{ + ResourceId: &sprovider.ResourceId{ + StorageId: "storageid", + OpaqueId: "rootopaqueid", }, - Path: "./nested/", - }, - Query: "Name:foo.pdf", + Path: "./nested/nestedpdf.pdf", + } + nestedRI = &sprovider.ResourceInfo{ + Id: &sprovider.ResourceId{ + StorageId: "storageid", + OpaqueId: "nestedopaqueid", + }, + Path: "nestedpdf.pdf", + Size: 12345, + } + err := i.Add(nestedRef, nestedRI) + Expect(err).ToNot(HaveOccurred()) + }) + + It("finds files living deeper in the tree by filename, prefix or substring match", func() { + queries := []string{"nestedpdf.pdf", "nested*", "*tedpdf.*"} + for _, query := range queries { + assertDocCount(ref.ResourceId, query, 1) + } + }) + + It("does not find the higher levels when limiting the searched directory", func() { + res, err := i.Search(ctx, &searchsvc.SearchIndexRequest{ + Ref: &searchmsg.Reference{ + ResourceId: &searchmsg.ResourceID{ + StorageId: ref.ResourceId.StorageId, + OpaqueId: ref.ResourceId.OpaqueId, + }, + Path: "./nested/", + }, + Query: "Name:foo.pdf", + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(0)) }) - Expect(err).ToNot(HaveOccurred()) - Expect(res).ToNot(BeNil()) - Expect(len(res.Matches)).To(Equal(0)) }) }) }) diff --git a/extensions/search/pkg/search/provider/searchprovider.go b/extensions/search/pkg/search/provider/searchprovider.go index 2c6fc4607..233d12f19 100644 --- a/extensions/search/pkg/search/provider/searchprovider.go +++ b/extensions/search/pkg/search/provider/searchprovider.go @@ -125,7 +125,7 @@ func (p *Provider) Search(ctx context.Context, req *searchsvc.SearchRequest) (*s _, rootStorageID := storagespace.SplitStorageID(space.Root.StorageId) res, err := p.indexClient.Search(ctx, &searchsvc.SearchIndexRequest{ - Query: "Name:" + strings.ReplaceAll(strings.ToLower(req.Query), " ", `\ `), + Query: formatQuery(req.Query), Ref: &searchmsg.Reference{ ResourceId: &searchmsg.ResourceID{ StorageId: space.Root.StorageId, @@ -217,3 +217,13 @@ func (p *Provider) logDocCount() { } p.logger.Debug().Interface("count", c).Msg("new document count") } + +func formatQuery(q string) string { + query := q + if strings.Contains(q, ":") { + return q // Sophisticated field based search + } + + // this is a basic filename search + return "Name:*" + strings.ReplaceAll(strings.ToLower(query), " ", `\ `) + "*" +} diff --git a/extensions/search/pkg/search/provider/searchprovider_test.go b/extensions/search/pkg/search/provider/searchprovider_test.go index 2f6e2a2ab..288a559a4 100644 --- a/extensions/search/pkg/search/provider/searchprovider_test.go +++ b/extensions/search/pkg/search/provider/searchprovider_test.go @@ -155,6 +155,15 @@ var _ = Describe("Searchprovider", func() { })) }) + It("does not mess with field-based searches", func() { + p.Search(ctx, &searchsvc.SearchRequest{ + Query: "Size:<10", + }) + indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { + return req.Query == "Size:<10" + })) + }) + It("escapes special characters", func() { p.Search(ctx, &searchsvc.SearchRequest{ Query: "Foo oo.pdf", diff --git a/extensions/webdav/pkg/service/v0/search.go b/extensions/webdav/pkg/service/v0/search.go index e90382984..96288df10 100644 --- a/extensions/webdav/pkg/service/v0/search.go +++ b/extensions/webdav/pkg/service/v0/search.go @@ -43,7 +43,7 @@ func (g Webdav) Search(w http.ResponseWriter, r *http.Request) { ctx := revactx.ContextSetToken(r.Context(), t) ctx = metadata.Set(ctx, revactx.TokenHeader, t) rsp, err := g.searchClient.Search(ctx, &searchsvc.SearchRequest{ - Query: "*" + rep.SearchFiles.Search.Pattern + "*", + Query: rep.SearchFiles.Search.Pattern, }) if err != nil { e := merrors.Parse(err.Error()) From 4a36ba78110f4ff9fdb6852713f82cb264f6fd68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 11:35:29 +0200 Subject: [PATCH 8/9] Allow using lowercase fields --- .../pkg/search/provider/searchprovider.go | 9 +++++-- .../search/provider/searchprovider_test.go | 25 ++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/extensions/search/pkg/search/provider/searchprovider.go b/extensions/search/pkg/search/provider/searchprovider.go index 233d12f19..067d9665e 100644 --- a/extensions/search/pkg/search/provider/searchprovider.go +++ b/extensions/search/pkg/search/provider/searchprovider.go @@ -220,8 +220,13 @@ func (p *Provider) logDocCount() { func formatQuery(q string) string { query := q - if strings.Contains(q, ":") { - return q // Sophisticated field based search + fields := []string{"RootID", "Path", "ID", "Name", "Size", "Mtime", "MimeType", "Type"} + for _, field := range fields { + query = strings.ReplaceAll(query, strings.ToLower(field)+":", field+":") + } + + if strings.Contains(query, ":") { + return query // Sophisticated field based search } // this is a basic filename search diff --git a/extensions/search/pkg/search/provider/searchprovider_test.go b/extensions/search/pkg/search/provider/searchprovider_test.go index 288a559a4..5f7278273 100644 --- a/extensions/search/pkg/search/provider/searchprovider_test.go +++ b/extensions/search/pkg/search/provider/searchprovider_test.go @@ -151,7 +151,7 @@ var _ = Describe("Searchprovider", func() { Query: "Foo.pdf", }) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == "Name:foo.pdf" + return req.Query == "Name:*foo.pdf*" })) }) @@ -164,12 +164,29 @@ var _ = Describe("Searchprovider", func() { })) }) + It("uppercases field names", func() { + tests := []struct { + Original string + Expected string + }{ + {Original: "size:<100", Expected: "Size:<100"}, + } + for _, test := range tests { + p.Search(ctx, &searchsvc.SearchRequest{ + Query: test.Original, + }) + indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { + return req.Query == test.Expected + })) + } + }) + It("escapes special characters", func() { p.Search(ctx, &searchsvc.SearchRequest{ Query: "Foo oo.pdf", }) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == `Name:foo\ oo.pdf` + return req.Query == `Name:*foo\ oo.pdf*` })) }) @@ -187,7 +204,7 @@ var _ = Describe("Searchprovider", func() { Expect(match.Entity.Ref.Path).To(Equal("./path/to/Foo.pdf")) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == "Name:foo" && req.Ref.ResourceId.OpaqueId == personalSpace.Root.OpaqueId && req.Ref.Path == "" + return req.Query == "Name:*foo*" && req.Ref.ResourceId.OpaqueId == personalSpace.Root.OpaqueId && req.Ref.Path == "" })) }) }) @@ -264,7 +281,7 @@ var _ = Describe("Searchprovider", func() { Expect(match.Entity.Ref.Path).To(Equal("./to/Shared.pdf")) indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool { - return req.Query == "Name:foo" && req.Ref.ResourceId.StorageId == grantSpace.Root.StorageId && req.Ref.Path == "./grant/path" + return req.Query == "Name:*foo*" && req.Ref.ResourceId.StorageId == grantSpace.Root.StorageId && req.Ref.Path == "./grant/path" })) }) From c4f25c6a55f2560f08c07755f77ecfa6ed2743f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 12 May 2022 15:41:05 +0200 Subject: [PATCH 9/9] Return a proper resource type in the propstat response Fixes https://github.com/owncloud/web/issues/6944 --- extensions/webdav/pkg/service/v0/search.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/extensions/webdav/pkg/service/v0/search.go b/extensions/webdav/pkg/service/v0/search.go index 96288df10..817f401a1 100644 --- a/extensions/webdav/pkg/service/v0/search.go +++ b/extensions/webdav/pkg/service/v0/search.go @@ -9,6 +9,7 @@ import ( "strconv" "time" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/owncloud/ocis/v2/extensions/webdav/pkg/net" "github.com/owncloud/ocis/v2/extensions/webdav/pkg/prop" @@ -128,9 +129,15 @@ func matchToPropResponse(ctx context.Context, match *searchmsg.Match) (*propfind propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getcontenttype", match.Entity.MimeType)) size := strconv.FormatUint(match.Entity.Size, 10) - propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:size", size)) - propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("d:getcontentlength", size)) - + if match.Entity.Type == uint64(provider.ResourceType_RESOURCE_TYPE_CONTAINER) { + propstatOK.Prop = append(propstatOK.Prop, prop.Raw("d:resourcetype", "")) + propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:size", size)) + } else { + propstatOK.Prop = append(propstatOK.Prop, + prop.Escaped("d:resourcetype", ""), + prop.Escaped("d:getcontentlength", size), + ) + } // TODO find name for score property score := strconv.FormatFloat(float64(match.Score), 'f', -1, 64) propstatOK.Prop = append(propstatOK.Prop, prop.Escaped("oc:score", score))