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
Hotfix for #164: PHPCRLoader should be transactional #165
Hotfix for #164: PHPCRLoader should be transactional #165
Conversation
$dm->expects($this->once())->method('transactional')->will($this->throwException($exception)); | ||
$purger->expects($this->never())->method('purge'); | ||
|
||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just $this->setExpectedException()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because setExpectedException()
just checks for the exception type: I'm explicitly checking for the same exception
i guess you need to add a conflicts statement to composer.json so that this is not used with phpcr-odm < 1.2 (to have transaction support) |
|
||
$this->dm->transactional(function ($dm) use ($purger, $append, $that, $fixtures) { | ||
if ($append === false) { | ||
$this->purge(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should use $that
, not $this
for PHP 5.3 support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! That's my f-up because I tested it locally with 5.6.3
@Ocramius do you plan to clean this PR up? (btw, i have no commit rights on this repository, so i won't merge this ;-) |
@dbu I'm planning to do once PHPCR-ODM 1.3 is out |
any news ? |
@Ocramius Could you squash and rebase your commits? |
…ingle_ the `DocumentManager#transactional()` call as of doctrine/phpcr-odm#582
…tManager#transactional()` call as of doctrine/phpcr-odm#582
…entManager#transactional()`fails, as of doctrine/phpcr-odm#582
…nager#transactional()` call as of doctrine/phpcr-odm#582
c173438
to
aa58c8e
Compare
…DocumentManager` definitions are available
… in closure scope
return; | ||
} | ||
|
||
// hold my beer while I do some mocking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to :-P
…ip-1.1 * Ocramius-hotfix/#164-phpcr-loader-transactional: #164 #165 - PHP 5.3 compat: `$this` is not accessible in closure scope #164 #165 - making sure that the `Doctrine\ODM\PHPCR\DocumentManager` definitions are available #164 - Wrap `PHPCRExecutor#execute()` body with a `DocumentManager#transactional()` call as of doctrine/phpcr-odm#582 #164 - PHPCR executor should stop loading/purging when `DocumentManager#transactional()`fails, as of doctrine/phpcr-odm#582 #164 - PHPCR executor should skip purging when told to do so #164 - PHPCR executor should expect purging to use a `DocumentManager#transactional()` call as of doctrine/phpcr-odm#582 #164 - PHPCR purger should expect fixture loading to use _a single_ the `DocumentManager#transactional()` call as of doctrine/phpcr-odm#582 #164 - PHPCR purger should expect fixture loading to use the transactional of doctrine/phpcr-odm#582
$fixture->expects($this->once())->method('load')->with($dm); | ||
$dm | ||
->expects($this->once()) | ||
->method('transactional') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is hiding symfony-cmf/symfony-cmf#217 :-P
Ping @ElectricMaxxx
See #164
see doctrine/phpcr-odm#582