Fix sync for folders synced by earlier versions

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".

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)

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)