Skip to content
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

Fix wrong handling for nullable fields in upsert and update #2318

Merged
merged 2 commits into from
Jun 15, 2021
Merged

Fix wrong handling for nullable fields in upsert and update #2318

merged 2 commits into from
Jun 15, 2021

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented May 31, 2021 •
edited
Loading

Q A
Type bug
BC Break no
Fixed issues Fixes #2310

Summary

#2310 reported another instance of #2301, but on closer inspection it turns out that this was not the same bug but the same symptoms. The bug was wrong handling of null values during an upsert. I found two separate issues:

  1. Nullable embedded fields are not set to null when the upsert results in a new document, which causes the bug described in Nullable embedded document not initialized when using aggregations #2310,
  2. Nullable embedded fields are not set to null on update but rather unset, which also causes the bug described in Nullable embedded document not initialized when using aggregations #2310,
  3. Nullable references are always set to null on upsert when they shouldn't.

I've kept the tests added in #2316 and expanded them to cover all insert, upsert, and update scenarios. I've also added new tests to UpsertTest to specifically test for the behaviour unrelated to typed properties.

@alcaeus alcaeus added the Bug label May 31, 2021
@alcaeus alcaeus added this to the 2.2.2 milestone May 31, 2021
@alcaeus alcaeus requested a review from malarzm May 31, 2021 10:01
@alcaeus alcaeus self-assigned this May 31, 2021
@alcaeus alcaeus changed the title Fix wrong handling for nullable fields in upsert Fix wrong handling for nullable fields in upsert and update May 31, 2021
@alcaeus alcaeus mentioned this pull request May 31, 2021
Closed
@alcaeus
Copy link
Member Author

alcaeus commented May 31, 2021

I'll have to give the CI pipeline for 2.x a look...

@alcaeus @wuchen90
Comprehensively test nullable behaviour for embedOne
0d145dd 
Co-authored-by: wuchen90 <wu.chen@agriconomie.com>
@alcaeus alcaeus force-pushed the gh-2310 branch 4 times, most recently from fc8be44 to 36e03a6 Compare June 2, 2021 11:47
@alcaeus
Copy link
Member Author

alcaeus commented Jun 2, 2021

@malarzm finally managed to fix the bug and the build, so this is ready for a review.

malarzm
malarzm approved these changes Jun 8, 2021
View reviewed changes
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

This one ended up bigger than I've expected :)

