diff --git a/classes/install/DowngradeNotSupportedException.inc.php b/classes/install/DowngradeNotSupportedException.inc.php index 5d20cb424cf..28618c5a487 100644 --- a/classes/install/DowngradeNotSupportedException.inc.php +++ b/classes/install/DowngradeNotSupportedException.inc.php @@ -12,10 +12,16 @@ namespace PKP\install; -class DowngradeNotSupportedException extends \Exception +use Exception; +use Throwable; + +class DowngradeNotSupportedException extends Exception { - public function __construct() + /** + * Constructor + */ + public function __construct(string $message = 'Downgrade not supported', int $code = 0, ?Throwable $previous = null) { - parent::__construct('Downgrade not supported!'); + parent::__construct($message, $code, $previous); } } diff --git a/classes/install/Installer.inc.php b/classes/install/Installer.inc.php index 243a788cfed..ce971dc30f1 100644 --- a/classes/install/Installer.inc.php +++ b/classes/install/Installer.inc.php @@ -19,7 +19,7 @@ use APP\core\Application; use APP\file\LibraryFileManager; use APP\i18n\AppLocale; - +use Exception; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; @@ -454,7 +454,7 @@ public function executeAction($action) try { $migration->up(); $this->migrations[] = $migration; - } catch (\Exception $e) { + } catch (Exception $e) { // Log an error message $this->setError( self::INSTALLER_ERROR_DB, @@ -466,7 +466,11 @@ public function executeAction($action) try { $this->log(sprintf('revert migration: %s', get_class($previousMigration))); $previousMigration->down(); - } catch (\PKP\install\DowngradeNotSupportedException $e) { + } catch (DowngradeNotSupportedException $e) { + $this->log(sprintf('downgrade for "%s" unsupported: %s', get_class($previousMigration), $e->getMessage())); + break; + } catch (Exception $e) { + $this->log(sprintf('error while downgrading "%s": %s', get_class($previousMigration), Config::getVar('debug', 'show_stacktrace') ? (string) $e : $e->getMessage())); break; } } @@ -725,8 +729,8 @@ public function setCurrentVersion($version) * * @param $installer object * @param $attr array Attributes: array containing - * 'key' => 'EMAIL_KEY_HERE', - * 'locales' => 'en_US,fr_CA,...' + * 'key' => 'EMAIL_KEY_HERE', + * 'locales' => 'en_US,fr_CA,...' */ public function installEmailTemplate($installer, $attr) { @@ -849,12 +853,12 @@ public function addPluginVersions() 0, 0, // Major, minor, revision, build Core::getCurrentDate(), // Date installed - 1, // Current + 1, // Current 'plugins.' . $category, // Type basename($plugin->getPluginPath()), // Product - '', // Class name - 0, // Lazy load - $plugin->isSitePlugin() // Site wide + '', // Class name + 0, // Lazy load + $plugin->isSitePlugin() // Site wide ); } $versionDao->insertVersion($pluginVersion, true); @@ -1028,15 +1032,16 @@ public function migrateMetadataSettings() foreach ($metadataSettings as $metadataSetting) { foreach ($contextIds as $contextId) { $result = $contextDao->retrieve( - ' - SELECT * FROM ' . $contextDao->settingsTableName . ' WHERE - ' . $contextDao->primaryKeyColumn . ' = ? - AND ( - setting_name = ? - OR setting_name = ? - OR setting_name = ? - ) - ', + 'SELECT * + FROM ' . $contextDao->settingsTableName . ' + WHERE + ' . $contextDao->primaryKeyColumn . ' = ? + AND ( + setting_name = ? + OR setting_name = ? + OR setting_name = ? + ) + ', [ $contextId, $metadataSetting . 'EnabledWorkflow', @@ -1057,13 +1062,12 @@ public function migrateMetadataSettings() if ($value !== METADATA_DISABLE) { $contextDao->update( - ' - INSERT INTO ' . $contextDao->settingsTableName . ' ( - ' . $contextDao->primaryKeyColumn . ', - locale, - setting_name, - setting_value - ) VALUES (?, ?, ?, ?)', + 'INSERT INTO ' . $contextDao->settingsTableName . ' ( + ' . $contextDao->primaryKeyColumn . ', + locale, + setting_name, + setting_value + ) VALUES (?, ?, ?, ?)', [ $contextId, '', @@ -1074,15 +1078,14 @@ public function migrateMetadataSettings() } $contextDao->update( - ' - DELETE FROM ' . $contextDao->settingsTableName . ' WHERE - ' . $contextDao->primaryKeyColumn . ' = ? - AND ( - setting_name = ? - OR setting_name = ? - OR setting_name = ? - ) - ', + 'DELETE FROM ' . $contextDao->settingsTableName . ' WHERE + ' . $contextDao->primaryKeyColumn . ' = ? + AND ( + setting_name = ? + OR setting_name = ? + OR setting_name = ? + ) + ', [ $contextId, $metadataSetting . 'EnabledWorkflow', @@ -1112,9 +1115,9 @@ public function setStatsEmailSettings() for ($users = $userGroupDao->getUsersById($userGroup->getId(), $context->getId()); $user = $users->next();) { $notificationSubscriptionSettingsDao->update( 'INSERT INTO notification_subscription_settings - (setting_name, setting_value, user_id, context, setting_type) - VALUES - (?, ?, ?, ?, ?)', + (setting_name, setting_value, user_id, context, setting_type) + VALUES + (?, ?, ?, ?, ?)', [ 'blocked_emailed_notification', PKPNotification::NOTIFICATION_TYPE_EDITORIAL_REPORT, diff --git a/classes/migration/install/CommonMigration.inc.php b/classes/migration/install/CommonMigration.inc.php index b92871784e6..d3701b5982b 100644 --- a/classes/migration/install/CommonMigration.inc.php +++ b/classes/migration/install/CommonMigration.inc.php @@ -98,11 +98,9 @@ public function up(): void $table->bigInteger('user_id'); $table->string('locale', 14)->default(''); $table->string('setting_name', 255); - $table->bigInteger('assoc_type')->default(0); - $table->bigInteger('assoc_id')->default(0); $table->text('setting_value')->nullable(); $table->index(['user_id'], 'user_settings_user_id'); - $table->unique(['user_id', 'locale', 'setting_name', 'assoc_type', 'assoc_id'], 'user_settings_pkey'); + $table->unique(['user_id', 'locale', 'setting_name'], 'user_settings_pkey'); $table->index(['setting_name', 'locale'], 'user_settings_locale_setting_name_index'); }); diff --git a/classes/migration/upgrade/PKPv3_3_0UpgradeMigration.inc.php b/classes/migration/upgrade/PKPv3_3_0UpgradeMigration.inc.php index e2c20add18b..ee3129687cf 100755 --- a/classes/migration/upgrade/PKPv3_3_0UpgradeMigration.inc.php +++ b/classes/migration/upgrade/PKPv3_3_0UpgradeMigration.inc.php @@ -94,10 +94,6 @@ public function up(): void }); } - // Use nulls instead of 0 for assoc_type/id in user_settings - DB::table('user_settings')->where('assoc_type', 0)->update(['assoc_type' => null]); - DB::table('user_settings')->where('assoc_id', 0)->update(['assoc_id' => null]); - // pkp/pkp-lib#6093 Don't allow nulls (previously an upgrade workaround) Schema::table('announcement_types', function (Blueprint $table) { $table->bigInteger('assoc_type')->nullable(false)->change(); diff --git a/classes/migration/upgrade/v3_4_0/I6093_AddForeignKeys.inc.php b/classes/migration/upgrade/v3_4_0/I6093_AddForeignKeys.inc.php index c5179b381f3..256e45f3a07 100755 --- a/classes/migration/upgrade/v3_4_0/I6093_AddForeignKeys.inc.php +++ b/classes/migration/upgrade/v3_4_0/I6093_AddForeignKeys.inc.php @@ -16,6 +16,7 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; +use PKP\install\DowngradeNotSupportedException; class I6093_AddForeignKeys extends \PKP\migration\Migration { @@ -75,6 +76,6 @@ public function up(): void */ public function down(): void { - throw new \Exception('Downgrade unsupported due to removed data!'); + throw new DowngradeNotSupportedException('Downgrade unsupported due to removed data'); } } diff --git a/classes/migration/upgrade/v3_4_0/I7167_RemoveDuplicatedUserSettingsAndDeprecatedFields.inc.php b/classes/migration/upgrade/v3_4_0/I7167_RemoveDuplicatedUserSettingsAndDeprecatedFields.inc.php new file mode 100644 index 00000000000..085590e4789 --- /dev/null +++ b/classes/migration/upgrade/v3_4_0/I7167_RemoveDuplicatedUserSettingsAndDeprecatedFields.inc.php @@ -0,0 +1,113 @@ + COALESCE(duplicated.assoc_type, -999999) + OR COALESCE(s.assoc_id, -999999) <> COALESCE(duplicated.assoc_id, -999999) + ) + -- Attempts to find a better fitting record among the duplicates (preference is given to the smaller assoc_id/assoc_type values) + LEFT JOIN user_settings best + ON best.setting_name = duplicated.setting_name + AND best.user_id = duplicated.user_id + AND best.locale = duplicated.locale + AND ( + COALESCE(best.assoc_id, 999999) < COALESCE(duplicated.assoc_id, 999999) + OR ( + COALESCE(best.assoc_id, 999999) = COALESCE(duplicated.assoc_id, 999999) + AND COALESCE(best.assoc_type, 999999) < COALESCE(duplicated.assoc_type, 999999) + ) + ) + -- Ensures a better record was found (if not found, it means the current duplicated record is the best and shouldn't be removed) + WHERE best.user_id IS NOT NULL" + ); + } else { + DB::unprepared( + "DELETE FROM user_settings s + USING user_settings duplicated + -- Attempts to find a better fitting record among the duplicates (preference is given to the smaller assoc_id/assoc_type values) + LEFT JOIN user_settings best + ON best.setting_name = duplicated.setting_name + AND best.user_id = duplicated.user_id + AND best.locale = duplicated.locale + AND ( + COALESCE(best.assoc_id, 999999) < COALESCE(duplicated.assoc_id, 999999) + OR ( + COALESCE(best.assoc_id, 999999) = COALESCE(duplicated.assoc_id, 999999) + AND COALESCE(best.assoc_type, 999999) < COALESCE(duplicated.assoc_type, 999999) + ) + ) + -- Locates all duplicated settings (same key fields, except the assoc_type/assoc_id) + WHERE s.setting_name = duplicated.setting_name + AND s.user_id = duplicated.user_id + AND s.locale = duplicated.locale + AND ( + COALESCE(s.assoc_type, -999999) <> COALESCE(duplicated.assoc_type, -999999) + OR COALESCE(s.assoc_id, -999999) <> COALESCE(duplicated.assoc_id, -999999) + ) + -- Ensures a better record was found (if not found, it means the current duplicated record is the best and shouldn't be removed) + AND best.user_id IS NOT NULL" + ); + } + + // Here we should be free of duplicates, so it's safe to remove the columns without creating duplicated entries. + Schema::table( + 'user_settings', + function (Blueprint $table): void { + $table->dropIndex('user_settings_pkey'); + $table->dropColumn('assoc_id', 'assoc_type'); + // Restore the primary/unique index, using the previous field order + $table->unique(['user_id', 'locale', 'setting_name'], 'user_settings_pkey'); + } + ); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + throw new DowngradeNotSupportedException('Downgrade unsupported due to removed data'); + } +} diff --git a/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.inc.php b/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.inc.php index 8c7ba0f479c..7fdd6f1b73c 100755 --- a/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.inc.php +++ b/classes/migration/upgrade/v3_4_0/PreflightCheckMigration.inc.php @@ -7,7 +7,7 @@ * Copyright (c) 2000-2021 John Willinsky * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING. * - * @class I6093_AddForeignKeys + * @class PreflightCheckMigration * @brief Check for common problems early in the upgrade process. */ @@ -78,6 +78,9 @@ public function up(): void } } + /** + * Rollback the migrations. + */ public function down(): void { if ($fallbackVersion = $this->setFallbackVersion()) { diff --git a/classes/plugins/PluginRegistry.inc.php b/classes/plugins/PluginRegistry.inc.php index bfb559e0ce7..fec2f284dd5 100644 --- a/classes/plugins/PluginRegistry.inc.php +++ b/classes/plugins/PluginRegistry.inc.php @@ -89,21 +89,12 @@ public static function register($category, $plugin, $path, $mainContextId = null $pluginName = $plugin->getName(); $plugins = & self::getPlugins(); - // Allow the plugin to register. - if (!$plugin->register($category, $path, $mainContextId)) { + // If the plugin is already loaded or failed/refused to register + if (isset($plugins[$category][$pluginName]) || !$plugin->register($category, $path, $mainContextId)) { return false; } - // If the plugin was already loaded, do not load it again. - if (isset($plugins[$category][$pluginName])) { - return false; - } - - if (isset($plugins[$category])) { - $plugins[$category][$pluginName] = $plugin; - } else { - $plugins[$category] = [$pluginName => $plugin]; - } + $plugins[$category][$pluginName] = $plugin; return true; } @@ -258,9 +249,12 @@ public static function getCategories() */ public static function loadAllPlugins($enabledOnly = false) { - // Retrieve and register categories (order is significant). - foreach (self::getCategories() as $category) { - self::loadCategory($category, $enabledOnly); + static $isLoaded; + if (!$isLoaded) { + // Retrieve and register categories (order is significant). + foreach (self::getCategories() as $category) { + self::loadCategory($category, $enabledOnly); + } } return self::getAllPlugins(); } diff --git a/classes/user/DAO.inc.php b/classes/user/DAO.inc.php index a583186db37..52b490cd8be 100644 --- a/classes/user/DAO.inc.php +++ b/classes/user/DAO.inc.php @@ -275,10 +275,10 @@ public function changeSitePrimaryLocale($oldLocale, $newLocale) // get all names of all users in the new locale $result = DB::select( 'SELECT DISTINCT us.user_id, usg.setting_value AS given_name, usf.setting_value AS family_name, usp.setting_value AS preferred_public_name - FROM user_settings us - LEFT JOIN user_settings usg ON (usg.user_id = us.user_id AND usg.locale = ? AND usg.setting_name = ?) - LEFT JOIN user_settings usf ON (usf.user_id = us.user_id AND usf.locale = ? AND usf.setting_name = ?) - LEFT JOIN user_settings usp ON (usp.user_id = us.user_id AND usp.locale = ? AND usp.setting_name = ?)', + FROM user_settings us + LEFT JOIN user_settings usg ON (usg.user_id = us.user_id AND usg.locale = ? AND usg.setting_name = ?) + LEFT JOIN user_settings usf ON (usf.user_id = us.user_id AND usf.locale = ? AND usf.setting_name = ?) + LEFT JOIN user_settings usp ON (usp.user_id = us.user_id AND usp.locale = ? AND usp.setting_name = ?)', [$newLocale, Identity::IDENTITY_SETTING_GIVENNAME, $newLocale, Identity::IDENTITY_SETTING_FAMILYNAME, $newLocale, 'preferredPublicName'] ); foreach ($result as $row) { @@ -287,20 +287,20 @@ public function changeSitePrimaryLocale($oldLocale, $newLocale) // if no user name exists in the new locale, insert them all foreach ($settingNames as $settingName) { DB::insert( - "INSERT INTO user_settings (user_id, locale, setting_name, setting_value, setting_type) - SELECT DISTINCT us.user_id, ?, ?, us.setting_value, 'string' - FROM user_settings us - WHERE us.setting_name = ? AND us.locale = ? AND us.user_id = ?", + "INSERT INTO user_settings (user_id, locale, setting_name, setting_value) + SELECT DISTINCT us.user_id, ?, ?, us.setting_value + FROM user_settings us + WHERE us.setting_name = ? AND us.locale = ? AND us.user_id = ?", [$newLocale, $settingName, $settingName, $oldLocale, $userId] ); } } elseif (empty($row->given_name)) { // if the given name does not exist in the new locale (but one of the other names do exist), insert it DB::insert( - "INSERT INTO user_settings (user_id, locale, setting_name, setting_value, setting_type) - SELECT DISTINCT us.user_id, ?, ?, us.setting_value, 'string' - FROM user_settings us - WHERE us.setting_name = ? AND us.locale = ? AND us.user_id = ?", + "INSERT INTO user_settings (user_id, locale, setting_name, setting_value) + SELECT DISTINCT us.user_id, ?, ?, us.setting_value + FROM user_settings us + WHERE us.setting_name = ? AND us.locale = ? AND us.user_id = ?", [$newLocale, Identity::IDENTITY_SETTING_GIVENNAME, Identity::IDENTITY_SETTING_GIVENNAME, $oldLocale, $userId] ); } diff --git a/dtd/install.dtd b/dtd/install.dtd index 6940607b235..32fdbef2044 100644 --- a/dtd/install.dtd +++ b/dtd/install.dtd @@ -44,7 +44,8 @@ condition CDATA #IMPLIED> + class CDATA #REQUIRED + fallback CDATA #IMPLIED>