Permalink
Browse files

detect empty node name in persist cascade early

  • Loading branch information...
1 parent b4f40fd commit a4075caadd97312cc53bccf57ff8baf593df65d4 @dbu dbu committed Mar 13, 2013
Showing with 6 additions and 2 deletions.
  1. +2 −2 lib/Doctrine/ODM/PHPCR/Id/IdException.php
  2. +4 −0 lib/Doctrine/ODM/PHPCR/UnitOfWork.php
@@ -28,11 +28,11 @@ public static function noIdNoParent($document, $parent)
return new self($message);
}
- public static function noIdNoName($document, $nodename)
+ public static function noIdNoName($document, $fieldName)
{
$message = sprintf(
'NodeName property "%s" may not be empty in document of class "%s"',
- $nodename,
+ $fieldName,
get_class($document)
);
@@ -20,6 +20,7 @@
namespace Doctrine\ODM\PHPCR;
use Doctrine\ODM\PHPCR\Mapping\ClassMetadata;
+use Doctrine\ODM\PHPCR\Id\IdException;
use Doctrine\Common\Util\ClassUtils;
use Doctrine\ODM\PHPCR\Mapping\MappingException;
use Doctrine\ODM\PHPCR\Id\IdGenerator;
@@ -675,6 +676,9 @@ private function cascadeScheduleInsert($class, $document, &$visited)
$nodename = $childClass->nodename
? $childClass->reflFields[$childClass->nodename]->getValue($child)
: basename($childClass->getIdentifierValue($child));
+ if (empty($nodename)) {
+ throw IdException::noIdNoName($child, $childClass->nodename);
+ }
$childId = $id.'/'.$nodename;
$childClass->setIdentifierValue($child, $childId);
@dbu

dbu Mar 14, 2013

Member

@lsmith77 actually, should we not make sure the parent is correct, then use the id strategy and check if the generated id is legal (child of that parent)? though for assigned id strategy this would be annoying. should we differentiate the cases? also, we could use the collection key if there is no other name - for existing children, the ChildrenCollection uses the nodename as key...

if we implement http://www.doctrine-project.org/jira/browse/PHPCR-103 as id strategy this code here would fail.

@lsmith77

lsmith77 Mar 14, 2013

Member

you mean check if there is a mismatch between the state of the child and the parent its assigned to?
and yes using the collection key as another way to determine the nodename makes sense .. i think there is even a helper method used in the computeChangeSet() method for this purpose.

@dbu

dbu via email Mar 14, 2013

Member
@lsmith77

lsmith77 Mar 14, 2013

Member

when do we trigger the strategy when persisting a new document? when we call persit() or when we executeInserts()?

@dbu

dbu via email Mar 14, 2013

Member
$this->doScheduleInsert($child, $visited, ClassMetadata::GENERATOR_TYPE_ASSIGNED);

0 comments on commit a4075ca

Please sign in to comment.