doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 502 forks source link

WriteConcern option never passes into MongoDB #2292

Closed ossinkine closed 3 years ago

ossinkine commented 3 years ago

Bug Report

Q A
BC Break no
Version 2.2.0

Summary

I'm sorry in advance if I've just not figured out. I want to be sure the transaction was processed on majority replicas, I've set up w: majority option in doctrine_mongodb.default_commit_options, and try to debug where this option passed into MongoDB and did not find any cases. DocumentPersister::getWriteOptions returns ['w' => 'majority'] and then options passew to MongoDB\Collection::updateOne which never usew w key, it expects writeConcern key with object instance of MongoDB\Driver\WriteConcern.

Current behavior

WriteConcern option never passes into MongoDB.

How to reproduce

Just try to pass w as write options and check it's never used in mongodb/mongodb.

Expected behavior

WriteConcern correctly passes info mongodb/mongodb and exists in request.

alcaeus commented 3 years ago

Now that you mention it...updating the commit options to use WriteConcern has completely escaped my mind when updating to the new driver. Marking as a bug and will start fixing soon.

ossinkine commented 3 years ago

@alcaeus passing w option in DocumentManager::flush or specifying in Document annotation also does not work. What is a true way to pass it?

alcaeus commented 3 years ago

The true way is to pass a write concern option:

$dm->flush(['writeConcern' => new \MongoDB\Driver\WriteConcern('majority')])
ossinkine commented 3 years ago

Looks like this method should be removed or refactored because it works wrong and consfusing

alcaeus commented 3 years ago

You've found the source of the problem. Want to create a PR to fix the issue? :)

ossinkine commented 3 years ago

Yes I can if you describe the correct behavior in your opinion

alcaeus commented 3 years ago

Instead of passing on the w key, the method you pointed out should create a WriteConcern object and trigger a deprecation to let everyone know that passing a w key is no longer supported. There are some places where ODM assumes that as default, so be sure to check those out as well. Please let me know (here or in the doctrine or Symfony-devs slacks) if you run into any problems or have questions.

ossinkine commented 3 years ago

@alcaeus Please tell which base branch I should use to fix

alcaeus commented 3 years ago

2.2.x is our current branch for bug fixing. Thanks!

alcaeus commented 3 years ago

Fixed in #2294.