[BUGFIX] EXT:form - catch YAML parsing errors 14/54014/7
authorDaniel Lorenz <daniel.lorenz@extco.de>
Fri, 8 Sep 2017 11:52:32 +0000 (13:52 +0200)
committerChristian Kuhn <lolli@schwarzbu.ch>
Fri, 8 Sep 2017 15:00:03 +0000 (17:00 +0200)
Catche YAML parsing errors and display this them alongside
their form definition files in form module and plugin.

Resolves: #82369
Releases: master, 8.7
Change-Id: Icf71027d21d0a8e30c238a51369676715de2e5c5
Reviewed-on: https://review.typo3.org/54014
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Mathias Brodala <mbrodala@pagemachine.de>
Tested-by: Mathias Brodala <mbrodala@pagemachine.de>
Reviewed-by: Carlos Meyer <cm@davitec.de>
Tested-by: Carlos Meyer <cm@davitec.de>
Reviewed-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Tested-by: Bjoern Jacob <bjoern.jacob@tritum.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/form/Classes/Hooks/DataStructureIdentifierHook.php
typo3/sysext/form/Classes/Mvc/Configuration/YamlSource.php
typo3/sysext/form/Classes/Mvc/Persistence/FormPersistenceManager.php
typo3/sysext/form/Resources/Private/Backend/Templates/FormManager/Index.html
typo3/sysext/form/Resources/Private/Language/Database.xlf
typo3/sysext/form/Tests/Unit/Hooks/DataStructureIdentifierHookTest.php

index 13436ba..cf284ea 100644 (file)
@@ -110,10 +110,19 @@ class DataStructureIdentifierHook
                         $formIsAccessible = true;
                     }
 
-                    $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
-                        $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
-                        $form['persistenceIdentifier'],
-                    ];
+                    if ($form['invalid']) {
+                        $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
+                            $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
+                            $form['persistenceIdentifier'],
+                            'overlay-missing'
+                        ];
+                    } else {
+                        $dataStructure['sheets']['sDEF']['ROOT']['el']['settings.persistenceIdentifier']['TCEforms']['config']['items'][] = [
+                            $form['name'] . ' (' . $form['persistenceIdentifier'] . ')',
+                            $form['persistenceIdentifier'],
+                            'content-form'
+                        ];
+                    }
                 }
 
                 if (!empty($identifier['ext-form-persistenceIdentifier']) && !$formIsAccessible) {
index d342c54..a82c83d 100644 (file)
@@ -107,7 +107,7 @@ class YamlSource
                 }
             } catch (ParseException $exception) {
                 throw new ParseErrorException(
-                    'A parse error occurred while parsing file "' . $fileIdentifier . '". Error message: ' . $exception->getMessage(),
+                    'An error occurred while parsing file "' . $fileIdentifier . '": ' . $exception->getMessage(),
                     1480195405
                 );
             }
index fca1789..64eef01 100644 (file)
@@ -105,7 +105,22 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
         } else {
             $file = $this->getFileByIdentifier($persistenceIdentifier);
         }
