Implement sorting of collections in REST API

Author: schlatterbeck

doc/rest.txt

@@ -344,6 +344,20 @@ Other data types: Date, Interval Integer, Number need examples and may
 need work to allow range searches. Full text search (e.g. over the
 body of msgs) is a work in progress.
 
+Sorting
+^^^^^^^
+
+Collection endpoints support sorting. This is controlled by specifying a
+``@sort`` parameter with a list of properties of the searched class.
+Optionally properties can include a sign ('+' or '-') to specify
+ascending or descending sort, respectively. If no sign is given,
+ascending sort is selected for this property. The following example
+would sort by status (in ascending order of the status.order property)
+and then by id of an issue::
+
+    @sort=status,-id
+
+
 Pagination
 ^^^^^^^^^^
 

roundup/rest.py

@@ -650,6 +650,7 @@ def get_collection(self, class_name, input):
         }
         verbose = 1
         display_props = {}
+        sort = []
         for form_field in input.value:
             key = form_field.name
             value = form_field.value
@@ -670,6 +671,24 @@ def get_collection(self, class_name, input):
                     except KeyError as err:
                         raise UsageError("Failed to find property '%s' "
                                          "for class %s."%(i, class_name))
+            elif key == "@sort":
+                f = value.split(",")
+                allprops=class_obj.getprops(protected=True)
+                for p in f :
+                    if not p:
+                        raise UsageError("Empty property "
+                                         "for class %s."%(class_name))
+                    if p[0] in ('-', '+'):
+                        pn = p[1:]
+                        ss = p[0]
+                    else:
+                        ss = '+'
+                        pn = p
+                    # Only include properties where we have search permission
+                    if self.db.security.hasSearchPermission(
+                        uid, class_name, pn
+                    ):
+                        sort.append((ss, pn))
             elif key.startswith("@"):
                 # ignore any unsupported/previously handled control key
                 # like @apiver
@@ -707,17 +726,13 @@ def get_collection(self, class_name, input):
                             filter_props[key]=[filter_props[key],value]
                     else:
                         filter_props[key] = value
-        if not filter_props:
-            obj_list = class_obj.list()
-        else:
-            obj_list = class_obj.filter(None, filter_props)
-
-        # Sort list as specified by sortorder
-        # This is more useful for things where there is an
-        # explicit order. E.G. status has an order that is
-        # roughly the progression of the issue through
-        # the states so open is before closed.
-        obj_list.sort()
+        l = [filter_props]
+        if sort:
+            l.append(sort)
+        obj_list = class_obj.filter(None, *l)
+
+        # Note: We don't sort explicitly in python. The filter implementation
+        # of the DB already sorts by ID if no sort option was given.
 
         # add verbose elements. 2 and above get identifying label.
         if verbose > 1:

test/rest_common.py

@@ -118,6 +118,46 @@ def get_header (self, header, not_found=None):
         except (AttributeError, KeyError, TypeError):
             return not_found
 
+    def create_stati(self):
+        try:
+            self.db.status.create(name='open', order='9')
+        except ValueError:
+            pass
+        try:
+            self.db.status.create(name='closed', order='91')
+        except ValueError:
+            pass
+        try:
+            self.db.priority.create(name='normal')
+        except ValueError:
+            pass
+        try:
+            self.db.priority.create(name='critical')
+        except ValueError:
+            pass
+
+    def create_sampledata(self):
+        """ Create sample data common to some test cases
+        """
+        self.create_stati()
+        self.db.issue.create(
+            title='foo1',
+            status=self.db.status.lookup('open'),
+            priority=self.db.priority.lookup('normal'),
+            nosy = [ "1", "2" ]
+        )
+        issue_open_norm = self.db.issue.create(
+            title='foo2',
+            status=self.db.status.lookup('open'),
+            priority=self.db.priority.lookup('normal'),
+            assignedto = "3"
+        )
+        issue_open_crit = self.db.issue.create(
+            title='foo5',
+            status=self.db.status.lookup('open'),
+            priority=self.db.priority.lookup('critical')
+        )
+
     def testGet(self):
         """
         Retrieve all three users
@@ -167,40 +207,7 @@ def testOutputFormat(self):
         """ test of @fields and @verbose implementation """
 
         self.maxDiff = 4000
-        # create sample data
-        try:
-            self.db.status.create(name='open')
-        except ValueError:
-            pass
-        try:
-            self.db.status.create(name='closed')
-        except ValueError:
-            pass
-        try:
-            self.db.priority.create(name='normal')
-        except ValueError:
-            pass
-        try:
-            self.db.priority.create(name='critical')
-        except ValueError:
-            pass
-        self.db.issue.create(
-            title='foo1',
-            status=self.db.status.lookup('open'),
-            priority=self.db.priority.lookup('normal'),
-            nosy = [ "1", "2" ]
-        )
-        issue_open_norm = self.db.issue.create(
-            title='foo2',
-            status=self.db.status.lookup('open'),
-            priority=self.db.priority.lookup('normal'),
-            assignedto = "3"
-        )
-        issue_open_crit = self.db.issue.create(
-            title='foo5',
-            status=self.db.status.lookup('open'),
-            priority=self.db.priority.lookup('critical')
-        )
+        self.create_sampledata()
         base_path = self.db.config['TRACKER_WEB'] + 'rest/data/issue/'
 
 
@@ -425,28 +432,41 @@ def testOutputFormat(self):
         results['data']['@etag'] = '' # etag depends on date, set to empty
         self.assertDictEqual(expected,results)
 
+    def testSorting(self):
+        self.maxDiff = 4000
+        self.create_sampledata()
+        self.db.issue.set('1', status='7')
+        self.db.issue.set('2', status='2')
+        self.db.issue.set('3', status='2')
+        self.db.commit()
+        base_path = self.db.config['TRACKER_WEB'] + 'rest/data/issue/'
+        # change some data for sorting on later
+        form = cgi.FieldStorage()
+        form.list = [
+            cgi.MiniFieldStorage('@fields', 'status'),
+            cgi.MiniFieldStorage('@sort', 'status,-id'),
+            cgi.MiniFieldStorage('@verbose', '0')
+        ]
+
+        # status is sorted by orderprop (property 'order')
+        # which provides the same ordering as the status ID
+        expected={'data': {
+            '@total_size': 3,
+            'collection': [
+                {'link': base_path + '3', 'status': '2', 'id': '3'},
+                {'link': base_path + '2', 'status': '2', 'id': '2'},
+                {'link': base_path + '1', 'status': '7', 'id': '1'}]}}
+
+        results = self.server.get_collection('issue', form)
+        self.assertDictEqual(expected, results)
+
     def testFilter(self):
         """
         Retrieve all three users
         obtain data for 'joe'
         """
         # create sample data
-        try:
-            self.db.status.create(name='open')
-        except ValueError:
-            pass
-        try:
-            self.db.status.create(name='closed')
-        except ValueError:
-            pass
-        try:
-            self.db.priority.create(name='normal')
-        except ValueError:
-            pass
-        try:
-            self.db.priority.create(name='critical')
-        except ValueError:
-            pass
+        self.create_stati()
         self.db.issue.create(
             title='foo4',
             status=self.db.status.lookup('closed'),