[BUGFIX] Improve domObjectId splitting for inline in flexform 18/48518/4
authorAnja Leichsenring <aleichsenring@ab-softlab.de>
Wed, 8 Jun 2016 11:56:15 +0000 (13:56 +0200)
committerSusanne Moog <typo3@susannemoog.de>
Thu, 9 Jun 2016 09:43:51 +0000 (11:43 +0200)
We use a regex to retrieve relevant informations for the handling
of flexform datastructures and inline records. It failed for some
use cases like deeply nested structures.
A rework of the regex and some additional unit tests stabilize
this fragile area.

Resolves: #76268
Relates: #73004
Releases: master, 7.6
Change-Id: I4f2afed7f07b3f4f0346bf7a63541e8313f7a992
Reviewed-on: https://review.typo3.org/48518
Reviewed-by: ToM <mail@interactive-values.at>
Tested-by: ToM <mail@interactive-values.at>
Reviewed-by: Susanne Moog <typo3@susannemoog.de>
Tested-by: Susanne Moog <typo3@susannemoog.de>
typo3/sysext/backend/Classes/Controller/FormInlineAjaxController.php
typo3/sysext/backend/Tests/Unit/Controller/FormInlineAjaxControllerTest.php

index 0dc3915..a7e698e 100644 (file)
@@ -872,46 +872,11 @@ class FormInlineAjaxController
      */
     protected function getParentConfigFromFlexForm(array $parentConfig, $domObjectId)
     {
-        // Substitute FlexForm addition and make parsing a bit easier
-        $domObjectId = str_replace('---', ':', $domObjectId);
-        // The starting pattern of an object identifier (e.g. "data-<firstPidValue>-<anything>)
-        $pattern = '/^data' . '-' . '(?<firstPidValue>.+?)' . '-' . '(?<anything>.+)$/';
+        list($flexFormPath, $foreignTableName) = $this->splitDomObjectId($domObjectId);
 
-        $flexFormPath = [];
-        // Will be checked against the FlexForm configuration as an additional safeguard
-        $foreignTableName = '';
-
-        if (preg_match($pattern, $domObjectId, $match)) {
-            // For new records the flexform path should be the second to last array element,
-            // followed by the foreign table name. For existing records it should be the third
-            // array element from the end as the UID of the inline record is provided as well.
-            $parts = array_slice(explode('-', $match['anything'], 4), -2, 2);
-
-            if (count($parts) !== 2 || !isset($parts[0]) || strpos($parts[0], ':') === false) {
-                throw new \UnexpectedValueException(
-                    'DOM Object ID ' . $domObjectId . ' does not contain required information '
-                    . 'to extract inline field configuration.',
-                    1446996136
-                );
-            }
-
-            $fieldParts = GeneralUtility::trimExplode(':', $parts[0]);
-
-            // FlexForm parts start with data:
-            if (empty($fieldParts) || !isset($fieldParts[1]) || $fieldParts[1] !== 'data') {
-                throw new \UnexpectedValueException(
-                    'Malformed flexform identifier: ' . $parts[2],
-                    1446996254
-                );
-            }
-
-            $flexFormPath = array_slice($fieldParts, 2);
-            $foreignTableNameParts = explode('-', $parts[1]);
-            $foreignTableName = $foreignTableNameParts[0];
-        }
 
         $childConfig = $parentConfig['ds']['sheets'];
-
+        $flexFormPath = explode(':', $flexFormPath);
         foreach ($flexFormPath as $flexFormNode) {
             // We are dealing with configuration information from a flexform,
             // not value storage, identifiers that reference language or
@@ -969,4 +934,50 @@ class FormInlineAjaxController
 
         return $databaseRow;
     }
+
+    /**
+     * split the domObjectID and retrieve the needed parts
+     *
+     * @param string $domObjectId
+     *
+     * @return array
+     */
+    protected function splitDomObjectId($domObjectId)
+    {
+
+        // Substitute FlexForm addition and make parsing a bit easier
+        $domObjectId = str_replace('---', ':', $domObjectId);
+        $pattern = '/:data:(?<flexformPath>.*?)-(?<tableName>[^-]+)(?:-(?:NEW)?\w+)?$/';
+
+        /* EXPLANATION for the regex:
+         * according https://regex101.com/
+         *
+         * :data: matches the characters :data: literally (case sensitive)
+         * (?<flexformPath>.*?) Named capturing group flexformPath
+         * .*? matches any character (except newline)
+         * Quantifier: *? Between zero and unlimited times, as few times as possible, expanding as needed [lazy]
+         * - matches the character - literally
+         * (?<tableName>[^-]+) Named capturing group tableName
+         * [^-]+ match a single character not present in the list below
+         * Quantifier: + Between one and unlimited times, as many times as possible, giving back as needed [greedy]
+         * - the literal character -
+         * (?:-(?:NEW)?\w+)? Non-capturing group
+         * Quantifier: ? Between zero and one time, as many times as possible, giving back as needed [greedy]
+         * - matches the character - literally
+         * (?:NEW)? Non-capturing group
+         * Quantifier: ? Between zero and one time, as many times as possible, giving back as needed [greedy]
+         * NEW matches the characters NEW literally (case sensitive)
+         * \w+ match any word character [a-zA-Z0-9_]
+         * Quantifier: + Between one and unlimited times, as many times as possible, giving back as needed [greedy]
+         * $ assert position at end of a line
+         */
+
+        if (preg_match($pattern, $domObjectId, $match)) {
+
+            return array($match['flexformPath'], $match['tableName']);
+        }
+
+        return [];
+
+    }
 }
