diff --git a/setup/itopdesignformat.class.inc.php b/setup/itopdesignformat.class.inc.php index 1708632fb..b868e9951 100644 --- a/setup/itopdesignformat.class.inc.php +++ b/setup/itopdesignformat.class.inc.php @@ -784,18 +784,13 @@ class iTopDesignFormat $oFieldsSemanticNodeList = $oXPath->query("fields_semantic", $oPropertiesNode); if ($oFieldsSemanticNodeList->length > 0) { $oSemanticNode = $oFieldsSemanticNodeList->item(0); - } - else { + } else { $oSemanticNode = $oPropertiesNode->ownerDocument->createElement("fields_semantic"); $oPropertiesNode->appendChild($oSemanticNode); } - // Create state_attribute node - $oStateNode = $oSemanticNode->ownerDocument->createElement("state_attribute", $oNode->nodeValue); - $oSemanticNode->appendChild($oStateNode); - - // Remove current node from lifecycle - $this->DeleteNode($oNode); + // Move to state_attribute node + $this->MoveNode($oNode, $oSemanticNode, "state_attribute"); } // New Enum values format @@ -1030,10 +1025,149 @@ class iTopDesignFormat */ protected function DeleteNode($oNode) { - if ($oNode->nextSibling && ($oNode->nextSibling instanceof DOMText) && ($oNode->nextSibling->isWhitespaceInElementContent())) - { + if ($oNode->nextSibling && ($oNode->nextSibling instanceof DOMText) && ($oNode->nextSibling->isWhitespaceInElementContent())) { $oNode->parentNode->removeChild($oNode->nextSibling); } $oNode->parentNode->removeChild($oNode); } + + /** + * Move $oNode under $oParentNode and adds the correct _delta flag to it depending on its original flag (or its ancestors') and its destination parent flag (or its ancestors') + * + * +----------------------+-----------------+---------------+--------------+ + * |\ Dest. parent node | | | | + * | -------------------- | In definition | In deletion | Structural | + * | Node to move \| | | | + * +----------------------+-----------------+---------------+--------------+ + * | | Remove _delta | Remove node | Set _delta | + * | In definition | flag from node | completely | flag from | + * | | | | self or anc.| + * +----------------------+-----------------+---------------+--------------+ + * | | Remove node | Remove node | Set _delta | + * | In deletion | completely | completely | flag from | + * | | | | self | + * +----------------------+-----------------+---------------+--------------+ + * | | Remove _delta | Remove node | Set _delta | + * | Structural | from all child | completely | flag from | + * | | nodes | | self | + * +----------------------+-----------------+---------------+--------------+ + * + * @param \DOMElement $oNode + * @param \DOMElement $oDestParentNode + * @param string|null $sNewNodeName New name for the moved node (eg. "bar" => "bar" + * + * @since 3.0.0 + */ + protected function MoveNode(DOMElement $oNode, DOMElement $oDestParentNode, ?string $sNewNodeName = null) + { + // Check if node / dest. parent are currently in definition / deletion from the delta + $bIsNodeInDeltaDefinition = $this->IsNodeInDeltaDefinition($oNode); + $bIsNodeInDeltaDeletion = $this->IsNodeInDeltaDeletion($oNode); + $bIsDestParentNodeInDeltaDefinition = $this->IsNodeInDeltaDefinition($oDestParentNode); + $bIsDestParentNodeInDeltaDeletion = $this->IsNodeInDeltaDeletion($oDestParentNode); + + // Prepare the new node + if (is_null($sNewNodeName)) { + $sNewNodeName = $oNode->nodeName; + } + $oNewNode = $oDestParentNode->ownerDocument->createElement($sNewNodeName, $oNode->nodeValue); + + // Compute new _delta flag + $sNewDeltaFlag = null; + $bAppendNodeToDestParentNode = true; + if ((false === $bIsDestParentNodeInDeltaDefinition) && (false === $bIsDestParentNodeInDeltaDeletion)) { + if ($bIsNodeInDeltaDefinition) { + $sNewDeltaFlag = $this->GetDeltaFlagFromSelfOrAncestors($oNode); + } else { + $sCurrentDeltaFlag = $oNode->getAttribute('_delta'); + if (!empty($sCurrentDeltaFlag)) { + $sNewDeltaFlag = $sCurrentDeltaFlag; + } + } + + } elseif ($bIsDestParentNodeInDeltaDefinition) { + if ($bIsNodeInDeltaDefinition) { + // Do nothing, there is no need for a flag + } elseif ($bIsNodeInDeltaDeletion) { + $bAppendNodeToDestParentNode = false; + } else { + // Clean _delta flag from all child nodes + // TODO + } + } elseif ($bIsDestParentNodeInDeltaDeletion) { + $bAppendNodeToDestParentNode = false; + } + + // Update flag + if (!is_null($sNewDeltaFlag)) { + $oNewNode->setAttribute('_delta', $sNewDeltaFlag); + } + + // Move newly created under destination parent + if ($bAppendNodeToDestParentNode) { + $oDestParentNode->appendChild($oNewNode); + } + + // Remove current node from source parent + $this->DeleteNode($oNode); + } + + /** + * @see \ModelFactory::DELTA_FLAG_IN_DEFINITION_VALUES + * + * @param \DOMElement $oNode + * + * @return bool True if $oNode or one of its ancestors is "in the *delta* definition" + * @since 3.0.0 + */ + protected function IsNodeInDeltaDefinition(DOMElement $oNode): bool + { + $bIsInDefinition = false; + for ($oParent = $oNode; $oParent instanceof DOMElement; $oParent = $oParent->parentNode) { + if (in_array($this->GetDeltaFlagFromSelfOrAncestors($oParent), ModelFactory::DELTA_FLAG_IN_DEFINITION_VALUES)) { + $bIsInDefinition = true; + break; + } + } + + return $bIsInDefinition; + } + + /** + * @see \ModelFactory::DELTA_FLAG_IN_DELETION_VALUES + * + * @param \DOMElement $oNode + * + * @return bool True if $oNode or one of its ancestors is "in the *delta* deletion" + * @since 3.0.0 + */ + protected function IsNodeInDeltaDeletion(DOMElement $oNode): bool + { + $bIsInDefinition = false; + for ($oParent = $oNode; $oParent instanceof DOMElement; $oParent = $oParent->parentNode) { + if (in_array($this->GetDeltaFlagFromSelfOrAncestors($oParent), ModelFactory::DELTA_FLAG_IN_DELETION_VALUES)) { + $bIsInDefinition = true; + break; + } + } + + return $bIsInDefinition; + } + + /** + * @param \DOMElement $oNode + * + * @return string|null The _delta flag of the $oNode or from the closest ancestor with one; if none found null will be returned + * @since 3.0.0 + */ + protected function GetDeltaFlagFromSelfOrAncestors(DOMElement $oNode): ?string + { + for ($oParent = $oNode; $oParent instanceof DOMElement; $oParent = $oParent->parentNode) { + if ($oParent->hasAttribute('_delta')) { + return $oParent->getAttribute('_delta'); + } + } + + return null; + } } diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index f391571f3..97e5ff36c 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -503,6 +503,17 @@ class MFDictModule extends MFModule */ class ModelFactory { + /** + * @var array Values of the _delta flag meaning that a node is "in definition" = currently being added to the delta + * @since 3.0.0 + */ + public const DELTA_FLAG_IN_DEFINITION_VALUES = ['define', 'define_if_not_exists', 'redefine', 'force']; + /** + * @var array Values of the _delta flag meaning that a node is "in deletion" = currently being removed from the delta + * @since 3.0.0 + */ + public const DELTA_FLAG_IN_DELETION_VALUES = ['delete', 'delete_if_exists']; + protected $aRootDirs; protected $oDOMDocument; protected $oRoot; @@ -682,8 +693,8 @@ class ModelFactory } } - switch ($sDeltaSpec) - { + // IMPORTANT: In case of a new flag value, mind to update the iTopDesignFormat methods + switch ($sDeltaSpec) { case 'if_exists': case 'must_exist': case 'merge': @@ -692,10 +703,8 @@ class ModelFactory $bIfExists = ($sDeltaSpec == 'if_exists'); $sSearchId = $oSourceNode->hasAttribute('_rename_from') ? $oSourceNode->getAttribute('_rename_from') : $oSourceNode->getAttribute('id'); $oTargetNode = $oSourceNode->MergeInto($oTargetParentNode, $sSearchId, $bMustExist, $bIfExists); - if ($oTargetNode) - { - foreach ($oSourceNode->childNodes as $oSourceChild) - { + if ($oTargetNode) { + foreach ($oSourceNode->childNodes as $oSourceChild) { // Continue deeper $this->LoadDelta($oSourceChild, $oTargetNode); } @@ -1406,16 +1415,14 @@ EOF { $sAlteration = $oNodeClone->getAttribute('_alteration'); $oNodeClone->removeAttribute('_alteration'); - if ($oNodeClone->hasAttribute('_old_id')) - { + if ($oNodeClone->hasAttribute('_old_id')) { $oNodeClone->setAttribute('_rename_from', $oNodeClone->getAttribute('_old_id')); $oNodeClone->removeAttribute('_old_id'); } - switch ($sAlteration) - { + // IMPORTANT: In case of a new flag value, mind to update the iTopDesignFormat methods + switch ($sAlteration) { case '': - if ($oNodeClone->hasAttribute('id')) - { + if ($oNodeClone->hasAttribute('id')) { $oNodeClone->setAttribute('_delta', 'must_exist'); } break; diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.expected.xml new file mode 100644 index 000000000..5b43dabbd --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.expected.xml @@ -0,0 +1,10 @@ + + + + + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.input.xml new file mode 100644 index 000000000..4f49347ec --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_deleted.input.xml @@ -0,0 +1,14 @@ + + + + + In self delete + + + In parent delete + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.expected.xml new file mode 100644 index 000000000..26d860196 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.expected.xml @@ -0,0 +1,10 @@ + + + + + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.input.xml new file mode 100644 index 000000000..55932a9a9 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_in-definition.input.xml @@ -0,0 +1,14 @@ + + + + + In self delete + + + In parent delete + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.expected.xml new file mode 100644 index 000000000..4e3c107e4 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.expected.xml @@ -0,0 +1,12 @@ + + + + + + + I'm a future sibling + In self delete + In parent delete + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.input.xml new file mode 100644 index 000000000..971d7c8e4 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_deleted_to_not-in-definition.input.xml @@ -0,0 +1,14 @@ + + + + + In self delete + + + In parent delete + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.expected.xml new file mode 100644 index 000000000..282c2feb3 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.expected.xml @@ -0,0 +1,10 @@ + + + + + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.input.xml new file mode 100644 index 000000000..f036c14e4 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_deleted.input.xml @@ -0,0 +1,14 @@ + + + + + In self definition + + + In parent definition + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.expected.xml new file mode 100644 index 000000000..3c8bca3d7 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.expected.xml @@ -0,0 +1,12 @@ + + + + + + + I'm a future sibling + In self definition + In parent definition + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.input.xml new file mode 100644 index 000000000..52f38c6ec --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_in-definition.input.xml @@ -0,0 +1,14 @@ + + + + + In self definition + + + In parent definition + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.expected.xml new file mode 100644 index 000000000..2428628c1 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.expected.xml @@ -0,0 +1,12 @@ + + + + + + + I'm a future sibling + In self definition + In parent definition + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.input.xml new file mode 100644 index 000000000..139d4ddfc --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_in-definition_to_not-in-definition.input.xml @@ -0,0 +1,14 @@ + + + + + In self definition + + + In parent definition + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.expected.xml new file mode 100644 index 000000000..eb36b9950 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.expected.xml @@ -0,0 +1,9 @@ + + + + + + I'm a future sibling + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.input.xml new file mode 100644 index 000000000..238f7ebc7 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_deleted.input.xml @@ -0,0 +1,11 @@ + + + + + Not in definition + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.expected.xml new file mode 100644 index 000000000..603e6dc4b --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.expected.xml @@ -0,0 +1,10 @@ + + + + + + I'm a future sibling + Not in definition + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.input.xml new file mode 100644 index 000000000..a2bae1f7a --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_in-definition.input.xml @@ -0,0 +1,11 @@ + + + + + Not in definition + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.expected.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.expected.xml new file mode 100644 index 000000000..7cd174e99 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.expected.xml @@ -0,0 +1,10 @@ + + + + + + I'm a future sibling + Not in definition + + + diff --git a/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.input.xml b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.input.xml new file mode 100644 index 000000000..e33c2f345 --- /dev/null +++ b/test/setup/iTopDesignFormat/MoveNode-samples/from_not-in-definition_to_not-in-definition.input.xml @@ -0,0 +1,11 @@ + + + + + Not in definition + + + I'm a future sibling + + + \ No newline at end of file diff --git a/test/setup/iTopDesignFormat/iTopDesignFormatTest.php b/test/setup/iTopDesignFormat/iTopDesignFormatTest.php index c108468db..aa284c816 100644 --- a/test/setup/iTopDesignFormat/iTopDesignFormatTest.php +++ b/test/setup/iTopDesignFormat/iTopDesignFormatTest.php @@ -4,6 +4,7 @@ namespace Combodo\iTop\Test\UnitTest\Setup; use Combodo\iTop\Test\UnitTest\ItopTestCase; use DOMDocument; +use DOMXPath; use iTopDesignFormat; @@ -34,8 +35,7 @@ class TestForITopDesignFormatClass extends ItopTestCase * @dataProvider ConvertProvider * * @param string $sTargetVersion - * @param string $sInputXmlFileName example "1.7_to_1.6.input" - * @param string $sExpectedXmlFileName example "1.7_to_1.6.expected" + * @param string $sInputXmlFileName Example "1.7_to_1.6" * * @throws \Exception */ @@ -66,6 +66,63 @@ class TestForITopDesignFormatClass extends ItopTestCase ); } + /** + * @covers iTopDesignFormat::MoveNode + * @dataProvider MoveNodeProvider + * + * @param string $sXmlFileName Example "from_deleted_to_not-in-definition" + */ + public function testMoveNode(string $sXmlFileName) + { + $sSamplesRelDirPath = 'MoveNode-samples/'; + $sInputXml = $this->GetFileContent($sSamplesRelDirPath.$sXmlFileName.'.input'); + $sExpectedXml = $this->GetFileContent($sSamplesRelDirPath.$sXmlFileName.'.expected'); + + // Prepare document + $oInputDocument = new DOMDocument(); + libxml_clear_errors(); + $oInputDocument->preserveWhiteSpace = false; + $oInputDocument->loadXML($sInputXml); + $oInputDocument->formatOutput = true; + $oDesignFormat = new iTopDesignFormat($oInputDocument); + + $oXPath = new DOMXPath($oInputDocument); + + // Move nodes + // Note: We could have pass the XPaths in the provider, but as for now they are the same for all cases, it's easier to read like this. Feel free to change it in the future if necessary. + $oFNode = $oXPath->query("//f")->item(0); + // - Self node + $oCNodeList = $oXPath->query("//c"); + if ($oCNodeList->length > 0) { + $oCNode = $oCNodeList->item(0); + $this->InvokeNonPublicMethod('iTopDesignFormat', 'MoveNode', $oDesignFormat, [$oCNode, $oFNode]); + } + // - In parent node + $oENodeList = $oXPath->query("//e"); + if ($oENodeList->length > 0) { + $oENode = $oENodeList->item(0); + $this->InvokeNonPublicMethod('iTopDesignFormat', 'MoveNode', $oDesignFormat, [$oENode, $oFNode]); + } + + $sConvertedXml = $oInputDocument->saveXML(); + $this->assertEquals($sExpectedXml, $sConvertedXml); + } + + public function MoveNodeProvider() + { + return array( + 'From deleted to deleted' => array('from_deleted_to_deleted'), + 'From deleted to in definition' => array('from_deleted_to_in-definition'), + 'From deleted to not in definition' => array('from_deleted_to_not-in-definition'), + 'From in definition to deleted' => array('from_in-definition_to_deleted'), + 'From in definition to in definition' => array('from_in-definition_to_in-definition'), + 'From in definition to not in definition' => array('from_in-definition_to_not-in-definition'), + 'From not in definition to deleted' => array('from_not-in-definition_to_deleted'), + 'From not in definition to in definition' => array('from_not-in-definition_to_in-definition'), + 'From not in definition to not in definition' => array('from_not-in-definition_to_not-in-definition'), + ); + } + private function GetFileContent($sFileName) { $sCurrentPath = __DIR__;