From 07f6de1996f874d14fa45e3a65f08634b970b724 Mon Sep 17 00:00:00 2001 From: Constantine Nathanson Date: Sun, 4 Oct 2026 12:23:42 +0300 Subject: [PATCH] Fix `sync` deleting raw files synced by earlier versions Ignore `.cld-sync` entries whose remote file does not exist. In dynamic folder mode, match remote raw files with local files of the same content that were saved without the extension, e.g. `notes` or `notes (1)`. Delete test assets of all resource types in `delete_cld_folder_if_exists`. Co-Authored-By: Claude Opus 5.5 (1M context) --- cloudinary_cli/modules/sync.py | 24 +++++++++- test/helper_test.py | 5 +- test/test_modules/test_cli_sync.py | 74 +++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/cloudinary_cli/modules/sync.py b/cloudinary_cli/modules/sync.py index d7395ae..a49a92c 100644 --- a/cloudinary_cli/modules/sync.py +++ b/cloudinary_cli/modules/sync.py @@ -135,8 +135,8 @@ def __init__(self, local_dir, remote_dir, include_hidden, concurrent_workers, fo # handle fixed folder mode public_id differences diverse_file_names = read_json_from_file(self.sync_meta_file, does_not_exist_ok=True) - self.diverse_file_names = dict( - (normalize_file_extension(k), normalize_file_extension(v)) for k, v in diverse_file_names.items()) + self.diverse_file_names = self._verify_diverse_file_names(dict( + (normalize_file_extension(k), normalize_file_extension(v)) for k, v in diverse_file_names.items())) inverted_diverse_file_names = invert_dict(self.diverse_file_names) cloudinarized_local_file_names = [self.diverse_file_names.get(f, f) for f in local_file_names] @@ -302,6 +302,26 @@ def _normalize_remote_file_names(self, remote_files, local_files): return {dt["normalized_unique_path"]: dt for dt in remote_files.values()} + def _verify_diverse_file_names(self, diverse_file_names): + """ + Keeps entries of existing remote files and maps local raw files saved without extension, e.g. 'notes (1)'. + """ + # drop entries that point to a file that is not on Cloudinary, e.g. 'notes.txt' -> 'notes' + file_names = {k: v for k, v in diverse_file_names.items() if v in self.remote_files} + for name, dt in self.remote_files.items(): + # only unmatched raw files in dynamic folder mode can have a local copy without extension + if (self.folder_mode != "dynamic" or dt["resource_type"] != "raw" or name in self.local_files + or name in file_names.values()): + continue + # local copies are named after the display name: 'notes', 'notes (1)', 'notes (2)', ... + for local_name in self._local_candidates(path.splitext(dt["normalized_path"])[0]): + # take a free local file with the same content + if (local_name not in self.remote_files and local_name not in file_names + and self.local_files[local_name]["etag"] == dt["etag"]): + file_names[local_name] = name + break + return file_names + def _local_candidates(self, candidate_path): filename, extension = path.splitext(candidate_path) # match the whole name only, otherwise "notes" also matches "notes.txt". diff --git a/test/helper_test.py b/test/helper_test.py index ce2f118..5e12dd1 100644 --- a/test/helper_test.py +++ b/test/helper_test.py @@ -128,8 +128,9 @@ def delete_cld_folder_if_exists(folder, folder_mode = "fixed"): cloudinary.api.delete_resources_by_prefix(folder) else: assets = query_cld_folder(folder, folder_mode) - if (len(assets)): - cloudinary.api.delete_resources([f["public_id"] for f in assets.values()]) + for resource_type in {f["resource_type"] for f in assets.values()}: + cloudinary.api.delete_resources([f["public_id"] for f in assets.values() + if f["resource_type"] == resource_type], resource_type=resource_type) try: cloudinary.api.delete_folder(folder) diff --git a/test/test_modules/test_cli_sync.py b/test/test_modules/test_cli_sync.py index 595e4b1..36ccac4 100644 --- a/test/test_modules/test_cli_sync.py +++ b/test/test_modules/test_cli_sync.py @@ -14,7 +14,7 @@ from test.helper_test import unique_suffix, RESOURCES_DIR, TEST_FILES_DIR, delete_cld_folder_if_exists, retry_assertion, \ get_request_url, get_params, URLLIB3_REQUEST from test.test_modules.test_cli_upload_dir import UPLOAD_MOCK_RESPONSE -from cloudinary_cli.utils.api_utils import get_folder_mode, _display_path +from cloudinary_cli.utils.api_utils import get_folder_mode, _display_path, query_cld_folder from cloudinary_cli.modules.sync import SyncDir from cloudinary_cli.utils.utils import etag @@ -218,6 +218,78 @@ def test_cli_sync_duplicate_file_names_dynamic_folder_mode(self): self.assertIn("Done!", result.output) + def _local_files(self, files): + local_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, local_dir, True) + for name, content in files.items(): + Path(local_dir, name).write_text(content) + return local_dir + + def _sync(self, direction, local_dir, *expected): + result = self.runner.invoke(cli, ['sync', direction, '-F', local_dir, self.CLD_SYNC_DIR]) + self.assertEqual(0, result.exit_code, result.output) + for text in expected: + self.assertIn(text, result.output) + return result + + def _wait_for_cld_files(self, count): + for _ in range(10): + if len(query_cld_folder(self.CLD_SYNC_DIR, self.folder_mode)) == count: + return + time.sleep(1) + + def _assert_nothing_to_sync(self, direction, local_dir, count): + result = self._sync(direction, local_dir, f"Skipping {count} items", "Done!") + self.assertNotIn("Deleting", result.output) + self.assertNotIn("Uploading", result.output) + self.assertNotIn("Downloading", result.output) + + @unittest.skipUnless(get_folder_mode() == "dynamic", "requires dynamic folder mode") + def test_cli_sync_push_raw_files_with_cld_sync_entries_without_extension(self): + local_dir = self._local_files({"notes.txt": "txt", "notes.csv": "csv"}) + self._sync('--push', local_dir, "Synced | 2") + self._wait_for_cld_files(2) + # entries saved by versions that did not keep the extension of raw files + Path(local_dir, ".cld-sync").write_text(json.dumps({"notes.txt": "notes", "notes.csv": "notes"})) + + self._assert_nothing_to_sync('--push', local_dir, 2) + self._assert_nothing_to_sync('--push', local_dir, 2) + self._assert_nothing_to_sync('--pull', local_dir, 2) + + @unittest.skipUnless(get_folder_mode() == "dynamic", "requires dynamic folder mode") + def test_cli_sync_raw_file_saved_without_extension(self): + self._sync('--push', self._local_files({"notes.txt": "txt"}), "Synced | 1") + self._wait_for_cld_files(1) + # file pulled by versions that did not keep the extension of raw files + local_dir = self._local_files({"notes": "txt"}) + + self._assert_nothing_to_sync('--push', local_dir, 1) + self._assert_nothing_to_sync('--pull', local_dir, 1) + + @unittest.skipUnless(get_folder_mode() == "dynamic", "requires dynamic folder mode") + def test_cli_sync_push_raw_duplicates_saved_without_extension(self): + result = self.runner.invoke(cli, ['sync', '--push', '-F', self._local_files({"b.txt": "b", "c.txt": "c"}), + self.CLD_SYNC_DIR, '-o', 'display_name', 'notes']) + self.assertEqual(0, result.exit_code, result.output) + self._wait_for_cld_files(2) + # files pulled by versions that did not keep the extension of raw files, the first one deleted remotely + local_dir = self._local_files({"notes (1)": "a", "notes (2)": "b", "notes (3)": "c"}) + + result = self._sync('--push', local_dir, "Skipping 2 items", "Synced | 1") + self.assertNotIn("Deleting", result.output) + self._wait_for_cld_files(3) + + @unittest.skipUnless(get_folder_mode() == "dynamic", "requires dynamic folder mode") + def test_cli_sync_raw_file_keeps_extension(self): + local_dir = self._local_files({"notes.txt": "txt"}) + self._sync('--push', local_dir, "Synced | 1") + self.assertFalse(Path(local_dir, ".cld-sync").exists()) + self._wait_for_cld_files(1) + + self._sync('--pull', self.LOCAL_SYNC_PULL_DIR, "Synced | 1") + self.assertTrue(Path(self.LOCAL_SYNC_PULL_DIR, "notes.txt").is_file()) + self._assert_nothing_to_sync('--push', self.LOCAL_SYNC_PULL_DIR, 1) + @retry_assertion def test_cli_sync_push_dry_run(self): self._upload_sync_files(TEST_FILES_DIR)