From 24600625834156b0a977a433555e12cd84b6d831 Mon Sep 17 00:00:00 2001 From: Herman Snevajs Date: Thu, 19 Oct 2023 13:23:03 +0200 Subject: [PATCH 1/3] Add endpoint to rename a project Task: https://github.com/MerginMaps/MerginMaps-Cloud-TEST/issues/1 URL: PATCH /v2/projects/ Body: { "name": "new_project_name" } - check the user have permission to update the project - check the new name uses allowed characters - check if a project with the new name already exists --- server/mergin/sync/errors.py | 18 +++++++ server/mergin/sync/public_api.yaml | 2 +- server/mergin/sync/public_api_v2.yaml | 50 +++++++++++++++++++ .../mergin/sync/public_api_v2_controller.py | 36 +++++++++++-- server/mergin/tests/test_public_api_v2.py | 33 +++++++++++- 5 files changed, 134 insertions(+), 5 deletions(-) diff --git a/server/mergin/sync/errors.py b/server/mergin/sync/errors.py index fa22d858..5ae19604 100644 --- a/server/mergin/sync/errors.py +++ b/server/mergin/sync/errors.py @@ -34,3 +34,21 @@ def to_dict(self) -> Dict: data["current_usage"] = self.current_usage data["storage_limit"] = self.storage_limit return data + + +class InvalidProjectName(ResponseError): + code = "InvalidProjectName" + detail = "Entered project name is invalid, don't start project name with . and use only alphanumeric or these -._!" + + def to_dict(self) -> Dict: + data = super().to_dict() + return data + + +class ProjectNameAlreadyExists(ResponseError): + code = "ProjectNameAlreadyExists" + detail = "Entered project name already exists" + + def to_dict(self) -> Dict: + data = super().to_dict() + return data diff --git a/server/mergin/sync/public_api.yaml b/server/mergin/sync/public_api.yaml index 21a5a8b0..50c20c10 100644 --- a/server/mergin/sync/public_api.yaml +++ b/server/mergin/sync/public_api.yaml @@ -977,7 +977,7 @@ components: UnsupportedMediaType: description: Payload format is in an unsupported format. ConflictResp: - description: Request could not be processed becuase of conflict in resources + description: Request could not be processed because of conflict in resources UnprocessableEntity: description: Request was correct and yet server could not process it ProjectsLimitHitResp: diff --git a/server/mergin/sync/public_api_v2.yaml b/server/mergin/sync/public_api_v2.yaml index 0eae3ce7..8875286a 100644 --- a/server/mergin/sync/public_api_v2.yaml +++ b/server/mergin/sync/public_api_v2.yaml @@ -27,6 +27,31 @@ paths: "404": $ref: "#/components/responses/NotFound" x-openapi-router-controller: mergin.sync.public_api_v2_controller + patch: + tags: + - project + summary: Rename project + operationId: rename_project + parameters: + - $ref: "#/components/parameters/ProjectId" + responses: + "200": + description: OK + "400": + description: Request could not be processed by server + content: + application/json: + schema: + anyOf: + - $ref: "#/components/schemas/InvalidProjectName" + - $ref: "#/components/responses/BadRequest" + "403": + $ref: "#/components/responses/Unauthorized" + "404": + $ref: "#/components/responses/NotFound" + "409": + $ref: "#/components/schemas/ProjectNameAlreadyExists" + x-openapi-router-controller: mergin.sync.public_api_v2_controller /projects/{id}/scheduleDelete: post: tags: @@ -65,3 +90,28 @@ components: type: string format: uuid pattern: \b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b + schemas: + CustomError: + type: object + properties: + code: + type: string + detail: + type: string + required: + - code + - detail + InvalidProjectName: + allOf: + - $ref: '#/components/schemas/CustomError' + type: object + example: + code: InvalidProjectName + detail: Entered project name is invalid, don't start project name with . and use only alphanumeric or these -._! (InvalidProjectName) + ProjectNameAlreadyExists: + allOf: + - $ref: '#/components/schemas/CustomError' + type: object + example: + code: ProjectNameAlreadyExists + detail: Entered project name already exists (ProjectNameAlreadyExists) diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 78ddadc4..db650e34 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -4,15 +4,18 @@ from datetime import datetime -from connexion import NoContent -from flask import abort +from connexion import NoContent, request +from flask import abort, jsonify, make_response, current_app from flask_login import current_user from mergin import db from mergin.auth import auth_required -from mergin.sync.models import Project from mergin.sync.permissions import ProjectPermissions, require_project_by_uuid +from .errors import InvalidProjectName, ProjectNameAlreadyExists +from .models import Project +from .utils import is_name_allowed + @auth_required def schedule_delete_project(id_): @@ -43,3 +46,30 @@ def delete_project_now(id_): db.session.commit() return NoContent, 204 + + +@auth_required +def rename_project(id_): + """Rename project + + :param id_: Project_id + :type id_: str + + :rtype: None + """ + project = require_project_by_uuid(id_, ProjectPermissions.Update) + new_name = request.json["name"].strip() + + if not is_name_allowed(new_name): + abort(make_response(jsonify(InvalidProjectName().to_dict()), 400)) + + new_name_exists = Project.query.filter_by( + workspace_id=project.workspace_id, name=new_name + ).first() + if new_name_exists: + abort(make_response(jsonify(ProjectNameAlreadyExists().to_dict()), 409)) + + project.name = new_name + db.session.commit() + + return "", 200 diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 2213ced4..09054a86 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -1,9 +1,10 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import json from mergin.sync.models import Project -from tests import test_project, test_workspace_id +from tests import test_project, test_workspace_id, json_headers def test_schedule_delete_project(client): @@ -37,3 +38,33 @@ def test_delete_after_schedule(client): assert response.status_code == 204 response = client.delete(f"v2/projects/{project.id}") assert response.status_code == 204 + + +def test_rename_project(client): + project = Project.query.filter_by( + workspace_id=test_workspace_id, name=test_project + ).first() + data = {"name": " new_project_name "} + response = client.patch( + f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers + ) + assert response.status_code == 200 + assert project.name == "new_project_name" + # repeat - the new name is already occupied + response = client.patch( + f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers + ) + assert response.status_code == 409 + assert response.json["code"] == "ProjectNameAlreadyExists" + assert "Entered project name already exists" in response.json["detail"] + # illegal project name + data = {"name": ".new_name"} + response = client.patch( + f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers + ) + assert response.status_code == 400 + assert response.json["code"] == "InvalidProjectName" + assert ( + "Entered project name is invalid, don't start project name with . " + "and use only alphanumeric or these -._!" + ) in response.json["detail"] From ddb915661324b835bcc80a6729067a486283aea5 Mon Sep 17 00:00:00 2001 From: Marek Spano <2093904+luxusko@users.noreply.github.com> Date: Tue, 24 Oct 2023 10:48:30 +0200 Subject: [PATCH 2/3] Update project v2 endpoint review --- server/mergin/sync/errors.py | 18 ---- server/mergin/sync/public_api_v2.yaml | 91 ++++++++++--------- .../mergin/sync/public_api_v2_controller.py | 28 +++--- server/mergin/tests/test_public_api_v2.py | 31 ++----- 4 files changed, 73 insertions(+), 95 deletions(-) diff --git a/server/mergin/sync/errors.py b/server/mergin/sync/errors.py index 5ae19604..fa22d858 100644 --- a/server/mergin/sync/errors.py +++ b/server/mergin/sync/errors.py @@ -34,21 +34,3 @@ def to_dict(self) -> Dict: data["current_usage"] = self.current_usage data["storage_limit"] = self.storage_limit return data - - -class InvalidProjectName(ResponseError): - code = "InvalidProjectName" - detail = "Entered project name is invalid, don't start project name with . and use only alphanumeric or these -._!" - - def to_dict(self) -> Dict: - data = super().to_dict() - return data - - -class ProjectNameAlreadyExists(ResponseError): - code = "ProjectNameAlreadyExists" - detail = "Entered project name already exists" - - def to_dict(self) -> Dict: - data = super().to_dict() - return data diff --git a/server/mergin/sync/public_api_v2.yaml b/server/mergin/sync/public_api_v2.yaml index 8875286a..4973696a 100644 --- a/server/mergin/sync/public_api_v2.yaml +++ b/server/mergin/sync/public_api_v2.yaml @@ -10,47 +10,73 @@ tags: description: Mergin project paths: /projects/{id}: + parameters: + - $ref: "#/components/parameters/ProjectId" delete: tags: - project summary: Delete project immediately operationId: delete_project_now - parameters: - - $ref: "#/components/parameters/ProjectId" responses: "204": $ref: "#/components/responses/NoContent" "400": $ref: "#/components/responses/BadRequest" - "403": + "401": $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" "404": $ref: "#/components/responses/NotFound" x-openapi-router-controller: mergin.sync.public_api_v2_controller patch: tags: - project - summary: Rename project - operationId: rename_project - parameters: - - $ref: "#/components/parameters/ProjectId" + summary: Update project + operationId: update_project + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - name + properties: + name: + type: string + example: survey responses: - "200": - description: OK + "204": + $ref: "#/components/responses/NoContent" "400": - description: Request could not be processed by server + description: Invalid project name content: application/json: schema: - anyOf: - - $ref: "#/components/schemas/InvalidProjectName" - - $ref: "#/components/responses/BadRequest" - "403": + type: object + required: + - code + - detail + properties: + code: + type: string + enum: + - InvalidProjectName + example: InvalidProjectName + detail: + type: string + enum: + - "Entered project name is invalid" + example: "Entered project name is invalid" + "401": $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" "404": $ref: "#/components/responses/NotFound" "409": - $ref: "#/components/schemas/ProjectNameAlreadyExists" + $ref: "#/components/responses/Conflict" x-openapi-router-controller: mergin.sync.public_api_v2_controller /projects/{id}/scheduleDelete: post: @@ -65,8 +91,10 @@ paths: $ref: "#/components/responses/NoContent" "400": $ref: "#/components/responses/BadRequest" - "403": + "401": $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" "404": $ref: "#/components/responses/NotFound" x-openapi-router-controller: mergin.sync.public_api_v2_controller @@ -75,11 +103,15 @@ components: NoContent: description: No content BadRequest: - description: Invalid request. + description: Invalid request Unauthorized: description: Authentication information is missing or invalid + Forbidden: + description: Invalid token NotFound: description: Not found + Conflict: + description: Conflict parameters: ProjectId: name: id @@ -90,28 +122,3 @@ components: type: string format: uuid pattern: \b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b - schemas: - CustomError: - type: object - properties: - code: - type: string - detail: - type: string - required: - - code - - detail - InvalidProjectName: - allOf: - - $ref: '#/components/schemas/CustomError' - type: object - example: - code: InvalidProjectName - detail: Entered project name is invalid, don't start project name with . and use only alphanumeric or these -._! (InvalidProjectName) - ProjectNameAlreadyExists: - allOf: - - $ref: '#/components/schemas/CustomError' - type: object - example: - code: ProjectNameAlreadyExists - detail: Entered project name already exists (ProjectNameAlreadyExists) diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index db650e34..5f16138f 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -5,16 +5,14 @@ from datetime import datetime from connexion import NoContent, request -from flask import abort, jsonify, make_response, current_app +from flask import abort, jsonify from flask_login import current_user from mergin import db from mergin.auth import auth_required +from mergin.sync.models import Project from mergin.sync.permissions import ProjectPermissions, require_project_by_uuid - -from .errors import InvalidProjectName, ProjectNameAlreadyExists -from .models import Project -from .utils import is_name_allowed +from mergin.sync.utils import is_name_allowed @auth_required @@ -49,27 +47,29 @@ def delete_project_now(id_): @auth_required -def rename_project(id_): +def update_project(id_): """Rename project :param id_: Project_id :type id_: str - - :rtype: None """ project = require_project_by_uuid(id_, ProjectPermissions.Update) - new_name = request.json["name"].strip() + new_name = request.json["name"] if not is_name_allowed(new_name): - abort(make_response(jsonify(InvalidProjectName().to_dict()), 400)) - + return ( + jsonify( + code="InvalidProjectName", detail="Entered project name is invalid" + ), + 400, + ) new_name_exists = Project.query.filter_by( workspace_id=project.workspace_id, name=new_name ).first() if new_name_exists: - abort(make_response(jsonify(ProjectNameAlreadyExists().to_dict()), 409)) + abort(409, "Name already exist within workspace") - project.name = new_name + project.name = new_name.strip() db.session.commit() - return "", 200 + return NoContent, 204 diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 09054a86..3bf70b8f 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -1,10 +1,9 @@ # Copyright (C) Lutra Consulting Limited # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import json from mergin.sync.models import Project -from tests import test_project, test_workspace_id, json_headers +from tests import test_project, test_workspace_id def test_schedule_delete_project(client): @@ -44,27 +43,17 @@ def test_rename_project(client): project = Project.query.filter_by( workspace_id=test_workspace_id, name=test_project ).first() - data = {"name": " new_project_name "} - response = client.patch( - f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers - ) - assert response.status_code == 200 + data = {"name": "new_project_name"} + response = client.patch(f"v2/projects/{project.id}", json=data) + assert response.status_code == 204 assert project.name == "new_project_name" - # repeat - the new name is already occupied - response = client.patch( - f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers - ) + # name already exists + response = client.patch(f"v2/projects/{project.id}", json=data) assert response.status_code == 409 - assert response.json["code"] == "ProjectNameAlreadyExists" - assert "Entered project name already exists" in response.json["detail"] - # illegal project name - data = {"name": ".new_name"} + # invalid project name + response = client.patch(f"v2/projects/{project.id}", json={"name": ".new_name"}) + assert response.status_code == 400 response = client.patch( - f"v2/projects/{project.id}", data=json.dumps(data), headers=json_headers + f"v2/projects/{project.id}", json={"name": " new_project_name"} ) assert response.status_code == 400 - assert response.json["code"] == "InvalidProjectName" - assert ( - "Entered project name is invalid, don't start project name with . " - "and use only alphanumeric or these -._!" - ) in response.json["detail"] From 83e0ac15d29b63eb6db9924a31268d537df66840 Mon Sep 17 00:00:00 2001 From: Marek Spano <2093904+luxusko@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:38:17 +0200 Subject: [PATCH 3/3] Integration test for update project v2 --- server/mergin/tests/test_auth.py | 24 ++++++++++++++++++++++- server/mergin/tests/test_public_api_v2.py | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/mergin/tests/test_auth.py b/server/mergin/tests/test_auth.py index 3101b225..2eb3621a 100644 --- a/server/mergin/tests/test_auth.py +++ b/server/mergin/tests/test_auth.py @@ -14,7 +14,13 @@ from ..auth.tasks import prune_removed_users from .. import db from ..sync.models import Project -from . import test_workspace_id, json_headers, DEFAULT_USER, test_workspace_name +from . import ( + test_workspace_id, + json_headers, + DEFAULT_USER, + test_workspace_name, + test_project, +) from .utils import add_user, login_as_admin, login @@ -718,3 +724,19 @@ def test_admin_login(client, data, expected): add_user("user", "user") resp = client.post("/app/admin/login", data=json.dumps(data), headers=json_headers) assert resp.status_code == expected + + +def test_update_project_v2(client): + project = Project.query.filter_by( + workspace_id=test_workspace_id, name=test_project + ).first() + data = {"name": "new_project_name"} + resp = client.patch(f"v2/projects/{project.id}", json=data) + assert resp.status_code == 401 + user = add_user("test", "test") + login(client, "test", "test") + resp = client.patch(f"v2/projects/{project.id}", json=data) + assert resp.status_code == 403 + login_as_admin(client) + resp = client.patch(f"v2/projects/{project.id}", json=data) + assert resp.status_code == 204 diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 3bf70b8f..6694ea5e 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -53,7 +53,9 @@ def test_rename_project(client): # invalid project name response = client.patch(f"v2/projects/{project.id}", json={"name": ".new_name"}) assert response.status_code == 400 + assert response.json["code"] == "InvalidProjectName" response = client.patch( f"v2/projects/{project.id}", json={"name": " new_project_name"} ) assert response.status_code == 400 + assert response.json["code"] == "InvalidProjectName"