Find just one file over limit in transaction

Author: MarcelGeo

mergin/client_push.py

@@ -486,14 +486,17 @@ def get_push_changes_batch(mc, mp: MerginProject) -> Tuple[LocalChanges, int]:
         updated=[LocalChange(**change) for change in changes["updated"]],
         removed=[LocalChange(**change) for change in changes["removed"]],
     )
-    if local_changes.get_media_upload_size() > MAX_UPLOAD_MEDIA_SIZE:
+
+    over_limit_media = local_changes.get_media_upload_over_size(MAX_UPLOAD_MEDIA_SIZE)
+    if over_limit_media:
         raise ClientError(
-            f"Total size of media files to upload exceeds the maximum allowed size of {MAX_UPLOAD_MEDIA_SIZE / (1024**3)} GiB."
+            f"File {over_limit_media.path} to upload exceeds the maximum allowed size of {MAX_UPLOAD_MEDIA_SIZE / (1024**3)} GB."
         )
 
-    if local_changes.get_gpgk_upload_size() > MAX_UPLOAD_VERSIONED_SIZE:
+    over_limit_gpkg = local_changes.get_gpgk_upload_over_size(MAX_UPLOAD_VERSIONED_SIZE)
+    if over_limit_gpkg:
         raise ClientError(
-            f"Total size of GPKG files to upload exceeds the maximum allowed size of {MAX_UPLOAD_VERSIONED_SIZE / (1024**3)} GiB."
+            f"Geopackage {over_limit_gpkg.path} to upload exceeds the maximum allowed size of {MAX_UPLOAD_VERSIONED_SIZE / (1024**3)} GB."
         )
 
     return local_changes, sum(len(v) for v in changes.values())

mergin/common.py

@@ -24,10 +24,10 @@
 # seconds to wait between sync callback calls
 SYNC_CALLBACK_WAIT = 0.01
 
-# maximum size of media files able to upload in one push (in bytes)
+# maximum size of media file able to upload in one push (in bytes)
 MAX_UPLOAD_MEDIA_SIZE = 10 * (1024**3)
 
-# maximum size of GPKG files able to upload in one push (in bytes)
+# maximum size of GPKG file able to upload in one push (in bytes)
 MAX_UPLOAD_VERSIONED_SIZE = 5 * (1024**3)
 
 # default URL for submitting logs

mergin/local_changes.py

@@ -113,23 +113,22 @@ def update_chunks(self, server_chunks: List[Tuple[str, str]]) -> None:
         for change in self.updated:
             change.chunks = self._map_unique_chunks(change.chunks, server_chunks)
 
-    def get_media_upload_size(self) -> int:
+    def get_media_upload_over_size(self, size_limit: int) -> Optional[LocalChange]:
         """
-        Calculate the total size of media files in added and updated changes.
+        Find the first media file in added and updated changes that exceeds the size limit.
+        :return: The first LocalChange that exceeds the size limit, or None if no such file exists.
         """
-        total_size = 0
         for change in self.get_upload_changes():
-            if not is_versioned_file(change.path):
-                total_size += change.size
-        return total_size
+            if not is_versioned_file(change.path) and change.size > size_limit:
+                return change
 
-    def get_gpgk_upload_size(self) -> int:
+    def get_gpgk_upload_over_size(self, size_limit: int) -> Optional[LocalChange]:
         """
-        Calculate the total size of gpgk files in added and updated changes.
-        Do not calculate diffs (only new or overwriten files).
+        Find the first GPKG file in added and updated changes that exceeds the size limit.
+        Do not include diffs (only new or overwritten files).
+        :param size_limit: The size limit in bytes.
+        :return: The first LocalChange that exceeds the size limit, or None if no such file exists.
         """
-        total_size = 0
         for change in self.get_upload_changes():
-            if is_versioned_file(change.path) and not change.diff:
-                total_size += change.size
-        return total_size
+            if is_versioned_file(change.path) and not change.diff and change.size > size_limit:
+                return change

mergin/test/test_client.py

