CRUD endpoint POST response is a single-item list instead of an object - #338
Open
juneja-varun wants to merge 1 commit into
Open
CRUD endpoint POST response is a single-item list instead of an object#338juneja-varun wants to merge 1 commit into
juneja-varun wants to merge 1 commit into
Conversation
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.
Closes #211
I POST to a PiccoloCRUD endpoint to create a resource and expect to read the new id straight off the parsed response, like
response.json()["id"]. Instead the body comes back as a single-item list -[{"id": 4}]- so every client has to unwrap it first, which isn't how a REST API creating one resource should behave (and there's no way to POST multiple rows to this endpoint, so the list never has more or less than one item anyway).The cause:
row.save().run()on a new row doesinsert(self).returning(pk_column), which always returns a list (even for a single-row insert), and that list was being JSON-dumped directly. Two lines above, theBaseUserbranch already does the right thing (dump_json({"id": user.id})), so this just brings the general-table branch in line with that existing pattern - unwrap the single item before dumping it, exactly as proposed in the issue itself.Verified this doesn't silently change anything else that depends on the response shape:
FastAPIWrapper's POST route declaresresponse_model=self.ModelOut(a single object, not a list) for its Swagger schema - but since the underlying handler returns a rawResponseobject directly, FastAPI doesn't re-validate against that schema at runtime, so the actual response contradicted its own already-published schema before this fix. Grepped the rest of the codebase (including e2e tests and docs) for anything else parsing this response as a list - found and fixed one existing test (TestResponses.test_postin the FastAPI test suite) that had encoded the buggy shape as its expected value; updated it to match the corrected response.Added a dedicated regression test confirming the response body is a dict (and that its
idmatches the actual inserted row), confirmed it fails on unpatched code with the exact[{'id': 1}]shape and passes with the fix. Full suite green (216 passed, 3 skipped), isort/black/flake8/mypy clean.Worth flagging explicitly since it's a response-shape change: any existing client currently unwrapping a single-item list (
response.json()[0]["id"]) would need to update to readresponse.json()["id"]directly after this lands - though that's exactly the shape the issue is asking for, and the shape already documented inModelOut.