From 3e79e732728cde3dbff3f36c36965151125221f6 Mon Sep 17 00:00:00 2001
From: mpl <mathieu.lonjaret@gmail.com>
Date: Wed, 25 Feb 2015 16:08:13 +0100
Subject: [PATCH] search: implement post-search Created sort

Fix some UI queries, since UI always queries with "-created".

Also add Unsorted, to override the CreatedDesc default, for when we
really do not want to sort, e.g. when we don't have a corpus and
trying to sort would make us error out.

Issue #579

Change-Id: Ife0aa816c5e8cac8dc6612d0ffd104238abc6838
---
 pkg/search/query.go                        | 45 ++++++++++++++++++++--
 pkg/search/query_test.go                   | 13 +++++--
 server/camlistored/ui/server_connection.js |  2 +
 3 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/pkg/search/query.go b/pkg/search/query.go
index 2822a76f5..58200b746 100644
--- a/pkg/search/query.go
+++ b/pkg/search/query.go
@@ -44,6 +44,7 @@ type SortType int
 
 const (
 	UnspecifiedSort SortType = iota
+	Unsorted
 	LastModifiedDesc
 	LastModifiedAsc
 	CreatedDesc
@@ -53,6 +54,7 @@ const (
 )
 
 var sortName = map[SortType][]byte{
+	Unsorted:         []byte(`"unsorted"`),
 	LastModifiedDesc: []byte(`"-mod"`),
 	LastModifiedAsc:  []byte(`"mod"`),
 	CreatedDesc:      []byte(`"-created"`),
@@ -88,8 +90,13 @@ type SearchQuery struct {
 	Expression string      `json:"expression,omitempty"`
 	Constraint *Constraint `json:"constraint,omitempty"`
 
-	Limit int      `json:"limit,omitempty"` // optional. default is automatic. negative means no limit.
-	Sort  SortType `json:"sort,omitempty"`  // optional. default is automatic or unsorted.
+	// Limit is the maximum number of returned results. A negative value means no
+	// limit. If unspecified, a default (of 200) will be used.
+	Limit int `json:"limit,omitempty"`
+
+	// Sort specifies how the results will be sorted. It defaults to CreatedDesc when the
+	// query is about permanodes only.
+	Sort SortType `json:"sort,omitempty"`
 
 	// Continue specifies the opaque token (as returned by a
 	// SearchResult) for where to continue fetching results when
@@ -134,7 +141,7 @@ func (q *SearchQuery) plannedQuery(expr *SearchQuery) *SearchQuery {
 			pq.Limit = expr.Limit
 		}
 	}
-	if pq.Sort == 0 {
+	if pq.Sort == UnspecifiedSort {
 		if pq.Constraint.onlyMatchesPermanode() {
 			pq.Sort = CreatedDesc
 		}
@@ -862,12 +869,42 @@ func (h *Handler) Query(rawq *SearchQuery) (*SearchResult, error) {
 	}
 	if !cands.sorted {
 		switch q.Sort {
-		case UnspecifiedSort:
+		case UnspecifiedSort, Unsorted:
 			// Nothing to do.
 		case BlobRefAsc:
 			sort.Sort(sortSearchResultBlobs{res.Blobs, func(a, b *SearchResultBlob) bool {
 				return a.Blob.Less(b.Blob)
 			}})
+		case CreatedDesc, CreatedAsc:
+			if corpus == nil {
+				return nil, errors.New("TODO: Sorting without a corpus unsupported")
+			}
+			var err error
+			corpus.RLock()
+			sort.Sort(sortSearchResultBlobs{res.Blobs, func(a, b *SearchResultBlob) bool {
+				if err != nil {
+					return false
+				}
+				ta, ok := corpus.PermanodeAnyTimeLocked(a.Blob)
+				if !ok {
+					err = fmt.Errorf("no ctime or modtime found for %v", a.Blob)
+					return false
+				}
+				tb, ok := corpus.PermanodeAnyTimeLocked(b.Blob)
+				if !ok {
+					err = fmt.Errorf("no ctime or modtime found for %v", b.Blob)
+					return false
+				}
+				if q.Sort == CreatedAsc {
+					return ta.Before(tb)
+				}
+				return tb.Before(ta)
+			}})
+			corpus.RUnlock()
+			if err != nil {
+				return nil, err
+			}
+		// TODO(mpl): LastModifiedDesc, LastModifiedAsc
 		default:
 			return nil, errors.New("TODO: unsupported sort+query combination.")
 		}
diff --git a/pkg/search/query_test.go b/pkg/search/query_test.go
index 406b2a39f..d115a46a3 100644
--- a/pkg/search/query_test.go
+++ b/pkg/search/query_test.go
@@ -49,8 +49,9 @@ func (i indexType) String() string {
 }
 
 type queryTest struct {
-	t  testing.TB
-	id *indextest.IndexDeps
+	t     testing.TB
+	id    *indextest.IndexDeps
+	itype indexType
 
 	Handler func() *Handler
 }
@@ -90,8 +91,9 @@ func testQueryType(t testing.TB, fn func(*queryTest), itype indexType) {
 		}
 	}
 	qt := &queryTest{
-		t:  t,
-		id: indextest.NewIndexDeps(idx),
+		t:     t,
+		id:    indextest.NewIndexDeps(idx),
+		itype: itype,
 	}
 	qt.id.Fataler = t
 	qt.Handler = func() *Handler {
@@ -118,6 +120,9 @@ func dumpRes(t *testing.T, res *SearchResult) {
 }
 
 func (qt *queryTest) wantRes(req *SearchQuery, wanted ...blob.Ref) {
+	if qt.itype == indexClassic {
+		req.Sort = Unsorted
+	}
 	res, err := qt.Handler().Query(req)
 	if err != nil {
 		qt.t.Fatal(err)
diff --git a/server/camlistored/ui/server_connection.js b/server/camlistored/ui/server_connection.js
index 9d73942d3..ac0641539 100644
--- a/server/camlistored/ui/server_connection.js
+++ b/server/camlistored/ui/server_connection.js
@@ -218,6 +218,8 @@ cam.ServerConnection.prototype.permanodeOfSignerAttrValue = function(signer, att
 // @param {?object} opt_describe The describe property to send for the query
 cam.ServerConnection.prototype.buildQuery = function(callerQuery, opt_describe, opt_limit, opt_continuationToken) {
 	var query = {
+		// TODO(mpl): it'd be better to not ask for a sort when none is needed (less work for server),
+		// e.g. for a plain BlobRefPrefix query.
 		sort: "-created"
 	};