Merge pull request #11 from locomotivemtl/joel/fix-hierarchical-master

JoelAlphonso · web-flow · commit bdfc56d9b509 · 2020-11-27T15:33:52.000-05:00
fix hierarchical-trait master object loading

BREAKING: the master method now returns the master ID and masterObject returns the actual object
diff --git a/.travis.yml b/.travis.yml
@@ -1,10 +1,8 @@
 language: php
 
 php:
-    - 5.6
-    - 7.0
-    - 7.1
-    - 7.2
+    - 7.3
+    - 8.0
     - nightly
 
 env:
diff --git a/composer.json b/composer.json
@@ -13,11 +13,11 @@
     "prefer-stable": true,
     "extra": {
         "branch-alias": {
-            "dev-master": "0.7.x-dev"
+            "dev-master": "0.8.x-dev"
         }
     },
     "require": {
-        "php": ">=5.6.0 || >=7.0",
+        "php": "^7.3 || ^8.0",
         "psr/log": "^1.0",
         "locomotivemtl/charcoal-config": "~0.10",
         "locomotivemtl/charcoal-core": "~0.5",
diff --git a/src/Charcoal/Object/HierarchicalCollection.php b/src/Charcoal/Object/HierarchicalCollection.php
@@ -71,21 +71,21 @@ public function sortTree()
 
         foreach ($this->objects as $object) {
             // Repair bad hierarchy.
-            if ($object->hasMaster() && $object->getMaster()->id() === $object->id()) {
+            if ($object->hasMaster() && $object->getMaster() === $object->id()) {
                 $object->setMaster(0);
                 $object->update([ 'master' ]);
             }
 
             if ($object->hasMaster()) {
-                $childObjects[$object->getMaster()->id()][] = $object;
+                $childObjects[$object->getMaster()][] = $object;
             } else {
                 $rootObjects[] = $object;
             }
         }
 
         if (empty($rootObjects) && !empty($childObjects)) {
             foreach ($childObjects as $parentId => $children) {
-                $parentObj = $children[0]->getMaster();
+                $parentObj = $children[0]->getMasterObject();
                 $parentObj->auxiliary = true;
 
                 $rootObjects[] = $parentObj;
@@ -190,15 +190,15 @@ private function sortDescendantObjects(
             foreach ($childObjects[$parentObj->id()] as $object) {
                 if ($count === 0 && $object->hasMaster()) {
                     $myParents = [];
-                    $myParent  = $object->getMaster();
+                    $myParent  = $object->getMasterObject();
                     while ($myParent) {
                         $myParents[] = $myParent;
 
                         if (!$myParent->hasMaster()) {
                             break;
                         }
 
-                        $myParent = $myParent->getMaster();
+                        $myParent = $myParent->getMasterObject();
                     }
 
                     $numParents = count($myParents);
@@ -236,15 +236,15 @@ private function sortDescendantObjects(
                 // If the page starts in a subtree, print the parents.
                 if ($count === $start && $object->hasMaster()) {
                     $myParents = [];
-                    $myParent  = $object->getMaster();
+                    $myParent  = $object->getMasterObject();
                     while ($myParent) {
                         $myParents[] = $myParent;
 
                         if (!$myParent->hasMaster()) {
                             break;
                         }
 
-                        $myParent = $myParent->getMaster();
+                        $myParent = $myParent->getMasterObject();
                     }
 
                     $numParents = count($myParents);
diff --git a/src/Charcoal/Object/HierarchicalInterface.php b/src/Charcoal/Object/HierarchicalInterface.php
@@ -44,10 +44,17 @@ public function hierarchyLevel();
     /**
      * Retrieve this object's immediate parent.
      *
-     * @return HierarchicalInterface|null
+     * @return string|integer|null
      */
     public function getMaster();
 
+    /**
+     * Retrieve this object's immediate parent as object.
+     *
+     * @return HierarchicalInterface|null
+     */
+    public function getMasterObject();
+
     /**
      * Retrieve the top-level ancestor of this object.
      *
diff --git a/src/Charcoal/Object/HierarchicalTrait.php b/src/Charcoal/Object/HierarchicalTrait.php
@@ -16,7 +16,7 @@ trait HierarchicalTrait
     /**
      * The object's parent, if any, in the hierarchy.
      *
-     * @var HierarchicalInterface|null
+     * @var string|integer|null
      */
     protected $master;
 
@@ -41,6 +41,13 @@ trait HierarchicalTrait
      */
     private $siblings;
 
+    /**
+     * The object's parent object, if any, in the hierarchy.
+     *
+     * @var HierarchicalInterface|null
+     */
+    private $masterObject;
+
     /**
      * A store of cached objects.
      *
@@ -65,23 +72,11 @@ public function resetHierarchy()
     /**
      * Set this object's immediate parent.
      *
-     * @param  mixed $master The object's parent (or master).
-     * @throws UnexpectedValueException The current object cannot be its own parent.
+     * @param mixed $master The object's parent (or master).
      * @return HierarchicalInterface Chainable
      */
     public function setMaster($master)
     {
-        $master = $this->objFromIdent($master);
-
-        if ($master instanceof ModelInterface) {
-            if ($master->id() === $this->id()) {
-                throw new UnexpectedValueException(sprintf(
-                    'Can not be ones own parent: %s',
-                    $master->id()
-                ));
-            }
-        }
-
         $this->master = $master;
 
         $this->resetHierarchy();
@@ -92,13 +87,38 @@ public function setMaster($master)
     /**
      * Retrieve this object's immediate parent.
      *
-     * @return HierarchicalInterface|null
+     * @return string|null
      */
     public function getMaster()
     {
         return $this->master;
     }
 
+    /**
+     * Retrieve this object's immediate parent as object.
+     * @return HierarchicalInterface|null
+     * @throws UnexpectedValueException The current object cannot be its own parent.
+     */
+    public function getMasterObject()
+    {
+        if (!$this->masterObject && $this->hasMaster()) {
+            $master = $this->objFromIdent($this->getMaster());
+
+            if ($master instanceof ModelInterface) {
+                if ($master->id() === $this->id()) {
+                    throw new UnexpectedValueException(sprintf(
+                        'Can not be ones own parent: %s',
+                        $master->id()
+                    ));
+                }
+            }
+
+            $this->masterObject = $master;
+        }
+
+        return $this->masterObject;
+    }
+
     /**
      * Determine if this object has a direct parent.
      *
@@ -145,7 +165,7 @@ public function isLastLevel()
     public function hierarchyLevel()
     {
         $hierarchy = $this->hierarchy();
-        $level = (count($hierarchy) + 1);
+        $level     = (count($hierarchy) + 1);
 
         return $level;
     }
@@ -184,10 +204,10 @@ public function hierarchy()
     {
         if (!isset($this->hierarchy)) {
             $hierarchy = [];
-            $master = $this->getMaster();
+            $master    = $this->getMasterObject();
             while ($master) {
                 $hierarchy[] = $master;
-                $master = $master->getMaster();
+                $master      = $master->getMasterObject();
             }
 
             $this->hierarchy = $hierarchy;
@@ -204,6 +224,7 @@ public function hierarchy()
     public function invertedHierarchy()
     {
         $hierarchy = $this->hierarchy();
+
         return array_reverse($hierarchy);
     }
 
@@ -216,7 +237,8 @@ public function invertedHierarchy()
     public function isMasterOf($child)
     {
         $child = $this->objFromIdent($child);
-        return ($child->getMaster() == $this);
+
+        return ($child->getMaster() === $this->id());
     }
 
     /**
@@ -240,6 +262,7 @@ public function recursiveIsMasterOf($child)
     public function hasChildren()
     {
         $numChildren = $this->numChildren();
+
         return ($numChildren > 0);
     }
 
@@ -250,6 +273,7 @@ public function hasChildren()
     public function numChildren()
     {
         $children = $this->children();
+
         return count($children);
     }
 
@@ -274,13 +298,14 @@ public function setChildren(array $children)
         foreach ($children as $c) {
             $this->addChild($c);
         }
+
         return $this;
     }
 
     /**
      * @param mixed $child The child object (or ident) to add.
-     * @throws UnexpectedValueException The current object cannot be its own child.
      * @return HierarchicalInterface Chainable
+     * @throws UnexpectedValueException The current object cannot be its own child.
      */
     public function addChild($child)
     {
@@ -311,6 +336,7 @@ public function children()
         }
 
         $this->children = $this->loadChildren();
+
         return $this->children;
     }
 
@@ -329,7 +355,8 @@ public function isChildOf($master)
         if ($master === null) {
             return false;
         }
-        return ($master == $this->getMaster());
+
+        return ($master->id() === $this->getMaster());
     }
 
     /**
@@ -342,7 +369,7 @@ public function recursiveIsChildOf($master)
             return true;
         }
 
-        if ($this->hasParents() && $this->getMaster()->recursiveIsChildOf($master)) {
+        if ($this->hasParents() && $this->getMasterObject()->recursiveIsChildOf($master)) {
             return true;
         }
 
@@ -355,6 +382,7 @@ public function recursiveIsChildOf($master)
     public function hasSiblings()
     {
         $numSiblings = $this->numSiblings();
+
         return ($numSiblings > 1);
     }
 
@@ -364,6 +392,7 @@ public function hasSiblings()
     public function numSiblings()
     {
         $siblings = $this->siblings();
+
         return count($siblings);
     }
 
@@ -376,7 +405,7 @@ public function siblings()
         if ($this->siblings !== null) {
             return $this->siblings;
         }
-        $master = $this->getMaster();
+        $master = $this->getMasterObject();
         if ($master === null) {
             // Todo: return all top-level objects.
             $siblings = [];
@@ -385,6 +414,7 @@ public function siblings()
             $siblings = $master->children();
         }
         $this->siblings = $siblings;
+
         return $this->siblings;
     }
 
@@ -395,13 +425,14 @@ public function siblings()
     public function isSiblingOf($sibling)
     {
         $sibling = $this->objFromIdent($sibling);
-        return ($sibling->getMaster() == $this->getMaster());
+
+        return ($sibling->getMaster() === $this->getMaster());
     }
 
     /**
      * @param mixed $ident The ident.
-     * @throws InvalidArgumentException If the identifier is not a scalar value.
      * @return HierarchicalInterface|null
+     * @throws InvalidArgumentException If the identifier is not a scalar value.
      */
     private function objFromIdent($ident)
     {
diff --git a/tests/Charcoal/Object/HierarchicalTraitTest.php b/tests/Charcoal/Object/HierarchicalTraitTest.php

-Original file line number
+Diff line change
@@ @@ -1,10 +1,8 @@ @@
 language: php
 php:
 -    - 5.6
 -    - 7.0
 -    - 7.1
 -    - 7.2
 +    - 7.3
 +    - 8.0
     - nightly
 env:
Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ trait HierarchicalTrait`
`16`	`16`	`/**`
`17`	`17`	`* The object's parent, if any, in the hierarchy.`
`18`	`18`	`*`
`19`		`- * @var HierarchicalInterface\|null`
	`19`	`+ * @var string\|integer\|null`
`20`	`20`	`*/`
`21`	`21`	`protected $master;`
`22`	`22`
`@@ -41,6 +41,13 @@ trait HierarchicalTrait`
`41`	`41`	`*/`
`42`	`42`	`private $siblings;`
`43`	`43`
	`44`	`+ /**`
	`45`	`+ * The object's parent object, if any, in the hierarchy.`
	`46`	`+ *`
	`47`	`+ * @var HierarchicalInterface\|null`
	`48`	`+ */`
	`49`	`+ private $masterObject;`
	`50`	`+`
`44`	`51`	`/**`
`45`	`52`	`* A store of cached objects.`
`46`	`53`	`*`
`@@ -65,23 +72,11 @@ public function resetHierarchy()`
`65`	`72`	`/**`
`66`	`73`	`* Set this object's immediate parent.`
`67`	`74`	`*`
`68`		`- * @param mixed $master The object's parent (or master).`
`69`		`- * @throws UnexpectedValueException The current object cannot be its own parent.`
	`75`	`+ * @param mixed $master The object's parent (or master).`
`70`	`76`	`* @return HierarchicalInterface Chainable`
`71`	`77`	`*/`
`72`	`78`	`public function setMaster($master)`
`73`	`79`	`{`
`74`		`- $master = $this->objFromIdent($master);`
`75`		`-`
`76`		`- if ($master instanceof ModelInterface) {`
`77`		`- if ($master->id() === $this->id()) {`
`78`		`- throw new UnexpectedValueException(sprintf(`
`79`		`- 'Can not be ones own parent: %s',`
`80`		`- $master->id()`
`81`		`- ));`
`82`		`- }`
`83`		`- }`
`84`		`-`
`85`	`80`	`$this->master = $master;`
`86`	`81`
`87`	`82`	`$this->resetHierarchy();`
`@@ -92,13 +87,38 @@ public function setMaster($master)`
`92`	`87`	`/**`
`93`	`88`	`* Retrieve this object's immediate parent.`
`94`	`89`	`*`
`95`		`- * @return HierarchicalInterface\|null`
	`90`	`+ * @return string\|null`
`96`	`91`	`*/`
`97`	`92`	`public function getMaster()`
`98`	`93`	`{`
`99`	`94`	`return $this->master;`
`100`	`95`	`}`
`101`	`96`
	`97`	`+ /**`
	`98`	`+ * Retrieve this object's immediate parent as object.`
	`99`	`+ * @return HierarchicalInterface\|null`
	`100`	`+ * @throws UnexpectedValueException The current object cannot be its own parent.`
	`101`	`+ */`
	`102`	`+ public function getMasterObject()`
	`103`	`+ {`
	`104`	`+ if (!$this->masterObject && $this->hasMaster()) {`
	`105`	`+ $master = $this->objFromIdent($this->getMaster());`
	`106`	`+`
	`107`	`+ if ($master instanceof ModelInterface) {`
	`108`	`+ if ($master->id() === $this->id()) {`
	`109`	`+ throw new UnexpectedValueException(sprintf(`
	`110`	`+ 'Can not be ones own parent: %s',`
	`111`	`+ $master->id()`
	`112`	`+ ));`
	`113`	`+ }`
	`114`	`+ }`
	`115`	`+`
	`116`	`+ $this->masterObject = $master;`
	`117`	`+ }`
	`118`	`+`
	`119`	`+ return $this->masterObject;`
	`120`	`+ }`
	`121`	`+`
`102`	`122`	`/**`
`103`	`123`	`* Determine if this object has a direct parent.`
`104`	`124`	`*`
`@@ -145,7 +165,7 @@ public function isLastLevel()`
`145`	`165`	`public function hierarchyLevel()`
`146`	`166`	`{`
`147`	`167`	`$hierarchy = $this->hierarchy();`
`148`		`- $level = (count($hierarchy) + 1);`
	`168`	`+ $level = (count($hierarchy) + 1);`
`149`	`169`
`150`	`170`	`return $level;`
`151`	`171`	`}`
`@@ -184,10 +204,10 @@ public function hierarchy()`
`184`	`204`	`{`
`185`	`205`	`if (!isset($this->hierarchy)) {`
`186`	`206`	`$hierarchy = [];`
`187`		`- $master = $this->getMaster();`
	`207`	`+ $master = $this->getMasterObject();`
`188`	`208`	`while ($master) {`
`189`	`209`	`$hierarchy[] = $master;`
`190`		`- $master = $master->getMaster();`
	`210`	`+ $master = $master->getMasterObject();`
`191`	`211`	`}`
`192`	`212`
`193`	`213`	`$this->hierarchy = $hierarchy;`
`@@ -204,6 +224,7 @@ public function hierarchy()`
`204`	`224`	`public function invertedHierarchy()`
`205`	`225`	`{`
`206`	`226`	`$hierarchy = $this->hierarchy();`
	`227`	`+`
`207`	`228`	`return array_reverse($hierarchy);`
`208`	`229`	`}`
`209`	`230`
`@@ -216,7 +237,8 @@ public function invertedHierarchy()`
`216`	`237`	`public function isMasterOf($child)`
`217`	`238`	`{`
`218`	`239`	`$child = $this->objFromIdent($child);`
`219`		`- return ($child->getMaster() == $this);`
	`240`	`+`
	`241`	`+ return ($child->getMaster() === $this->id());`
`220`	`242`	`}`
`221`	`243`
`222`	`244`	`/**`
`@@ -240,6 +262,7 @@ public function recursiveIsMasterOf($child)`
`240`	`262`	`public function hasChildren()`
`241`	`263`	`{`
`242`	`264`	`$numChildren = $this->numChildren();`
	`265`	`+`
`243`	`266`	`return ($numChildren > 0);`
`244`	`267`	`}`
`245`	`268`
`@@ -250,6 +273,7 @@ public function hasChildren()`
`250`	`273`	`public function numChildren()`
`251`	`274`	`{`
`252`	`275`	`$children = $this->children();`
	`276`	`+`
`253`	`277`	`return count($children);`
`254`	`278`	`}`
`255`	`279`
`@@ -274,13 +298,14 @@ public function setChildren(array $children)`
`274`	`298`	`foreach ($children as $c) {`
`275`	`299`	`$this->addChild($c);`
`276`	`300`	`}`
	`301`	`+`
`277`	`302`	`return $this;`
`278`	`303`	`}`
`279`	`304`
`280`	`305`	`/**`
`281`	`306`	`* @param mixed $child The child object (or ident) to add.`
`282`		`- * @throws UnexpectedValueException The current object cannot be its own child.`
`283`	`307`	`* @return HierarchicalInterface Chainable`
	`308`	`+ * @throws UnexpectedValueException The current object cannot be its own child.`
`284`	`309`	`*/`
`285`	`310`	`public function addChild($child)`
`286`	`311`	`{`
`@@ -311,6 +336,7 @@ public function children()`
`311`	`336`	`}`
`312`	`337`
`313`	`338`	`$this->children = $this->loadChildren();`
	`339`	`+`
`314`	`340`	`return $this->children;`
`315`	`341`	`}`
`316`	`342`
`@@ -329,7 +355,8 @@ public function isChildOf($master)`
`329`	`355`	`if ($master === null) {`
`330`	`356`	`return false;`
`331`	`357`	`}`
`332`		`- return ($master == $this->getMaster());`
	`358`	`+`
	`359`	`+ return ($master->id() === $this->getMaster());`
`333`	`360`	`}`
`334`	`361`
`335`	`362`	`/**`
`@@ -342,7 +369,7 @@ public function recursiveIsChildOf($master)`
`342`	`369`	`return true;`
`343`	`370`	`}`
`344`	`371`
`345`		`- if ($this->hasParents() && $this->getMaster()->recursiveIsChildOf($master)) {`
	`372`	`+ if ($this->hasParents() && $this->getMasterObject()->recursiveIsChildOf($master)) {`
`346`	`373`	`return true;`
`347`	`374`	`}`
`348`	`375`
`@@ -355,6 +382,7 @@ public function recursiveIsChildOf($master)`
`355`	`382`	`public function hasSiblings()`
`356`	`383`	`{`
`357`	`384`	`$numSiblings = $this->numSiblings();`
	`385`	`+`
`358`	`386`	`return ($numSiblings > 1);`
`359`	`387`	`}`
`360`	`388`
`@@ -364,6 +392,7 @@ public function hasSiblings()`
`364`	`392`	`public function numSiblings()`
`365`	`393`	`{`
`366`	`394`	`$siblings = $this->siblings();`
	`395`	`+`
`367`	`396`	`return count($siblings);`
`368`	`397`	`}`
`369`	`398`
`@@ -376,7 +405,7 @@ public function siblings()`
`376`	`405`	`if ($this->siblings !== null) {`
`377`	`406`	`return $this->siblings;`
`378`	`407`	`}`
`379`		`- $master = $this->getMaster();`
	`408`	`+ $master = $this->getMasterObject();`
`380`	`409`	`if ($master === null) {`
`381`	`410`	`// Todo: return all top-level objects.`
`382`	`411`	`$siblings = [];`
`@@ -385,6 +414,7 @@ public function siblings()`
`385`	`414`	`$siblings = $master->children();`
`386`	`415`	`}`
`387`	`416`	`$this->siblings = $siblings;`
	`417`	`+`
`388`	`418`	`return $this->siblings;`
`389`	`419`	`}`
`390`	`420`
`@@ -395,13 +425,14 @@ public function siblings()`
`395`	`425`	`public function isSiblingOf($sibling)`
`396`	`426`	`{`
`397`	`427`	`$sibling = $this->objFromIdent($sibling);`
`398`		`- return ($sibling->getMaster() == $this->getMaster());`
	`428`	`+`
	`429`	`+ return ($sibling->getMaster() === $this->getMaster());`
`399`	`430`	`}`
`400`	`431`
`401`	`432`	`/**`
`402`	`433`	`* @param mixed $ident The ident.`
`403`		`- * @throws InvalidArgumentException If the identifier is not a scalar value.`
`404`	`434`	`* @return HierarchicalInterface\|null`
	`435`	`+ * @throws InvalidArgumentException If the identifier is not a scalar value.`
`405`	`436`	`*/`
`406`	`437`	`private function objFromIdent($ident)`
`407`	`438`	`{`