[backend][assetmodel] Fix handling of unexpected files

The AssetModel class was designed with a clean prefix
in mind. If for some reason a rogue file is created outside
the prefix architecture, this would generate an error.

This patch fixes this by ensuring that only the folders are
parsed when it makes sense.

Fixes #233

beat/editor/backend/assetmodel.py

@@ -81,7 +81,10 @@ class AssetModel(QStringListModel):
 
         if self.asset_type in [AssetType.DATABASE, AssetType.PROTOCOLTEMPLATE]:
             # These assets have no user associated with them
-            for asset_folder in os.scandir(self.asset_folder):
+            asset_folders = [
+                entry for entry in os.scandir(self.asset_folder) if entry.is_dir()
+            ]
+            for asset_folder in asset_folders:
                 json_files = _find_json_files(asset_folder)
                 if json_files:
                     if self.__latest_only:
@@ -95,10 +98,15 @@ class AssetModel(QStringListModel):
                         )
         else:
             # Assets belonging to a user
-            asset_users = os.scandir(self.asset_folder)
+            asset_users = [
+                entry for entry in os.scandir(self.asset_folder) if entry.is_dir()
+            ]
 
             for asset_user in asset_users:
-                for asset_folder in os.scandir(asset_user):
+                asset_folders = [
+                    entry for entry in os.scandir(asset_user) if entry.is_dir()
+                ]
+                for asset_folder in asset_folders:
                     if self.asset_type == AssetType.EXPERIMENT:
                         for root, dirs, files in os.walk(asset_folder, topdown=False):
                             if dirs:
@@ -200,5 +208,5 @@ class AssetModel(QStringListModel):
 
         asset = Asset(self.prefix_path, self.asset_type, asset_name)
         if not os.path.exists(asset.declaration_path):
-            raise RuntimeError("Invalid asset {}".format(asset_name))
+            raise ValueError("Invalid asset {}".format(asset_name))
         return asset.declaration_path

beat/editor/test/test_assetmodel.py

@@ -23,6 +23,7 @@
 #                                                                             #
 ###############################################################################
 
+import os
 import pytest
 
 from ..backend.assetmodel import AssetModel
@@ -30,7 +31,8 @@ from ..backend.asset import AssetType
 from ..utils import dataformat_basetypes
 
 
-def create_model(qtbot, test_prefix, asset_type):
+@pytest.fixture()
+def asset_model(qtbot, test_prefix, asset_type):
     model = AssetModel()
 
     with qtbot.waitSignals(
@@ -47,10 +49,10 @@ def create_model(qtbot, test_prefix, asset_type):
 class TestAssetModel:
     """Test that the mock editor works correctly"""
 
-    def test_model_load(self, qtbot, test_prefix, asset_type):
-        model = create_model(qtbot, test_prefix, asset_type)
-        asset_list = model.stringList()
+    def test_model_load(self, asset_model):
+        asset_list = asset_model.stringList()
         assert len(asset_list) > 0
+        asset_type = asset_model.asset_type
 
         if asset_type == AssetType.DATAFORMAT:
             basetypes = dataformat_basetypes()
@@ -66,33 +68,84 @@ class TestAssetModel:
             for item in asset_list:
                 assert len(item.split("/")) == split_size
 
-    def test_json_path(self, qtbot, test_prefix, asset_type):
-        model = create_model(qtbot, test_prefix, asset_type)
-        asset_list = model.stringList()
+    def test_json_path(self, asset_model):
+        asset_list = asset_model.stringList()
         assert len(asset_list) > 0
 
-        if asset_type == AssetType.DATAFORMAT:
+        if asset_model.asset_type == AssetType.DATAFORMAT:
             basetypes = dataformat_basetypes()
             asset_list = [item for item in asset_list if item not in basetypes]
 
         for item in asset_list:
-            path = model.json_path(item)
+            path = asset_model.json_path(item)
             assert path
 
-    def test_json_path_invalid_asset(self, qtbot, test_prefix, asset_type):
-        model = create_model(qtbot, test_prefix, asset_type)
-        with pytest.raises(RuntimeError):
-            model.json_path("invalid")
+    def test_json_path_invalid_asset(self, asset_model):
+        asset_type = asset_model.asset_type
+
+        with pytest.raises(ValueError):
+            if asset_type == AssetType.EXPERIMENT:
+                path = "invalid/invalid/invalid/invalid/1"
+            elif asset_type in [AssetType.DATABASE, AssetType.PROTOCOLTEMPLATE]:
+                path = "invalid_name/1"
+            else:
+                path = "invalid/invalid_name/1"
+            asset_model.json_path(path)
 
-    def test_latest_only(self, qtbot, test_prefix, asset_type):
-        model = create_model(qtbot, test_prefix, asset_type)
-        asset_list = model.stringList()
+    def test_latest_only(self, asset_model):
+        asset_list = asset_model.stringList()
         assert len(asset_list) > 0
-        model.setLatestOnlyEnabled(False)
-        new_asset_list = model.stringList()
-        if asset_type == AssetType.EXPERIMENT:
+        asset_model.setLatestOnlyEnabled(False)
+        new_asset_list = asset_model.stringList()
+
+        if asset_model.asset_type == AssetType.EXPERIMENT:
             assert len(new_asset_list) == len(asset_list)
             assert new_asset_list == asset_list
         else:
             assert len(new_asset_list) > len(asset_list)
             assert new_asset_list != asset_list
+
+    def test_latest_only_no_reload(self, asset_model):
+        asset_list = asset_model.stringList()
+        assert len(asset_list) > 0
+        asset_model.setLatestOnlyEnabled(True)
+        new_asset_list = asset_model.stringList()
+        assert asset_list == new_asset_list
+
+    def test_setting_same_type_no_reload(self, qtbot, asset_model):
+        asset_list = asset_model.stringList()
+        assert len(asset_list) > 0
+        with qtbot.assertNotEmitted(asset_model.assetTypeChanged):
+            asset_model.setAssetType(asset_model.asset_type)
+        new_asset_list = asset_model.stringList()
+        assert asset_list == new_asset_list
+
+    def test_setting_same_path_no_reload(self, qtbot, asset_model):
+        asset_list = asset_model.stringList()
+        assert len(asset_list) > 0
+        with qtbot.assertNotEmitted(asset_model.prefixPathChanged):
+            asset_model.setPrefixPath(asset_model.prefix_path)
+        new_asset_list = asset_model.stringList()
+        assert asset_list == new_asset_list
+
+    def test_unexpected_files(self, asset_model):
+        """This test ensures that unexpected files in the prefix don't break
+           the AssetModel class loading code.
+        """
+
+        def __create_file(path):
+            with open(os.path.join(path, "unexpected.txt"), "wt") as unexpected_file:
+                unexpected_file.write("nothing")
+
+        asset_list = asset_model.stringList()
+
+        folders = os.scandir(asset_model.asset_folder)
+
+        for folder in folders:
+            __create_file(os.path.join(asset_model.asset_folder, folder))
+
+        __create_file(asset_model.asset_folder)
+
+        asset_model.reload()
+
+        assert asset_model.stringList() == asset_list