@alcaeus alcaeus merged commit 5aa06c0 into doctrine:2.2.x Jun 15, 2021
@alcaeus alcaeus deleted the gh-2310 branch June 15, 2021 08:36
alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jun 29, 2021
@alcaeus
Merge branch '2.2.x' into 2.3.x
0c4407c 
* 2.2.x:
  Fix handling of upserts during scheduling for deletion (doctrine#2334)
  Fix wrong handling for nullable fields in upsert and update (doctrine#2318)
  [2.2] Fix builds (doctrine#2319)
  Allow mixed value in $not operator (doctrine#2307)
  Fix errors with nullable typed associations (doctrine#2302)
  Fix using null values in partial filter expressions (doctrine#2300)
  Fix preparation of $elemMatch operators in queries (doctrine#2298)
  Fix query preparation when in elemMatch (doctrine#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (doctrine#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jun 29, 2021
@alcaeus
Merge branch '2.2.x' into 2.3.x
7919638 
* 2.2.x:
  Fix handling of upserts during scheduling for deletion (doctrine#2334)
  Fix wrong handling for nullable fields in upsert and update (doctrine#2318)
  [2.2] Fix builds (doctrine#2319)
  Allow mixed value in $not operator (doctrine#2307)
  Fix errors with nullable typed associations (doctrine#2302)
  Fix using null values in partial filter expressions (doctrine#2300)
  Fix preparation of $elemMatch operators in queries (doctrine#2298)
  Fix query preparation when in elemMatch (doctrine#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (doctrine#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jun 29, 2021
@alcaeus
Merge branch '2.2.x' into 2.3.x
b6f3f32 
* 2.2.x:
  Fix handling of upserts during scheduling for deletion (doctrine#2334)
  Fix wrong handling for nullable fields in upsert and update (doctrine#2318)
  [2.2] Fix builds (doctrine#2319)
  Allow mixed value in $not operator (doctrine#2307)
  Fix errors with nullable typed associations (doctrine#2302)
  Fix using null values in partial filter expressions (doctrine#2300)
  Fix preparation of $elemMatch operators in queries (doctrine#2298)
  Fix query preparation when in elemMatch (doctrine#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (doctrine#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Jun 29, 2021
@alcaeus
Merge branch '2.2.x' into 2.3.x
c186b46 
* 2.2.x:
  Fix handling of upserts during scheduling for deletion (doctrine#2334)
  Fix wrong handling for nullable fields in upsert and update (doctrine#2318)
  [2.2] Fix builds (doctrine#2319)
  Allow mixed value in $not operator (doctrine#2307)
  Fix errors with nullable typed associations (doctrine#2302)
  Fix using null values in partial filter expressions (doctrine#2300)
  Fix preparation of $elemMatch operators in queries (doctrine#2298)
  Fix query preparation when in elemMatch (doctrine#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (doctrine#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
alcaeus added a commit to alcaeus/mongodb-odm that referenced this pull request Aug 2, 2021
@alcaeus
Merge branch '2.2.x' into merge-2.2.x-into-2.3.x
f013d11 
* 2.2.x:
  Fix wrong assertion (doctrine#2335)
  Fix handling of upserts during scheduling for deletion (doctrine#2334)
  Fix wrong handling for nullable fields in upsert and update (doctrine#2318)
  [2.2] Fix builds (doctrine#2319)
  Allow mixed value in $not operator (doctrine#2307)
  Fix errors with nullable typed associations (doctrine#2302)
  Fix using null values in partial filter expressions (doctrine#2300)
  Fix preparation of $elemMatch operators in queries (doctrine#2298)
  Fix query preparation when in elemMatch (doctrine#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (doctrine#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
alcaeus added a commit that referenced this pull request Aug 5, 2021
@alcaeus @buffcode
@malarzm @SenseException @ossinkine @rrajkomar @wuchen90 @franmomu @webmozart
Merge 2.2.x into 2.3.x (#2350)
08b3799 
* Fix locking when ClassMetadata is unserialized

Caching / unserializing ClassMetadata broke locking functionality

Fixes #2278

* Test serialization of lock/version fields

* Update working-with-objects.rst

Detach doc text from code block

* Update storage-strategies.rst

* Fix invalid strict comparison when validating mappings

* Correctly handle write concern specified in defaultCommitOptions (#2294)

* Fix documentation for uploadFromFile

* Fix mapping of the nullable option for XML driver

* Fix query preparation when in elemMatch (#2299)

* Fix preparation of $elemMatch operators in queries (#2298)

* Fix using null values in partial filter expressions (#2300)

* Fix errors with nullable typed associations (#2302)

* Fix initialising nullable associations

* Fix error when merging documents with uninitialised typed properties

* Allow mixed value in $not operator (#2307)

* [2.2] Fix builds (#2319)

* Fix wrong handling for nullable fields in upsert and update (#2318)

* Comprehensively test nullable behaviour for embedOne

Co-authored-by: wuchen90 <wu.chen@agriconomie.com>

* Fix handling of nullable fields for upsert

Co-authored-by: wuchen90 <wu.chen@agriconomie.com>

* Fix handling of upserts during scheduling for deletion (#2334)

* Fix handling of upserts during scheduling for deletion

* Added test

* Fix wrong assertion (#2335)

This was uncovered by Psalm testing when merging 2.2.x up into 2.3.x.

* Remove psalm-baseline.xml

Co-authored-by: buffcode <buffcode@users.noreply.github.com>
Co-authored-by: Laurens Stötzel <l.stoetzel@meeva.de>
Co-authored-by: Maciej Malarz <malarzm@gmail.com>
Co-authored-by: jeeiex <78592605+jeeiex@users.noreply.github.com>
Co-authored-by: Claudio Zizza <859964+SenseException@users.noreply.github.com>
Co-authored-by: Gocha Ossinkine <ossinkine@ya.ru>
Co-authored-by: Ryan RAJKOMAR <rrajkomar@users.noreply.github.com>
Co-authored-by: wuchen90 <wu.chen@agriconomie.com>
Co-authored-by: Fran Moreno <franmomu@gmail.com>
Co-authored-by: Bernhard Schussek <bschussek@gmail.com>
@github-actions github-actions bot mentioned this pull request Aug 5, 2021
Merged
alcaeus added a commit that referenced this pull request Aug 5, 2021
@alcaeus
Merge branch '2.2.x' into 2.2.x-merge-up-into-2.3.x_iA5R4YuE
81b0647 
* 2.2.x:
  Fix wrong assertion (#2335)
  Fix handling of upserts during scheduling for deletion (#2334)
  Fix wrong handling for nullable fields in upsert and update (#2318)
  [2.2] Fix builds (#2319)
  Allow mixed value in $not operator (#2307)
  Fix errors with nullable typed associations (#2302)
  Fix using null values in partial filter expressions (#2300)
  Fix preparation of $elemMatch operators in queries (#2298)
  Fix query preparation when in elemMatch (#2299)
  Fix mapping of the nullable option for XML driver
  Fix documentation for uploadFromFile
  Correctly handle write concern specified in defaultCommitOptions (#2294)
  Fix invalid strict comparison when validating mappings
  Update storage-strategies.rst
  Update working-with-objects.rst
  Test serialization of lock/version fields
  Fix locking when ClassMetadata is unserialized
@ivan-wolf ivan-wolf mentioned this pull request Apr 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malarzm malarzm malarzm approved these changes

Assignees

@alcaeus alcaeus

Labels
Bug
Projects
None yet
Milestone
2.2.2
Development

Successfully merging this pull request may close these issues.

Nullable embedded document not initialized when using aggregations
2 participants
@alcaeus @malarzm