@@ -3211,3 +3211,21 @@ def test_client_project_sync_retry(mc):
         with pytest.raises(ClientError):
             mc.sync_project(project_dir)
     assert mock_push_project_async.call_count == 2
+
+def test_push_file_limits(mc):
+    test_project = "test_push_file_limits"
+    project = API_USER + "/" + test_project
+    project_dir = os.path.join(TMP_DIR, test_project)
+    cleanup(mc, project, [project_dir])
+    mc.create_project(test_project)
+    mc.download_project(project, project_dir)
+    shutil.copy(os.path.join(TEST_DATA_DIR, "base.gpkg"), project_dir)
+    # setting to some minimal value to mock limit hit
+    with patch("mergin.client_push.MAX_UPLOAD_VERSIONED_SIZE", 1):
+        with pytest.raises(ClientError, match=f"base.gpkg to upload exceeds the maximum allowed size of {1/1024**3}"):
+            mc.push_project(project_dir)
+    
+    shutil.copy(os.path.join(TEST_DATA_DIR, "test.txt"), project_dir)
+    with patch("mergin.client_push.MAX_UPLOAD_MEDIA_SIZE", 1):
+        with pytest.raises(ClientError, match=f"test.txt to upload exceeds the maximum allowed size of {1/1024**3}"):
+            mc.push_project(project_dir)

mergin/test/test_local_changes.py

@@ -1,6 +1,6 @@
 from datetime import datetime
 
-from ..local_changes import LocalChange, LocalChanges
+from ..local_changes import LocalChange, LocalChanges, MAX_UPLOAD_CHANGES
 
 
 def test_local_changes_from_dict():
@@ -120,60 +120,91 @@ def test_local_changes_get_upload_changes():
     assert upload_changes[1].path == "file2.txt"  # Second change is from updated
 
 
-def test_local_changes_get_media_upload_size():
-    """Test the get_media_upload_size method of LocalChanges."""
+def test_local_changes_get_media_upload_over_size():
+    """Test the get_media_upload_file method of LocalChanges."""
+    # Define constants
+    SIZE_LIMIT_MB = 10
+    SIZE_LIMIT_BYTES = SIZE_LIMIT_MB * 1024 * 1024
+    SMALL_FILE_SIZE = 1024
+    LARGE_FILE_SIZE = 15 * 1024 * 1024
+
     # Create sample LocalChange instances
     added = [
-        LocalChange(path="file1.txt", checksum="abc123", size=1024, mtime=datetime.now()),
-        LocalChange(path="file2.jpg", checksum="xyz789", size=2048, mtime=datetime.now()),
+        LocalChange(path="file1.txt", checksum="abc123", size=SMALL_FILE_SIZE, mtime=datetime.now()),
+        LocalChange(path="file2.jpg", checksum="xyz789", size=LARGE_FILE_SIZE, mtime=datetime.now()),  # Over limit
     ]
     updated = [
-        LocalChange(path="file3.mp4", checksum="lmn456", size=5120, mtime=datetime.now()),
-        LocalChange(path="file4.gpkg", checksum="opq123", size=1024, mtime=datetime.now()),
+        LocalChange(path="file3.mp4", checksum="lmn456", size=5 * 1024 * 1024, mtime=datetime.now()),
+        LocalChange(path="file4.gpkg", checksum="opq123", size=SMALL_FILE_SIZE, mtime=datetime.now()),
     ]
 
     # Initialize LocalChanges
     local_changes = LocalChanges(added=added, updated=updated)
 
-    # Call get_media_upload_size
-    media_size = local_changes.get_media_upload_size()
+    # Call get_media_upload_file with a size limit
+    media_file = local_changes.get_media_upload_over_size(SIZE_LIMIT_BYTES)
 
     # Assertions
-    assert media_size == 8192  # Only non-versioned files (txt, jpg, mp4) are included
+    assert media_file is not None
+    assert media_file.path == "file2.jpg"  # The first file over the limit
+    assert media_file.size == LARGE_FILE_SIZE
+
 
