Merge pull request #4115 from kinow/warn-invalid-sort-keys

Raise an error when invalid sort keys are provided by a GraphQL client

CHANGES.md

@@ -72,6 +72,9 @@ how workflows are restarted
 
 ### Enhancements
 
+[#4115](https://github.com/cylc/cylc-flow/pull/4115) - Raise an error when
+invalid sort keys are provided clients.
+
 [#4105](https://github.com/cylc/cylc-flow/pull/4105) - Replace the
 `cylc executable` global config setting with `cylc path`, for consistency with
 `cylc` invocation in job scripts.

cylc/flow/network/schema.py

@@ -76,15 +76,19 @@ def sort_elements(elements, args):
     """Sort iterable of elements by given attribute."""
     sort_args = args.get('sort')
     if sort_args and elements:
-        sort_keys = [
-            key
-            for key in [to_snake_case(k) for k in sort_args.keys]
-            if hasattr(elements[0], key)
-        ]
-        if sort_keys:
-            elements.sort(
-                key=attrgetter(*sort_keys),
-                reverse=sort_args.reverse)
+        keys = [
+            key for key in [to_snake_case(k) for k in sort_args.keys]]
+        if not keys:
+            raise ValueError('You must provide at least one key to sort')
+        keys_not_in_schema = [
+            key for key in keys if not hasattr(elements[0], key)]
+        if keys_not_in_schema:
+            raise ValueError(f'''The following sort keys are not in the
+            schema: {', '.join(keys_not_in_schema)}''')
+        # sort using the keys provided
+        elements.sort(
+            key=attrgetter(*keys),
+            reverse=sort_args.reverse)
     return elements
 
 

tests/unit/network/test_schema.py

@@ -0,0 +1,84 @@
+# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from dataclasses import dataclass
+from inspect import isclass
+
+import pytest
+
+from cylc.flow.network.schema import sort_elements, SortArgs
+
+
+@dataclass
+class DummyObject:
+    value: int
+
+
+@pytest.mark.parametrize(
+    'elements,sort_args,expected_result',
+    [
+        # sort asc by key
+        (
+            [DummyObject(1), DummyObject(3), DummyObject(2)],
+            {
+                'keys': ['value'],
+                'reverse': False  # NOTE: GraphQL ensures reverse is not None!
+            },
+            [DummyObject(1), DummyObject(2), DummyObject(3)]
+        ),
+        # sort desc by key
+        (
+            [DummyObject(1), DummyObject(3), DummyObject(2)],
+            {
+                'keys': ['value'],
+                'reverse': True
+            },
+            [DummyObject(3), DummyObject(2), DummyObject(1)]
+        ),
+        # raise error when no keys given
+        (
+            [DummyObject(1), DummyObject(3), DummyObject(2)],
+            {
+                'keys': [],
+                'reverse': True
+            },
+            ValueError
+        ),
+        # raise error when any of the keys given are not in the schema
+        (
+            [DummyObject(1), DummyObject(3), DummyObject(2)],
+            {
+                'keys': ['value', 'river_name'],
+                'reverse': True
+            },
+            ValueError
+        )
+    ]
+)
+def test_sort_args(elements, sort_args, expected_result):
+    """Test the sorting function used by the schema."""
+    sort = SortArgs()
+    sort.keys = sort_args['keys']
+    sort.reverse = sort_args['reverse']
+    args = {
+        'sort': sort
+    }
+    if isclass(expected_result):
+        with pytest.raises(expected_result):
+            sort_elements(elements, args)
+    else:
+        sort_elements(elements, args)
+        assert elements == expected_result