pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
306 stars 445 forks source link

Wrong or missing config.inc.php variable breaks the upgrade #9781

Open marcbria opened 7 months ago

marcbria commented 7 months ago

Describe the bug The issue was initially posted in the support forum here.

In short, if config.inc.php is not properly updated between minor upgrades, the upgrade can't finish the process and left a corrupted DB.

To Reproduce Steps to reproduce the behavior:

  1. Upgrade any OJS 3.3, to 3.4 without changing config.inc.php (ie: be sure sendmail vars are not set).
  2. See error.

What application are you using? OJS, OMP or OPS version 3.3.x

Additional information Detailed error log is:

/opt/php80/bin/php tools/upgrade.php check
PHP Fatal error: Uncaught Exception: Mailer driver isn’t specified in the application’s config in /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php:315
Stack trace:
#0 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php(268): PKP\core\PKPContainer::getDefaultMailer()
#1 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php(117): PKP\core\PKPContainer->loadConfiguration()
#2 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPApplication.php(231): PKP\core\PKPContainer->registerConfiguredProviders()
#3 /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPApplication.php(203): PKP\core\PKPApplication->initializeLaravelContainer()
#4 /var/www/user/data/www/ojs/classes/core/Application.php(50): PKP\core\PKPApplication->__construct()
#5 /var/www/user/data/www/ojs/lib/pkp/includes/bootstrap.php(37): APP\core\Application->__construct()
#6 /var/www/user/data/www/ojs/lib/pkp/classes/cliTool/CommandLineTool.php(41): require_once(‘/var/www/user…’)
#7 /var/www/user/data/www/ojs/tools/bootstrap.php(17): require(‘/var/www/user…’)
#8 /var/www/user/data/www/ojs/tools/upgrade.php(19): require(‘/var/www/user…’)
#9 {main}
thrown in /var/www/user/data/www/ojs/lib/pkp/classes/core/PKPContainer.php on line 315

As far as config.inc.php usually change between minor or major upgrades, this becomes a quite usual issue. As far as the DB is left corrupted and some fellows don't remember to backup-before-upgrade, I feel this need to be properly priorized.

A possible minimal solution is adding pre-fight check of the config.inc.php to confirm all the essential/new vars are included (or at least, the new ones added between usual upgrade paths). List of new vars is published in the Release Notes.

A middle ground solution would be adding a config validator that could be cached based on the modified date + application version.

An improved solution (suggested by Andrew Gearhart) would be create a "config builder". It will be able to handle valid, discontinued, and required settings across different software versions, specifying supported values (like "On/Off, True/False, 1/0, String, Integer, Float"). Using YAML/JSON file will facilitate the editing and potentially implementing a front-end TUI for creating the config file can streamline the process and address previous challenges in passing configurations.

The final solution will probably be stop maintaining a homebrew config file and adopt standard tools for config file maintenance. Preferably that would mean Laravel's (since we're going that way with so much else).

For any final solution other issues also need to be taken in consideration as:

asmecher commented 7 months ago

I've just double-checked what happens when the driver isn't specified on a local 3.4.0-x checkout, and running php tools/upgrade.php upgrade dies immediately with Mailer driver isn't specified in the application's config error message. The database isn't corrupted in the process, so it should just be necessary to add the missing configuration (as documented in the release notes) and re-run the upgrade.

Can anyone verify that the database can be corrupted without this config being added?