From: rschueppel Date: Wed, 6 Dec 2017 16:44:00 +0000 (+0100) Subject: [TASK] Resolve Todos in InheritancesResolverService X-Git-Tag: v9.2.0~571 X-Git-Url: http://git.typo3.org/Packages/TYPO3.CMS.git/commitdiff_plain/728c4feda87cefc8f7fb4dd93579edf63c63e228 [TASK] Resolve Todos in InheritancesResolverService Add some comments to understand InheritanceResolverService class. Minor refactoring of duplicated code. Add Test for the doc example. Resolves: #83238 Releases: master, 8.7 Change-Id: I3aed06a76b609ef78641530a19e3379f202c84ec Reviewed-on: https://review.typo3.org/54948 Tested-by: TYPO3com Reviewed-by: Anja Leichsenring Tested-by: Anja Leichsenring Reviewed-by: Mathias Schreiber Tested-by: Mathias Schreiber Reviewed-by: Christian Kuhn Tested-by: Christian Kuhn --- diff --git a/typo3/sysext/form/Classes/Mvc/Configuration/InheritancesResolverService.php b/typo3/sysext/form/Classes/Mvc/Configuration/InheritancesResolverService.php index 0ab799325cca..c561afeef91c 100644 --- a/typo3/sysext/form/Classes/Mvc/Configuration/InheritancesResolverService.php +++ b/typo3/sysext/form/Classes/Mvc/Configuration/InheritancesResolverService.php @@ -16,7 +16,6 @@ namespace TYPO3\CMS\Form\Mvc\Configuration; */ use TYPO3\CMS\Core\Utility\ArrayUtility; -use TYPO3\CMS\Core\Utility\Exception\MissingArrayPathException; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Object\ObjectManager; use TYPO3\CMS\Form\Mvc\Configuration\Exception\CycleInheritancesException; @@ -24,6 +23,38 @@ use TYPO3\CMS\Form\Mvc\Configuration\Exception\CycleInheritancesException; /** * Resolve declared inheritances within an configuration array * + * Basic concept: + * - Take a large YAML config and replace the key '__inheritance' by the referenced YAML partial (of the same config file) + * - Maybe also override some keys of the referenced partial + * - Avoid endless loop by reference cycles + * + * e.g. + * --------------------- + * + * Form: + * part1: + * key1: value1 + * key2: value2 + * key3: value3 + * part2: + * __inheritance: + * 10: Form.part1 + * key2: another_value + * + * will result in: + * --------------------- + * + * Form: + * part1: + * key1: value1 + * key2: value2 + * key3: value3 + * part2: + * key1: value1 + * key2: another_value + * key3: value3 + * + * --------------------- * Scope: frontend / backend * @internal */ @@ -52,7 +83,7 @@ class InheritancesResolverService protected $inheritanceStack = []; /** - * Needed to park a configuration path for cyclically inheritances + * Needed to buffer a configuration path for cyclically inheritances * detection while inheritances for this path is ongoing. * * @var string @@ -122,9 +153,14 @@ class InheritancesResolverService /** * Resolve all inheritances within a configuration. * - * @todo: More description - * @param array $configuration - * @param array $pathStack + * Takes a YAML config mapped to associative array $configuration + * - replace all findings of key '__inheritance' recursively + * - perform a deep search in config by iteration, thus check for endless loop by reference cycle + * + * Return the completed configuration. + * + * @param array $configuration - a mapped YAML configuration (full or partial) + * @param array $pathStack - an identifier for YAML key as array (Form.part1.key => {Form, part1, key}) * @param bool $setInheritancePathToCkeck * @return array */ @@ -134,43 +170,36 @@ class InheritancesResolverService bool $setInheritancePathToCkeck = true ): array { foreach ($configuration as $key => $values) { + //add current key to pathStack $pathStack[] = $key; $path = implode('.', $pathStack); + //check endless loop for current path $this->throwExceptionIfCycleInheritances($path, $path); + + //overwrite service property 'inheritancePathToCheck' with current path if ($setInheritancePathToCkeck) { $this->inheritancePathToCkeck = $path; } + //if value of subnode is an array, perform a deep search iteration step if (is_array($configuration[$key])) { if (isset($configuration[$key][self::INHERITANCE_OPERATOR])) { - try { - $inheritances = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $path . '.' . self::INHERITANCE_OPERATOR, - '.' - ); - } catch (MissingArrayPathException $exception) { - $inheritances = null; - } + $inheritances = $this->getValueByPath($this->referenceConfiguration, $path . '.' . self::INHERITANCE_OPERATOR); + //and replace the __inheritance operator by the respective partial if (is_array($inheritances)) { $inheritedConfigurations = $this->resolveInheritancesRecursive($inheritances); - - $configuration[$key] = array_replace_recursive( - $inheritedConfigurations, - $configuration[$key] - ); + $configuration[$key] = array_replace_recursive($inheritedConfigurations, $configuration[$key]); } + //remove the inheritance operator from configuration unset($configuration[$key][self::INHERITANCE_OPERATOR]); } if (!empty($configuration[$key])) { - $configuration[$key] = $this->resolve( - $configuration[$key], - $pathStack - ); + // resolve subnode of YAML config + $configuration[$key] = $this->resolve($configuration[$key], $pathStack); } } array_pop($pathStack); @@ -182,7 +211,8 @@ class InheritancesResolverService /** * Additional helper for the resolve method. * - * @todo: More description + * Takes all inheritances (an array of YAML paths), and check them for endless loops + * * @param array $inheritances * @return array * @throws CycleInheritancesException @@ -193,15 +223,7 @@ class InheritancesResolverService $inheritedConfigurations = []; foreach ($inheritances as $inheritancePath) { $this->throwExceptionIfCycleInheritances($inheritancePath, $inheritancePath); - try { - $inheritedConfiguration = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $inheritancePath, - '.' - ); - } catch (MissingArrayPathException $exception) { - $inheritedConfiguration = null; - } + $inheritedConfiguration = $this->getValueByPath($this->referenceConfiguration, $inheritancePath); if ( isset($inheritedConfiguration[self::INHERITANCE_OPERATOR]) @@ -250,56 +272,23 @@ class InheritancesResolverService /** * Throw an exception if a cycle is detected. * - * @todo: More description * @param string $path * @param string $pathToCheck * @throws CycleInheritancesException */ protected function throwExceptionIfCycleInheritances(string $path, string $pathToCheck) { - try { - $configuration = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $path, - '.' - ); - } catch (MissingArrayPathException $exception) { - $configuration = null; - } + $configuration = $this->getValueByPath($this->referenceConfiguration, $path); if (isset($configuration[self::INHERITANCE_OPERATOR])) { - try { - $inheritances = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $path . '.' . self::INHERITANCE_OPERATOR, - '.' - ); - } catch (MissingArrayPathException $exception) { - $inheritances = null; - } + $inheritances = $this->getValueByPath($this->referenceConfiguration, $path . '.' . self::INHERITANCE_OPERATOR); if (is_array($inheritances)) { foreach ($inheritances as $inheritancePath) { - try { - $configuration = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $inheritancePath, - '.' - ); - } catch (MissingArrayPathException $exception) { - $configuration = null; - } + $configuration = $this->getValueByPath($this->referenceConfiguration, $inheritancePath); if (isset($configuration[self::INHERITANCE_OPERATOR])) { - try { - $_inheritances = ArrayUtility::getValueByPath( - $this->referenceConfiguration, - $inheritancePath . '.' . self::INHERITANCE_OPERATOR, - '.' - ); - } catch (MissingArrayPathException $exception) { - $_inheritances = null; - } + $_inheritances = $this->getValueByPath($this->referenceConfiguration, $inheritancePath . '.' . self::INHERITANCE_OPERATOR); foreach ($_inheritances as $_inheritancePath) { if (strpos($pathToCheck, $_inheritancePath) === 0) { @@ -350,4 +339,22 @@ class InheritancesResolverService } return $result; } + + /** + * Check the given array representation of a YAML config for the given path and return it's value / sub-array. + * If path is not found, return null; + * + * @param array $config + * @param string $path + * @param string $delimiter + * @return string|array|null + */ + protected function getValueByPath(array $config, string $path, string $delimiter = '.') + { + try { + return ArrayUtility::getValueByPath($config, $path, $delimiter); + } catch (\RuntimeException $exception) { + return null; + } + } } diff --git a/typo3/sysext/form/Tests/Unit/Mvc/Configuration/InheritancesResolverServiceTest.php b/typo3/sysext/form/Tests/Unit/Mvc/Configuration/InheritancesResolverServiceTest.php index fa73a7bf901a..208e37b58963 100644 --- a/typo3/sysext/form/Tests/Unit/Mvc/Configuration/InheritancesResolverServiceTest.php +++ b/typo3/sysext/form/Tests/Unit/Mvc/Configuration/InheritancesResolverServiceTest.php @@ -32,6 +32,46 @@ class InheritancesResolverServiceTest extends \TYPO3\TestingFramework\Core\Unit\ $this->subject = new InheritancesResolverService(); } + /** + * @test + * Test for the explicit example in service class comment + */ + public function getDocExampleInheritance() + { + $input = [ + 'Form' => [ + 'part1' => [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ], + 'part2' => [ + '__inheritances' => [ + 10 => 'Form.part1', + ], + 'key2' => 'another_value' + ], + ], + ]; + + $expected = [ + 'Form' => [ + 'part1' => [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ], + 'part2' => [ + 'key1' => 'value1', + 'key2' => 'another_value', + 'key3' => 'value3' + ], + ], + ]; + + $this->assertSame($expected, $this->subject->reset()->setReferenceConfiguration($input)->getResolvedConfiguration($input)); + } + /** * @test */