Store loaded app filename so it can delete it when requested

golem/apps/__init__.py

@@ -1,7 +1,7 @@
 import hashlib
 import logging
 from pathlib import Path
-from typing import Dict, Any, Iterator, Type
+from typing import Dict, Any, Iterator, Type, Tuple
 
 from dataclasses import dataclass, field
 from dataclasses_json import dataclass_json, config
@@ -82,26 +82,16 @@ def load_app_from_json_file(json_file: Path) -> AppDefinition:
         raise ValueError(msg)
 
 
-def load_apps_from_dir(app_dir: Path) -> Iterator[AppDefinition]:
+def load_apps_from_dir(app_dir: Path) -> Iterator[Tuple[Path, AppDefinition]]:
     """ Read every file in the given directory and attempt to parse it. Ignore
         files which don't contain valid app definitions. """
     for json_file in app_dir.iterdir():
         try:
-            yield load_app_from_json_file(json_file)
+            yield (json_file, load_app_from_json_file(json_file))
         except ValueError:
             continue
 
 
-def delete_app_from_dir(app_dir, app_def: AppDefinition) -> bool:
-    filename = app_json_file_name(app_def)
-    file = app_dir / filename
-    if not file.exists():
-        logger.warning('Can not delete app, file not found. file=%r', file)
-        return False
-    file.unlink()
-    return True
-
-
 def app_json_file_name(app_def: AppDefinition) -> str:
     filename = f"{app_def.name}_{app_def.version}_{app_def.id}.json"
     filename = sanitize_filename(filename, replacement_text="_")

golem/apps/manager.py

@@ -2,9 +2,7 @@
 from typing import Dict, List, Tuple
 from pathlib import Path
 
-from golem.apps import (
-    AppId, AppDefinition, load_apps_from_dir, delete_app_from_dir,
-)
+from golem.apps import AppId, AppDefinition, load_apps_from_dir
 from golem.apps.default import save_built_in_app_definitions
 from golem.model import AppConfiguration
 
@@ -17,14 +15,15 @@ class AppManager:
     def __init__(self, app_dir: Path, save_apps=True) -> None:
         self._apps: Dict[AppId, AppDefinition] = {}
         self._state = AppStates()
-        self._app_dir = app_dir
+        self._app_file_names: Dict[AppId, Path] = dict()
 
         # Save build in apps, then load apps from path
         built_in_apps: List[AppId] = []
         if save_apps:
             built_in_apps = save_built_in_app_definitions(app_dir)
-        for app_def in load_apps_from_dir(app_dir):
+        for (app_def_path, app_def) in load_apps_from_dir(app_dir):
             self.register_app(app_def)
+            self._app_file_names[app_def.id] = app_def_path
         for app_id in built_in_apps:
             self.set_enabled(app_id, True)
 
@@ -78,8 +77,8 @@ def app(self, app_id: AppId) -> AppDefinition:
     def delete(self, app_id: AppId) -> bool:
         # Delete self._state from the database first
         del self._state[app_id]
-        delete_app_from_dir(self._app_dir, self._apps[app_id])
         del self._apps[app_id]
+        self._app_file_names[app_id].unlink()
         return True
 
 

tests/golem/apps/test_app_manager.py

@@ -1,3 +1,5 @@
+from mock import Mock
+
 from golem.apps.manager import AppManager
 from golem.apps import (
     AppDefinition,
@@ -42,9 +44,12 @@ def test_re_register(self):
 
     def test_delete_app(self):
         self.app_manager.register_app(APP_DEF)
+        self.app_manager._app_file_names[APP_ID] = mocked_file = Mock()
+        mocked_file.unlink = Mock()
         self.assertEqual(self.app_manager.apps(), [(APP_ID, APP_DEF)])
         self.app_manager.delete(APP_ID)
         self.assertEqual(self.app_manager.apps(), [])
+        mocked_file.unlink.assert_called_once_with()
 
 
 class TestSetEnabled(AppManagerTestBase):
@@ -117,4 +122,4 @@ def test_register(self):
         app_file.write_text(APP_DEF.to_json(), encoding='utf-8')
         bogus_file.write_text('(╯°□°）╯︵ ┻━┻', encoding='utf-8')
         loaded_apps = list(load_apps_from_dir(self.new_path))
-        self.assertEqual(loaded_apps, [APP_DEF])
+        self.assertEqual(loaded_apps, [(app_file, APP_DEF)])