feat(search): sort results via sortProperties / order_by

protogen/gen/opencloud/services/search/v0/search.swagger.json

@@ -742,6 +742,13 @@
             "$ref": "#/definitions/v0AggregationFilter"
           },
           "description": "Optional. Filters narrowing the matches to buckets of a prior\naggregation; the engine ANDs them with `query`."
+        },
+        "orderBy": {
+          "type": "array",
+          "items": {
+            "$ref": "#/definitions/v0SortProperty"
+          },
+          "description": "Optional. Fields to sort the matches by, in order of precedence. When\nempty, matches are sorted by relevance score. Each backend translates\nthis to its native sort (bleve: SortBy, OpenSearch: sort clause)."
         }
       }
     },
@@ -807,6 +814,13 @@
           "type": "integer",
           "format": "int32",
           "description": "Optional. Number of leading matches to skip in the globally sorted,\ncross-space merged result list."
+        },
+        "orderBy": {
+          "type": "array",
+          "items": {
+            "$ref": "#/definitions/v0SortProperty"
+          },
+          "description": "Optional. Fields to sort the matches by, in order of precedence. When\nempty, matches are sorted by relevance score. Only a subset of the\nindexed fields is sortable; the graph service validates this before\nforwarding the request."
         }
       }
     },
@@ -836,6 +850,19 @@
         }
       }
     },
