From dcd8a086cac942339d1920ade19cd4311590dc6c Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 10 Feb 2024 14:41:59 +0100 Subject: [PATCH 1/3] Change how new table for CSV is generated Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 5 ----- src/Plugins/Import/ImportCsv.php | 20 +++++++------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2c519664be..2a3de8d772 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -8696,7 +8696,6 @@ $nameArray === false - $nameArray === false @@ -8717,10 +8716,6 @@ - - $result - $result - diff --git a/src/Plugins/Import/ImportCsv.php b/src/Plugins/Import/ImportCsv.php index 1ed2e05d9e..d3804174f2 100644 --- a/src/Plugins/Import/ImportCsv.php +++ b/src/Plugins/Import/ImportCsv.php @@ -28,10 +28,11 @@ use function array_pad; use function array_shift; use function basename; use function count; +use function in_array; use function mb_strlen; -use function mb_strtolower; use function mb_substr; use function preg_grep; +use function preg_quote; use function preg_replace; use function preg_split; use function rtrim; @@ -690,8 +691,8 @@ class ImportCsv extends AbstractImportCsv private function getTableNameFromImport(string $databaseName): string { $importFileName = basename(ImportSettings::$importFileName, '.csv'); - $importFileName = mb_strtolower($importFileName); - $importFileName = (string) preg_replace('/[^a-zA-Z0-9_]/', '_', $importFileName); + $importFileName = rtrim($importFileName); + $importFileName = (string) preg_replace('/[^\x{0001}-\x{FFFF}]/u', '_', $importFileName); // get new table name, if user didn't provide one, set the default name if (isset($_REQUEST['csv_new_tbl_name']) && (string) $_REQUEST['csv_new_tbl_name'] !== '') { @@ -699,23 +700,16 @@ class ImportCsv extends AbstractImportCsv } if ($databaseName !== '') { - $result = DatabaseInterface::getInstance()->fetchResult('SHOW TABLES'); - - // logic to get table name from filename - // if no table then use filename as table name - if ($result === []) { - return $importFileName; - } + $existingTables = DatabaseInterface::getInstance()->getTables($databaseName); // check to see if {filename} as table exist - $nameArray = preg_grep('/' . $importFileName . '/isU', $result); // if no use filename as table name - if ($nameArray === false || $nameArray === []) { + if (! in_array($importFileName, $existingTables, true)) { return $importFileName; } // check if {filename}_ as table exist - $nameArray = preg_grep('/' . $importFileName . '_/isU', $result); + $nameArray = preg_grep('/^' . preg_quote($importFileName, '/') . '_/isU', $existingTables); if ($nameArray === false) { return $importFileName; } From d7fd5d59ce2a31279398d56e3e631a9ff25c7bb5 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 10 Feb 2024 15:11:38 +0100 Subject: [PATCH 2/3] FR: Refactor name generation of file imports Signed-off-by: Kamil Tekiela --- psalm-baseline.xml | 11 +++----- src/Import/Import.php | 34 +++++++++++++++++++++++++ src/Plugins/Import/ImportCsv.php | 35 ++++++-------------------- src/Plugins/Import/ImportMediawiki.php | 27 +++++++------------- src/Plugins/Import/ImportShp.php | 8 +++--- 5 files changed, 58 insertions(+), 57 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 2a3de8d772..be897aca5f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6425,8 +6425,11 @@ DatabaseInterface::getInstance() DatabaseInterface::getInstance() DatabaseInterface::getInstance() + DatabaseInterface::getInstance() + DatabaseInterface::getInstance() + $nameArray === false statements[0])]]> @@ -8692,11 +8695,7 @@ DatabaseInterface::getInstance() DatabaseInterface::getInstance() - DatabaseInterface::getInstance() - - $nameArray === false - $colNames @@ -8796,9 +8795,6 @@ - - DatabaseInterface::getInstance() - string @@ -8871,7 +8867,6 @@ Config::getInstance() - DatabaseInterface::getInstance() diff --git a/src/Import/Import.php b/src/Import/Import.php index fc2172c58b..e72b0a1b71 100644 --- a/src/Import/Import.php +++ b/src/Import/Import.php @@ -31,6 +31,7 @@ use function explode; use function function_exists; use function htmlspecialchars; use function implode; +use function in_array; use function is_numeric; use function max; use function mb_chr; @@ -41,8 +42,11 @@ use function mb_strpos; use function mb_strtoupper; use function mb_substr; use function mb_substr_count; +use function preg_grep; use function preg_match; +use function preg_quote; use function preg_replace; +use function rtrim; use function sprintf; use function str_contains; use function str_starts_with; @@ -1381,4 +1385,34 @@ class Import $active, ); } + + public function getNextAvailableTableName(string $databaseName, string $proposedTableName): string + { + if ($proposedTableName === '') { + $proposedTableName = 'TABLE'; + } + + $importFileName = rtrim($proposedTableName); + $importFileName = (string) preg_replace('/[^\x{0001}-\x{FFFF}]/u', '_', $importFileName); + + if ($databaseName !== '') { + $existingTables = DatabaseInterface::getInstance()->getTables($databaseName); + + // check to see if {filename} as table exist + // if no use filename as table name + if (! in_array($importFileName, $existingTables, true)) { + return $importFileName; + } + + // check if {filename}_ as table exist + $nameArray = preg_grep('/^' . preg_quote($importFileName, '/') . '_/isU', $existingTables); + if ($nameArray === false) { + return $importFileName; + } + + return $importFileName . '_' . (count($nameArray) + 1); + } + + return $importFileName; + } } diff --git a/src/Plugins/Import/ImportCsv.php b/src/Plugins/Import/ImportCsv.php index d3804174f2..bb0b153cd5 100644 --- a/src/Plugins/Import/ImportCsv.php +++ b/src/Plugins/Import/ImportCsv.php @@ -26,14 +26,10 @@ use PhpMyAdmin\Util; use function __; use function array_pad; use function array_shift; -use function basename; use function count; -use function in_array; use function mb_strlen; use function mb_substr; -use function preg_grep; -use function preg_quote; -use function preg_replace; +use function pathinfo; use function preg_split; use function rtrim; use function str_contains; @@ -41,6 +37,8 @@ use function strlen; use function strtr; use function trim; +use const PATHINFO_FILENAME; + /** * Handles the import for the CSV format */ @@ -690,34 +688,15 @@ class ImportCsv extends AbstractImportCsv private function getTableNameFromImport(string $databaseName): string { - $importFileName = basename(ImportSettings::$importFileName, '.csv'); - $importFileName = rtrim($importFileName); - $importFileName = (string) preg_replace('/[^\x{0001}-\x{FFFF}]/u', '_', $importFileName); - // get new table name, if user didn't provide one, set the default name if (isset($_REQUEST['csv_new_tbl_name']) && (string) $_REQUEST['csv_new_tbl_name'] !== '') { return $_REQUEST['csv_new_tbl_name']; } - if ($databaseName !== '') { - $existingTables = DatabaseInterface::getInstance()->getTables($databaseName); - - // check to see if {filename} as table exist - // if no use filename as table name - if (! in_array($importFileName, $existingTables, true)) { - return $importFileName; - } - - // check if {filename}_ as table exist - $nameArray = preg_grep('/^' . preg_quote($importFileName, '/') . '_/isU', $existingTables); - if ($nameArray === false) { - return $importFileName; - } - - return $importFileName . '_' . (count($nameArray) + 1); - } - - return $importFileName; + return $this->import->getNextAvailableTableName( + $databaseName, + pathinfo(ImportSettings::$importFileName, PATHINFO_FILENAME), + ); } /** diff --git a/src/Plugins/Import/ImportMediawiki.php b/src/Plugins/Import/ImportMediawiki.php index 6593480cc9..76e4def01a 100644 --- a/src/Plugins/Import/ImportMediawiki.php +++ b/src/Plugins/Import/ImportMediawiki.php @@ -8,7 +8,6 @@ declare(strict_types=1); namespace PhpMyAdmin\Plugins\Import; use PhpMyAdmin\Current; -use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\File; use PhpMyAdmin\Import\ImportSettings; use PhpMyAdmin\Import\ImportTable; @@ -21,6 +20,7 @@ use function count; use function explode; use function mb_strlen; use function mb_substr; +use function pathinfo; use function preg_match; use function str_contains; use function str_replace; @@ -28,6 +28,8 @@ use function str_starts_with; use function strlen; use function trim; +use const PATHINFO_FILENAME; + /** * Handles the import for the MediaWiki format */ @@ -288,7 +290,12 @@ class ImportMediawiki extends ImportPlugin { if ($this->analyze) { // Set the table name - $this->setTableName($table->tableName); + if ($table->tableName === '') { + $table->tableName = $this->import->getNextAvailableTableName( + Current::$database, + pathinfo(ImportSettings::$importFileName, PATHINFO_FILENAME), + ); + } // Set generic names for table headers if they don't exist if ($table->columns === []) { @@ -311,22 +318,6 @@ class ImportMediawiki extends ImportPlugin $this->import->runQuery('', $sqlStatements); } - /** - * Sets the table name - * - * @param string $tableName reference to the name of the table - */ - private function setTableName(string &$tableName): void - { - if ($tableName !== '') { - return; - } - - $result = DatabaseInterface::getInstance()->fetchResult('SHOW TABLES'); - // todo check if the name below already exists - $tableName = 'TABLE ' . (count($result) + 1); - } - /** * Set generic names for table headers, if they don't exist * diff --git a/src/Plugins/Import/ImportShp.php b/src/Plugins/Import/ImportShp.php index df94983a36..e85434342a 100644 --- a/src/Plugins/Import/ImportShp.php +++ b/src/Plugins/Import/ImportShp.php @@ -9,7 +9,6 @@ namespace PhpMyAdmin\Plugins\Import; use PhpMyAdmin\Config; use PhpMyAdmin\Current; -use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\File; use PhpMyAdmin\Gis\GisFactory; use PhpMyAdmin\Gis\GisMultiLineString; @@ -40,6 +39,7 @@ use function trim; use function unlink; use const LOCK_EX; +use const PATHINFO_FILENAME; /** * Handles the import for ESRI Shape files @@ -265,8 +265,10 @@ class ImportShp extends ImportPlugin // Set table name based on the number of tables if (Current::$database !== '') { - $result = DatabaseInterface::getInstance()->fetchResult('SHOW TABLES'); - $tableName = 'TABLE ' . (count($result) + 1); + $tableName = $this->import->getNextAvailableTableName( + Current::$database, + pathinfo(ImportSettings::$importFileName, PATHINFO_FILENAME), + ); } else { $tableName = 'TBL_NAME'; } From 7ebe99560ab1bfc9336a649098f1a4d740d766c4 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 10 Feb 2024 22:31:37 +0100 Subject: [PATCH 3/3] More Import refactoring Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 34 +----------- psalm-baseline.xml | 20 ++----- src/Plugins/Import/ImportOds.php | 28 ++-------- src/Plugins/Import/ImportXml.php | 93 ++++++++++---------------------- 4 files changed, 37 insertions(+), 138 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 375b84a442..5a1a09f874 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -11522,7 +11522,7 @@ parameters: - message: "#^Cannot cast mixed to string\\.$#" - count: 2 + count: 1 path: src/Plugins/Import/ImportOds.php - @@ -11610,19 +11610,9 @@ parameters: count: 1 path: src/Plugins/Import/ImportSql.php - - - message: "#^Call to function in_array\\(\\) requires parameter \\#3 to be set\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - - - message: "#^Cannot access property \\$database on mixed\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - message: "#^Cannot cast mixed to string\\.$#" - count: 10 + count: 6 path: src/Plugins/Import/ImportXml.php - @@ -11630,26 +11620,6 @@ parameters: count: 1 path: src/Plugins/Import/ImportXml.php - - - message: "#^Only booleans are allowed in &&, \\(SimpleXMLElement\\|null\\) given on the left side\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - - - message: "#^Only booleans are allowed in &&, int given on the right side\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - - - message: "#^Variable property access on \\(SimpleXMLElement\\|null\\)\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - - - message: "#^Variable property access on mixed\\.$#" - count: 1 - path: src/Plugins/Import/ImportXml.php - - message: "#^Method PhpMyAdmin\\\\Plugins\\\\Import\\\\ShapeFileImport\\:\\:readSHP\\(\\) never returns false so it can be removed from the return type\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index be897aca5f..01e65634aa 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6426,7 +6426,6 @@ DatabaseInterface::getInstance() DatabaseInterface::getInstance() DatabaseInterface::getInstance() - DatabaseInterface::getInstance() $nameArray === false @@ -8850,7 +8849,6 @@ - children('office', true)->body]]> @@ -8938,24 +8936,14 @@ - - - - - $attrs - $rowAttr - $tblAttr - $v2 $val2 $val3 attributes - attributes - attributes @@ -8963,17 +8951,15 @@ + + children($namespaces['pma'] ?? null)->structure_schemas]]> + attributes - children - count string[] - - $xml - diff --git a/src/Plugins/Import/ImportOds.php b/src/Plugins/Import/ImportOds.php index bea11b777f..72cf767072 100644 --- a/src/Plugins/Import/ImportOds.php +++ b/src/Plugins/Import/ImportOds.php @@ -158,27 +158,7 @@ class ImportOds extends ImportPlugin } } - [$tables, $rows] = $this->readSheets($sheets, isset($_REQUEST['ods_col_names'])); - - /** - * Bring accumulated rows into the corresponding table - */ - foreach ($tables as $table) { - foreach ($rows as $row) { - if ($table->tableName !== $row->tableName) { - continue; - } - - if ($table->columns === []) { - $table->columns = $row->columns; - } - - $table->rows = $row->rows; - } - } - - /* No longer needed */ - unset($rows); + $tables = $this->readSheets($sheets, isset($_REQUEST['ods_col_names'])); /* Obtain the best-fit MySQL types for each column */ $analyses = array_map($this->import->analyzeTable(...), $tables); @@ -331,11 +311,10 @@ class ImportOds extends ImportPlugin /** * @param mixed[]|SimpleXMLElement $sheets Sheets of the spreadsheet. * - * @return array{ImportTable[], ImportTable[]} + * @return ImportTable[] */ private function readSheets(array|SimpleXMLElement $sheets, bool $colNamesInFirstRow): array { - $tables = []; $maxCols = 0; $rows = []; @@ -367,13 +346,12 @@ class ImportOds extends ImportPlugin /* Store the table name so we know where to place the row set */ $tblAttr = $sheet->attributes('table', true); - $tables[] = new ImportTable((string) $tblAttr['name']); /* Store the current sheet in the accumulator */ $rows[] = new ImportTable((string) $tblAttr['name'], $colNames, $tempRows); $maxCols = 0; } - return [$tables, $rows]; + return $rows; } } diff --git a/src/Plugins/Import/ImportXml.php b/src/Plugins/Import/ImportXml.php index b202c4f6da..ba3704e288 100644 --- a/src/Plugins/Import/ImportXml.php +++ b/src/Plugins/Import/ImportXml.php @@ -21,6 +21,7 @@ use SimpleXMLElement; use function __; use function array_map; +use function array_values; use function in_array; use function simplexml_load_string; use function str_replace; @@ -100,7 +101,6 @@ class ImportXml extends ImportPlugin echo Message::error(__( 'The XML file specified was either malformed or incomplete. Please correct the issue and try again.', ))->getDisplay(); - unset($xml); ImportSettings::$finished = false; return []; @@ -114,8 +114,7 @@ class ImportXml extends ImportPlugin /** * Get the database name, collation and charset */ - $dbAttr = $xml->children($namespaces['pma'] ?? null) - ->{'structure_schemas'}->{'database'}; + $dbAttr = $xml->children($namespaces['pma'] ?? null)->structure_schemas->database; if ($dbAttr instanceof SimpleXMLElement) { $dbAttr = $dbAttr->attributes(); @@ -127,8 +126,7 @@ class ImportXml extends ImportPlugin * If the structure section is not present * get the database name from the data section */ - $dbAttr = $xml->children() - ->attributes(); + $dbAttr = $xml->children()->attributes(); $dbName = (string) $dbAttr['name']; $collation = null; $charset = null; @@ -141,7 +139,6 @@ class ImportXml extends ImportPlugin echo Message::error(__( 'The XML file specified was either malformed or incomplete. Please correct the issue and try again.', ))->getDisplay(); - unset($xml); ImportSettings::$finished = false; return []; @@ -191,77 +188,45 @@ class ImportXml extends ImportPlugin /** * Move down the XML tree to the actual data */ - $xml = $xml->children()->children(); + $databaseXml = $xml->database; - $dataPresent = false; $analyses = null; + /** @var ImportTable[] $tables */ $tables = []; - /** - * Only attempt to analyze/collect data if there is data present - */ - if ($xml && $xml->children()->count()) { - $dataPresent = true; - $rows = []; + $databaseName = (string) $databaseXml['name']; - /** - * Process all database content - */ - foreach ($xml as $v1) { - /** @psalm-suppress PossiblyNullReference */ - $tblAttr = $v1->attributes(); + /** @var SimpleXMLElement $tableRowXml */ + foreach ($databaseXml->table as $tableRowXml) { + $tableName = (string) $tableRowXml['name']; - $isInTables = false; - foreach ($tables as $table) { - if ($table->tableName === (string) $tblAttr['name']) { - $isInTables = true; - break; - } + $table = $tables[$tableName] ?? new ImportTable($tableName); + + $tableRow = []; + /** @var SimpleXMLElement $tableCellXml */ + foreach ($tableRowXml->column as $tableCellXml) { + /** @psalm-suppress PossiblyNullArrayAccess */ + $columnName = (string) $tableCellXml['name']; + if (! in_array($columnName, $table->columns, true)) { + $table->columns[] = $columnName; } - if (! $isInTables) { - $tables[] = new ImportTable((string) $tblAttr['name']); - } - - $tempRow = []; - $tempCells = []; - foreach ($v1 as $v2) { - /** @psalm-suppress PossiblyNullReference */ - $rowAttr = $v2->attributes(); - if (! in_array((string) $rowAttr['name'], $tempRow)) { - $tempRow[] = (string) $rowAttr['name']; - } - - $tempCells[] = (string) $v2; - } - - $rows[] = [(string) $tblAttr['name'], $tempRow, $tempCells]; + $tableRow[] = (string) $tableCellXml; } - unset($xml); + $table->rows[] = $tableRow; - /** - * Bring accumulated rows into the corresponding table - */ - foreach ($tables as $table) { - foreach ($rows as $row) { - if ($table->tableName !== $row[0]) { - continue; - } + $tables[$tableName] = $table; + } - if ($table->columns === []) { - $table->columns = $row[1]; - } + $tables = array_values($tables); - $table->rows[] = $row[2]; - } - } + foreach ($tables as $table) { + $table->tableName = $this->import->getNextAvailableTableName($databaseName, $table->tableName); + } - unset($rows); - - if (! $structPresent) { - $analyses = array_map($this->import->analyzeTable(...), $tables); - } + if (! $structPresent) { + $analyses = array_map($this->import->analyzeTable(...), $tables); } unset($xml); @@ -271,7 +236,7 @@ class ImportXml extends ImportPlugin * Set values to NULL if they were not present * to maintain Import::buildSql() call integrity */ - if ($dataPresent && $analyses === null && ! $structPresent) { + if ($tables !== [] && $analyses === null) { $create = null; }