[TASK] impexp: Remove 'max number of records' restriction 81/55381/3
authorChristian Kuhn <lolli@schwarzbu.ch>
Wed, 17 Jan 2018 11:35:56 +0000 (12:35 +0100)
committerChristian Kuhn <lolli@schwarzbu.ch>
Wed, 17 Jan 2018 12:49:18 +0000 (13:49 +0100)
The 'export' module has a restriction to limit the number
of exported records. The patch removes this from the
interface:
* The default limit is arbitrary, removing that limit
  declutters the interface a little bit
* Users probably always did just set this to some number
  high enough to export everything they wanted
* It was not clear what exactly is counted. At least it
  was not the total number of exported records ...
* The error and flash messages were unclear and sometimes
  not shown at all
* The limit could lead to data integrity issues in the
  export file, for example if a page links to some other
  page that is not exported
* Presets that increased the limit still work
* To prevent a breaking patch, the affected PHP method
  arguments deprecate some arguments instead of fully
  removing them in v9

Change-Id: Iadb365ff2ccd77ed38cbde81b76e37990a0c6f17
Resolves: #83592
Releases: master
Reviewed-on: https://review.typo3.org/55381
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
Reviewed-by: Reiner Teubner <rteubner@me.com>
Tested-by: Reiner Teubner <rteubner@me.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
typo3/sysext/core/Documentation/Changelog/9.1/Deprecation-83592-ImpexpRemovedMaximumNumberOfRecordsRestriction.rst [new file with mode: 0644]
typo3/sysext/impexp/Classes/Controller/ImportExportController.php
typo3/sysext/impexp/Resources/Private/Language/locallang.xlf
typo3/sysext/impexp/Resources/Private/Language/locallang_csh.xlf
typo3/sysext/impexp/Resources/Private/Partials/Export/Configuration.html
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php

