diff --git a/protogen/gen/opencloud/services/search/v0/search.pb.go b/protogen/gen/opencloud/services/search/v0/search.pb.go index 53630ca26f..492f88021c 100644 --- a/protogen/gen/opencloud/services/search/v0/search.pb.go +++ b/protogen/gen/opencloud/services/search/v0/search.pb.go @@ -536,11 +536,12 @@ type SortProperty struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - // Required. The field to sort on, in graph notation. Supported: "name", - // "size", "lastModifiedDateTime", "photo.takenDateTime". A field is only - // sortable if it is indexed as a sortable type in both backends AND - // present on the Match entity (the service layer needs the sort key to - // merge per-space result streams). + // Required. The field to sort on, in graph notation ("name", "size", + // "lastModifiedDateTime", "mimeType" or a scalar facet field such as + // "photo.takenDateTime" or "audio.artist"). A field is sortable when it is + // indexed as a scalar in both backends AND carried on the Match entity + // (the service layer needs the sort key to merge per-space result + // streams); see the search package's IsSortableField. Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // Optional. Sort in descending order. Defaults to ascending. IsDescending bool `protobuf:"varint,2,opt,name=is_descending,json=isDescending,proto3" json:"is_descending,omitempty"` diff --git a/protogen/gen/opencloud/services/search/v0/search.swagger.json b/protogen/gen/opencloud/services/search/v0/search.swagger.json index f242ff3fb3..ef667bc2b1 100644 --- a/protogen/gen/opencloud/services/search/v0/search.swagger.json +++ b/protogen/gen/opencloud/services/search/v0/search.swagger.json @@ -778,7 +778,7 @@ "properties": { "name": { "type": "string", - "description": "Required. The field to sort on, in graph notation. Supported: \"name\",\n\"size\", \"lastModifiedDateTime\", \"photo.takenDateTime\". A field is only\nsortable if it is indexed as a sortable type in both backends AND\npresent on the Match entity (the service layer needs the sort key to\nmerge per-space result streams)." + "description": "Required. The field to sort on, in graph notation (\"name\", \"size\",\n\"lastModifiedDateTime\", \"mimeType\" or a scalar facet field such as\n\"photo.takenDateTime\" or \"audio.artist\"). A field is sortable when it is\nindexed as a scalar in both backends AND carried on the Match entity\n(the service layer needs the sort key to merge per-space result\nstreams); see the search package's IsSortableField." }, "isDescending": { "type": "boolean", diff --git a/protogen/proto/opencloud/services/search/v0/search.proto b/protogen/proto/opencloud/services/search/v0/search.proto index e8bf9ee2e4..68acdf1f6e 100644 --- a/protogen/proto/opencloud/services/search/v0/search.proto +++ b/protogen/proto/opencloud/services/search/v0/search.proto @@ -157,11 +157,12 @@ message AggregationOption { } message SortProperty { - // Required. The field to sort on, in graph notation. Supported: "name", - // "size", "lastModifiedDateTime", "photo.takenDateTime". A field is only - // sortable if it is indexed as a sortable type in both backends AND - // present on the Match entity (the service layer needs the sort key to - // merge per-space result streams). + // Required. The field to sort on, in graph notation ("name", "size", + // "lastModifiedDateTime", "mimeType" or a scalar facet field such as + // "photo.takenDateTime" or "audio.artist"). A field is sortable when it is + // indexed as a scalar in both backends AND carried on the Match entity + // (the service layer needs the sort key to merge per-space result + // streams); see the search package's IsSortableField. string name = 1; // Optional. Sort in descending order. Defaults to ascending. bool is_descending = 2; diff --git a/services/graph/pkg/service/v0/searchquery.go b/services/graph/pkg/service/v0/searchquery.go index b524dc02b9..fd48333d65 100644 --- a/services/graph/pkg/service/v0/searchquery.go +++ b/services/graph/pkg/service/v0/searchquery.go @@ -175,23 +175,14 @@ func validateAggregations(aggs []libregraph.AggregationOption) error { return nil } -// sortableFields is the set of fields accepted in sortProperties (mirrored in -// the openapi spec). A field qualifies only if it is indexed as a sortable -// type in both search backends AND carried on the Match entity: the service -// layer re-sorts matches when merging the per-space result streams and needs -// the sort key on the match itself. -var sortableFields = map[string]struct{}{ - "name": {}, - "size": {}, - "lastModifiedDateTime": {}, - "photo.takenDateTime": {}, -} - -// validateSortProperties rejects sorting by fields outside sortableFields. +// validateSortProperties rejects sorting by unknown or multivalued fields. +// Sortable are scalar fields carried on the search hit: name, size, +// lastModifiedDateTime, mimeType and the facet fields (photo.takenDateTime, +// audio.artist, image.width, ...); see search.IsSortableField. func validateSortProperties(sortProperties []libregraph.SortProperty) error { for _, sp := range sortProperties { - if _, ok := sortableFields[sp.Name]; !ok { - return fmt.Errorf("field %q is not sortable; sortable fields: name, size, lastModifiedDateTime, photo.takenDateTime", sp.Name) + if !search.IsSortableField(sp.Name) { + return fmt.Errorf("field %q is not sortable; sortable are scalar hit fields such as name, size, lastModifiedDateTime, mimeType or photo.takenDateTime", sp.Name) } } return nil diff --git a/services/graph/pkg/service/v0/searchquery_test.go b/services/graph/pkg/service/v0/searchquery_test.go index 0db86ef2c0..c89fc9d881 100644 --- a/services/graph/pkg/service/v0/searchquery_test.go +++ b/services/graph/pkg/service/v0/searchquery_test.go @@ -225,23 +225,56 @@ var _ = ginkgo.Describe("SearchQuery", func() { Expect(captured.OrderBy[1].IsDescending).To(BeFalse()) }) - ginkgo.It("rejects sorting by an unsupported field with 400", func() { - g := graphWithSearch(stubSearchService{ - search: func(*searchsvc.SearchRequest) (*searchsvc.SearchResponse, error) { - ginkgo.Fail("search service must not be called when validation fails") - return nil, nil - }, - }) - rr := postSearchQuery(g, `{ - "requests": [{ - "entityTypes": ["driveItem"], - "query": {"queryString": "mediatype:image"}, - "sortProperties": [{"name": "photo.iso"}] - }] - }`) - Expect(rr.Code).To(Equal(http.StatusBadRequest), rr.Body.String()) - Expect(rr.Body.String()).To(ContainSubstring("photo.iso")) - }) + ginkgo.DescribeTable("accepts sorting by scalar hit fields", + func(field string) { + g := graphWithSearch(stubSearchService{ + search: func(*searchsvc.SearchRequest) (*searchsvc.SearchResponse, error) { + return &searchsvc.SearchResponse{}, nil + }, + }) + rr := postSearchQuery(g, `{ + "requests": [{ + "entityTypes": ["driveItem"], + "query": {"queryString": "*"}, + "sortProperties": [{"name": "`+field+`"}] + }] + }`) + Expect(rr.Code).To(Equal(http.StatusOK), rr.Body.String()) + }, + ginkgo.Entry("name", "name"), + ginkgo.Entry("size", "size"), + ginkgo.Entry("lastModifiedDateTime", "lastModifiedDateTime"), + ginkgo.Entry("mimeType", "mimeType"), + ginkgo.Entry("photo.takenDateTime", "photo.takenDateTime"), + ginkgo.Entry("photo.iso", "photo.iso"), + ginkgo.Entry("audio.artist", "audio.artist"), + ginkgo.Entry("image.width", "image.width"), + ) + + ginkgo.DescribeTable("rejects sorting by unsortable fields with 400", + func(field string) { + g := graphWithSearch(stubSearchService{ + search: func(*searchsvc.SearchRequest) (*searchsvc.SearchResponse, error) { + ginkgo.Fail("search service must not be called when validation fails") + return nil, nil + }, + }) + rr := postSearchQuery(g, `{ + "requests": [{ + "entityTypes": ["driveItem"], + "query": {"queryString": "mediatype:image"}, + "sortProperties": [{"name": "`+field+`"}] + }] + }`) + Expect(rr.Code).To(Equal(http.StatusBadRequest), rr.Body.String()) + Expect(rr.Body.String()).To(ContainSubstring(field)) + }, + ginkgo.Entry("unknown field", "definitelyNotAField"), + ginkgo.Entry("multivalued field", "tags"), + ginkgo.Entry("internal index field name", "Mtime"), + ginkgo.Entry("bare audio facet", "audio"), + ginkgo.Entry("bare location facet", "location"), + ) ginkgo.It("rejects a terms aggregation on a numeric field with 400", func() { g := graphWithSearch(stubSearchService{ diff --git a/services/search/pkg/search/sort.go b/services/search/pkg/search/sort.go new file mode 100644 index 0000000000..af61fd99d7 --- /dev/null +++ b/services/search/pkg/search/sort.go @@ -0,0 +1,257 @@ +package search + +import ( + "reflect" + "strings" + + "google.golang.org/protobuf/reflect/protoreflect" + + searchmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" + searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" +) + +// Sorting support for search results (graph sortProperties / proto order_by). +// +// A field is sortable when both of the following hold: +// - it is indexed as a scalar (string, number, bool or time), so the +// engines can sort on it natively; multivalued fields like Tags are not +// sortable +// - it is carried on the match entity, so the service layer can read the +// sort key when merging the per-space result streams +// +// Both sets are derived by reflection (index side: the Resource type, match +// side: the Entity proto), so new facet fields become sortable automatically. + +// sortIndexAliases maps the graph-facing names of top-level fields to their +// index field names. Facet fields (photo.*, audio.*, ...) share the same +// dotted names in both worlds and need no alias. Top-level fields are only +// exposed under these graph names; internal fields like RootID or Deleted +// stay unsortable. +var sortIndexAliases = map[string]string{ + "name": "Name", + "size": "Size", + "lastModifiedDateTime": "Mtime", + "mimeType": "MimeType", +} + +// entityJSONAliases maps graph-facing names to the Entity proto's JSON names +// where the two disagree. +var entityJSONAliases = map[string]string{ + "lastModifiedDateTime": "lastModifiedTime", +} + +// IsSortableField reports whether results can be sorted by the field. +func IsSortableField(name string) bool { + _, ok := SortIndexField(name) + return ok +} + +// SortIndexField translates a graph sortProperties name into the index field +// name to sort on, reporting whether the field is sortable at all. +func SortIndexField(name string) (string, bool) { + field := name + if alias, ok := sortIndexAliases[name]; ok { + field = alias + } else if !strings.Contains(name, ".") { + return "", false + } + if !sortableIndexFields[field] { + return "", false + } + if !entityFieldResolvable(name) { + return "", false + } + return field, true +} + +// CompareMatches orders match a relative to b according to orderBy: -1 when a +// comes first, 1 when b comes first, 0 when the sort keys tie (callers fall +// back to the score). Matches missing a sort key sort after those that have +// it, regardless of direction. +func CompareMatches(a, b *searchmsg.Match, orderBy []*searchsvc.SortProperty) int { + for _, sp := range orderBy { + ka := matchSortKey(a, sp.GetName()) + kb := matchSortKey(b, sp.GetName()) + if !ka.present && !kb.present { + continue + } + if !ka.present { + return 1 + } + if !kb.present { + return -1 + } + c := 0 + switch { + case ka.isString: + c = strings.Compare(ka.str, kb.str) + case ka.num < kb.num: + c = -1 + case ka.num > kb.num: + c = 1 + } + if c == 0 { + continue + } + if sp.GetIsDescending() { + c = -c + } + return c + } + return 0 +} + +// sortableIndexFields is the set of scalar indexed fields, keyed by index +// field name. +var sortableIndexFields = buildSortableFieldSet() + +func buildSortableFieldSet() map[string]bool { + out := map[string]bool{} + collectScalarFields(out, "", reflect.TypeOf(Resource{})) + return out +} + +func collectScalarFields(out map[string]bool, prefix string, t reflect.Type) { + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return + } + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.IsExported() { + continue + } + if f.Anonymous { + collectScalarFields(out, prefix, f.Type) + continue + } + path := prefix + jsonFieldName(f) + ft := f.Type + for ft.Kind() == reflect.Ptr { + ft = ft.Elem() + } + switch ft.Kind() { + case reflect.String, reflect.Bool, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64: + out[path] = true + case reflect.Struct: + if ft == timeType { + out[path] = true + continue + } + collectScalarFields(out, path+".", ft) + } + } +} + +func entityPath(name string) []string { + if alias, ok := entityJSONAliases[name]; ok { + name = alias + } + return strings.Split(name, ".") +} + +const timestampFullName = protoreflect.FullName("google.protobuf.Timestamp") + +// entityFieldResolvable reports whether the graph field name resolves to a +// scalar (or timestamp) field on the match entity. +func entityFieldResolvable(name string) bool { + md := (&searchmsg.Entity{}).ProtoReflect().Descriptor() + segments := entityPath(name) + for i, seg := range segments { + fd := md.Fields().ByJSONName(seg) + if fd == nil || fd.IsList() || fd.IsMap() { + return false + } + if i < len(segments)-1 { + if fd.Kind() != protoreflect.MessageKind { + return false + } + md = fd.Message() + continue + } + switch fd.Kind() { + case protoreflect.StringKind, protoreflect.BoolKind, + protoreflect.Int32Kind, protoreflect.Int64Kind, + protoreflect.Sint32Kind, protoreflect.Sint64Kind, + protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind, + protoreflect.Uint32Kind, protoreflect.Uint64Kind, + protoreflect.Fixed32Kind, protoreflect.Fixed64Kind, + protoreflect.FloatKind, protoreflect.DoubleKind: + return true + case protoreflect.MessageKind: + return fd.Message().FullName() == timestampFullName + } + return false + } + return false +} + +// sortKey is the comparable value of a sort field on a concrete match. +type sortKey struct { + present bool + isString bool + str string + num float64 +} + +// matchSortKey extracts the sort key for the graph field name from a match by +// walking the entity proto along the field's JSON names. +func matchSortKey(m *searchmsg.Match, name string) sortKey { + entity := m.GetEntity() + if entity == nil { + return sortKey{} + } + msg := entity.ProtoReflect() + segments := entityPath(name) + for i, seg := range segments { + fd := msg.Descriptor().Fields().ByJSONName(seg) + if fd == nil || fd.IsList() || fd.IsMap() { + return sortKey{} + } + if i < len(segments)-1 { + if fd.Kind() != protoreflect.MessageKind || !msg.Has(fd) { + return sortKey{} + } + msg = msg.Get(fd).Message() + continue + } + if fd.HasPresence() && !msg.Has(fd) { + return sortKey{} + } + v := msg.Get(fd) + switch fd.Kind() { + case protoreflect.StringKind: + return sortKey{present: true, isString: true, str: v.String()} + case protoreflect.BoolKind: + num := 0.0 + if v.Bool() { + num = 1.0 + } + return sortKey{present: true, num: num} + case protoreflect.Int32Kind, protoreflect.Int64Kind, + protoreflect.Sint32Kind, protoreflect.Sint64Kind, + protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind: + return sortKey{present: true, num: float64(v.Int())} + case protoreflect.Uint32Kind, protoreflect.Uint64Kind, + protoreflect.Fixed32Kind, protoreflect.Fixed64Kind: + return sortKey{present: true, num: float64(v.Uint())} + case protoreflect.FloatKind, protoreflect.DoubleKind: + return sortKey{present: true, num: v.Float()} + case protoreflect.MessageKind: + if fd.Message().FullName() != timestampFullName { + return sortKey{} + } + ts := v.Message() + seconds := ts.Get(ts.Descriptor().Fields().ByName("seconds")).Int() + nanos := ts.Get(ts.Descriptor().Fields().ByName("nanos")).Int() + return sortKey{present: true, num: float64(seconds) + float64(nanos)/1e9} + } + return sortKey{} + } + return sortKey{} +} diff --git a/services/search/pkg/search/sort_test.go b/services/search/pkg/search/sort_test.go new file mode 100644 index 0000000000..dd294d65e2 --- /dev/null +++ b/services/search/pkg/search/sort_test.go @@ -0,0 +1,125 @@ +package search_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "google.golang.org/protobuf/types/known/timestamppb" + + searchmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/search/v0" + searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" + "github.com/opencloud-eu/opencloud/services/search/pkg/search" +) + +var _ = Describe("SortIndexField", func() { + DescribeTable("maps graph field names to index fields", + func(name string, wantField string, wantOK bool) { + field, ok := search.SortIndexField(name) + Expect(ok).To(Equal(wantOK)) + Expect(field).To(Equal(wantField)) + }, + // top-level fields are exposed under their graph names + Entry("name", "name", "Name", true), + Entry("size", "size", "Size", true), + Entry("lastModifiedDateTime", "lastModifiedDateTime", "Mtime", true), + Entry("mimeType", "mimeType", "MimeType", true), + // facet fields keep their dotted names + Entry("photo.takenDateTime", "photo.takenDateTime", "photo.takenDateTime", true), + Entry("photo.iso", "photo.iso", "photo.iso", true), + Entry("photo.cameraModel", "photo.cameraModel", "photo.cameraModel", true), + Entry("audio.artist", "audio.artist", "audio.artist", true), + Entry("audio.year", "audio.year", "audio.year", true), + Entry("image.width", "image.width", "image.width", true), + Entry("location.latitude", "location.latitude", "location.latitude", true), + // bare facets (message-typed, no scalar value) are not sortable + Entry("audio (bare facet)", "audio", "", false), + Entry("photo (bare facet)", "photo", "", false), + Entry("location (bare facet)", "location", "", false), + Entry("location (dotted but not scalar)", "location.", "", false), + // multivalued fields are not sortable + Entry("tags (repeated)", "tags", "", false), + Entry("Tags (index name, repeated)", "Tags", "", false), + // internal index fields are not exposed under their index names + Entry("Name (index name)", "Name", "", false), + Entry("Mtime (index name)", "Mtime", "", false), + Entry("RootID", "RootID", "", false), + Entry("Deleted", "Deleted", "", false), + // unknown fields + Entry("unknown", "definitelyNotAField", "", false), + Entry("unknown facet field", "photo.definitelyNotAField", "", false), + Entry("empty", "", "", false), + ) +}) + +var _ = Describe("CompareMatches", func() { + match := func(mutate func(e *searchmsg.Entity)) *searchmsg.Match { + e := &searchmsg.Entity{} + mutate(e) + return &searchmsg.Match{Entity: e} + } + asc := func(name string) []*searchsvc.SortProperty { + return []*searchsvc.SortProperty{{Name: name}} + } + desc := func(name string) []*searchsvc.SortProperty { + return []*searchsvc.SortProperty{{Name: name, IsDescending: true}} + } + + It("compares string fields lexicographically", func() { + a := match(func(e *searchmsg.Entity) { e.Name = "a.jpg" }) + b := match(func(e *searchmsg.Entity) { e.Name = "b.jpg" }) + Expect(search.CompareMatches(a, b, asc("name"))).To(Equal(-1)) + Expect(search.CompareMatches(b, a, asc("name"))).To(Equal(1)) + Expect(search.CompareMatches(a, b, desc("name"))).To(Equal(1)) + }) + + It("compares numeric fields numerically", func() { + small := match(func(e *searchmsg.Entity) { e.Size = 9 }) + big := match(func(e *searchmsg.Entity) { e.Size = 10 }) + Expect(search.CompareMatches(small, big, asc("size"))).To(Equal(-1)) + Expect(search.CompareMatches(small, big, desc("size"))).To(Equal(1)) + }) + + It("compares timestamps", func() { + older := match(func(e *searchmsg.Entity) { + e.Photo = &searchmsg.Photo{TakenDateTime: ×tamppb.Timestamp{Seconds: 100}} + }) + newer := match(func(e *searchmsg.Entity) { + e.Photo = &searchmsg.Photo{TakenDateTime: ×tamppb.Timestamp{Seconds: 200}} + }) + Expect(search.CompareMatches(older, newer, asc("photo.takenDateTime"))).To(Equal(-1)) + Expect(search.CompareMatches(older, newer, desc("photo.takenDateTime"))).To(Equal(1)) + }) + + It("compares lastModifiedDateTime via the entity's lastModifiedTime", func() { + older := match(func(e *searchmsg.Entity) { + e.LastModifiedTime = ×tamppb.Timestamp{Seconds: 100} + }) + newer := match(func(e *searchmsg.Entity) { + e.LastModifiedTime = ×tamppb.Timestamp{Seconds: 200} + }) + Expect(search.CompareMatches(older, newer, asc("lastModifiedDateTime"))).To(Equal(-1)) + }) + + It("sorts matches missing the field after those that have it, in both directions", func() { + has := match(func(e *searchmsg.Entity) { + e.Photo = &searchmsg.Photo{TakenDateTime: ×tamppb.Timestamp{Seconds: 100}} + }) + missing := match(func(e *searchmsg.Entity) {}) + Expect(search.CompareMatches(has, missing, asc("photo.takenDateTime"))).To(Equal(-1)) + Expect(search.CompareMatches(missing, has, asc("photo.takenDateTime"))).To(Equal(1)) + Expect(search.CompareMatches(has, missing, desc("photo.takenDateTime"))).To(Equal(-1)) + }) + + It("falls through to the next sort property on ties", func() { + a := match(func(e *searchmsg.Entity) { e.Size = 5; e.Name = "a" }) + b := match(func(e *searchmsg.Entity) { e.Size = 5; e.Name = "b" }) + orderBy := []*searchsvc.SortProperty{{Name: "size"}, {Name: "name"}} + Expect(search.CompareMatches(a, b, orderBy)).To(Equal(-1)) + }) + + It("returns 0 for full ties and empty orderBy", func() { + a := match(func(e *searchmsg.Entity) { e.Size = 5 }) + b := match(func(e *searchmsg.Entity) { e.Size = 5 }) + Expect(search.CompareMatches(a, b, asc("size"))).To(Equal(0)) + Expect(search.CompareMatches(a, b, nil)).To(Equal(0)) + }) +}) diff --git a/vendor/github.com/opencloud-eu/libre-graph-api-go/model_sort_property.go b/vendor/github.com/opencloud-eu/libre-graph-api-go/model_sort_property.go index 9a463a95f1..ff7b63e42a 100644 --- a/vendor/github.com/opencloud-eu/libre-graph-api-go/model_sort_property.go +++ b/vendor/github.com/opencloud-eu/libre-graph-api-go/model_sort_property.go @@ -21,7 +21,7 @@ var _ MappedNullable = &SortProperty{} // SortProperty Indicates the order to sort search results in. Follows the [MS Graph sortProperty](https://learn.microsoft.com/en-us/graph/api/resources/sortproperty) resource type. type SortProperty struct { - // The name of the property to sort the search results by. The sortable fields are `name`, `size`, `lastModifiedDateTime` and `photo.takenDateTime`. Sorting by any other field is rejected with `invalidRequest`. Required. + // The name of the property to sort the search results by. Sortable are the scalar properties carried on the search hit's resource: `name`, `size`, `lastModifiedDateTime`, `mimeType` and the scalar facet properties such as `photo.takenDateTime`, `photo.iso`, `audio.artist`, `audio.year` or `image.width`. Strings sort lexicographically, numbers and dates by value. Multivalued properties (e.g. `tags`) and unknown properties are rejected with `invalidRequest`. Required. Name string `json:"name"` // Set to `true` to sort the results in descending order. Optional, defaults to `false` (ascending). IsDescending *bool `json:"isDescending,omitempty"`