Test cases and cleanup

lib/pbench/server/api/resources/datasets_list.py

@@ -5,7 +5,7 @@
 from flask import current_app
 from flask.json import jsonify
 from flask.wrappers import Request, Response
-from sqlalchemy import and_, asc, cast, func, desc, or_, String
+from sqlalchemy import and_, asc, cast, desc, func, or_, String
 from sqlalchemy.exc import ProgrammingError, StatementError
 from sqlalchemy.orm import aliased, Query
 from sqlalchemy.sql.expression import Alias
@@ -404,21 +404,15 @@ def datasets(
                 k = sort
                 order = asc
             else:
-                try:
-                    k, o = sort.split(":", maxsplit=1)
-                except ValueError:
-                    raise APIAbort(
-                        HTTPStatus.BAD_REQUEST,
-                        f"sort {sort!r} must have the form 'k[:o]'",
-                    )
+                k, o = sort.split(":", maxsplit=1)
                 if o.lower() == "asc":
                     order = asc
                 elif o.lower() == "desc":
                     order = desc
                 else:
                     raise APIAbort(
                         HTTPStatus.BAD_REQUEST,
-                        f"sort order in {sort!r} must be 'asc' or 'desc'",
+                        f"The sort order in {sort!r} must be 'asc' or 'desc'",
                     )
 
             if not Metadata.is_key_path(k, Metadata.METADATA_KEYS, metalog_key_ok=True):

lib/pbench/test/unit/server/conftest.py

@@ -386,8 +386,8 @@ def more_datasets(
         test        20 private  1970-01-01:00:42
         fio_1        3 public   1978-06-26:08:00
         fio_2       20 public   2022-01-01:00:00
-        uperf_1     20 private  1978-06-26:08:10
-        uperf_2     20 private  1978-06-26:09:01
+        uperf_1     20 private  1978-06-26:08:01
+        uperf_2     20 private  1978-06-26:09:00
         uperf_3     20 private  1978-06-26:09:30
         uperf_4     20 private  1978-06-26:10:00
 

lib/pbench/test/unit/server/test_datasets_list.py

@@ -816,45 +816,65 @@ def test_key_and_dates(self, query_as):
         "sort,results",
         [
             (
+                # Simple sort by name
                 "dataset.name",
                 ["fio_1", "fio_2", "test", "uperf_1", "uperf_2", "uperf_3", "uperf_4"],
             ),
             (
+                # Simple sort by name with explicit "ascending" order
                 "dataset.name:asc",
                 ["fio_1", "fio_2", "test", "uperf_1", "uperf_2", "uperf_3", "uperf_4"],
             ),
             (
+                # Simple sort by name with "descending" order
                 "dataset.name:desc",
                 ["uperf_4", "uperf_3", "uperf_2", "uperf_1", "test", "fio_2", "fio_1"],
             ),
             (
+                # Sort by date timestamp
                 "dataset.uploaded",
                 ["test", "fio_1", "uperf_1", "uperf_2", "uperf_3", "uperf_4", "fio_2"],
             ),
             (
+                # Sort by date timestamp with descending order
                 "dataset.uploaded:desc",
                 ["fio_2", "uperf_4", "uperf_3", "uperf_2", "uperf_1", "fio_1", "test"],
             ),
             (
+                # Sort by a "dataset.metalog" value
                 "dataset.metalog.run.controller",
                 ["test", "fio_1", "fio_2", "uperf_1", "uperf_2", "uperf_3", "uperf_4"],
             ),
             (
+                # Sort by a general global metadata value in descending order
                 "global.test.sequence:desc",
                 ["fio_1", "fio_2", "test", "uperf_1", "uperf_2", "uperf_3", "uperf_4"],
             ),
             (
+                # Sprt by a general global metadata value in ascending order
                 "global.test.sequence",
                 ["uperf_4", "uperf_3", "uperf_2", "uperf_1", "test", "fio_2", "fio_1"],
             ),
             (
+                # Sort two keys across distinct metadata namespaces asc/desc
                 "user.test.odd,global.test.sequence:desc",
                 ["fio_1", "test", "uperf_2", "uperf_4", "fio_2", "uperf_1", "uperf_3"],
             ),
             (
+                # Sort two keys across distinct metadata namespaces desc/desc
                 "user.test.odd:desc,dataset.name:desc",
                 ["uperf_3", "uperf_1", "fio_2", "uperf_4", "uperf_2", "test", "fio_1"],
             ),
+            (
+                # Sort by a JSON sub-object containing two keys ascending
+                "global.test",
+                ["uperf_4", "uperf_3", "uperf_2", "uperf_1", "test", "fio_2", "fio_1"],
+            ),
+            (
+                # Sort by a JSON sub-object containing two keys descending
+                "global.test:desc",
+                ["fio_1", "fio_2", "test", "uperf_1", "uperf_2", "uperf_3", "uperf_4"],
+            ),
         ],
     )
     def test_dataset_sort(self, server_config, query_as, sort, results):
@@ -868,9 +888,7 @@ def test_dataset_sort(self, server_config, query_as, sort, results):
         Args:
             server_config: The PbenchServerConfig object
             query_as: A fixture to provide a helper that executes the API call
-            login: The username as which to perform a query
-            query: A JSON representation of the query parameters (these will be
-                automatically supplemented with a metadata request term)
+            sort: A JSON representation of the sort query parameter value
             results: A list of the dataset names we expect to be returned
         """
 
@@ -880,7 +898,54 @@ def test_dataset_sort(self, server_config, query_as, sort, results):
         for i, d in enumerate(all):
             odd = i & 1
             Metadata.setvalue(d, "global.test.sequence", i)
+            Metadata.setvalue(d, "global.test.mcguffin", 100 - i)
             Metadata.setvalue(d, "user.test.odd", odd, user=test)
         query = {"sort": sort, "metadata": ["dataset.uploaded"]}
         result = query_as(query, "test", HTTPStatus.OK)
         self.compare_results(result.json, results, query, server_config)
+
+    @pytest.mark.parametrize(
+        "sort,message",
+        [
+            (
+                # Specify a sort by a Dataset table column that doesn't exist
+                "dataset.noname",
+                "Metadata key 'dataset.noname' is not supported",
+            ),
+            (
+                # Specify a sort using an undefined order keyword
+                "dataset.name:backwards",
+                "The sort order in 'dataset.name:backwards' must be 'asc' or 'desc'",
+            ),
+            (
+                # Specify a sort using bad sort order syntax
+                "dataset.name:desc:",
+                "The sort order in 'dataset.name:desc:' must be 'asc' or 'desc'",
+            ),
+            (
+                # Specify a sort using a bad metadata namespace
+                "xyzzy.uploaded",
+                "Metadata key 'xyzzy.uploaded' is not supported",
+            ),
+        ],
+    )
+    def test_dataset_sort_errors(self, server_config, query_as, sort, message):
+        """Test `datasets/list?sort` error cases
+
+        Args:
+            server_config: The PbenchServerConfig object
+            query_as: A fixture to provide a helper that executes the API call
+            sort: A JSON representation of the sort query parameter value
+            message: The expected error message
+        """
+
+        # Assign "sequence numbers" in the inverse order of name
+        test = User.query(username="test")
+        all = Database.db_session.query(Dataset).order_by(desc(Dataset.name)).all()
+        for i, d in enumerate(all):
+            odd = i & 1
+            Metadata.setvalue(d, "global.test.sequence", i)
+            Metadata.setvalue(d, "user.test.odd", odd, user=test)
+        query = {"sort": sort}
+        result = query_as(query, "test", HTTPStatus.BAD_REQUEST)
+        assert result.json["message"] == message