Skip to content

Commit

Permalink
Improve database collation detection (matomo-org#22594)
Browse files Browse the repository at this point in the history
Co-authored-by: caddoo <[email protected]>
  • Loading branch information
mneudert and caddoo authored Sep 25, 2024
1 parent ec8cf14 commit 6b9d136
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
25 changes: 12 additions & 13 deletions core/Db/Schema/Mysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -672,26 +672,19 @@ public function supportsComplexColumnUpdates(): bool
/**
* Returns the default collation for a charset.
*
* Will return an empty string for an unknown charset
* (can happen for alias charsets like "utf8").
*
* @param string $charset
*
* @return string
* @throws Exception
*/
public function getDefaultCollationForCharset(string $charset): string
{
$result = $this->getDb()->fetchRow(
'SHOW COLLATION WHERE `Default` = "Yes" AND `Charset` = ?',
[$charset]
);
$result = $this->getDb()->fetchRow('SHOW CHARACTER SET WHERE `Charset` = ?', [$charset]);

if (!isset($result['Collation'])) {
throw new Exception(sprintf(
'Failed to detect default collation for character set "%s"',
$charset
));
}

return $result['Collation'];
return $result['Default collation'] ?? '';
}

public function getDefaultPort(): int
Expand Down Expand Up @@ -797,7 +790,13 @@ protected function getDatabaseCreateOptions(): string
$charset = DbHelper::getDefaultCharset();
$collation = $this->getDefaultCollationForCharset($charset);

return "DEFAULT CHARACTER SET $charset COLLATE $collation";
$options = "DEFAULT CHARACTER SET $charset";

if ('' !== $collation) {
$options .= " COLLATE $collation";
}

return $options;
}

protected function getTableEngine()
Expand Down
18 changes: 10 additions & 8 deletions plugins/Diagnostics/Diagnostic/DatabaseAbilitiesCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,17 @@ protected function checkCollation(): DiagnosticResultItem
$collationConnection = Db::get()->fetchOne('SELECT @@collation_connection');
$collationCharset = DbHelper::getDefaultCollationForCharset($dbSettings->getUsedCharset());

return new DiagnosticResultItem(
DiagnosticResult::STATUS_WARNING,
sprintf(
'Connection collation<br/><br/>%s<br/><br/>%s<br/>%s<br/>',
$this->translator->translate('Diagnostics_DatabaseCollationNotConfigured'),
$this->translator->translate('Diagnostics_DatabaseCollationConnection', [$collationConnection]),
$this->translator->translate('Diagnostics_DatabaseCollationCharset', [$collationCharset])
)
$message = sprintf(
'Connection collation<br/><br/>%s<br/><br/>%s<br/>',
$this->translator->translate('Diagnostics_DatabaseCollationNotConfigured'),
$this->translator->translate('Diagnostics_DatabaseCollationConnection', [$collationConnection])
);

if ('' !== $collationCharset) {
$message .= $this->translator->translate('Diagnostics_DatabaseCollationCharset', [$collationCharset]) . '<br/>';
}

return new DiagnosticResultItem(DiagnosticResult::STATUS_WARNING, $message);
}

protected function checkLoadDataInfile()
Expand Down
7 changes: 2 additions & 5 deletions tests/PHPUnit/Integration/DbHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,9 @@ public function testGetDefaultCollationForCharset(): void
self::assertStringStartsWith($expectedPrefix, $collation);
}

public function testGetDefaultCollationForCharsetThrowsForInvalidCharset(): void
public function testGetDefaultCollationForCharsetWithoutDefault(): void
{
self::expectException(\Exception::class);
self::expectExceptionMessage('Failed to detect default collation for character set "invalid"');

DbHelper::getDefaultCollationForCharset('invalid');
self::assertSame('', DbHelper::getDefaultCollationForCharset('invalid'));
}

private function assertDbExists($dbName)
Expand Down

0 comments on commit 6b9d136

Please sign in to comment.