Skip to content

refactor: move secid_server.py to factory pattern for testability#6

Merged
kurtseifried merged 1 commit into
mainfrom
refactor/factory-pattern-testable
May 28, 2026
Merged

refactor: move secid_server.py to factory pattern for testability#6
kurtseifried merged 1 commit into
mainfrom
refactor/factory-pattern-testable

Conversation

@kurtseifried

Copy link
Copy Markdown
Contributor

Summary

Moves `secid_server.py` from "all bootstrap at module import" to the standard FastAPI factory pattern. Unblocks HTTP-layer testing and makes the module ASGI-deployable.

Pre-Phase-1 prerequisite for the broader testing-maturity initiative.

The problem this solves

Pre-refactor, importing `secid_server` triggered:

  1. `argparse.parse_known_args()` — runs at import time
  2. Registry-path discovery with `sys.exit(1)` if none found
  3. Storage initialization
  4. App and route definitions
  5. Conditional MCP mount

Pytest couldn't safely `import secid_server` without either crashing (no registry directory in test environment) or loading a host-specific registry path. CI smoke tests had to skip importing the module entirely.

The fix

Standard FastAPI factory pattern:

```python
@DataClass
class ServerConfig:
registry_dirs: list[str]
storage_type: str = "memory"
storage_kwargs: dict = field(default_factory=dict)
load_mode: str = "lazy"

def create_app(config: ServerConfig) -> FastAPI:
"""Side-effect-free factory."""
...

def main(argv: Optional[list[str]] = None) -> int:
"""CLI entry point — parses args, calls create_app, runs uvicorn."""
...

if name == "main":
sys.exit(main())
```

What's now testable

Six new tests using `fastapi.testclient.TestClient`:

  • `test_create_app_factory_returns_fastapi_app`
  • `test_health_endpoint_returns_ok`
  • `test_resolve_endpoint_returns_envelope_for_garbage_input`
  • `test_resolve_endpoint_requires_secid_param`
  • `test_create_app_no_side_effects_on_import`
  • `test_secid_server_module_imports_without_side_effects` (regression guard for the refactor itself)

ASGI deployment path

```python

myapp.py

from secid_server import create_app, ServerConfig
import os

config = ServerConfig(
registry_dirs=os.environ.get("SECID_REGISTRY", "./registry").split(":"),
storage_type=os.environ.get("SECID_STORAGE", "memory"),
)
app = create_app(config)
```

```bash
uvicorn myapp:app --host 0.0.0.0 --port 8000
```

Verification

  • 13/13 tests pass locally
  • CLI verified: `python3 secid_server.py --help` produces the same usage as before
  • `from secid_server import create_app, ServerConfig, main` works

CI changes

`pip install httpx` added to the test workflow (FastAPI's TestClient dependency).

🤖 Generated with Claude Code

Before this change, secid_server.py ran argparse.parse_known_args() and
sys.exit(1) at module import time. Importing the module during pytest
either crashed (no registry directory) or loaded a host-specific
registry path. Surfaced during Phase 0 of the testing-maturity
initiative — CI smoke tests had to skip importing secid_server entirely.

The refactor introduces a factory pattern that's standard for FastAPI
deployments:

  - ServerConfig dataclass holds all configuration (registry_dirs,
    storage_type, storage_kwargs, load_mode)
  - create_app(config) is the importable factory; no module-level
    side effects
  - main(argv) wraps the CLI bootstrap — parses args, resolves
    default registry paths, builds ServerConfig, calls create_app,
    runs uvicorn
  - if __name__ == "__main__": just calls main()

ASGI deployments can now do:
  from secid_server import create_app, ServerConfig
  config = ServerConfig(registry_dirs=os.environ["SECID_REGISTRY"].split(":"))
  app = create_app(config)
  # then deploy with: uvicorn myapp:app

Tests extended to use fastapi.testclient.TestClient against the factory:
  - test_create_app_factory_returns_fastapi_app
  - test_health_endpoint_returns_ok
  - test_resolve_endpoint_returns_envelope_for_garbage_input
  - test_resolve_endpoint_requires_secid_param
  - test_create_app_no_side_effects_on_import
  - test_secid_server_module_imports_without_side_effects (regression guard)

13/13 tests pass locally. CLI verified working via `--help` post-refactor.

CI workflow updated to install httpx (FastAPI's TestClient dependency).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@kurtseifried kurtseifried merged commit 23b1287 into main May 28, 2026
@kurtseifried kurtseifried deleted the refactor/factory-pattern-testable branch May 28, 2026 20:22
kurtseifried added a commit that referenced this pull request May 28, 2026
…MCP (#7)

Updates README.md, CLAUDE.md, and AGENTS.md to reflect:

1. Reference-implementation framing for the Python server. It's not just
   "one of N implementations"; it's specifically the one that demonstrates
   every server-side feature (REST + optional MCP + pluggable storage).
   TS and Go ports (planned) will focus on production throughput and
   serve REST only.

2. Optional MCP dependency. /mcp is mounted only when the `mcp` Python
   package is installed (pip install mcp). Without it the server starts
   normally and the REST API works; /mcp is logged as disabled. Now
   documented explicitly in README.

3. Three planned implementations (Python active; TypeScript + Go planned),
   instead of the prior "two implementations" framing.

4. Current factory-pattern shape (post PR #6) noted in CLAUDE.md's Key
   Design Decisions and AGENTS.md's Project Structure.

5. AGENTS.md testing section updated — test_smoke.py exists and CI runs;
   removed the "no automated suite" line.

No code changes; documentation only.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant