diff --git a/tests/test_sync_to_array_newer_cache.py b/tests/test_sync_to_array_newer_cache.py index cad23a88..a5b06b45 100644 --- a/tests/test_sync_to_array_newer_cache.py +++ b/tests/test_sync_to_array_newer_cache.py @@ -124,3 +124,82 @@ def test_array_copy_same_size_deletes_cache_without_copy(tmp_path, max_workers): mock_copy.assert_not_called() assert array_file.read_bytes() == NEW assert not cache_file.exists() + + +# --------------------------------------------------------------------------- +# The other direction: the array copy was changed after the cache copy. +# +# Caching keeps the original modification time on both sides (copystat on the +# cache copy, rename for the backup), so an untouched pair has matching mtimes. +# When a tool rewrites the array side directly, the cache copy is the stale one +# and must not be copied over it. +# --------------------------------------------------------------------------- + +STALE_CACHE = b"stale copy left on cache" * 10 +PROCESSED_ARRAY = b"processed on the array by another tool" * 10 +BASE_TIME = 1_700_000_000 + + +def _set_mtime(path, seconds): + os.utime(path, (seconds, seconds)) + + +@pytest.mark.parametrize("max_workers", WORKERS) +@pytest.mark.parametrize("array_side", ["same-name", "backup"]) +def test_newer_array_copy_is_left_alone(tmp_path, max_workers, array_side): + svc, cache_root, array_root = _make_service(tmp_path) + cache_file = cache_root / "Film (2020)" / "Film.mkv" + name = "Film.mkv" if array_side == "same-name" else "Film.mkv.plexcached" + array_side_file = array_root / "Film (2020)" / name + _write(cache_file, STALE_CACHE) + _write(array_side_file, PROCESSED_ARRAY) + _set_mtime(cache_file, BASE_TIME) + _set_mtime(array_side_file, BASE_TIME + 3600) + + result = svc.sync_to_array([str(cache_file)], dry_run=False, max_workers=max_workers) + + assert result.affected_count == 0 + assert any("newer than the cache copy" in e for e in result.errors), result.errors + assert array_side_file.read_bytes() == PROCESSED_ARRAY + assert cache_file.read_bytes() == STALE_CACHE + assert not (array_root / "Film (2020)" / "Film.mkv.pc-part").exists() + if array_side == "backup": + assert not (array_root / "Film (2020)" / "Film.mkv").exists() + + +@pytest.mark.parametrize("max_workers", WORKERS) +def test_newer_cache_copy_still_replaces_array(tmp_path, max_workers): + """Explicit mtimes for the #222 direction: the cache copy was rewritten later.""" + svc, cache_root, array_root = _make_service(tmp_path) + cache_file = cache_root / "Film.mkv" + array_file = array_root / "Film.mkv" + _write(cache_file, NEW) + _write(array_file, OLD) + _set_mtime(array_file, BASE_TIME) + _set_mtime(cache_file, BASE_TIME + 3600) + + result = svc.sync_to_array([str(cache_file)], dry_run=False, max_workers=max_workers) + + assert result.affected_count == 1, result.errors + assert array_file.read_bytes() == NEW + assert not cache_file.exists() + + +@pytest.mark.parametrize("max_workers", WORKERS) +@pytest.mark.parametrize("array_offset", [0, 1], ids=["same-mtime", "within-tolerance"]) +def test_matching_mtimes_keep_the_cache_version(tmp_path, max_workers, array_offset): + """A tool that keeps the original mtime gives no signal; the cache copy wins as in #222.""" + svc, cache_root, array_root = _make_service(tmp_path) + cache_file = cache_root / "Film.mkv" + backup = array_root / "Film.mkv.plexcached" + _write(cache_file, NEW) + _write(backup, OLD) + _set_mtime(cache_file, BASE_TIME) + _set_mtime(backup, BASE_TIME + array_offset) + + result = svc.sync_to_array([str(cache_file)], dry_run=False, max_workers=max_workers) + + assert result.affected_count == 1, result.errors + assert (array_root / "Film.mkv").read_bytes() == NEW + assert not backup.exists() + assert not cache_file.exists() diff --git a/web/services/maintenance_service.py b/web/services/maintenance_service.py index 90897b0e..522ea213 100644 --- a/web/services/maintenance_service.py +++ b/web/services/maintenance_service.py @@ -1557,6 +1557,26 @@ def _array_copy_matches(self, cache_path: str, array_path: str) -> Optional[str] return candidate return None + # Filesystems that store modification times coarsely (FAT keeps 2 seconds) + # can make an untouched copy look slightly newer; ignore differences this small. + _NEWER_MTIME_TOLERANCE_SECONDS = 2 + + def _newer_array_copy(self, cache_path: str, array_path: str) -> Optional[str]: + """Return the array-side file that was modified after the cache copy, if any. + + Caching keeps the original modification time (copystat on the cache + copy, rename for the .plexcached backup), so an untouched pair has + matching mtimes and whichever side a tool rewrote is the newer one. + A newer array copy means the cache copy is stale and must not be + copied over it. + """ + cache_mtime = os.path.getmtime(cache_path) + for candidate in (array_path, array_path + PLEXCACHED_EXTENSION): + if (os.path.exists(candidate) + and os.path.getmtime(candidate) > cache_mtime + self._NEWER_MTIME_TOLERANCE_SECONDS): + return candidate + return None + def _sync_one_to_array(self, cache_path: str, array_path: str, bytes_progress_callback: Optional[Callable] = None) -> Optional[str]: """Move one cache file to the array. Returns an error message, or None on success. @@ -1588,6 +1608,11 @@ def _sync_one_to_array(self, cache_path: str, array_path: str, os.remove(cache_path) return None + if self._newer_array_copy(cache_path, array_path) is not None: + logging.warning(f"Array copy is newer than the cache copy, leaving both in place: " + f"{os.path.basename(cache_path)}") + return "Array copy is newer than the cache copy, left both in place for review" + if os.path.exists(array_path) or os.path.exists(backup_path): logging.info(f"Cache copy differs from the array copy, keeping the cache version: " f"{os.path.basename(cache_path)}") @@ -1622,6 +1647,8 @@ def sync_to_array(self, paths: List[str], dry_run: bool = True, For each file (see ``_sync_one_to_array``): - If the array already holds an identical copy (.plexcached backup or same-name file of the same size): restore it, delete cache copy + - If the array copy was modified after the cache copy, the cache copy + is stale: leave both in place and report it for review - Otherwise the cache copy is the version to keep (a Sonarr/Radarr upgrade, a Tdarr pass, or no array copy at all): copy it to the array, verify, then remove the old backup and the cache copy @@ -1637,7 +1664,8 @@ def sync_to_array(self, paths: List[str], dry_run: bool = True, array_path = self._cache_to_array_path(cache_path) if array_path: try: - if self._array_copy_matches(cache_path, array_path) is None: + if (self._array_copy_matches(cache_path, array_path) is None + and self._newer_array_copy(cache_path, array_path) is None): total_bytes += os.path.getsize(cache_path) except OSError: pass diff --git a/web/templates/maintenance/partials/audit_results.html b/web/templates/maintenance/partials/audit_results.html index e85b652b..468e81a5 100644 --- a/web/templates/maintenance/partials/audit_results.html +++ b/web/templates/maintenance/partials/audit_results.html @@ -919,7 +919,7 @@

whatWillHappen = ` What will happen: `; @@ -929,6 +929,7 @@

  1. If the array copy matches the cache copy, restore it (rename .plexcached back to original)
  2. If the cache copy differs, it is the newer version: copy it to the array and verify
    3. +
  3. If the array copy was changed after the cache copy, leave both in place and report it for review
  4. Delete the cache copy
`;