From 07ba7fba979344871902d64b172896e442c2bf8b Mon Sep 17 00:00:00 2001 From: marcel Date: Mon, 11 Oct 2021 18:25:20 +0200 Subject: [PATCH 1/5] added commands to add or remove project collaborators added command to get project collaborators added commands to add or remove project collaborators added command to get project collaborators --- README.md | 7 +++++ mergin/cli.py | 52 ++++++++++++++++++++++++++++++++++++-- mergin/client.py | 33 ++++++++++++++++++++++++ mergin/merginproject.py | 2 +- mergin/test/test_client.py | 37 +++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1e8406ed..94aa215c 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,9 @@ Commands: show-file-history Displays information about a single version of a... show-version Displays information about a single version of a... status Show all changes in project files - upstream and... + share Show project permissions + share-add Add user to project permissions + share-del Remove user from project's collaborators ``` For example, to download a project: @@ -111,6 +114,10 @@ it is possible to run other commands without specifying username/password. ## Development +### Setup local dependencies +pip install -e ../ + + ### How to release 1. Update version in `setup.py` and `mergin/version.py` diff --git a/mergin/cli.py b/mergin/cli.py index 2a241d22..54525978 100755 --- a/mergin/cli.py +++ b/mergin/cli.py @@ -31,8 +31,11 @@ download_project_finalize, download_project_is_running, ) -from mergin.client_pull import pull_project_async, pull_project_is_running, pull_project_finalize, pull_project_cancel -from mergin.client_push import push_project_async, push_project_is_running, push_project_finalize, push_project_cancel +from mergin.client_pull import pull_project_async, pull_project_is_running, pull_project_finalize, \ + pull_project_cancel +from mergin.client_push import push_project_async, push_project_is_running, push_project_finalize, \ + push_project_cancel + from pygeodiff import GeoDiff @@ -269,6 +272,51 @@ def download(ctx, project, directory, version): _print_unhandled_exception() +@cli.command() +@click.argument("project") +@click.argument("usernames", nargs=-1) +@click.option("--permissions") +@click.pass_context +def share_add(ctx, project, usernames, permissions): + mc = ctx.obj["client"] + if mc is None: + return + usernames = list(usernames) + print(usernames) + mc.add_user_permissions_to_project(project, usernames, permissions) + + +@cli.command() +@click.argument("project") +@click.argument("usernames", nargs=-1) +@click.pass_context +def share_del(ctx, project, usernames): + mc = ctx.obj["client"] + if mc is None: + return + usernames = list(usernames) + mc.remove_user_permissions_to_project(project, usernames) + + +@cli.command() +@click.argument("project") +@click.pass_context +def share(ctx, project): + mc = ctx.obj["client"] + if mc is None: + return + access_list = mc.share_list(project) + + owners = ", ".join(access_list.get("owners")) + writers = ", ".join(access_list.get("writers")) + readers = ", ".join(access_list.get("readers")) + + result = f"owners: {owners}\n" \ + f"writers: {writers}\n" \ + f"readers: {readers}" + click.echo(result) + + @cli.command() @click.argument("filepath") @click.argument("output") diff --git a/mergin/client.py b/mergin/client.py index 15dc5194..f79d29bc 100644 --- a/mergin/client.py +++ b/mergin/client.py @@ -519,6 +519,39 @@ def set_project_access(self, project_path, access): detail = f"Project path: {project_path}" raise ClientError(str(e), detail) + def add_user_permissions_to_project(self, project_path, usernames, permission_level): + project_info = self.project_info(project_path) + access = project_info.get('access') + for name in usernames: + if permission_level == "owner": + access.get("ownersnames").append(name) + if permission_level == "writer" or permission_level == "owner": + access.get("writersnames").append(name) + if permission_level == "writer" or permission_level == "owner" or permission_level == "reader": + access.get("readersnames").append(name) + self.set_project_access(project_path, access) + + def remove_user_permissions_to_project(self, project_path, usernames): + project_info = self.project_info(project_path) + access = project_info.get('access') + for name in usernames: + if name in access.get("ownersnames"): + access.get("ownersnames").remove(name) + if name in access.get("writersnames"): + access.get("writersnames").remove(name) + if name in access.get("readersnames"): + access.get("readersnames").remove(name) + self.set_project_access(project_path, access) + + def share_list(self, project_path): + project_info = self.project_info(project_path) + access = project_info.get('access') + result = {} + result["owners"] = access.get("ownersnames") + result["writers"] = access.get("writersnames") + result["readers"] = access.get("readersnames") + return result + def push_project(self, directory): """ Upload local changes to the repository. diff --git a/mergin/merginproject.py b/mergin/merginproject.py index d13c41e0..8003f991 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -21,7 +21,7 @@ # python paths. try: from .deps import pygeodiff -except ImportError: +except ImportError or ModuleNotFoundError: import pygeodiff diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py index 908d73f2..521d8186 100644 --- a/mergin/test/test_client.py +++ b/mergin/test/test_client.py @@ -941,6 +941,43 @@ def test_download_diffs(mc): assert "Available versions: [1, 2, 3, 4]" in str(e.value) +def test_modify_project_permissions(mc): + test_project = 'test_project' + project = API_USER + '/' + test_project + project_dir = os.path.join(TMP_DIR, test_project) + download_dir = os.path.join(TMP_DIR, 'download', test_project) + + cleanup(mc, project, [project_dir, download_dir]) + # prepare local project + shutil.copytree(TEST_DATA_DIR, project_dir) + + # create remote project + mc.create_project_and_push(test_project, directory=project_dir) + + # check basic metadata about created project + project_info = mc.project_info(project) + assert project_info['version'] == 'v1' + assert project_info['name'] == test_project + assert project_info['namespace'] == API_USER + + permissions = mc.share_list(project) + assert permissions["owners"] == [API_USER] + assert permissions["writers"] == [API_USER] + assert permissions["readers"] == [API_USER] + + mc.add_user_permissions_to_project(project, [API_USER2], "writer") + permissions = mc.share_list(project) + assert set(permissions["owners"]) == {API_USER} + assert set(permissions["writers"]) == {API_USER, API_USER2} + assert set(permissions["readers"]) == {API_USER, API_USER2} + + mc.remove_user_permissions_to_project(project, [API_USER2]) + permissions = mc.share_list(project) + assert permissions["owners"] == [API_USER] + assert permissions["writers"] == [API_USER] + assert permissions["readers"] == [API_USER] + + def _use_wal(db_file): """ Ensures that sqlite database is using WAL journal mode """ con = sqlite3.connect(db_file) From abb5eca30ac86c937e63d12277e64bfcaf4fd2b6 Mon Sep 17 00:00:00 2001 From: marcel Date: Tue, 12 Oct 2021 12:04:14 +0200 Subject: [PATCH 2/5] modified processing of response from project/push/cancel endpoint because has been changed --- mergin/client_push.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/mergin/client_push.py b/mergin/client_push.py index 6afd18dc..5aad6d57 100644 --- a/mergin/client_push.py +++ b/mergin/client_push.py @@ -70,7 +70,10 @@ def upload_blocking(self, mc, mp): resp_dict = json.load(resp) mp.log.debug(f"Upload finished: {self.file_path}") if not (resp_dict['size'] == len(data) and resp_dict['checksum'] == checksum.hexdigest()): - mc.post("/v1/project/push/cancel/{}".format(self.transaction_id)) + try: + mc.post("/v1/project/push/cancel/{}".format(self.transaction_id)) + except ClientError: + pass raise ClientError("Mismatch between uploaded file chunk {} and local one".format(self.chunk_id)) @@ -263,9 +266,11 @@ def push_project_finalize(job): # if push finish fails, the transaction is not killed, so we # need to cancel it so it does not block further uploads job.mp.log.info("canceling the pending transaction...") - resp_cancel = job.mc.post("/v1/project/push/cancel/%s" % job.transaction_id) - server_resp_cancel = json.load(resp_cancel) - job.mp.log.info("cancel response: " + str(server_resp_cancel)) + try: + resp_cancel = job.mc.post("/v1/project/push/cancel/%s" % job.transaction_id) + job.mp.log.info("cancel response: " + resp_cancel.msg) + except ClientError as err2: + job.mp.log.info("cancel response: " + str(err2)) raise err job.mp.metadata = { @@ -293,7 +298,7 @@ def push_project_cancel(job): job.executor.shutdown(wait=True) try: resp_cancel = job.mc.post("/v1/project/push/cancel/%s" % job.transaction_id) - job.server_resp = json.load(resp_cancel) + job.server_resp = resp_cancel.msg except ClientError as err: job.mp.log.error("--- push cancelling failed! " + str(err)) raise err From 0d70ee8beebd8d615edc32ee474fa684f9115ebb Mon Sep 17 00:00:00 2001 From: marcel Date: Tue, 12 Oct 2021 13:51:46 +0200 Subject: [PATCH 3/5] comments processed --- README.md | 2 +- mergin/cli.py | 25 ++++++++++++------------- mergin/client.py | 25 +++++++++++++++++++++++-- mergin/test/test_client.py | 8 ++++---- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 94aa215c..21ae5cb1 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ Commands: status Show all changes in project files - upstream and... share Show project permissions share-add Add user to project permissions - share-del Remove user from project's collaborators + share-remove Remove user from project's collaborators ``` For example, to download a project: diff --git a/mergin/cli.py b/mergin/cli.py index 54525978..ae99c5e1 100755 --- a/mergin/cli.py +++ b/mergin/cli.py @@ -282,7 +282,6 @@ def share_add(ctx, project, usernames, permissions): if mc is None: return usernames = list(usernames) - print(usernames) mc.add_user_permissions_to_project(project, usernames, permissions) @@ -290,12 +289,12 @@ def share_add(ctx, project, usernames, permissions): @click.argument("project") @click.argument("usernames", nargs=-1) @click.pass_context -def share_del(ctx, project, usernames): +def share_remove(ctx, project, usernames): mc = ctx.obj["client"] if mc is None: return usernames = list(usernames) - mc.remove_user_permissions_to_project(project, usernames) + mc.remove_user_permissions_from_project(project, usernames) @cli.command() @@ -305,16 +304,16 @@ def share(ctx, project): mc = ctx.obj["client"] if mc is None: return - access_list = mc.share_list(project) - - owners = ", ".join(access_list.get("owners")) - writers = ", ".join(access_list.get("writers")) - readers = ", ".join(access_list.get("readers")) - - result = f"owners: {owners}\n" \ - f"writers: {writers}\n" \ - f"readers: {readers}" - click.echo(result) + access_list = mc.project_user_permissions(project) + + for username in access_list.get("owners"): + click.echo("{:20}\t{:20}".format(username, "owner")) + for username in access_list.get("writers"): + if username not in access_list.get("owners"): + click.echo("{:20}\t{:20}".format(username, "writer")) + for username in access_list.get("readers"): + if username not in access_list.get("writers"): + click.echo("{:20}\t{:20}".format(username, "reader")) @cli.command() diff --git a/mergin/client.py b/mergin/client.py index f79d29bc..8095047e 100644 --- a/mergin/client.py +++ b/mergin/client.py @@ -520,6 +520,15 @@ def set_project_access(self, project_path, access): raise ClientError(str(e), detail) def add_user_permissions_to_project(self, project_path, usernames, permission_level): + """ + Add specified permissions to specified users to project + :param project_path: project full name (/) + :param usernames: list of usernames to be granted specified permission level + :param permission_level: string (reader, writer, owner) + """ + if permission_level not in ["owner", "reader", "writer"]: + raise ClientError("Unsupported permission level") + project_info = self.project_info(project_path) access = project_info.get('access') for name in usernames: @@ -531,7 +540,12 @@ def add_user_permissions_to_project(self, project_path, usernames, permission_le access.get("readersnames").append(name) self.set_project_access(project_path, access) - def remove_user_permissions_to_project(self, project_path, usernames): + def remove_user_permissions_from_project(self, project_path, usernames): + """ + Removes specified users from project + :param project_path: project full name (/) + :param usernames: list of usernames to be granted specified permission level + """ project_info = self.project_info(project_path) access = project_info.get('access') for name in usernames: @@ -543,7 +557,14 @@ def remove_user_permissions_to_project(self, project_path, usernames): access.get("readersnames").remove(name) self.set_project_access(project_path, access) - def share_list(self, project_path): + def project_user_permissions(self, project_path): + """ + Returns permissions for project + :param project_path: project full name (/) + :return dict("owners": list(usernames), + "writers": list(usernames), + "readers": list(usernames)) + """ project_info = self.project_info(project_path) access = project_info.get('access') result = {} diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py index 521d8186..80c38252 100644 --- a/mergin/test/test_client.py +++ b/mergin/test/test_client.py @@ -960,19 +960,19 @@ def test_modify_project_permissions(mc): assert project_info['name'] == test_project assert project_info['namespace'] == API_USER - permissions = mc.share_list(project) + permissions = mc.project_user_permissions(project) assert permissions["owners"] == [API_USER] assert permissions["writers"] == [API_USER] assert permissions["readers"] == [API_USER] mc.add_user_permissions_to_project(project, [API_USER2], "writer") - permissions = mc.share_list(project) + permissions = mc.project_user_permissions(project) assert set(permissions["owners"]) == {API_USER} assert set(permissions["writers"]) == {API_USER, API_USER2} assert set(permissions["readers"]) == {API_USER, API_USER2} - mc.remove_user_permissions_to_project(project, [API_USER2]) - permissions = mc.share_list(project) + mc.remove_user_permissions_from_project(project, [API_USER2]) + permissions = mc.project_user_permissions(project) assert permissions["owners"] == [API_USER] assert permissions["writers"] == [API_USER] assert permissions["readers"] == [API_USER] From ef310f1847e70a45d116056094e4e69c9a8f816d Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 13 Oct 2021 14:56:10 +0200 Subject: [PATCH 4/5] added docs to cli commands --- README.md | 2 +- mergin/cli.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 21ae5cb1..acf72f95 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ Commands: status Show all changes in project files - upstream and... share Show project permissions share-add Add user to project permissions - share-remove Remove user from project's collaborators + share-remove Remove user from project's collaborators ``` For example, to download a project: diff --git a/mergin/cli.py b/mergin/cli.py index ae99c5e1..148f8c18 100755 --- a/mergin/cli.py +++ b/mergin/cli.py @@ -275,9 +275,10 @@ def download(ctx, project, directory, version): @cli.command() @click.argument("project") @click.argument("usernames", nargs=-1) -@click.option("--permissions") +@click.option("--permissions", help="permissions to be granted to project (reader, writer, owner)") @click.pass_context def share_add(ctx, project, usernames, permissions): + """Add permissions to [users] to project""" mc = ctx.obj["client"] if mc is None: return @@ -290,6 +291,7 @@ def share_add(ctx, project, usernames, permissions): @click.argument("usernames", nargs=-1) @click.pass_context def share_remove(ctx, project, usernames): + """Remove [users] permissions from project""" mc = ctx.obj["client"] if mc is None: return @@ -301,6 +303,7 @@ def share_remove(ctx, project, usernames): @click.argument("project") @click.pass_context def share(ctx, project): + """Fetch permissions to project""" mc = ctx.obj["client"] if mc is None: return From b8da4fa5b597bf17bb903d4bf2f53109ee26f1bf Mon Sep 17 00:00:00 2001 From: marcel Date: Wed, 13 Oct 2021 23:50:46 +0200 Subject: [PATCH 5/5] multiple exceptions block fix --- mergin/merginproject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mergin/merginproject.py b/mergin/merginproject.py index 8003f991..ce6d741e 100644 --- a/mergin/merginproject.py +++ b/mergin/merginproject.py @@ -21,7 +21,7 @@ # python paths. try: from .deps import pygeodiff -except ImportError or ModuleNotFoundError: +except (ImportError, ModuleNotFoundError): import pygeodiff