From da57e2f942c60b49333e89caa8db0d181b10697f Mon Sep 17 00:00:00 2001 From: ufedaseyeuconsultant Date: Fri, 9 Oct 2026 20:32:37 +0300 Subject: [PATCH 1/2] build: add an isolated_apps import-linter contract for openedx_catalog openedx-platform's own import-linter config already treats openedx_catalog (and openedx_learning) as an isolated app, reachable from outside only through its api/models_api/data/tests modules, which is how a foreign-key import into openedx_catalog.models (instead of models_api) got caught on the platform's CI and not here: this repo's own .importlinter only layers top-level packages, it says nothing about which modules within a package may be reached from outside it. Adds an equivalent "isolated_apps" contract type, scoped to openedx_catalog only for now; extending it to openedx_content, openedx_learning, or openedx_tagging would surface pre-existing violations (confirmed by testing the broader scope and reverting), out of scope for this change. IsolatedAppsContract itself is ported verbatim from openedx-platform's openedx.testing.importlinter.isolated_apps_contract (this library can't depend on openedx-platform), with one change: self.isolated_apps and self.allowed_modules are copied through list() before iterating, working around a pylint false positive (E1133) caused by pylint not understanding that Contract._populate_fields() replaces these class-level Field descriptors with their parsed list values at the instance level. Co-Authored-By: Claude Sonnet 5 --- .importlinter | 21 ++++++++ test_utils/isolated_apps_contract.py | 72 ++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 test_utils/isolated_apps_contract.py diff --git a/.importlinter b/.importlinter index 98997d1a2..f671f632a 100644 --- a/.importlinter +++ b/.importlinter @@ -11,6 +11,8 @@ root_packages = openedx_tagging openedx_django_lib openedx_core +contract_types = + isolated_apps: test_utils.isolated_apps_contract.IsolatedAppsContract # This is layering between our top-level src folders (mostly Django apps). # This should be updated as new Django apps are added. @@ -76,3 +78,22 @@ layers= # to create Learning Packages and manage the draft and publish states for # various types of content. openedx_content.applets.publishing + +# Mirrors openedx-platform's own isolated-apps contract (openedx.testing.importlinter. +# isolated_apps_contract.IsolatedAppsContract, see test_utils/isolated_apps_contract.py +# for provenance): code outside an isolated app may only reach its public surface +# (api, models_api, data, tests), never a private internal module. src_layering above +# only says which top-level packages may depend on which; it says nothing about which +# modules *within* a package may be reached from outside it, which is what let a +# foreign-key import reach into openedx_catalog.models directly instead of +# openedx_catalog.models_api (see src/openedx_learning/applets/cbe/models/criteria.py). +[importlinter:contract:isolated_apps] +name = "code outside an isolated app only reaches its public API" +type = isolated_apps +isolated_apps = + openedx_catalog +allowed_modules = + api + models_api + data + tests diff --git a/test_utils/isolated_apps_contract.py b/test_utils/isolated_apps_contract.py new file mode 100644 index 000000000..b28b990a1 --- /dev/null +++ b/test_utils/isolated_apps_contract.py @@ -0,0 +1,72 @@ +""" +An import-linter contract that flags imports of an isolated app's private modules. + +Ported from openedx-platform's ``openedx.testing.importlinter.isolated_apps_contract`` +(as of commit 7e3ed6881c048fb93624100df25380e76eabbcc9), since this library can't +depend on openedx-platform. Kept dependency-free beyond ``import-linter`` itself so +it can be copied back and forth between the two without drifting into either one's +internals. +""" + +from importlinter import Contract, ContractCheck, fields, output + + +class IsolatedAppsContract(Contract): + """ + Contract that defines most of an 'app' (python package) as private, and + ensures that python code outside of the package doesn't import anything + other than the public API defined in the package's `api.py` file. + """ + isolated_apps = fields.ListField(subfield=fields.StringField()) + # List of allowed modules (like ["api", "urls"] to allow "import x.api") + allowed_modules = fields.ListField(subfield=fields.StringField()) + + def check(self, graph, verbose): + """Find every import of an isolated app's modules from outside that app, skipping allowed modules.""" + forbidden_imports_found = [] + # Contract._populate_fields() replaces these class-level Field descriptors with + # their parsed list[str] values on the instance; pylint can't see that and still + # treats them as ListField (not iterable), hence copying through list() here. + isolated_apps: list[str] = list(self.isolated_apps) + allowed_modules: list[str] = list(self.allowed_modules) + + for package in isolated_apps: + output.verbose_print( + verbose, + f"Getting import details for anything that imports {package}..." + ) + modules = graph.find_descendants(package) + for module in modules: + # We have a list of modules like "api.py" that *are* allowed to be imported from anywhere: + for allowed_module in allowed_modules: + if module.endswith(f".{allowed_module}"): + break + else: + # See who is importing this: + importers = graph.find_modules_that_directly_import(module) + for importer in importers: + if importer.startswith(package): + continue # Ignore imports from within the same package + # Add this import to our list of contract violations: + import_details = graph.get_import_details(importer=importer, imported=module) + for import_detail in import_details: + forbidden_imports_found.append({**import_detail, "package": package}) + + return ContractCheck( + kept=not bool(forbidden_imports_found), + metadata={ + 'forbidden_imports_found': forbidden_imports_found, + } + ) + + def render_broken_contract(self, check): + """Print each forbidden import's location and source line.""" + for details in check.metadata['forbidden_imports_found']: + package = details['package'] + importer = details['importer'] + line_number = details['line_number'] + line_contents = details['line_contents'] + output.print_error(f'{importer}:{line_number}: imported from non-public API of {package}:') + output.indent_cursor() + output.print_error(line_contents) + output.new_line() From fcd44fa1826f837c80f09150279019ca6f584716 Mon Sep 17 00:00:00 2001 From: ufedaseyeuconsultant Date: Sat, 10 Oct 2026 01:31:43 +0300 Subject: [PATCH 2/2] fix: trim isolated_apps_contract.py and its config comment per review Drops the commit hash and the "copied back and forth" sentence from the module docstring (nothing copies this file back to openedx-platform, so that process doesn't exist), removes the two method docstrings now that the file is closer to openedx-platform's original, and cuts the list() workaround's annotations and three-line comment down to one line (the annotations did nothing: mypy skips the untyped check() method's body, and pylint passes without them either way). Also shortens the comment above the new contract in .importlinter: drops the clause about how the bad import happened, since that history belongs in #853 and #860, not in the config. Co-Authored-By: Claude Sonnet 5 --- .importlinter | 11 +++-------- test_utils/isolated_apps_contract.py | 17 +++++------------ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/.importlinter b/.importlinter index f671f632a..5aad15ed9 100644 --- a/.importlinter +++ b/.importlinter @@ -79,14 +79,9 @@ layers= # various types of content. openedx_content.applets.publishing -# Mirrors openedx-platform's own isolated-apps contract (openedx.testing.importlinter. -# isolated_apps_contract.IsolatedAppsContract, see test_utils/isolated_apps_contract.py -# for provenance): code outside an isolated app may only reach its public surface -# (api, models_api, data, tests), never a private internal module. src_layering above -# only says which top-level packages may depend on which; it says nothing about which -# modules *within* a package may be reached from outside it, which is what let a -# foreign-key import reach into openedx_catalog.models directly instead of -# openedx_catalog.models_api (see src/openedx_learning/applets/cbe/models/criteria.py). +# Mirrors openedx-platform's isolated-apps contract (see test_utils/isolated_apps_contract.py): +# code outside an isolated app may only import its public modules (api, models_api, data, +# tests). src_layering above only controls which top-level packages may depend on which. [importlinter:contract:isolated_apps] name = "code outside an isolated app only reaches its public API" type = isolated_apps diff --git a/test_utils/isolated_apps_contract.py b/test_utils/isolated_apps_contract.py index b28b990a1..6b8620c86 100644 --- a/test_utils/isolated_apps_contract.py +++ b/test_utils/isolated_apps_contract.py @@ -1,11 +1,8 @@ """ An import-linter contract that flags imports of an isolated app's private modules. -Ported from openedx-platform's ``openedx.testing.importlinter.isolated_apps_contract`` -(as of commit 7e3ed6881c048fb93624100df25380e76eabbcc9), since this library can't -depend on openedx-platform. Kept dependency-free beyond ``import-linter`` itself so -it can be copied back and forth between the two without drifting into either one's -internals. +Ported from openedx-platform's ``openedx.testing.importlinter.isolated_apps_contract``, +since this library can't depend on openedx-platform. """ from importlinter import Contract, ContractCheck, fields, output @@ -22,13 +19,10 @@ class IsolatedAppsContract(Contract): allowed_modules = fields.ListField(subfield=fields.StringField()) def check(self, graph, verbose): - """Find every import of an isolated app's modules from outside that app, skipping allowed modules.""" forbidden_imports_found = [] - # Contract._populate_fields() replaces these class-level Field descriptors with - # their parsed list[str] values on the instance; pylint can't see that and still - # treats them as ListField (not iterable), hence copying through list() here. - isolated_apps: list[str] = list(self.isolated_apps) - allowed_modules: list[str] = list(self.allowed_modules) + # pylint can't tell import-linter replaces these Field descriptors with lists. + isolated_apps = list(self.isolated_apps) + allowed_modules = list(self.allowed_modules) for package in isolated_apps: output.verbose_print( @@ -60,7 +54,6 @@ def check(self, graph, verbose): ) def render_broken_contract(self, check): - """Print each forbidden import's location and source line.""" for details in check.metadata['forbidden_imports_found']: package = details['package'] importer = details['importer']