Fix T49280: Make texture files (in texture node) sortable

Rather than making them sortable, I made them automatically sorted upon
saving the node. The colour map comes first, then the other maps in
alphabetical order.

pillar/api/nodes/__init__.py

@@ -402,6 +402,29 @@ def nodes_convert_markdown(nodes):
         convert_markdown(node)
 
 
+only_for_textures = only_for_node_type_decorator('texture')
+
+
+@only_for_textures
+def texture_sort_files(node, original=None):
+    """Sort files alphabetically by map type, with colour map first."""
+
+    try:
+        files = node['properties']['files']
+    except KeyError:
+        return
+
+    # Sort the map types alphabetically, ensuring 'color' comes first.
+    as_dict = {f['map_type']: f for f in files}
+    types = sorted(as_dict.keys(), key=lambda k: '\0' if k == 'color' else k)
+    node['properties']['files'] = [as_dict[map_type] for map_type in types]
+
+
+def textures_sort_files(nodes):
+    for node in nodes:
+        texture_sort_files(node)
+
+
 def setup_app(app, url_prefix):
     from . import patch
     patch.setup_app(app, url_prefix=url_prefix)
@@ -411,6 +434,7 @@ def setup_app(app, url_prefix):
 
     app.on_replace_nodes += before_replacing_node
     app.on_replace_nodes += convert_markdown
+    app.on_replace_nodes += texture_sort_files
     app.on_replace_nodes += deduct_content_type
     app.on_replace_nodes += node_set_default_picture
     app.on_replaced_nodes += after_replacing_node
@@ -419,9 +443,11 @@ def setup_app(app, url_prefix):
     app.on_insert_nodes += nodes_deduct_content_type
     app.on_insert_nodes += nodes_set_default_picture
     app.on_insert_nodes += nodes_convert_markdown
+    app.on_insert_nodes += textures_sort_files
     app.on_inserted_nodes += after_inserting_nodes
 
     app.on_update_nodes += convert_markdown
+    app.on_update_nodes += texture_sort_files
 
     app.on_delete_item_nodes += before_deleting_node
     app.on_deleted_item_nodes += after_deleting_node

tests/test_api/test_nodes.py

@@ -376,3 +376,102 @@ class NodeSharingTest(AbstractPillarTest):
         node = resp.json()
         self.assertNotIn('short_code', node)
         self.assertNotIn('short_link', node)
+
+
+class TextureSortFilesTest(AbstractPillarTest):
+    def setUp(self, **kwargs):
+        super().setUp(**kwargs)
+
+        self.create_valid_auth_token(user_id=ctd.EXAMPLE_PROJECT_OWNER_ID, token='token')
+        self.project_id, _ = self.ensure_project_exists()
+
+    def mkfile(self, file_id: str):
+        file_id, _ = self.ensure_file_exists(file_overrides={
+            '_id': ObjectId(file_id),
+            'content_type': 'image/png'})
+        return file_id
+
+    def test_happy(self):
+        file_id_1 = self.mkfile('cafef00dcafef00dcafef00d')
+        file_id_2 = self.mkfile('cafef00dcafef00dcafecafe')
+        file_id_3 = self.mkfile('cafef00dcafef00ddeadbeef')
+
+        # Create a texture node in the 'wrong' order
+        resp = self.post('/api/nodes', expected_status=201, auth_token='token', json={
+            'project': self.project_id,
+            'node_type': 'texture',
+            'name': str(self),
+            'properties': {
+                'files': [
+                    {'map_type': 'specular', 'file': file_id_1},
+                    {'map_type': 'color', 'file': file_id_2},
+                    {'map_type': 'alpha', 'file': file_id_3},
+                ]
+            },
+            'user': ctd.EXAMPLE_PROJECT_OWNER_ID,
+        })
+        node_id = resp.json()['_id']
+
+        resp = self.get(f'/api/nodes/{node_id}', auth_token='token')
+        node = resp.json()
+        map_types = [f['map_type'] for f in node['properties']['files']]
+        self.assertEqual(['color', 'alpha', 'specular'], map_types)
+
+    def test_no_color_map(self):
+        file_id_1 = self.mkfile('cafef00dcafef00dcafef00d')
+        file_id_2 = self.mkfile('cafef00dcafef00dcafecafe')
+        file_id_3 = self.mkfile('cafef00dcafef00ddeadbeef')
+
+        # Create a texture node in the 'wrong' order
+        resp = self.post('/api/nodes', expected_status=201, auth_token='token', json={
+            'project': self.project_id,
+            'node_type': 'texture',
+            'name': str(self),
+            'properties': {
+                'files': [
+                    {'map_type': 'specular', 'file': file_id_1},
+                    {'map_type': 'bump', 'file': file_id_2},
+                    {'map_type': 'alpha', 'file': file_id_3},
+                ]
+            },
+            'user': ctd.EXAMPLE_PROJECT_OWNER_ID,
+        })
+        node_id = resp.json()['_id']
+
+        resp = self.get(f'/api/nodes/{node_id}', auth_token='token')
+        node = resp.json()
+        map_types = [f['map_type'] for f in node['properties']['files']]
+        self.assertEqual(['alpha', 'bump', 'specular'], map_types)
+
+    def test_empty_files_list(self):
+        # Create a texture node without any files.
+        resp = self.post('/api/nodes', expected_status=201, auth_token='token', json={
+            'project': self.project_id,
+            'node_type': 'texture',
+            'name': str(self),
+            'properties': {
+                'files': []
+            },
+            'user': ctd.EXAMPLE_PROJECT_OWNER_ID,
+        })
+        node_id = resp.json()['_id']
+
+        resp = self.get(f'/api/nodes/{node_id}', auth_token='token')
+        node = resp.json()
+        self.assertEqual([], node['properties']['files'])
+
+    def test_no_files_list(self):
+        # Create a texture node without any files.
+        resp = self.post('/api/nodes', expected_status=201, auth_token='token', json={
+            'project': self.project_id,
+            'node_type': 'texture',
+            'name': str(self),
+            'properties': {},
+            'user': ctd.EXAMPLE_PROJECT_OWNER_ID,
+        })
+        node_id = resp.json()['_id']
+
+        resp = self.get(f'/api/nodes/{node_id}', auth_token='token')
+        node = resp.json()
+        self.assertNotIn('files', node['properties'])
+