diff --git a/sources/Forms/FormsException.php b/sources/Forms/FormsException.php index 99d9cd62f..7a0755dd2 100644 --- a/sources/Forms/FormsException.php +++ b/sources/Forms/FormsException.php @@ -24,5 +24,4 @@ class FormsException extends Exception parent::__construct($sMessage, $iCode, $oPrevious); IssueLog::Exception(get_class($this).' occurs: '.$sMessage, $this, null, $aContext); } - } diff --git a/sources/PropertyTree/AbstractProperty.php b/sources/PropertyTree/AbstractProperty.php index 44eda7532..a03cfbddf 100644 --- a/sources/PropertyTree/AbstractProperty.php +++ b/sources/PropertyTree/AbstractProperty.php @@ -16,13 +16,14 @@ use utils; */ abstract class AbstractProperty { + protected ?AbstractProperty $oParent; protected string $sId; protected ?string $sLabel; /** @var array */ protected array $aChildren = []; protected ?AbstractValueType $oValueType; - protected ?string $sParentId; + protected ?string $sIdWithPath; /** * Init property tree node from xml dom node @@ -34,14 +35,15 @@ abstract class AbstractProperty * @throws \DOMFormatException * @throws \Combodo\iTop\PropertyTree\PropertyTreeException */ - public function InitFromDomNode(DesignElement $oDomNode, string $sParentId = ''): void + public function InitFromDomNode(DesignElement $oDomNode, ?AbstractProperty $oParent = null): void { - if (utils::IsNotNullOrEmptyString($sParentId)) { - $this->sId = $sParentId.'__'; + $this->oParent = $oParent; + $this->sId = $oDomNode->getAttribute('id'); + if (is_null($oParent)) { + $this->sIdWithPath = $this->sId; } else { - $this->sId = ''; + $this->sIdWithPath = $oParent->sIdWithPath.'__'.$this->sId; } - $this->sId .= $oDomNode->getAttribute('id'); $this->sLabel = $oDomNode->GetChildText('label'); } @@ -61,4 +63,19 @@ abstract class AbstractProperty { return $this->aChildren; } + + public function GetSibling(string $sId): ?AbstractProperty + { + if (is_null($this->oParent)) { + return null; + } + + foreach ($this->oParent->GetChildren() as $oSibling) { + if ($oSibling->sId == $sId) { + return $oSibling; + } + } + + return null; + } } diff --git a/sources/PropertyTree/CollectionOfTrees.php b/sources/PropertyTree/CollectionOfTrees.php index 4a9bb5cf3..7974fe838 100644 --- a/sources/PropertyTree/CollectionOfTrees.php +++ b/sources/PropertyTree/CollectionOfTrees.php @@ -20,9 +20,9 @@ class CollectionOfTrees extends AbstractProperty /** * @inheritDoc */ - public function InitFromDomNode(DesignElement $oDomNode, string $sParentId = ''): void + public function InitFromDomNode(DesignElement $oDomNode, ?AbstractProperty $oParent = null): void { - parent::InitFromDomNode($oDomNode, $sParentId); + parent::InitFromDomNode($oDomNode, $oParent); $oPropertyTreeFactory = PropertyTreeFactory::GetInstance(); $this->sButtonLabel = $oDomNode->GetChildText('button-label'); @@ -30,7 +30,7 @@ class CollectionOfTrees extends AbstractProperty // read child properties foreach ($oDomNode->GetUniqueElement('prototype')->childNodes as $oNode) { if ($oNode instanceof DesignElement) { - $this->AddChild($oPropertyTreeFactory->CreateNodeFromDom($oNode, $this->sId)); + $this->AddChild($oPropertyTreeFactory->CreateNodeFromDom($oNode, $this)); } } } @@ -39,7 +39,7 @@ class CollectionOfTrees extends AbstractProperty { $sFormBlockClass = CollectionBlock::class; - $sSubTreeClass = 'SubFormFor__'.$this->sId; + $sSubTreeClass = 'SubFormFor__'.$this->sIdWithPath; // Create the collection node $sLocalPHP = <<sId extends Combodo\iTop\Forms\Block\Base\FormBlock + class $sSubTreeClass extends Combodo\iTop\Forms\Block\Base\FormBlock { protected function BuildForm(): void { diff --git a/sources/PropertyTree/Property.php b/sources/PropertyTree/Property.php index d8a39a570..70e53c693 100644 --- a/sources/PropertyTree/Property.php +++ b/sources/PropertyTree/Property.php @@ -9,6 +9,7 @@ namespace Combodo\iTop\PropertyTree; use Combodo\iTop\DesignElement; use Combodo\iTop\PropertyTree\ValueType\ValueTypeFactory; +use Exception; use Expression; use utils; @@ -22,9 +23,9 @@ class Property extends AbstractProperty /** * @inheritDoc */ - public function InitFromDomNode(DesignElement $oDomNode, string $sParentId = ''): void + public function InitFromDomNode(DesignElement $oDomNode, ?AbstractProperty $oParent = null): void { - parent::InitFromDomNode($oDomNode); + parent::InitFromDomNode($oDomNode, $oParent); $oValueTypeNode = $oDomNode->GetOptionalElement('value-type'); if ($oValueTypeNode) { @@ -34,6 +35,12 @@ class Property extends AbstractProperty $this->sRelevanceCondition = $oDomNode->GetChildText('relevance-condition'); } + /** + * @param $aPHPFragments + * + * @return string + * @throws \Combodo\iTop\PropertyTree\PropertyTreeException + */ public function ToPHPFormBlock(&$aPHPFragments = []): string { $sFormBlockClass = $this->oValueType->GetFormBlockClass(); @@ -42,12 +49,23 @@ class Property extends AbstractProperty $sRelevanceCondition = ''; $sBinding = null; if (!is_null($this->sRelevanceCondition)) { - $oExpression = Expression::FromOQL($this->sRelevanceCondition); - $aFieldsToResolve = $oExpression->ListRequiredFields(); + try { + $oExpression = Expression::FromOQL($this->sRelevanceCondition); + } catch (Exception $e) { + throw new PropertyTreeException("Node: {$this->sId}, invalid syntax in relevance condition: ".$e->getMessage()); + } + $aFieldsToResolve = array_unique($oExpression->ListRequiredFields()); foreach ($aFieldsToResolve as $sFieldToResolve) { if (preg_match('/(?\w+)\.(?\w+)/', $sFieldToResolve, $aMatches) === 1) { $sNode = $aMatches['node']; + $oSibling = $this->GetSibling($sNode); + if (is_null($oSibling)) { + throw new PropertyTreeException("Node: {$this->sId}, invalid source in relevance condition: $sNode"); + } $sOutput = $aMatches['output']; + if (!in_array($sOutput, $oSibling->oValueType->GetOutputs())) { + throw new PropertyTreeException("Node: {$this->sId}, invalid output in relevance condition: $sFieldToResolve"); + } $sBinding .= "\n ->AddInputDependsOn('{$sNode}.$sOutput', '$sNode', '$sOutput')"; } else { // TODO Erreur field sans alias diff --git a/sources/PropertyTree/PropertyTree.php b/sources/PropertyTree/PropertyTree.php index eb1e55148..99a52ffd5 100644 --- a/sources/PropertyTree/PropertyTree.php +++ b/sources/PropertyTree/PropertyTree.php @@ -17,15 +17,15 @@ class PropertyTree extends AbstractProperty /** * @inheritdoc */ - public function InitFromDomNode(DesignElement $oDomNode, string $sParentId = ''): void + public function InitFromDomNode(DesignElement $oDomNode, ?AbstractProperty $oParent = null): void { - parent::InitFromDomNode($oDomNode, $sParentId); + parent::InitFromDomNode($oDomNode, $oParent); $oPropertyTreeFactory = PropertyTreeFactory::GetInstance(); // read child properties foreach ($oDomNode->GetUniqueElement('nodes')->childNodes as $oNode) { if ($oNode instanceof DesignElement) { - $this->AddChild($oPropertyTreeFactory->CreateNodeFromDom($oNode, $this->sId)); + $this->AddChild($oPropertyTreeFactory->CreateNodeFromDom($oNode, $this)); } } } diff --git a/sources/PropertyTree/PropertyTreeFactory.php b/sources/PropertyTree/PropertyTreeFactory.php index 5abaa009a..fff06f9e8 100644 --- a/sources/PropertyTree/PropertyTreeFactory.php +++ b/sources/PropertyTree/PropertyTreeFactory.php @@ -38,14 +38,14 @@ class PropertyTreeFactory * @throws \Combodo\iTop\PropertyTree\PropertyTreeException * @throws \DOMFormatException */ - public function CreateNodeFromDom(DesignElement $oDomNode, string $sParentId = ''): AbstractProperty + public function CreateNodeFromDom(DesignElement $oDomNode, ?AbstractProperty $oParent = null): AbstractProperty { $sNodeType = $oDomNode->getAttribute('xsi:type'); // The class of the property tree node is given by the xsi:type attribute if (is_a($sNodeType, AbstractProperty::class, true)) { $oNode = new $sNodeType(); - $oNode->InitFromDomNode($oDomNode, $sParentId); + $oNode->InitFromDomNode($oDomNode, $oParent); return $oNode; } diff --git a/sources/PropertyTree/ValueType/AbstractValueType.php b/sources/PropertyTree/ValueType/AbstractValueType.php index a8a8cf4b8..0fbe61019 100644 --- a/sources/PropertyTree/ValueType/AbstractValueType.php +++ b/sources/PropertyTree/ValueType/AbstractValueType.php @@ -18,6 +18,7 @@ abstract class AbstractValueType abstract public function GetFormBlockClass(): string; protected array $aInputs = []; + protected array $aOutputs = []; public function InitFromDomNode(DesignElement $oDomNode): void { @@ -30,10 +31,18 @@ abstract class AbstractValueType $this->aInputs[$sInputName] = $sInputValue; } } + foreach ($oBlockNode->GetOutputs() as $oOutput) { + $this->aOutputs[] = $oOutput->GetName(); + } } public function GetInputs(): array { return $this->aInputs; } + + public function GetOutputs(): array + { + return $this->aOutputs; + } } diff --git a/tests/php-unit-tests/phpunit.xml.dist b/tests/php-unit-tests/phpunit.xml.dist index 1d26c02a9..ae06ca8e0 100644 --- a/tests/php-unit-tests/phpunit.xml.dist +++ b/tests/php-unit-tests/phpunit.xml.dist @@ -51,9 +51,6 @@ unitary-tests/webservices - - unitary-tests/sources/Forms - ../../env-production/*/test diff --git a/tests/php-unit-tests/unitary-tests/sources/Forms/Compiler/TestFormsCompiler.php b/tests/php-unit-tests/unitary-tests/sources/Forms/Compiler/TestFormsCompiler.php index f50409937..05bbc62ce 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Forms/Compiler/TestFormsCompiler.php +++ b/tests/php-unit-tests/unitary-tests/sources/Forms/Compiler/TestFormsCompiler.php @@ -5,8 +5,6 @@ * @license http://opensource.org/licenses/AGPL-3.0 */ -use Combodo\iTop\Forms\Block\AbstractTypeFormBlock; -use Combodo\iTop\Forms\Block\Expression\BooleanExpressionFormBlock; use Combodo\iTop\Forms\Compiler\FormsCompiler; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; @@ -42,7 +40,7 @@ class TestFormsCompiler extends ItopDataTestCase - + @@ -255,7 +253,7 @@ class FormFor__collection_of_trees_test extends Combodo\iTop\Forms\Block\Base\Fo { protected function BuildForm(): void { - \$this->Add('collection_of_trees_test__sub_tree_collection', 'Combodo\iTop\Forms\Block\Base\CollectionBlock', [ + \$this->Add('sub_tree_collection', 'Combodo\iTop\Forms\Block\Base\CollectionBlock', [ 'label' => 'UI:SubTree', 'button_label' => 'UI:AddSubTree', 'block_entry_type' => 'SubFormFor__collection_of_trees_test__sub_tree_collection', @@ -265,7 +263,7 @@ class FormFor__collection_of_trees_test extends Combodo\iTop\Forms\Block\Base\Fo PHP, ], - 'Input static' => [ + 'Static inputs should be bound and invalid input should be ignored' => [ 'sXMLContent' => << @@ -294,7 +292,7 @@ class FormFor__input_static_test extends Combodo\iTop\Forms\Block\Base\FormBlock PHP, ], - 'Input binding' => [ + 'Dynamic input should be bound' => [ 'sXMLContent' => << @@ -331,7 +329,7 @@ class FormFor__input_binding_test extends Combodo\iTop\Forms\Block\Base\FormBloc PHP, ], - 'Relevance condition' => [ + 'Relevance condition should generate a boolean block expression' => [ 'sXMLContent' => << @@ -373,6 +371,57 @@ class FormFor__RelevanceCondition extends Combodo\iTop\Forms\Block\Base\FormBloc PHP, ], + 'Complex Relevance condition should generate a boolean block expression' => [ + 'sXMLContent' => << + + + + + + + + + + + + + + + IF(source_a_property.text != '', source_a_property.text, source_b_property.text) + + + + + +XML, + 'sExpectedPHP' => <<Add('source_a_property', 'Combodo\iTop\Forms\Block\Base\TextFormBlock', [ + 'label' => 'UI:Source', + ]); + + \$this->Add('source_b_property', 'Combodo\iTop\Forms\Block\Base\TextFormBlock', [ + 'label' => 'UI:Source', + ]); + + \$this->Add('dependant_property_relevance_condition', 'Combodo\iTop\Forms\Block\Expression\BooleanExpressionFormBlock', [ + 'expression' => "IF(source_a_property.text != '', source_a_property.text, source_b_property.text)", + ]) + ->AddInputDependsOn('source_a_property.text', 'source_a_property', 'text') + ->AddInputDependsOn('source_b_property.text', 'source_b_property', 'text'); + + \$this->Add('dependant_property', 'Combodo\iTop\Forms\Block\Base\TextFormBlock', [ + 'label' => 'UI:Dependant', + ]) + ->InputDependsOn('visible', 'dependant_property_relevance_condition', 'result'); + } +} +PHP, + ], 'test' => [ 'sXMLContent' => << @@ -391,4 +440,86 @@ PHP, ], ]; } + + /** + * @dataProvider CompileFormFromInvalidXMLProvider + * @param string $sXMLContent + * @param string $sExpectedClass + * @param string $sExpectedMessage + * + * @return void + * @throws \Combodo\iTop\Forms\Compiler\FormsCompilerException + * @throws \Combodo\iTop\PropertyTree\PropertyTreeException + * @throws \DOMFormatException + */ + public function testCompileFormFromInvalidXML(string $sXMLContent, string $sExpectedClass, string $sExpectedMessage) + { + $this->expectException($sExpectedClass); + $this->expectExceptionMessage($sExpectedMessage); + FormsCompiler::GetInstance()->CompileFormFromXML($sXMLContent); + } + + public function CompileFormFromInvalidXMLProvider() + { + return [ + 'Invalid OQL expression in condition' => [ + 'sXMLContent' => << + + + + + source_property.text == 'count' + + + + + +XML, + 'sExpectedClass' => 'Combodo\iTop\PropertyTree\PropertyTreeException', + 'sExpectedMessage' => 'Node: dependant_property, invalid syntax in relevance condition: Unexpected token EQ - found \'=\' at 22 in \'source_property.text == \'count\'\'', + ], + + 'Unknown source in relevance condition' => [ + 'sXMLContent' => << + + + + + source_property.text = 'count' + + + + + +XML, + 'sExpectedClass' => 'Combodo\iTop\PropertyTree\PropertyTreeException', + 'sExpectedMessage' => 'Node: dependant_property, invalid source in relevance condition: source_property', + ], + + 'Unknown output in relevance condition' => [ + 'sXMLContent' => << + + + + + + + + + + source_property.text_output != 'count' + + + + + +XML, + 'sExpectedClass' => 'Combodo\iTop\PropertyTree\PropertyTreeException', + 'sExpectedMessage' => 'Node: dependant_property, invalid output in relevance condition: source_property.text_output', + ], + ]; + } }