FIX localclient move cleanup directories across 'buckets'

Author: kroq-gar78

cottoncandy/localclient.py

@@ -3,6 +3,7 @@
 import os
 import shutil
 from io import BytesIO as StringIO
+from typing import Optional
 
 from .backend import CCBackEnd, CloudStream
 from .utils import SEPARATOR, remove_root, remove_trivial_magic, sanitize_metadata
@@ -17,7 +18,7 @@ class LocalClient(CCBackEnd):
     Handle metadata in CouldStream objects by storing a json file (.meta.json).
     """
 
-    def __init__(self, path):
+    def __init__(self, path: str):
         if not os.path.isdir(path):
             os.makedirs(path)
         self.path = path
@@ -256,7 +257,7 @@ def move(self, source, destination, source_bucket, destination_bucket,
         shutil.move(source, destination)
         shutil.move(source_metadata, destination_metadata)
         # Clean up empty parent directories at source to mimic S3 behavior
-        self._cleanup_empty_dirs(os.path.dirname(source))
+        self._cleanup_empty_dirs(os.path.dirname(source), root_path=source_bucket)
         return True
 
     def delete(self, cloud_name, recursive=False, delete=False):
@@ -353,7 +354,7 @@ def get_object_size(self, object_name):
         size = os.path.getsize(file_name)
         return size
 
-    def _cleanup_empty_dirs(self, dir_path: str) -> None:
+    def _cleanup_empty_dirs(self, dir_path: str, root_path: Optional[str] = None) -> None:
         """Remove empty parent directories up to self.path.
 
         This mimics S3 behavior where directories don't exist independently -
@@ -364,10 +365,14 @@ def _cleanup_empty_dirs(self, dir_path: str) -> None:
         ----------
         dir_path : str
             The directory path to start cleaning from
+        root_path : Optional[str]
+            The root path to stop cleaning at. If None, defaults to self.path.
         """
         # Normalize paths for comparison
         dir_path = os.path.normpath(dir_path)
-        root_path = os.path.normpath(self.path)
+        if root_path is None:
+            root_path = self.path
+        root_path = os.path.normpath(root_path)
 
         # Ensure the directory is under the root path before attempting cleanup
         try:

cottoncandy/tests/test_localclient.py

@@ -2,6 +2,8 @@
 from pathlib import Path
 import time
 
+import cottoncandy as cc
+
 from ..localclient import LocalClient
 
 
@@ -134,3 +136,85 @@ def test_move_cleans_up_empty_directories(cci, object_name):
         and cci.download_object(object_name + '/source') == content
     cci.rm(object_name + '/source')
     time.sleep(cci.wait_time)
+
+
+def test_move_cleans_up_source_bucket_directories_across_buckets(tmp_path):
+    """Moving across buckets should remove empty source directories."""
+    source_bucket = tmp_path / 'source-bucket'
+    destination_bucket = tmp_path / 'destination-bucket'
+    source_bucket.mkdir()
+    destination_bucket.mkdir()
+
+    cci_src = cc.get_interface(
+        bucket_name=str(source_bucket),
+        backend='local',
+        verbose=False,
+    )
+    cci_dest = cc.get_interface(
+        bucket_name=str(destination_bucket),
+        backend='local',
+        verbose=False,
+    )
+    source_key = 'nested/a/b/file.txt'
+    destination_key = 'moved/file.txt'
+    content = b'cross bucket move content'
+
+    cci_src.upload_object(source_key, BytesIO(content))
+
+    source_root = source_bucket / 'nested'
+    source_a = source_root / 'a'
+    source_b = source_a / 'b'
+    source_file = source_b / 'file.txt'
+
+    assert source_file.exists()
+
+    # mv from src --> dest, calling from src client
+    cci_src.mv(
+        source_name=source_key,
+        dest_name=destination_key,
+        source_bucket=str(source_bucket),
+        dest_bucket=str(destination_bucket),
+        overwrite=True,
+    )
+
+    moved_file = destination_bucket / 'moved' / 'file.txt'
+    moved_metadata = destination_bucket / 'moved' / 'file.txt.meta.json'
+
+    assert moved_file.exists()
+    assert cci_dest.download_object(destination_key) == content
+    assert moved_metadata.exists()
+    assert not cci_src.exists_object(source_key, bucket_name=str(source_bucket))
+
+    # The source bucket should be cleaned up back to the bucket root.
+    assert not source_file.exists()
+    assert not source_b.exists()
+    assert not source_a.exists()
+    assert not source_root.exists()
+    assert source_bucket.exists()
+
+    # Cleanup
+    cci_dest.rm(destination_key)
+    assert not moved_file.exists()
+    assert not moved_metadata.exists()
+    assert not (destination_bucket / 'moved').exists()
+
+    # Now try the same thing, but calling `.mv()` from the destination client instead.
+    cci_src.upload_object(source_key, BytesIO(content))
+    cci_dest.mv(
+        source_name=source_key,
+        dest_name=destination_key,
+        source_bucket=str(source_bucket),
+        dest_bucket=str(destination_bucket),
+        overwrite=True,
+    )
+
+    assert moved_file.exists()
+    assert cci_dest.download_object(destination_key) == content
+    assert moved_metadata.exists()
+    assert not cci_src.exists_object(source_key, bucket_name=str(source_bucket))
+
+    assert not source_file.exists()
+    assert not source_b.exists()
+    assert not source_a.exists()
+    assert not source_root.exists()
+    assert source_bucket.exists()