Use request-body DTOs for environment create/rename endpoints#137
Merged
Conversation
Align the two write endpoints on /api/v1/environments with the OpenAPI
contract (docs/openapi/viron-api.json), which already specifies request
bodies and the CreateEnvironmentRequest / UpdateEnvironmentNameRequest
schemas that were never implemented:
- POST /api/v1/environments now takes a CreateEnvironmentRequest body
({name, numGrids, gridSize}) instead of POST /{name}/{numGrids}/{gridSize}.
- PATCH /api/v1/environments/{id}/name now takes an UpdateEnvironmentNameRequest
body ({name}) instead of PATCH /{id}/name/{name}.
Both DTOs carry the validation the old path variables enforced (@notblank
name, @min(1) numGrids/gridSize) via @Valid @RequestBody, so invalid input
returns 400 through the existing GlobalExceptionHandler.
Updates the Python client (environmentService) and its tests to send JSON
bodies, the MockMvc controller tests (including new validation cases), and
the drifted MVP.md / PLANNING.md endpoint descriptions. No change to
viron-api.json: the spec was already correct and the code is now aligned to it.
This is a breaking change to the two endpoints' URLs; it is acceptable
pre-MVP and the spec was always the intended contract.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The RestTemplate-based EnvironmentService (consumed by DebugController) still built the old path-variable URLs for create/rename, which now 404 after the controller change. Send CreateEnvironmentRequest / UpdateEnvironmentNameRequest bodies via HttpEntity to the new paths, keeping the Java client in lockstep with the server and the Python client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dmccoystephenson
left a comment
Member
Author
There was a problem hiding this comment.
Self-review rubric (adversarial; anchored on green CI run 27467410219 + the diff):
- Scope: PASS — 9 files, all required for the environment write-endpoint refactor (2 DTOs, controller, Java client SDK, Python client, both test surfaces, the two drifted docs). No unrelated churn.
- Tests-new: PASS — new endpoint behaviour exercised by the rewritten 6 MockMvc create/rename tests + 3 new validation tests (blank name, non-positive numGrids, blank rename); Python client tests updated to assert the JSON bodies.
- Sibling structure: PASS —
CreateEnvironmentRequest/UpdateEnvironmentNameRequestmirrorEnvironmentDto(Lombok@Data/@NoArgsConstructor/@AllArgsConstructor+@Schema) and form a parallel request-DTO pair. - Sibling renames: PASS — the key item. The endpoint-shape change was applied across the whole parallel set: server controller, Java client SDK (
services/EnvironmentService), Python client, both test surfaces, andMVP.md+PLANNING.md. - Docs: PASS —
MVP.md+PLANNING.mdupdated;READMEalready used the body form;viron-api.jsonalready correct (no change). Pre-existing wholesale Postman drift split to #136 rather than partially patched. - Issue resolution: PASS (honest partial) — resolves the environment half of #128; the entity half is explicitly split to #135 and this PR does not auto-close #128. No false closure.
- CI: PASS —
buildgreen on PR head. - DTO boundary: PASS —
createEnvironmentstill returnsEnvironmentDto; no internal model (Environment) leaked. - Spec alignment: PASS —
POST /api/v1/environments(bodyCreateEnvironmentRequest) andPATCH /api/v1/environments/{id}/name(bodyUpdateEnvironmentNameRequest) matchviron-api.jsonexactly. The pre-existing200-vs-201response-status discrepancy is left as-is and tracked in #135. - Java/Python parallelism: PASS — server + Java client + Python client all updated.
- Override correctness: PASS —
git diffshows zero@Overridechurn in production.
Substantive finding surfaced AND fixed during review: the first push updated the server, the Python client, and the docs but missed the Java client SDK (services/EnvironmentService, consumed by DebugController), which still built the old path-variable URLs and would have 404'd at runtime. CI would not have caught it (no test covers that client). The adversarial parallelism grep caught it; fixed in the second commit (88129d0). This is exactly the cross-representation drift this repo's loop exists to prevent.
Summary: ready to merge as the environment half of #128; entity half tracked in #135, Postman drift in #136.
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns the two environment write endpoints with the OpenAPI contract, which already specifies request bodies and the
CreateEnvironmentRequest/UpdateEnvironmentNameRequestschemas that were never implemented (the gap #128 calls out):POST /api/v1/environments/{name}/{numGrids}/{gridSize}POST /api/v1/environments+CreateEnvironmentRequestbody{name, numGrids, gridSize}PATCH /api/v1/environments/{id}/name/{name}PATCH /api/v1/environments/{id}/name+UpdateEnvironmentNameRequestbody{name}@NotBlankname,@Min(1)numGrids/gridSize) via@Valid @RequestBody; invalid input returns 400 through the existingGlobalExceptionHandler.EnvironmentController).services/EnvironmentService, consumed byDebugController) — now sendsHttpEntitybodies to the new paths instead of the old path-variable URLs (would otherwise 404).services/environmentService).docs/MVP.md/docs/PLANNING.mdendpoint descriptions, which still documented the path-variable forms.docs/openapi/viron-api.json— the spec was already correct; the code is now aligned to it.Scope / split
This resolves the environment half of #128. The entity write endpoints are the same anti-pattern but are not defined in the OpenAPI spec at all, so aligning them additionally requires authoring the entity resource in
viron-api.json(a wire-contract change needing human review). That work is split into #135 to keep this PR coherent and auto-mergeable. Pre-existing wholesale Postman collection drift is split into #136.Partially addresses #128 (environment endpoints); does not auto-close it — see #135 for the remaining entity portion.
Known out-of-scope discrepancy
The spec says
POST /api/v1/environmentsreturns 200, but the controller returns 201 Created (more correct for a creation). This PR is input-shape-only and leaves the status as-is; tracked in #135 for a spec-accuracy pass.The two endpoints' URLs change. Acceptable pre-MVP; the spec was always the intended contract (confirmed with the maintainer).
Test plan
./mvnw test(full suite) — 216/216 pass (was 213; +3 validation tests), JDK 21pytest src/test/python— 80/80 pass (Python 3.11). CI does not run Python, so this local run is the Python anchor.docs/openapi/viron-api.json(path, verb, body schema match exactly)🤖 Generated with Claude Code