Simplifying FsCache, covering firmware bug scenario in UT.

src/downloader/file_system.py

@@ -42,7 +42,7 @@ def __init__(self, config: Dict[str, Any], path_dictionary: Dict[str, str], logg
         self._logger = logger
         self._unique_temp_filenames: Set[Optional[str]] = set()
         self._unique_temp_filenames.add(None)
-        self._fs_cache = _FsCache()
+        self._fs_cache = FsCache()
 
     def create_for_system_scope(self) -> 'FileSystem':
         return self.create_for_config(self._config)
@@ -159,7 +159,7 @@ class FolderCreationError(Exception):
 
 
 class _FileSystem(FileSystem):
-    def __init__(self, config: Dict[str, Any], path_dictionary: Dict[str, str], logger: Logger, unique_temp_filenames: Set[Optional[str]], fs_cache: '_FsCache'):
+    def __init__(self, config: Dict[str, Any], path_dictionary: Dict[str, str], logger: Logger, unique_temp_filenames: Set[Optional[str]], fs_cache: 'FsCache'):
         self._config = config
         self._path_dictionary = path_dictionary
         self._logger = logger
@@ -179,15 +179,14 @@ def resolve(self, path: str) -> str:
         return str(Path(path).resolve())
 
     def is_file(self, path: str) -> bool:
-        if self._fs_cache.contains_file(path):
+        full_path = self._path(path)
+        if self._fs_cache.contains_file(full_path):
             self._quick_hit += 1
             return True
-        else:
-            full_path = self._path(path)
-            if os.path.isfile(full_path):
-                self._slow_hit += 1
-                self._fs_cache.add_file(path, full_path)
-                return True
+        elif os.path.isfile(full_path):
+            self._slow_hit += 1
+            self._fs_cache.add_file(full_path)
+            return True
 
         return False
 
@@ -205,7 +204,7 @@ def precache_is_file_with_folders(self, folders: List[str]) -> None:
                 continue
 
             files = [f.path for f in os.scandir(full_folder_path) if f.is_file()]
-            self._fs_cache.add_many_files(files, base_path)
+            self._fs_cache.add_many_files(files)
 
     def read_file_contents(self, path: str) -> str:
         with open(self._path(path), 'r') as f:
@@ -225,22 +224,22 @@ def move(self, source: str, target: str) -> None:
         self._logger.debug(f'Moving "{source}" to "{target}". {full_source} -> {full_target}')
         self._logger.debug(self._path_dictionary)
         os.replace(full_source, full_target)
-        self._fs_cache.remove_file(source, full_source)
-        self._fs_cache.add_file(target, full_target)
+        self._fs_cache.remove_file(full_source)
+        self._fs_cache.add_file(full_target)
 
     def copy(self, source: str, target: str) -> None:
         full_source = self._path(source)
         full_target = self._path(target)
         shutil.copyfile(full_source, full_target)
-        self._fs_cache.add_file(target, full_target)
+        self._fs_cache.add_file(full_target)
 
     def copy_fast(self, source: str, target: str) -> None:
         full_source = self._path(source)
         full_target = self._path(target)
         with open(full_source, 'rb') as fsource:
             with open(full_target, 'wb') as ftarget:
                 shutil.copyfileobj(fsource, ftarget, length=1024 * 1024 * 4)
-        self._fs_cache.add_file(target, full_target)
+        self._fs_cache.add_file(full_target)
 
     def hash(self, path: str) -> str:
         return hash_file(self._path(path))
@@ -358,7 +357,7 @@ def _unlink(self, path: str, verbose: bool) -> bool:
             self._logger.print(f'Removing {path} ({full_path})')
         try:
             Path(full_path).unlink()
-            self._fs_cache.remove_file(path, full_path)
+            self._fs_cache.remove_file(full_path)
             return True
         except FileNotFoundError as _:
             return False
@@ -417,19 +416,15 @@ def _load_json(file_path: str) -> Dict[str, Any]:
         return json.loads(f.read())
 
 
-class _FsCache:
+class FsCache:
     def __init__(self): self._files: Set[str] = set()
     def contains_file(self, path: str) -> bool: return path in self._files
 
-    def add_many_files(self, paths: List[str], base_path: Optional[str]) -> None:
+    def add_many_files(self, paths: List[str]) -> None:
         self._files.update(paths)
-        if base_path is not None:
-            self._files.update(f.replace(base_path + '/', '') for f in paths)
 
-    def add_file(self, path: str, full_path: str) -> None:
+    def add_file(self, path: str) -> None:
         self._files.add(path)
-        if len(path) != len(full_path): self._files.add(full_path)
 
-    def remove_file(self, path: str, full_path: str) -> None:
+    def remove_file(self, path: str) -> None:
         if path in self._files: self._files.remove(path)
-        if len(path) != len(full_path) and full_path in self._files: self._files.remove(full_path)

src/test/fake_file_system_factory.py

@@ -20,7 +20,7 @@
 
 from downloader.constants import K_BASE_PATH, STORAGE_PATHS_PRIORITY_SEQUENCE
 from downloader.file_system import FileSystemFactory as ProductionFileSystemFactory, FileSystem as ProductionFileSystem, \
-    absolute_parent_folder, is_windows, FolderCreationError
+    absolute_parent_folder, is_windows, FolderCreationError, FsCache
 from downloader.other import ClosableValue, UnreachableException
 from test.fake_importer_implicit_inputs import FileSystemState
 from downloader.logger import NoLogger
@@ -45,15 +45,16 @@ def __init__(self, state=None, write_records=None, config=None):
         self._state = state if state is not None else FileSystemState(config=config)
         self._fake_failures = {}
         self._write_records = write_records if write_records is not None else []
+        self._fs_cache = FsCache()
 
     def set_create_folders_buggy(self):
         self._fake_failures['create_folders'] = True
 
     def create_for_config(self, config):
-        return FakeFileSystem(self._state, config, self._fake_failures, self._write_records)
+        return FakeFileSystem(self._state, config, self._fake_failures, self._write_records, self._fs_cache)
 
     def create_for_system_scope(self):
-        return FakeFileSystem(self._state, self._state.config, self._fake_failures, self._write_records)
+        return FakeFileSystem(self._state, self._state.config, self._fake_failures, self._write_records, self._fs_cache)
 
     @property
     def data(self):
@@ -74,12 +75,13 @@ def from_state(files=None, folders=None, config=None, base_path=None, path_dicti
 class FakeFileSystem(ProductionFileSystem):
     unique_temp_filename_index = 0
 
-    def __init__(self, state, config, fake_failures, write_records):
+    def __init__(self, state, config, fake_failures, write_records, fs_cache: FsCache):
         self.state = state
         self._config = config
         self._fake_failures = fake_failures
         self._write_records = write_records
         self._current_temp_file_index = 0
+        self._fs_cache = fs_cache
 
     @property
     def write_records(self):
@@ -108,7 +110,15 @@ def resolve(self, path):
         return self._path(path)
 
     def is_file(self, path):
-        return self._path(path) in self.state.files
+        full_path = self._path(path)
+        if self._fs_cache.contains_file(full_path):
+            return True
+
+        if full_path in self.state.files:
+            self._fs_cache.add_file(full_path)
+            return True
+
+        return False
 
     def print_debug(self):
         pass
@@ -146,9 +156,13 @@ def set_copy_buggy(self):
     def move(self, source, target):
         source_file = self._path(source)
         target_file = self._path(target)
+        if source_file not in self.state.files:
+            raise Exception('Source file "%s" does not exist!' % source_file)
         self.state.files[target_file] = self.state.files[source_file]
         self.state.files.pop(source_file)
         self._write_records.append(_Record('move', (source_file, target_file)))
+        self._fs_cache.add_file(target_file)
+        self._fs_cache.remove_file(source_file)
 
     def copy(self, source, target):
         source_file = self._path(source)
@@ -159,6 +173,7 @@ def copy(self, source, target):
 
         self.state.files[target_file] = source_description
         self._write_records.append(_Record('copy', (source_file, target_file)))
+        self._fs_cache.add_file(target_file)
 
     def copy_fast(self, source, target):
         self.copy(source, target)
@@ -218,10 +233,11 @@ def download_target_path(self, path):
         return self._path(path)
 
     def unlink(self, path, verbose=True):
-        file = self._path(path)
-        if file in self.state.files:
-            self.state.files.pop(file)
-            self._write_records.append(_Record('unlink', file))
+        full_path = self._path(path)
+        if full_path in self.state.files:
+            self.state.files.pop(full_path)
+            self._write_records.append(_Record('unlink', full_path))
+            self._fs_cache.remove_file(full_path)
             return True
         else:
             return False

src/test/unit/online_importer/test_online_importer.py

@@ -193,6 +193,12 @@ def test_download_distribution_mister_with_mister___on_empty_store___needs_reboo
         ]), sut.fs_records)
         self.assertReports(sut, [FILE_MiSTer], needs_reboot=True)
 
+    def test_two_databases_one_with_mister___on_empty_store___needs_reboot_and_does_not_throw_due_to_bad_fs_cache_handling(self):
+        self.assertReports(OnlineImporter()
+                           .add_db(db_distribution_mister(files={FILE_MiSTer: file_mister_descr()}), empty_test_store())
+                           .add_db(db_test_being_empty_descr(), empty_test_store())
+                           .download(False), [FILE_MiSTer], needs_reboot=True)
+
     def test_download_distribution_mister_with_pdfviewer___on_empty_store_and_fs___needs_reboot(self):
         sut = OnlineImporter()
         store = empty_test_store()