Skip to content

[4.0] Add minimum db version check to the installation's database connection check#28135

Merged
wilsonge merged 12 commits intojoomla:4.0-devfrom
richard67:4.0-dev-installation-tweaks-1
Mar 4, 2020
Merged

[4.0] Add minimum db version check to the installation's database connection check#28135
wilsonge merged 12 commits intojoomla:4.0-devfrom
richard67:4.0-dev-installation-tweaks-1

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Feb 29, 2020

Pull Request for Issue #26106 .

Summary of Changes

This pull request (PR) adds the check for the minimum database version supported by the database driver to the database connection verification step of the setup model.

We have the same check in the database model when the database is created if necessary and then the installation is performed, but this check is be too late for the setup form to handle the error correctly.

In addition this PR adds a check for the minimum database version supported by the CMS, which might be different to the requirement of the database driver. In fact, the more restrictive requirement, i.e. the larger minimum version, will be used for the check.

Testing Instructions

Preparation

You either need to have a database which doesn't fulfill the minimum server version requirement of the particular database driver and the CMS, and another one which fulfilles these requirements, or you have to patch the database driver's minimum version during the test when being instructed so. In addition you will have to modify the minimum server version requirement of the CMS when being instructed so. Instructions how to make these modifications you find below in this section.

The minimum versions currently set in the database drivers are:

  • 5.6 for MySQLi and MySQL (PDO) drivers when using a MySQL database
  • 10.0 for MySQLi and MySQL (PDO) drivers when using a MariaDB database
  • 9.4.0 for PostgreSQL

The requirements described in our docs for J4, and for MariaDB we don't have an official statement) are different for PostgreSQL: The docs say 11.0, i.e. the CMS is more restirctive than the driver. For MariaDB the docs say nothing.

So the CMS requirements introduced by this PR are as follows:

  • 5.6 for MySQL database
  • 10.0 for MariaDB database
  • 11.0 for PostgreSQL database

The minimum version as required by the particular database driver for the particular database type can be patched at the following places:

For MySQL databases

MySQLi driver - file libraries/vendor/joomla/database/src/Mysqli/MysqliDriver.php, line 86
MySQL (PDO) driver - file libraries/vendor/joomla/database/src/Mysql/MysqlDriver.php, line 72

protected static $dbMinimum = '5.6';

For MariaDB databases

MySQLi driver - file libraries/vendor/joomla/database/src/Mysqli/MysqliDriver.php, line 94
MySQL (PDO) driver - file libraries/vendor/joomla/database/src/Mysql/MysqlDriver.php, line 80

protected static $dbMinMariadb = '10.0';

For PostgreSQL databases

PostgreSQL (PDO) driver - file libraries/vendor/joomla/database/src/Pgsql/PgsqlDriver.php, line 54

protected static $dbMinimum = '9.4.0';

The minimum version as required by the CMS for the particular database type can be patched at the following places:

For MySQL databases

protected static $dbMinimumMySql = '5.6';

For MariaDB databases

protected static $dbMinimumMariaDb = '10.0';

For PostgreSQL databases

protected static $dbMinimumPostgreSql = '11.0';

Procedure

Step 1: On a clean 4.0-dev without the patch of this PR applied, make a new installation. Either use a database which doesn't fulfill the driver's minimum version requirement, or patch the driver's minimum version as described above to a version higher than the version of your database. Fill in all information for a successful installation, including valid database connection information. Then select the "Install Joomla" button at the bottom.

Result: Either an error alert is shown about the database version requirement, see section "Expected result" below, or the installation succeeds as described in issue #26106 . In the latter case skip the following step 2 and continue with step 3.

Step 2: Now either change database name to another database which fulfills the minimum version requirements or patch the driver's minimum version as described above to a version lower than or equal to the version of your database. Then select again the "Install Joomla" button at the bottom.

Result: An information alert is shown about an unspecified database error, see section "Actual result" below.

Step 3: Delete the session cookies saved in your browser for the tested domain/server, and delete the configuration.php file in your Joomla folder.

Step 4: Apply the patch.

Step 5: Either user a database which doesn't fulfill the driver's minimum version requirement, or if necessary patch the driver's minimum version so that this condition is fulfilled, i.e. the driver's requirement is higher than the version of your database.

Step 6: Make a new installation. Fill in all information for a successful installation, including valid database connection information. Then select the "Install Joomla" button at the bottom.

Result: An error alert is shown about the database version requirement, see section "Expected result" below.

Step 7: Now either change database name to another database which fulfills the minimum version requirement by the driver or patch the driver's minimum version as described above to a version lower than or equal to the version of your database.

Step 8: Make sure that the database doesn't_ fulfill the CMS' minimum version requirement by patching the CMS' minimum version so that this condition is fulfilled, i.e. the CMS' requirement is higher than the version of your database.

Step 9: Select again the "Install Joomla" button at the bottom.

Result: An error alert is shown about the database version requirement, see section "Expected result" below.

Step 10: Make sure that the database fulfills the CMS' minimum version requirement by patching the CMS' minimum version so that the CMS' requirement is lower than or equal to the version of your database.

Step 11: Select again the "Install Joomla" button at the bottom.

Result: The installation is started, and if the database fulfills the requirement and everything else is correct, too, it will finish with success.

Expected result

When specifying a database which doesn't fulfill the minimum version requirement of the selected db driver or the CMS, an error alert is shown.

