diff --git a/.importlinter b/.importlinter index 98997d1a2..5aad15ed9 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,17 @@ 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 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 +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..6b8620c86 --- /dev/null +++ b/test_utils/isolated_apps_contract.py @@ -0,0 +1,65 @@ +""" +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``, +since this library can't depend on openedx-platform. +""" + +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): + forbidden_imports_found = [] + # 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( + 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): + 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()