diff --git a/typo3/sysext/core/Documentation/Changelog/9.1/Deprecation-83592-ImpexpRemovedMaximumNumberOfRecordsRestriction.rst b/typo3/sysext/core/Documentation/Changelog/9.1/Deprecation-83592-ImpexpRemovedMaximumNumberOfRecordsRestriction.rst
new file mode 100644 (file)
index 0000000..f9b494e
--- /dev/null
@@ -0,0 +1,48 @@
+.. include:: ../../Includes.txt
+
+=============================================================================
+Deprecation: #83592 - impexp: Removed "Maximum number of records" restriction
+=============================================================================
+
+See :issue:`83592`
+
+Description
+===========
+
+When exporting pages or records using the "Export" interface of
+extension :php:`impexp`, the restriction to export only a maximum
+number of records has been removed.
+
+
+Impact
+======
+
+The export module now exports any number of records. It is up
+to the user to restrict this in a sane way: During import the
+number of records influences import runtime. This heavily depends
+on given server performance, an artificial limit given by the system
+is not suitable.
+
+On PHP level, two export related methods changed their signature:
+
+* :php:`TYPO3\CMS\Impexp\Controller\ImportExportController->addRecordsForPid()` -
+  Third method argument deprecated
+
+* :php:`TYPO3\CMS\Impexp\Controller\ImportExportController->exec_listQueryPid` -
+  Third method argument deprecated
+
+
+Affected Installations
+======================
+
+Backend interface users are probably not affected much: Users of the
+export module usually set the 'maximum number of records' value high
+enough to export everything they wanted already. The interface now
+just misses the according input fields.
+
+On PHP level, the extension scanner will find extensions that use
+the changed methods and checks if they are called with the
+correct number of arguments. Additionally, :php:`E_USER_DEPRECATED`
+errors are logged at runtime if using these methods the old way.
+
+.. index:: Backend, PHP-API, FullyScanned, ext:impexp
\ No newline at end of file
index a42c600..0391585 100644 (file)
@@ -368,8 +368,6 @@ class ImportExportController extends BaseScriptClass
     {
         // BUILDING EXPORT DATA:
         // Processing of InData array values:
-        $inData['pagetree']['maxNumber'] = MathUtility::forceIntegerInRange($inData['pagetree']['maxNumber'], 1, 1000000, 100);
-        $inData['listCfg']['maxNumber'] = MathUtility::forceIntegerInRange($inData['listCfg']['maxNumber'], 1, 1000000, 100);
         $inData['maxFileSize'] = MathUtility::forceIntegerInRange($inData['maxFileSize'], 1, 1000000, 1000);
         $inData['filename'] = trim(preg_replace('/[^[:alnum:]._-]*/', '', preg_replace('/\\.(t3d|xml)$/', '', $inData['filename'])));
         if (strlen($inData['filename'])) {
@@ -430,7 +428,7 @@ class ImportExportController extends BaseScriptClass
             foreach ($inData['list'] as $ref) {
                 $rParts = explode(':', $ref);
                 if ($beUser->check('tables_select', $rParts[0])) {
-                    $statement = $this->exec_listQueryPid($rParts[0], $rParts[1], MathUtility::forceIntegerInRange($inData['listCfg']['maxNumber'], 1));
+                    $statement = $this->exec_listQueryPid($rParts[0], $rParts[1]);
                     while ($subTrow = $statement->fetch()) {
                         $this->export->export_addRecord($rParts[0], $subTrow);
                     }
@@ -450,7 +448,7 @@ class ImportExportController extends BaseScriptClass
                 $this->treeHTML = $pagetree->printTree($tree);
                 $idH = $pagetree->buffer_idH;
             } elseif ($inData['pagetree']['levels'] == -2) {
-                $this->addRecordsForPid($inData['pagetree']['id'], $inData['pagetree']['tables'], $inData['pagetree']['maxNumber']);
+                $this->addRecordsForPid($inData['pagetree']['id'], $inData['pagetree']['tables']);
             } else {
                 // Based on depth
                 // Drawing tree:
@@ -494,7 +492,7 @@ class ImportExportController extends BaseScriptClass
                 $flatList = $this->export->setPageTree($idH);
                 foreach ($flatList as $k => $value) {
                     $this->export->export_addRecord('pages', BackendUtility::getRecord('pages', $k));
-                    $this->addRecordsForPid($k, $inData['pagetree']['tables'], $inData['pagetree']['maxNumber']);
+                    $this->addRecordsForPid($k, $inData['pagetree']['tables']);
                 }
             }
         }
@@ -605,9 +603,9 @@ class ImportExportController extends BaseScriptClass
      *
      * @param int $k Page id for which to select records to add
      * @param array $tables Array of table names to select from
-     * @param int $maxNumber Max amount of records to select
+     * @param int $maxNumber @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
-    public function addRecordsForPid($k, $tables, $maxNumber)
+    public function addRecordsForPid($k, $tables, $maxNumber = null)
     {
         if (!is_array($tables)) {
             return;
@@ -615,7 +613,13 @@ class ImportExportController extends BaseScriptClass
         foreach ($GLOBALS['TCA'] as $table => $value) {
             if ($table !== 'pages' && (in_array($table, $tables) || in_array('_ALL', $tables))) {
                 if ($this->getBackendUser()->check('tables_select', $table) && !$GLOBALS['TCA'][$table]['ctrl']['is_static']) {
-                    $statement = $this->exec_listQueryPid($table, $k, MathUtility::forceIntegerInRange($maxNumber, 1));
+                    if ($maxNumber !== null) {
+                        // @deprecated since TYPO3 v9, will be removed in TYPO3 v10. Remove this if in v10
+                        // and the 3rd method argument. trigger_error() is called by method exec_listQueryPid() below
+                        $statement = $this->exec_listQueryPid($table, $k, MathUtility::forceIntegerInRange($maxNumber, 1));
+                    } else {
+                        $statement = $this->exec_listQueryPid($table, $k);
+                    }
                     while ($subTrow = $statement->fetch()) {
                         $this->export->export_addRecord($table, $subTrow);
                     }
@@ -629,11 +633,20 @@ class ImportExportController extends BaseScriptClass
      *
      * @param string $table Table to select from
      * @param int $pid Page ID to select from
-     * @param int $limit Max number of records to select
+     * @param int $limit @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      * @return \Doctrine\DBAL\Driver\Statement Query statement
      */
-    public function exec_listQueryPid($table, $pid, $limit)
+    public function exec_listQueryPid($table, $pid, $limit = null)
     {
+        // @deprecated In v10, remove this if and the method argument
+        if ($limit !== null) {
+            trigger_error(
+                'The third argument of addRecordsForPid() and exec_listQueryPid() has been'
+                . ' deprecated, do not limit exports anymore. The parameter will be removed in TYPO3 v10.',
+                E_USER_DEPRECATED
+            );
+        }
+
         $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($table);
 
         $orderBy = $GLOBALS['TCA'][$table]['ctrl']['sortby'] ?: $GLOBALS['TCA'][$table]['ctrl']['default_sortby'];
@@ -653,8 +666,12 @@ class ImportExportController extends BaseScriptClass
                     'pid',
                     $queryBuilder->createNamedParameter($pid, \PDO::PARAM_INT)
                 )
-            )
-            ->setMaxResults($limit);
+            );
+
+        // @deprecated In v10, remove this if
+        if ($limit !== null) {
+            $queryBuilder->setMaxResults($limit);
+        }
 
         foreach (QueryHelper::parseOrderBy((string)$orderBy) as $orderPair) {
             list($fieldName, $order) = $orderPair;
@@ -663,8 +680,8 @@ class ImportExportController extends BaseScriptClass
 
         $statement = $queryBuilder->execute();
 
-        // Warning about hitting limit:
-        if ($statement->rowCount() == $limit) {
+        // @deprecated In v10, remove this if, and the two getLL locallang target keys
+        if ($limit !== null && $statement->rowCount() == $limit) {
             $limitWarning = sprintf($this->lang->getLL('makeconfig_anSqlQueryReturned'), $limit);
             /** @var FlashMessage $flashMessage */
             $flashMessage = GeneralUtility::makeInstance(
index d37b5cb..b524344 100644 (file)
@@ -78,9 +78,6 @@
                        <trans-unit id="makeconfig_includeTables">
                                <source>Include tables:</source>
                        </trans-unit>
-                       <trans-unit id="makeconfig_maxNumberOfRecords">
-                               <source>Max number of records:</source>
-                       </trans-unit>
                        <trans-unit id="makeconfig_exportSingleRecord">
                                <source>Export single record:</source>
                        </trans-unit>
index 82898ae..8cafb9d 100644 (file)
@@ -121,12 +121,6 @@ EXT:impexp/Resources/Public/Images/cshimages/export1.png</source>
                                <source>Here two tables from different pages are selected. This is done by first selecting the one, saving it as a preset, then select the other and merge with the saved preset.
 Here you see how to use the Web&gt;List module to export all records from a table on a page.</source>
                        </trans-unit>
-                       <trans-unit id="tableListMaxNumber.alttitle">
-                               <source>Max number to export</source>
-                       </trans-unit>
-                       <trans-unit id="tableListMaxNumber.description">
-                               <source>Run-away brake that will make sure a large amount of records does not halt the interface. Default is 100, but you can change it to anything you like of course.</source>
-                       </trans-unit>
                        <trans-unit id="inclRelations.alttitle">
                                <source>Include relations</source>
                        </trans-unit>
index cf2b056..5a6b268 100644 (file)
                                                                 options="{tableSelectOptions}" multiple="multiple" value="{inData.pagetree.tables}"
                                                                 size="{f:if(condition: '{tableSelectOptions -> f:count()} > 9', then: '10', else: '5')}" />
 </div>
-<div class="form-group">
-       <label for="impexp-pagetree-maxnumber">
-               <f:translate key="makeconfig_maxNumberOfRecords" />
-       </label>
-       <f:form.textfield class="form-control" name="tx_impexp[pagetree][maxNumber]" id="impexp-pagetree-maxnumber" value="{inData.pagetree.maxNumber}" />
-</div>
 <f:if condition="{records -> f:count()} > 0">
        <h4>
                <f:translate key="makeconfig_exportSingleRecord" />
                        </f:for>
                </div>
        </div>
-       <div class="form-group">
-               <label for="impexp-listcfg-maxnumber">
-                       <f:translate key="makeconfig_maxNumberOfRecords" />
-                       <f:be.buttons.csh table="xMOD_tx_impexp" field="tableListMaxNumber" />
-               </label>
-               <f:form.textfield class="form-control" name="tx_impexp[listCfg][maxNumber]" id="impexp-listcfg-maxnumber"value="{inData.listCfg.maxNumber}" />
-       </div>
 </f:if>
 <h4><f:translate key="makeconfig_relationsAndExclusions" /></h4>
 <div class="form-group">
index b69d451..f67d613 100644 (file)
@@ -135,4 +135,16 @@ return [
             'Breaking-83241-ExtbaseRemovedCustomFunctionalityForDataMapper-getPlainValue.rst',
         ],
     ],
+    'TYPO3\CMS\Impexp\Controller\ImportExportController->addRecordsForPid' => [
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-83592-ImpexpRemovedMaximumNumberOfRecordsRestriction.rst'
+        ],
+    ],
+    'TYPO3\CMS\Impexp\Controller\ImportExportController->exec_listQueryPid' => [
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-83592-ImpexpRemovedMaximumNumberOfRecordsRestriction.rst'
+        ],
+    ],
 ];