Skip to content

Fix upserts for documents with YAML mapping #1440

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.

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

Merged
merged 1 commit into from
Jun 22, 2016

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Jun 20, 2016

Fixes #1435.

This also uncovered a bug where using the increment strategy for ID generation could lead to errors when upserting documents due to a name conflict with strategies.

@alcaeus alcaeus added the Bug label Jun 20, 2016
@alcaeus alcaeus added this to the 1.1.1 milestone Jun 20, 2016
@@ -146,7 +146,7 @@ public function prepareUpdateData($document)
if ($new === null && $mapping['nullable'] !== true) {
$updateData['$unset'][$mapping['name']] = true;
} else {
if ($new !== null && $mapping['strategy'] === ClassMetadataInfo::STORAGE_STRATEGY_INCREMENT) {
if ($new !== null && isset($mapping['strategy']) && $mapping['strategy'] === ClassMetadataInfo::STORAGE_STRATEGY_INCREMENT) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this line is fixing #1435 but I don't think this is covered by tests you added?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I see that now. I'll update the test, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test updated - it now checks to ensure IDs are not updated, since all other fields will have a mapping set, regardless of the driver being used.

@alcaeus alcaeus force-pushed the fix-upsert-strategy-mishap branch from 95da1e5 to 09105f3 Compare June 22, 2016 05:24
@alcaeus alcaeus force-pushed the fix-upsert-strategy-mishap branch from 09105f3 to ef6c168 Compare June 22, 2016 09:08
@malarzm malarzm merged commit 63382fb into doctrine:master Jun 22, 2016
@malarzm
Copy link
Member

malarzm commented Jun 22, 2016

1 step closer to 1.1.1 :)

@alcaeus alcaeus deleted the fix-upsert-strategy-mishap branch June 22, 2016 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants