perf(state): skip response body on HEAD requests#8348
Open
Nayte91 wants to merge 1 commit into
Open
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.
This is the follow-up to the
HEADremark I left in #7856 (limitation #1): today aHEADis processed exactly like aGET— the collection is fully read, hydrated and serialized, and Symfony only strips the body right before sending. So the database does all the work for nothing.This PR makes a
HEADrequest skip body construction entirely: the (lazy) Doctrine collection is never iterated → no rowSELECT. The response still carries its headers, just no body — which is whatHEADis for.One single commit, decoupled from the Content-Range topic on purpose: it stands on its own even if #7856 isn't merged.
RFC 9110 section followed
§9.3.2 HEAD — "The HEAD method is identical to GET except that the server MUST NOT send content in the response." The spec explicitly allows a server to omit header fields whose value is only determined while generating the content, and states this is "preferable to generating and discarding the content for a HEAD request, since HEAD is usually requested for the sake of efficiency." That's exactly what we do.
Summed up design considerations
$request— aHEADis matched to theGEToperation, so$operation->getMethod()returnsGET)Main logic implemented
A
HEADshort-circuit at each of the 3 points where a response body is born — right afteroriginal_datais captured (so headers still compute) and before any iteration:SerializeProcessor— standard serialization (all formats) → forwards an empty body.Hydra\State\JsonStreamerProcessor&Serializer\State\JsonStreamerProcessor— thejsonStream: truepaths, which build the body independently ofSerializeProcessor(and, in listener mode, via their ownkernel.viewlistener — hence their own guard).Detection is
$request->isMethod('HEAD'): the operation can't tell us, by construction.Tests
HeadRequestTest(functional) — backed by a spy paginator whosegetIterator()/count()throw, so aHEADreturning200proves the collection was never iterated (a naive "200 + empty body" test would pass even if theSELECTran). Covers: collectionHEAD(no iteration),GET(does iterate → sanity check, keeps the test non-vacuous),OPTIONS(unaffected), and ajsonStream: trueresource.SerializeProcessorTest(unit) — asserts the serializer is never called onHEAD.Limitations & concerns
Content-Lengthis no longer set onHEAD(we don't build the body to measure it). Allowed by RFC 9110 §9.3.2, but a small observable change for strict clients._resources, surrogate keys) aren't populated onHEADsince serialization is skipped → cache-tag headers may differ betweenGETandHEAD. Fine IMO (aHEADhas no rows), but worth flagging.securityexpression on a collection that iteratesobjectstill triggers theSELECT— it runs in the provider, before the processors, so this optimization can't prevent it. The common case (is_granted('ROLE_X')) is unaffected.HEADon an item saves nothing (the single row is already fetched by the provider); the body is still skipped, but there's no DB win there, for now. The whole point of this PR is collections.