Export plugins refactoring (#18071)

This is a simplification of the code to remove confusing ($row[$j] == '0' || $row[$j] != ''). To make the code simpler, I added a @var annotation and replace for with foreach.

I realize this adds Psalm warnings, and they are valid warnings. But they are because of dead code related to $GLOBALS['what']. This just looks like another nonsense of its own that needs a little more work to be removed so I kept it out of this PR. For the time being, let's keep them in Psalm baseline.

* Simplify ExportCsv
* Simplify ExportHtmlword
* Simplify ExportTexytext
* Fix broken tests

https://github.com/phpmyadmin/phpmyadmin/pull/18071

---------

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
This commit is contained in:
Kamil Tekiela 2023-02-15 12:18:05 +00:00 committed by GitHub
parent 2b35303aae
commit 9187dc8ec2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 67 additions and 53 deletions

View File

@ -9,6 +9,7 @@ namespace PhpMyAdmin\Plugins\Export;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Dbal\Connection;
use PhpMyAdmin\Dbal\ResultInterface;
use PhpMyAdmin\Plugins\ExportPlugin;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup;
@ -18,11 +19,10 @@ use PhpMyAdmin\Properties\Options\Items\TextPropertyItem;
use PhpMyAdmin\Properties\Plugins\ExportPluginProperties;
use function __;
use function implode;
use function mb_strtolower;
use function mb_substr;
use function preg_replace;
use function str_replace;
use function trim;
use const PHP_EOL;
@ -214,7 +214,7 @@ class ExportCsv extends ExportPlugin
): bool {
$GLOBALS['what'] = $GLOBALS['what'] ?? null;
$GLOBALS['csv_terminated'] = $GLOBALS['csv_terminated'] ?? null;
$GLOBALS['csv_separator'] = $GLOBALS['csv_separator'] ?? null;
$GLOBALS['csv_separator'] = $GLOBALS['csv_separator'] ?? '';
$GLOBALS['csv_enclosed'] = $GLOBALS['csv_enclosed'] ?? null;
$GLOBALS['csv_escaped'] = $GLOBALS['csv_escaped'] ?? null;
@ -222,26 +222,30 @@ class ExportCsv extends ExportPlugin
$table_alias = $table;
$this->initAlias($aliases, $db_alias, $table_alias);
// Gets the data from the database
/**
* Gets the data from the database
*
* @var ResultInterface $result
* @psalm-ignore-var
*/
$result = $GLOBALS['dbi']->query(
$sqlQuery,
Connection::TYPE_USER,
DatabaseInterface::QUERY_UNBUFFERED
);
$fields_cnt = $result->numFields();
// If required, get fields name at the first line
if (isset($GLOBALS['csv_columns']) && $GLOBALS['csv_columns']) {
$schema_insert = '';
$insertFields = [];
foreach ($result->getFieldNames() as $col_as) {
if (! empty($aliases[$db]['tables'][$table]['columns'][$col_as])) {
$col_as = $aliases[$db]['tables'][$table]['columns'][$col_as];
}
if ($GLOBALS['csv_enclosed'] == '') {
$schema_insert .= $col_as;
$insertFields[] = $col_as;
} else {
$schema_insert .= $GLOBALS['csv_enclosed']
$insertFields[] = $GLOBALS['csv_enclosed']
. str_replace(
$GLOBALS['csv_enclosed'],
$GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'],
@ -249,11 +253,9 @@ class ExportCsv extends ExportPlugin
)
. $GLOBALS['csv_enclosed'];
}
$schema_insert .= $GLOBALS['csv_separator'];
}
$schema_insert = trim(mb_substr($schema_insert, 0, -1));
$schema_insert = implode($GLOBALS['csv_separator'], $insertFields);
if (! $this->export->outputHandler($schema_insert . $GLOBALS['csv_terminated'])) {
return false;
}
@ -261,68 +263,63 @@ class ExportCsv extends ExportPlugin
// Format the data
while ($row = $result->fetchRow()) {
$schema_insert = '';
for ($j = 0; $j < $fields_cnt; $j++) {
if (! isset($row[$j])) {
$schema_insert .= $GLOBALS[$GLOBALS['what'] . '_null'];
} elseif ($row[$j] == '0' || $row[$j] != '') {
$insertValues = [];
foreach ($row as $field) {
if ($field === null) {
$insertValues[] = $GLOBALS[$GLOBALS['what'] . '_null'];
} elseif ($field !== '') {
// always enclose fields
if ($GLOBALS['what'] === 'excel') {
$row[$j] = preg_replace("/\015(\012)?/", "\012", $row[$j]);
$field = preg_replace("/\015(\012)?/", "\012", $field);
}
// remove CRLF characters within field
if (
isset($GLOBALS[$GLOBALS['what'] . '_removeCRLF']) && $GLOBALS[$GLOBALS['what'] . '_removeCRLF']
) {
$row[$j] = str_replace(
$field = str_replace(
[
"\r",
"\n",
],
'',
$row[$j]
$field
);
}
if ($GLOBALS['csv_enclosed'] == '') {
$schema_insert .= $row[$j];
$insertValues[] = $field;
} else {
// also double the escape string if found in the data
if ($GLOBALS['csv_escaped'] != $GLOBALS['csv_enclosed']) {
$schema_insert .= $GLOBALS['csv_enclosed']
$insertValues[] = $GLOBALS['csv_enclosed']
. str_replace(
$GLOBALS['csv_enclosed'],
$GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'],
str_replace(
$GLOBALS['csv_escaped'],
$GLOBALS['csv_escaped'] . $GLOBALS['csv_escaped'],
$row[$j]
$field
)
)
. $GLOBALS['csv_enclosed'];
} else {
// avoid a problem when escape string equals enclose
$schema_insert .= $GLOBALS['csv_enclosed']
$insertValues[] = $GLOBALS['csv_enclosed']
. str_replace(
$GLOBALS['csv_enclosed'],
$GLOBALS['csv_escaped'] . $GLOBALS['csv_enclosed'],
$row[$j]
$field
)
. $GLOBALS['csv_enclosed'];
}
}
} else {
$schema_insert .= '';
$insertValues[] = '';
}
if ($j >= $fields_cnt - 1) {
continue;
}
$schema_insert .= $GLOBALS['csv_separator'];
}
$schema_insert = implode($GLOBALS['csv_separator'], $insertValues);
if (! $this->export->outputHandler($schema_insert . $GLOBALS['csv_terminated'])) {
return false;
}

View File

@ -10,6 +10,7 @@ namespace PhpMyAdmin\Plugins\Export;
use PhpMyAdmin\Database\Triggers;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Dbal\Connection;
use PhpMyAdmin\Dbal\ResultInterface;
use PhpMyAdmin\Plugins\ExportPlugin;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup;
@ -201,13 +202,17 @@ class ExportHtmlword extends ExportPlugin
return false;
}
// Gets the data from the database
/**
* Gets the data from the database
*
* @var ResultInterface $result
* @psalm-ignore-var
*/
$result = $GLOBALS['dbi']->query(
$sqlQuery,
Connection::TYPE_USER,
DatabaseInterface::QUERY_UNBUFFERED
);
$fields_cnt = $result->numFields();
// If required, get fields name at the first line
if (isset($GLOBALS['htmlword_columns'])) {
@ -231,13 +236,11 @@ class ExportHtmlword extends ExportPlugin
// Format the data
while ($row = $result->fetchRow()) {
$schema_insert = '<tr class="print-category">';
for ($j = 0; $j < $fields_cnt; $j++) {
if (! isset($row[$j])) {
foreach ($row as $field) {
if ($field === null) {
$value = $GLOBALS[$GLOBALS['what'] . '_null'];
} elseif ($row[$j] == '0' || $row[$j] != '') {
$value = $row[$j];
} else {
$value = '';
$value = $field;
}
$schema_insert .= '<td class="print">'

View File

@ -10,6 +10,7 @@ namespace PhpMyAdmin\Plugins\Export;
use PhpMyAdmin\Database\Triggers;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Dbal\Connection;
use PhpMyAdmin\Dbal\ResultInterface;
use PhpMyAdmin\Plugins\ExportPlugin;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyMainGroup;
use PhpMyAdmin\Properties\Options\Groups\OptionsPropertyRootGroup;
@ -181,13 +182,17 @@ class ExportTexytext extends ExportPlugin
return false;
}
// Gets the data from the database
/**
* Gets the data from the database
*
* @var ResultInterface $result
* @psalm-ignore-var
*/
$result = $GLOBALS['dbi']->query(
$sqlQuery,
Connection::TYPE_USER,
DatabaseInterface::QUERY_UNBUFFERED
);
$fields_cnt = $result->numFields();
// If required, get fields name at the first line
if (isset($GLOBALS[$GLOBALS['what'] . '_columns'])) {
@ -209,11 +214,11 @@ class ExportTexytext extends ExportPlugin
// Format the data
while ($row = $result->fetchRow()) {
$text_output = '';
for ($j = 0; $j < $fields_cnt; $j++) {
if (! isset($row[$j])) {
foreach ($row as $field) {
if ($field === null) {
$value = $GLOBALS[$GLOBALS['what'] . '_null'];
} elseif ($row[$j] == '0' || $row[$j] != '') {
$value = $row[$j];
} elseif ($field !== '') {
$value = $field;
} else {
$value = ' ';
}

View File

@ -6260,6 +6260,11 @@ parameters:
count: 1
path: libraries/classes/Plugins/Export/ExportCsv.php
-
message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#"
count: 3
path: libraries/classes/Plugins/Export/ExportCsv.php
-
message: "#^Method PhpMyAdmin\\\\Plugins\\\\Export\\\\ExportHtmlword\\:\\:exportData\\(\\) has parameter \\$aliases with no value type specified in iterable type array\\.$#"
count: 1

View File

@ -10899,6 +10899,9 @@
</RedundantCondition>
</file>
<file src="libraries/classes/Plugins/Export/ExportCsv.php">
<InvalidArgument>
<code>$insertValues</code>
</InvalidArgument>
<InvalidArrayOffset>
<code>$GLOBALS['excel_columns']</code>
<code>$GLOBALS['what']</code>
@ -10907,17 +10910,20 @@
<MixedArgument>
<code>$col_as</code>
</MixedArgument>
<MixedArgumentTypeCoercion>
<code>$insertFields</code>
</MixedArgumentTypeCoercion>
<MixedAssignment>
<code>$GLOBALS['what']</code>
<code>$GLOBALS['what']</code>
<code>$col_as</code>
<code>$insertFields[]</code>
<code>$insertValues[]</code>
</MixedAssignment>
<MixedOperand>
<code>$GLOBALS[$GLOBALS['what'] . '_null']</code>
<code>$GLOBALS['what']</code>
<code>$GLOBALS['what']</code>
<code>$GLOBALS['what']</code>
<code>$col_as</code>
</MixedOperand>
<PossiblyNullArgument>
<code>$GLOBALS['csv_enclosed']</code>
@ -10941,8 +10947,6 @@
<code>$GLOBALS['csv_escaped']</code>
<code>$GLOBALS['csv_escaped']</code>
<code>$GLOBALS['csv_escaped']</code>
<code>$GLOBALS['csv_separator']</code>
<code>$GLOBALS['csv_separator']</code>
<code>$GLOBALS['csv_terminated']</code>
<code>$GLOBALS['csv_terminated']</code>
</PossiblyNullOperand>

View File

@ -404,7 +404,7 @@ class ExportCsvTest extends AbstractTestCase
$result = ob_get_clean();
$this->assertEquals(
'idnamedatetimefiel;1abcd2011-01-20 02:00:02;2foo2010-01-20 02:00:02;3Abcd2012-01-20 02:00:02;',
'idnamedatetimefield;1abcd2011-01-20 02:00:02;2foo2010-01-20 02:00:02;3Abcd2012-01-20 02:00:02;',
$result
);
@ -422,7 +422,7 @@ class ExportCsvTest extends AbstractTestCase
$result = ob_get_clean();
$this->assertEquals(
'"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";'
'"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";'
. '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";',
$result
);
@ -443,7 +443,7 @@ class ExportCsvTest extends AbstractTestCase
$result = ob_get_clean();
$this->assertEquals(
'"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";'
'"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";'
. '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";',
$result
);
@ -463,7 +463,7 @@ class ExportCsvTest extends AbstractTestCase
$result = ob_get_clean();
$this->assertEquals(
'"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";'
'"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";'
. '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";',
$result
);
@ -483,7 +483,7 @@ class ExportCsvTest extends AbstractTestCase
$result = ob_get_clean();
$this->assertEquals(
'"id""name""datetimefield;"1""abcd""2011-01-20 02:00:02";'
'"id""name""datetimefield";"1""abcd""2011-01-20 02:00:02";'
. '"2""foo""2010-01-20 02:00:02";"3""Abcd""2012-01-20 02:00:02";',
$result
);