Skip to content

Commit ac77797

Browse files
MauricioFauthMauricioFauth
authored
Merge pull request #19035 from kamil-tekiela/Refactor-import-4
Split $createDb from buildSql() method
2 parents 9308f89 + 583db25 commit ac77797

11 files changed

Lines changed: 74 additions & 68 deletions

File tree

‎phpstan-baseline.neon‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8600,6 +8600,11 @@ parameters:
86008600
count: 1
86018601
path: src/Import/Import.php
86028602

8603+
-
8604+
message: "#^Method PhpMyAdmin\\\\Import\\\\Import\\:\\:createDatabase\\(\\) should return array\\<string\\> but returns mixed\\.$#"
8605+
count: 1
8606+
path: src/Import/Import.php
8607+
86038608
-
86048609
message: "#^Method PhpMyAdmin\\\\Import\\\\Import\\:\\:detectSize\\(\\) should return int\\|string but returns mixed\\.$#"
86058610
count: 3
@@ -11395,11 +11400,6 @@ parameters:
1139511400
count: 1
1139611401
path: src/Plugins/Import/ImportCsv.php
1139711402

11398-
-
11399-
message: "#^Parameter \\#1 \\$dbName of method PhpMyAdmin\\\\Import\\\\Import\\:\\:buildSql\\(\\) expects string, mixed given\\.$#"
11400-
count: 1
11401-
path: src/Plugins/Import/ImportCsv.php
11402-
1140311403
-
1140411404
message: "#^Parameter \\#1 \\$serverMessage of static method PhpMyAdmin\\\\Html\\\\Generator\\:\\:mysqlDie\\(\\) expects string, mixed given\\.$#"
1140511405
count: 1
@@ -11440,6 +11440,11 @@ parameters:
1144011440
count: 1
1144111441
path: src/Plugins/Import/ImportCsv.php
1144211442

11443+
-
11444+
message: "#^Parameter \\#4 \\$sqlData of method PhpMyAdmin\\\\Import\\\\Import\\:\\:createDatabase\\(\\) expects array\\<string\\>, mixed given\\.$#"
11445+
count: 1
11446+
path: src/Plugins/Import/ImportCsv.php
11447+
1144311448
-
1144411449
message: "#^Cannot cast mixed to string\\.$#"
1144511450
count: 5

‎psalm-baseline.xml‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6428,8 +6428,6 @@
64286428
<MixedAssignment>
64296429
<code><![CDATA[$active]]></code>
64306430
<code><![CDATA[$cellValue]]></code>
6431-
<code><![CDATA[$charset]]></code>
6432-
<code><![CDATA[$collation]]></code>
64336431
<code><![CDATA[$importPlugin]]></code>
64346432
</MixedAssignment>
64356433
<MixedInferredReturnType>
@@ -6440,10 +6438,6 @@
64406438
<code><![CDATA[getProperties]]></code>
64416439
</MixedMethodCall>
64426440
<MixedOperand>
6443-
<code><![CDATA[$charset]]></code>
6444-
<code><![CDATA[$charset]]></code>
6445-
<code><![CDATA[$collation]]></code>
6446-
<code><![CDATA[$collation]]></code>
64476441
<code><![CDATA[$importPlugin->getProperties()->getExtension()]]></code>
64486442
<code><![CDATA[$size[self::D]]]></code>
64496443
<code><![CDATA[ImportSettings::$maximumTime]]></code>
@@ -8702,13 +8696,9 @@
87028696
<code><![CDATA[$GLOBALS['errorUrl']]]></code>
87038697
<code><![CDATA[$fields[]]]></code>
87048698
</MixedAssignment>
8705-
<PossiblyInvalidArgument>
8706-
<code><![CDATA[$dbName]]></code>
8707-
</PossiblyInvalidArgument>
87088699
<PossiblyInvalidCast>
87098700
<code><![CDATA[$_REQUEST['csv_new_db_name']]]></code>
87108701
<code><![CDATA[$_REQUEST['csv_new_tbl_name']]]></code>
8711-
<code><![CDATA[$dbName]]></code>
87128702
</PossiblyInvalidCast>
87138703
<PossiblyInvalidOperand>
87148704
<code><![CDATA[$maxLines]]></code>

‎src/Import/Import.php‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -888,32 +888,18 @@ public function analyzeTable(ImportTable $table): array
888888
* @param ImportTable[] $tables
889889
* @param array{0:ColumnType[], 1:(int|string)[], 2?:true[]}[]|null $analyses Analyses of the tables
890890
* @param string[]|null $additionalSql Additional SQL to be executed
891-
* @param mixed[]|null $options Associative array of options
892891
* @param string[] $sqlData List of SQL to be executed
893892
*/
894893
public function buildSql(
895894
string $dbName,
896895
array $tables,
897896
array|null $analyses = null,
898897
array|null $additionalSql = null,
899-
bool $createDb = true,
900-
array|null $options = null,
901898
array &$sqlData = [],
902899
): void {
903900
/* Needed to quell the beast that is Message */
904901
ImportSettings::$importNotice = '';
905902

906-
/* Take care of the options */
907-
$collation = $options['db_collation'] ?? 'utf8_general_ci';
908-
$charset = $options['db_charset'] ?? 'utf8';
909-
910-
if ($createDb) {
911-
$sql = 'CREATE DATABASE IF NOT EXISTS ' . Util::backquote($dbName)
912-
. ' DEFAULT CHARACTER SET ' . $charset . ' COLLATE ' . $collation
913-
. ';';
914-
$this->runQuery($sql, $sqlData);
915-
}
916-
917903
/* Run the $additional_sql statements supplied by the caller plug-in */
918904
if ($additionalSql != null) {
919905
/* Clean the SQL first */
@@ -977,8 +963,7 @@ public function buildSql(
977963
$tempSQLStr .= ', ';
978964
}
979965

980-
$tempSQLStr .= ') DEFAULT CHARACTER SET ' . $charset
981-
. ' COLLATE ' . $collation . ';';
966+
$tempSQLStr .= ')';
982967

983968
/**
984969
* Each SQL statement is executed immediately
@@ -994,7 +979,6 @@ public function buildSql(
994979
*
995980
* Only one insert query is formed for each table
996981
*/
997-
$tempSQLStr = '';
998982
$colCount = 0;
999983
$dbi = DatabaseInterface::getInstance();
1000984
foreach ($tables as $i => $table) {
@@ -1066,8 +1050,6 @@ public function buildSql(
10661050
unset($table->rows[$j]);
10671051
}
10681052

1069-
$tempSQLStr .= ';';
1070-
10711053
/**
10721054
* Each SQL statement is executed immediately
10731055
* after it is formed so that we don't have
@@ -1076,9 +1058,6 @@ public function buildSql(
10761058
$this->runQuery($tempSQLStr, $sqlData);
10771059
}
10781060

1079-
/* No longer needed */
1080-
unset($tempSQLStr);
1081-
10821061
/**
10831062
* A work in progress
10841063
*/
@@ -1404,4 +1383,18 @@ public function getNextAvailableTableName(string $databaseName, string $proposed
14041383

14051384
return $importFileName;
14061385
}
1386+
1387+
/**
1388+
* @param string[] $sqlData List of SQL statements to be executed
1389+
*
1390+
* @return string[]
1391+
*/
1392+
public function createDatabase(string $dbName, string $charset, string $collation, array $sqlData): array
1393+
{
1394+
$sql = 'CREATE DATABASE IF NOT EXISTS ' . Util::backquote($dbName)
1395+
. ' DEFAULT CHARACTER SET ' . $charset . ' COLLATE ' . $collation;
1396+
$this->runQuery($sql, $sqlData);
1397+
1398+
return $sqlData;
1399+
}
14071400
}

‎src/Plugins/Import/ImportCsv.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PhpMyAdmin\Properties\Options\Items\TextPropertyItem;
2323
use PhpMyAdmin\Properties\Plugins\ImportPluginProperties;
2424
use PhpMyAdmin\Util;
25+
use Webmozart\Assert\Assert;
2526

2627
use function __;
2728
use function array_pad;
@@ -590,6 +591,7 @@ public function doImport(File|null $importHandle = null): array
590591
*/
591592
if (isset($_REQUEST['csv_new_db_name']) && (string) $_REQUEST['csv_new_db_name'] !== '') {
592593
$newDb = $_REQUEST['csv_new_db_name'];
594+
Assert::string($newDb);
593595
} else {
594596
$result = $dbi->fetchResult('SHOW DATABASES');
595597

@@ -599,7 +601,11 @@ public function doImport(File|null $importHandle = null): array
599601
$dbName = Current::$database !== '' ? Current::$database : $newDb;
600602
$createDb = Current::$database === '';
601603

602-
$this->import->buildSql($dbName, [$table], [$analysis], createDb:$createDb, sqlData:$sqlStatements);
604+
if ($createDb) {
605+
$sqlStatements = $this->import->createDatabase($dbName, 'utf8', 'utf8_general_ci', $sqlStatements);
606+
}
607+
608+
$this->import->buildSql($dbName, [$table], [$analysis], sqlData: $sqlStatements);
603609
}
604610

605611
// Commit any possible data in buffers

‎src/Plugins/Import/ImportMediawiki.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,16 @@ private function importDataOneTable(ImportTable $table, array &$sqlStatements):
305305
// Obtain the best-fit MySQL types for each column
306306
$analysis = $this->import->analyzeTable($table);
307307

308+
$dbName = Current::$database !== '' ? Current::$database : 'mediawiki_DB';
309+
310+
if (Current::$database === '') {
311+
$sqlStatements = $this->import->createDatabase($dbName, 'utf8', 'utf8_general_ci', $sqlStatements);
312+
}
313+
308314
$this->import->buildSql(
309-
Current::$database !== '' ? Current::$database : 'mediawiki_DB',
315+
$dbName,
310316
[$table],
311317
[$analysis],
312-
createDb: Current::$database === '',
313318
sqlData: $sqlStatements,
314319
);
315320
}

‎src/Plugins/Import/ImportOds.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,12 @@ public function doImport(File|null $importHandle = null): array
167167
$dbName = Current::$database !== '' ? Current::$database : 'ODS_DB';
168168
$createDb = Current::$database === '';
169169

170+
if ($createDb) {
171+
$sqlStatements = $this->import->createDatabase($dbName, 'utf8', 'utf8_general_ci', $sqlStatements);
172+
}
173+
170174
/* Created and execute necessary SQL statements from data */
171-
$this->import->buildSql($dbName, $tables, $analyses, createDb:$createDb, sqlData:$sqlStatements);
175+
$this->import->buildSql($dbName, $tables, $analyses, sqlData: $sqlStatements);
172176

173177
/* Commit any possible data in buffers */
174178
$this->import->runQuery('', $sqlStatements);

‎src/Plugins/Import/ImportShp.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,11 @@ public function doImport(File|null $importHandle = null): array
288288

289289
// Created and execute necessary SQL statements from data
290290
$sqlStatements = [];
291-
$this->import->buildSql($dbName, [$table], [$analysis], createDb:$createDb, sqlData:$sqlStatements);
291+
if ($createDb) {
292+
$sqlStatements = $this->import->createDatabase($dbName, 'utf8', 'utf8_general_ci', []);
293+
}
294+
295+
$this->import->buildSql($dbName, [$table], [$analysis], sqlData: $sqlStatements);
292296

293297
ImportSettings::$finished = true;
294298
$GLOBALS['error'] = false;

‎src/Plugins/Import/ImportXml.php‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -240,21 +240,23 @@ public function doImport(File|null $importHandle = null): array
240240
$create = null;
241241
}
242242

