Skip to content

Conversation

@ilyasokay
Copy link
Contributor

Transaction which is supported on Mongo v4 and above is almost ready. Just need 1-2 days to check completely. Thanks for your help @Smolevich .

@codecov-io
Copy link

codecov-io commented Apr 11, 2020

Codecov Report

Merging #2019 into master will increase coverage by 0.29%.
The diff coverage is 88.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2019      +/-   ##
============================================
+ Coverage     87.04%   87.33%   +0.29%     
- Complexity      667      680      +13     
============================================
  Files            31       31              
  Lines          1521     1556      +35     
============================================
+ Hits           1324     1359      +35     
  Misses          197      197              
Impacted Files Coverage Δ Complexity Δ
src/Jenssegers/Mongodb/Query/Builder.php 89.17% <78.94%> (+0.28%) 170.00 <1.00> (+5.00)
src/Jenssegers/Mongodb/Connection.php 92.94% <100.00%> (+1.63%) 46.00 <8.00> (+8.00)
...Jenssegers/Mongodb/MongodbQueueServiceProvider.php 100.00% <0.00%> (+20.00%) 2.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7acac7...7cbfa84. Read the comment docs.

@ilyasokay
Copy link
Contributor Author

There is still work to do with handling deadlock in transaction, especially configuring exception messages of mongodb.

@Smolevich
Copy link
Contributor

@ilyasokay do you have plans to continue work with PR?

@ilyasokay
Copy link
Contributor Author

@ilyasokay do you have plans to continue work with PR?

@Smolevich Sorry for the delay, I had some urgent works to do; but I will continue this pr as soon as possible.

@Smolevich Smolevich removed their request for review June 3, 2020 07:23

$wheres = $this->compileWheres();
$result = $this->collection->UpdateMany($wheres, $query, $options);
$result = $this->collection->UpdateMany($wheres, $query, array_merge($options, $this->session()));
Copy link
Member

Choose a reason for hiding this comment

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

As per signature below:

Suggested change
$result = $this->collection->UpdateMany($wheres, $query, array_merge($options, $this->session()));
$result = $this->collection->UpdateMany($wheres, $query, $this->session($options));

There are a few more instances of this in the class.


protected function session(array $options = [])
{
if ($session = $this->connection->getSession()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion to not override an existing session:

Suggested change
if ($session = $this->connection->getSession()) {
if (!isset($options['session']) && $session = $this->connection->getSession()) {

return new Schema\Grammar();
}

public function beginTransaction(array $options = [])
Copy link
Member

Choose a reason for hiding this comment

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

What is the expected behaviour if a transaction is already in progress?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @alcaeus , there is another PR which looks promising. If you'd like and have a time, please take a look #1904

Thanks!

@divine
Copy link
Contributor

divine commented Aug 9, 2020

Closing in favor of #1904

@divine divine closed this Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants