From 90458f604833fc97ea016460a4d38a089d8afe6f Mon Sep 17 00:00:00 2001 From: Eric Espie Date: Fri, 22 Mar 2024 10:11:25 +0100 Subject: [PATCH] =?UTF-8?q?N=C2=B06974=20-=20Flatten=20classes=20in=20data?= =?UTF-8?q?model=20-=20Allow=20=5Fdelta=3D"merge"=20in=20strict=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/designdocument.class.inc.php | 20 ++ setup/modelfactory.class.inc.php | 10 +- .../unitary-tests/setup/ModelFactoryTest.php | 223 ++++++++++++++---- 3 files changed, 201 insertions(+), 52 deletions(-) diff --git a/core/designdocument.class.inc.php b/core/designdocument.class.inc.php index e04c0fb54..6137f7fd1 100644 --- a/core/designdocument.class.inc.php +++ b/core/designdocument.class.inc.php @@ -339,6 +339,26 @@ class DesignElement extends \DOMElement return false; } + /** + * True if the node is contained in a _delta="merge" tree + * @return bool + */ + public function IsInSpecifiedMerge(): bool + { + // Iterate through the parents: reset the flag if any of them has a flag set + for ($oParent = $this; $oParent instanceof MFElement; $oParent = $oParent->parentNode) { + $sDeltaSpec = $oParent->getAttribute('_delta'); + if ($sDeltaSpec === 'merge') { + return true; + } + if (in_array($sDeltaSpec, ['define', 'define_if_not_exists', 'force', 'redefine'])) { + return false; + } + } + + return false; + } + /** * Find the child node matching the given node. * UNSAFE: may return nodes marked as _alteration="removed" diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 573f19b26..945f3c703 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -832,6 +832,7 @@ class ModelFactory case '': $bMustExist = ($sDeltaSpec === 'must_exist'); $bIfExists = ($sDeltaSpec === 'if_exists'); + $bSpecifiedMerge = $oSourceNode->IsInSpecifiedMerge(); /** @var MFElement $oTargetNode */ $oTargetNode = $oTargetParentNode->_FindChildNode($oSourceNode, $sSearchId); @@ -847,7 +848,7 @@ class ModelFactory // Do not continue deeper $oTargetNode = null; } else { - if ($sMode === self::LOAD_DELTA_MODE_STRICT && ($sSearchId !== '' || is_null($oSourceNode->firstElementChild))) { + if (!$bSpecifiedMerge && $sMode === self::LOAD_DELTA_MODE_STRICT && ($sSearchId !== '' || is_null($oSourceNode->firstElementChild))) { $iLine = ModelFactory::GetXMLLineNumber($oSourceNode); $sItopNodePath = DesignDocument::GetItopNodePath($oSourceNode); throw new MFException($sItopNodePath.' at line '.$iLine.': could not be found or marked as removed (strict mode)', @@ -867,12 +868,15 @@ class ModelFactory // Do not continue deeper everything is already copied $oTargetNode = null; } else { - // copy the node with attributes and continue deeper + // copy the node with attributes (except _delta) and continue deeper $oTargetNode = $oTargetDocument->importNode($oSourceNode, false); foreach ($oSourceNode->attributes as $oAttributeNode) { $oTargetNode->setAttribute($oAttributeNode->name, $oAttributeNode->value); } - if ($sSearchId !== '') { + if ($oTargetNode->hasAttribute('_delta')) { + $oTargetNode->removeAttribute('_delta'); + } + if ($sSearchId !== '' || $bSpecifiedMerge) { // Add the node by default $oTargetParentNode->AddChildNode($oTargetNode); } else { diff --git a/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php index 7fd9f2a60..f213e06dd 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php @@ -75,10 +75,12 @@ class ModelFactoryTest extends ItopTestCase // Canonicalize the expected XML (to cope with indentation) $oExpectedDocument = new DOMDocument(); $oExpectedDocument->preserveWhiteSpace = false; - $oExpectedDocument->loadXML($sXML); $oExpectedDocument->formatOutput = true; + $oExpectedDocument->loadXML($sXML); - return $oExpectedDocument->C14N(false, true); + $sSavedXML = $oExpectedDocument->SaveXML(); + + return str_replace(' encoding="UTF-8"', '', $sSavedXML); } /** @@ -665,7 +667,7 @@ class ModelFactoryTest extends ItopTestCase $oFactory = $this->MakeVanillaModelFactory($sInitialXML); $oFactoryDocument = $this->GetNonPublicProperty($oFactory, 'oDOMDocument'); $sExpectedXML = null; - if (\utils::StartsWith($sExpectedXMLOrErrorMessage, '<')) { + if (\utils::StartsWith(trim($sExpectedXMLOrErrorMessage), '<')) { $sExpectedXML = $sExpectedXMLOrErrorMessage; } @@ -3026,14 +3028,16 @@ XML , ], 'Conditionally deleted class' => [ - 'sInitialXMLInternal' => ' + 'sInitialXMLInternal' => ' + ', - 'sExpectedXMLDelta' => ' + 'sExpectedXMLDelta' => ' + @@ -3411,15 +3415,21 @@ XML $oDocument->loadXML($sDeltaXML); /* @var MFElement $oDeltaRoot */ $oDeltaRoot = $oDocument->firstChild; + $sExpectedXML = null; + if (\utils::StartsWith(trim($sExpectedXMLInLaxMode), '<')) { + $sExpectedXML = $sExpectedXMLInLaxMode; + } try { $oFactory->LoadDelta($oDeltaRoot, $oFactoryDocument, ModelFactory::LOAD_DELTA_MODE_LAX); - $this->AssertEqualModels($sExpectedXMLInLaxMode, $oFactory, 'LoadDelta(lax) did not produce the expected result'); + $this->assertNotNull($sExpectedXML, "LoadDelta(lax) should have failed with exception: $sExpectedXMLInLaxMode"); + $this->AssertEqualModels($sExpectedXML, $oFactory, 'LoadDelta(lax) did not produce the expected result'); } catch (ExpectationFailedException $e) { throw $e; } catch (\Exception $e) { - $this->assertNull($sExpectedXMLInLaxMode, 'LoadDelta(lax) should not have failed with exception: '.$e->getMessage()); + $this->assertNull($sExpectedXML, 'LoadDelta(lax) should not have failed with exception: '.$e->getMessage()); + $this->assertEquals($sExpectedXMLInLaxMode, $e->getMessage()); } // Load in Strict mode @@ -3430,118 +3440,233 @@ XML $oDocument->loadXML($sDeltaXML); /* @var MFElement $oDeltaRoot */ $oDeltaRoot = $oDocument->firstChild; + $sExpectedXML = null; + if (\utils::StartsWith(trim($sExpectedXMLInStrictMode), '<')) { + $sExpectedXML = $sExpectedXMLInStrictMode; + } try { $oFactory->LoadDelta($oDeltaRoot, $oFactoryDocument, ModelFactory::LOAD_DELTA_MODE_STRICT); - $this->AssertEqualModels($sExpectedXMLInStrictMode, $oFactory, 'LoadDelta(strict) did not produce the expected result'); + $this->assertNotNull($sExpectedXML, "LoadDelta(lax) should have failed with exception: $sExpectedXMLInStrictMode"); + $this->AssertEqualModels($sExpectedXML, $oFactory, 'LoadDelta(strict) did not produce the expected result'); } catch (ExpectationFailedException $e) { throw $e; } catch (\Exception $e) { - $this->assertNull($sExpectedXMLInStrictMode, 'LoadDelta(strict) should not have failed with exception: '.$e->getMessage()); + $this->assertNull($sExpectedXML, 'LoadDelta(strict) should not have failed with exception: '.$e->getMessage()); + $this->assertEquals($sExpectedXMLInStrictMode, $e->getMessage()); } } - public function LoadDeltaModeProvider() { return [ - 'merge delta have different behavior depending on the mode' => [ + 'default no _delta have different behavior depending on the mode' => [ 'sInitialXML' => ' - - + ', - 'sDeltaXML' => ' + 'sDeltaXML' => ' + cmdbAbstractObject ', - 'sExpectedXMLInLaxMode' => ' + 'sExpectedXMLInLaxMode' => ' + cmdbAbstractObject ', - 'sExpectedXMLInStrictMode' => null, + 'sExpectedXMLInStrictMode' => '/itop_design/nodeA/nodeB[C_1] at line 4: could not be found or marked as removed (strict mode)', ], + 'mode specified in delta takes precedence' => [ 'sInitialXML' => ' - - + ', - 'sDeltaXML' => ' + 'sDeltaXML' => ' + cmdbAbstractObject ', - 'sExpectedXMLInLaxMode' => null, - 'sExpectedXMLInStrictMode' => null, + 'sExpectedXMLInLaxMode' => '/itop_design/nodeA/nodeB[C_1] at line 4: could not be found or marked as removed (strict mode)', + 'sExpectedXMLInStrictMode' => '/itop_design/nodeA/nodeB[C_1] at line 4: could not be found or marked as removed (strict mode)', ], - 'merge leaf nodes have different behavior depending on the mode' => [ + + 'default no _delta leaf nodes have different behavior depending on the mode' => [ 'sInitialXML' => ' - Test + Test ', - 'sDeltaXML' => ' + 'sDeltaXML' => ' + Taste ', - 'sExpectedXMLInLaxMode' => ' - Taste + 'sExpectedXMLInLaxMode' => ' + + Taste ', - 'sExpectedXMLInStrictMode' => null, + 'sExpectedXMLInStrictMode' => '/itop_design/nodeA at line 3: cannot be modified without _delta flag (strict mode)', ], - 'merge existing leaf nodes without text have same behavior' => [ + + 'default no _delta on existing leaf nodes without text have same behavior' => [ 'sInitialXML' => ' - -', - 'sDeltaXML' => ' ', - 'sExpectedXMLInLaxMode' => ' - -', - 'sExpectedXMLInStrictMode' => ' - -', - ], - 'merge non-existing leaf nodes without text have different behavior' => [ - 'sInitialXML' => ' + 'sDeltaXML' => ' -', - 'sDeltaXML' => ' ', - 'sExpectedXMLInLaxMode' => ' - + 'sExpectedXMLInLaxMode' => ' + + +', + 'sExpectedXMLInStrictMode' => ' + + ', - 'sExpectedXMLInStrictMode' => null, ], - 'merge non-existing nodes with sub-nodes defined' => [ + + 'default no _delta on non-existing leaf nodes without text have different behavior' => [ 'sInitialXML' => ' ', - 'sDeltaXML' => ' + 'sDeltaXML' => ' + + +', + 'sExpectedXMLInLaxMode' => ' + + +', + 'sExpectedXMLInStrictMode' => '/itop_design/nodeA at line 3: could not be found or marked as removed (strict mode)', + ], + + 'default no _delta on non-existing nodes with sub-nodes defined' => [ + 'sInitialXML' => ' + +', + 'sDeltaXML' => ' + ', - 'sExpectedXMLInLaxMode' => ' + 'sExpectedXMLInLaxMode' => ' + ', - 'sExpectedXMLInStrictMode' => ' + 'sExpectedXMLInStrictMode' => ' + +', + ], + + 'merge _delta on non-existing node must create node' => [ + 'sInitialXML' => ' + + + + +', + 'sDeltaXML' => ' + + + + + + Test + + + + +', + 'sExpectedXMLInLaxMode' => ' + + + + + + Test + + + + +', + 'sExpectedXMLInStrictMode' => ' + + + + + + Test + + + + +', + ], + + 'merge _delta on existing tree must merge...' => [ + 'sInitialXML' => ' + + + + + + + + +', + 'sDeltaXML' => ' + + + + + + Test + + + + +', + 'sExpectedXMLInLaxMode' => ' + + + + + + Test + + + + +', + 'sExpectedXMLInStrictMode' => ' + + + + + + Test + + + + ', ], ];