8000 Allow empty string identifiers by Ocramius · Pull Request #5924 · doctrine/orm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow empty string identifiers #5924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Jul 7, 2016
Merged

Allow empty string identifiers #5924

merged 14 commits into from
Jul 7, 2016

Conversation

Ocramius
Copy link
Member
@Ocramius Ocramius commented Jul 7, 2016

This PR fixes a subtle bug that disallowed the UnitOfWork from containing entities with a '' identifier.

Note: this fix targets ORM 2.6, because empty identifiers are still subject to bug #5923

@Ocramius Ocramius added the Bug label Jul 7, 2016
@Ocramius Ocramius added this to the 2.6.0 milestone Jul 7, 2016
Ocramius added 4 commits July 7, 2016 20:39
@Ocramius
Removing note that is not valid anymore
7544934
@Ocramius
Registering a managed entity with an empty identifier is to be disall…
0d82128 
…owed
@Ocramius
Empty identifiers must be disallowed
dbcdc1d
@Ocramius
Rewrote ManyToOne SLC tests to not rely on multi-level auto-generat…
5d12593 
…ed identifiers

Background:

Test relied on an `A->B->C` association:

 * `A#id` being `B`
 * `B#id` being `C`
 * `C#id` being an auto-generated identifier (post-insert)

This cannot work, because it breaks the UnitOfWork's identity map.
Specifically, no entries for `A` and `B` can exist in the identity map until `C` entries
are persisted (post-insert).

That means that the identifier generator for `A` and `B` should not be an "assigned"
generator, but should instead be a post-insert generator waiting for other entities
to be persisted.

We cannot fix this in ORM 2.x, but we'll need to invent something for 3.x in order to
fix that (directed graph, or caching the order of operations in the metadata graph).
@Ocramius Ocramius mentioned this pull request Jul 7, 2016
Closed
@deeky666 deeky666 merged commit 355d2c3 into master Jul 7, 2016
@deeky666
Copy link
Member
deeky666 commented Jul 7, 2016

hiding

@Ocramius Ocramius deleted the fix/allow-empty-identifier-string-as-proxy-identifier branch July 7, 2016 21:12
@Ocramius
Copy link
Member Author
Ocramius commented Jul 7, 2016

@deeky666 thanks!

@Ocramius Ocramius assigned deeky666 and unassigned guilhermeblanco Jul 7, 2016
@vhenzl vhenzl mentioned this pull request Sep 22, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@deeky666 deeky666

Labels
Bug
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Ocramius @deeky666 @guilhermeblanco
0