From 8362025643bdbe6c93ce4b3c33e61c2c370039fc Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Wed, 24 Jan 2024 20:00:18 -0100 Subject: [PATCH 1/3] better indexes Signed-off-by: Maxence Lange --- .../Version28000Date20231126110901.php | 68 +++++++++++++++++ .../Version29000Date20231126110901.php | 39 ++++++---- .../Version29000Date20240124132201.php | 75 +++++++++++++++++++ .../Version29000Date20240124132202.php | 53 +++++++++++++ lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + lib/private/AppConfig.php | 43 ++++++----- version.php | 2 +- 8 files changed, 249 insertions(+), 37 deletions(-) create mode 100644 core/Migrations/Version28000Date20231126110901.php create mode 100644 core/Migrations/Version29000Date20240124132201.php create mode 100644 core/Migrations/Version29000Date20240124132202.php diff --git a/core/Migrations/Version28000Date20231126110901.php b/core/Migrations/Version28000Date20231126110901.php new file mode 100644 index 00000000000..ec1e80d24ba --- /dev/null +++ b/core/Migrations/Version28000Date20231126110901.php @@ -0,0 +1,68 @@ + + * + * @author Maxence Lange + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Create new fields for type and lazy loading in appconfig for the new IAppConfig API. + */ +class Version28000Date20231126110901 extends SimpleMigrationStep { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + return null; + + /** + * this migration was needed during Nextcloud 28 to prep the migration to 29 and a + * new IAppConfig as its API require 'lazy' and 'type' database field. + * + * some changes in the migration process and the expected result have made its execution + * useless, therefore ignored. + * + * @see Version29000Date20240124132201 + * @see Version29000Date20240124132202 + */ + // /** @var ISchemaWrapper $schema */ + // $schema = $schemaClosure(); + // + // if (!$schema->hasTable('appconfig')) { + // return null; + // } + // + // $table = $schema->getTable('appconfig'); + // if ($table->hasColumn('lazy')) { + // return null; + // } + // + // // type=2 means value is typed as MIXED + // $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]); + // $table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]); + // + // return $schema; + } +} diff --git a/core/Migrations/Version29000Date20231126110901.php b/core/Migrations/Version29000Date20231126110901.php index 3867074e013..cbbac6b7777 100644 --- a/core/Migrations/Version29000Date20231126110901.php +++ b/core/Migrations/Version29000Date20231126110901.php @@ -39,27 +39,34 @@ class Version29000Date20231126110901 extends SimpleMigrationStep { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); - if (!$schema->hasTable('appconfig')) { - return null; - } - $table = $schema->getTable('appconfig'); if ($table->hasColumn('lazy')) { return null; } - // type=2 means value is typed as MIXED - $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]); - $table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]); - - if ($table->hasIndex('appconfig_config_key_index')) { - $table->dropIndex('appconfig_config_key_index'); - } - - $table->addIndex(['lazy'], 'ac_lazy_i'); - $table->addIndex(['appid', 'lazy'], 'ac_app_lazy_i'); - $table->addIndex(['appid', 'lazy', 'configkey'], 'ac_app_lazy_key_i'); + /** + * This code is now useless, after a discussion about boolean on oracle; + * it has been decided to use another type for the lazy field + * + * a better migration process is available there: + * + * @see Version29000Date20240124132201 for the revert of current migration + * @see Version29000Date20240124132202 for the new migration process + */ + return null; - return $schema; + // // type=2 means value is typed as MIXED + // $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2]); + // $table->addColumn('lazy', Types::BOOLEAN, ['notnull' => false, 'default' => false]); + // + // if ($table->hasIndex('appconfig_config_key_index')) { + // $table->dropIndex('appconfig_config_key_index'); + // } + // + // $table->addIndex(['lazy'], 'ac_lazy_i'); + // $table->addIndex(['appid', 'lazy'], 'ac_app_lazy_i'); + // $table->addIndex(['appid', 'lazy', 'configkey'], 'ac_app_lazy_key_i'); + // + // return $schema; } } diff --git a/core/Migrations/Version29000Date20240124132201.php b/core/Migrations/Version29000Date20240124132201.php new file mode 100644 index 00000000000..86c21973c48 --- /dev/null +++ b/core/Migrations/Version29000Date20240124132201.php @@ -0,0 +1,75 @@ + + * + * @author Maxence Lange + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Create new fields for type and lazy loading in appconfig for the new IAppConfig API. + */ +class Version29000Date20240124132201 extends SimpleMigrationStep { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('appconfig'); + + // we will drop 'lazy', we start to clean related indexes first + if ($table->hasIndex('ac_lazy_i')) { + $table->dropIndex('ac_lazy_i'); + } + + if ($table->hasIndex('ac_app_lazy_i')) { + $table->dropIndex('ac_app_lazy_i'); + } + + if ($table->hasIndex('ac_app_lazy_key_i')) { + $table->dropIndex('ac_app_lazy_key_i'); + } + + if ($table->hasColumn('lazy')) { + $table->dropColumn('lazy'); + } + + // create field 'type' if it does not exist yet, or fix the fact that it is missing 'unsigned' + if (!$table->hasColumn('type')) { + $table->addColumn('type', Types::INTEGER, ['notnull' => true, 'default' => 2, 'unsigned' => true]); + } else { + $table->modifyColumn('type', ['notnull' => true, 'default' => 2, 'unsigned' => true]); + } + + // not needed anymore + if ($table->hasIndex('appconfig_config_key_index')) { + $table->dropIndex('appconfig_config_key_index'); + } + + return $schema; + } +} diff --git a/core/Migrations/Version29000Date20240124132202.php b/core/Migrations/Version29000Date20240124132202.php new file mode 100644 index 00000000000..b393477a52a --- /dev/null +++ b/core/Migrations/Version29000Date20240124132202.php @@ -0,0 +1,53 @@ + + * + * @author Maxence Lange + * + * @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 . + * + */ + +namespace OC\Core\Migrations; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Create new fields for type and lazy loading in appconfig for the new IAppConfig API. + */ +class Version29000Date20240124132202 extends SimpleMigrationStep { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $table = $schema->getTable('appconfig'); + $table->addColumn('lazy', Types::SMALLINT, ['notnull' => true, 'default' => 0, 'length' => 1, 'unsigned' => true]); + + /** + * we only need an index on lazy, the group 'appid'+'configkey' already + * exists as primary ({@see Version13000Date20170718121200}) + */ + $table->addIndex(['lazy'], 'ac_lazy_i'); + + return $schema; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index f1de00a49bf..2fffd0c2f2d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1241,8 +1241,11 @@ return array( 'OC\\Core\\Migrations\\Version28000Date20230906104802' => $baseDir . '/core/Migrations/Version28000Date20230906104802.php', 'OC\\Core\\Migrations\\Version28000Date20231004103301' => $baseDir . '/core/Migrations/Version28000Date20231004103301.php', 'OC\\Core\\Migrations\\Version28000Date20231103104802' => $baseDir . '/core/Migrations/Version28000Date20231103104802.php', + 'OC\\Core\\Migrations\\Version28000Date20231126110901' => $baseDir . '/core/Migrations/Version28000Date20231126110901.php', 'OC\\Core\\Migrations\\Version29000Date20231126110901' => $baseDir . '/core/Migrations/Version29000Date20231126110901.php', 'OC\\Core\\Migrations\\Version29000Date20231213104850' => $baseDir . '/core/Migrations/Version29000Date20231213104850.php', + 'OC\\Core\\Migrations\\Version29000Date20240124132201' => $baseDir . '/core/Migrations/Version29000Date20240124132201.php', + 'OC\\Core\\Migrations\\Version29000Date20240124132202' => $baseDir . '/core/Migrations/Version29000Date20240124132202.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 017918b3f44..d38db8050cc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1274,8 +1274,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version28000Date20230906104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20230906104802.php', 'OC\\Core\\Migrations\\Version28000Date20231004103301' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231004103301.php', 'OC\\Core\\Migrations\\Version28000Date20231103104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231103104802.php', + 'OC\\Core\\Migrations\\Version28000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231126110901.php', 'OC\\Core\\Migrations\\Version29000Date20231126110901' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231126110901.php', 'OC\\Core\\Migrations\\Version29000Date20231213104850' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231213104850.php', + 'OC\\Core\\Migrations\\Version29000Date20240124132201' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132201.php', + 'OC\\Core\\Migrations\\Version29000Date20240124132202' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20240124132202.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php index d15aff37c5b..8064fe186f8 100644 --- a/lib/private/AppConfig.php +++ b/lib/private/AppConfig.php @@ -83,6 +83,9 @@ class AppConfig implements IAppConfig { /** * $migrationCompleted is only needed to manage the previous structure * of the database during the upgrading process to nc29. + * + * only when upgrading from a version prior 28.0.2 + * * @TODO: remove this value in Nextcloud 30+ */ private bool $migrationCompleted = true; @@ -735,7 +738,7 @@ class AppConfig implements IAppConfig { $insert = $this->connection->getQueryBuilder(); $insert->insert('appconfig') ->setValue('appid', $insert->createNamedParameter($app)) - ->setValue('lazy', $insert->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL)) + ->setValue('lazy', $insert->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) ->setValue('type', $insert->createNamedParameter($type, IQueryBuilder::PARAM_INT)) ->setValue('configkey', $insert->createNamedParameter($key)) ->setValue('configvalue', $insert->createNamedParameter($value)); @@ -788,7 +791,7 @@ class AppConfig implements IAppConfig { $update = $this->connection->getQueryBuilder(); $update->update('appconfig') ->set('configvalue', $update->createNamedParameter($value)) - ->set('lazy', $update->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL)) + ->set('lazy', $update->createNamedParameter(($lazy) ? 1 : 0, IQueryBuilder::PARAM_INT)) ->set('type', $update->createNamedParameter($type, IQueryBuilder::PARAM_INT)) ->where($update->expr()->eq('appid', $update->createNamedParameter($app))) ->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key))); @@ -925,7 +928,7 @@ class AppConfig implements IAppConfig { $update = $this->connection->getQueryBuilder(); $update->update('appconfig') - ->set('lazy', $update->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL)) + ->set('lazy', $update->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT)) ->where($update->expr()->eq('appid', $update->createNamedParameter($app))) ->andWhere($update->expr()->eq('configkey', $update->createNamedParameter($key))); $update->executeStatement(); @@ -1157,21 +1160,13 @@ class AppConfig implements IAppConfig { if (!$this->migrationCompleted) { $qb->select('appid', 'configkey', 'configvalue'); } else { - $qb->select('appid', 'configkey', 'configvalue', 'type', 'lazy'); + // we only need value from lazy when loadConfig does not specify it + $qb->select('appid', 'configkey', 'configvalue', 'type'); + if ($lazy !== null) { - if ($this->connection->getDatabaseProvider() === IDBConnection::PLATFORM_ORACLE) { - // Oracle does not like empty string nor false boolean !? - if ($lazy) { - $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter('1', IQueryBuilder::PARAM_INT))); - } else { - $qb->where($qb->expr()->orX( - $qb->expr()->isNull('lazy'), - $qb->expr()->eq('lazy', $qb->createNamedParameter('0', IQueryBuilder::PARAM_INT)) - )); - } - } else { - $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy, IQueryBuilder::PARAM_BOOL))); - } + $qb->where($qb->expr()->eq('lazy', $qb->createNamedParameter($lazy ? 1 : 0, IQueryBuilder::PARAM_INT))); + } else { + $qb->addSelect('lazy'); } } @@ -1195,9 +1190,8 @@ class AppConfig implements IAppConfig { $rows = $result->fetchAll(); foreach ($rows as $row) { - // if migration is not completed, 'lazy' and 'type' does not exist in $row - // also on oracle, lazy can be null ... - if ($row['lazy'] ?? false) { + // most of the time, 'lazy' is not in the select because its value is already known + if (($row['lazy'] ?? ($lazy ?? 0) ? 1 : 0) === 1) { $cache = &$this->lazyCache; } else { $cache = &$this->fastCache; @@ -1282,6 +1276,15 @@ class AppConfig implements IAppConfig { * @deprecated */ public function setValue($app, $key, $value) { + /** + * TODO: would it be overkill, or decently improve performance, to catch + * call to this method with $key='enabled' and 'hide' config value related + * to $app when the app is disabled (by modifying entry in database: lazy=lazy+2) + * or enabled (lazy=lazy-2) + * + * this solution would remove the loading of config values from disabled app + * unless calling the method {@see loadConfigAll()} + */ return $this->setTypedValue($app, $key, (string)$value, false, self::VALUE_MIXED); } diff --git a/version.php b/version.php index 47ada5b8777..a1855bfad5a 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level // when updating major/minor version number. -$OC_Version = [29, 0, 0, 4]; +$OC_Version = [29, 0, 0, 5]; // The human-readable string $OC_VersionString = '29.0.0 dev'; From 31e8ed42a137c69cc3683f67e9b8fd3b9878d224 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 25 Jan 2024 10:33:39 -0100 Subject: [PATCH 2/3] Update core/Migrations/Version29000Date20240124132201.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Maxence Lange --- core/Migrations/Version29000Date20240124132201.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Migrations/Version29000Date20240124132201.php b/core/Migrations/Version29000Date20240124132201.php index 86c21973c48..dd287da1f89 100644 --- a/core/Migrations/Version29000Date20240124132201.php +++ b/core/Migrations/Version29000Date20240124132201.php @@ -32,7 +32,7 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; /** - * Create new fields for type and lazy loading in appconfig for the new IAppConfig API. + * Create new column for type and remove previous lazy column in appconfig (will be recreated by Version29000Date20240124132202) for the new IAppConfig API. */ class Version29000Date20240124132201 extends SimpleMigrationStep { public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { From a845e0f73addd5cb7837ed1362b42ccf43ccc89c Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Thu, 25 Jan 2024 10:33:47 -0100 Subject: [PATCH 3/3] Update core/Migrations/Version29000Date20240124132202.php Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> Signed-off-by: Maxence Lange --- core/Migrations/Version29000Date20240124132202.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Migrations/Version29000Date20240124132202.php b/core/Migrations/Version29000Date20240124132202.php index b393477a52a..cca0d6a5a71 100644 --- a/core/Migrations/Version29000Date20240124132202.php +++ b/core/Migrations/Version29000Date20240124132202.php @@ -32,7 +32,7 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; /** - * Create new fields for type and lazy loading in appconfig for the new IAppConfig API. + * Create new column and index for lazy loading in appconfig for the new IAppConfig API. */ class Version29000Date20240124132202 extends SimpleMigrationStep { public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {