Merge pull request #584 from MerginMaps/change-path-to-query-file

Fix subfolders geopackages diffs

Author: MarcelGeo

server/mergin/sync/models.py

@@ -411,6 +411,11 @@ def get_delta_changes(
                 result.extend(DeltaChangeSchema(many=True).load(existing_delta.changes))
                 continue
 
+            if checkpoint.rank == 0:
+                raise ValueError(
+                    f"Missing expected checkpoint of rank 0 for version {checkpoint.end}"
+                )
+
             # If higher rank delta checkopoint does not exists we need to create it
             if checkpoint.rank > 0:
                 new_checkpoint = ProjectVersionDelta.create_checkpoint(
@@ -918,6 +923,11 @@ def construct_checkpoint(self) -> bool:
         if os.path.exists(self.abs_path):
             return True
 
+        if self.rank == 0:
+            raise ValueError(
+                "Checkpoint of rank 0 should be created by user upload, cannot be constructed"
+            )
+
         # merged diffs can only be created for certain versions
         if self.version % LOG_BASE:
             return False
@@ -1245,6 +1255,11 @@ def create_checkpoint(
         """
         Creates and caches new checkpoint and any required FileDiff checkpoints recursively if needed.
         """
+        if checkpoint.rank == 0:
+            raise ValueError(
+                f"Missing expected checkpoint of rank 0 for version {checkpoint.end}"
+            )
+
         delta_range = []
         # our new checkpoint will be created by adding last individual delta to previous checkpoints
         expected_checkpoints = Checkpoint.get_checkpoints(
@@ -1267,7 +1282,7 @@ def create_checkpoint(
         # make sure we have all components, if not, created them (recursively)
         for item in expected_checkpoints:
             existing_delta = existing_delta_map.get((item.rank, item.end))
-            if not existing_delta:
+            if not existing_delta and item.rank > 0:
                 existing_delta = cls.create_checkpoint(project_id, item)
 
             if existing_delta:

server/mergin/sync/public_api_v2.yaml

@@ -280,7 +280,7 @@ paths:
         "404":
           $ref: "#/components/responses/NotFound"
       x-openapi-router-controller: mergin.sync.public_api_v2_controller
-  /projects/{id}/raw/diff/{file}:
+  /projects/{id}/raw/diff:
     get:
       tags:
         - project
@@ -290,7 +290,7 @@ paths:
         - $ref: "#/components/parameters/ProjectId"
         - name: file
           required: true
-          in: path
+          in: query
           description: File id of the diff file to download
           schema:
             type: string
@@ -443,7 +443,7 @@ paths:
       summary: List projects in the workspace
       operationId: list_workspace_projects
       parameters:
-        - $ref: '#/components/parameters/WorkspaceId'
+        - $ref: "#/components/parameters/WorkspaceId"
         - name: page
           in: query
           description: page number

server/mergin/sync/public_api_v2_controller.py

@@ -457,7 +457,14 @@ def get_project_delta(id: str, since: str, to: Optional[str] = None):
             400,
             f"""The 'since' parameter must be less than or equal to the {"'to' parameter" if to_provided else 'latest project version'}""",
         )
-    delta_changes = project.get_delta_changes(since_version, to_version) or []
+
+    try:
+        delta_changes = project.get_delta_changes(since_version, to_version) or []
+    except ValueError:
+        logging.exception(
+            f"Failed to get delta changes for project {project.id} between versions {since_version} and {to_version}"
+        )
+        abort(422)
 
     return (
         DeltaChangeRespSchema().dump(

server/mergin/tests/test_public_api_v2.py

@@ -212,7 +212,9 @@ def test_file_diff_download(client, diff_project):
         file_path_id=gpkg_file.id, version=4, rank=0
     ).first()
 
-    response = client.get(f"v2/projects/{diff_project.id}/raw/diff/{diff_file.path}")
+    response = client.get(
+        f"v2/projects/{diff_project.id}/raw/diff?file={diff_file.path}"
+    )
     assert response.status_code == 200
     assert response.content_type == "application/octet-stream"
 
@@ -231,7 +233,7 @@ def test_file_diff_download(client, diff_project):
     assert not os.path.exists(diff.abs_path)
 
     # download merged diff with its reconstuction on the fly
-    response = client.get(f"v2/projects/{diff_project.id}/raw/diff/{diff.path}")
+    response = client.get(f"v2/projects/{diff_project.id}/raw/diff?file={diff.path}")
     assert response.status_code == 200
     assert response.content_type == "application/octet-stream"
     assert os.path.exists(diff.abs_path)
@@ -240,13 +242,39 @@ def test_file_diff_download(client, diff_project):
     with patch.object(FileDiff, "construct_checkpoint") as construct_checkpoint_mock:
         os.remove(diff.abs_path)
         construct_checkpoint_mock.return_value = False
-        response = client.get(f"v2/projects/{diff_project.id}/raw/diff/{diff.path}")
+        response = client.get(
+            f"v2/projects/{diff_project.id}/raw/diff?file={diff.path}"
+        )
         assert response.status_code == 422
         assert response.json["code"] == DiffDownloadError.code
 
-    response = client.get(f"v2/projects/{diff_project.id}/raw/diff/{diff.path}+1")
+    response = client.get(f"v2/projects/{diff_project.id}/raw/diff?file={diff.path}+1")
     assert response.status_code == 404
 
+    subfolder_test_gpkg = os.path.join(
+        diff_project.storage.project_dir, "test_dir", "test.gpkg"
+    )
+    os.makedirs(os.path.dirname(subfolder_test_gpkg), exist_ok=True)
+    shutil.copy(
+        os.path.join(test_project_dir, "base.gpkg"),
+        subfolder_test_gpkg,
+    )
+    # shutil.copy(
+    #     test_gpkg_file,
+    #     os.path.join(diff_project.storage.project_dir, "v9", "test_dir", "test.gpkg"),
+    # )
+    push_change(
+        diff_project, "added", "test_dir/test.gpkg", diff_project.storage.project_dir
+    )
+    sql = f"UPDATE simple SET rating={1000}"
+    execute_query(subfolder_test_gpkg, sql)
+    pv = push_change(
+        diff_project, "updated", "test_dir/test.gpkg", diff_project.storage.project_dir
+    )
+    fd = FileDiff.query.filter_by(version=pv.name, rank=0).first()
+    response = client.get(f"v2/projects/{diff_project.id}/raw/diff?file={fd.path}")
+    assert response.status_code == 200
+
 
 def test_create_diff_checkpoint(diff_project):
     """Test creation of diff checkpoints"""
@@ -738,7 +766,7 @@ def test_project_version_delta_changes(client, diff_project: Project):
 
     # check if checkpoint will be there
     response = client.get(
-        f"v2/projects/{diff_project.id}/raw/diff/{delta[0].diffs[0].id}"
+        f"v2/projects/{diff_project.id}/raw/diff?file={delta[0].diffs[0].id}"
     )
     assert response.status_code == 200
 
@@ -1339,7 +1367,9 @@ def test_project_pull_diffs(client, diff_project):
     second_diff = delta[0]["diffs"][1]
     assert first_diff["id"] == current_diffs[0].path
     assert second_diff["id"] == current_diffs[1].path
-    response = client.get(f"v2/projects/{diff_project.id}/raw/diff/{first_diff['id']}")
+    response = client.get(
+        f"v2/projects/{diff_project.id}/raw/diff?file={first_diff['id']}"
+    )
     assert response.status_code == 200