# HG changeset patch
# User Goffi <goffi@goffi.org>
# Date 1623001377 -7200
# Node ID 0e6e176cb5723c9cc125bde723c7feb293382718
# Parent  96059231d58f5f8c2cd5fc932f9cb8b4ad7a004b
pgsql: fix items order:

Due to a misinterpretation, items where returned in wrong order when RSM was used.
Pubsub vanilla `max_items` is reversing the items order to get last items, but when this
attribute is not used (RSM being used or not), the items must be in chronological order.

This patch fixes it, so RSM returns oldest items by defaut, and empty `<before/>` must be
used to get last page (and thus most recent items).

Items are always finally ordered chronologically inside the returned page (default by
`update` time, but this can be modified when a specific order is requested).

Reference field is also fixed to use `updated` instead of `item_id` when item are ordered
by `update`.

diff -r 96059231d58f -r 0e6e176cb572 sat_pubsub/pgsql_storage.py
--- a/sat_pubsub/pgsql_storage.py	Thu Apr 22 18:27:03 2021 +0200
+++ b/sat_pubsub/pgsql_storage.py	Sun Jun 06 19:42:57 2021 +0200
@@ -1,5 +1,4 @@
 #!/usr/bin/env python3
-#-*- coding: utf-8 -*-
 
 # Copyright (c) 2012-2021 Jérôme Poisson
 # Copyright (c) 2013-2016 Adrien Cossa
@@ -882,7 +881,7 @@
 
     nodeType = 'leaf'
 
-    def getOrderBy(self, ext_data, direction='DESC'):
+    def getOrderBy(self, ext_data, direction='ASC'):
         """Return ORDER BY clause corresponding to Order By key in ext_data
 
         @param ext_data (dict): extra data as used in getItems
@@ -1139,7 +1138,7 @@
 
         query.extend(query_filters)
 
-        return self.getOrderBy(ext_data)
+        return self.getOrderBy(ext_data, direction="ASC")
 
     def _getItems(self, cursor, authorized_groups, unrestricted, maxItems, ext_data, ids_only):
         self._checkNodeExists(cursor)
@@ -1156,6 +1155,10 @@
             query = ["SELECT data::text,items.access_model,item_id,created,updated"]
 
         query_order = self._appendSourcesAndFilters(query, args, authorized_groups, unrestricted, ext_data)
+        if query_order.startswith("ORDER BY updated"):
+            ref_field = "updated"
+        else:
+            ref_field = "item_id"
 
         if 'rsm' in ext_data:
             rsm = ext_data['rsm']
@@ -1178,29 +1181,48 @@
             elif rsm.before is not None:
                 if rsm.before != '':
                     query.append(
-                        "AND item_id>(SELECT item_id FROM items WHERE node_id=%s AND "
-                        "item=%s LIMIT 1)"
+                        f"AND {ref_field}<(SELECT {ref_field} FROM items WHERE "
+                        "node_id=%s AND item=%s LIMIT 1)"
                     )
                     args.extend((self.nodeDbId, rsm.before))
-                if maxItems is not None:
-                    # if we have maxItems (i.e. a limit), we need to reverse order
-                    # in a first query to get the right items
-                    query.insert(0,"SELECT * from (")
-                    query.append(self.getOrderBy(ext_data, direction='ASC'))
-                    query.append("LIMIT %s) as x")
+                # we need to reverse order in a first query to get the right
+                # items
+                query.insert(0,"SELECT * from (")
+                query.append(self.getOrderBy(ext_data, direction='DESC'))
+                query.append("LIMIT %s) as x")
+                if maxItems is None:
+                    # we default to 10 max when we request last page
+                    log.msg(
+                        "WARNING: getting last page with RSM without max, "
+                        "we default to max=10"
+                    )
+                    args.append(10)
+                else:
                     args.append(maxItems)
             elif rsm.after:
                 query.append(
-                    "AND item_id<(SELECT item_id FROM items WHERE node_id=%s AND item=%s "
-                    "LIMIT 1)"
+                    f"AND {ref_field}>(SELECT {ref_field} FROM items WHERE node_id=%s "
+                    "AND item=%s LIMIT 1)"
                 )
                 args.extend((self.nodeDbId, rsm.after))
+        else:
+            rsm = False
 
         query.append(query_order)
 
         if maxItems is not None:
-            query.append("LIMIT %s")
-            args.append(maxItems)
+            if rsm == False:
+                # if we have legacy maxItems (i.e. not RSM max), we need to
+                # reverse order (maxItems get last items, cf.
+                # https://xmpp.org/extensions/xep-0060.html#subscriber-retrieve-requestrecent)
+                # in a first query to get the right items
+                query.insert(0,"SELECT * from (")
+                query.insert(len(query)-1, self.getOrderBy(ext_data, direction='DESC'))
+                query.insert(len(query)-1, "LIMIT %s) as x")
+                args.append(maxItems)
+            else:
+                query.append("LIMIT %s")
+                args.append(maxItems)
 
         cursor.execute(' '.join(query), args)