Upload changes limits

.github/workflows/autotests.yml

@@ -19,7 +19,7 @@ jobs:
 
       - uses: actions/setup-python@v2
         with:
-          python-version: '3.x'
+          python-version: '3.8'
 
       - name: Install python package dependencies
         run: |

mergin/client_push.py

@@ -22,7 +22,7 @@
 import time
 from typing import List, Tuple, Optional, ByteString
 
-from .local_changes import LocalChange, LocalChanges
+from .local_changes import ChangesValidationError, LocalChange, LocalChanges
 
 from .common import (
     MAX_UPLOAD_VERSIONED_SIZE,
@@ -481,22 +481,14 @@ def get_push_changes_batch(mc, mp: MerginProject) -> Tuple[LocalChanges, int]:
     project_role = mp.project_role()
     changes = filter_changes(mc, project_role, changes)
 
-    local_changes = LocalChanges(
-        added=[LocalChange(**change) for change in changes["added"]],
-        updated=[LocalChange(**change) for change in changes["updated"]],
-        removed=[LocalChange(**change) for change in changes["removed"]],
-    )
-
-    over_limit_media = local_changes.get_media_upload_over_size(MAX_UPLOAD_MEDIA_SIZE)
-    if over_limit_media:
-        raise ClientError(
-            f"File {over_limit_media.path} to upload exceeds the maximum allowed size of {MAX_UPLOAD_MEDIA_SIZE / (1024**3)} GB."
+    try:
+        local_changes = LocalChanges(
+            added=[LocalChange(**change) for change in changes["added"]],
+            updated=[LocalChange(**change) for change in changes["updated"]],
+            removed=[LocalChange(**change) for change in changes["removed"]],
         )
-
-    over_limit_gpkg = local_changes.get_gpgk_upload_over_size(MAX_UPLOAD_VERSIONED_SIZE)
-    if over_limit_gpkg:
+    except ChangesValidationError as e:
         raise ClientError(
-            f"Geopackage {over_limit_gpkg.path} to upload exceeds the maximum allowed size of {MAX_UPLOAD_VERSIONED_SIZE / (1024**3)} GB."
+            f"Some files exceeded maximum upload size. Files: {', '.join([c.path for c in e.invalid_changes])}. Maximum size for media files is {e.max_media_upload_size / (1024**3)} GB and for geopackage files {e.max_versioned_upload_size / (1024**3)} GB."
         )
-
     return local_changes, sum(len(v) for v in changes.values())

mergin/local_changes.py

@@ -3,10 +3,20 @@
 from typing import Optional, List, Tuple
 
 from .utils import is_versioned_file
+from .common import MAX_UPLOAD_MEDIA_SIZE, MAX_UPLOAD_VERSIONED_SIZE
 
 MAX_UPLOAD_CHANGES = 100
 
 
+# The custom exception
+class ChangesValidationError(Exception):
+    def __init__(self, message, invalid_changes=[], max_media_upload_size=None, max_versioned_upload_size=None):
+        super().__init__(message)
+        self.invalid_changes = invalid_changes if invalid_changes is not None else []
+        self.max_media_upload_size = max_media_upload_size
+        self.max_versioned_upload_size = max_versioned_upload_size
+
+
 @dataclass
 class BaseLocalChange:
     path: str
@@ -63,7 +73,20 @@ def __post_init__(self):
         """
         Enforce a limit of changes combined from `added` and `updated`.
         """
-        total_changes = len(self.get_upload_changes())
+        upload_changes = self.get_upload_changes()
+        total_changes = len(upload_changes)
+        oversize_changes = []
+        for change in upload_changes:
+            if not is_versioned_file(change.path) and change.size > MAX_UPLOAD_MEDIA_SIZE:
+                oversize_changes.append(change)
+            elif not change.diff and change.size > MAX_UPLOAD_VERSIONED_SIZE:
+                oversize_changes.append(change)
+        if oversize_changes:
+            error = ChangesValidationError("Some files exceed the maximum upload size", oversize_changes)
+            error.max_media_upload_size = MAX_UPLOAD_MEDIA_SIZE
+            error.max_versioned_upload_size = MAX_UPLOAD_VERSIONED_SIZE
+            raise error
+
         if total_changes > MAX_UPLOAD_CHANGES:
             # Calculate how many changes to keep from `added` and `updated`
             added_limit = min(len(self.added), MAX_UPLOAD_CHANGES)
@@ -112,23 +135,3 @@ 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_over_size(self, size_limit: int) -> Optional[LocalChange]:
-        """
-        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.
-        """
-        for change in self.get_upload_changes():
-            if not is_versioned_file(change.path) and change.size > size_limit:
-                return change
-
-    def get_gpgk_upload_over_size(self, size_limit: int) -> Optional[LocalChange]:
-        """
-        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.
-        """
-        for change in self.get_upload_changes():
-            if is_versioned_file(change.path) and not change.diff and change.size > size_limit:
-                return change

mergin/test/test_client.py

@@ -2328,8 +2328,10 @@ def test_clean_diff_files(mc):
     shutil.copy(mp.fpath("inserted_1_A.gpkg"), mp.fpath(f_updated))
     mc.push_project(project_dir)
 
-    diff_files = glob.glob("*-diff-*", root_dir=os.path.split(mp.fpath_meta("inserted_1_A.gpkg"))[0])
+    directory = os.path.split(mp.fpath_meta("inserted_1_A.gpkg"))[0]
+    diff_files = [f for f in os.listdir(directory) if "-diff-" in f]
 
+    # Assert that no matching files are found
     assert diff_files == []
 
 
@@ -3215,6 +3217,7 @@ def test_client_project_sync_retry(mc):
             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
@@ -3224,11 +3227,11 @@ def test_push_file_limits(mc):
     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}"):
+    with patch("mergin.local_changes.MAX_UPLOAD_VERSIONED_SIZE", 1):
+        with pytest.raises(ClientError, match=f"Some files exceeded maximum upload size. Files: base.gpkg."):
             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}"):
