From 409af2092e5d3edf8d26fa349c40dfa4bb9300b4 Mon Sep 17 00:00:00 2001 From: Robbie Muir Date: Wed, 21 May 2025 22:48:23 +0200 Subject: [PATCH 1/3] Added onion enforcer --- .github/workflows/python-app.yml | 3 + src/directories.py | 3 + src/onion_enforcer.py | 134 +++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 src/directories.py create mode 100644 src/onion_enforcer.py diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 91aa3d6..089f579 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -28,6 +28,9 @@ jobs: python -m pip install --upgrade pip pip install poetry poetry install --no-interaction --no-root --with dev + - name: Check onion architecture + run: | + poetry run python -m src.onion_enforcer - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names diff --git a/src/directories.py b/src/directories.py new file mode 100644 index 0000000..8f6187b --- /dev/null +++ b/src/directories.py @@ -0,0 +1,3 @@ +from pathlib import Path + +root_dir = Path(__file__).parent diff --git a/src/onion_enforcer.py b/src/onion_enforcer.py new file mode 100644 index 0000000..1eda9a7 --- /dev/null +++ b/src/onion_enforcer.py @@ -0,0 +1,134 @@ +from abc import ABC, abstractmethod +from dataclasses import dataclass +from itertools import product +from pathlib import Path +from typing import Union, Optional + +from src.directories import root_dir + +__all__ = [] + + +class OnionEnforcerError(Exception): + def __init__(self, issues: list["OnionIssue"]) -> None: + super().__init__("\n" + "\n".join(map(str, issues))) + self.issues = issues + + +@dataclass +class OnionIssue: + rule: "ProjectStructureRule" + file: Path + line: int + + def __str__(self) -> str: + return f'{self.rule}: File "{self.file}", line {self.line}' + + +@dataclass(frozen=True) +class ProjectStructureRule: + upper: Union["Module", "File", "Package"] + lower: Union["Module", "File"] + + def __str__(self) -> str: + return f"{self.lower} cannot import from {self.upper}" + + def __repr__(self) -> str: + return f"{self.upper} > {self.lower}" + + def find_issue(self) -> Optional[OnionIssue]: + import_names = self.upper.get_import_names() + + if isinstance(self.lower, Module): + file_iter = self.lower.path.rglob("*.py") + else: + file_iter = [self.lower.path] + + for py_file in file_iter: + with py_file.open("r", encoding="utf-8") as f: + for k, line in enumerate(f): + if not (line.startswith("import") or line.startswith("from")): + continue + for a, b in product(["from", "import"], import_names): + if line.startswith(f"{a} {b}"): + return OnionIssue(rule=self, file=py_file, line=k + 1) + return None + + +@dataclass(frozen=True) +class Importable(ABC): + @abstractmethod + def get_import_names(self) -> list[str]: + pass + + +@dataclass(frozen=True) +class Package(Importable): + name: str + + def get_import_names(self) -> list[str]: + return [self.name] + + +@dataclass(frozen=True) +class Module(Importable): + name: str + + def __post_init__(self) -> None: + assert self.path.is_dir() + assert (self.path / "__init__.py").exists() + + @property + def path(self) -> Path: + return root_dir / self.name + + def get_import_names(self) -> list[str]: + return [f"{root_dir.name}.{self.name}", self.name] + + def __gt__(self, other: Union["Module", "File"]) -> ProjectStructureRule: + assert isinstance(other, (Module, File)) + return ProjectStructureRule(self, other) + + def __lt__(self, other: Union["Module", "File", Package]) -> ProjectStructureRule: + assert isinstance(other, (Module, File, Package)) + return ProjectStructureRule(other, self) + + +@dataclass(frozen=True) +class File(Importable): + name: str + + def __post_init__(self) -> None: + assert self.path.is_file() + assert not self.name.endswith(".py") + + @property + def path(self) -> Path: + return root_dir / (self.name + ".py") + + def get_import_names(self) -> list[str]: + return [f"{root_dir.name}.{self.name}", self.name] + + +def check_repo() -> None: + module_hierarchy = [Module("app"), Module("engine"), Module("models"), Module("tools")] + rules: list[ProjectStructureRule] = [] + + for k in range(len(module_hierarchy) - 1): + higher = module_hierarchy[k] + for lower in module_hierarchy[k + 1 :]: + rules.append(lower < higher) + + for m in module_hierarchy: + rules.append(m > File("directories")) + rules.append(m < File("onion_enforcer")) + + issues = [i for i in [r.find_issue() for r in rules] if i is not None] + if len(issues): + for i in issues: + i.rule.find_issue() + raise OnionEnforcerError(issues) + + +if __name__ == "__main__": + check_repo() From 81523e2dc26ba91983572f1a8d228b0f1d5d0b4c Mon Sep 17 00:00:00 2001 From: Robbie Muir Date: Wed, 21 May 2025 23:19:10 +0200 Subject: [PATCH 2/3] minor changes --- src/onion_enforcer.py | 2 -- tests/test_onion_enforcer.py | 13 +++++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 tests/test_onion_enforcer.py diff --git a/src/onion_enforcer.py b/src/onion_enforcer.py index 1eda9a7..85ee1d9 100644 --- a/src/onion_enforcer.py +++ b/src/onion_enforcer.py @@ -125,8 +125,6 @@ def check_repo() -> None: issues = [i for i in [r.find_issue() for r in rules] if i is not None] if len(issues): - for i in issues: - i.rule.find_issue() raise OnionEnforcerError(issues) diff --git a/tests/test_onion_enforcer.py b/tests/test_onion_enforcer.py new file mode 100644 index 0000000..6070d42 --- /dev/null +++ b/tests/test_onion_enforcer.py @@ -0,0 +1,13 @@ +from itertools import count +from unittest import TestCase + +from src.models.assets import AssetRepo, AssetType, AssetInfo +from src.models.ids import BusId, AssetId +from src.models.player import PlayerId + +from src.onion_enforcer import check_repo # noqa + + +class TestOnionEnforcer(TestCase): + def test_onion_enforcer(self) -> None: + check_repo() From 8582cf075a69706f53813e7bd5fd58407c54aaed Mon Sep 17 00:00:00 2001 From: Robbie Muir Date: Sat, 24 May 2025 14:38:43 +0200 Subject: [PATCH 3/3] minor changes --- tests/test_onion_enforcer.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/test_onion_enforcer.py b/tests/test_onion_enforcer.py index 6070d42..aeafd2f 100644 --- a/tests/test_onion_enforcer.py +++ b/tests/test_onion_enforcer.py @@ -1,10 +1,5 @@ -from itertools import count from unittest import TestCase -from src.models.assets import AssetRepo, AssetType, AssetInfo -from src.models.ids import BusId, AssetId -from src.models.player import PlayerId - from src.onion_enforcer import check_repo # noqa