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.

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

Fix bad transaction support #27

Merged
merged 7 commits into from
Sep 20, 2015
Merged

Fix bad transaction support #27

merged 7 commits into from
Sep 20, 2015

Conversation

prolic
Copy link
Member

@prolic prolic commented Sep 19, 2015

  • make use of $isolated operator
  • collection name is extracted from stream name
  • update readme

fixes: #26


This adapter uses die `$isolated` operator to achieve transaction safety for a single collection.
Keep in mind, that `$isolated` does not work with sharded clusters. Therefore it's not safe to use this adapter
in a shared cluster environment, as we MongoDB can't guarantee transaction safety.
Copy link
Member

Choose a reason for hiding this comment

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

"As we MongoDB" should be "As with MongoDB"?

@@ -187,12 +169,16 @@ public function create(Stream $stream)
*/
public function appendTo(StreamName $streamName, array $streamEvents)
{
$this->currentStreamName = $streamName;
Copy link
Member

Choose a reason for hiding this comment

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

We need to make sure that only one stream can be modified per transaction. So we should check here that either $this->currentStreamName is null or the same as $streamName. On transaction commit/rollback $this->currentStreamName should be reset to null

@codeliner
Copy link
Member

@prolic Great job 👍 . I think the BC break is ok here as we really need to change the behavior of the adapter. A new major version will underline the importance of the adjustments.
PR looks good. Just a minor note. Should be easy to fix.

@prolic
Copy link
Member Author

prolic commented Sep 20, 2015

I totally missed the checks for current stream name, just slipped through my mind ;)

@codeliner
Copy link
Member

Seems like travis won't start this build. However, I'm going to merge the PR and trigger a new build with the merge.

codeliner added a commit that referenced this pull request Sep 20, 2015
Fix bad transaction support
@codeliner codeliner merged commit a0da508 into prooph:develop Sep 20, 2015
@codeliner codeliner added this to the 2.0 Release milestone Sep 20, 2015
@prolic prolic deleted the isolated branch September 20, 2015 13:47
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.

None yet

3 participants