diff --git a/.gitignore b/.gitignore index d043c9dd..f462e87d 100644 --- a/.gitignore +++ b/.gitignore @@ -64,5 +64,8 @@ docs/openedx_authz.*.rst *.sqlite3 *.db +# Generated by `make extract_translations` / `extract_schema_translations` +src/openedx_authz/engine/schema/_generated_translations.py + # IDE .vscode/ diff --git a/Makefile b/Makefile index 58a20476..7352c87f 100644 --- a/Makefile +++ b/Makefile @@ -68,6 +68,7 @@ selfcheck: ## check that the Makefile is well-formed extract_translations: ## extract strings to be translated, outputting .mo files rm -rf docs/_build + python manage.py extract_schema_translations cd src/openedx_authz && i18n_tool extract --no-segment compile_translations: ## compile translation files, outputting .po files for each supported language diff --git a/src/openedx_authz/engine/schema/translation.py b/src/openedx_authz/engine/schema/translation.py new file mode 100644 index 00000000..cae2c7b8 --- /dev/null +++ b/src/openedx_authz/engine/schema/translation.py @@ -0,0 +1,170 @@ +"""Extract translatable strings from the static authz schema (ADR 0020). + +The authz schema's ``display_name`` and ``description`` fields are source-language +text that end users see through APIs and frontend clients, so they need to go +through the Open edX translation process (OEP-58). Standard extraction tools only +scan Python and JavaScript source, so they can't find strings written in YAML. + +This module reads every discovered schema resource directly with ``yaml.safe_load``, +independent of the schema ``load``/``validate``/``compile`` pipeline: it only needs +the raw ``display_name``/``description`` fields, not resolved role extensions, +priority conflicts, or any of the richer typed documents those steps build. That +keeps it usable (and testable) even before the rest of the pipeline exists, and +immune to changes in how that pipeline models conflicts or precedence. + +It emits a generated Python module containing one ``pgettext()`` call per +translatable field, which ``i18n_tool extract`` (a thin wrapper around Django's +``makemessages``) then picks up like any other source file. ``pgettext`` is used +instead of plain ``gettext`` because ADR 0020 requires a permission's display name +and description to remain separate translation messages even when they contain +the same English text; a plain ``gettext`` call would merge identical msgids into +one catalog entry. The context string also carries the field's stable identifier, +so two different roles that happen to share a one-word name like "Admin" stay +separate messages too. + +This generated module is never imported or executed: its only purpose is to be +valid Python source for ``makemessages`` to scan. At runtime, the API (once it +exists, see openedx-authz#432's own note that this depends on the still-unmerged +schema loader and catalog API) looks up a field's source-language value directly +in the compiled catalog with the same ``pgettext(context, value)`` pair used here, +so the generated calls must stay in sync with however that lookup builds its +context string. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +import yaml + +from openedx_authz.engine.schema.discovery import DiscoveredResource, SchemaDiscovery, SchemaDiscoveryError + +GENERATED_FILE_HEADER = '''"""Generated by extract_schema_translations. Do not edit, do not import. + +This file exists only so that ``i18n_tool extract`` (Django's ``makemessages``) +can find the authz schema's source-language display_name/description strings, +which otherwise live in YAML and are invisible to it. Regenerate it with: + + python manage.py extract_schema_translations + +See openedx_authz.engine.schema.translation for why pgettext is used here. +""" +from django.utils.translation import pgettext +''' + + +@dataclass(frozen=True) +class TranslatableMessage: + """One ``pgettext(context, message)`` pair extracted from the schema. + + Attributes: + context: Disambiguates otherwise-identical messages, formatted as + ``"{field_kind}.{field_name}:{stable_id}"``, e.g. + ``"permission.display_name:courses.view_course"``. + message: The source-language (English) string to translate. + """ + + context: str + message: str + + +class SchemaTranslationExtractionError(Exception): + """A schema resource could not be read or parsed for extraction.""" + + +def extract_messages(resources: list[DiscoveredResource] | None = None) -> list[TranslatableMessage]: + """Extract every translatable schema string from the given (or discovered) resources. + + Args: + resources: Resources to extract from. Defaults to everything + ``SchemaDiscovery`` finds (every registered schema directory), which + is what the management command uses. + + Returns: + Deduplicated messages, sorted by context, so the generated file's + diffs are stable across runs regardless of discovery order. + + Raises: + SchemaTranslationExtractionError: A resource's bytes couldn't be read, + or weren't valid YAML, or the document wasn't a mapping at the top + level. A translation build must fail loudly here (ADR 0020) rather + than silently skip a file, the same way schema validation does. + """ + if resources is None: + try: + resources = SchemaDiscovery().discover() + except SchemaDiscoveryError as exc: + raise SchemaTranslationExtractionError(str(exc)) from exc + + messages: dict[str, TranslatableMessage] = {} + for resource in resources: + for extracted in _extract_from_resource(resource): + # Last one wins on a context collision; in practice this only happens + # if two files genuinely define the same stable id, which validation + # (a separate step) is responsible for catching. + messages[extracted.context] = extracted + + return sorted(messages.values(), key=lambda m: m.context) + + +def _extract_from_resource(resource: DiscoveredResource) -> list[TranslatableMessage]: + """Parse one resource's YAML and pull its translatable fields out.""" + try: + contents = resource.read_bytes() + except SchemaDiscoveryError as exc: + raise SchemaTranslationExtractionError(str(exc)) from exc + + try: + document = yaml.safe_load(contents) + except yaml.YAMLError as exc: + raise SchemaTranslationExtractionError( + f"Invalid YAML in {resource.package}:{resource.resource_path}: {exc}" + ) from exc + + if document is None: + return [] + if not isinstance(document, dict): + raise SchemaTranslationExtractionError( + f"Schema file {resource.package}:{resource.resource_path} must be a mapping " + f"at the top level, got {type(document).__name__}." + ) + + messages: list[TranslatableMessage] = [] + for category in document.get("permission_categories") or []: + messages.extend(_messages_for_entry("permission_category", category.get("id", ""), category)) + for permission in document.get("permissions") or []: + stable_id = f"{permission.get('namespace', '')}.{permission.get('name', '')}" + messages.extend(_messages_for_entry("permission", stable_id, permission)) + for role in document.get("roles") or []: + messages.extend(_messages_for_entry("role", role.get("id", ""), role)) + for extension in document.get("role_extensions") or []: + # display_name/description are optional on an extension (it may only + # touch permissions or the icon), so only emit the fields it actually + # overrides instead of a pair of empty messages for every extension. + messages.extend(_messages_for_entry("role_extension", extension.get("role", ""), extension, optional=True)) + + return messages + + +def _messages_for_entry( + field_kind: str, stable_id: str, entry: dict, *, optional: bool = False +) -> list[TranslatableMessage]: + """Build the display_name/description messages for one schema entry.""" + messages = [] + for field_name in ("display_name", "description"): + value = entry.get(field_name) + if value is None: + if optional: + continue + value = "" + messages.append(TranslatableMessage(context=f"{field_kind}.{field_name}:{stable_id}", message=str(value))) + return messages + + +def render_module(messages: list[TranslatableMessage]) -> str: + """Render the extracted messages as a generated Python module's source text.""" + lines = [GENERATED_FILE_HEADER] + for extracted in messages: + lines.append(f"pgettext({extracted.context!r}, {extracted.message!r})") + lines.append("") # Trailing newline. + return "\n".join(lines) diff --git a/src/openedx_authz/management/commands/extract_schema_translations.py b/src/openedx_authz/management/commands/extract_schema_translations.py new file mode 100644 index 00000000..90af27a3 --- /dev/null +++ b/src/openedx_authz/management/commands/extract_schema_translations.py @@ -0,0 +1,59 @@ +"""Django management command to extract translatable strings from the authz schema. + +See openedx_authz.engine.schema.translation for why this exists and how it works. +""" + +import os + +from django.core.management.base import BaseCommand, CommandError + +from openedx_authz import ROOT_DIRECTORY +from openedx_authz.engine.schema.translation import ( + SchemaTranslationExtractionError, + extract_messages, + render_module, +) + +DEFAULT_OUTPUT_PATH = os.path.join(ROOT_DIRECTORY, "engine", "schema", "_generated_translations.py") + + +class Command(BaseCommand): + """Generate a Python module with one pgettext() call per translatable schema string. + + Run this before ``i18n_tool extract`` (see the ``extract_translations`` Makefile + target), so the generated calls are on disk for ``makemessages`` to scan. + + Example Usage: + python manage.py extract_schema_translations + python manage.py extract_schema_translations --output /path/to/file.py + """ + + help = "Extract translatable display_name/description strings from the authz schema into a generated module." + + def add_arguments(self, parser) -> None: + """Add command-line arguments to the argument parser. + + Args: + parser: The Django argument parser instance to configure. + """ + parser.add_argument( + "--output", + type=str, + default=DEFAULT_OUTPUT_PATH, + help="Path to write the generated module to.", + ) + + def handle(self, *args, **options) -> None: + """Extract schema translation messages and write the generated module.""" + try: + messages = extract_messages() + except SchemaTranslationExtractionError as exc: + raise CommandError(str(exc)) from exc + + output_path = options["output"] + with open(output_path, "w", encoding="utf-8") as output_file: + output_file.write(render_module(messages)) + + self.stdout.write( + self.style.SUCCESS(f"Extracted {len(messages)} translatable schema string(s) to {output_path}") + ) diff --git a/src/openedx_authz/tests/schema/test_translation.py b/src/openedx_authz/tests/schema/test_translation.py new file mode 100644 index 00000000..55ea321a --- /dev/null +++ b/src/openedx_authz/tests/schema/test_translation.py @@ -0,0 +1,267 @@ +"""Tests for schema translation string extraction (ADR 0020). + +The suite is grouped by the behaviour under test: + +* ``TestRealSchemaExtraction`` -- extraction runs cleanly against the real, + merged schema files and produces well-formed messages. +* ``TestFieldExtraction`` -- each schema entry type yields the right messages. +* ``TestContextDisambiguation`` -- identical text in different fields stays + as separate messages, per ADR 0020. +* ``TestExtractionErrors`` -- malformed input is a hard error. +* ``TestRenderModule`` -- the generated module is valid, scannable Python. +""" + +import pytest + +from openedx_authz.engine.schema.discovery import SchemaDiscovery, SchemaDiscoveryError +from openedx_authz.engine.schema.translation import ( + SchemaTranslationExtractionError, + TranslatableMessage, + extract_messages, + render_module, +) + +SCHEMA_DIR = "openedx_authz/authz/schema" + + +class FakeResource: + """A minimal stand-in for DiscoveredResource that reads from a string. + + extract_messages only ever calls read_bytes(), and reads .package/.resource_path + for error messages, so this is all it needs; it avoids having to stand up a real + importable package on disk for every synthetic/error-path test case. + """ + + def __init__(self, contents: str, package: str = "fake_pkg", resource_path: str = "schema/fake.yaml"): + self._contents = contents + self.package = package + self.resource_path = resource_path + + def read_bytes(self) -> bytes: + return self._contents.encode("utf-8") + + +def _resource(yaml_text: str) -> FakeResource: + return FakeResource(yaml_text) + + +class TestRealSchemaExtraction: + """Extraction against the real, already-merged schema files.""" + + def test_extracts_messages_from_the_real_schema(self): + """Running against the actual discovered schema yields well-formed messages.""" + resources = SchemaDiscovery(passed_in_directories=[SCHEMA_DIR]).discover() + messages = extract_messages(resources) + + assert messages + for message in messages: + assert message.context + assert ":" in message.context + assert message.message + + def test_known_role_display_name_is_present(self): + """A role we know exists in course_roles.yaml shows up with its real text.""" + resources = SchemaDiscovery(passed_in_directories=[SCHEMA_DIR]).discover() + messages = {m.context: m.message for m in extract_messages(resources)} + + assert messages["role.display_name:course_admin"] == "Course Admin" + + def test_is_deterministic(self): + """Extraction over the same input returns an identical, sorted list.""" + resources = SchemaDiscovery(passed_in_directories=[SCHEMA_DIR]).discover() + assert extract_messages(resources) == extract_messages(resources) + + def test_default_resources_are_discovered_when_none_given(self): + """Calling extract_messages() with no args discovers the real schema itself.""" + assert extract_messages() == extract_messages(SchemaDiscovery(passed_in_directories=[SCHEMA_DIR]).discover()) + + +class TestFieldExtraction: + """Each schema entry type yields the display_name/description messages it should.""" + + def test_permission_category(self): + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +permission_categories: + - id: course_content + display_name: Course content + description: Permissions for viewing and editing course content. +""")]) + assert messages == [ + TranslatableMessage( + "permission_category.description:course_content", + "Permissions for viewing and editing course content.", + ), + TranslatableMessage("permission_category.display_name:course_content", "Course content"), + ] + + def test_permission_stable_id_combines_namespace_and_name(self): + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +permissions: + - namespace: courses + name: view_course + display_name: View course + description: View course configuration and content. + category: course_content + scopes: [course-v1] +""")]) + contexts = {m.context for m in messages} + assert contexts == { + "permission.display_name:courses.view_course", + "permission.description:courses.view_course", + } + + def test_role(self): + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_observer + display_name: Course observer + description: Can review a course without changing it. + scopes: [course-v1] + permissions: [courses.view_course] +""")]) + contexts = {m.context: m.message for m in messages} + assert contexts == { + "role.display_name:course_observer": "Course observer", + "role.description:course_observer": "Can review a course without changing it.", + } + + def test_role_extension_only_emits_fields_it_actually_overrides(self): + """A role_extension that doesn't touch display_name/description emits nothing for them.""" + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +role_extensions: + - role: course_editor + add_permissions: [courses.export_course] +""")]) + assert messages == [] + + def test_role_extension_with_overrides(self): + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +role_extensions: + - role: course_editor + display_name: Course author + description: Creates and exports course content. +""")]) + contexts = {m.context: m.message for m in messages} + assert contexts == { + "role_extension.display_name:course_editor": "Course author", + "role_extension.description:course_editor": "Creates and exports course content.", + } + + def test_missing_entries_are_fine(self): + """A file that only defines some blocks doesn't error on the missing ones.""" + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_observer + display_name: Course observer + description: Can review a course without changing it. +""")]) + assert len(messages) == 2 + + def test_empty_file_yields_no_messages(self): + """An empty YAML file (e.g. all comments) parses to None, not an error.""" + assert extract_messages([_resource("# just a comment, no content\n")]) == [] + + +class TestContextDisambiguation: + """ADR 0020: identical English text in different fields stays separate.""" + + def test_identical_display_name_and_description_text_stay_separate(self): + """A display_name and description that happen to be identical text don't merge.""" + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_admin + display_name: Admin + description: Admin + scopes: [course-v1] +""")]) + assert len(messages) == 2 + assert messages[0].message == messages[1].message == "Admin" + assert messages[0].context != messages[1].context + + def test_same_display_name_text_across_different_roles_stays_separate(self): + """Two different roles sharing display text ("Admin") get distinct contexts.""" + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_admin + display_name: Admin + scopes: [course-v1] + - id: library_admin + display_name: Admin + scopes: [lib] +""")]) + display_name_messages = [m for m in messages if m.context.startswith("role.display_name:")] + assert len(display_name_messages) == 2 + assert {m.context for m in display_name_messages} == { + "role.display_name:course_admin", + "role.display_name:library_admin", + } + + +class TestExtractionErrors: + """Malformed schema input is a hard error, not a silent skip (ADR 0020).""" + + def test_invalid_yaml_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource("roles: [this is: not: valid: yaml")]) + + def test_non_mapping_top_level_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource("- just\n- a\n- list\n")]) + + def test_unreadable_resource_raises(self): + class BrokenResource(FakeResource): + def read_bytes(self) -> bytes: + raise SchemaDiscoveryError("could not read resource") + + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([BrokenResource("")]) + + def test_discovery_failure_propagates_when_resources_not_given(self, monkeypatch): + def _raise(): + raise SchemaDiscoveryError("boom") + + monkeypatch.setattr(SchemaDiscovery, "discover", lambda self: _raise()) + + with pytest.raises(SchemaTranslationExtractionError): + extract_messages() + + +class TestRenderModule: + """The generated module is valid, scannable Python source.""" + + def test_output_is_valid_python(self): + messages = [ + TranslatableMessage("role.display_name:course_admin", "Course Admin"), + TranslatableMessage("role.description:course_admin", "Can manage everything."), + ] + source = render_module(messages) + compile(source, "", "exec") # Raises SyntaxError if malformed. + + def test_strings_with_quotes_and_newlines_are_escaped_safely(self): + """repr()-based rendering must survive content a naive f-string would break on.""" + messages = [TranslatableMessage("role.description:tricky", 'Has "quotes", a \'single\' and\na newline.')] + source = render_module(messages) + compile(source, "", "exec") + assert 'Has "quotes"' in source or "quotes" in source + + def test_empty_message_list_is_still_valid_python(self): + compile(render_module([]), "", "exec") + + def test_output_contains_do_not_edit_warning(self): + assert "Do not edit" in render_module([]) diff --git a/src/openedx_authz/tests/test_commands.py b/src/openedx_authz/tests/test_commands.py index 8d270571..4f772eb5 100644 --- a/src/openedx_authz/tests/test_commands.py +++ b/src/openedx_authz/tests/test_commands.py @@ -7,6 +7,7 @@ from unittest import TestCase from unittest.mock import Mock, patch +import pytest from ddt import data, ddt from django.core.management import call_command from django.core.management.base import CommandError @@ -22,6 +23,7 @@ ) from openedx_authz.constants.roles import LIBRARY_ADMIN from openedx_authz.engine.enforcer import AuthzEnforcer +from openedx_authz.engine.schema.translation import SchemaTranslationExtractionError from openedx_authz.management.commands.load_policies import Command as LoadPoliciesCommand from openedx_authz.tests.test_utils import ( make_action_key, @@ -509,3 +511,34 @@ def test_handle_clear_existing_both_denied(self, mock_confirm, mock_casbin_enfor command._delete_existing_roles.assert_not_called() command._delete_permissions_inheritance.assert_not_called() command.migrate_policies.assert_called_once_with(mock_source_enforcer, mock_target_enforcer) + + +class ExtractSchemaTranslationsCommandTests(TestCase): + """ + Tests for the `extract_schema_translations` Django management command. + + This test class verifies the behavior of the extract_schema_translations + command, including: + - Writing a valid, non-empty generated module to the given --output path + - Surfacing extraction failures as a CommandError + """ + + def test_writes_generated_module_to_output_path(self): + """The command extracts the real schema and writes it to --output.""" + with NamedTemporaryFile(suffix=".py") as output_file: + call_command("extract_schema_translations", output=output_file.name) + + with open(output_file.name, encoding="utf-8") as generated: + source = generated.read() + + assert "pgettext(" in source + assert "Do not edit" in source + compile(source, "", "exec") # Raises SyntaxError if malformed. + + @patch("openedx_authz.management.commands.extract_schema_translations.extract_messages") + def test_extraction_failure_raises_command_error(self, mock_extract_messages): + """An extraction failure surfaces as a CommandError, not a silent skip.""" + mock_extract_messages.side_effect = SchemaTranslationExtractionError("broken schema file") + + with NamedTemporaryFile(suffix=".py") as output_file, pytest.raises(CommandError): + call_command("extract_schema_translations", output=output_file.name)