Merge remote-tracking branch 'origin/feature/8772_form_dependencies_manager' into feature/8772_form_dependencies_manager

sources/Application/WebPage/ErrorPage.php

@@ -60,7 +60,7 @@ class ErrorPage extends NiceWebPage
 			$this->add('<div class="message message-valid">'.nl2br(Dict::S('UI:ErrorPage:KittyDisclaimer')).'</div>');
 		}
 		if (!is_null($oException)) {
-			$this->log_exception($sText, $oException);
+			$this->log_exception($oException->getMessage(), $oException);
 			return;
 		}
 		$this->log_error($sText);

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 (!is_a($sBlockClass, AbstractFormBlock::class, true)) {
-		if ($oRef->isSubclassOf(AbstractFormBlock::class) === false) {
+			throw new FormBlockException('The block type '.json_encode($sBlockClass).' is not a subclass of AbstractFormBlock.');
-			throw new FormBlockException("The block type '$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];
 	}
 

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);
-	}
 }

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
 	{

sources/Forms/Register/RegisterException.php

@@ -7,13 +7,8 @@
 
 namespace Combodo\iTop\Forms\Register;
 
-use Exception;
+use Combodo\iTop\Forms\FormsException;
-use Throwable;
 
-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);
-	}
 }

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) {
+	public function GivenSubFormBlock(AbstractFormBlock $oParent, string $sName, string $ssBlockClass = FormBlock::class): AbstractFormBlock
-			if ($aIO['io_type'] === FormInput::class) {
+	{
-				$oBlock->AddInput($aIO['name'], $aIO['data_type']);
+		$oParent->Add($sName, $ssBlockClass, []);
-			} 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']);
-				}
-			}
-		}
 
-		return $oBlock;
+		return $oParent->Get($sName);
 	}
 }

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');
+	}
 }