diff --git a/server/mergin/sync/models.py b/server/mergin/sync/models.py index 3358ba35..bc01c523 100644 --- a/server/mergin/sync/models.py +++ b/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: diff --git a/server/mergin/sync/public_api_v2.yaml b/server/mergin/sync/public_api_v2.yaml index 67ca4523..b01f6781 100644 --- a/server/mergin/sync/public_api_v2.yaml +++ b/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 diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 795f81aa..251fcf97 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/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( diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 95654032..528a45d0 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/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