index 69c9d59..0433ea8 100644 (file)
@@ -62,4 +62,108 @@ class FormInlineAjaxControllerTest extends UnitTestCase
         $mayUploadFile = $mockObject->_call('checkInlineFileTypeAccessForField', $selectorData, $fileData);
         $this->assertTrue($mayUploadFile);
     }
+
+
+    /**
+     * @dataProvider splitDomObjectIdDataProviderForTableName
+     * @param string $dataStructure
+     * @param string $expectedTableName
+     * @test
+     *
+     * test for the flexform domobject identifier split
+     */
+    public function splitDomObjectIdResolvesTablenameCorrectly($dataStructure, $expectedTableName)
+    {
+        $mock = $this->getAccessibleMock(FormInlineAjaxController::class, ['dummy'], [], '', false);
+        $result = $mock->_call('splitDomObjectId', $dataStructure);
+        $this->assertSame($expectedTableName, $result[1]);
+    }
+
+    /**
+     * @return array
+     */
+    public function splitDomObjectIdDataProviderForTableName()
+    {
+        return [
+            'news new' => [
+                'data-335-tx_news_domain_model_news-2-content_elements-tt_content-999-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file',
+                'tx_news_domain_model_file'
+            ],
+            'load existing child' => [
+                'data-318-tx_styleguide_flex-4-flex_3---data---sInline---lDEF---inline_1---vDEF-tx_styleguide_flex_flex_3_inline_1_child-4',
+                'tx_styleguide_flex_flex_3_inline_1_child'
+            ],
+            'create new child' => [
+                'data-318-tx_styleguide_flex-4-flex_3---data---sInline---lDEF---inline_1---vDEF-tx_styleguide_flex_flex_3_inline_1_child',
+                'tx_styleguide_flex_flex_3_inline_1_child'
+            ],
+            'insert new after' => [
+                'data-336-tt_content-1000-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file-6',
+                'tx_news_domain_model_file'
+            ],
+            'fal simple' => [
+                'data-336-tt_content-998-pi_flexform---data---sheet.tabGeneral---lDEF---settings.image---vDEF-sys_file_reference-837',
+                'sys_file_reference'
+            ],
+            'fal down deep' => [
+                'data-335-tx_news_domain_model_news-2-content_elements-tt_content-999-pi_flexform---data---sheet.tabGeneral---lDEF---settings.image---vDEF-sys_file_reference',
+                'sys_file_reference'
+            ],
+            'new record after others' => ['data-336-tt_content-1000-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file-NEW5757f36287214984252204', 'tx_news_domain_model_file'],
+        ];
+    }
+
+    /**
+     * @dataProvider splitDomObjectIdDataProviderForFlexFormPath
+     *
+     * @param string $dataStructure
+     * @param string $expectedFlexformPath
+     *
+     * @test
+     *
+     * test for the flexform domobject identifier split
+     */
+    public function splitDomObjectIdResolvesFlexformPathCorrectly($dataStructure, $expectedFlexformPath)
+    {
+        $mock = $this->getAccessibleMock(FormInlineAjaxController::class, ['dummy'], [], '', false);
+        $result = $mock->_call('splitDomObjectId', $dataStructure);
+        $this->assertSame($expectedFlexformPath, $result[0]);
+    }
+
+    /**
+     * @return array
+     */
+    public function splitDomObjectIdDataProviderForFlexFormPath()
+    {
+        return [
+            'news new' => [
+                'data-335-tx_news_domain_model_news-2-content_elements-tt_content-999-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file',
+                'sheet.tabGeneral:lDEF:settings.related_files:vDEF'
+            ],
+            'load existing child' => [
+                'data-318-tx_styleguide_flex-4-flex_3---data---sInline---lDEF---inline_1---vDEF-tx_styleguide_flex_flex_3_inline_1_child-4',
+                'sInline:lDEF:inline_1:vDEF'
+            ],
+            'create new child' => [
+                'data-318-tx_styleguide_flex-4-flex_3---data---sInline---lDEF---inline_1---vDEF-tx_styleguide_flex_flex_3_inline_1_child',
+                'sInline:lDEF:inline_1:vDEF'
+            ],
+            'insert new after' => [
+                'data-336-tt_content-1000-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file-6',
+                'sheet.tabGeneral:lDEF:settings.related_files:vDEF'
+            ],
+            'fal simple' => [
+                'data-336-tt_content-998-pi_flexform---data---sheet.tabGeneral---lDEF---settings.image---vDEF-sys_file_reference-837',
+                'sheet.tabGeneral:lDEF:settings.image:vDEF'
+            ],
+            'fal down deep' => [
+                'data-335-tx_news_domain_model_news-2-content_elements-tt_content-999-pi_flexform---data---sheet.tabGeneral---lDEF---settings.image---vDEF-sys_file_reference',
+                'sheet.tabGeneral:lDEF:settings.image:vDEF'
+            ],
+            'new record after others' => [
+                'data-336-tt_content-1000-pi_flexform---data---sheet.tabGeneral---lDEF---settings.related_files---vDEF-tx_news_domain_model_file-NEW5757f36287214984252204',
+                'sheet.tabGeneral:lDEF:settings.related_files:vDEF'
+            ],
+        ];
+    }
 }