243+
/* Created and execute necessary SQL statements from data */
244+
$sqlStatements = [];
245+
243246
/* Set database name to the currently selected one, if applicable */
244247
if (Current::$database !== '') {
245248
/* Override the database name in the XML file, if one is selected */
246249
$dbName = Current::$database;
247-
$options = null;
248250
} else {
249-
/* Set database collation/charset */
250-
$options = ['db_collation' => $collation, 'db_charset' => $charset];
251+
$sqlStatements = $this->import->createDatabase(
252+
$dbName,
253+
$charset ?? 'utf8',
254+
$collation ?? 'utf8_general_ci',
255+
[],
256+
);
251257
}
252258

253-
$createDb = Current::$database === '';
254-
255-
/* Created and execute necessary SQL statements from data */
256-
$sqlStatements = [];
257-
$this->import->buildSql($dbName, $tables, $analyses, $create, $createDb, $options, $sqlStatements);
259+
$this->import->buildSql($dbName, $tables, $analyses, $create, $sqlStatements);
258260

259261
/* Commit any possible data in buffers */
260262
$this->import->runQuery('', $sqlStatements);

‎tests/unit/Plugins/Import/ImportCsvTest.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,10 @@ public function testDoImportNormal(): void
261261
$this->object->doImport();
262262

263263
self::assertSame(
264-
'CREATE DATABASE IF NOT EXISTS `CSV_DB 1` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;'
265-
. 'CREATE TABLE IF NOT EXISTS `CSV_DB 1`.`db_test` (`COL 1` varchar(5), `COL 2` varchar(5))'
266-
. ' DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;INSERT INTO `CSV_DB 1`.`db_test`'
267-
. ' (`COL 1`, `COL 2`) VALUES (\'Row 1\', \'Row 2\'),' . "\n" . ' (\'123\', \'456\');;',
264+
'CREATE DATABASE IF NOT EXISTS `CSV_DB 1` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;'
265+
. 'CREATE TABLE IF NOT EXISTS `CSV_DB 1`.`db_test` (`COL 1` varchar(5), `COL 2` varchar(5));'
266+
. 'INSERT INTO `CSV_DB 1`.`db_test`'
267+
. ' (`COL 1`, `COL 2`) VALUES (\'Row 1\', \'Row 2\'),' . "\n" . ' (\'123\', \'456\');',
268268
$GLOBALS['sql_query'],
269269
);
270270

