From 5533363cae5d39228c76ed485938d250f49c2ec4 Mon Sep 17 00:00:00 2001 From: Jakub Kadlcik Date: Thu, 4 Jan 2024 12:03:52 +0100 Subject: [PATCH] frontend, python: allow ignoring errors that a project already exists Fix #2140 --- frontend/coprs_frontend/coprs/forms.py | 14 ++++++++--- .../coprs/views/apiv3_ns/apiv3_projects.py | 24 +++++++++++++++---- .../coprs/views/apiv3_ns/schema/fields.py | 8 +++++++ .../coprs/views/apiv3_ns/schema/schemas.py | 5 ++-- .../tests/test_apiv3/test_projects.py | 22 +++++++++++++++++ python/copr/v3/proxies/project.py | 3 ++- 6 files changed, 65 insertions(+), 11 deletions(-) diff --git a/frontend/coprs_frontend/coprs/forms.py b/frontend/coprs_frontend/coprs/forms.py index 81851efed..fa0567632 100644 --- a/frontend/coprs_frontend/coprs/forms.py +++ b/frontend/coprs_frontend/coprs/forms.py @@ -262,7 +262,7 @@ def __call__(self, form, field): class CoprUniqueNameValidator(object): - def __init__(self, message=None, user=None, group=None): + def __init__(self, message=None, user=None, group=None, exist_ok=False): if not message: if group is None: message = "You already have a project named '{}'." @@ -273,6 +273,8 @@ def __init__(self, message=None, user=None, group=None): user = flask.g.user self.user = user self.group = group + self.exist_ok = exist_ok + self.copr = None def __call__(self, form, field): if self.group: @@ -282,6 +284,11 @@ def __call__(self, form, field): existing = CoprsLogic.exists_for_user( self.user, field.data).first() + # Save the existing copr instance, so we can later return it without + # querying the database again + if existing and self.exist_ok: + self.copr = existing + if existing and str(existing.id) != form.id.data: raise wtforms.ValidationError(self.message.format(field.data)) @@ -704,7 +711,7 @@ def errors(self): class CoprFormFactory(object): @staticmethod - def create_form_cls(user=None, group=None, copr=None): + def create_form_cls(user=None, group=None, copr=None, exist_ok=False): class F(CoprForm): # also use id here, to be able to find out whether user # is updating a copr if so, we don't want to shout @@ -717,7 +724,8 @@ class F(CoprForm): validators=[ wtforms.validators.DataRequired(), NameCharactersValidator(), - CoprUniqueNameValidator(user=user, group=group), + CoprUniqueNameValidator(user=user, group=group, + exist_ok=exist_ok), NameNotNumberValidator() ]) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py index 955355a6e..e1949f5e9 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_projects.py @@ -26,7 +26,7 @@ project_delete_input_model, fullname_params, pagination_project_model, - ownername_params, + project_params, pagination_params, ) from coprs.views.apiv3_ns.schema.docs import query_docs @@ -139,7 +139,7 @@ def get(self, ownername, projectname): class ProjectList(Resource): @restx_pagination @query_to_parameters - @apiv3_projects_ns.doc(params=ownername_params | pagination_params) + @apiv3_projects_ns.doc(params=project_params | pagination_params) @apiv3_projects_ns.marshal_list_with(pagination_project_model) @apiv3_projects_ns.response( HTTPStatus.PARTIAL_CONTENT.value, HTTPStatus.PARTIAL_CONTENT.description @@ -184,25 +184,39 @@ def get(self, query, **kwargs): @apiv3_projects_ns.route("/add/") class ProjectAdd(Resource): @restx_api_login_required - @apiv3_projects_ns.doc(params=ownername_params) + @query_to_parameters + @apiv3_projects_ns.doc(params=project_params) @apiv3_projects_ns.marshal_with(project_model) @apiv3_projects_ns.expect(project_add_input_model) @apiv3_projects_ns.response(HTTPStatus.OK.value, "Copr project created") @apiv3_projects_ns.response( HTTPStatus.BAD_REQUEST.value, HTTPStatus.BAD_REQUEST.description ) - def post(self, ownername): + def post(self, ownername, exist_ok=False): """ Create new Copr project Create new Copr project for ownername with specified data inserted in form. """ + exist_ok = flask.request.args.get("exist_ok") == "True" user, group = owner2tuple(ownername) data = rename_fields(get_form_compatible_data(preserve=["chroots"])) - form_class = forms.CoprFormFactory.create_form_cls(user=user, group=group) + form_class = forms.CoprFormFactory.create_form_cls(user=user, group=group, + exist_ok=exist_ok) set_defaults(data, form_class) form = form_class(data, meta={"csrf": False}) if not form.validate_on_submit(): + if exist_ok: + # This is an ugly hack to avoid additional database query. + # If a project with this owner and name already exists, the + # `CoprUniqueNameValidator` saved its instance. Let's find the + # validator and return the existing copr instance. + for validator in form.name.validators: + if not isinstance(validator, forms.CoprUniqueNameValidator): + continue + if not validator.copr: + continue + return to_dict(validator.copr) raise InvalidForm(form) validate_chroots(get_input_dict(), MockChrootsLogic.get_multiple()) diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/fields.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/fields.py index cd243cd10..30fe8f08d 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/fields.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/fields.py @@ -406,6 +406,14 @@ ) ) +exist_ok = Boolean( + description=( + "Don't fail if a project with this owner and name already exist, " + "return the existing instance instead. Please be aware that the " + "project attributes are not updated in such case." + ) +) + # TODO: these needs description chroot_repos = Raw() diff --git a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py index be43ee4da..dac227707 100644 --- a/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py +++ b/frontend/coprs_frontend/coprs/views/apiv3_ns/schema/schemas.py @@ -481,8 +481,9 @@ class FullnameSchema(ParamsSchema): @dataclass -class OwnernameSchema(ParamsSchema): +class ProjectParamsSchema(ParamsSchema): ownername: String + exist_ok: Boolean # OUTPUT MODELS @@ -515,5 +516,5 @@ class OwnernameSchema(ParamsSchema): package_get_params = PackageGet.get_cls().params_schema() project_chroot_get_params = ProjectChrootGet.get_cls().params_schema() fullname_params = FullnameSchema.get_cls().params_schema() -ownername_params = OwnernameSchema.get_cls().params_schema() +project_params = ProjectParamsSchema.get_cls().params_schema() pagination_params = PaginationMeta.get_cls().params_schema() diff --git a/frontend/coprs_frontend/tests/test_apiv3/test_projects.py b/frontend/coprs_frontend/tests/test_apiv3/test_projects.py index 2a8a9cd6c..bfd932aa1 100644 --- a/frontend/coprs_frontend/tests/test_apiv3/test_projects.py +++ b/frontend/coprs_frontend/tests/test_apiv3/test_projects.py @@ -426,3 +426,25 @@ def test_perms_set_sends_emails_2(self, send_mail): r = self.auth_post('/request/user2/barcopr', permissions, u) assert r.status_code == 200 assert len(calls) == 2 + + @TransactionDecorator("u1") + @pytest.mark.usefixtures("f_users", "f_users_api", "f_mock_chroots", "f_db") + def test_add_exist_ok(self): + route = "/api_3/project/add/{}".format(self.transaction_username) + data = {"name": "foo", "chroots": ["fedora-rawhide-i386"]} + + # There is no conflict, we can obviously create the project + response = self.api3.post(route, data) + assert response.status_code == 200 + + # The project already exists + response = self.api3.post(route, data) + assert response.status_code == 400 + assert "already have a project" in json.loads(response.data)["error"] + + # When using exist_ok, the request is successful, and existing project + # is returned + route += "?exist_ok=True" + response = self.api3.post(route, data) + assert response.status_code == 200 + assert json.loads(response.data)["full_name"] == "user1/foo" diff --git a/python/copr/v3/proxies/project.py b/python/copr/v3/proxies/project.py index 89b062ea2..ece6d13cc 100644 --- a/python/copr/v3/proxies/project.py +++ b/python/copr/v3/proxies/project.py @@ -72,7 +72,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No delete_after_days=None, multilib=False, module_hotfixes=False, bootstrap=None, bootstrap_image=None, isolation=None, follow_fedora_branching=True, fedora_review=None, appstream=False, runtime_dependencies=None, packit_forge_projects_allowed=None, - repo_priority=None): + repo_priority=None, exist_ok=False): """ Create a project @@ -115,6 +115,7 @@ def add(self, ownername, projectname, chroots, description=None, instructions=No endpoint = "/project/add/{ownername}" params = { "ownername": ownername, + "exist_ok": exist_ok, } data = { "name": projectname,