+    "v0SortProperty": {
+      "type": "object",
+      "properties": {
+        "name": {
+          "type": "string",
+          "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",
+          "description": "Optional. Sort in descending order. Defaults to ascending."
+        }
+      }
+    },
     "v0Video": {
       "type": "object",
       "properties": {

protogen/proto/opencloud/services/search/v0/search.proto

@@ -83,6 +83,11 @@ message SearchRequest {
   // Optional. Number of leading matches to skip in the globally sorted,
   // cross-space merged result list.
   int32 from = 7 [(google.api.field_behavior) = OPTIONAL];
+  // Optional. Fields to sort the matches by, in order of precedence. When
+  // empty, matches are sorted by relevance score. Only a subset of the
+  // indexed fields is sortable; the graph service validates this before
+  // forwarding the request.
+  repeated SortProperty order_by = 8 [(google.api.field_behavior) = OPTIONAL];
 }
 
 message SearchResponse {
@@ -115,6 +120,10 @@ message SearchIndexRequest {
   // Optional. Filters narrowing the matches to buckets of a prior
   // aggregation; the engine ANDs them with `query`.
   repeated AggregationFilter aggregation_filters = 6 [(google.api.field_behavior) = OPTIONAL];
+  // Optional. Fields to sort the matches by, in order of precedence. When
+  // empty, matches are sorted by relevance score. Each backend translates
+  // this to its native sort (bleve: SortBy, OpenSearch: sort clause).
+  repeated SortProperty order_by = 7 [(google.api.field_behavior) = OPTIONAL];
 }
 
 message SearchIndexResponse {
@@ -155,6 +164,18 @@ message MetricDefinition {
   MetricKind kind = 1;
 }
 
+message SortProperty {
+  // 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;
+}
+
 enum MetricKind {
   METRIC_KIND_UNSPECIFIED = 0;
   METRIC_KIND_SUM = 1;

services/graph/pkg/service/v0/searchquery.go

@@ -86,9 +86,6 @@ func validateSearchExpand(r *http.Request) error {
 // unsupportedProperty names a property of the spec this endpoint does not
 // evaluate yet.
 func unsupportedProperty(sr libregraph.SearchRequest) string {
-	if len(sr.SortProperties) > 0 {
-		return "sortProperties"
-	}
 	var geohash func(aggs []libregraph.AggregationOption) bool
 	geohash = func(aggs []libregraph.AggregationOption) bool {
 		for _, a := range aggs {
@@ -117,6 +114,9 @@ func searchRequestOf(sr libregraph.SearchRequest) (*searchsvc.SearchRequest, err
 	if err := validateAggregations(sr.Aggregations); err != nil {
 		return nil, err
 	}
+	if err := validateSortProperties(sr.SortProperties); err != nil {
+		return nil, err
+	}
 	aggregations := libregraphAggregationsToSearch(sr.Aggregations)
 	filters, err := aggregationFiltersToSearch(sr.AggregationFilters)
 	if err != nil {
@@ -141,6 +141,7 @@ func searchRequestOf(sr libregraph.SearchRequest) (*searchsvc.SearchRequest, err
 		PageSize:           &size,
 		Aggregations:       aggregations,
 		AggregationFilters: filters,
+		OrderBy:            libregraphSortToSearch(sr.SortProperties),
 	}, nil
 }
 
@@ -262,6 +263,34 @@ func validateAggregations(aggs []libregraph.AggregationOption) error {
 	return nil
 }
 
+// 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 !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
+}
+
+func libregraphSortToSearch(in []libregraph.SortProperty) []*searchsvc.SortProperty {
+	if len(in) == 0 {
+		return nil
+	}
+	out := make([]*searchsvc.SortProperty, 0, len(in))
+	for _, sp := range in {
+		p := &searchsvc.SortProperty{Name: sp.Name}
+		if sp.IsDescending != nil {
+			p.IsDescending = *sp.IsDescending
+		}
+		out = append(out, p)
+	}
+	return out
+}
+
 // renderSearchError answers with the status the search service failed with.
 func (g Graph) renderSearchError(w http.ResponseWriter, r *http.Request, err error) {
 	e := merrors.Parse(err.Error())

services/graph/pkg/service/v0/searchquery_test.go

@@ -412,13 +412,53 @@ var _ = ginkgo.Describe("SearchQuery", func() {
 			Expect(rr.Body.String()).To(ContainSubstring("notSupported"))
 			Expect(rr.Body.String()).To(ContainSubstring(property))
 		},
-		ginkgo.Entry("sortProperties", `"sortProperties": [{"name": "name"}]`, "sortProperties"),
 		ginkgo.Entry("a geohash aggregation",
 			`"aggregations": [{"field": "location", "@libre.graph.geohashDefinition": {"precision": 5}}]`, "geohashDefinition"),
 		ginkgo.Entry("a nested geohash aggregation",
 			`"aggregations": [{"field": "audio.artist", "@libre.graph.subAggregations": [{"field": "location", "@libre.graph.geohashDefinition": {"precision": 5}}]}]`, "geohashDefinition"),
 	)
 
+	ginkgo.It("forwards sortProperties to the search service as order_by", func() {
+		g, captured := graphWithSearchAnswer(&searchsvc.SearchResponse{})
+		rr := postSearchQuery(g, searchQueryBody(`"sortProperties": [{"name": "photo.takenDateTime", "isDescending": true}, {"name": "name"}]`))
+		Expect(rr.Code).To(Equal(http.StatusOK), rr.Body.String())
+
+		Expect(captured().GetOrderBy()).To(HaveLen(2))
+		Expect(captured().GetOrderBy()[0].GetName()).To(Equal("photo.takenDateTime"))
+		Expect(captured().GetOrderBy()[0].GetIsDescending()).To(BeTrue())
+		Expect(captured().GetOrderBy()[1].GetName()).To(Equal("name"))
+		Expect(captured().GetOrderBy()[1].GetIsDescending()).To(BeFalse())
+	})
+
+	ginkgo.DescribeTable("accepts sorting by scalar hit fields",
+		func(field string) {
+			g, _ := graphWithSearchAnswer(&searchsvc.SearchResponse{})
+			rr := postSearchQuery(g, searchQueryBody(fmt.Sprintf(`"sortProperties": [{"name": %q}]`, 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) {
+			rr := postSearchQuery(graphWithoutSearch(), searchQueryBody(fmt.Sprintf(`"sortProperties": [{"name": %q}]`, 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 an $expand it does not know with 400", func() {
 		req := httptest.NewRequest(http.MethodPost, "/search/query?$expand=thumbnails,permissions", bytes.NewBufferString(searchQueryBody(`"from": 0`)))
 		rr := httptest.NewRecorder()

services/search/pkg/bleve/backend.go

@@ -2,6 +2,7 @@ package bleve
 
 import (
 	"context"
+	"fmt"
 	"math"
 	"time"
 
@@ -96,8 +97,19 @@ func (b *Backend) Search(ctx context.Context, sir *searchService.SearchIndexRequ
 
 	bleveReq := bleve.NewSearchRequest(q)
 	bleveReq.Highlight = bleve.NewHighlight()
-	// ties by id, like the cross-space merge
-	bleveReq.SortBy([]string{"-_score", "_id"})
+	// order_by first, then like the cross-space merge: score, ties by id
+	sortOrder := make([]string, 0, len(sir.GetOrderBy())+2)
+	for _, sp := range sir.GetOrderBy() {
+		field, ok := search.SortIndexField(sp.GetName())
+		if !ok {
+			return nil, errtypes.BadRequest(fmt.Sprintf("field %q is not sortable", sp.GetName()))
+		}
+		if sp.GetIsDescending() {
+			field = "-" + field
+		}
+		sortOrder = append(sortOrder, field)
+	}
+	bleveReq.SortBy(append(sortOrder, "-_score", "_id"))
 	bleveReq.Size = size
 
 	collector, err := newAggCollector(sir.GetAggregations())

services/search/pkg/opensearch/backend.go

@@ -126,14 +126,28 @@ func (b *Backend) Search(ctx context.Context, sir *searchService.SearchIndexRequ
 
 	searchParams := opensearchgoAPI.SearchParams{
 		SourceExcludes: []string{"Content"}, // Do not send back the full content in the search response, as it is only needed for highlighting and can be large. The highlighted snippets will be sent back in the response instead.
-		// ties by id, like the cross-space merge
-		Sort:        []string{"_score:desc", "ID:asc"},
-		TrackScores: conversions.ToPointer(true),
+		TrackScores:    conversions.ToPointer(true),
 		// count every match, the default stops at 10000
 		TrackTotalHits: true,
 		Size:           conversions.ToPointer(size),
 	}
 
+	// order_by first, missing values last in both directions, then like the
+	// cross-space merge: score, ties by id
+	sortClause := make([]map[string]any, 0, len(sir.GetOrderBy())+2)
+	for _, sp := range sir.GetOrderBy() {
+		field, ok := search.SortIndexField(sp.GetName())
+		if !ok {
+			return nil, errtypes.BadRequest(fmt.Sprintf("field %q is not sortable", sp.GetName()))
+		}
+		order := "asc"
+		if sp.GetIsDescending() {
+			order = "desc"
+		}
+		sortClause = append(sortClause, map[string]any{field: map[string]any{"order": order, "missing": "_last"}})
+	}
+	sortClause = append(sortClause, map[string]any{"_score": map[string]any{"order": "desc"}}, map[string]any{"ID": map[string]any{"order": "asc"}})
+
 	aggregationsBody, err := aggs.Build(sir.GetAggregations())
 	if err != nil {
 		return nil, errtypes.BadRequest(err.Error())
@@ -158,6 +172,7 @@ func (b *Backend) Search(ctx context.Context, sir *searchService.SearchIndexRequ
 				},
 			},
 			Aggs: aggregationsBody,
+			Sort: sortClause,
 		},
 	)
 	if err != nil {

services/search/pkg/opensearch/internal/osu/request.go

@@ -96,6 +96,7 @@ func BuildSearchReq(req *opensearchgoAPI.SearchReq, q Builder, p ...SearchBodyPa
 type SearchBodyParams struct {
 	Highlight *BodyParamHighlight `json:"highlight,omitempty"`
 	Aggs      map[string]any      `json:"aggs,omitempty"`
+	Sort      []map[string]any    `json:"sort,omitempty"`
 }
 
 //----------------------------------------------------------------------------//

services/search/pkg/search/service.go

@@ -313,8 +313,15 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se
 	}
 	aggregation.Finalize(req.GetAggregations(), aggregations)
 
-	// compile one sorted list of matches from all spaces and apply from/limit if needed
-	sort.Sort(matches)
+	// every engine answers in order_by order (by score without one); the merge
+	// re-establishes it across spaces, ties by score and id
+	orderBy := req.GetOrderBy()
+	sort.SliceStable(matches, func(i, j int) bool {
+		if c := CompareMatches(matches[i], matches[j], orderBy); c != 0 {
+			return c < 0
+		}
+		return matches.Less(i, j)
+	})
 	limit := PageSizeOrDefault(req.PageSize)
 	if from := int(req.GetFrom()); from > 0 {
 		if from < len(matches) {
@@ -469,6 +476,7 @@ func (s *Service) searchIndex(ctx context.Context, req *searchsvc.SearchRequest,
 		Query:              req.Query,
 		Aggregations:       req.GetAggregations(),
 		AggregationFilters: req.GetAggregationFilters(),
+		OrderBy:            req.GetOrderBy(),
 		Ref: &searchmsg.Reference{
 			ResourceId: searchRootID,
 			Path:       searchPathPrefix,

services/search/pkg/search/service_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/stretchr/testify/mock"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/testing/protocmp"
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/opencloud-eu/opencloud/pkg/conversions"
 	"github.com/opencloud-eu/opencloud/pkg/log"
@@ -345,6 +346,48 @@ var _ = Describe("Searchprovider", func() {
 			})
 		})
 
+		Context("with two personal spaces returning photos", func() {
+			photoMatch := func(space, name string, taken int64, score float32) *searchmsg.Match {
+				return &searchmsg.Match{Score: score, Entity: &searchmsg.Entity{
+					Id:    &searchmsg.ResourceID{StorageId: "storageid", SpaceId: space, OpaqueId: name},
+					Ref:   &searchmsg.Reference{ResourceId: &searchmsg.ResourceID{StorageId: "storageid", SpaceId: space, OpaqueId: space}, Path: "./" + name},
+					Name:  name,
+					Photo: &searchmsg.Photo{TakenDateTime: &timestamppb.Timestamp{Seconds: taken}},
+				}}
+			}
+			names := func(req *searchsvc.SearchRequest) []string {
+				res, err := s.Search(ctx, req)
+				Expect(err).ToNot(HaveOccurred())
+				out := []string{}
+				for _, m := range res.Matches {
+					out = append(out, m.GetEntity().GetName())
+				}
+				return out
+			}
+
+			BeforeEach(func() {
+				listsSpacesAB()
+				searchOfSpace("a").Return(&searchsvc.SearchIndexResponse{TotalMatches: 2, Matches: []*searchmsg.Match{
+					photoMatch("a", "a-old.jpg", 100, 0.9), photoMatch("a", "a-new.jpg", 300, 0.1),
+				}}, nil)
+				searchOfSpace("b").Return(&searchsvc.SearchIndexResponse{TotalMatches: 2, Matches: []*searchmsg.Match{
+					photoMatch("b", "b-newest.jpg", 400, 0.5), photoMatch("b", "b-mid.jpg", 200, 0.4),
+				}}, nil)
+			})
+
+			It("forwards order_by to the engine and merges matches across spaces in that order", func() {
+				byTaken := []*searchsvc.SortProperty{{Name: "photo.takenDateTime", IsDescending: true}}
+				Expect(names(&searchsvc.SearchRequest{Query: "mediatype:image", OrderBy: byTaken})).To(Equal([]string{"b-newest.jpg", "a-new.jpg", "b-mid.jpg", "a-old.jpg"}))
+				indexClient.AssertCalled(GinkgoT(), "Search", mock.Anything, mock.MatchedBy(func(req *searchsvc.SearchIndexRequest) bool {
+					return len(req.GetOrderBy()) == 1 && req.GetOrderBy()[0].GetName() == "photo.takenDateTime" && req.GetOrderBy()[0].GetIsDescending()
+				}))
+			})
+
+			It("merges matches by score when no order_by is given", func() {
+				Expect(names(&searchsvc.SearchRequest{Query: "mediatype:image"})).To(Equal([]string{"a-old.jpg", "b-newest.jpg", "b-mid.jpg", "a-new.jpg"}))
+			})
+		})
+
 		Context("with two personal spaces returning matches of the same score", func() {
 			match := func(space, name string) *searchmsg.Match {
 				return &searchmsg.Match{Score: 1, Entity: &searchmsg.Entity{