Skip to content

Commit

Permalink
Migrate database pending bigint conversions check to new API
Browse files Browse the repository at this point in the history
Signed-off-by: Côme Chilliet <[email protected]>
  • Loading branch information
come-nc committed Nov 16, 2023
1 parent 2efb953 commit 97056c8
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 112 deletions.
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingIndices.php',
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => $baseDir . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php',
'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => $baseDir . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php',
'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => $baseDir . '/../lib/SetupChecks/DefaultPhoneRegionSet.php',
'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => $baseDir . '/../lib/SetupChecks/EmailTestSuccessful.php',
'OCA\\Settings\\SetupChecks\\FileLocking' => $baseDir . '/../lib/SetupChecks/FileLocking.php',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class ComposerStaticInitSettings
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingColumns' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingColumns.php',
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingIndices' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingIndices.php',
'OCA\\Settings\\SetupChecks\\DatabaseHasMissingPrimaryKeys' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabaseHasMissingPrimaryKeys.php',
'OCA\\Settings\\SetupChecks\\DatabasePendingBigIntConversions' => __DIR__ . '/..' . '/../lib/SetupChecks/DatabasePendingBigIntConversions.php',
'OCA\\Settings\\SetupChecks\\DefaultPhoneRegionSet' => __DIR__ . '/..' . '/../lib/SetupChecks/DefaultPhoneRegionSet.php',
'OCA\\Settings\\SetupChecks\\EmailTestSuccessful' => __DIR__ . '/..' . '/../lib/SetupChecks/EmailTestSuccessful.php',
'OCA\\Settings\\SetupChecks\\FileLocking' => __DIR__ . '/..' . '/../lib/SetupChecks/FileLocking.php',
Expand Down
2 changes: 2 additions & 0 deletions apps/settings/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
use OCA\Settings\SetupChecks\DatabaseHasMissingColumns;
use OCA\Settings\SetupChecks\DatabaseHasMissingIndices;
use OCA\Settings\SetupChecks\DatabaseHasMissingPrimaryKeys;
use OCA\Settings\SetupChecks\DatabasePendingBigIntConversions;
use OCA\Settings\SetupChecks\DefaultPhoneRegionSet;
use OCA\Settings\SetupChecks\EmailTestSuccessful;
use OCA\Settings\SetupChecks\FileLocking;
Expand Down Expand Up @@ -166,6 +167,7 @@ public function register(IRegistrationContext $context): void {
$context->registerSetupCheck(DatabaseHasMissingColumns::class);
$context->registerSetupCheck(DatabaseHasMissingIndices::class);
$context->registerSetupCheck(DatabaseHasMissingPrimaryKeys::class);
$context->registerSetupCheck(DatabasePendingBigIntConversions::class);
$context->registerSetupCheck(DefaultPhoneRegionSet::class);
$context->registerSetupCheck(EmailTestSuccessful::class);
$context->registerSetupCheck(FileLocking::class);
Expand Down
56 changes: 0 additions & 56 deletions apps/settings/lib/Controller/CheckSetupController.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,17 @@
use GuzzleHttp\Exception\ClientException;
use OC;
use OC\AppFramework\Http;
use OC\DB\Connection;
use OC\DB\SchemaWrapper;
use OC\IntegrityCheck\Checker;
use OCP\App\IAppManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
use OCP\AppFramework\Http\DataDisplayResponse;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\DB\Types;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IDateTimeFormatter;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IServerContainer;
Expand All @@ -91,16 +87,12 @@ class CheckSetupController extends Controller {
private $logger;
/** @var IEventDispatcher */
private $dispatcher;
/** @var Connection */
private $db;
/** @var ILockingProvider */
private $lockingProvider;
/** @var IDateTimeFormatter */
private $dateTimeFormatter;
/** @var IniGetWrapper */
private $iniGetWrapper;
/** @var IDBConnection */
private $connection;
/** @var ITempManager */
private $tempManager;
/** @var IManager */
Expand All @@ -120,11 +112,9 @@ public function __construct($AppName,
Checker $checker,
LoggerInterface $logger,
IEventDispatcher $dispatcher,
Connection $db,
ILockingProvider $lockingProvider,
IDateTimeFormatter $dateTimeFormatter,
IniGetWrapper $iniGetWrapper,
IDBConnection $connection,
ITempManager $tempManager,
IManager $manager,
IAppManager $appManager,
Expand All @@ -139,11 +129,9 @@ public function __construct($AppName,
$this->checker = $checker;
$this->logger = $logger;
$this->dispatcher = $dispatcher;
$this->db = $db;
$this->lockingProvider = $lockingProvider;
$this->dateTimeFormatter = $dateTimeFormatter;
$this->iniGetWrapper = $iniGetWrapper;
$this->connection = $connection;
$this->tempManager = $tempManager;
$this->manager = $manager;
$this->appManager = $appManager;
Expand Down Expand Up @@ -528,49 +516,6 @@ protected function isMysqlUsedWithoutUTF8MB4(): bool {
return ($this->config->getSystemValue('dbtype', 'sqlite') === 'mysql') && ($this->config->getSystemValue('mysql.utf8mb4', false) === false);
}

protected function hasBigIntConversionPendingColumns(): array {
// copy of ConvertFilecacheBigInt::getColumnsByTable()
$tables = [
'activity' => ['activity_id', 'object_id'],
'activity_mq' => ['mail_id'],
'authtoken' => ['id'],
'bruteforce_attempts' => ['id'],
'federated_reshares' => ['share_id'],
'filecache' => ['fileid', 'storage', 'parent', 'mimetype', 'mimepart', 'mtime', 'storage_mtime'],
'filecache_extended' => ['fileid'],
'files_trash' => ['auto_id'],
'file_locks' => ['id'],
'file_metadata' => ['id'],
'jobs' => ['id'],
'mimetypes' => ['id'],
'mounts' => ['id', 'storage_id', 'root_id', 'mount_id'],
'share_external' => ['id', 'parent'],
'storages' => ['numeric_id'],
];

$schema = new SchemaWrapper($this->db);
$isSqlite = $this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_SQLITE;
$pendingColumns = [];

foreach ($tables as $tableName => $columns) {
if (!$schema->hasTable($tableName)) {
continue;
}

$table = $schema->getTable($tableName);
foreach ($columns as $columnName) {
$column = $table->getColumn($columnName);
$isAutoIncrement = $column->getAutoincrement();
$isAutoIncrementOnSqlite = $isSqlite && $isAutoIncrement;
if ($column->getType()->getName() !== Types::BIGINT && !$isAutoIncrementOnSqlite) {
$pendingColumns[] = $tableName . '.' . $columnName;
}
}
}

return $pendingColumns;
}

protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool {
$objectStore = $this->config->getSystemValue('objectstore', null);
$objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null);
Expand Down Expand Up @@ -634,7 +579,6 @@ public function check() {
'appDirsWithDifferentOwner' => $this->getAppDirsWithDifferentOwner(),
'isImagickEnabled' => $this->isImagickEnabled(),
'areWebauthnExtensionsEnabled' => $this->areWebauthnExtensionsEnabled(),
'pendingBigIntConversionColumns' => $this->hasBigIntConversionPendingColumns(),
'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(),
'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(),
'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'),
Expand Down
99 changes: 99 additions & 0 deletions apps/settings/lib/SetupChecks/DatabasePendingBigIntConversions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2023 Côme Chilliet <[email protected]>
*
* @author Côme Chilliet <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\Settings\SetupChecks;

use OC\Core\Command\Db\ConvertFilecacheBigInt;
use OC\DB\Connection;
use OC\DB\SchemaWrapper;
use OCP\DB\Types;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;

class DatabasePendingBigIntConversions implements ISetupCheck {
public function __construct(
private IL10N $l10n,
private IURLGenerator $urlGenerator,
private Connection $db,
private IEventDispatcher $dispatcher,
private IDBConnection $connection,
) {
}

public function getCategory(): string {
return 'database';
}

public function getName(): string {
return $this->l10n->t('Database pending bigint migrations');
}

protected function getBigIntConversionPendingColumns(): array {
$tables = ConvertFilecacheBigInt::getColumnsByTable();

$schema = new SchemaWrapper($this->db);
$isSqlite = $this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_SQLITE;
$pendingColumns = [];

foreach ($tables as $tableName => $columns) {
if (!$schema->hasTable($tableName)) {
continue;
}

$table = $schema->getTable($tableName);
foreach ($columns as $columnName) {
$column = $table->getColumn($columnName);
$isAutoIncrement = $column->getAutoincrement();
$isAutoIncrementOnSqlite = $isSqlite && $isAutoIncrement;
if ($column->getType()->getName() !== Types::BIGINT && !$isAutoIncrementOnSqlite) {

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method Doctrine\DBAL\Types\Type::getName has been marked as deprecated
$pendingColumns[] = $tableName . '.' . $columnName;
}
}
}

return $pendingColumns;
}

public function run(): SetupResult {
$pendingColumns = $this->getBigIntConversionPendingColumns();
if (empty($pendingColumns)) {
return SetupResult::success('None');
} else {
$list = '';
foreach ($pendingColumns as $pendingColumn) {
$list .= "\n$pendingColumn";
}
$list .= "\n";
return SetupResult::info(
$this->l10n->t('Some columns in the database are missing a conversion to big int. Due to the fact that changing column types on big tables could take some time they were not changed automatically. By running "occ db:convert-filecache-bigint" those pending changes could be applied manually. This operation needs to be made while the instance is offline.').$list,
$this->urlGenerator->linkToDocs('admin-bigint-conversion')
);
}
}
}
23 changes: 0 additions & 23 deletions apps/settings/tests/Controller/CheckSetupControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
namespace OCA\Settings\Tests\Controller;

use bantu\IniGetWrapper\IniGetWrapper;
use Doctrine\DBAL\Platforms\SqlitePlatform;
use OC;
use OC\DB\Connection;
use OC\IntegrityCheck\Checker;
use OCA\Settings\Controller\CheckSetupController;
use OCP\App\IAppManager;
Expand All @@ -49,7 +47,6 @@
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IDateTimeFormatter;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IServerContainer;
Expand Down Expand Up @@ -88,16 +85,12 @@ class CheckSetupControllerTest extends TestCase {
private $checker;
/** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */
private $dispatcher;
/** @var Connection|\PHPUnit\Framework\MockObject\MockObject */
private $db;
/** @var ILockingProvider|\PHPUnit\Framework\MockObject\MockObject */
private $lockingProvider;
/** @var IDateTimeFormatter|\PHPUnit\Framework\MockObject\MockObject */
private $dateTimeFormatter;
/** @var IniGetWrapper|\PHPUnit\Framework\MockObject\MockObject */
private $iniGetWrapper;
/** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */
private $connection;
/** @var ITempManager|\PHPUnit\Framework\MockObject\MockObject */
private $tempManager;
/** @var IManager|\PHPUnit\Framework\MockObject\MockObject */
Expand Down Expand Up @@ -138,13 +131,9 @@ protected function setUp(): void {
$this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker')
->disableOriginalConstructor()->getMock();
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$this->db = $this->getMockBuilder(Connection::class)
->disableOriginalConstructor()->getMock();
$this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock();
$this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock();
$this->iniGetWrapper = $this->getMockBuilder(IniGetWrapper::class)->getMock();
$this->connection = $this->getMockBuilder(IDBConnection::class)
->disableOriginalConstructor()->getMock();
$this->tempManager = $this->getMockBuilder(ITempManager::class)->getMock();
$this->notificationManager = $this->getMockBuilder(IManager::class)->getMock();
$this->appManager = $this->createMock(IAppManager::class);
Expand All @@ -161,11 +150,9 @@ protected function setUp(): void {
$this->checker,
$this->logger,
$this->dispatcher,
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->connection,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down Expand Up @@ -307,9 +294,6 @@ public function testCheck() {
}
return '';
});
$sqlitePlatform = $this->getMockBuilder(SqlitePlatform::class)->getMock();
$this->connection->method('getDatabasePlatform')
->willReturn($sqlitePlatform);

$expected = new DataResponse(
[
Expand All @@ -332,7 +316,6 @@ public function testCheck() {
'appDirsWithDifferentOwner' => [],
'isImagickEnabled' => false,
'areWebauthnExtensionsEnabled' => false,
'pendingBigIntConversionColumns' => [],
'isMysqlUsedWithoutUTF8MB4' => false,
'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true,
'reverseProxyGeneratedURL' => 'https://server/index.php',
Expand All @@ -357,11 +340,9 @@ public function testGetCurlVersion() {
$this->checker,
$this->logger,
$this->dispatcher,
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->connection,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down Expand Up @@ -1083,11 +1064,9 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool
$this->checker,
$this->logger,
$this->dispatcher,
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->connection,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down Expand Up @@ -1136,11 +1115,9 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m
$this->checker,
$this->logger,
$this->dispatcher,
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->iniGetWrapper,
$this->connection,
$this->tempManager,
$this->notificationManager,
$this->appManager,
Expand Down
Loading

0 comments on commit 97056c8

Please sign in to comment.