+def test_local_changes_get_gpgk_upload_over_size():
+    """Test the get_gpgk_upload_file method of LocalChanges."""
+    # Define constants
+    SIZE_LIMIT_MB = 10
+    SIZE_LIMIT_BYTES = SIZE_LIMIT_MB * 1024 * 1024
+    SMALL_FILE_SIZE = 1024
+    LARGE_FILE_SIZE = 15 * 1024 * 1024
 
-def test_local_changes_get_gpgk_upload_size():
-    """Test the get_gpgk_upload_size method of LocalChanges."""
     # Create sample LocalChange instances
     added = [
-        LocalChange(path="file1.gpkg", checksum="abc123", size=1024, mtime=datetime.now()),
-        LocalChange(path="file2.gpkg", checksum="xyz789", size=2048, mtime=datetime.now(), diff={"path": "diff1"}),
+        LocalChange(path="file1.gpkg", checksum="abc123", size=SMALL_FILE_SIZE, mtime=datetime.now()),
+        LocalChange(
+            path="file2.gpkg", checksum="xyz789", size=LARGE_FILE_SIZE, mtime=datetime.now(), diff=None
+        ),  # Over limit
     ]
     updated = [
-        LocalChange(path="file3.gpkg", checksum="lmn456", size=5120, mtime=datetime.now()),
-        LocalChange(path="file4.txt", checksum="opq123", size=1024, mtime=datetime.now()),
+        LocalChange(path="file3.gpkg", checksum="lmn456", size=5 * 1024 * 1024, mtime=datetime.now()),
+        LocalChange(path="file4.txt", checksum="opq123", size=SMALL_FILE_SIZE, mtime=datetime.now()),
     ]
 
     # Initialize LocalChanges
     local_changes = LocalChanges(added=added, updated=updated)
 
-    # Call get_gpgk_upload_size
-    gpkg_size = local_changes.get_gpgk_upload_size()
+    # Call get_gpgk_upload_file with a size limit
+    gpkg_file = local_changes.get_gpgk_upload_over_size(SIZE_LIMIT_BYTES)
 
     # Assertions
-    assert gpkg_size == 6144  # Only GPKG files without diffs are included
+    assert gpkg_file is not None
+    assert gpkg_file.path == "file2.gpkg"  # The first GPKG file over the limit
+    assert gpkg_file.size == LARGE_FILE_SIZE
+    assert gpkg_file.diff is None  # Ensure it doesn't include diffs
 
 
 def test_local_changes_post_init():
     """Test the __post_init__ method of LocalChanges."""
+    # Define constants
+    ADDED_COUNT = 80
+    UPDATED_COUNT = 21
+    SMALL_FILE_SIZE = 1024
+    LARGE_FILE_SIZE = 2048
+
     # Create more than MAX_UPLOAD_CHANGES changes
-    added = [LocalChange(path=f"file{i}.txt", checksum="abc123", size=1024, mtime=datetime.now()) for i in range(80)]
-    updated = [LocalChange(path=f"file{i}.txt", checksum="xyz789", size=2048, mtime=datetime.now()) for i in range(21)]
+    added = [
+        LocalChange(path=f"file{i}.txt", checksum="abc123", size=SMALL_FILE_SIZE, mtime=datetime.now())
+        for i in range(ADDED_COUNT)
+    ]
+    updated = [
+        LocalChange(path=f"file{i}.txt", checksum="xyz789", size=LARGE_FILE_SIZE, mtime=datetime.now())
+        for i in range(UPDATED_COUNT)
+    ]
 
     # Initialize LocalChanges
     local_changes = LocalChanges(added=added, updated=updated)
 
     # Assertions
-    assert len(local_changes.added) == 80  # All 80 added changes are included
-    assert len(local_changes.updated) == 20  # Only 20 updated changes are included to respect the limit
-    assert len(local_changes.added) + len(local_changes.updated) == 100  # Total is limited to MAX_UPLOAD_CHANGES
+    assert len(local_changes.added) == ADDED_COUNT  # All added changes are included
+    assert len(local_changes.updated) == MAX_UPLOAD_CHANGES - ADDED_COUNT  # Only enough updated changes are included
+    assert len(local_changes.added) + len(local_changes.updated) == MAX_UPLOAD_CHANGES  # Total is limited