From d7ccf1925c528380735af5e347bcb5c01e08935a Mon Sep 17 00:00:00 2001 From: gersona Date: Sat, 16 Nov 2024 21:06:10 +0300 Subject: [PATCH 01/12] additioal test for machinery settings API --- weblate/api/tests.py | 111 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/weblate/api/tests.py b/weblate/api/tests.py index 197cd42e8704..fe6134ebdc1d 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -7,6 +7,7 @@ from datetime import UTC, datetime, timedelta from io import BytesIO +import responses from django.core.files import File from django.urls import reverse from rest_framework.exceptions import ErrorDetail @@ -2010,6 +2011,116 @@ def test_credits(self) -> None: ) self.assertEqual(response.data, []) + @responses.activate + def test_install_machinery(self) -> None: + from weblate.machinery.tests import DeepLTranslationTest + + # unauthenticated + self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=403, + authenticated=True, + superuser=False, + request={"service": "weblate"}, + ) + + # missing service + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=400, + superuser=True, + request={}, + ) + + # unknown service + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=400, + superuser=True, + request={"service": "unknown"}, + ) + + # no form + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=200, + superuser=True, + request={"service": "weblate"}, + ) + + # missing required field + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=400, + superuser=True, + request={ + "service": "deepl", + "configuration": '{"wrong": ""}', + }, + ) + + # invalid field + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=400, + superuser=True, + request={ + "service": "deepl", + "configuration": "{malformed_json", + }, + ) + + # valid form + DeepLTranslationTest.mock_response() + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=200, + superuser=True, + request={ + "service": "deepl", + "configuration": '{"key": "x", "url": "https://api.deepl.com/v2/"}', + }, + ) + + # update configuration + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=200, + superuser=True, + request={ + "service": "deepl", + "configuration": '{"key": "xy", "url": "https://api.deepl.com/v2/"}', + "update": "true", + }, + ) + + # test list configurations + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="get", + code=200, + superuser=True, + ) + self.assertIn("weblate", response.data) + self.assertEqual(response.data["deepl"]["key"], "xy") + class ComponentAPITest(APIBaseTest): def setUp(self) -> None: From 7812e901a6a2e929b5e4093dafffc9a9d426980b Mon Sep 17 00:00:00 2001 From: gersona Date: Sat, 16 Nov 2024 21:08:11 +0300 Subject: [PATCH 02/12] refactor service configuration out to machinery.models --- .../management/commands/install_machinery.py | 35 ++++++------------- weblate/machinery/models.py | 30 ++++++++++++++++ 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/weblate/machinery/management/commands/install_machinery.py b/weblate/machinery/management/commands/install_machinery.py index cb0594d23590..a6387a1d290e 100644 --- a/weblate/machinery/management/commands/install_machinery.py +++ b/weblate/machinery/management/commands/install_machinery.py @@ -2,12 +2,10 @@ # # SPDX-License-Identifier: GPL-3.0-or-later -import json - from django.core.management.base import CommandError from weblate.configuration.models import Setting, SettingCategory -from weblate.machinery.models import MACHINERY +from weblate.machinery.models import validate_service_configuration from weblate.utils.management.base import BaseCommand @@ -26,31 +24,20 @@ def add_arguments(self, parser) -> None: help="Update existing service configuration", ) - def validate_form(self, form) -> None: - if not form.is_valid(): - for error in form.non_field_errors(): + def handle(self, *args, **options) -> None: + try: + service, configuration, errors = validate_service_configuration( + options["service"], options["configuration"] + ) + except Exception as e: + raise CommandError(str(e)) from e + + if errors: + for error in errors: self.stderr.write(error) - for field in form: - for error in field.errors: - self.stderr.write(f"Error in {field.name}: {error}") msg = "Invalid add-on configuration!" raise CommandError(msg) - def handle(self, *args, **options) -> None: - try: - service = MACHINERY[options["service"]] - except KeyError as error: - msg = "Service not found: {}".format(options["service"]) - raise CommandError(msg) from error - try: - configuration = json.loads(options["configuration"]) - except ValueError as error: - msg = f"Invalid service configuration: {error}" - raise CommandError(msg) from error - if service.settings_form is not None: - form = service.settings_form(service, data=configuration) - self.validate_form(form) - setting, created = Setting.objects.get_or_create( category=SettingCategory.MT, name=options["service"], diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index ede738232703..77742d0790a4 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -2,6 +2,8 @@ # # SPDX-License-Identifier: GPL-3.0-or-later +import json + from appconf import AppConf from weblate.utils.classloader import ClassLoader @@ -50,3 +52,31 @@ class WeblateConf(AppConf): class Meta: prefix = "" + + +def validate_service_configuration( + service_name: str, configuration_json: str +) -> tuple[BatchMachineTranslation, dict, list[str]]: + try: + service = MACHINERY[service_name] + except KeyError as error: + msg = f"Service not found: {service_name}" + raise ValueError(msg) from error + try: + configuration = json.loads(configuration_json) + except ValueError as error: + msg = f"Invalid service configuration: {error}" + raise ValueError(msg) from error + + errors = [] + if service.settings_form is not None: + form = service.settings_form(service, data=configuration) + # validate form + if not form.is_valid(): + errors.extend(list(form.non_field_errors())) + + for field in form: + errors.extend( + [f"Error in {field.name}: {error}" for error in field.errors] + ) + return service, configuration, errors From 37ee3614a5e4b3363970bec97e9746a3f3f6b34a Mon Sep 17 00:00:00 2001 From: gersona Date: Sat, 16 Nov 2024 21:09:16 +0300 Subject: [PATCH 03/12] new machinery_settings endpoint for Project API --- weblate/api/serializers.py | 4 ++++ weblate/api/views.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/weblate/api/serializers.py b/weblate/api/serializers.py index 2227d2b24b1f..ab118b7760a6 100644 --- a/weblate/api/serializers.py +++ b/weblate/api/serializers.py @@ -397,6 +397,9 @@ class ProjectSerializer(serializers.ModelSerializer[Project]): credits_url = serializers.HyperlinkedIdentityField( view_name="api:project-credits", lookup_field="slug" ) + machinery_settings = serializers.HyperlinkedIdentityField( + view_name="api:machinery-settings", lookup_field="slug" + ) class Meta: model = Project @@ -422,6 +425,7 @@ class Meta: "enable_hooks", "language_aliases", "enforced_2fa", + "machinery_settings", ) extra_kwargs = { "url": {"view_name": "api:project-detail", "lookup_field": "slug"} diff --git a/weblate/api/views.py b/weblate/api/views.py index d5c545825e53..9e3065592625 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -37,6 +37,7 @@ HTTP_200_OK, HTTP_201_CREATED, HTTP_204_NO_CONTENT, + HTTP_400_BAD_REQUEST, HTTP_423_LOCKED, HTTP_500_INTERNAL_SERVER_ERROR, ) @@ -85,6 +86,7 @@ from weblate.auth.models import AuthenticatedHttpRequest, Group, Role, User from weblate.formats.models import EXPORTERS from weblate.lang.models import Language +from weblate.machinery.models import validate_service_configuration from weblate.memory.models import Memory from weblate.screenshots.models import Screenshot from weblate.trans.exceptions import FileParseError @@ -1010,6 +1012,39 @@ def file(self, request: Request, **kwargs): name=instance.slug, ) + @action(detail=True, methods=["get", "post"]) + def machinery_settings(self, request: Request, **kwargs): + project = self.get_object() + # if get, just list all machinery configurations + if request.method == "POST": + if not request.user.has_perm("project.edit", project): + self.permission_denied(request, "Can not edit machinery configuration") + + try: + service_name = request.data["service"] + except KeyError: + return Response( + {"errors": ["Missing service name"]}, status=HTTP_400_BAD_REQUEST + ) + + try: + service, configuration, errors = validate_service_configuration( + service_name, request.data.get("configuration", "{}") + ) + except Exception as error: + errors = [str(error)] + + if errors: + return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST) + + project.machinery_settings[service_name] = configuration + project.save(update_fields=["machinery_settings"]) + return Response( + {"message": f"Service installed: {service.name}"}, status=HTTP_200_OK + ) + + return Response(project.machinery_settings, status=HTTP_200_OK) + class ComponentViewSet( MultipleFieldViewSet, UpdateModelMixin, DestroyModelMixin, CreditsMixin From 286c7ca882da8fbfe1db64303c98fc66c998abff Mon Sep 17 00:00:00 2001 From: gersona Date: Mon, 18 Nov 2024 08:54:02 +0300 Subject: [PATCH 04/12] docstrings and documentation added --- docs/admin/machine.rst | 2 ++ docs/api.rst | 24 ++++++++++++++++++++++++ docs/changes.rst | 4 ++++ weblate/api/tests.py | 3 ++- weblate/api/views.py | 2 +- weblate/machinery/models.py | 13 +++++++++++-- 6 files changed, 44 insertions(+), 4 deletions(-) diff --git a/docs/admin/machine.rst b/docs/admin/machine.rst index 9b8f816f50df..501688376bf0 100644 --- a/docs/admin/machine.rst +++ b/docs/admin/machine.rst @@ -22,6 +22,8 @@ the project settings: The services translate from the source language as configured at :ref:`component`, see :ref:`component-source_language`. +Per-project automatic suggestion can also be configured via the :ref:`api`. + .. seealso:: :ref:`machine-translation` diff --git a/docs/api.rst b/docs/api.rst index 1a5c8301db45..ec1e1c6a1150 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -1235,6 +1235,30 @@ Projects :>json string full_name: Full name of the contributor :>json string change_count: Number of changes done in the time range + +.. http:get:: /api/projects/{string:project}/machinery_settings/ + + .. versionadded:: 5.9 + + Returns automatic suggestion settings for a project, consisting of the configurations defined for each translation service installed. + + :param project: Project URL slug + :type project: string + :>json object suggestion_settings: Configuration for all installed services. + + +.. http:post:: /api/projects/{string:project}/machinery_settings/ + + .. versionadded:: 5.9 + + Create or update the service configuration for a project. + + :param project: Project URL slug + :type project: string + :form string service: Service name + :form string configuration: Service configuration in JSON + + Components ++++++++++ diff --git a/docs/changes.rst b/docs/changes.rst index a74406266771..3e6732ded2e5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,10 @@ Weblate 5.9 Not yet released. **New features** +* Per-project :ref:`machine-translation-setup` can now be configured via the Project :ref:`api`. + + * Added :http:get:`/api/projects/{string:project}/machinery_settings/`. + * Added :http:post:`/api/projects/{string:project}/machinery_settings/`. **Improvements** diff --git a/weblate/api/tests.py b/weblate/api/tests.py index fe6134ebdc1d..ac1c2694069a 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -2013,6 +2013,7 @@ def test_credits(self) -> None: @responses.activate def test_install_machinery(self) -> None: + """Test the machinery settings API endpoint for various scenarios.""" from weblate.machinery.tests import DeepLTranslationTest # unauthenticated @@ -2110,7 +2111,7 @@ def test_install_machinery(self) -> None: }, ) - # test list configurations + # list configurations response = self.do_request( "api:project-machinery-settings", self.project_kwargs, diff --git a/weblate/api/views.py b/weblate/api/views.py index 9e3065592625..a6259624f785 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -1014,8 +1014,8 @@ def file(self, request: Request, **kwargs): @action(detail=True, methods=["get", "post"]) def machinery_settings(self, request: Request, **kwargs): + """List or create/update machinery configuration for a project.""" project = self.get_object() - # if get, just list all machinery configurations if request.method == "POST": if not request.user.has_perm("project.edit", project): self.permission_denied(request, "Can not edit machinery configuration") diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index 77742d0790a4..c48748991aed 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -55,15 +55,24 @@ class Meta: def validate_service_configuration( - service_name: str, configuration_json: str + service_name: str, configuration: str ) -> tuple[BatchMachineTranslation, dict, list[str]]: + """ + Validate given service configuration. + + :param service_name: Name of the service as defined in WEBLATE_MACHINERY + :param configuration: JSON encoded configuration for the service + :return: A tuple containing the validated service class, configuration + and a list of errors + :raises ValueError: When service is not found or configuration is invalid + """ try: service = MACHINERY[service_name] except KeyError as error: msg = f"Service not found: {service_name}" raise ValueError(msg) from error try: - configuration = json.loads(configuration_json) + configuration = json.loads(configuration) except ValueError as error: msg = f"Invalid service configuration: {error}" raise ValueError(msg) from error From 748ef0662f17c348f6dd45a8c1df7de372d60e09 Mon Sep 17 00:00:00 2001 From: gersona Date: Mon, 18 Nov 2024 09:29:38 +0300 Subject: [PATCH 05/12] refactor validate_service_configuration to accept configuration as dict --- weblate/api/serializers.py | 2 +- weblate/api/tests.py | 10 ++++++---- weblate/machinery/models.py | 20 ++++++++++++-------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/weblate/api/serializers.py b/weblate/api/serializers.py index ab118b7760a6..64152968ca33 100644 --- a/weblate/api/serializers.py +++ b/weblate/api/serializers.py @@ -398,7 +398,7 @@ class ProjectSerializer(serializers.ModelSerializer[Project]): view_name="api:project-credits", lookup_field="slug" ) machinery_settings = serializers.HyperlinkedIdentityField( - view_name="api:machinery-settings", lookup_field="slug" + view_name="api:project-machinery-settings", lookup_field="slug" ) class Meta: diff --git a/weblate/api/tests.py b/weblate/api/tests.py index ac1c2694069a..50a8fedbc82a 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -2066,11 +2066,12 @@ def test_install_machinery(self) -> None: superuser=True, request={ "service": "deepl", - "configuration": '{"wrong": ""}', + "configuration": {"wrong": ""}, }, + format="json", ) - # invalid field + # invalid field with multipart response = self.do_request( "api:project-machinery-settings", self.project_kwargs, @@ -2083,7 +2084,7 @@ def test_install_machinery(self) -> None: }, ) - # valid form + # valid form with multipart DeepLTranslationTest.mock_response() response = self.do_request( "api:project-machinery-settings", @@ -2106,9 +2107,10 @@ def test_install_machinery(self) -> None: superuser=True, request={ "service": "deepl", - "configuration": '{"key": "xy", "url": "https://api.deepl.com/v2/"}', + "configuration": {"key": "xy", "url": "https://api.deepl.com/v2/"}, "update": "true", }, + format="json", ) # list configurations diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index c48748991aed..ace063f8b2f4 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -55,7 +55,7 @@ class Meta: def validate_service_configuration( - service_name: str, configuration: str + service_name: str, configuration: str | dict ) -> tuple[BatchMachineTranslation, dict, list[str]]: """ Validate given service configuration. @@ -71,15 +71,19 @@ def validate_service_configuration( except KeyError as error: msg = f"Service not found: {service_name}" raise ValueError(msg) from error - try: - configuration = json.loads(configuration) - except ValueError as error: - msg = f"Invalid service configuration: {error}" - raise ValueError(msg) from error + + if isinstance(configuration, str): + try: + service_configuration = json.loads(configuration) + except ValueError as error: + msg = f"Invalid service configuration: {error}" + raise ValueError(msg) from error + else: + service_configuration = configuration errors = [] if service.settings_form is not None: - form = service.settings_form(service, data=configuration) + form = service.settings_form(service, data=service_configuration) # validate form if not form.is_valid(): errors.extend(list(form.non_field_errors())) @@ -88,4 +92,4 @@ def validate_service_configuration( errors.extend( [f"Error in {field.name}: {error}" for error in field.errors] ) - return service, configuration, errors + return service, service_configuration, errors From 315371cc16209e14ba617c76b4aae779538de616 Mon Sep 17 00:00:00 2001 From: gersona Date: Tue, 19 Nov 2024 15:28:50 +0300 Subject: [PATCH 06/12] fix possible stack trace leak --- weblate/api/views.py | 12 +++++++----- .../management/commands/install_machinery.py | 11 +++++------ weblate/machinery/models.py | 8 ++++---- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/weblate/api/views.py b/weblate/api/views.py index a6259624f785..a2f2e1d1ade1 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -1027,12 +1027,14 @@ def machinery_settings(self, request: Request, **kwargs): {"errors": ["Missing service name"]}, status=HTTP_400_BAD_REQUEST ) - try: - service, configuration, errors = validate_service_configuration( - service_name, request.data.get("configuration", "{}") + service, configuration, errors = validate_service_configuration( + service_name, request.data.get("configuration", "{}") + ) + if service is None: + return Response( + {"errors": errors}, + status=HTTP_400_BAD_REQUEST, ) - except Exception as error: - errors = [str(error)] if errors: return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST) diff --git a/weblate/machinery/management/commands/install_machinery.py b/weblate/machinery/management/commands/install_machinery.py index a6387a1d290e..48b9f55ca0a8 100644 --- a/weblate/machinery/management/commands/install_machinery.py +++ b/weblate/machinery/management/commands/install_machinery.py @@ -25,12 +25,11 @@ def add_arguments(self, parser) -> None: ) def handle(self, *args, **options) -> None: - try: - service, configuration, errors = validate_service_configuration( - options["service"], options["configuration"] - ) - except Exception as e: - raise CommandError(str(e)) from e + service, configuration, errors = validate_service_configuration( + options["service"], options["configuration"] + ) + if service is None: + raise CommandError("\n".join(errors)) if errors: for error in errors: diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index ace063f8b2f4..6ad8398b16a1 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -56,7 +56,7 @@ class Meta: def validate_service_configuration( service_name: str, configuration: str | dict -) -> tuple[BatchMachineTranslation, dict, list[str]]: +) -> tuple[BatchMachineTranslation | None, dict, list[str]]: """ Validate given service configuration. @@ -68,16 +68,16 @@ def validate_service_configuration( """ try: service = MACHINERY[service_name] - except KeyError as error: + except KeyError: msg = f"Service not found: {service_name}" - raise ValueError(msg) from error + return None, {}, [msg] if isinstance(configuration, str): try: service_configuration = json.loads(configuration) except ValueError as error: msg = f"Invalid service configuration: {error}" - raise ValueError(msg) from error + return service, {}, [msg] else: service_configuration = configuration From 2eab96059937fa1ce75e85a013d610d4f87a8a4a Mon Sep 17 00:00:00 2001 From: Gersona Date: Tue, 26 Nov 2024 17:54:48 +0300 Subject: [PATCH 07/12] check permissions for GET request on mechinery settings --- weblate/api/tests.py | 12 +++++++++++- weblate/api/views.py | 7 ++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/weblate/api/tests.py b/weblate/api/tests.py index 50a8fedbc82a..2e01013dc35b 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -2016,7 +2016,17 @@ def test_install_machinery(self) -> None: """Test the machinery settings API endpoint for various scenarios.""" from weblate.machinery.tests import DeepLTranslationTest - # unauthenticated + # unauthenticated get + self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="get", + code=403, + authenticated=True, + superuser=False, + ) + + # unauthenticated post self.do_request( "api:project-machinery-settings", self.project_kwargs, diff --git a/weblate/api/views.py b/weblate/api/views.py index a2f2e1d1ade1..5826c4383095 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -1016,10 +1016,11 @@ def file(self, request: Request, **kwargs): def machinery_settings(self, request: Request, **kwargs): """List or create/update machinery configuration for a project.""" project = self.get_object() - if request.method == "POST": - if not request.user.has_perm("project.edit", project): - self.permission_denied(request, "Can not edit machinery configuration") + if not request.user.has_perm("project.edit", project): + self.permission_denied(request, "Can not edit machinery configuration") + + if request.method == "POST": try: service_name = request.data["service"] except KeyError: From 17d36e9dbbd33ce18685dbd48d9a8925ac72094a Mon Sep 17 00:00:00 2001 From: Gersona Date: Wed, 27 Nov 2024 18:14:08 +0300 Subject: [PATCH 08/12] additional PATCH and PUT methods for machine settings --- weblate/api/serializers.py | 71 ++++++++++++++++++++++++ weblate/api/tests.py | 107 +++++++++++++++++++++++++++++++++--- weblate/api/views.py | 75 +++++++++++++++++++++---- weblate/machinery/models.py | 5 +- 4 files changed, 238 insertions(+), 20 deletions(-) diff --git a/weblate/api/serializers.py b/weblate/api/serializers.py index 64152968ca33..d51d78765e48 100644 --- a/weblate/api/serializers.py +++ b/weblate/api/serializers.py @@ -10,6 +10,13 @@ from django.conf import settings from django.db.models import Model +from drf_spectacular.extensions import OpenApiSerializerExtension +from drf_spectacular.plumbing import build_basic_type, build_object_type +from drf_spectacular.utils import ( + OpenApiExample, + extend_schema_serializer, + inline_serializer, +) from rest_framework import serializers from weblate.accounts.models import Subscription @@ -44,6 +51,7 @@ ) if TYPE_CHECKING: + from drf_spectacular.openapi import AutoSchema from rest_framework.request import Request _MT = TypeVar("_MT", bound=Model) # Model Type @@ -1517,3 +1525,66 @@ class MetricsSerializer(ReadOnlySerializer): child=serializers.IntegerField(), source="get_celery_queues" ) name = serializers.CharField(source="get_name") + + +@extend_schema_serializer( + examples=[ + OpenApiExample( + "Service settings example", + value={ + "service": "service_name", + "configuration": {"key": "xxxxx", "url": "https://api.service.com/"}, + }, + ) + ] +) +class ServiceConfigSerializer(serializers.Serializer): + service = serializers.CharField() + configuration = serializers.DictField() + + +@extend_schema_serializer( + examples=[ + OpenApiExample( + "Service settings example", + value={ + "service1": {"key": "XXXXXXX", "url": "https://api.service.com/"}, + "service2": {"secret": "SECRET_KEY", "credentials": "XXXXXXX"}, + }, + request_only=False, + response_only=True, + ), + OpenApiExample( + "Error message", value={"errors": "Error message"}, status_codes=[400] + ), + ] +) +class ProjectMachinerySettingsSerializer(ReadOnlySerializer): + def to_representation(self, instance: Project): + return dict(instance.machinery_settings) + + +class ProjectMachinerySettingsSerializerExtension(OpenApiSerializerExtension): + target_class = ProjectMachinerySettingsSerializer + + def map_serializer(self, auto_schema: AutoSchema, direction): + return build_object_type(properties={"service_name": build_basic_type(dict)}) + + +EditServiceSettingsResponseSerializer = { + 200: inline_serializer( + "Simple message serializer", + fields={ + "message": serializers.CharField(), + }, + ), + 201: inline_serializer( + "Simple message serializer", + fields={ + "message": serializers.CharField(), + }, + ), + 400: inline_serializer( + "Simple error message serializer", fields={"errors": serializers.CharField()} + ), +} diff --git a/weblate/api/tests.py b/weblate/api/tests.py index 2e01013dc35b..da7ed80a6b95 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -2014,7 +2014,7 @@ def test_credits(self) -> None: @responses.activate def test_install_machinery(self) -> None: """Test the machinery settings API endpoint for various scenarios.""" - from weblate.machinery.tests import DeepLTranslationTest + from weblate.machinery.tests import AlibabaTranslationTest, DeepLTranslationTest # unauthenticated get self.do_request( @@ -2057,12 +2057,12 @@ def test_install_machinery(self) -> None: request={"service": "unknown"}, ) - # no form + # create configuration: no form response = self.do_request( "api:project-machinery-settings", self.project_kwargs, method="post", - code=200, + code=201, superuser=True, request={"service": "weblate"}, ) @@ -2094,13 +2094,13 @@ def test_install_machinery(self) -> None: }, ) - # valid form with multipart + # create configuration: valid form with multipart DeepLTranslationTest.mock_response() response = self.do_request( "api:project-machinery-settings", self.project_kwargs, method="post", - code=200, + code=201, superuser=True, request={ "service": "deepl", @@ -2108,17 +2108,55 @@ def test_install_machinery(self) -> None: }, ) - # update configuration + # create configuration: valid form with json + AlibabaTranslationTest().mock_response() response = self.do_request( "api:project-machinery-settings", self.project_kwargs, method="post", + code=201, + superuser=True, + request={ + "service": "alibaba", + "configuration": { + "key": "alibaba-key-v1", + "secret": "alibaba-secret", + "region": "alibaba-region", + }, + }, + format="json", + ) + + # update configuration: incorrect method + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="post", + code=400, + superuser=True, + request={ + "service": "deepl", + "configuration": { + "key": "deepl-key-v1", + "url": "https://api.deepl.com/v2/", + }, + }, + format="json", + ) + + # update configuration: valid method + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="patch", code=200, superuser=True, request={ "service": "deepl", - "configuration": {"key": "xy", "url": "https://api.deepl.com/v2/"}, - "update": "true", + "configuration": { + "key": "deepl-key-v2", + "url": "https://api.deepl.com/v2/", + }, }, format="json", ) @@ -2132,7 +2170,58 @@ def test_install_machinery(self) -> None: superuser=True, ) self.assertIn("weblate", response.data) - self.assertEqual(response.data["deepl"]["key"], "xy") + self.assertEqual(response.data["deepl"]["key"], "deepl-key-v2") + self.assertEqual(response.data["alibaba"]["key"], "alibaba-key-v1") + + # remove configuration + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="patch", + code=200, + superuser=True, + request={"service": "deepl", "configuration": None}, + format="json", + ) + + # check configuration no longer exists + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="get", + code=200, + superuser=True, + ) + self.assertNotIn("deepl", response.data) + + # replace all configurations + new_config = { + "deepl": {"key": "deepl-key-v3", "url": "https://api.deepl.com/v2/"}, + "alibaba": { + "key": "alibaba-key-v2", + "secret": "alibaba-secret", + "region": "alibaba-region", + }, + } + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="put", + code=201, + superuser=True, + request=new_config, + format="json", + ) + + # check all configurations + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="get", + superuser=True, + ) + + self.assertEqual(new_config, response.data) class ComponentAPITest(APIBaseTest): diff --git a/weblate/api/views.py b/weblate/api/views.py index 5826c4383095..606227703e9e 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -58,6 +58,7 @@ ChangeSerializer, ComponentListSerializer, ComponentSerializer, + EditServiceSettingsResponseSerializer, FullUserSerializer, GroupSerializer, LabelSerializer, @@ -69,12 +70,14 @@ MonolingualUnitSerializer, NewUnitSerializer, NotificationSerializer, + ProjectMachinerySettingsSerializer, ProjectSerializer, RepoRequestSerializer, RoleSerializer, ScreenshotCreateSerializer, ScreenshotFileSerializer, ScreenshotSerializer, + ServiceConfigSerializer, StatisticsSerializer, TranslationSerializer, UnitSerializer, @@ -1012,15 +1015,23 @@ def file(self, request: Request, **kwargs): name=instance.slug, ) - @action(detail=True, methods=["get", "post"]) + @extend_schema(responses=ProjectMachinerySettingsSerializer, methods=["GET"]) + @extend_schema( + request=ServiceConfigSerializer, + responses=EditServiceSettingsResponseSerializer, + methods=["POST", "PATCH", "PUT"], + ) + @action(detail=True, methods=["get", "post", "patch", "put"]) def machinery_settings(self, request: Request, **kwargs): """List or create/update machinery configuration for a project.""" project = self.get_object() if not request.user.has_perm("project.edit", project): - self.permission_denied(request, "Can not edit machinery configuration") + self.permission_denied( + request, "Can not retrieve/edit machinery configuration" + ) - if request.method == "POST": + if request.method in {"POST", "PATCH"}: try: service_name = request.data["service"] except KeyError: @@ -1031,22 +1042,66 @@ def machinery_settings(self, request: Request, **kwargs): service, configuration, errors = validate_service_configuration( service_name, request.data.get("configuration", "{}") ) - if service is None: + + if service is None or errors: + return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST) + + if request.method == "PATCH": + if configuration: + # update a configuration + project.machinery_settings[service_name] = configuration + project.save(update_fields=["machinery_settings"]) + return Response( + {"message": f"Service updated: {service.name}"}, + status=HTTP_200_OK, + ) + # remove a configuration + project.machinery_settings.pop(service_name, None) + project.save(update_fields=["machinery_settings"]) return Response( - {"errors": errors}, + {"message": f"Service removed: {service.name}"}, + status=HTTP_200_OK, + ) + # POST + if service_name in project.machinery_settings: + return Response( + {"errors": ["Service already exists"]}, status=HTTP_400_BAD_REQUEST, ) - if errors: - return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST) - project.machinery_settings[service_name] = configuration project.save(update_fields=["machinery_settings"]) return Response( - {"message": f"Service installed: {service.name}"}, status=HTTP_200_OK + {"message": f"Service installed: {service.name}"}, + status=HTTP_201_CREATED, + ) + + if request.method == "PUT": + # replace all service configuration + valid_configurations: dict[str, dict] = {} + for service_name, configuration in request.data.items(): + service, configuration, errors = validate_service_configuration( + service_name, configuration + ) + + if service is None or errors: + return Response({"errors": errors}, status=HTTP_400_BAD_REQUEST) + + valid_configurations[service_name] = configuration + + project.machinery_settings = valid_configurations + project.save(update_fields=["machinery_settings"]) + return Response( + { + "message": f"Services installed: {", ".join(valid_configurations.keys())}" + }, + status=HTTP_201_CREATED, ) - return Response(project.machinery_settings, status=HTTP_200_OK) + # get request + return Response( + data=ProjectMachinerySettingsSerializer(project).data, status=HTTP_200_OK + ) class ComponentViewSet( diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index 6ad8398b16a1..78840ac3cba5 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -90,6 +90,9 @@ def validate_service_configuration( for field in form: errors.extend( - [f"Error in {field.name}: {error}" for error in field.errors] + [ + f"Error in {field.name} ({service_name}): {error}" + for error in field.errors + ] ) return service, service_configuration, errors From 7a1cb7cff74429cb39feb2d40df3da8782a2f49b Mon Sep 17 00:00:00 2001 From: Gersona Date: Wed, 27 Nov 2024 18:50:56 +0300 Subject: [PATCH 09/12] update OpenAPI documentation --- weblate/api/serializers.py | 46 ++++++++++++++--------------- weblate/api/views.py | 59 +++++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/weblate/api/serializers.py b/weblate/api/serializers.py index d51d78765e48..cac6d70c4d34 100644 --- a/weblate/api/serializers.py +++ b/weblate/api/serializers.py @@ -1538,7 +1538,7 @@ class MetricsSerializer(ReadOnlySerializer): ) ] ) -class ServiceConfigSerializer(serializers.Serializer): +class SingleServiceConfigSerializer(serializers.Serializer): service = serializers.CharField() configuration = serializers.DictField() @@ -1553,13 +1553,10 @@ class ServiceConfigSerializer(serializers.Serializer): }, request_only=False, response_only=True, - ), - OpenApiExample( - "Error message", value={"errors": "Error message"}, status_codes=[400] - ), + ) ] ) -class ProjectMachinerySettingsSerializer(ReadOnlySerializer): +class ProjectMachinerySettingsSerializer(serializers.Serializer): def to_representation(self, instance: Project): return dict(instance.machinery_settings) @@ -1571,20 +1568,23 @@ def map_serializer(self, auto_schema: AutoSchema, direction): return build_object_type(properties={"service_name": build_basic_type(dict)}) -EditServiceSettingsResponseSerializer = { - 200: inline_serializer( - "Simple message serializer", - fields={ - "message": serializers.CharField(), - }, - ), - 201: inline_serializer( - "Simple message serializer", - fields={ - "message": serializers.CharField(), - }, - ), - 400: inline_serializer( - "Simple error message serializer", fields={"errors": serializers.CharField()} - ), -} +def edit_service_settings_response_serializer(*codes) -> int: + _serializers = { + 200: inline_serializer( + "Simple message serializer", + fields={ + "message": serializers.CharField(), + }, + ), + 201: inline_serializer( + "Simple message serializer", + fields={ + "message": serializers.CharField(), + }, + ), + 400: inline_serializer( + "Simple error message serializer", + fields={"errors": serializers.CharField()}, + ), + } + return {code: _serializers[code] for code in codes} diff --git a/weblate/api/views.py b/weblate/api/views.py index 606227703e9e..a28971c03e1c 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -58,7 +58,6 @@ ChangeSerializer, ComponentListSerializer, ComponentSerializer, - EditServiceSettingsResponseSerializer, FullUserSerializer, GroupSerializer, LabelSerializer, @@ -77,13 +76,14 @@ ScreenshotCreateSerializer, ScreenshotFileSerializer, ScreenshotSerializer, - ServiceConfigSerializer, + SingleServiceConfigSerializer, StatisticsSerializer, TranslationSerializer, UnitSerializer, UnitWriteSerializer, UploadRequestSerializer, UserStatisticsSerializer, + edit_service_settings_response_serializer, get_reverse_kwargs, ) from weblate.auth.models import AuthenticatedHttpRequest, Group, Role, User @@ -1015,11 +1015,28 @@ def file(self, request: Request, **kwargs): name=instance.slug, ) - @extend_schema(responses=ProjectMachinerySettingsSerializer, methods=["GET"]) @extend_schema( - request=ServiceConfigSerializer, - responses=EditServiceSettingsResponseSerializer, - methods=["POST", "PATCH", "PUT"], + responses=ProjectMachinerySettingsSerializer, + methods=["GET"], + description="List machinery settings for a project.", + ) + @extend_schema( + request=SingleServiceConfigSerializer, + responses=edit_service_settings_response_serializer(201, 400), + methods=["POST"], + description="Install a new machinery service", + ) + @extend_schema( + request=SingleServiceConfigSerializer, + responses=edit_service_settings_response_serializer(200, 400), + methods=["PATCH"], + description="Partially update a single service. Leave configuration blank to remove the service", + ) + @extend_schema( + request=ProjectMachinerySettingsSerializer, + responses=edit_service_settings_response_serializer(200, 400), + methods=["PUT"], + description="Replace configuration for all services.", ) @action(detail=True, methods=["get", "post", "patch", "put"]) def machinery_settings(self, request: Request, **kwargs): @@ -1062,21 +1079,22 @@ def machinery_settings(self, request: Request, **kwargs): {"message": f"Service removed: {service.name}"}, status=HTTP_200_OK, ) - # POST - if service_name in project.machinery_settings: + + if request.method == "POST": + if service_name in project.machinery_settings: + return Response( + {"errors": ["Service already exists"]}, + status=HTTP_400_BAD_REQUEST, + ) + + project.machinery_settings[service_name] = configuration + project.save(update_fields=["machinery_settings"]) return Response( - {"errors": ["Service already exists"]}, - status=HTTP_400_BAD_REQUEST, + {"message": f"Service installed: {service.name}"}, + status=HTTP_201_CREATED, ) - project.machinery_settings[service_name] = configuration - project.save(update_fields=["machinery_settings"]) - return Response( - {"message": f"Service installed: {service.name}"}, - status=HTTP_201_CREATED, - ) - - if request.method == "PUT": + elif request.method == "PUT": # replace all service configuration valid_configurations: dict[str, dict] = {} for service_name, configuration in request.data.items(): @@ -1098,9 +1116,10 @@ def machinery_settings(self, request: Request, **kwargs): status=HTTP_201_CREATED, ) - # get request + # GET method return Response( - data=ProjectMachinerySettingsSerializer(project).data, status=HTTP_200_OK + data=ProjectMachinerySettingsSerializer(project).data, + status=HTTP_200_OK, ) From 853f9727a27259928ba30c650a512a0e73939c0c Mon Sep 17 00:00:00 2001 From: Gersona Date: Thu, 28 Nov 2024 20:09:04 +0300 Subject: [PATCH 10/12] fix invalid serialzer name --- weblate/api/serializers.py | 10 ++++++---- weblate/api/views.py | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/weblate/api/serializers.py b/weblate/api/serializers.py index cac6d70c4d34..e29b070e478c 100644 --- a/weblate/api/serializers.py +++ b/weblate/api/serializers.py @@ -1568,22 +1568,24 @@ def map_serializer(self, auto_schema: AutoSchema, direction): return build_object_type(properties={"service_name": build_basic_type(dict)}) -def edit_service_settings_response_serializer(*codes) -> int: +def edit_service_settings_response_serializer( + method: str, *codes +) -> dict[int, serializers.Serializer]: _serializers = { 200: inline_serializer( - "Simple message serializer", + f"{method}_200_Message_response_serializer", fields={ "message": serializers.CharField(), }, ), 201: inline_serializer( - "Simple message serializer", + f"{method}_201_Message_response_serializer", fields={ "message": serializers.CharField(), }, ), 400: inline_serializer( - "Simple error message serializer", + f"{method}_400_Error_message_serializer", fields={"errors": serializers.CharField()}, ), } diff --git a/weblate/api/views.py b/weblate/api/views.py index a28971c03e1c..5ac3ea936d4c 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -1022,19 +1022,19 @@ def file(self, request: Request, **kwargs): ) @extend_schema( request=SingleServiceConfigSerializer, - responses=edit_service_settings_response_serializer(201, 400), + responses=edit_service_settings_response_serializer("post", 201, 400), methods=["POST"], description="Install a new machinery service", ) @extend_schema( request=SingleServiceConfigSerializer, - responses=edit_service_settings_response_serializer(200, 400), + responses=edit_service_settings_response_serializer("patch", 200, 400), methods=["PATCH"], description="Partially update a single service. Leave configuration blank to remove the service", ) @extend_schema( request=ProjectMachinerySettingsSerializer, - responses=edit_service_settings_response_serializer(200, 400), + responses=edit_service_settings_response_serializer("put", 200, 400), methods=["PUT"], description="Replace configuration for all services.", ) From d1c2acbae85b7cfa3072ec16121e181859aee560 Mon Sep 17 00:00:00 2001 From: Gersona Date: Thu, 28 Nov 2024 20:22:28 +0300 Subject: [PATCH 11/12] SyntaxError fix --- weblate/api/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weblate/api/views.py b/weblate/api/views.py index 5ac3ea936d4c..e41b71524d6c 100644 --- a/weblate/api/views.py +++ b/weblate/api/views.py @@ -1111,7 +1111,7 @@ def machinery_settings(self, request: Request, **kwargs): project.save(update_fields=["machinery_settings"]) return Response( { - "message": f"Services installed: {", ".join(valid_configurations.keys())}" + "message": f"Services installed: {', '.join(valid_configurations.keys())}" }, status=HTTP_201_CREATED, ) From fe29380d57129d8a0ba9e40065e15939ab760877 Mon Sep 17 00:00:00 2001 From: Gersona Date: Fri, 29 Nov 2024 06:47:30 +0300 Subject: [PATCH 12/12] improve test coverage --- weblate/api/tests.py | 29 +++++++++++++++ .../management/commands/install_machinery.py | 4 +-- weblate/machinery/models.py | 2 +- weblate/machinery/tests.py | 35 +++++++++++++++++-- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/weblate/api/tests.py b/weblate/api/tests.py index da7ed80a6b95..418ad7204dd3 100644 --- a/weblate/api/tests.py +++ b/weblate/api/tests.py @@ -2194,6 +2194,34 @@ def test_install_machinery(self) -> None: ) self.assertNotIn("deepl", response.data) + # invalid replace all configurations (missing required config) + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="put", + code=400, + superuser=True, + request={ + "deepl": {"key": "deepl-key-valid", "url": "https://api.deepl.com/v2/"}, + "unknown": {"key": "alibaba-key-invalid"}, + }, + format="json", + ) + + # invalid replace all configurations (missing required config) + response = self.do_request( + "api:project-machinery-settings", + self.project_kwargs, + method="put", + code=400, + superuser=True, + request={ + "deepl": {"key": "deepl-key-valid", "url": "https://api.deepl.com/v2/"}, + "alibaba": {"key": "alibaba-key-invalid"}, + }, + format="json", + ) + # replace all configurations new_config = { "deepl": {"key": "deepl-key-v3", "url": "https://api.deepl.com/v2/"}, @@ -2203,6 +2231,7 @@ def test_install_machinery(self) -> None: "region": "alibaba-region", }, } + response = self.do_request( "api:project-machinery-settings", self.project_kwargs, diff --git a/weblate/machinery/management/commands/install_machinery.py b/weblate/machinery/management/commands/install_machinery.py index 48b9f55ca0a8..7611eec5a2c3 100644 --- a/weblate/machinery/management/commands/install_machinery.py +++ b/weblate/machinery/management/commands/install_machinery.py @@ -28,10 +28,8 @@ def handle(self, *args, **options) -> None: service, configuration, errors = validate_service_configuration( options["service"], options["configuration"] ) - if service is None: - raise CommandError("\n".join(errors)) - if errors: + if service is None or errors: for error in errors: self.stderr.write(error) msg = "Invalid add-on configuration!" diff --git a/weblate/machinery/models.py b/weblate/machinery/models.py index 78840ac3cba5..c9cc74791ab0 100644 --- a/weblate/machinery/models.py +++ b/weblate/machinery/models.py @@ -76,7 +76,7 @@ def validate_service_configuration( try: service_configuration = json.loads(configuration) except ValueError as error: - msg = f"Invalid service configuration: {error}" + msg = f"Invalid service configuration ({service_name}): {error}" return service, {}, [msg] else: service_configuration = configuration diff --git a/weblate/machinery/tests.py b/weblate/machinery/tests.py index ccda2092d7c4..8fe11356a487 100644 --- a/weblate/machinery/tests.py +++ b/weblate/machinery/tests.py @@ -2628,7 +2628,7 @@ def test_list_addons(self) -> None: call_command("list_machinery", stdout=output) self.assertIn("DeepL", output.getvalue()) - def test_install_no_form(self) -> None: + def test_valid_install_no_form(self) -> None: output = StringIO() call_command( "install_machinery", @@ -2639,6 +2639,17 @@ def test_install_no_form(self) -> None: ) self.assertIn("Service installed: Weblate", output.getvalue()) + def test_install_unknown_service(self) -> None: + output = StringIO() + with self.assertRaises(CommandError): + call_command( + "install_machinery", + "--service", + "unknown", + stdout=output, + stderr=output, + ) + def test_install_missing_form(self) -> None: output = StringIO() with self.assertRaises(CommandError): @@ -2672,7 +2683,27 @@ def test_install_valid_form(self) -> None: "--service", "deepl", "--configuration", - '{"key": "x", "url": "https://api.deepl.com/v2/"}', + '{"key": "x1", "url": "https://api.deepl.com/v2/"}', stdout=output, stderr=output, ) + self.assertTrue( + Setting.objects.filter(category=SettingCategory.MT, name="deepl").exists() + ) + + # update configuration + call_command( + "install_machinery", + "--service", + "deepl", + "--configuration", + '{"key": "x2", "url": "https://api.deepl.com/v2/"}', + "--update", + stdout=output, + stderr=output, + ) + + setting = Setting.objects.get(category=SettingCategory.MT, name="deepl") + self.assertEqual( + setting.value, {"key": "x2", "url": "https://api.deepl.com/v2/"} + )