+    with patch("mergin.local_changes.MAX_UPLOAD_MEDIA_SIZE", 1):
+        with pytest.raises(ClientError, match=f"Some files exceeded maximum upload size. Files: test.txt."):
             mc.push_project(project_dir)

mergin/test/test_local_changes.py

@@ -1,6 +1,8 @@
 from datetime import datetime
+import pytest
+from unittest.mock import patch
 
-from ..local_changes import LocalChange, LocalChanges, MAX_UPLOAD_CHANGES
+from ..local_changes import ChangesValidationError, LocalChange, LocalChanges, MAX_UPLOAD_CHANGES
 
 
 def test_local_changes_from_dict():
@@ -120,10 +122,10 @@ 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_over_size():
+def test_local_changes_post_init_validation_media():
     """Test the get_media_upload_file method of LocalChanges."""
     # Define constants
-    SIZE_LIMIT_MB = 10
+    SIZE_LIMIT_MB = 5
     SIZE_LIMIT_BYTES = SIZE_LIMIT_MB * 1024 * 1024
     SMALL_FILE_SIZE = 1024
     LARGE_FILE_SIZE = 15 * 1024 * 1024
@@ -139,18 +141,16 @@ def test_local_changes_get_media_upload_over_size():
     ]
 
     # Initialize LocalChanges
-    local_changes = LocalChanges(added=added, updated=updated)
-
-    # Call get_media_upload_file with a size limit
-    media_file = local_changes.get_media_upload_over_size(SIZE_LIMIT_BYTES)
-
-    # Assertions
-    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
+    with patch("mergin.local_changes.MAX_UPLOAD_MEDIA_SIZE", SIZE_LIMIT_BYTES):
+        with pytest.raises(ChangesValidationError, match="Some files exceed") as err:
+            LocalChanges(added=added, updated=updated)
+            print(err.value.invalid_changes)
+    assert len(err.value.invalid_changes) == 1
+    assert "file2.jpg" == err.value.invalid_changes[0].path
+    assert err.value.invalid_changes[0].size == LARGE_FILE_SIZE
 
 
-def test_local_changes_get_gpgk_upload_over_size():
+def test_local_changes_post_init_validation_media():
     """Test the get_gpgk_upload_file method of LocalChanges."""
     # Define constants
     SIZE_LIMIT_MB = 10
@@ -166,21 +166,23 @@ def test_local_changes_get_gpgk_upload_over_size():
         ),  # Over limit
     ]
     updated = [
-        LocalChange(path="file3.gpkg", checksum="lmn456", size=5 * 1024 * 1024, mtime=datetime.now()),
+        LocalChange(
+            path="file3.gpkg",
+            checksum="lmn456",
+            size=SIZE_LIMIT_BYTES + 1,
+            mtime=datetime.now(),
+            diff={"path": "file3-diff.gpkg", "checksum": "diff123", "size": 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_file with a size limit
-    gpkg_file = local_changes.get_gpgk_upload_over_size(SIZE_LIMIT_BYTES)
-
-    # Assertions
-    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
+    with patch("mergin.local_changes.MAX_UPLOAD_VERSIONED_SIZE", SIZE_LIMIT_BYTES):
+        with pytest.raises(ChangesValidationError) as err:
+            LocalChanges(added=added, updated=updated)
+    assert len(err.value.invalid_changes) == 1
+    assert "file2.gpkg" == err.value.invalid_changes[0].path
+    assert err.value.invalid_changes[0].size == LARGE_FILE_SIZE
 
 
 def test_local_changes_post_init():