From b48b4db24ec0f04e950b9dcdb34ecb61beb01e60 Mon Sep 17 00:00:00 2001 From: bnbong Date: Sat, 2 May 2026 15:51:30 +0900 Subject: [PATCH 1/2] [FEAT] Support architecture presets in dynamic project generation --- docs/en/reference/faq.md | 5 +- docs/en/reference/preset-feature-matrix.md | 78 +++++++ mkdocs.yml | 1 + .../backend/project_builder/__init__.py | 3 + .../backend/project_builder/preset_layout.py | 188 ++++++++++++++++ src/fastapi_fastkit/cli.py | 66 ++++-- tests/test_backends/test_preset_layout.py | 205 ++++++++++++++++++ .../test_cli_interactive_integration.py | 195 +++++++++++++++++ 8 files changed, 717 insertions(+), 24 deletions(-) create mode 100644 docs/en/reference/preset-feature-matrix.md create mode 100644 src/fastapi_fastkit/backend/project_builder/preset_layout.py create mode 100644 tests/test_backends/test_preset_layout.py diff --git a/docs/en/reference/faq.md b/docs/en/reference/faq.md index 63b7da2..e63f682 100644 --- a/docs/en/reference/faq.md +++ b/docs/en/reference/faq.md @@ -182,7 +182,10 @@ Interactive mode walks you through these steps in order: 1. **Project information** — name, author, email, description. 2. **Architecture preset** — picks the project layout. The recommended - default is `domain-starter`; press Enter to accept it. + default is `domain-starter`; press Enter to accept it. See the + [preset / feature matrix](preset-feature-matrix.md) for the exact + layout each preset produces and which feature combinations require + manual wiring. 3. **Feature selections** — database, authentication, background tasks, caching, monitoring, testing, utilities, deployment. 4. **Package manager and custom packages** — pip / uv / pdm / poetry, diff --git a/docs/en/reference/preset-feature-matrix.md b/docs/en/reference/preset-feature-matrix.md new file mode 100644 index 0000000..ebb3778 --- /dev/null +++ b/docs/en/reference/preset-feature-matrix.md @@ -0,0 +1,78 @@ +# Architecture preset / feature matrix + +Interactive `fastkit init --interactive` asks for an **architecture preset** +([issue #44](https://github.com/bnbong/FastAPI-fastkit/issues/44)) before +collecting feature selections. The preset shapes the generated project's +layout — different presets ship a different base template and put generated +config files in different locations so they sit next to the existing +structure rather than in a parallel `src/config/` tree. + +This page is the source of truth for what each preset does, where files +land, and which feature combinations require manual wiring. + +## Preset → base template + +| Preset | Base template | Description | +|---|---|---| +| `minimal` | `fastapi-empty` | Smallest viable FastAPI app — placeholder `main.py` is regenerated from your feature selections. | +| `single-module` | `fastapi-single-module` | Single-file FastAPI app — `main.py` is regenerated. | +| `classic-layered` | `fastapi-default` | Layered split (`api/routes`, `crud`, `schemas`, `core`). Shipped `main.py` is preserved. | +| `domain-starter` | `fastapi-domain-starter` | Domain-oriented (`src/app/domains//`). Shipped `main.py` is preserved. **Recommended default.** | + +## Generated file locations + +| Preset | `main.py` overlay | Database config target | Auth config target | +|---|---|---|---| +| `minimal` | regenerated at `src/main.py` | `src/config/database.py` | `src/config/auth.py` | +| `single-module` | regenerated at `src/main.py` | `src/config/database.py` | `src/config/auth.py` | +| `classic-layered` | preserved (template-shipped) | `src/core/database.py` | `src/core/auth.py` | +| `domain-starter` | preserved (template-shipped) | `src/app/core/database.py` | `src/app/core/auth.py` | + +## Database / auth feature support per preset + +These features are supported across **every** preset — the package install +always succeeds; the difference is whether the dynamic `main.py` overlay +also wires them up automatically. + +| Feature | `minimal` / `single-module` | `classic-layered` / `domain-starter` | +|---|---|---| +| **Database** (PostgreSQL, MySQL, SQLite, MongoDB) | Generates the config module **and** stubs `await init_db()` calls in the regenerated `main.py`. | Generates the config module at the preset's path. The shipped `main.py` is **preserved**, so wire `get_db()` into routers manually. | +| **Authentication** (JWT, FastAPI-Users, OAuth2, Session-based) | Generates the auth config module. JWT also imports `HTTPBearer` in the regenerated `main.py`. | Generates the auth config module at the preset's path. No imports added to `main.py` — wire dependencies manually. | +| **Background tasks** (Celery, Dramatiq) | Packages installed; no main.py overlay today. | Same. | +| **Caching** (Redis) | Packages installed; no main.py overlay today. | Same. | +| **CORS** (utility) | `CORSMiddleware` added to the regenerated `main.py` with `allow_origins=['*']`. | **Already wired** in the shipped `main.py` (conditional on `settings.all_cors_origins`). Activate by setting `BACKEND_CORS_ORIGINS` in `.env` — no code edits required. | +| **Testing** (Basic / Coverage / Advanced) | `pytest.ini` is generated at the project root. | Same. | +| **Deployment** (Docker, docker-compose) | `Dockerfile` and/or `docker-compose.yml` written at the project root. | Same. | + +## When you'll see a "Preset compatibility" warning + +For presets that **preserve the shipped `main.py`** (`classic-layered`, +`domain-starter`), some feature selections won't be auto-wired into the +app. The CLI surfaces a one-shot warning at the end of generation listing +which selections need manual wiring: + +| Selected feature | Triggers a warning under `classic-layered` / `domain-starter`? | +|---|---| +| `CORS` (utility) | ❌ — already wired in the shipped `main.py`. Just populate `BACKEND_CORS_ORIGINS` in `.env`. | +| `Rate-Limiting` (utility) | ✅ — `slowapi` limiter setup is not added | +| `Prometheus` (monitoring) | ✅ — `Instrumentator().instrument(app)` is not called | +| Any database / auth selection | ⚠️ — config files are generated, but you must `Depends()` them into your routers | + +For `minimal` and `single-module` presets the dynamic `main.py` overlay +handles CORS, rate-limiting, and Prometheus instrumentation automatically; +no warnings fire. + +## Unsupported combinations (stay safe) + +The strategist deliberately **does not** attempt to splice generated code +into a template-shipped `main.py`. Doing so would risk producing broken +imports or duplicating routers. The contract is: + +- Selected packages are always installed (so `pip freeze` matches the + user's intent). +- Generated config modules always land at the preset-appropriate path. +- For preserve-main presets, the user is told which selections still need + manual wiring instead of getting silently broken code. + +If you need full auto-wiring of every feature, pick `minimal` or +`single-module` — they regenerate `main.py` from feature flags. diff --git a/mkdocs.yml b/mkdocs.yml index a605f37..8ad37a8 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -140,6 +140,7 @@ nav: - Translation Guide: contributing/translation-guide.md - Reference: - FAQ: reference/faq.md + - Architecture Preset Matrix: reference/preset-feature-matrix.md - Template Quality Assurance: reference/template-quality-assurance.md - Changelog: changelog.md diff --git a/src/fastapi_fastkit/backend/project_builder/__init__.py b/src/fastapi_fastkit/backend/project_builder/__init__.py index 4c15734..c493abf 100644 --- a/src/fastapi_fastkit/backend/project_builder/__init__.py +++ b/src/fastapi_fastkit/backend/project_builder/__init__.py @@ -10,8 +10,11 @@ # -------------------------------------------------------------------------- from .config_generator import DynamicConfigGenerator from .dependency_collector import DependencyCollector +from .preset_layout import PresetLayoutStrategist, PresetProfile __all__ = [ "DependencyCollector", "DynamicConfigGenerator", + "PresetLayoutStrategist", + "PresetProfile", ] diff --git a/src/fastapi_fastkit/backend/project_builder/preset_layout.py b/src/fastapi_fastkit/backend/project_builder/preset_layout.py new file mode 100644 index 0000000..eaea1d6 --- /dev/null +++ b/src/fastapi_fastkit/backend/project_builder/preset_layout.py @@ -0,0 +1,188 @@ +# -------------------------------------------------------------------------- +# Architecture-preset layout strategy for interactive project generation. +# +# Maps each architecture preset (issue #44) to the actual generation +# decisions interactive ``init`` makes: +# +# - which template ships as the base scaffold, +# - whether the dynamic ``main.py`` overlay should overwrite the shipped one, +# - where database / auth config files should land so they sit next to the +# template's existing structure rather than in a parallel ``src/config``, +# - and which feature combinations need a "you must wire this up manually" +# warning because the dynamic ``main.py`` overlay isn't applied. +# +# Keeping every preset's layout knowledge in one place lets the CLI flow stay +# linear ("ask the strategist where to write the file") instead of growing a +# branching maze of preset-specific if/else blocks. +# +# @author bnbong bbbong9@gmail.com +# -------------------------------------------------------------------------- +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Dict, List, Tuple + +# Canonical preset id used when a caller doesn't supply one. Picked to +# preserve pre-#45 behaviour: interactive ``init`` historically deployed +# ``fastapi-empty`` and regenerated ``src/main.py`` from feature flags. +_FALLBACK_PRESET_ID: str = "minimal" + + +@dataclass(frozen=True) +class PresetProfile: + """Per-preset generation decisions. Treat as a value object.""" + + preset_id: str + base_template: str + regenerate_main: bool + main_py_relpath: str + db_config_relpath: str + auth_config_relpath: str + # Hint shown when ``regenerate_main`` is False and the user picked a + # feature whose dynamic main.py overlay won't run. Empty string means + # "no special note for this preset". + manual_wiring_note: str = "" + extra_warning_targets: Tuple[str, ...] = field(default_factory=tuple) + + +_PRESET_PROFILES: Dict[str, PresetProfile] = { + "minimal": PresetProfile( + preset_id="minimal", + base_template="fastapi-empty", + regenerate_main=True, + main_py_relpath="src/main.py", + db_config_relpath="src/config/database.py", + auth_config_relpath="src/config/auth.py", + ), + "single-module": PresetProfile( + preset_id="single-module", + base_template="fastapi-single-module", + regenerate_main=True, + main_py_relpath="src/main.py", + db_config_relpath="src/config/database.py", + auth_config_relpath="src/config/auth.py", + ), + "classic-layered": PresetProfile( + preset_id="classic-layered", + base_template="fastapi-default", + regenerate_main=False, + main_py_relpath="src/main.py", + db_config_relpath="src/core/database.py", + auth_config_relpath="src/core/auth.py", + # CORS is intentionally NOT in this list: fastapi-default's shipped + # main.py already imports CORSMiddleware and adds it conditionally + # on settings.all_cors_origins, so the user only has to populate + # BACKEND_CORS_ORIGINS in .env — no code edits needed. + manual_wiring_note=( + "fastapi-default's shipped src/main.py is preserved. The " + "selections below need manual wiring there (CORS is already " + "wired — set BACKEND_CORS_ORIGINS in .env to activate it)." + ), + extra_warning_targets=("Rate-Limiting", "Prometheus"), + ), + "domain-starter": PresetProfile( + preset_id="domain-starter", + base_template="fastapi-domain-starter", + regenerate_main=False, + main_py_relpath="src/app/main.py", + db_config_relpath="src/app/core/database.py", + auth_config_relpath="src/app/core/auth.py", + manual_wiring_note=( + "fastapi-domain-starter's shipped src/app/main.py is preserved. " + "The selections below need manual wiring there (CORS is already " + "wired — set BACKEND_CORS_ORIGINS in .env to activate it)." + ), + extra_warning_targets=("Rate-Limiting", "Prometheus"), + ), +} + + +class PresetLayoutStrategist: + """Single source of truth for preset → generation-layout decisions.""" + + def __init__(self, preset_id: str | None) -> None: + # Empty / None / unknown ids fall back to ``minimal`` so older callers + # that pre-date the architecture-preset prompt keep working. + canonical = (preset_id or _FALLBACK_PRESET_ID).strip() + self.profile: PresetProfile = _PRESET_PROFILES.get( + canonical, _PRESET_PROFILES[_FALLBACK_PRESET_ID] + ) + + @classmethod + def supported_presets(cls) -> List[str]: + """Return the ordered list of preset ids the strategist understands.""" + return list(_PRESET_PROFILES.keys()) + + @property + def preset_id(self) -> str: + return self.profile.preset_id + + @property + def base_template(self) -> str: + return self.profile.base_template + + @property + def should_regenerate_main(self) -> bool: + return self.profile.regenerate_main + + def main_py_target(self, project_dir: str) -> Path: + """Absolute path where the dynamic main.py overlay should land.""" + return Path(project_dir) / self.profile.main_py_relpath + + def db_config_target(self, project_dir: str) -> Path: + """Absolute path for the generated database config module.""" + return Path(project_dir) / self.profile.db_config_relpath + + def auth_config_target(self, project_dir: str) -> Path: + """Absolute path for the generated authentication config module.""" + return Path(project_dir) / self.profile.auth_config_relpath + + def compatibility_warnings(self, config: Dict[str, Any]) -> List[str]: + """Return user-facing warnings for unsupported preset/feature mixes. + + The dynamic ``main.py`` overlay (CORS middleware wiring, Prometheus + instrumentation, rate-limit hookup) only runs for presets that + regenerate ``main.py``. For the other presets we keep the + template-shipped ``main.py`` intact and surface a single warning + listing the affected features so users know to wire them up + themselves rather than assuming the package install was enough. + """ + if self.profile.regenerate_main: + return [] + + affected = self._affected_overlay_targets(config) + if not affected: + return [] + + warnings: List[str] = [] + if self.profile.manual_wiring_note: + warnings.append(self.profile.manual_wiring_note) + warnings.append( + "Affected selections (packages installed, but no dynamic main.py " + "edits applied for the '" + + self.profile.preset_id + + "' preset): " + + ", ".join(affected) + ) + return warnings + + def _affected_overlay_targets(self, config: Dict[str, Any]) -> List[str]: + """Detect which of the user's selections rely on main.py overlay.""" + triggered: List[str] = [] + utilities = set(config.get("utilities") or []) + + for target in self.profile.extra_warning_targets: + if target in {"CORS", "Rate-Limiting"}: + if target in utilities: + triggered.append(target) + elif target == "Prometheus": + if config.get("monitoring") == "Prometheus": + triggered.append(target) + return triggered + + +__all__ = [ + "PresetLayoutStrategist", + "PresetProfile", +] diff --git a/src/fastapi_fastkit/cli.py b/src/fastapi_fastkit/cli.py index 6d85f37..3930d8a 100644 --- a/src/fastapi_fastkit/cli.py +++ b/src/fastapi_fastkit/cli.py @@ -8,6 +8,7 @@ import shutil import subprocess import sys +from pathlib import Path from typing import Union, cast import click @@ -422,8 +423,17 @@ def init( try: user_local = settings.USER_WORKSPACE - # Use fastapi-empty template as base - template = "fastapi-empty" + # Pick the base template from the architecture preset chosen + # earlier in the interactive flow. Older callers without a + # preset fall back to ``minimal`` (= fastapi-empty), preserving + # pre-#45 behaviour. + from fastapi_fastkit.backend.project_builder import ( + PresetLayoutStrategist, + ) + + preset_id = config.get("architecture_preset") + strategist = PresetLayoutStrategist(preset_id) + template = strategist.base_template template_dir = settings.FASTKIT_TEMPLATE_ROOT target_template = os.path.join(template_dir, template) @@ -472,38 +482,42 @@ def init( generator = DynamicConfigGenerator(config, project_dir) - # Generate main.py with selected features - main_py_content = generator.generate_main_py() - main_py_path = os.path.join(project_dir, "src", "main.py") - if not os.path.exists(main_py_path): - main_py_path = os.path.join(project_dir, "main.py") - - with open(main_py_path, "w") as f: - f.write(main_py_content) + # main.py overlay — only regenerated for presets that ship a + # placeholder app (minimal, single-module). For richer presets + # (classic-layered, domain-starter) we keep the template's + # router-aware main.py intact. + if strategist.should_regenerate_main: + main_py_path = strategist.main_py_target(project_dir) + if not main_py_path.exists(): + fallback = Path(project_dir) / "main.py" + if fallback.exists(): + main_py_path = fallback + main_py_path.parent.mkdir(parents=True, exist_ok=True) + main_py_path.write_text(generator.generate_main_py()) + else: + print_info( + f"Preserving template-shipped main.py for preset " + f"'{strategist.preset_id}'." + ) - # Generate database configuration if selected + # Generate database configuration if selected — preset chooses + # where the file lives so it sits next to the existing structure. db_info = config.get("database", {}) if isinstance(db_info, dict) and db_info.get("type") != "None": db_config_content = generator.generate_database_config() if db_config_content: - db_config_path = os.path.join( - project_dir, "src", "config", "database.py" - ) - os.makedirs(os.path.dirname(db_config_path), exist_ok=True) - with open(db_config_path, "w") as f: - f.write(db_config_content) + db_config_path = strategist.db_config_target(project_dir) + db_config_path.parent.mkdir(parents=True, exist_ok=True) + db_config_path.write_text(db_config_content) # Generate auth configuration if selected auth_type = config.get("authentication", "None") if auth_type != "None": auth_config_content = generator.generate_auth_config() if auth_config_content: - auth_config_path = os.path.join( - project_dir, "src", "config", "auth.py" - ) - os.makedirs(os.path.dirname(auth_config_path), exist_ok=True) - with open(auth_config_path, "w") as f: - f.write(auth_config_content) + auth_config_path = strategist.auth_config_target(project_dir) + auth_config_path.parent.mkdir(parents=True, exist_ok=True) + auth_config_path.write_text(auth_config_content) # Generate test configuration if testing selected testing_type = config.get("testing", "None") @@ -520,6 +534,12 @@ def init( generator.generate_docker_files() print_success("Generated Docker deployment files") + # Surface preset-specific warnings (e.g. "you picked a preset + # whose shipped main.py we kept; CORS/Prometheus must be wired + # manually"). + for warning in strategist.compatibility_warnings(config): + print_warning(warning, title="Preset compatibility") + print_success("Generated configuration files for selected stack") # Create virtual environment and install dependencies diff --git a/tests/test_backends/test_preset_layout.py b/tests/test_backends/test_preset_layout.py new file mode 100644 index 0000000..dee0c32 --- /dev/null +++ b/tests/test_backends/test_preset_layout.py @@ -0,0 +1,205 @@ +# -------------------------------------------------------------------------- +# Tests for project_builder.preset_layout. +# +# Covers each preset's static layout decisions plus the compatibility +# warnings surfaced when a preset preserves the template-shipped main.py. +# +# @author bnbong bbbong9@gmail.com +# -------------------------------------------------------------------------- +from pathlib import Path + +import pytest + +from fastapi_fastkit.backend.project_builder.preset_layout import ( + PresetLayoutStrategist, +) + + +class TestSupportedPresets: + """Sanity checks for the canonical preset list.""" + + def test_all_four_presets_listed_in_order(self) -> None: + assert PresetLayoutStrategist.supported_presets() == [ + "minimal", + "single-module", + "classic-layered", + "domain-starter", + ] + + +class TestBaseTemplateMapping: + """Each preset must point at a real shipped template.""" + + @pytest.mark.parametrize( + "preset_id, expected_base", + [ + ("minimal", "fastapi-empty"), + ("single-module", "fastapi-single-module"), + ("classic-layered", "fastapi-default"), + ("domain-starter", "fastapi-domain-starter"), + ], + ) + def test_base_template_for_each_preset( + self, preset_id: str, expected_base: str + ) -> None: + strategist = PresetLayoutStrategist(preset_id) + assert strategist.base_template == expected_base + + def test_unknown_preset_falls_back_to_minimal(self) -> None: + strategist = PresetLayoutStrategist("not-a-real-preset") + assert strategist.preset_id == "minimal" + assert strategist.base_template == "fastapi-empty" + + def test_none_preset_falls_back_to_minimal(self) -> None: + strategist = PresetLayoutStrategist(None) + assert strategist.preset_id == "minimal" + + def test_empty_string_preset_falls_back_to_minimal(self) -> None: + strategist = PresetLayoutStrategist("") + assert strategist.preset_id == "minimal" + + +class TestRegenerationPolicy: + """Only minimal / single-module presets regenerate main.py.""" + + @pytest.mark.parametrize( + "preset_id, expected", + [ + ("minimal", True), + ("single-module", True), + ("classic-layered", False), + ("domain-starter", False), + ], + ) + def test_should_regenerate_main(self, preset_id: str, expected: bool) -> None: + strategist = PresetLayoutStrategist(preset_id) + assert strategist.should_regenerate_main is expected + + +class TestFileTargets: + """Generated files land at preset-appropriate paths.""" + + @pytest.mark.parametrize( + "preset_id, expected_main, expected_db, expected_auth", + [ + ( + "minimal", + "src/main.py", + "src/config/database.py", + "src/config/auth.py", + ), + ( + "single-module", + "src/main.py", + "src/config/database.py", + "src/config/auth.py", + ), + ( + "classic-layered", + "src/main.py", + "src/core/database.py", + "src/core/auth.py", + ), + ( + "domain-starter", + "src/app/main.py", + "src/app/core/database.py", + "src/app/core/auth.py", + ), + ], + ) + def test_targets_match_documented_matrix( + self, + preset_id: str, + expected_main: str, + expected_db: str, + expected_auth: str, + ) -> None: + strategist = PresetLayoutStrategist(preset_id) + project = "/proj" + + assert strategist.main_py_target(project) == Path(project) / expected_main + assert strategist.db_config_target(project) == Path(project) / expected_db + assert strategist.auth_config_target(project) == Path(project) / expected_auth + + +class TestCompatibilityWarnings: + """Warnings only fire on presets that don't regenerate main.py.""" + + def test_minimal_never_warns(self) -> None: + strategist = PresetLayoutStrategist("minimal") + config = {"utilities": ["CORS", "Rate-Limiting"], "monitoring": "Prometheus"} + assert strategist.compatibility_warnings(config) == [] + + def test_single_module_never_warns(self) -> None: + strategist = PresetLayoutStrategist("single-module") + config = {"utilities": ["CORS"], "monitoring": "Prometheus"} + assert strategist.compatibility_warnings(config) == [] + + def test_classic_layered_warns_on_rate_limiting(self) -> None: + strategist = PresetLayoutStrategist("classic-layered") + config = {"utilities": ["Rate-Limiting"], "monitoring": "None"} + warnings = strategist.compatibility_warnings(config) + # Two lines: the manual-wiring note and the affected-features list. + assert len(warnings) == 2 + assert any("Rate-Limiting" in line for line in warnings) + assert any("classic-layered" in line for line in warnings) + + def test_domain_starter_warns_on_prometheus(self) -> None: + strategist = PresetLayoutStrategist("domain-starter") + config = {"utilities": [], "monitoring": "Prometheus"} + warnings = strategist.compatibility_warnings(config) + assert any("Prometheus" in line for line in warnings) + assert any("domain-starter" in line for line in warnings) + + def test_preserve_main_silent_for_cors_only(self) -> None: + """CORS alone must NOT fire a warning on preserve-main presets. + + Both fastapi-default and fastapi-domain-starter already wire + CORSMiddleware in their shipped main.py, conditional on + ``settings.all_cors_origins`` — picking CORS in the wizard only + means the user needs to populate BACKEND_CORS_ORIGINS in .env. + """ + for preset_id in ("classic-layered", "domain-starter"): + strategist = PresetLayoutStrategist(preset_id) + config = {"utilities": ["CORS"], "monitoring": "None"} + assert ( + strategist.compatibility_warnings(config) == [] + ), f"{preset_id} should not warn for CORS-only selection" + + def test_classic_layered_silent_when_no_overlay_features(self) -> None: + strategist = PresetLayoutStrategist("classic-layered") + # Database / auth only — these don't trigger main.py overlay + # warnings (config files still get generated at the preset path). + config = { + "database": {"type": "PostgreSQL"}, + "authentication": "JWT", + "utilities": [], + "monitoring": "None", + } + assert strategist.compatibility_warnings(config) == [] + + def test_domain_starter_collects_multiple_warnings(self) -> None: + strategist = PresetLayoutStrategist("domain-starter") + config = { + # CORS is intentionally included here too: it must NOT show up + # in the *affected-selections* line, since the shipped main.py + # already wires it. (The leading manual-wiring note may still + # mention CORS informationally — "CORS is already wired — + # set BACKEND_CORS_ORIGINS in .env" — so we scope the strict + # check to the affected-features line.) + "utilities": ["CORS", "Rate-Limiting"], + "monitoring": "Prometheus", + } + warnings = strategist.compatibility_warnings(config) + affected_line = next( + (line for line in warnings if line.startswith("Affected selections")), + "", + ) + assert affected_line, f"expected an 'Affected selections' line, got: {warnings}" + assert "CORS" not in affected_line, ( + "CORS is pre-wired in the shipped main.py; affected-selections " + "list must omit it" + ) + assert "Rate-Limiting" in affected_line + assert "Prometheus" in affected_line diff --git a/tests/test_cli_operations/test_cli_interactive_integration.py b/tests/test_cli_operations/test_cli_interactive_integration.py index 4fea1ed..1f2f72e 100644 --- a/tests/test_cli_operations/test_cli_interactive_integration.py +++ b/tests/test_cli_operations/test_cli_interactive_integration.py @@ -8,6 +8,7 @@ from typing import Any from unittest.mock import MagicMock, patch +import pytest from click.testing import CliRunner from fastapi_fastkit.cli import fastkit_cli @@ -308,3 +309,197 @@ def test_init_interactive_cleans_up_on_failure( assert ( not project_path.exists() ), "Failed interactive init must leave no partial project folder" + + # Each tuple: (preset prompt input, project name suffix, + # base template marker, marker file produced by the preset's + # base template, dynamic db config target relative path, + # should the dynamic main.py overlay overwrite the shipped one). + _PRESET_LAYOUT_CASES = [ + ( + "1", + "minimal", + "fastapi-empty", + "src/main.py", + "src/config/database.py", + True, + ), + ( + "2", + "single-module", + "fastapi-single-module", + "src/main.py", + "src/config/database.py", + True, + ), + ( + "3", + "classic-layered", + "fastapi-default", + "src/main.py", + "src/core/database.py", + False, + ), + ( + "4", + "domain-starter", + "fastapi-domain-starter", + "src/app/main.py", + "src/app/core/database.py", + False, + ), + ] + + @pytest.mark.parametrize( + "preset_choice,suffix,base_template_name,main_relpath,db_relpath,regenerates_main", + _PRESET_LAYOUT_CASES, + ids=[case[1] for case in _PRESET_LAYOUT_CASES], + ) + @patch("fastapi_fastkit.cli.subprocess.run") + @patch("fastapi_fastkit.backend.package_managers.uv_manager.UvManager.is_available") + def test_init_interactive_layout_per_preset( + self, + mock_uv_available: MagicMock, + mock_subprocess: MagicMock, + temp_dir: str, + preset_choice: str, + suffix: str, + base_template_name: str, + main_relpath: str, + db_relpath: str, + regenerates_main: bool, + ) -> None: + """Each architecture preset deploys a different base template and + writes generated database config into the preset-specific path. + + The dynamic main.py overlay only replaces the shipped main.py for + presets that opt in (minimal, single-module); the richer presets + (classic-layered, domain-starter) preserve the template-shipped + main.py as a sentinel that we don't clobber router-aware code. + """ + # given + os.chdir(temp_dir) + project_name = f"layout-{suffix}" + + mock_uv_available.return_value = True + + def _mock_subprocess(*args: Any, **kwargs: Any) -> MagicMock: + if args and "venv" in str(args[0]): + venv_path = Path(temp_dir) / project_name / ".venv" + venv_path.mkdir(parents=True, exist_ok=True) + bin_dir = "Scripts" if os.name == "nt" else "bin" + (venv_path / bin_dir).mkdir(exist_ok=True) + mock_result = MagicMock() + mock_result.returncode = 0 + return mock_result + + mock_subprocess.side_effect = _mock_subprocess + + # Pick a feature combination that triggers warnings on richer + # presets (CORS + Prometheus) so the same input string exercises + # both code paths. + result = self.runner.invoke( + fastkit_cli, + ["init", "--interactive"], + input="\n".join( + [ + project_name, + "Layout Tester", + "layout@test.com", + f"Layout regression for {suffix}", + preset_choice, # Architecture preset + "1", # Database: PostgreSQL + "5", # Authentication: None (last option) + "3", # Background Tasks: None (last option) + "2", # Caching: None (last option) + "3", # Monitoring: Prometheus + "1", # Testing: Basic + "1", # Utilities: CORS + "3", # Deployment: None + "2", # Package manager: uv + "", # Custom packages: skip + "Y", # Proceed + "Y", # Create project folder + ] + ), + ) + + # then + project_path = Path(temp_dir) / project_name + assert project_path.exists() and project_path.is_dir(), ( + f"[{suffix}] project not created. exit_code={result.exit_code}\n" + f"output:\n{result.output}" + ) + + # The base template's signature file ends up in the project, which + # is how we tell that the strategist picked the right template. + main_py = project_path / main_relpath + assert main_py.exists(), ( + f"[{suffix}] expected main.py at {main_relpath}; not found.\n" + f"output:\n{result.output}" + ) + + # Database config lands at the preset-specific target path. + db_config = project_path / db_relpath + assert db_config.exists(), ( + f"[{suffix}] expected generated db config at {db_relpath}; " + f"contents:\n{list(project_path.rglob('*'))}" + ) + + # Dynamic main.py marker is the FastAPI-fastkit header comment + # that ``DynamicConfigGenerator.generate_main_py`` always emits. + # Presence/absence is the contract: regenerated for minimal / + # single-module, preserved for classic-layered / domain-starter. + main_text = main_py.read_text() + if regenerates_main: + assert "Generated by FastAPI-fastkit" in main_text, ( + f"[{suffix}] main.py was not regenerated by the dynamic " + f"overlay. content head:\n{main_text[:400]}" + ) + else: + assert "Generated by FastAPI-fastkit" not in main_text, ( + f"[{suffix}] template-shipped main.py was overwritten by " + f"the dynamic overlay; preserve-main contract violated. " + f"content head:\n{main_text[:400]}" + ) + + # Compatibility warnings only fire on preserve-main presets, and + # only for selections whose dynamic main.py overlay isn't applied. + # CORS is pre-wired in the shipped main.py for both preserve-main + # presets, so the warning must NOT mention it — it should mention + # Prometheus (selected above) which truly does need manual wiring. + if regenerates_main: + assert "Preset compatibility" not in result.output, ( + f"[{suffix}] unexpected compatibility warning. " + f"output:\n{result.output}" + ) + else: + assert "Preset compatibility" in result.output, ( + f"[{suffix}] expected a 'Preset compatibility' warning for " + f"Prometheus on a preserve-main preset.\n" + f"output:\n{result.output}" + ) + assert "Prometheus" in result.output, ( + f"[{suffix}] warning must list Prometheus.\n" + f"output:\n{result.output}" + ) + # The warning's "affected selections" line lists each affected + # feature individually; CORS must not appear there because it + # is already wired in the shipped main.py. + warning_section = result.output.split("Preset compatibility", 1)[1] + affected_line = next( + ( + line + for line in warning_section.splitlines() + if "Affected selections" in line + ), + "", + ) + assert affected_line, ( + f"[{suffix}] expected an 'Affected selections' line.\n" + f"output:\n{result.output}" + ) + assert "CORS" not in affected_line, ( + f"[{suffix}] CORS must NOT appear in the affected-selections " + f"line — it is pre-wired in the shipped main.py.\n" + f"output:\n{result.output}" + ) From 83f0e75e19065d3206be1c87316cb4808feaa50a Mon Sep 17 00:00:00 2001 From: bnbong Date: Sat, 2 May 2026 17:10:27 +0900 Subject: [PATCH 2/2] [FIX] fix hardcoded dockerfile generator --- .../project_builder/config_generator.py | 20 +++-- .../backend/project_builder/preset_layout.py | 15 ++++ src/fastapi_fastkit/cli.py | 18 ++-- tests/test_backends/test_preset_layout.py | 19 +++++ .../test_project_builder_config_generator.py | 29 +++++++ .../test_cli_interactive_integration.py | 84 +++++++++++++++++++ 6 files changed, 173 insertions(+), 12 deletions(-) diff --git a/src/fastapi_fastkit/backend/project_builder/config_generator.py b/src/fastapi_fastkit/backend/project_builder/config_generator.py index 4dbb206..a769183 100644 --- a/src/fastapi_fastkit/backend/project_builder/config_generator.py +++ b/src/fastapi_fastkit/backend/project_builder/config_generator.py @@ -447,12 +447,20 @@ def _generate_fastapi_users_config(self) -> str: return "\n".join(content) - def generate_docker_files(self) -> None: - """Generate Dockerfile and docker-compose.yml.""" + def generate_docker_files(self, app_module: str = "src.main:app") -> None: + """Generate Dockerfile and docker-compose.yml. + + The ``app_module`` is the ``module:attr`` string baked into the + Dockerfile's ``CMD`` (and into docker-compose's `command` if the + compose generator ever needs it). Architecture presets that put + the FastAPI app at a non-default location (e.g. ``domain-starter`` + ships ``src/app/main.py``) must pass the matching dotted path so + the generated container actually starts. + """ deployment = self.config.get("deployment", []) if "Docker" in deployment: - dockerfile_content = self._generate_dockerfile() + dockerfile_content = self._generate_dockerfile(app_module=app_module) dockerfile_path = self.project_dir / "Dockerfile" with open(dockerfile_path, "w") as f: f.write(dockerfile_content) @@ -463,8 +471,8 @@ def generate_docker_files(self) -> None: with open(compose_path, "w") as f: f.write(compose_content) - def _generate_dockerfile(self) -> str: - """Generate Dockerfile content.""" + def _generate_dockerfile(self, app_module: str = "src.main:app") -> str: + """Generate Dockerfile content with a layout-aware uvicorn target.""" content = [] content.append( "# --------------------------------------------------------------------------" @@ -487,7 +495,7 @@ def _generate_dockerfile(self) -> str: content.append("") content.append("# Run application") content.append( - 'CMD ["uvicorn", "src.main:app", "--host", "0.0.0.0", "--port", "8000"]' + f'CMD ["uvicorn", "{app_module}", "--host", "0.0.0.0", "--port", "8000"]' ) content.append("") diff --git a/src/fastapi_fastkit/backend/project_builder/preset_layout.py b/src/fastapi_fastkit/backend/project_builder/preset_layout.py index eaea1d6..e9abc39 100644 --- a/src/fastapi_fastkit/backend/project_builder/preset_layout.py +++ b/src/fastapi_fastkit/backend/project_builder/preset_layout.py @@ -130,6 +130,21 @@ def main_py_target(self, project_dir: str) -> Path: """Absolute path where the dynamic main.py overlay should land.""" return Path(project_dir) / self.profile.main_py_relpath + @property + def app_module(self) -> str: + """Return the ``module:attr`` string uvicorn / Docker should target. + + Derived from ``main_py_relpath`` so docker generation, runserver, + and any future container-orchestration code all agree on the + entrypoint a given preset produces. + """ + # Strip the trailing ``.py`` and convert path separators to dots. + relpath = self.profile.main_py_relpath + if relpath.endswith(".py"): + relpath = relpath[: -len(".py")] + module_part = relpath.replace("/", ".").replace("\\", ".") + return f"{module_part}:app" + def db_config_target(self, project_dir: str) -> Path: """Absolute path for the generated database config module.""" return Path(project_dir) / self.profile.db_config_relpath diff --git a/src/fastapi_fastkit/cli.py b/src/fastapi_fastkit/cli.py index 3930d8a..81e955d 100644 --- a/src/fastapi_fastkit/cli.py +++ b/src/fastapi_fastkit/cli.py @@ -8,7 +8,6 @@ import shutil import subprocess import sys -from pathlib import Path from typing import Union, cast import click @@ -486,12 +485,14 @@ def init( # placeholder app (minimal, single-module). For richer presets # (classic-layered, domain-starter) we keep the template's # router-aware main.py intact. + # + # The strategist's ``main_py_target`` is always ``src/main.py`` + # for both regenerate-main presets, and both fastapi-empty and + # fastapi-single-module ship that file, so we can write + # straight to the strategist's path without a flat-``main.py`` + # fallback branch. if strategist.should_regenerate_main: main_py_path = strategist.main_py_target(project_dir) - if not main_py_path.exists(): - fallback = Path(project_dir) / "main.py" - if fallback.exists(): - main_py_path = fallback main_py_path.parent.mkdir(parents=True, exist_ok=True) main_py_path.write_text(generator.generate_main_py()) else: @@ -531,7 +532,12 @@ def init( # Generate Docker files if deployment selected deployment = config.get("deployment", []) if deployment and deployment != ["None"]: - generator.generate_docker_files() + # Thread the preset-aware app module so the generated + # Dockerfile's ``CMD ["uvicorn", ":app", ...]`` + # matches the layout the user actually generated. Default + # ``src.main:app`` only works for minimal / single-module / + # classic-layered; domain-starter needs ``src.app.main:app``. + generator.generate_docker_files(app_module=strategist.app_module) print_success("Generated Docker deployment files") # Surface preset-specific warnings (e.g. "you picked a preset diff --git a/tests/test_backends/test_preset_layout.py b/tests/test_backends/test_preset_layout.py index dee0c32..d526eeb 100644 --- a/tests/test_backends/test_preset_layout.py +++ b/tests/test_backends/test_preset_layout.py @@ -123,6 +123,25 @@ def test_targets_match_documented_matrix( assert strategist.auth_config_target(project) == Path(project) / expected_auth +class TestAppModule: + """The ``app_module`` property must match each preset's main.py path.""" + + @pytest.mark.parametrize( + "preset_id, expected_app_module", + [ + ("minimal", "src.main:app"), + ("single-module", "src.main:app"), + ("classic-layered", "src.main:app"), + ("domain-starter", "src.app.main:app"), + ], + ) + def test_app_module_matches_main_py_path( + self, preset_id: str, expected_app_module: str + ) -> None: + strategist = PresetLayoutStrategist(preset_id) + assert strategist.app_module == expected_app_module + + class TestCompatibilityWarnings: """Warnings only fire on presets that don't regenerate main.py.""" diff --git a/tests/test_backends/test_project_builder_config_generator.py b/tests/test_backends/test_project_builder_config_generator.py index cbe58ca..3520499 100644 --- a/tests/test_backends/test_project_builder_config_generator.py +++ b/tests/test_backends/test_project_builder_config_generator.py @@ -368,6 +368,35 @@ def test_generated_dockerfile_uses_exec_form_cmd(self) -> None: assert parsed[0] == "uvicorn" assert "src.main:app" in parsed + def test_generated_dockerfile_honors_custom_app_module(self) -> None: + """The Dockerfile CMD must target the caller-supplied app module. + + Regression for the Codex P1 finding on PR #55: domain-starter + ships ``src/app/main.py``, so the default ``src.main:app`` would + produce a container that fails at startup. Callers (like the + interactive init flow, via ``PresetLayoutStrategist.app_module``) + thread the layout-correct module through ``generate_docker_files``. + """ + # given + with tempfile.TemporaryDirectory() as tmp: + config = {"deployment": ["Docker"]} + generator = DynamicConfigGenerator(config, tmp) + + # when + generator.generate_docker_files(app_module="src.app.main:app") + + # then + dockerfile = Path(tmp) / "Dockerfile" + content = dockerfile.read_text() + cmd_lines = [ + line for line in content.splitlines() if line.startswith("CMD ") + ] + assert len(cmd_lines) == 1 + parsed = json.loads(cmd_lines[0][len("CMD ") :]) + assert "src.app.main:app" in parsed + # The bogus default must not leak in alongside the override. + assert "src.main:app" not in parsed + class TestHelperMethods: """Test cases for helper methods.""" diff --git a/tests/test_cli_operations/test_cli_interactive_integration.py b/tests/test_cli_operations/test_cli_interactive_integration.py index 1f2f72e..b575cf3 100644 --- a/tests/test_cli_operations/test_cli_interactive_integration.py +++ b/tests/test_cli_operations/test_cli_interactive_integration.py @@ -349,6 +349,90 @@ def test_init_interactive_cleans_up_on_failure( ), ] + @patch("fastapi_fastkit.cli.subprocess.run") + @patch("fastapi_fastkit.backend.package_managers.uv_manager.UvManager.is_available") + def test_domain_starter_dockerfile_targets_src_app_main( + self, + mock_uv_available: MagicMock, + mock_subprocess: MagicMock, + temp_dir: str, + ) -> None: + """Regression for Codex P1 on PR #55. + + domain-starter ships ``src/app/main.py``; the generated Dockerfile's + ``CMD`` must target ``src.app.main:app``, not the legacy default + ``src.main:app`` — otherwise the container fails to boot. + """ + # given + os.chdir(temp_dir) + project_name = "docker-domain-starter" + mock_uv_available.return_value = True + + def _mock_subprocess(*args: Any, **kwargs: Any) -> MagicMock: + if args and "venv" in str(args[0]): + venv_path = Path(temp_dir) / project_name / ".venv" + venv_path.mkdir(parents=True, exist_ok=True) + bin_dir = "Scripts" if os.name == "nt" else "bin" + (venv_path / bin_dir).mkdir(exist_ok=True) + mock_result = MagicMock() + mock_result.returncode = 0 + return mock_result + + mock_subprocess.side_effect = _mock_subprocess + + # when — pick the domain-starter preset and Docker-only deployment. + result = self.runner.invoke( + fastkit_cli, + ["init", "--interactive"], + input="\n".join( + [ + project_name, + "Docker Tester", + "docker@test.com", + "Domain-starter Docker regression", + "4", # Architecture preset: domain-starter + "6", # Database: None (last option) + "5", # Authentication: None (last option) + "3", # Background Tasks: None (last option) + "2", # Caching: None (last option) + "4", # Monitoring: None (last option) + "1", # Testing: Basic + "", # Utilities: skip + "1", # Deployment: Docker + "2", # Package manager: uv + "", # Custom packages: skip + "Y", # Proceed + "Y", # Create project folder + ] + ), + ) + + # then + project_path = Path(temp_dir) / project_name + assert project_path.exists(), ( + f"project not created. exit_code={result.exit_code}\n" + f"output:\n{result.output}" + ) + + dockerfile = project_path / "Dockerfile" + assert dockerfile.exists(), ( + f"Dockerfile missing despite Docker deployment selection.\n" + f"output:\n{result.output}" + ) + + content = dockerfile.read_text() + assert "src.app.main:app" in content, ( + "Domain-starter Dockerfile must target src.app.main:app; " + "the CMD currently reads:\n" + + "\n".join( + line for line in content.splitlines() if line.startswith("CMD ") + ) + ) + assert '"src.main:app"' not in content, ( + "Bogus legacy default src.main:app leaked into the generated " + "Dockerfile despite the preset override." + ) + @pytest.mark.parametrize( "preset_choice,suffix,base_template_name,main_relpath,db_relpath,regenerates_main", _PRESET_LAYOUT_CASES,