From bea1f4e04a79a4967c3c40a56b9dd02b8c7a0b16 Mon Sep 17 00:00:00 2001 From: JS Ng Date: Tue, 27 Oct 2026 22:48:50 +0800 Subject: [PATCH] refactor(audit): move campus.common.http to campus.audit.http (#970) The HTTP client's only production consumers were the audit service (audit.client, audit.web.auth) plus the cross-service test harness seam, making it audit domain rather than shared infrastructure. With the move, requests leaves campus.common's dependency list entirely: campus-common's only third-party dep is now bcrypt (via utils.secret). Mechanical move (git mv), import-path updates in audit client/web, the flask_test harness (monkeypatch target moves with the seam), unit tests, and the TESTING-GUIDE / audit-tracing docs. campus.common drops the http shim and __getattr__. --- campus/audit/client/__init__.py | 6 ++-- campus/audit/client/v1/traces.py | 2 +- campus/{common => audit}/http/__init__.py | 2 +- campus/{common => audit}/http/default.py | 2 +- campus/{common => audit}/http/errors.py | 2 +- campus/{common => audit}/http/interface.py | 4 +-- campus/audit/web/auth.py | 2 +- campus/common/__init__.py | 20 +++--------- docs/TESTING-GUIDE.md | 2 +- docs/audit-tracing.md | 2 +- tests/flask_test/json_client.py | 16 +++++----- tests/unit/audit/test_client.py | 4 +-- tests/unit/audit/test_web_apikey_auth.py | 2 +- tests/unit/audit/test_web_auth.py | 2 +- tests/unit/common/test_import_weight.py | 37 +++++++++++++--------- 15 files changed, 51 insertions(+), 54 deletions(-) rename campus/{common => audit}/http/__init__.py (98%) rename campus/{common => audit}/http/default.py (99%) rename campus/{common => audit}/http/errors.py (96%) rename campus/{common => audit}/http/interface.py (98%) diff --git a/campus/audit/client/__init__.py b/campus/audit/client/__init__.py index 7fdfdf83..cf785f03 100644 --- a/campus/audit/client/__init__.py +++ b/campus/audit/client/__init__.py @@ -14,8 +14,8 @@ __all__ = ("AuditClient",) +from campus.audit.http.interface import JsonClient from campus.common import env -from campus.common.http.interface import JsonClient from campus.common.utils import url from .v1 import AuditRoot @@ -47,7 +47,7 @@ def _create_http_client(base_url: str) -> JsonClient: return client_class(base_url=base_url) # Priority 2: Default client with explicit audit API key - from campus.common.http import DefaultClient + from campus.audit.http import DefaultClient api_key = env.get("AUDIT_API_KEY") if api_key: return DefaultClient(base_url=base_url, auth=api_key) # type: ignore[return-value] @@ -79,7 +79,7 @@ class AuditClient: This client provides access to audit service endpoints for internal service-to-server communication (e.g., campus.auth → campus.audit). - The client uses campus.common.http.DefaultClient which handles: + The client uses campus.audit.http.DefaultClient which handles: - Bearer auth via the AUDIT_API_KEY environment variable (preferred; the audit service only accepts `audit_v1_` API keys) (#699) - Basic auth via CLIENT_ID/CLIENT_SECRET environment variables diff --git a/campus/audit/client/v1/traces.py b/campus/audit/client/v1/traces.py index 09c62f14..ebeea000 100644 --- a/campus/audit/client/v1/traces.py +++ b/campus/audit/client/v1/traces.py @@ -13,7 +13,7 @@ import contextlib from typing import Any, Optional -from campus.common.http.interface import JsonClient, JsonResponse +from campus.audit.http.interface import JsonClient, JsonResponse from ..interface import SLASH, Resource, ResourceCollection, ResourceRoot diff --git a/campus/common/http/__init__.py b/campus/audit/http/__init__.py similarity index 98% rename from campus/common/http/__init__.py rename to campus/audit/http/__init__.py index 2105e5fd..57d9ec07 100644 --- a/campus/common/http/__init__.py +++ b/campus/audit/http/__init__.py @@ -1,4 +1,4 @@ -"""campus.common.http +"""campus.audit.http Abstractions for HTTP requests. diff --git a/campus/common/http/default.py b/campus/audit/http/default.py similarity index 99% rename from campus/common/http/default.py rename to campus/audit/http/default.py index aee4ad2c..747a3f85 100644 --- a/campus/common/http/default.py +++ b/campus/audit/http/default.py @@ -1,4 +1,4 @@ -"""campus.common.http.default +"""campus.audit.http.default Default implementation for JsonClient and JsonResponse, using `requests`. """ diff --git a/campus/common/http/errors.py b/campus/audit/http/errors.py similarity index 96% rename from campus/common/http/errors.py rename to campus/audit/http/errors.py index 7855f5c9..52c73e87 100644 --- a/campus/common/http/errors.py +++ b/campus/audit/http/errors.py @@ -1,4 +1,4 @@ -"""campus.common.http.errors +"""campus.audit.http.errors Common error types used across all campus HTTP client modules. """ diff --git a/campus/common/http/interface.py b/campus/audit/http/interface.py similarity index 98% rename from campus/common/http/interface.py rename to campus/audit/http/interface.py index 01c3549b..891b5e6c 100644 --- a/campus/common/http/interface.py +++ b/campus/audit/http/interface.py @@ -1,4 +1,4 @@ -"""campus.common.http.interface +"""campus.audit.http.interface Interface descriptions for the Campus client interface. @@ -18,7 +18,7 @@ from collections.abc import Mapping from typing import Any, Iterable, Protocol, Self, runtime_checkable -from campus.common.http.errors import ( +from campus.audit.http.errors import ( AccessDeniedError, AuthenticationError, ConflictError, diff --git a/campus/audit/web/auth.py b/campus/audit/web/auth.py index 1da5a4c3..85a2041a 100644 --- a/campus/audit/web/auth.py +++ b/campus/audit/web/auth.py @@ -120,8 +120,8 @@ import campus.flask_campus as flask_campus import campus.model as model +from campus.audit.http import DefaultClient, HttpClientError, JsonClient from campus.common import env -from campus.common.http import DefaultClient, HttpClientError, JsonClient from campus.common.utils import secret, url from ..decorators import scope_matches diff --git a/campus/common/__init__.py b/campus/common/__init__.py index 7da4c4f6..abc62d78 100644 --- a/campus/common/__init__.py +++ b/campus/common/__init__.py @@ -10,16 +10,17 @@ (#967). Submodules that need them must import lazily at call time — see the module docstrings in utils/url.py and errors/handlers.py BEFORE "tidying" any import up to module level, and follow the same pattern for -new code. The http submodule is itself lazy (default.py imports requests -at module level): access it via "from campus.common import http", -resolved by module __getattr__, or import campus.common.http directly. +new code. + +The former campus.common.http client moved to campus.audit.http (#970): +its only consumers were the audit service's client and the cross-service +test harness, making it audit domain rather than shared infrastructure. """ __all__ = [ "env", "errors", "introspect", - "http", "schema", "utils", "validation", @@ -33,14 +34,3 @@ utils, validation, ) - - -def __getattr__(name): - # PEP 562: http pulls in requests (and, through utils.secret, bcrypt) - # at import time, so it is resolved on demand instead of eagerly. - # import_module, not "from . import http": a from-import consults - # this same __getattr__ and recurses forever. - if name == "http": - import importlib - return importlib.import_module(__name__ + ".http") - raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/docs/TESTING-GUIDE.md b/docs/TESTING-GUIDE.md index f3fa6f6e..e2cf3c38 100644 --- a/docs/TESTING-GUIDE.md +++ b/docs/TESTING-GUIDE.md @@ -266,7 +266,7 @@ mgr.cleanup() ### Testing Service Code That Makes Its Own HTTP Calls (`json_client_class`) The transport patch above covers calls routed through `campus_python`. -Service code that builds its own HTTP client on `campus.common.http` +Service code that builds its own HTTP client on `campus.audit.http` (`DefaultClient`) does **not** go through that patch — instead, such clients expose a `json_client_class` **class attribute** that tests set to a test double. Established instances: diff --git a/docs/audit-tracing.md b/docs/audit-tracing.md index 7a731687..d635d8f4 100644 --- a/docs/audit-tracing.md +++ b/docs/audit-tracing.md @@ -48,7 +48,7 @@ originating request, which is what the audit UI's waterfall draws: and calls from outside a request context (e.g. the ingestion executor thread) stay unparented. The mirrored copy in `campus.audit.middleware.tracing` remains for server-side sessions - built on `campus.common.http`; the two are idempotent across each + built on `campus.audit.http`; the two are idempotent across each other. - In tests, `tests.flask_test.TestCampusRequest` merges the same headers so the api→auth chain behaves as in production. diff --git a/tests/flask_test/json_client.py b/tests/flask_test/json_client.py index 587a552c..c75fb031 100644 --- a/tests/flask_test/json_client.py +++ b/tests/flask_test/json_client.py @@ -3,7 +3,7 @@ Test-compatible JsonClient that uses Flask test clients for routing. This module provides a drop-in replacement for -campus.common.http.DefaultClient that uses Flask test clients for local +campus.audit.http.DefaultClient that uses Flask test clients for local testing without actual HTTP calls. """ @@ -12,8 +12,8 @@ import flask +from campus.audit.http.interface import JsonDict, JsonResponse from campus.common import env -from campus.common.http.interface import JsonDict, JsonResponse from campus.webauth.models import HttpHeader from .campus_request import get_test_app @@ -23,7 +23,7 @@ class TestJsonClient: """Test-compatible JsonClient using Flask test clients with routing. - This class implements the campus.common.http.JsonClient protocol and + This class implements the campus.audit.http.JsonClient protocol and routes requests to the correct Flask app based on base_url and path prefix, using the same routing mechanism as TestCampusRequest. @@ -245,12 +245,12 @@ def patch(self: Self, path: str, json: Any = None) -> JsonResponse: return FlaskTestResponse(response) - """Restore original campus.common.http.DefaultClient. + """Restore original campus.audit.http.DefaultClient. Call this in test teardown to clean up the monkey-patch. """ - import campus.common.http + import campus.audit.http - if hasattr(campus.common.http, "_original_DefaultClient"): - campus.common.http.DefaultClient = campus.common.http._original_DefaultClient - delattr(campus.common.http, "_original_DefaultClient") + if hasattr(campus.audit.http, "_original_DefaultClient"): + campus.audit.http.DefaultClient = campus.audit.http._original_DefaultClient + delattr(campus.audit.http, "_original_DefaultClient") diff --git a/tests/unit/audit/test_client.py b/tests/unit/audit/test_client.py index a08d5bdc..b5a057b6 100644 --- a/tests/unit/audit/test_client.py +++ b/tests/unit/audit/test_client.py @@ -44,7 +44,7 @@ def setUp(self): def test_api_key_sends_explicit_bearer(self): """AUDIT_API_KEY produces a Bearer Authorization header.""" from campus.audit.client import _create_http_client - from campus.common.http import DefaultClient + from campus.audit.http import DefaultClient _set_env(self, "AUDIT_API_KEY", "audit_v1_abcdefghijklmnopqrstuvwxyz") client = _create_http_client("https://audit.test") @@ -75,7 +75,7 @@ def test_api_key_ignores_ambient_credentials(self): def test_without_api_key_falls_back_to_ambient_basic(self): """Without AUDIT_API_KEY, ambient CLIENT_ID/CLIENT_SECRET apply.""" from campus.audit.client import _create_http_client - from campus.common.http import DefaultClient + from campus.audit.http import DefaultClient _del_env(self, "AUDIT_API_KEY") _del_env(self, "ACCESS_TOKEN") diff --git a/tests/unit/audit/test_web_apikey_auth.py b/tests/unit/audit/test_web_apikey_auth.py index 2df25852..cf0db2ab 100644 --- a/tests/unit/audit/test_web_apikey_auth.py +++ b/tests/unit/audit/test_web_apikey_auth.py @@ -525,7 +525,7 @@ def test_oauth_login_clears_apikey_session(self): self.client.get("/audit/traces").status_code, 200 ) - from campus.common.http.interface import JsonResponse + from campus.audit.http.interface import JsonResponse class _Resp(JsonResponse): # Explicit JsonResponse subclass so FakeAuth satisfies diff --git a/tests/unit/audit/test_web_auth.py b/tests/unit/audit/test_web_auth.py index 2d6f629c..f97bc4f1 100644 --- a/tests/unit/audit/test_web_auth.py +++ b/tests/unit/audit/test_web_auth.py @@ -39,7 +39,7 @@ import flask -from campus.common.http.interface import JsonResponse +from campus.audit.http.interface import JsonResponse PUBLIC_URL = "https://campus.test" CLIENT_ID = "auditweb.test" diff --git a/tests/unit/common/test_import_weight.py b/tests/unit/common/test_import_weight.py index f8bcf7f4..b143338c 100644 --- a/tests/unit/common/test_import_weight.py +++ b/tests/unit/common/test_import_weight.py @@ -4,12 +4,13 @@ class TestCommonImportWeight(unittest.TestCase): - """Guard the lazy-import contract of the campus.common namespace (#967). + """Guard the lean-import contract of the campus.common namespace (#967). - Importing campus.common must not drag in requests (via the lazy http - submodule) or bcrypt (reachable only through http's import of - utils.secret). The assertions run in a subprocess because this test - process may legitimately have those modules loaded by other imports. + Importing campus.common must not drag in requests or bcrypt: after + the http client moved to campus.audit.http (#970), nothing in the + namespace's eager import chain pulls a third-party module any more. + Assertions run in a subprocess because this test process may + legitimately have those modules loaded by other imports. """ def _modules_loaded_by(self, stmt: str) -> set[str]: @@ -29,19 +30,25 @@ def test_import_common_is_lean(self): self.assertNotIn("requests", loaded) self.assertNotIn("bcrypt", loaded) - def test_lazy_http_getattr(self): - """Test that `from campus.common import http` still resolves the module.""" - code = ( - "from campus.common import http;" - "import sys;" - "print(http.__name__, 'requests' in sys.modules)" - ) + def test_common_http_is_gone(self): + """Test that campus.common no longer exposes an http module (#970). + + Asserted via attribute access, not `import campus.common.http`: + an import succeeds as an empty namespace package whenever a + leftover campus/common/http directory exists on the path (e.g. + untracked __pycache__), which says nothing about the package. + """ + code = "import campus.common, sys\n" \ + "try:\n" \ + " campus.common.http\n" \ + "except AttributeError:\n" \ + " print('gone')\n" \ + "else:\n" \ + " print('present')" stdout = subprocess.run( [sys.executable, "-c", code], capture_output=True, text=True, check=True, ).stdout.strip() - module_name, requests_loaded = stdout.split() - self.assertEqual(module_name, "campus.common.http") - self.assertEqual(requests_loaded, "True") + self.assertEqual(stdout, "gone")