From 7f8d27e7b352c8e79aa718b1aa73720d7818ac0a Mon Sep 17 00:00:00 2001 From: Alexander Bruy Date: Wed, 11 Oct 2023 17:02:05 +0300 Subject: [PATCH 1/4] save full project information to .mergin/mergin.json file (fix #83) --- mergin/client.py | 10 +--------- mergin/client_pull.py | 19 ++----------------- mergin/client_push.py | 9 +-------- mergin/merginproject.py | 20 ++++++++++++++++---- 4 files changed, 20 insertions(+), 38 deletions(-) diff --git a/mergin/client.py b/mergin/client.py index f5062aca..14ebe033 100644 --- a/mergin/client.py +++ b/mergin/client.py @@ -519,15 +519,7 @@ def create_project_and_push(self, project_name, directory, is_public=False, name self.create_project(project_name, is_public) if directory: project_info = self.project_info(project_name) - MerginProject.write_metadata( - directory, - { - "name": project_name, - "version": "v0", - "files": [], - "project_id": project_info["id"], - }, - ) + MerginProject.write_metadata(directory, project_info) mp = MerginProject(directory) if mp.inspect_files(): self.push_project(directory) diff --git a/mergin/client_pull.py b/mergin/client_pull.py index 0b1b5ef3..7baf9463 100644 --- a/mergin/client_pull.py +++ b/mergin/client_pull.py @@ -215,15 +215,7 @@ def download_project_finalize(job): task.apply(job.directory, job.mp) # final update of project metadata - # TODO: why not exact copy of project info JSON ? - job.mp.update_metadata( - { - "name": job.project_path, - "version": job.version, - "project_id": job.project_info["id"], - "files": job.project_info["files"], - } - ) + job.mp.update_metadata(job.project_info) def download_project_cancel(job): @@ -613,14 +605,7 @@ def pull_project_finalize(job): job.mp.log.info("--- pull aborted") raise ClientError("Failed to apply pull changes: " + str(e)) - job.mp.update_metadata( - { - "name": job.project_path, - "version": job.version if job.version else "v0", # for new projects server version is "" - "project_id": job.project_info["id"], - "files": job.project_info["files"], - } - ) + job.mp.update_metadata(job.project_info) if job.mp.has_unfinished_pull(): job.mp.log.info("--- failed to complete pull -- project left in the unfinished pull state") diff --git a/mergin/client_push.py b/mergin/client_push.py index 1c357715..de48fc16 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -272,14 +272,7 @@ def push_project_finalize(job): job.mp.log.info("cancel response: " + str(err2)) raise err - job.mp.update_metadata( - { - "name": job.project_path, - "version": job.server_resp["version"], - "project_id": job.server_resp["id"], - "files": job.server_resp["files"], - } - ) + job.mp.update_metadata(job.server_resp) try: job.mp.apply_push_changes(job.changes) except Exception as e: diff --git a/mergin/merginproject.py b/mergin/merginproject.py index 93fa7206..9f1da0ae 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -139,7 +139,11 @@ def project_full_name(self) -> str: """Returns fully qualified project name: /""" if self._metadata is None: self._read_metadata() - return self._metadata["name"] + return ( + self._metadata["name"] + if "/" in self._metadata["name"] + else f"{self._metadata['namespace']}/{self._metadata['name']}" + ) def project_name(self) -> str: """Returns only project name, without its workspace name""" @@ -154,10 +158,18 @@ def workspace_name(self) -> str: return full_name[:slash_index] def project_id(self) -> str: - """Returns ID of the project (UUID using 8-4-4-4-12 formatting without braces)""" + """Returns ID of the project (UUID using 8-4-4-4-12 formatting without braces) + + Raises ClientError if project id is not present in the project metadata. + """ if self._metadata is None: self._read_metadata() - return self._metadata["project_id"] + + # "id" or "project_id" may not exist in projects downloaded with old client version + if "id" not in self._metadata and "project_id" not in self._metadata: + raise ClientError("Missed project id metadata. Please re-download your project.") + + return self._metadata["project_id"] if "/" in self._metadata["name"] else self._metadata["id"] def workspace_id(self) -> int: """Returns ID of the workspace where the project belongs""" @@ -169,7 +181,7 @@ def version(self) -> str: """Returns project version (e.g. "v123")""" if self._metadata is None: self._read_metadata() - return self._metadata["version"] + return self._metadata["version"] if self._metadata["version"] else "v0" def files(self) -> list: """Returns project's list of files (each file being a dictionary)""" From 35e350f022218df5cfdb139ad9b0734382824ae2 Mon Sep 17 00:00:00 2001 From: Alexander Bruy Date: Thu, 12 Oct 2023 10:48:58 +0300 Subject: [PATCH 2/4] add test --- mergin/test/test_client.py | 25 ++++++++ mergin/test/test_data/old_metadata.json | 79 +++++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 mergin/test/test_data/old_metadata.json diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py index febededf..734e92f1 100644 --- a/mergin/test/test_client.py +++ b/mergin/test/test_client.py @@ -2003,3 +2003,28 @@ def test_clean_diff_files(mc): diff_files = glob.glob("*-diff-*", root_dir=os.path.split(mp.fpath_meta("inserted_1_A.gpkg"))[0]) assert diff_files == [] + + +def test_project_metadata(mc): + test_project = "test_project_metadata" + project = API_USER + "/" + test_project + project_dir = os.path.join(TMP_DIR, test_project) + + cleanup(mc, project, [project_dir]) + # prepare local project + shutil.copytree(TEST_DATA_DIR, project_dir) + + # create remote project + mc.create_project_and_push(test_project, directory=project_dir) + + # copy metadata in old format + project_metadata = os.path.join(project_dir, ".mergin", "mergin.json") + old_metadata = os.path.join(project_dir, "old_metadata.json") + shutil.copyfile(old_metadata, project_metadata) + + # verify we have correct metadata + mp = MerginProject(project_dir) + assert mp.project_full_name() == f"{API_USER}/{test_project}" + assert mp.project_name() == test_project + assert mp.workspace_name() == API_USER + assert mp.version() == "v1" diff --git a/mergin/test/test_data/old_metadata.json b/mergin/test/test_data/old_metadata.json new file mode 100644 index 00000000..63411820 --- /dev/null +++ b/mergin/test/test_data/old_metadata.json @@ -0,0 +1,79 @@ +{ + "name": "test_plugin/test_project_metadata", + "version": "v1", + "project_id": "46e0a252-eb74-456c-9a64-b21d3fba59d6", + "files": [ + { + "checksum": "b3426f7d02643c1e0b24d738eee7a54f1c74eca4", + "mtime": "2023-10-12T06:38:49.314490Z", + "path": "two_tables_drop.gpkg", + "size": 118784 + }, + { + "checksum": "67e64b55b36e945df8c97afc46ff64acfe46b32a", + "mtime": "2023-10-12T06:38:49.314496Z", + "path": "base.gpkg", + "size": 98304 + }, + { + "checksum": "38662e5645cd0a80009d43fd0f32db79dffabafa", + "mtime": "2023-10-12T06:38:49.314497Z", + "path": "test.txt", + "size": 13 + }, + { + "checksum": "4973d10c6350604fe5347a11c7d2c59fb06a36e1", + "mtime": "2023-10-12T06:38:49.314497Z", + "path": "inserted_1_A_mod.gpkg", + "size": 98304 + }, + { + "checksum": "a191cd1dc405642b70f3525f638e1cf8d1052828", + "mtime": "2023-10-12T06:38:49.314498Z", + "path": "two_tables.gpkg", + "size": 118784 + }, + { + "checksum": "38662e5645cd0a80009d43fd0f32db79dffabafa", + "mtime": "2023-10-12T06:38:49.314499Z", + "path": "test3.txt", + "size": 13 + }, + { + "checksum": "da39a3ee5e6b4b0d3255bfef95601890afd80709", + "mtime": "2023-10-12T06:38:49.314499Z", + "path": "test.qgs", + "size": 0 + }, + { + "checksum": "473501ebd51a3ade52f07d8e6c3887bcf2d7803f", + "mtime": "2023-10-12T06:38:49.314500Z", + "path": "two_tables_1_A.gpkg", + "size": 118784 + }, + { + "checksum": "fa2a379a0626b9505ba61f85d207ad66e9548f83", + "mtime": "2023-10-12T06:38:49.314501Z", + "path": "inserted_1_A.gpkg", + "size": 98304 + }, + { + "checksum": "763598af125e24c25db319d688d1b7275aae6d58", + "mtime": "2023-10-12T06:38:49.314501Z", + "path": "inserted_1_B.gpkg", + "size": 98304 + }, + { + "checksum": "aca1d870a2aca068ce360f51c2eb1fc0d08a23cc", + "mtime": "2023-10-12T06:38:49.314502Z", + "path": "test_dir/modified_1_geom.gpkg", + "size": 98304 + }, + { + "checksum": "dc0c460d742019ca818f1d74c2bc8fbc28d3ad68", + "mtime": "2023-10-12T06:38:49.314502Z", + "path": "test_dir/test2.txt", + "size": 14 + } + ] +} \ No newline at end of file From 417a943500a964592788d5b11db1b6f64cf307d9 Mon Sep 17 00:00:00 2001 From: Alexander Bruy Date: Mon, 16 Oct 2023 12:19:41 +0300 Subject: [PATCH 3/4] address review --- mergin/merginproject.py | 22 ++++--- mergin/test/test_client.py | 22 +++++-- mergin/test/test_data/new_metadata.json | 43 +++++++++++++ mergin/test/test_data/old_metadata.json | 81 ++----------------------- 4 files changed, 77 insertions(+), 91 deletions(-) create mode 100644 mergin/test/test_data/new_metadata.json diff --git a/mergin/merginproject.py b/mergin/merginproject.py index 9f1da0ae..f57107bc 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -58,6 +58,7 @@ def __init__(self, directory): # metadata from JSON are lazy loaded self._metadata = None + self.is_old_metadata = False self.setup_logging(directory) @@ -139,11 +140,10 @@ def project_full_name(self) -> str: """Returns fully qualified project name: /""" if self._metadata is None: self._read_metadata() - return ( - self._metadata["name"] - if "/" in self._metadata["name"] - else f"{self._metadata['namespace']}/{self._metadata['name']}" - ) + if self.is_old_metadata: + return self._metadata["name"] + else: + return f"{self._metadata['namespace']}/{self._metadata['name']}" def project_name(self) -> str: """Returns only project name, without its workspace name""" @@ -160,14 +160,18 @@ def workspace_name(self) -> str: def project_id(self) -> str: """Returns ID of the project (UUID using 8-4-4-4-12 formatting without braces) - Raises ClientError if project id is not present in the project metadata. + Raises ClientError if project id is not present in the project metadata. This should + only happen with projects downloaded with old client, before February 2023, + see https://github.com/MerginMaps/mergin-py-client/pull/154 """ if self._metadata is None: self._read_metadata() # "id" or "project_id" may not exist in projects downloaded with old client version if "id" not in self._metadata and "project_id" not in self._metadata: - raise ClientError("Missed project id metadata. Please re-download your project.") + raise ClientError( + "The project directory has been created with an old version of the Mergin Maps client. Please delete the project directory and re-download the project." + ) return self._metadata["project_id"] if "/" in self._metadata["name"] else self._metadata["id"] @@ -181,7 +185,7 @@ def version(self) -> str: """Returns project version (e.g. "v123")""" if self._metadata is None: self._read_metadata() - return self._metadata["version"] if self._metadata["version"] else "v0" + return self._metadata["version"] def files(self) -> list: """Returns project's list of files (each file being a dictionary)""" @@ -208,6 +212,8 @@ def _read_metadata(self): with open(self.fpath_meta("mergin.json"), "r") as file: self._metadata = json.load(file) + self.is_old_metadata = "/" in self._metadata["name"] + def update_metadata(self, data: dict): """Writes project metadata and updates cached metadata.""" self._metadata = data diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py index 734e92f1..36cbd0d2 100644 --- a/mergin/test/test_client.py +++ b/mergin/test/test_client.py @@ -2011,20 +2011,30 @@ def test_project_metadata(mc): project_dir = os.path.join(TMP_DIR, test_project) cleanup(mc, project, [project_dir]) + # prepare local project shutil.copytree(TEST_DATA_DIR, project_dir) - # create remote project - mc.create_project_and_push(test_project, directory=project_dir) - # copy metadata in old format + os.makedirs(os.path.join(project_dir, ".mergin"), exist_ok=True) project_metadata = os.path.join(project_dir, ".mergin", "mergin.json") - old_metadata = os.path.join(project_dir, "old_metadata.json") - shutil.copyfile(old_metadata, project_metadata) + metadata_file = os.path.join(project_dir, "old_metadata.json") + shutil.copyfile(metadata_file, project_metadata) + + # verify we have correct metadata + mp = MerginProject(project_dir) + assert mp.project_full_name() == f"{API_USER}/{test_project}" + assert mp.project_name() == test_project + assert mp.workspace_name() == API_USER + assert mp.version() == "v0" + + # copy metadata in new format + metadata_file = os.path.join(project_dir, "new_metadata.json") + shutil.copyfile(metadata_file, project_metadata) # verify we have correct metadata mp = MerginProject(project_dir) assert mp.project_full_name() == f"{API_USER}/{test_project}" assert mp.project_name() == test_project assert mp.workspace_name() == API_USER - assert mp.version() == "v1" + assert mp.version() == "v0" diff --git a/mergin/test/test_data/new_metadata.json b/mergin/test/test_data/new_metadata.json new file mode 100644 index 00000000..5dcb4ff9 --- /dev/null +++ b/mergin/test/test_data/new_metadata.json @@ -0,0 +1,43 @@ +{ + "access": { + "owners": [ + 2 + ], + "ownersnames": [ + "test_plugin" + ], + "public": false, + "readers": [ + 2 + ], + "readersnames": [ + "test_plugin" + ], + "writers": [ + 2 + ], + "writersnames": [ + "test_plugin" + ] + }, + "created": "2023-10-16T09:17:27Z", + "creator": 2, + "disk_usage": 0, + "files": [], + "id": "a6f9d38c-e30d-49f2-bfc5-76495afdbf27", + "name": "test_project_metadata", + "namespace": "test_plugin", + "permissions": { + "delete": true, + "update": true, + "upload": true + }, + "removed_at": null, + "removed_by": null, + "role": "owner", + "tags": [], + "updated": "2023-10-16T09:17:27.345127", + "uploads": [], + "version": "v0", + "workspace_id": 18 +} \ No newline at end of file diff --git a/mergin/test/test_data/old_metadata.json b/mergin/test/test_data/old_metadata.json index 63411820..8363ad0d 100644 --- a/mergin/test/test_data/old_metadata.json +++ b/mergin/test/test_data/old_metadata.json @@ -1,79 +1,6 @@ { "name": "test_plugin/test_project_metadata", - "version": "v1", - "project_id": "46e0a252-eb74-456c-9a64-b21d3fba59d6", - "files": [ - { - "checksum": "b3426f7d02643c1e0b24d738eee7a54f1c74eca4", - "mtime": "2023-10-12T06:38:49.314490Z", - "path": "two_tables_drop.gpkg", - "size": 118784 - }, - { - "checksum": "67e64b55b36e945df8c97afc46ff64acfe46b32a", - "mtime": "2023-10-12T06:38:49.314496Z", - "path": "base.gpkg", - "size": 98304 - }, - { - "checksum": "38662e5645cd0a80009d43fd0f32db79dffabafa", - "mtime": "2023-10-12T06:38:49.314497Z", - "path": "test.txt", - "size": 13 - }, - { - "checksum": "4973d10c6350604fe5347a11c7d2c59fb06a36e1", - "mtime": "2023-10-12T06:38:49.314497Z", - "path": "inserted_1_A_mod.gpkg", - "size": 98304 - }, - { - "checksum": "a191cd1dc405642b70f3525f638e1cf8d1052828", - "mtime": "2023-10-12T06:38:49.314498Z", - "path": "two_tables.gpkg", - "size": 118784 - }, - { - "checksum": "38662e5645cd0a80009d43fd0f32db79dffabafa", - "mtime": "2023-10-12T06:38:49.314499Z", - "path": "test3.txt", - "size": 13 - }, - { - "checksum": "da39a3ee5e6b4b0d3255bfef95601890afd80709", - "mtime": "2023-10-12T06:38:49.314499Z", - "path": "test.qgs", - "size": 0 - }, - { - "checksum": "473501ebd51a3ade52f07d8e6c3887bcf2d7803f", - "mtime": "2023-10-12T06:38:49.314500Z", - "path": "two_tables_1_A.gpkg", - "size": 118784 - }, - { - "checksum": "fa2a379a0626b9505ba61f85d207ad66e9548f83", - "mtime": "2023-10-12T06:38:49.314501Z", - "path": "inserted_1_A.gpkg", - "size": 98304 - }, - { - "checksum": "763598af125e24c25db319d688d1b7275aae6d58", - "mtime": "2023-10-12T06:38:49.314501Z", - "path": "inserted_1_B.gpkg", - "size": 98304 - }, - { - "checksum": "aca1d870a2aca068ce360f51c2eb1fc0d08a23cc", - "mtime": "2023-10-12T06:38:49.314502Z", - "path": "test_dir/modified_1_geom.gpkg", - "size": 98304 - }, - { - "checksum": "dc0c460d742019ca818f1d74c2bc8fbc28d3ad68", - "mtime": "2023-10-12T06:38:49.314502Z", - "path": "test_dir/test2.txt", - "size": 14 - } - ] -} \ No newline at end of file + "version": "v0", + "project_id": "effeca08-ef22-4fc1-b620-5261c6a081eb", + "files": [] +} From 649cdd22f719c4940ede14271ba5f5a6d6c241a9 Mon Sep 17 00:00:00 2001 From: Alexander Bruy Date: Mon, 16 Oct 2023 16:32:11 +0300 Subject: [PATCH 4/4] address review --- mergin/merginproject.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/mergin/merginproject.py b/mergin/merginproject.py index f57107bc..1cd3fa91 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -168,12 +168,15 @@ def project_id(self) -> str: self._read_metadata() # "id" or "project_id" may not exist in projects downloaded with old client version - if "id" not in self._metadata and "project_id" not in self._metadata: - raise ClientError( - "The project directory has been created with an old version of the Mergin Maps client. Please delete the project directory and re-download the project." - ) - - return self._metadata["project_id"] if "/" in self._metadata["name"] else self._metadata["id"] + if self.is_old_metadata: + if "project_id" not in self._metadata: + raise ClientError( + "The project directory has been created with an old version of the Mergin Maps client. " + "Please delete the project directory and re-download the project." + ) + return self._metadata["project_id"] + else: + return self._metadata["id"] def workspace_id(self) -> int: """Returns ID of the workspace where the project belongs"""