For a MySQL database with the minimum version of the driver or the CMS patched to 8.0.0 as described in the testing instructions, with any of the 2 drivers MySQLi and MySQL (PDO):
j4-installation-tweaks-1_1

For a PostgreSQL database with the minimum version requirement of the CMS not fulfilled:
j4-installation-tweaks-1_2

After correcting the database connection information so a database is used which fulfills the minimum version requirements of the database driver and the CMS, the installation can be started.

Actual result

When specifying a database which doesn't fulfill the minimum version requirement of the selected db driver, either an error alert is shown as descibed above for the expected result, or the installation succeeds without any information as described in issue #26106 .

In case of the error alert, when correcting the database connection information so a database is used which fulfills the minimum version requirement, and then trying again to install, the installation can't be started. Following information alert is shown, and there is no way out except of deleting the session cookie and the configuration.php file and starting the installation from beginning:

j4-installation-tweaks-1_3

Finally, the requirements by the CMS are not checked at all, which is currently a mistake for PostgreSQL.

Documentation Changes Required

Not that I knew of any.

@mbabker
Copy link
Contributor

mbabker commented Feb 29, 2020

You need to inline these version checks. The Framework doesn’t have the same minimums as the CMS, so the database API’s check isn’t 100% reliable (the Framework still supports PostgreSQL 9.4, the CMS requires 11).

@richard67
Copy link
Member Author

You need to inline these version checks. The Framework doesn’t have the same minimums as the CMS, so the database API’s check isn’t 100% reliable (the Framework still supports PostgreSQL 9.4, the CMS requires 11).

@mbabker With "inline" you mean "implement locally", right? Any suggestion where to specify the minimum versions? Locally in file installation/src/Model/SetupModel.php?

@richard67
Copy link
Member Author

@zero-24 Please wait with testing, clarifications ongoing. Will ping you when ready. Of course your input is welcome, too.

@richard67 richard67 changed the title [4.0] Add minimum db version check to the installation's database connection check [4.0] [WiP] Add minimum db version check to the installation's database connection check Feb 29, 2020
@richard67 richard67 changed the title [4.0] [WiP] Add minimum db version check to the installation's database connection check [4.0] Add minimum db version check to the installation's database connection check Feb 29, 2020
@richard67
Copy link
Member Author

Ready for tests.

@richard67
Copy link
Member Author

richard67 commented Feb 29, 2020

The PostgreSQL system tests are failing in drone. I think it could be because the testing environment runs with something older than PostgreSQL 11. @HLeithner Could this be, and if so, should we update our testing environment or shall I change the CMS's database server version requirement in this PR for PostgreSQL, and if so, to which value?

Update: The name "postgres:9-alpine" in drone.yml sounds like 9. So that's definitely a bit too old for the CMS requirement but still ok for the database driver. Docker images for postgresql related ci tests in 4.0-dev should be updated to 11.

richard67 and others added 2 commits February 29, 2020 23:54
Co-Authored-By: Tobias Zulauf <zero-24@users.noreply.github.com>
Co-Authored-By: Tobias Zulauf <zero-24@users.noreply.github.com>
@richard67
Copy link
Member Author

richard67 commented Feb 29, 2020

PR for updating the docker images to PostgreSQL 11 is #28164 . That needs to be merged before this one here can be merged. Update: Has been merged meanwhile.

Co-Authored-By: Tobias Zulauf <zero-24@users.noreply.github.com>
@zero-24
Copy link
Contributor

zero-24 commented Feb 29, 2020

I have tested this item ✅ successfully on 0e79335

LGTM just on small version change suggested inline.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28135.

@richard67
Copy link
Member Author

@zero-24 Change already applied. Could you mark your test result again in the issue tracker so it fits to the last commit? Thanks in advance.

@wilsonge
Copy link
Contributor

Merged the tests - update branch and should be fine

@richard67
Copy link
Member Author

@wilsonge Done. Thanks for review.

Copy link
Contributor

@zero-24 zero-24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@zero-24 zero-24 added this to the Joomla 4.0 milestone Feb 29, 2020
@richard67
Copy link
Member Author

@zero-24 Could you test again? I had to make some changes in code. But testing procedure and results are the same.

@HLeithner Could you test, too? As far as I remember you have MariaDB available, something I don't have here.

Testers please report back database type and version used.

Thanks in advance.

@zero-24
Copy link
Contributor

zero-24 commented Mar 1, 2020

I have tested this item ✅ successfully on e2fb85b

Works good here with the latest changes. I have 10.1.38-MariaDB installed and tested by changing the required versions. Thanks @richard67


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28135.

@richard67
Copy link
Member Author

Thanks @zero-24 .

@alikon
Copy link
Contributor

alikon commented Mar 4, 2020

I have tested this item ✅ successfully on e2fb85b

tested with postgresql (pdo) changing minimum version as per instruction


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28135.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.0 milestone Mar 4, 2020
@alikon
Copy link
Contributor

alikon commented Mar 4, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28135.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 4, 2020
@wilsonge wilsonge merged commit b453bcc into joomla:4.0-dev Mar 4, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 4, 2020
@wilsonge
Copy link
Contributor

wilsonge commented Mar 4, 2020

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone Mar 4, 2020
@richard67
Copy link
Member Author

Thanks fellows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants