From e99b84b7d74058e0112c0c6d4286913fc24a3279 Mon Sep 17 00:00:00 2001 From: JS Ng Date: Thu, 1 Oct 2026 10:54:58 +0800 Subject: [PATCH] feat(client): resolve service URLs from explicit config; deprecate HOSTNAME (issue #52) Service base URLs are now resolved per service in this order: 1. Explicit URL config: CAMPUS_AUTH_URL / CAMPUS_API_URL env vars (mirrors campus-cli's CAMPUS_AUTH_URL / auth_url config key). 2. ENV/CAMPUS_ENV defaults: development Railway deployments, staging (campus.nyjc.dev) and production (campus.nyjc.app). The DEPLOY-suffix and testing branches that built https://{HOSTNAME} URLs are removed: HOSTNAME is a container hostname, not a routable domain, and the f-string dropped the port. Deployments relying on those settings now get a DeprecationWarning and the ENV-based default instead; the warning path will be removed once downstream consumers (nyxchange-timetable-v2, campus-suite) set explicit URL config. 14 unit tests pin the resolution contract; README documents the env vars and resolution order. --- README.md | 18 +++ campus_python/__init__.py | 83 ++++++++---- tests/unit/test_base_url_resolution.py | 175 +++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 29 deletions(-) create mode 100644 tests/unit/test_base_url_resolution.py diff --git a/README.md b/README.md index e5487ec..fbbd4dd 100644 --- a/README.md +++ b/README.md @@ -144,12 +144,30 @@ client.auth.client.set_bearer_authorization(access_token) # Now you can make authenticated requests ``` +## Service Base URLs + +Each service client (`campus.auth`, `campus.api`) resolves its base URL in this order: + +1. **Explicit URL config** — the `CAMPUS_AUTH_URL` / `CAMPUS_API_URL` environment variable, if set. Use this for local testing deployments and custom endpoints (e.g. `CAMPUS_AUTH_URL=http://localhost:5000`). +2. **ENV-based defaults** — selected by `ENV` (or `CAMPUS_ENV`): + +| ENV value | auth | api | +|-----------|------|-----| +| `development` (default) | `https://campusauth-development.up.railway.app` | `https://campusapi-development.up.railway.app` | +| `staging` | `https://auth.campus.nyjc.dev` | `https://api.campus.nyjc.dev` | +| `production` | `https://auth.campus.nyjc.app` | `https://api.campus.nyjc.app` | + +> **Deprecated (issue #52):** base URLs are no longer derived from the `HOSTNAME` environment variable. Deployments that relied on the `DEPLOY` service suffix (e.g. `campus.auth`) or `ENV=testing` to produce `https://{HOSTNAME}` URLs now get a `DeprecationWarning` and the ENV-based default instead — set `CAMPUS_AUTH_URL` / `CAMPUS_API_URL` explicitly to point the client at those deployments. + ## Environment Variables | Variable | Required | Mode | Description | |----------|----------|------|-------------| | `CLIENT_ID` | Yes | Server | OAuth client ID from Campus auth | | `CLIENT_SECRET` | Yes | Server | OAuth client secret from Campus auth | +| `CAMPUS_AUTH_URL` | No | All | Auth service base URL (overrides ENV default) | +| `CAMPUS_API_URL` | No | All | API service base URL (overrides ENV default) | +| `ENV` / `CAMPUS_ENV` | No | All | Deployment environment selecting default URLs: `development` (default), `staging`, `production` | ## Development diff --git a/campus_python/__init__.py b/campus_python/__init__.py index b78c39f..5fa7495 100644 --- a/campus_python/__init__.py +++ b/campus_python/__init__.py @@ -9,6 +9,7 @@ ) import logging +import warnings from collections.abc import Iterator from contextlib import contextmanager @@ -23,6 +24,49 @@ logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) +# Development Railway deployments, used when no explicit URL is configured +AUTH_DEVELOPMENT_URL = "https://campusauth-development.up.railway.app" +API_DEVELOPMENT_URL = "https://campusapi-development.up.railway.app" + + +def _resolve_base_url(service: str, url_var: str, development_url: str) -> str: + """Resolve the base URL for a Campus service. + + Precedence: + 1. Explicit URL config: the `url_var` environment variable + (CAMPUS_AUTH_URL / CAMPUS_API_URL). + 2. ENV-based defaults: development Railway deployments, + staging and production domains. + + Emits a DeprecationWarning if the environment would previously have + produced a HOSTNAME-derived URL (a DEPLOY service suffix or + ENV/CAMPUS_ENV=testing); those deployments must set `url_var` + explicitly or accept the ENV-based default (issue #52). + """ + explicit_url = env.get(url_var) + if explicit_url: + return explicit_url + + campus_env = env.get("ENV", env.get("CAMPUS_ENV", "development")) + if env.get("DEPLOY", "").endswith(f".{service}") or campus_env == "testing": + warnings.warn( + f"HOSTNAME-derived {service} base URLs are deprecated and no " + f"longer used; set {url_var} to configure the {service} base " + "URL explicitly.", + DeprecationWarning, + stacklevel=3, + ) + + match campus_env: + case "development" | "testing": + return development_url + case "staging": + return f"https://{service}.campus.nyjc.dev" + case "production": + return f"https://{service}.campus.nyjc.app" + case _: + raise ValueError("Invalid ENV value") + class Campus: """Unified Campus client interface. @@ -35,6 +79,10 @@ class Campus: - mode="device": For public clients (e.g., CLI) that don't have secrets. No credentials required; only public OAuth endpoints are accessible. + Service base URLs are resolved per service (auth, api) in this order: + 1. Explicit URL config: CAMPUS_AUTH_URL / CAMPUS_API_URL env vars. + 2. ENV/CAMPUS_ENV defaults: development (Railway), staging, production. + See the API Reference for usage examples. """ @@ -62,21 +110,9 @@ def __init__(self, timeout: int, mode: str = "server"): def auth(self) -> AuthRoot: """Get the auth service resource.""" if not hasattr(self, "_auth"): - # Use relative URL if in deployed auth service - if env.get("DEPLOY") and env.DEPLOY.endswith(".auth"): - base_url = f"https://{env.HOSTNAME}" - else: - match env.get("ENV", env.get("CAMPUS_ENV", "development")): - case "development": - base_url = "https://campusauth-development.up.railway.app" - case "testing": - base_url = f"https://{env.HOSTNAME}" - case "staging": - base_url = "https://auth.campus.nyjc.dev" - case "production": - base_url = "https://auth.campus.nyjc.app" - case _: - raise ValueError("Invalid ENV value") + base_url = _resolve_base_url( + "auth", "CAMPUS_AUTH_URL", AUTH_DEVELOPMENT_URL + ) self._auth = AuthRoot( json_client=CampusRequest( base_url=base_url, @@ -90,20 +126,9 @@ def auth(self) -> AuthRoot: def api(self) -> ApiRoot: """Get the api service resource.""" if not hasattr(self, "_api"): - if env.get("DEPLOY") and env.DEPLOY.endswith(".api"): - base_url = f"https://{env.HOSTNAME}" - else: - match env.get("ENV", env.get("CAMPUS_ENV", "development")): - case "development": - base_url = "https://campusapi-development.up.railway.app" - case "testing": - base_url = f"https://{env.HOSTNAME}" - case "staging": - base_url = "https://api.campus.nyjc.dev" - case "production": - base_url = "https://api.campus.nyjc.app" - case _: - raise ValueError("Invalid ENV value") + base_url = _resolve_base_url( + "api", "CAMPUS_API_URL", API_DEVELOPMENT_URL + ) self._api = ApiRoot( json_client=CampusRequest( base_url=base_url, diff --git a/tests/unit/test_base_url_resolution.py b/tests/unit/test_base_url_resolution.py new file mode 100644 index 0000000..a55ecbc --- /dev/null +++ b/tests/unit/test_base_url_resolution.py @@ -0,0 +1,175 @@ +"""Tests for Campus service base URL resolution. + +Pins the contract for issue #52: explicit URL config (CAMPUS_AUTH_URL / +CAMPUS_API_URL) takes precedence over ENV-based defaults, and settings +that previously produced HOSTNAME-derived URLs (DEPLOY service suffix, +ENV/CAMPUS_ENV=testing) emit a DeprecationWarning and fall back to the +ENV-based default instead. +""" + +import os +import unittest +import warnings + +import campus_python + +AUTH_DEV_URL = "https://campusauth-development.up.railway.app" +API_DEV_URL = "https://campusapi-development.up.railway.app" + +# All env vars that influence URL resolution +URL_VARS = ("CAMPUS_AUTH_URL", "CAMPUS_API_URL", "ENV", "CAMPUS_ENV", "DEPLOY") + + +class BaseUrlResolutionTestCase(unittest.TestCase): + """Base test case that saves and clears URL-related env vars.""" + + def setUp(self): + self.saved = {var: os.environ.get(var) for var in URL_VARS} + for var in URL_VARS: + os.environ.pop(var, None) + + def tearDown(self): + for var, value in self.saved.items(): + if value is not None: + os.environ[var] = value + else: + os.environ.pop(var, None) + + def new_client(self) -> campus_python.Campus: + """Create a device-mode client (no credentials required).""" + return campus_python.Campus(timeout=5, mode="device") + + +class TestExplicitUrlConfig(BaseUrlResolutionTestCase): + """Explicit URL config takes precedence over ENV-based defaults.""" + + def test_explicit_auth_url(self): + os.environ["CAMPUS_AUTH_URL"] = "https://auth.example.com" + campus = self.new_client() + self.assertEqual(campus.auth.base_url, "https://auth.example.com") + # api service is unaffected + self.assertEqual(campus.api.base_url, API_DEV_URL) + + def test_explicit_api_url(self): + os.environ["CAMPUS_API_URL"] = "http://localhost:8000" + campus = self.new_client() + self.assertEqual(campus.api.base_url, "http://localhost:8000") + # auth service is unaffected + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + + def test_explicit_url_overrides_env_default(self): + os.environ["ENV"] = "production" + os.environ["CAMPUS_AUTH_URL"] = "https://auth.example.com" + os.environ["CAMPUS_API_URL"] = "https://api.example.com" + campus = self.new_client() + self.assertEqual(campus.auth.base_url, "https://auth.example.com") + self.assertEqual(campus.api.base_url, "https://api.example.com") + + def test_explicit_url_no_deprecation_warning(self): + # Even with legacy settings present, explicit config is not deprecated + os.environ["ENV"] = "testing" + os.environ["DEPLOY"] = "campus.auth" + os.environ["CAMPUS_AUTH_URL"] = "http://localhost:5000" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + campus = self.new_client() + campus.auth + deprecations = [ + w for w in caught + if issubclass(w.category, DeprecationWarning) + ] + self.assertEqual(deprecations, []) + self.assertEqual(campus.auth.base_url, "http://localhost:5000") + + +class TestEnvDefaults(BaseUrlResolutionTestCase): + """ENV/CAMPUS_ENV-based defaults when no explicit URL is set.""" + + def test_development_default(self): + campus = self.new_client() + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + self.assertEqual(campus.api.base_url, API_DEV_URL) + + def test_staging_default(self): + os.environ["ENV"] = "staging" + campus = self.new_client() + self.assertEqual(campus.auth.base_url, "https://auth.campus.nyjc.dev") + self.assertEqual(campus.api.base_url, "https://api.campus.nyjc.dev") + + def test_production_default(self): + os.environ["ENV"] = "production" + campus = self.new_client() + self.assertEqual(campus.auth.base_url, "https://auth.campus.nyjc.app") + self.assertEqual(campus.api.base_url, "https://api.campus.nyjc.app") + + def test_campus_env_alias(self): + os.environ["CAMPUS_ENV"] = "staging" + campus = self.new_client() + self.assertEqual(campus.auth.base_url, "https://auth.campus.nyjc.dev") + self.assertEqual(campus.api.base_url, "https://api.campus.nyjc.dev") + + def test_invalid_env_raises(self): + os.environ["ENV"] = "nonsense" + campus = self.new_client() + with self.assertRaises(ValueError): + campus.auth + + +class TestHostnameDeprecation(BaseUrlResolutionTestCase): + """Settings that previously produced HOSTNAME-derived URLs now warn.""" + + def test_testing_env_warns_and_falls_back_to_development(self): + os.environ["ENV"] = "testing" + campus = self.new_client() + with self.assertWarns(DeprecationWarning): + campus.auth + campus.api + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + self.assertEqual(campus.api.base_url, API_DEV_URL) + + def test_deploy_auth_suffix_warns(self): + os.environ["DEPLOY"] = "campus.auth" + campus = self.new_client() + with self.assertWarns(DeprecationWarning): + campus.auth + # Falls back to the ENV-based default (development) + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + # api service did not match the .auth suffix: no HOSTNAME URL before + self.assertEqual(campus.api.base_url, API_DEV_URL) + + def test_deploy_api_suffix_warns(self): + os.environ["DEPLOY"] = "campus.api" + campus = self.new_client() + with self.assertWarns(DeprecationWarning): + campus.api + self.assertEqual(campus.api.base_url, API_DEV_URL) + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + + def test_warning_mentions_url_var(self): + os.environ["ENV"] = "testing" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + self.new_client().auth + deprecations = [ + w for w in caught + if issubclass(w.category, DeprecationWarning) + ] + self.assertEqual(len(deprecations), 1) + self.assertIn("CAMPUS_AUTH_URL", str(deprecations[0].message)) + + def test_unrelated_deploy_value_no_warning(self): + # A DEPLOY value without a service suffix never produced HOSTNAME URLs + os.environ["DEPLOY"] = "campus.other" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + campus = self.new_client() + deprecations = [ + w for w in caught + if issubclass(w.category, DeprecationWarning) + ] + self.assertEqual(deprecations, []) + self.assertEqual(campus.auth.base_url, AUTH_DEV_URL) + + +if __name__ == "__main__": + unittest.main()