Make ChnagesHandler cleaner

Author: harminius

mergin/client_push.py

@@ -19,7 +19,7 @@
 
 from .common import UPLOAD_CHUNK_SIZE, ClientError
 from .merginproject import MerginProject
-from .editor import filter_changes
+from .editor import is_editor_enabled, _apply_editor_filters
 from .utils import is_qgis_file, is_versioned_file
 
 
@@ -84,7 +84,7 @@ def upload_blocking(self, mc, mp):
                 raise ClientError("Mismatch between uploaded file chunk {} and local one".format(self.chunk_id))
 
 
-class UploadChangesHandler:
+class ChangesHandler:
     """
     Handles preparation of file changes to be uploaded to the server.
 
@@ -95,49 +95,71 @@ class UploadChangesHandler:
     - Generating upload-ready change groups for asynchronous job creation.
     """
 
-    def __init__(self, mp, client, project_info):
-        self.mp = mp
+    def __init__(self, client, project_info, changes):
         self.client = client
         self.project_info = project_info
-        self._raw_changes = mp.get_push_changes()
-        self._filtered_changes = filter_changes(client, project_info, self._raw_changes)
+        self._raw_changes = changes
 
     @staticmethod
     def is_blocking_file(file):
         return is_qgis_file(file["path"]) or is_versioned_file(file["path"])
 
-    def split_by_type(self) -> List[Dict[str, List[dict]]]:
+    def _filter_changes(self, changes: Dict[str, List[dict]]) -> Dict[str, List[dict]]:
+        """
+        Filters the given changes dictionary based on the editor's enabled state.
+
+        If the editor is not enabled, the changes dictionary is returned as-is. Otherwise, the changes are passed through the `_apply_editor_filters` method to apply any configured filters.
+
+        Args:
+            changes (dict[str, list[dict]]): A dictionary mapping file paths to lists of change dictionaries.
+
+        Returns:
+            dict[str, list[dict]]: The filtered changes dictionary.
+        """
+        if not is_editor_enabled(self.client, self.project_info):
+            return changes
+        return _apply_editor_filters(changes)
+
+    def _split_by_type(self, changes: Dict[str, List[dict]]) -> List[Dict[str, List[dict]]]:
         """
         Split raw filtered changes into two batches:
         1. Blocking: updated/removed and added files that are blocking
         2. Non-blocking: added files that are not blocking
 
-        Returns a list of dicts each with keys:
-        - added, updated, removed, blocking
+        Adds blocking key with a boolean value for each group
         """
-        blocking_group = {"added": [], "updated": [], "removed": [], "blocking": True}
-        non_blocking_group = {"added": [], "updated": [], "removed": [], "blocking": False}
+        blocking_changes = {"added": [], "updated": [], "removed": [], "blocking": True}
+        non_blocking_changes = {"added": [], "updated": [], "removed": [], "blocking": False}
 
-        for f in self._filtered_changes.get("added", []):
+        for f in changes.get("added", []):
             if self.is_blocking_file(f):
-                blocking_group["added"].append(f)
+                blocking_changes["added"].append(f)
             else:
-                non_blocking_group["added"].append(f)
+                non_blocking_changes["added"].append(f)
 
-        for f in self._filtered_changes.get("updated", []):
-            blocking_group["updated"].append(f)
+        for f in changes.get("updated", []):
+            blocking_changes["updated"].append(f)
 
-        for f in self._filtered_changes.get("removed", []):
-            blocking_group["removed"].append(f)
+        for f in changes.get("removed", []):
+            blocking_changes["removed"].append(f)
 
         result = []
-        if any(blocking_group[k] for k in ("added", "updated", "removed")):
-            result.append(blocking_group)
-        if any(non_blocking_group["added"]):
-            result.append(non_blocking_group)
+        if any(blocking_changes[k] for k in ("added", "updated", "removed")):
+            result.append(blocking_changes)
+        if any(non_blocking_changes["added"]):
+            result.append(non_blocking_changes)
 
         return result
 
+    def split(self) -> List[Dict[str, List[dict]]]:
+        """
+        Applies all configured internal filters and returns a list of change ready to be uploaded.
+        """
+        changes = self._filter_changes(self._raw_changes)
+        changes = self._split_by_type(changes)
+        # TODO: apply limits; changes = self._limit_by_file_count(changes)
+        return changes
+
 
 def push_project_async(mc, directory) -> Optional[List[UploadJob]]:
     """Starts push of a project and returns pending upload jobs"""
@@ -177,13 +199,14 @@ def push_project_async(mc, directory) -> Optional[List[UploadJob]]:
             + f"\n\nLocal version: {local_version}\nServer version: {server_version}"
         )
 
-    changes_handler = UploadChangesHandler(mp, mc, project_info)
-    changes_groups = changes_handler.split_by_type()
+    changes = mp.get_push_changes()
+    changes_handler = ChangesHandler(mc, project_info, changes)
+    changes_list = changes_handler.split()
 
     tmp_dir = tempfile.TemporaryDirectory(prefix="python-api-client-")
     jobs = []
 
-    for changes in changes_groups:
+    for changes in changes_list:
         mp.log.debug("push changes:\n" + pprint.pformat(changes))
 
         # If there are any versioned files (aka .gpkg) that are not updated through a diff,

mergin/editor.py

@@ -40,23 +40,6 @@ def _apply_editor_filters(changes: Dict[str, List[dict]]) -> Dict[str, List[dict
     return changes
 
 
-def filter_changes(mc, project_info: dict, changes: Dict[str, List[dict]]) -> Dict[str, List[dict]]:
-    """
-    Filters the given changes dictionary based on the editor's enabled state.
-
-    If the editor is not enabled, the changes dictionary is returned as-is. Otherwise, the changes are passed through the `_apply_editor_filters` method to apply any configured filters.
-
-    Args:
-        changes (dict[str, list[dict]]): A dictionary mapping file paths to lists of change dictionaries.
-
-    Returns:
-        dict[str, list[dict]]: The filtered changes dictionary.
-    """
-    if not is_editor_enabled(mc, project_info):
-        return changes
-    return _apply_editor_filters(changes)
-
-
 def prevent_conflicted_copy(path: str, mc, project_info: dict) -> bool:
     """
     Decides whether a file path should be blocked from creating a conflicted copy.

mergin/test/test_client.py

@@ -21,7 +21,7 @@
     TokenError,
     ServerType,
 )
-from ..client_push import push_project_async, push_project_cancel
+from ..client_push import push_project_async, push_project_cancel, ChangesHandler
 from ..client_pull import (
     download_project_async,
     download_project_wait,
@@ -38,7 +38,7 @@
 )
 from ..merginproject import pygeodiff
 from ..report import create_report
-from ..editor import EDITOR_ROLE_NAME, filter_changes, is_editor_enabled
+from ..editor import EDITOR_ROLE_NAME, is_editor_enabled
 from ..common import ErrorCode, WorkspaceRole, ProjectRole
 
 SERVER_URL = os.environ.get("TEST_MERGIN_URL")
@@ -2593,7 +2593,8 @@ def test_editor(mc: MerginClient):
         "updated": [{"path": "/folder/project.updated.Qgs"}],
         "removed": [{"path": "/folder/project.removed.qgs"}],
     }
-    qgs_changeset = filter_changes(mc, project_info, qgs_changeset)
+    changes_handler = ChangesHandler(mc, project_info, qgs_changeset)
+    qgs_changeset = changes_handler._filter_changes(qgs_changeset)
     assert sum(len(v) for v in qgs_changeset.values()) == 2