Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"phpcr/phpcr-implementation": "^2.1.0",
"phpcr/phpcr-utils": "^1.2.8",
"doctrine/instantiator": "^1.0.1",
"symfony/console": "^2.3|^3.0"
"symfony/console": "^2.3|^3.0",
"jackalope/jackalope-doctrine-dbal": "~1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this should be here...

},
"require-dev": {
"symfony/yaml": "^2.3|^3.0",
Expand Down
8 changes: 3 additions & 5 deletions lib/Doctrine/ODM/PHPCR/DocumentManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -917,13 +917,11 @@ public function initializeObject($document)
*/
public function getNodeForDocument($document)
{
if (!is_object($document)) {
throw new InvalidArgumentException('Parameter $document needs to be an object, '.gettype($document).' given');
if (!$identifier = $this->unitOfWork->getDocumentId($document)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we remove this validation?

Copy link
Contributor

Choose a reason for hiding this comment

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

if (null === $identifier = $this->...

throw new InvalidArgumentException('This document is not managed by this manager.');
Copy link
Contributor

Choose a reason for hiding this comment

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

sprintf('Document of class "%s" is not managed', get_class($document))

}

$path = $this->unitOfWork->getDocumentId($document);

return $this->session->getNode($path);
return $this->unitOfWork->getNodeByPathOrUuid($identifier);
}

/**
Expand Down
41 changes: 34 additions & 7 deletions lib/Doctrine/ODM/PHPCR/UnitOfWork.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use Doctrine\ODM\PHPCR\Exception\RuntimeException;
use Doctrine\ODM\PHPCR\Id\AssignedIdGenerator;
use Doctrine\ODM\PHPCR\Id\IdException;
use Doctrine\ODM\PHPCR\Mapping\Annotations\Id;
use Doctrine\ODM\PHPCR\Mapping\ClassMetadata;
use Doctrine\ODM\PHPCR\Mapping\MappingException;
use Doctrine\ODM\PHPCR\Id\IdGenerator;
Expand Down Expand Up @@ -674,7 +675,7 @@ public function getOrCreateProxy($targetId, $className, $locale = null)
$metadata = $this->dm->getClassMetadata($className);
$proxyDocument = $this->dm->getProxyFactory()->getProxy($className, array($metadata->identifier => $targetId));

// register the document under its own id
// register the document under its own id/uuid
Copy link
Contributor

Choose a reason for hiding this comment

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

path/uuid.

$this->registerDocument($proxyDocument, $targetId);

if ($locale) {
Expand All @@ -692,7 +693,12 @@ public function getOrCreateProxy($targetId, $className, $locale = null)
*/
public function refreshDocumentForProxy($className, Proxy $document)
{
$node = $this->session->getNode($this->determineDocumentId($document));
$pathOrUuid = $this->determineDocumentId($document);
$node = $this->getNodeByPathOrUuid($pathOrUuid);
if (UUIDHelper::isUUID($pathOrUuid)) {
Copy link
Contributor

@dantleech dantleech Jan 17, 2017

Choose a reason for hiding this comment

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

Empty line before if statement

$this->unregisterDocument($document);
$this->registerDocument($document, $node->getPath());
}

$hints = array('refresh' => true, 'fallback' => true);

Expand Down Expand Up @@ -3063,18 +3069,25 @@ private function unregisterDocument($document)

/**
* @param object $document
* @param string $id The document id to look for.
* @param string $pathOrUuid The document id to look for.
*
* @return string generated object hash
*/
public function registerDocument($document, $id)
public function registerDocument($document, $pathOrUuid)
{
$oid = spl_object_hash($document);
$this->documentIds[$oid] = $id;
$this->identityMap[$id] = $document;
$this->documentIds[$oid] = $pathOrUuid;
$this->identityMap[$pathOrUuid] = $document;

// frozen nodes need another state so they are managed but not included for updates
$frozen = $this->session->nodeExists($id) && $this->session->getNode($id)->isNodeType('nt:frozenNode');
try {
$node = $this->getNodeByPathOrUuid($pathOrUuid);
$frozen = $node instanceof NodeInterface ? $node->isNodeType('nt:frozenNode') : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

If the node is not an instance of NodeInterface then what is it? If we are expecting null then I think we should check forr null.

} catch (ItemNotFoundException $e) {
$frozen = false;
} catch (PathNotFoundException $e) {
$frozen = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

$frozen = false can be moved before the try block (doesn't need to be repeated twice)

}

$this->setDocumentState($oid, $frozen ? self::STATE_FROZEN : self::STATE_MANAGED);

Expand Down Expand Up @@ -3112,6 +3125,20 @@ public function getDocumentById($id)
return false;
}

/**
* Creates a node from a given path or an uuid
Copy link
Contributor

Choose a reason for hiding this comment

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

given path or UUID

*
* @param $pathOrUuid
*
* @return NodeInterface
*/
public function getNodeByPathOrUuid($pathOrUuid)
{
return UUIDHelper::isUUID($pathOrUuid)
? $this->session->getNodeByIdentifier($pathOrUuid)
: $this->session->getNode($pathOrUuid);
}

/**
* Get the object ID for the given document
*
Expand Down
54 changes: 44 additions & 10 deletions tests/Doctrine/Tests/ODM/PHPCR/UnitOfWorkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Doctrine\ODM\PHPCR\Mapping\ClassMetadata;
use Jackalope\Factory;
use Jackalope\Node;
use PHPCR\Util\UUIDHelper;

/**
* TODO: remove Jackalope dependency
Expand Down Expand Up @@ -56,7 +57,7 @@ public function setUp()
$this->session = $this->getMockBuilder('Jackalope\Session')
->disableOriginalConstructor()
->getMock();

$this->objectManager = $this->getMockBuilder('Jackalope\ObjectManager')
->disableOriginalConstructor()
->getMock();
Expand All @@ -74,54 +75,66 @@ public function setUp()
$cmf->setMetadataFor($this->type, $metadata);
}

protected function createNode($id, $username, $primaryType = 'rep:root')
protected function createNode($id, $username, $primaryType = 'rep:root', $uuid = null)
{
$repository = $this->getMockBuilder('Jackalope\Repository')->disableOriginalConstructor()->getMock();
$this->session->expects($this->any())
->method('getRepository')
->with()
->will($this->returnValue($repository));

$type = $this->getMockBuilder('Jackalope\NodeType\NodeType')->disableOriginalConstructor()->getMock();
$type->expects($this->any())
->method('getName')
->with()
->will($this->returnValue($primaryType));

$ntm = $this->getMockBuilder('Jackalope\NodeType\NodeTypeManager')->disableOriginalConstructor()->getMock();
$ntm->expects($this->any())
->method('getNodeType')
->with()
->will($this->returnValue($type));

$workspace = $this->getMockBuilder('Jackalope\Workspace')->disableOriginalConstructor()->getMock();
$workspace->expects($this->any())
->method('getNodeTypeManager')
->with()
->will($this->returnValue($ntm));

$this->session->expects($this->any())
->method('getWorkspace')
->with()
->will($this->returnValue($workspace));

$this->session->expects($this->any())
->method('nodeExists')
->with($id)
->will($this->returnValue(true));

$nodeData = array(
"jcr:primaryType" => $primaryType,
"jcr:system" => array(),
'username' => $username,
);
if (null !== $uuid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer a blank line before most if statements.

$nodeData['jcr:uuid'] = $uuid;
}

$node = new Node($this->factory, $nodeData, $id, $this->session, $this->objectManager);

$this->session->expects($this->any())
->method('getNode')
->with($id)
->will($this->returnValue($node));


if (null !== $uuid) {
$this->session->expects($this->any())
->method('getNodeByIdentifier')
->with($uuid)
->will($this->returnValue($node));
}


Copy link
Contributor

Choose a reason for hiding this comment

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

One blank line too many

return $node;
}

Expand Down Expand Up @@ -247,6 +260,27 @@ public function testComputeSingleDocumentChangeSetForRemovedDocument()
// Should not throw "InvalidArgumentException: Document has to be managed for single computation"
$this->uow->computeSingleDocumentChangeSet($object);
}

public function testGetOrCreateProxy()
{
$user = $this->uow->getOrCreateDocument($this->type, $this->createNode('/somepath', 'foo'));
$this->uow->clear();
$userAsReference = $this->uow->getOrCreateProxy($user->id, get_class($user));

$this->assertEquals(2, $this->uow->getDocumentState($userAsReference));
$this->assertEquals($userAsReference, $this->uow->getDocumentById($userAsReference->id));
}

public function testGetOrCreateProxyWithUuid()
{
$uuid = UUIDHelper::generateUUID();
$user = $this->uow->getOrCreateDocument($this->type, $this->createNode('/somepath', 'foo', 'rep:root', $uuid));
$this->uow->clear();
$userAsReference = $this->uow->getOrCreateProxy($uuid, get_class($user));

$this->assertEquals(2, $this->uow->getDocumentState($userAsReference));
$this->assertEquals($userAsReference, $this->uow->getDocumentById($uuid));
}
}

class UoWUser
Expand Down