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 d5789ce6..37d48910 100644 --- a/Makefile +++ b/Makefile @@ -75,6 +75,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/pyproject.toml b/pyproject.toml index 55bdccb2..b578c344 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ test-base = [ "code-annotations", "coverage", "ddt", + "polib", "pytest-cov", "pytest-django", ] diff --git a/src/openedx_authz/engine/schema/translation.py b/src/openedx_authz/engine/schema/translation.py new file mode 100644 index 00000000..28eb8cc2 --- /dev/null +++ b/src/openedx_authz/engine/schema/translation.py @@ -0,0 +1,257 @@ +"""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 the richer typed documents +those steps build, so it stays usable (and testable) against ad hoc or malformed +schema directories, not just a fully valid one. It does, however, resolve conflicts +between files the same way ``engine.schema.compilation`` does: by each document's +``priority`` (higher wins; identical content merges; a disagreement at equal +priority is an error), so the generated catalog reflects the same winner the +compiled schema would use at runtime. Re-deriving that one rule here (rather than +depending on the loader/compiler) is what keeps this module usable before, or +independently of, that pipeline. + +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 first ``pgettext()`` argument is gettext's ``msgctxt``: it +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 schema catalog +API looks up a field's source-language value directly 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 collections.abc import Callable +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 +''' + +# gettext's own internal glue byte between msgctxt and msgid. If it ends up inside +# either half, tools downstream of extraction (msguniq, as part of `makemessages`) +# fail outright trying to parse the result back apart, so it can never appear in +# generated content. +GETTEXT_CONTEXT_SEPARATOR = "\x04" + + +@dataclass(frozen=True) +class TranslatableMessage: + """One ``pgettext(context, message)`` pair extracted from the schema. + + Attributes: + context: gettext's ``msgctxt``. Disambiguates otherwise-identical + messages, formatted as ``"{field_kind}.{field_name}:{stable_id}"``, + e.g. ``"permission.display_name:courses.view_course"``. + message: gettext's ``msgid``, the source-language (English) string to + translate. + """ + + context: str + message: str + + +class SchemaTranslationExtractionError(Exception): + """A schema resource could not be read, parsed, or resolved for extraction.""" + + +@dataclass(frozen=True) +class _Candidate: + """A :class:`TranslatableMessage` plus the provenance needed to resolve conflicts.""" + + message: TranslatableMessage + priority: int + source: str + + +def _permission_stable_id(entry: dict) -> str: + """Build a permission's stable id from its namespace and name, both required.""" + namespace = entry.get("namespace") + name = entry.get("name") + if not namespace or not name: + return "" + return f"{namespace}.{name}" + + +# Each entry is (the document key holding this section's list, the field_kind used +# in the generated context string, a function pulling this entry's stable id out of +# its raw mapping). All four sections share the same display_name/description +# extraction logic (_messages_for_entry); only these three things differ between +# them, so a data table covers it without a class per section. +_SECTIONS: tuple[tuple[str, str, Callable[[dict], str]], ...] = ( + ("permission_categories", "permission_category", lambda entry: str(entry.get("id") or "")), + ("permissions", "permission", _permission_stable_id), + ("roles", "role", lambda entry: str(entry.get("id") or "")), + ("role_extensions", "role_extension", lambda entry: str(entry.get("role") or "")), +) + + +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, it + wasn't valid YAML, a section wasn't shaped as the schema requires, an + entry was missing its identifying field, a translatable string + collided with gettext's own context separator, or two documents + disagreed on the same field at equal priority. A translation build + must fail loudly here (ADR 0020) rather than silently produce a + catalog that doesn't match what the compiled schema would serve. + """ + if resources is None: + try: + resources = SchemaDiscovery().discover() + except SchemaDiscoveryError as exc: + raise SchemaTranslationExtractionError(str(exc)) from exc + + resolved: dict[str, _Candidate] = {} + for resource in resources: + for candidate in _extract_from_resource(resource): + existing = resolved.get(candidate.message.context) + resolved[candidate.message.context] = ( + candidate if existing is None else _resolve_conflict(existing, candidate) + ) + + return sorted((candidate.message for candidate in resolved.values()), key=lambda m: m.context) + + +def _resolve_conflict(existing: _Candidate, incoming: _Candidate) -> _Candidate: + """Resolve two competing messages for the same context, the way compilation.py does. + + Identical messages merge (both files contributed the same text); otherwise + higher ``priority`` wins; equal priority with disagreeing text is unresolvable. + """ + if existing.message == incoming.message: + return existing + if existing.priority == incoming.priority: + raise SchemaTranslationExtractionError( + f"Conflicting translation for {existing.message.context!r} at equal priority " + f"{existing.priority} ({existing.source} vs {incoming.source}): " + f"{existing.message.message!r} vs {incoming.message.message!r}." + ) + return existing if existing.priority > incoming.priority else incoming + + +def _extract_from_resource(resource: DiscoveredResource) -> list[_Candidate]: + """Parse one resource's YAML and pull its translatable fields out.""" + source = f"{resource.package}:{resource.resource_path}" + 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 {source}: {exc}") from exc + + if document is None: + return [] + if not isinstance(document, dict): + raise SchemaTranslationExtractionError( + f"Schema file {source} must be a mapping at the top level, got {type(document).__name__}." + ) + + # Required per authz-schema-v1.json; enforcing its presence is validation's job, + # not extraction's (same split loading.py makes), so a missing priority reads as + # 0, the lowest precedence. A malformed one can't be resolved at all, so that + # raises now rather than producing a nonsensical conflict winner later. + try: + priority = int(document.get("priority", 0)) + except (TypeError, ValueError) as exc: + raise SchemaTranslationExtractionError( + f"{source}: 'priority' must be an integer, got {document.get('priority')!r}." + ) from exc + + candidates: list[_Candidate] = [] + for section_key, field_kind, stable_id_of in _SECTIONS: + entries = document.get(section_key) or [] + if not isinstance(entries, list): + raise SchemaTranslationExtractionError( + f"{source}: {section_key!r} must be a list, got {type(entries).__name__}." + ) + for entry in entries: + if not isinstance(entry, dict): + raise SchemaTranslationExtractionError( + f"{source}: an entry in {section_key!r} must be a mapping, got {type(entry).__name__}." + ) + stable_id = stable_id_of(entry) + if not stable_id: + raise SchemaTranslationExtractionError( + f"{source}: a {field_kind} entry in {section_key!r} is missing its identifying field(s)." + ) + for message in _messages_for_entry(source, field_kind, stable_id, entry): + candidates.append(_Candidate(message=message, priority=priority, source=source)) + + return candidates + + +def _messages_for_entry(source: str, field_kind: str, stable_id: str, entry: dict) -> list[TranslatableMessage]: + """Build the display_name/description messages for one schema entry. + + A field that's absent (common on a ``role_extension``, which only overrides + what it mentions, but just as possible on any other entry) is skipped rather + than emitting an empty message: requiring these fields is validation's job, + not extraction's, so there's nothing useful to translate either way. + """ + messages = [] + for field_name in ("display_name", "description"): + value = entry.get(field_name) + if value is None: + continue + context = f"{field_kind}.{field_name}:{stable_id}" + message = str(value) + for part, label in ((context, "context"), (message, "text")): + if GETTEXT_CONTEXT_SEPARATOR in part: + raise SchemaTranslationExtractionError( + f"{source}: the {label} for {context!r} contains U+0004, which gettext reserves as its " + "own context separator and cannot appear in translatable content." + ) + messages.append(TranslatableMessage(context=context, message=message)) + 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..29798a73 --- /dev/null +++ b/src/openedx_authz/tests/schema/test_translation.py @@ -0,0 +1,427 @@ +"""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. +* ``TestSectionShapeValidation`` -- a section or entry that isn't a list of + mappings is a hard error, not an ``AttributeError``. +* ``TestStableIdValidation`` -- an entry missing its identifying field(s) is a + hard error, since it can't form a unique context. +* ``TestPriorityConflictResolution`` -- conflicts between documents resolve by + ``priority`` the same way ``engine.schema.compilation`` does. +* ``TestGettextContextSeparatorGuard`` -- content colliding with gettext's own + msgctxt/msgid separator byte 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")]) == [] + + def test_missing_display_name_or_description_is_skipped_not_emitted_empty(self): + """A missing field is skipped outright: requiring it is validation's job, not extraction's.""" + messages = extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_admin + display_name: Admin +""")]) + assert messages == [TranslatableMessage("role.display_name:course_admin", "Admin")] + + +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 TestSectionShapeValidation: + """A section or entry that isn't a list of mappings is a hard error, not an AttributeError.""" + + @pytest.mark.parametrize("section", ["permission_categories", "permissions", "roles", "role_extensions"]) + def test_section_not_a_list_raises(self, section): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(f""" +schema_version: "1.0" +priority: 100 +{section}: + not: a_list +""")]) + + @pytest.mark.parametrize("section", ["permission_categories", "permissions", "roles", "role_extensions"]) + def test_entry_not_a_mapping_raises(self, section): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(f""" +schema_version: "1.0" +priority: 100 +{section}: + - just_a_string +""")]) + + +class TestStableIdValidation: + """An entry missing its identifying field(s) can't form a unique context.""" + + def test_category_without_id_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +permission_categories: + - display_name: Course content +""")]) + + def test_permission_without_namespace_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +permissions: + - name: view_course + display_name: View course +""")]) + + def test_permission_without_name_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +permissions: + - namespace: courses + display_name: View course +""")]) + + def test_role_without_id_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - display_name: Course Admin +""")]) + + def test_role_extension_without_role_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +role_extensions: + - display_name: Course author +""")]) + + +class TestPriorityConflictResolution: + """Conflicts between documents resolve by priority the same way compilation.py's do.""" + + def _role(self, priority, display_name: str) -> FakeResource: + priority_line = "" if priority is None else f"priority: {priority}\n" + return _resource(f""" +schema_version: "1.0" +{priority_line}roles: + - id: course_admin + display_name: {display_name} +""") + + def test_identical_definitions_across_files_merge_without_error(self): + messages = extract_messages([self._role(100, "Admin"), self._role(200, "Admin")]) + assert [m.message for m in messages if m.context == "role.display_name:course_admin"] == ["Admin"] + + def test_higher_priority_wins_regardless_of_file_order(self): + low_first = extract_messages([self._role(100, "Low"), self._role(200, "High")]) + high_first = extract_messages([self._role(200, "High"), self._role(100, "Low")]) + + assert low_first == high_first + assert [m.message for m in low_first if m.context == "role.display_name:course_admin"] == ["High"] + + def test_equal_priority_with_disagreeing_text_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([self._role(100, "Admin"), self._role(100, "Administrator")]) + + def test_malformed_priority_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: not-a-number +roles: + - id: course_admin + display_name: Admin +""")]) + + def test_missing_priority_defaults_to_lowest(self): + messages = extract_messages([self._role(None, "Low"), self._role(1, "High")]) + assert [m.message for m in messages if m.context == "role.display_name:course_admin"] == ["High"] + + +class TestGettextContextSeparatorGuard: + """gettext's own msgctxt/msgid glue byte (U+0004) can't appear in generated content.""" + + def test_separator_in_message_text_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: course_admin + display_name: "bad\\x04char" +""")]) + + def test_separator_in_stable_id_raises(self): + with pytest.raises(SchemaTranslationExtractionError): + extract_messages([_resource(""" +schema_version: "1.0" +priority: 100 +roles: + - id: "bad\\x04id" + display_name: Admin +""")]) + + +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..bcf2aeab 100644 --- a/src/openedx_authz/tests/test_commands.py +++ b/src/openedx_authz/tests/test_commands.py @@ -3,10 +3,14 @@ """ import io -from tempfile import NamedTemporaryFile -from unittest import TestCase +import os +import shutil +from tempfile import NamedTemporaryFile, TemporaryDirectory +from unittest import TestCase, skipUnless from unittest.mock import Mock, patch +import polib +import pytest from ddt import data, ddt from django.core.management import call_command from django.core.management.base import CommandError @@ -22,6 +26,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 +514,66 @@ 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) + + +@skipUnless( + shutil.which("msguniq"), + "requires the system gettext tools (xgettext/msguniq) that makemessages shells out to", +) +class ExtractSchemaTranslationsEndToEndTests(TestCase): + """ + End to end: the generated module is real, scannable source for Django's + `makemessages`, the same tool `i18n_tool extract` wraps around. Skipped + wherever the system gettext tools it shells out to aren't installed. + """ + + def test_generated_module_produces_expected_catalog_entry(self): + """extract_schema_translations -> makemessages yields the real catalog entry.""" + with TemporaryDirectory() as scan_root: + os.makedirs(os.path.join(scan_root, "locale")) + output_path = os.path.join(scan_root, "generated_translations.py") + call_command("extract_schema_translations", output=output_path) + + cwd = os.getcwd() + os.chdir(scan_root) + try: + call_command("makemessages", locale=["en"], domain="django", verbosity=0) + finally: + os.chdir(cwd) + + catalog = polib.pofile(os.path.join(scan_root, "locale", "en", "LC_MESSAGES", "django.po")) + + entry = catalog.find("Course Admin", msgctxt="role.display_name:course_admin") + assert entry is not None + assert entry.msgid == "Course Admin" diff --git a/uv.lock b/uv.lock index cd6cc8f3..594bff74 100644 --- a/uv.lock +++ b/uv.lock @@ -1533,6 +1533,7 @@ dev = [ { name = "doc8" }, { name = "edx-i18n-tools" }, { name = "edx-lint" }, + { name = "polib" }, { name = "pydocstyle" }, { name = "pytest-cov" }, { name = "pytest-django" }, @@ -1551,6 +1552,7 @@ doc = [ { name = "ddt" }, { name = "django" }, { name = "doc8" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, { name = "sphinx" }, @@ -1564,6 +1566,7 @@ quality = [ { name = "ddt" }, { name = "django" }, { name = "edx-lint" }, + { name = "polib" }, { name = "pydocstyle" }, { name = "pytest-cov" }, { name = "pytest-django" }, @@ -1574,6 +1577,7 @@ test = [ { name = "coverage" }, { name = "ddt" }, { name = "django" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, ] @@ -1581,6 +1585,7 @@ test-base = [ { name = "code-annotations" }, { name = "coverage" }, { name = "ddt" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, ] @@ -1620,6 +1625,7 @@ dev = [ { name = "doc8" }, { name = "edx-i18n-tools" }, { name = "edx-lint" }, + { name = "polib" }, { name = "pydocstyle" }, { name = "pytest-cov" }, { name = "pytest-django" }, @@ -1638,6 +1644,7 @@ doc = [ { name = "ddt" }, { name = "django", specifier = ">=5.2,<6.0" }, { name = "doc8" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, { name = "sphinx" }, @@ -1651,6 +1658,7 @@ quality = [ { name = "ddt" }, { name = "django", specifier = ">=5.2,<6.0" }, { name = "edx-lint" }, + { name = "polib" }, { name = "pydocstyle" }, { name = "pytest-cov" }, { name = "pytest-django" }, @@ -1661,6 +1669,7 @@ test = [ { name = "coverage" }, { name = "ddt" }, { name = "django", specifier = ">=5.2,<6.0" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, ] @@ -1668,6 +1677,7 @@ test-base = [ { name = "code-annotations" }, { name = "coverage" }, { name = "ddt" }, + { name = "polib" }, { name = "pytest-cov" }, { name = "pytest-django" }, ]