-        return $this->yamlSource->load([$file]);
+
+        try {
+            $yaml = $this->yamlSource->load([$file]);
+        } catch (\Exception $e) {
+            $yaml = [
+                'identifier' => $file->getCombinedIdentifier(),
+                'label' => $e->getMessage(),
+                'error' => [
+                    'code' => $e->getCode(),
+                    'message' => $e->getMessage()
+                ],
+                'invalid' => true,
+            ];
+        }
+
+        return $yaml;
     }
 
     /**
@@ -237,6 +252,8 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
                     'removable' => true,
                     'location' => 'storage',
                     'duplicateIdentifier' => false,
+                    'invalid' => $form['invalid'],
+                    'error' => $form['error'],
                 ];
                 $identifiers[$form['identifier']]++;
             }
@@ -258,6 +275,8 @@ class FormPersistenceManager implements FormPersistenceManagerInterface
                     'removable' => $this->formSettings['persistenceManager']['allowDeleteFromExtensionPaths'] ? true: false,
                     'location' => 'extension',
                     'duplicateIdentifier' => false,
+                    'invalid' => $form['invalid'],
+                    'error' => $form['error'],
                 ];
                 $identifiers[$form['identifier']]++;
             }
index 26aec41..e59648e 100644 (file)
                                                        <f:for each="{forms}" as="form">
                                                                <tr>
                                                                        <td class="col-icon nowrap">
-                                                                               <f:if condition="{form.duplicateIdentifier}">
+                                                                               <f:if condition="{form.invalid}">
                                                                                        <f:then>
-                                                                                               <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_identifier')} {form.identifier}" data-toggle="tooltip" data-placement="top">
+                                                                                               <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.invalid')}" data-toggle="tooltip" data-placement="top">
                                                                                                        <core:icon identifier="overlay-missing" />
                                                                                                </span>
                                                                                        </f:then>
+                                                                                       <f:else if="{form.duplicateIdentifier}">
+                                                                                               <span title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_identifier')} {form.identifier}" data-toggle="tooltip" data-placement="top">
+                                                                                                       <core:icon identifier="overlay-missing" />
+                                                                                               </span>
+                                                                                       </f:else>
                                                                                        <f:else>
                                                                                                <span title="id={form.identifier}" data-toggle="tooltip" data-placement="right">
                                                                                                        <core:icon identifier="content-elements-mailform" />
@@ -47,9 +52,9 @@
                                                                                </f:if>
                                                                        </td>
                                                                        <td class="col-title col-responsive nowrap">
-                                                                               <f:if condition="{form.readOnly}">
+                                                                               <f:if condition="{form.invalid} || {form.readOnly}">
                                                                                                <f:then>
-                                                                                                       <div>{form.name}</div>
+                                                                                                       <div title="{form.name}">{form.name}</div>
                                                                                                </f:then>
                                                                                                <f:else>
                                                                                                        <f:link.action controller="FormEditor" action="index" arguments="{formPersistenceIdentifier: form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form')}" data="{toggle: 'tooltip', placement: 'right'}">{form.name}</f:link.action>
@@ -58,7 +63,7 @@
                                                                        </td>
                                                                        <td class="col-control nowrap">
                                                                                <div class="btn-group" role="group">
-                                                                                       <f:if condition="{form.readOnly}">
+                                                                                       <f:if condition="{form.invalid} || {form.readOnly}">
                                                                                                <f:then>
                                                                                                        <button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form_not_allowed')}"><core:icon identifier="actions-open" /></button>
                                                                                                </f:then>
                                                                                                        <f:link.action controller="FormEditor" action="index" arguments="{formPersistenceIdentifier: form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.edit_form')}" class="btn btn-default form-record-open"><core:icon identifier="actions-open" /></f:link.action>
                                                                                                </f:else>
                                                                                        </f:if>
-                                                                                       <a href="#" data-identifier="duplicateForm" data-form-persistence-identifier="{form.persistenceIdentifier}" data-form-name="{form.name}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_this_form')}" class="btn btn-default form-record-duplicate"><core:icon identifier="t3-form-icon-duplicate" /></a>
-                                                                                       <f:if condition="{form.removable}">
+                                                                                       <f:if condition="{form.invalid}">
                                                                                                <f:then>
-                                                                                                       <a href="#" data-identifier="removeForm" data-form-persistence-identifier="{form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form')}" class="btn btn-default form-record-delete"><core:icon identifier="actions-edit-delete" /></a>
+                                                                                                       <button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_form_not_allowed')}"><core:icon identifier="t3-form-icon-duplicate" /></button>
+                                                                                                       <button class="btn btn-default form-record-readonly" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
                                                                                                </f:then>
                                                                                                <f:else>
-                                                                                                       <button class="btn btn-default form-record-delete" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
+                                                                                                       <a href="#" data-identifier="duplicateForm" data-form-persistence-identifier="{form.persistenceIdentifier}" data-form-name="{form.name}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.duplicate_this_form')}" class="btn btn-default form-record-duplicate"><core:icon identifier="t3-form-icon-duplicate" /></a>
+                                                                                                       <f:if condition="{form.removable}">
+                                                                                                               <f:then>
+                                                                                                                       <a href="#" data-identifier="removeForm" data-form-persistence-identifier="{form.persistenceIdentifier}" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form')}" class="btn btn-default form-record-delete"><core:icon identifier="actions-edit-delete" /></a>
+                                                                                                               </f:then>
+                                                                                                               <f:else>
+                                                                                                                       <button class="btn btn-default form-record-delete" disabled="disabled" title="{f:translate(key: 'LLL:EXT:form/Resources/Private/Language/Database.xlf:formManager.delete_form_not_allowed')}"><core:icon identifier="actions-edit-delete" /></button>
+                                                                                                               </f:else>
+                                                                                                       </f:if>
                                                                                                </f:else>
                                                                                        </f:if>
                                                                                </div>
index 34651b1..92877d9 100644 (file)
             <trans-unit id="formManager.edit_form_not_allowed" xml:space="preserve">
                 <source>Edit this form is not allowed.</source>
             </trans-unit>
+            <trans-unit id="formManager.duplicate_form_not_allowed" xml:space="preserve">
+                <source>Duplicate this form is not allowed.</source>
+            </trans-unit>
             <trans-unit id="formManager.delete_form_not_allowed" xml:space="preserve">
                 <source>Delete this form is not allowed.</source>
             </trans-unit>
+            <trans-unit id="formManager.invalid" xml:space="preserve">
+                <source>Invalid YAML file!</source>
+            </trans-unit>
             <trans-unit id="formManager.duplicate_identifier" xml:space="preserve">
                 <source>Duplicate identifier!</source>
             </trans-unit>
index 7ac9369..74bc44f 100644 (file)
@@ -171,8 +171,12 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
 
     /**
      * @test
+     * @dataProvider parseDataStructureByIdentifierPostProcessDataProvider
+     *
+     * @param array $formDefinition
+     * @param array $expectedItem
      */
