Conversation
This branch has not been deployed
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.
Entry::blueprint()memoizes the resolved blueprint in Blink underentry-{$this->id()}-blueprint. An unsaved entry has no id yet, so every unsaved entry in the process shares the keyentry--blueprintand reads back whichever blueprint resolved first, regardless of which collection it belongs to.In a web request this is hard to notice, since you rarely create more than one entry. In a long-lived queue worker it bites. I ran into it with an importer that creates entries in two collections from the same job: almost every entry in one collection was saved with the other collection's blueprint handle, and from then on they threw a
BlueprintNotFoundExceptionon hydration, since that handle does not exist in their own collection.Minimal reproduction, no queue needed:
Whichever entry resolves first wins the key. It is symmetric, so the direction of the corruption just depends on which collection gets touched first.
The fix falls back to the object id when there is no entry id, so each instance gets its own key and the memoization is kept as is.
The
obj-prefix keeps object ids out of the entry id namespace.spl_object_id()returns small integers and ids are reused once an object is freed, so without it an unsaved entry could take the key of a saved entry with the id1. That is reachable on the Eloquent driver, where the default entries table uses auto-incrementing ids. Doctrine keeps the same two keyspaces in separate maps for the same reason.Same shape as #15330, where runtime form blueprints shared a blank
blueprint-contents--key, and #14739, where Bard and Link shared one key for two different types.Note on the changed test
it_respects_custom_blueprint_template_path_per_collectionneeded a small fixture change. It builds an unsaved entry inpagesusing a blueprint saved intocollections.articles, whichpagescannot resolve. It only passed because the articles entry resolved first and populated the shared key, so the pages entry read the articles blueprint back out.With the fix it correctly throws
BlueprintNotFoundException, so I gave thepagescollection its own copy of the blueprint. The assertions it actually cares about, the template path prefixes, are unchanged and still pass.