diff --git a/sources/Application/WebPage/ErrorPage.php b/sources/Application/WebPage/ErrorPage.php index b7af8a46d9..0fec50b952 100644 --- a/sources/Application/WebPage/ErrorPage.php +++ b/sources/Application/WebPage/ErrorPage.php @@ -60,7 +60,7 @@ class ErrorPage extends NiceWebPage $this->add('
'); } if (!is_null($oException)) { - $this->log_exception($sText, $oException); + $this->log_exception($oException->getMessage(), $oException); return; } $this->log_error($sText); diff --git a/sources/Forms/Block/Base/FormBlock.php b/sources/Forms/Block/Base/FormBlock.php index 4bf80a7943..43c8067b70 100644 --- a/sources/Forms/Block/Base/FormBlock.php +++ b/sources/Forms/Block/Base/FormBlock.php @@ -110,9 +110,8 @@ class FormBlock extends AbstractTypeFormBlock */ private function VerifyBlockClassName(string $sBlockClass): void { - $oRef = new ReflectionClass($sBlockClass); - if ($oRef->isSubclassOf(AbstractFormBlock::class) === false) { - throw new FormBlockException("The block type '$sBlockClass' is not a subclass of AbstractFormBlock."); + if (!is_a($sBlockClass, AbstractFormBlock::class, true)) { + throw new FormBlockException('The block type '.json_encode($sBlockClass).' is not a subclass of AbstractFormBlock.'); } } @@ -132,9 +131,13 @@ class FormBlock extends AbstractTypeFormBlock * @param string $sName name of the block * * @return AbstractFormBlock + * @throws \Combodo\iTop\Forms\Block\FormBlockException */ public function Get(string $sName): AbstractFormBlock { + if (!array_key_exists($sName, $this->aChildrenBlocks)) { + throw new FormBlockException('Block does not exist '.json_encode($sName)); + } return $this->aChildrenBlocks[$sName]; } diff --git a/sources/Forms/FormsException.php b/sources/Forms/FormsException.php index 97b5257cae..989c88354e 100644 --- a/sources/Forms/FormsException.php +++ b/sources/Forms/FormsException.php @@ -13,9 +13,4 @@ use Throwable; class FormsException extends Exception { - public function __construct(string $sMessage = '', int $iCode = 0, ?Throwable $oPrevious = null, array $aContext = []) - { - parent::__construct($sMessage, $iCode, $oPrevious); - IssueLog::Exception(get_class($this).' occurs: '.$sMessage, $this, null, $aContext); - } } diff --git a/sources/Forms/Register/IORegister.php b/sources/Forms/Register/IORegister.php index 9f9da0dc76..081b469922 100644 --- a/sources/Forms/Register/IORegister.php +++ b/sources/Forms/Register/IORegister.php @@ -31,9 +31,19 @@ class IORegister { } + /** + * @param string $sName + * @param string $sType + * + * @return void + * @throws \Combodo\iTop\Forms\IO\FormBlockIOException + */ public function AddInput(string $sName, string $sType): void { $oFormInput = new FormInput($sName, $sType, $this->oFormBlock); + if (array_key_exists($oFormInput->GetName(), $this->aInputs)) { + throw new RegisterException('Input already exists '.json_encode($oFormInput->GetName()).' for '.json_encode($this->oFormBlock->GetName())); + } $this->aInputs[$oFormInput->GetName()] = $oFormInput; } @@ -45,7 +55,9 @@ class IORegister * @param string $sOutputName * * @return $this - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Block\FormBlockException + * @throws \Combodo\iTop\Forms\IO\FormBlockIOException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function AddInputDependsOn(string $sName, string $sOutputBlockName, string $sOutputName): self { @@ -66,11 +78,16 @@ class IORegister * @param string $sOutputName the dependency output name * * @return $this - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Block\FormBlockException + * @throws \Combodo\iTop\Forms\IO\FormBlockIOException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function DependsOn(string $sInputName, string $sOutputBlockName, string $sOutputName): self { - $oOutputBlock = $this->oFormBlock->GetParent()->Get($sOutputBlockName); + $oOutputBlock = $this->oFormBlock->GetParent()?->Get($sOutputBlockName); + if (is_null($oOutputBlock)) { + throw new RegisterException('Output block not found '.json_encode($sOutputBlockName)); + } $oFormInput = $this->GetInput($sInputName); $oFormOutput = $oOutputBlock->GetOutput($sOutputName); $oFormOutput->BindToInput($oFormInput); @@ -85,7 +102,9 @@ class IORegister * @param string $sParentOutputName parent output name * * @return $this - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Block\FormBlockException + * @throws \Combodo\iTop\Forms\IO\FormBlockIOException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function ImpactParent(string $sOutputName, string $sParentOutputName): self { @@ -99,6 +118,9 @@ class IORegister public function AddOutput(string $sName, string $sType, AbstractConverter $oConverter = null): void { $oFormOutput = new FormOutput($sName, $sType, $this->oFormBlock, $oConverter); + if (array_key_exists($oFormOutput->GetName(), $this->aOutputs)) { + throw new RegisterException('Output already exists '.json_encode($oFormOutput->GetName()).' for '.json_encode($this->oFormBlock->GetName())); + } $this->aOutputs[$oFormOutput->GetName()] = $oFormOutput; } @@ -108,12 +130,12 @@ class IORegister * @param string $sName * * @return FormInput - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function GetInput(string $sName): FormInput { if (!array_key_exists($sName, $this->aInputs)) { - throw new FormBlockException('Missing input '.$sName.' for '.$this->oFormBlock->GetName()); + throw new RegisterException('Missing input '.json_encode($sName).' for '.json_encode($this->oFormBlock->GetName())); } return $this->aInputs[$sName]; @@ -167,12 +189,12 @@ class IORegister * @param string $sName output name * * @return FormOutput - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function GetOutput(string $sName): FormOutput { if (!array_key_exists($sName, $this->aOutputs)) { - throw new FormBlockException('Missing output '.json_encode($sName).' for '.json_encode($this->oFormBlock->GetName())); + throw new RegisterException('Missing output '.json_encode($sName).' for '.json_encode($this->oFormBlock->GetName())); } return $this->aOutputs[$sName]; @@ -304,7 +326,9 @@ class IORegister * @param string $sParentInputName parent input name * * @return $this - * @throws FormBlockException + * @throws \Combodo\iTop\Forms\Block\FormBlockException + * @throws \Combodo\iTop\Forms\IO\FormBlockIOException + * @throws \Combodo\iTop\Forms\Register\RegisterException */ public function DependsOnParent(string $sInputName, string $sParentInputName): self { diff --git a/sources/Forms/Register/RegisterException.php b/sources/Forms/Register/RegisterException.php index 466b626796..60b7ee31e0 100644 --- a/sources/Forms/Register/RegisterException.php +++ b/sources/Forms/Register/RegisterException.php @@ -7,13 +7,8 @@ namespace Combodo\iTop\Forms\Register; -use Exception; -use Throwable; +use Combodo\iTop\Forms\FormsException; -class RegisterException extends Exception +class RegisterException extends FormsException { - public function __construct(string $message = '', int $code = 0, ?Throwable $previous = null, array $aContext = []) - { - parent::__construct($message, $code, $previous); - } } diff --git a/tests/php-unit-tests/unitary-tests/sources/Forms/AbstractFormsTest.php b/tests/php-unit-tests/unitary-tests/sources/Forms/AbstractFormsTest.php index 0877ffe364..32161ccab6 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Forms/AbstractFormsTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Forms/AbstractFormsTest.php @@ -35,22 +35,15 @@ abstract class AbstractFormsTest extends ItopDataTestCase return new FormOutput($sName.'_output', $sType, $oBlock); } - public function GivenFormBlock(string $sName, array $aOptions = [], array $aIOs = []): AbstractFormBlock + public function GivenFormBlock(string $sName, string $sBlockClass = FormBlock::class): AbstractFormBlock { - $oBlock = new FormBlock($sName, $aOptions); + return new $sBlockClass($sName, []); + } - foreach ($aIOs as $aIO) { - if ($aIO['io_type'] === FormInput::class) { - $oBlock->AddInput($aIO['name'], $aIO['data_type']); - } else { - if (isset($aIO['converter_class'])) { - $oBlock->AddOutput($aIO['name'], $aIO['data_type'], new $aIO['converter_class']()); - } else { - $oBlock->AddOutput($aIO['name'], $aIO['data_type']); - } - } - } + public function GivenSubFormBlock(AbstractFormBlock $oParent, string $sName, string $ssBlockClass = FormBlock::class): AbstractFormBlock + { + $oParent->Add($sName, $ssBlockClass, []); - return $oBlock; + return $oParent->Get($sName); } } diff --git a/tests/php-unit-tests/unitary-tests/sources/Forms/Block/BlockTest.php b/tests/php-unit-tests/unitary-tests/sources/Forms/Block/BlockTest.php index 2bb7d8ef06..3ed2c1075a 100644 --- a/tests/php-unit-tests/unitary-tests/sources/Forms/Block/BlockTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/Forms/Block/BlockTest.php @@ -14,8 +14,10 @@ use Combodo\iTop\Forms\Block\Base\TextFormBlock; use Combodo\iTop\Forms\Block\FormBlockException; use Combodo\iTop\Forms\Forms; use Combodo\iTop\Forms\IFormBlock; +use Combodo\iTop\Forms\IO\Format\StringIOFormat; +use Combodo\iTop\Forms\Register\RegisterException; use Combodo\iTop\Service\InterfaceDiscovery\InterfaceDiscovery; -use Combodo\iTop\Test\UnitTest\ItopDataTestCase; +use Combodo\iTop\Test\UnitTest\sources\Forms\AbstractFormsTest; use OutOfBoundsException; use ReflectionException; use Symfony\Component\Form\AbstractType; @@ -25,7 +27,7 @@ use Symfony\Component\Form\Extension\Core\Type\TextType; * Test forms block. * */ -class BlockTest extends ItopDataTestCase +class BlockTest extends AbstractFormsTest { /** * Block get form type must return a class derived from Symfony form AbstractType. @@ -110,4 +112,57 @@ class BlockTest extends ItopDataTestCase $this->expectException(OutOfBoundsException::class); $oForm->get('birthdate'); } + + public function testAddingTwiceTheSameInputThrowsException(): void + { + $oFormBlock = $this->GivenFormBlock('OneBlock') + ->AddInput('test_input', StringIOFormat::class); + + $this->expectException(RegisterException::class); + $oFormBlock->AddInput('test_input', StringIOFormat::class); + } + + public function testAddingTwiceTheSameOutputThrowsException(): void + { + $oFormBlock = $this->GivenFormBlock('OneBlock') + ->AddOutput('test_output', StringIOFormat::class); + + $this->expectException(RegisterException::class); + $oFormBlock->AddOutput('test_output', StringIOFormat::class); + } + + public function testDependingOnNonExistingInputThrowsException(): void + { + $oParentBlock = $this->GivenFormBlock('ParentBlock'); + + $oFormBlock = $this->GivenSubFormBlock($oParentBlock, 'OneBlock') + ->AddInput('test_input', StringIOFormat::class); + + $this->GivenSubFormBlock($oParentBlock, 'OtherBlock') + ->AddOutput('test_output', StringIOFormat::class); + + $this->expectException(RegisterException::class); + $oFormBlock->DependsOn('non_existing_input', 'OtherBlock', 'test_output'); + } + + public function testDependingOnNonExistingOutputThrowsException(): void + { + $oParentBlock = $this->GivenFormBlock('ParentBlock'); + $oFormBlock = $this->GivenSubFormBlock($oParentBlock, 'OneBlock') + ->AddInput('test_input', StringIOFormat::class); + $this->GivenSubFormBlock($oParentBlock, 'OtherBlock') + ->AddOutput('test_output', StringIOFormat::class); + + $this->expectException(RegisterException::class); + $oFormBlock->DependsOn('test_input', 'OtherBlock', 'non_existing_output'); + } + + public function testDependingOnNonExistingBlockThrowsException(): void + { + $oFormBlock = $this->GivenFormBlock('OneBlock') + ->AddOutput('test_output', StringIOFormat::class); + + $this->expectException(RegisterException::class); + $oFormBlock->DependsOn('test_input', 'UnknownBlock', 'test'); + } }