@@ -306,10 +306,10 @@ public function testDoImportSkipHeaders(): void
306306
$this->object->doImport();
307307

308308
self::assertSame(
309-
'CREATE DATABASE IF NOT EXISTS `CSV_DB 1` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;'
310-
. 'CREATE TABLE IF NOT EXISTS `CSV_DB 1`.`db_test` (`Row 1` int(3), `Row 2` int(3))'
311-
. ' DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;INSERT INTO `CSV_DB 1`.`db_test`'
312-
. ' (`Row 1`, `Row 2`) VALUES (123, 456);;',
309+
'CREATE DATABASE IF NOT EXISTS `CSV_DB 1` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;'
310+
. 'CREATE TABLE IF NOT EXISTS `CSV_DB 1`.`db_test` (`Row 1` int(3), `Row 2` int(3));'
311+
. 'INSERT INTO `CSV_DB 1`.`db_test`'
312+
. ' (`Row 1`, `Row 2`) VALUES (123, 456);',
313313
$GLOBALS['sql_query'],
314314
);
315315

‎tests/unit/Plugins/Import/ImportOdsTest.php‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -185,28 +185,26 @@ public function testDoImportDataset2(bool $odsEmptyRowsMode): void
185185
// The process could probably detect that all the values for columns V to BL are empty
186186
// That would make the empty columns not needed and would create a cleaner structure
187187

188-
$endOfSql = ');;';
188+
$endOfSql = ');';
189189

190190
if (! $odsEmptyRowsMode) {
191191
$fullCols = 'NULL' . str_repeat(', NULL', 18);// 19 empty cells
192-
$endOfSql = '),' . "\n" . ' (' . $fullCols . '),' . "\n" . ' (' . $fullCols . ');;';
192+
$endOfSql = '),' . "\n" . ' (' . $fullCols . '),' . "\n" . ' (' . $fullCols . ');';
193193
}
194194

195195
//Test function called
196196
$this->object->doImport($importHandle);
197197

198198
self::assertSame(
199-
'CREATE DATABASE IF NOT EXISTS `ODS_DB` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;'
199+
'CREATE DATABASE IF NOT EXISTS `ODS_DB` DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;'
200200
. 'CREATE TABLE IF NOT EXISTS `ODS_DB`.`Shop` ('
201201
. '`Artikelnummer` varchar(7), `Name` varchar(41), `keywords` varchar(15), `EK_Preis` varchar(21),'
202202
. ' `Preis` varchar(23), `Details` varchar(10), `addInfo` varchar(22), `Einheit` varchar(3),'
203203
. ' `Wirkstoff` varchar(10), `verkuerztHaltbar` varchar(21), `kuehlkette` varchar(7),'
204204
. ' `Gebinde` varchar(71), `Verbrauchsnachweis` varchar(7), `Genehmigungspflichtig` varchar(7),'
205205
. ' `Gefahrstoff` varchar(11), `GefahrArbeitsbereich` varchar(14), `Verwendungszweck` varchar(10),'
206-
. ' `Verbrauch` varchar(10), `showLagerbestand` varchar(7)) '
207-
. 'DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;'
208-
. 'CREATE TABLE IF NOT EXISTS `ODS_DB`.`Feuille 1` (`value` varchar(19)) '
209-
. 'DEFAULT CHARACTER SET utf8 COLLATE utf8_general_ci;;'
206+
. ' `Verbrauch` varchar(10), `showLagerbestand` varchar(7));'
207+
. 'CREATE TABLE IF NOT EXISTS `ODS_DB`.`Feuille 1` (`value` varchar(19));'
210208
. 'INSERT INTO `ODS_DB`.`Shop` ('
211209
. '`Artikelnummer`, `Name`, `keywords`, `EK_Preis`, `Preis`, `Details`, `addInfo`, `Einheit`,'
212210
. ' `Wirkstoff`, `verkuerztHaltbar`, `kuehlkette`, `Gebinde`, `Verbrauchsnachweis`,'
@@ -247,7 +245,7 @@ public function testDoImportDataset2(bool $odsEmptyRowsMode): void
247245
. ' (\'true\'),' . "\n"
248246
. ' (\'12\')'
249247
. ($odsEmptyRowsMode ? '' : ',' . "\n" . ' (NULL)')
250-
. ($odsEmptyRowsMode ? ';;' : ',' . "\n" . ' (NULL);;'),
248+
. ($odsEmptyRowsMode ? ';' : ',' . "\n" . ' (NULL);'),
251249
$GLOBALS['sql_query'],
252250
);
253251

0 commit comments

Comments
 (0)