From 1baf7ca396c4f0fa8423afff86ed6f44b87fd9ad Mon Sep 17 00:00:00 2001 From: JS Date: Sun, 11 Oct 2026 16:10:55 +0800 Subject: [PATCH] fix: canonical trailing-slash URLs for subresource ID routes (#13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Companion to nyjc-computing/campus#407 (server PR campus#910). Item paths built through ResourceCollection parents already ended in '/', but paths assembled through Resource parents or f-strings did not: - Submissions.by_assignment/by_student f-strings - Credentials.Provider.User get/delete/new/update - Vaults key ops (make_path(key, end_slash=True)) The remaining call sites now pass end_slash=True explicitly per the issue checklist — a no-op today, but it keeps the intent visible and guards against path-construction changes. Verb routes (submit, activate, add/remove) and subresource-name routes (links, responses, feedback) stay unslashed per convention. Two unit-test assertions updated to the slashed credentials user path. --- campus_python/api/v1/assignments.py | 6 +++--- campus_python/api/v1/circles.py | 6 +++--- campus_python/api/v1/submissions.py | 12 ++++++------ campus_python/auth/v1/credentials.py | 12 ++++++------ campus_python/auth/v1/users.py | 6 +++--- campus_python/auth/v1/vaults.py | 8 ++++---- tests/unit/test_auth_coverage.py | 2 +- tests/unit/test_oauth_token_contract.py | 2 +- 8 files changed, 27 insertions(+), 27 deletions(-) diff --git a/campus_python/api/v1/assignments.py b/campus_python/api/v1/assignments.py index 53525fc..fcd5433 100644 --- a/campus_python/api/v1/assignments.py +++ b/campus_python/api/v1/assignments.py @@ -66,12 +66,12 @@ def links(self) -> "Assignments.Assignment.Links": return Assignments.Assignment.Links("links", parent=self) def delete(self) -> None: - resp = self.client.delete(self.make_path()) + resp = self.client.delete(self.make_path(end_slash=True)) resp.raise_for_status() return None def get(self) -> campus.model.Assignment: - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) resp.raise_for_status() return campus.model.Assignment.from_resource(resp.json()) @@ -83,7 +83,7 @@ def update(self, **updates) -> None: """ if not updates: raise ValueError("At least one field must be provided for update") - resp = self.client.patch(self.make_path(), json=updates) + resp = self.client.patch(self.make_path(end_slash=True), json=updates) resp.raise_for_status() return None diff --git a/campus_python/api/v1/circles.py b/campus_python/api/v1/circles.py index a50f197..46d4977 100644 --- a/campus_python/api/v1/circles.py +++ b/campus_python/api/v1/circles.py @@ -55,12 +55,12 @@ def members(self) -> "Circles.Circle.CircleMembers": return Circles.Circle.CircleMembers("members", parent=self) def delete(self) -> None: - resp = self.client.delete(self.make_path()) + resp = self.client.delete(self.make_path(end_slash=True)) resp.raise_for_status() return None def get(self) -> campus.model.Circle: - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) resp.raise_for_status() return campus.model.Circle.from_resource(resp.json()) @@ -84,7 +84,7 @@ def update( payload: dict[str, Any] = {"name": name} if description is not None: payload["description"] = description - resp = self.client.patch(self.make_path(), json=payload) + resp = self.client.patch(self.make_path(end_slash=True), json=payload) resp.raise_for_status() return None diff --git a/campus_python/api/v1/submissions.py b/campus_python/api/v1/submissions.py index 564ba3e..8dde784 100644 --- a/campus_python/api/v1/submissions.py +++ b/campus_python/api/v1/submissions.py @@ -59,7 +59,7 @@ def by_assignment(self, assignment_id: str) -> "list[campus.model.Submission]": Returns: List of Submission objects """ - path = f"{self.make_path()}/by-assignment/{assignment_id}" + path = f"{self.make_path()}/by-assignment/{assignment_id}/" resp = self.client.get(path) resp.raise_for_status() return [ @@ -76,7 +76,7 @@ def by_student(self, student_id: str) -> "list[campus.model.Submission]": Returns: List of Submission objects """ - path = f"{self.make_path()}/by-student/{student_id}" + path = f"{self.make_path()}/by-student/{student_id}/" resp = self.client.get(path) resp.raise_for_status() return [ @@ -130,13 +130,13 @@ def feedback(self) -> "Submissions.Submission.Feedback": def delete(self) -> None: """Delete this submission.""" - resp = self.client.delete(self.make_path()) + resp = self.client.delete(self.make_path(end_slash=True)) resp.raise_for_status() return None def get(self) -> campus.model.Submission: """Get this submission.""" - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) resp.raise_for_status() return campus.model.Submission.from_resource(resp.json()) @@ -167,7 +167,7 @@ def update( if not payload: raise ValueError("At least one field must be provided for update") - resp = self.client.patch(self.make_path(), json=payload) + resp = self.client.patch(self.make_path(end_slash=True), json=payload) resp.raise_for_status() return None @@ -186,7 +186,7 @@ def unsubmit(self) -> None: means "omit the field" there. """ resp = self.client.patch( - self.make_path(), json={"submitted_at": None} + self.make_path(end_slash=True), json={"submitted_at": None} ) resp.raise_for_status() return None diff --git a/campus_python/auth/v1/credentials.py b/campus_python/auth/v1/credentials.py index 3747234..b1ee4aa 100644 --- a/campus_python/auth/v1/credentials.py +++ b/campus_python/auth/v1/credentials.py @@ -40,14 +40,14 @@ def __getitem__(self, user_id: str) -> "Credentials.Provider.User": def get(self, token_id: str) -> campus.model.UserCredentials: resp = self.client.get( - self.make_path(), + self.make_path(end_slash=True), query={"token_id": token_id} ) resp.raise_for_status() return campus.model.UserCredentials.from_resource(resp.json()) def list(self) -> "list[campus.model.UserCredentials]": - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) # Raise error if status code is not 2XX or 3XX resp.raise_for_status() return [ @@ -61,7 +61,7 @@ class User(Resource): def delete(self) -> None: client_id = env.CLIENT_ID resp = self.client.delete( - self.make_path(), + self.make_path(end_slash=True), json={"client_id": client_id} ) # Raise error if status code is not 2XX or 3XX @@ -70,7 +70,7 @@ def delete(self) -> None: def get(self) -> campus.model.UserCredentials: client_id = env.CLIENT_ID resp = self.client.get( - self.make_path(), + self.make_path(end_slash=True), query={"client_id": client_id} ) # Raise error if status code is not 2XX or 3XX @@ -92,7 +92,7 @@ def new( The created UserCredentials """ resp = self.client.post( - self.make_path(), + self.make_path(end_slash=True), json={"scopes": scopes, "expires_in": expires_in} ) # Raise error if status code is not 2XX or 3XX @@ -105,6 +105,6 @@ def update(self, token: campus.model.OAuthToken) -> None: "client_id": client_id, "token": token.to_resource() } - resp = self.client.patch(self.make_path(), json=json_data) + resp = self.client.patch(self.make_path(end_slash=True), json=json_data) # Raise error if status code is not 2XX or 3XX resp.raise_for_status() diff --git a/campus_python/auth/v1/users.py b/campus_python/auth/v1/users.py index 1c4217a..787695e 100644 --- a/campus_python/auth/v1/users.py +++ b/campus_python/auth/v1/users.py @@ -46,11 +46,11 @@ def activate(self) -> campus.model.User: return campus.model.User.from_resource(resp.json()) def delete(self) -> None: - resp = self.client.delete(self.make_path()) + resp = self.client.delete(self.make_path(end_slash=True)) resp.raise_for_status() def get(self) -> campus.model.User: - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) resp.raise_for_status() return campus.model.User.from_resource(resp.json()) @@ -67,6 +67,6 @@ def update(self, *, name: str) -> campus.model.User: Returns: The updated User """ - resp = self.client.patch(self.make_path(), json={"name": name}) + resp = self.client.patch(self.make_path(end_slash=True), json={"name": name}) resp.raise_for_status() return campus.model.User.from_resource(resp.json()) diff --git a/campus_python/auth/v1/vaults.py b/campus_python/auth/v1/vaults.py index 330a614..25db0e2 100644 --- a/campus_python/auth/v1/vaults.py +++ b/campus_python/auth/v1/vaults.py @@ -24,20 +24,20 @@ class Vault(Resource): """A single vault (label) exposing mapping-like access.""" def keys(self) -> "list[str]": - resp = self.client.get(self.make_path()) + resp = self.client.get(self.make_path(end_slash=True)) resp.raise_for_status() body = resp.json() return body["keys"] def __delitem__(self, key: str) -> None: - resp = self.client.delete(self.make_path(key)) + resp = self.client.delete(self.make_path(key, end_slash=True)) if resp.status_code == 404: raise KeyError(key) resp.raise_for_status() return None def __getitem__(self, key: str) -> str: - resp = self.client.get(self.make_path(key)) + resp = self.client.get(self.make_path(key, end_slash=True)) # translate 404 into KeyError for route compatibility if resp.status_code == 404: raise KeyError(key) @@ -47,6 +47,6 @@ def __getitem__(self, key: str) -> str: return body["key"] def __setitem__(self, key: str, value: Any) -> None: - resp = self.client.post(self.make_path(key), json={"value": value}) + resp = self.client.post(self.make_path(key, end_slash=True), json={"value": value}) resp.raise_for_status() return None diff --git a/tests/unit/test_auth_coverage.py b/tests/unit/test_auth_coverage.py index d2e9db3..2d55acb 100644 --- a/tests/unit/test_auth_coverage.py +++ b/tests/unit/test_auth_coverage.py @@ -128,7 +128,7 @@ def test_new_posts_scopes_and_expires_in(self): scopes=["campus.profile"], expires_in=3600 ) self.client.post.assert_called_once_with( - "/auth/v1/credentials/campus/user1", + "/auth/v1/credentials/campus/user1/", json={"scopes": ["campus.profile"], "expires_in": 3600}, ) self.assertIsInstance(creds, campus.model.UserCredentials) diff --git a/tests/unit/test_oauth_token_contract.py b/tests/unit/test_oauth_token_contract.py index 494baa6..87fb440 100644 --- a/tests/unit/test_oauth_token_contract.py +++ b/tests/unit/test_oauth_token_contract.py @@ -136,7 +136,7 @@ def test_update_patches_credentials_endpoint(self): self.auth.credentials["campus"]["user1"].update(self.token) self.client.patch.assert_called_once() args, kwargs = self.client.patch.call_args - self.assertEqual(args[0], "/auth/v1/credentials/campus/user1") + self.assertEqual(args[0], "/auth/v1/credentials/campus/user1/") self.assertEqual(kwargs["json"]["client_id"], "cid123") self.assertEqual(kwargs["json"]["token"], self.token.to_resource())