-    public function parseDataStructureByIdentifierPostProcessAddsExistingFormItems()
+    public function parseDataStructureByIdentifierPostProcessAddsExistingFormItems(array $formDefinition, array $expectedItem)
     {
         $objectMangerProphecy = $this->prophesize(ObjectManager::class);
         GeneralUtility::setSingletonInstance(ObjectManager::class, $objectMangerProphecy->reveal());
@@ -180,17 +184,7 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
         $objectMangerProphecy->get(FormPersistenceManagerInterface::class)
             ->willReturn($formPersistenceManagerProphecy->reveal());
 
-        $existingForms = [
-            [
-                'persistenceIdentifier' => 'hugo1',
-                'name' => 'myHugo1',
-            ],
-            [
-                'persistenceIdentifier' => 'hugo2',
-                'name' => 'myHugo2',
-            ]
-        ];
-        $formPersistenceManagerProphecy->listForms()->shouldBeCalled()->willReturn($existingForms);
+        $formPersistenceManagerProphecy->listForms()->shouldBeCalled()->willReturn([$formDefinition]);
 
         $incomingDataStructure = [
             'sheets' => [
@@ -228,14 +222,7 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
                                                 0 => 'default, no value',
                                                 1 => '',
                                             ],
-                                            1 => [
-                                                0 => 'myHugo1 (hugo1)',
-                                                1 => 'hugo1',
-                                            ],
-                                            2 => [
-                                                0 => 'myHugo2 (hugo2)',
-                                                1 => 'hugo2',
-                                            ],
+                                            1 => $expectedItem,
                                         ],
                                     ],
                                 ],
@@ -255,6 +242,39 @@ class DataStructureIdentifierHookTest extends \TYPO3\TestingFramework\Core\Unit\
     }
 
     /**
+     * @return array
+     */
+    public function parseDataStructureByIdentifierPostProcessDataProvider(): array
+    {
+        return [
+            'simple' => [
+                [
+                    'persistenceIdentifier' => 'hugo1',
+                    'name' => 'myHugo1',
+                ],
+                [
+                    'myHugo1 (hugo1)',
+                    'hugo1',
+                    'content-form',
+                ],
+            ],
+            'invalid' => [
+                [
+                    'persistenceIdentifier' => 'Error.yaml',
+                    'label' => 'Test Error Label',
+                    'name' => 'Test Error Name',
+                    'invalid' => true,
+                ],
+                [
+                    'Test Error Name (Error.yaml)',
+                    'Error.yaml',
+                    'overlay-missing',
+                ],
+            ],
+        ];
+    }
+
+    /**
      * Data provider for implodeArrayKeysReturnsString
      *
      * @return array