Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Allow to specify port number or UNIX socket in host option also for MySQL (PDO) and PostgreSQL (PDO) #310

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

richard67
Copy link
Contributor

@richard67 richard67 commented Aug 31, 2024

Pull Request for CMS Issue joomla/joomla-cms#43902 .

Summary of Changes

This pull request (PR) makes it possible to specify a port number or UNIX socket in the host option for MySQL (PDO) and PostgreSQL (PDO) drivers in the same way as it is already possible for the MySQLi driver.

All these drivers already allow to specify a port number or a UNIX socket name with separate options.

But the CMS does not use these parameters. It does not have any fields in the configuration or installation form for that. Instead of that, it allows to append a port number to the host name separated by a colon, or to specify a UNIX socket with the host parameter.

The MySQLi driver contains code in its connect method to extract the port number or UNIX socket from the host parameter if that parameter contains one of these.

The MySQL (PDO) and PostgreSQL (PDO) drivers don't have such code.

It seems that for the MySQL (PDO) this is not a problem and it can handle such a host string as it is.

But the PostgreSQL (PDO) driver doesn't work with that, see the referred CMS issue.

This PR moves the code for extracting the port number or UNIX socket from the MySQLi driver's connect method to a new method in the base database driver so it can be used by any kind of database driver. The default port number is passed as a parameter to that method.

The PDO driver is changed so that it uses the new method for the MySQL (PDO) and PostgreSQL (PDO) drivers.

So it should work the same way for all database drivers used by the CMS: If the host option specifies a port number or UNIX socket, the port number or socket name from the host option are used (and override the corresponding parameters), otherwise port number or socket name from the options are used, if given, or the default port if nothing is given.

Testing Instructions

See CMS issue joomla/joomla-cms#43902 .

Documentation Changes Required

None.

Extract host and port number or socket name from host option for mysql and pgsql drivers
@alikon
Copy link
Contributor

alikon commented Sep 1, 2024

i've tested successfully with a non standard port with pgsql

@richard67
Copy link
Contributor Author

i've tested successfully with a non standard port with pgsql

@alikon Thanks a lot, that’s much appreciated. Could you also test with MySQLi and MySQL (PDO if that still works? That would be really great.

@alikon
Copy link
Contributor

alikon commented Sep 1, 2024

yes they still works like before with or without port, standard or not

@richard67
Copy link
Contributor Author

Thanks @alikon for testing.

Would be good if @muhme could test with his docker scenario and if the changes for the system tests proposed with CMS PR joomla/joomla-cms#43968 work with the changes from this PR here.

@muhme
Copy link

muhme commented Sep 1, 2024

First test with PostgreSQL (PDO) using host.docker.internal:7013 was ✅ 👍 😄 I'll continue testing ...

As this was my 1st time I tested Joomla Framework Database, here are the steps after cloning joomla-cms to remember:

rm -rf libraries/vendor/joomla/database
git submodule add -f https://github.com/joomla-framework/database.git libraries/vendor/joomla/database
cd libraries/vendor/joomla/database
gh pr checkout 310
cd -
composer install

@muhme
Copy link

muhme commented Sep 1, 2024

Edited: There is nothing wrong with #33 - only the hack for 33 in JBT is to fix.

Tested all five variants mariadb, mariadbi, mysql, mysqli and pgsql with Joomla 5.3-dev + this PR #310 + joomla-projects/joomla-cypress#33 + joomla/joomla-cms#43968 + hacked version of JBT which is using host:port for database connection successfully ✅ 👍 😄

  1. Verify Cypress based Joomla Web Installer is working, with switching database variant, e.g.
scripts/database.sh mysql
  1. Checking configuration.php e.g.
        public $dbtype = 'mysql';
        public $host = 'host.docker.internal:7011';
  1. Checking cypress.config.mjs e.g.
  env: {
    db_type: 'MySQL (PDO)',
    db_host: 'host.docker.internal',
    db_port: '7011',
  1. Running one System Test with Cypress custom database command, e.g. db_createUser():
scripts/test.sh administrator/components/com_users/Users.cy.js
  1. Double checking Joomla backend with Global Configuration | Server, e.g.
    Screenshot

Note: With this PR #310 we have host:port in Joomla backend, Web Installer and configuration.php. And with joomla-projects/joomla-cypress#33 + joomla/joomla-cms#43968 we have additional db_port field for the Cypress System Tests. This allows continue to work the System Tests' Cypress custom database commands with db_host.

I'll continue with IPv6 tests ...

@muhme
Copy link

muhme commented Sep 3, 2024

Continued testing in the environment above with an IPv4 address, the same five database variants tested with the five steps successfully ✅
IPv4

IPv6 needs an additional small change in joomla-cypress to add the square brackets, but hangs in Web installer, I have to dig deeper ...

@HLeithner HLeithner merged commit a82dc3c into joomla-framework:3.x-dev Sep 4, 2024
1 of 2 checks passed
@HLeithner
Copy link
Contributor

thanks

@richard67 richard67 deleted the 3.x-dev-portnumber branch September 4, 2024 17:34
@richard67
Copy link
Contributor Author

Thanks for merging and thanks to all testers and reviewers.

@muhme
Copy link

muhme commented Sep 7, 2024

I continued testing and as warm-up I tested simple IPv6 addresses for database host. Warming-up environment is 5.3-dev, without this #310 and with joomla-projects/joomla-cypress#33 (irrelevant, but already included) and joomla-projects/joomla-cypress#36 (mandataroy, new created for this test).

I found that square brackets are mandatory for IPv6 addresses when using MariaDB and MySQL (MySQLi or DBO) drivers, even without specifying a port. However, square brackets do not work with the PostgreSQL driver. The Joomla Web Installer simply passes this through without modification.

Tested successfully with joomla-projects/joomla-cypress#36 all five database variants by editing cypress.config.mjs:

  1. Cypress System Tests based Joomla installation (scripts/test.sh 53 install/Installation.cy.js novnc)
  2. checked $host entry in configuration.php file
  3. Running one System Test with Cypress custom database command db_createUser (scripts/test.sh administrator/components/com_users/Users.cy.js)
  4. Double checking Joomla backend with Global Configuration | Server, e.g.
    mysqli
    pgsql

Now, I'm going to test this PR #310 with IPv6 addresses only and IPv6 addresses with port number ...

@muhme
Copy link

muhme commented Sep 9, 2024

IPv6 addresses are working with PostgreSQL in installation step (PHP driver), but I have to correct myself, IPv6 addresses are not working with PostgreSQL in Cypress custom database command (with JavaScript module postgres). My solution proposal is to replace NPM module postgres with NPM module pg as prepared with https://github.com/muhme/joomla-cms/tree/pg-for-postgres branch, see muhme/joomla-cms@2e8dc3f commit.

@muhme
Copy link

muhme commented Sep 9, 2024

Still testing without this #310 PR – now IPv6 addresses with port numbers – to see the problem.

Test env is 5.3-dev + joomla-projects/joomla-cypress#33 (irrelevant, but already included) + joomla-projects/joomla-cypress#36 (mandatory, new created for this test) + using pg module. (To ensure not using default database server port running e.g. socat TCP6-LISTEN:4711,fork,reuseaddr TCP6:[fd00::5]:3306 one one different container).

  1. Cypress System Tests based Joomla installation (scripts/test.sh 53 install/Installation.cy.js novnc)
  2. checked $host entry in configuration.php file
  3. Running one System Test with Cypress custom database command db_createUser (scripts/test.sh administrator/components/com_users/Users.cy.js)
  4. Double checking Joomla backend with Global Configuration | Server, e.g.

Tested sucessfully MariaDB and MySQL (MySQLi and DBO) drivers with Joomla Web Installaler and System Tests (with the new pg module):
mysqli

System Tests for PostgreSQL with cypress.config.mjs configuration is running successfully:

    db_host: 'fd00::a',
    db_port: '4713',

PostgreSQL with Joomla Web Installer (PHP) is not running. Verified with and without square brackets :
pgsql
pgsql2

Next is to test this PR #310 with IPv6 addresses and port numbers ...

@muhme
Copy link

muhme commented Sep 10, 2024

I still have one more test without this #310 PR – now sockets. Test env JBT:

scripts/create.sh 53 IPv6 https://github.com/muhme/joomla-cms:pg-for-postgres
# loop with editing branch_53/cypress.config.mjs
scripts/test.sh 53 tests/System/integration/install/Installation.cy.js novnc
scripts/test.sh 53 tests/System/integration/administrator/components/com_users/Users.cy.js novnc

Web Installation fails for all five database variants:
Screenshot 2024-09-10 at 06 29 04
Screenshot 2024-09-10 at 06 17 17
Screenshot 2024-09-10 at 06 24 39

In Joomla web server container jbt_53 I can access the databases with command line clients, e.g.

mariadb --socket=/jbt/run/mariadb-socket/mysqld.sock -u root -p
psql -h /jbt/run/postgresql-socket/ -U root -d postgres

Also System Tests for four database variants are failing (after installation via host):

  getaddrinfo ENOTFOUND /jbt/run/mysql-socket/mysqld.sock
  getaddrinfo ENOTFOUND /jbt/run/mariadb-socket/mysqld.sock

The only one working is PostgreSQL (PDO) with the new pg module ✅ in the System Tests.

The results are poor; am I doing something wrong? Could someone please re-check one database variant with a Unix socket in their own installation using the Joomla Web Installer?

@muhme
Copy link

muhme commented Sep 11, 2024

am I doing something wrong?

Yes, needed format prefix for using sockets is unix:
socket

Therefore without this #310 PR – sockets are working for: mariadb ✅, mariadbi ✅, mysql ✅, mysqli ✅, pgsql (with today postgres driver) ✅ and pgsql (with new pg driver) ✅ in Joomla Web Installer running from Cypress System Tests.

But not working for System Tests with custom database commands (error e.g. getaddrinfo ENOTFOUND unix:/jbt/run/mariadb-socket/mysqld.sock) -> ignored for the moment.

So, I am now ready to test this PR 😅

@muhme
Copy link

muhme commented Sep 12, 2024

7 test cases x 5 database variants (mysqli, mysql, mariadbi, mariadb and pgsql) = 35 Joomla installations

Based on https://github.com/muhme/joomla-cms/tree/pg-for-postgres + this PR #310 + joomla-projects/joomla-cypress#33 + joomla-projects/joomla-cypress#36 + joomla/joomla-cms#43968

Running Joomla installation install/Installation.cy.js

  1. ✅ 5 x using hostname
  2. ✅ 5 x using hostname + non-standard port
  3. ✅ 5 x using IPv4 address
  4. ✅ 5 x using IPv4 address + non-standard port
  5. ✅ 5 x using IPv6 address (without brackets, e.g. fd00::3)
  6. ✅ 5 x using database Unix socket
  7. 🟥 using IPv6 address + non-standard port
  • fails for MySQL and MariaDB with hanging endless (> 30 minutes - I have to restart the container) in "Installation running" (see screenshot) -> which might be worth raising as a separate issue
    running

For PostgreSQL configuration:
config

There is an error:
error

Not using square brackets:
config2
error2

Verified from Joomla web server container, command line client access is possible:

jbt_53 # psql -h fd00::6 -p 4711 -U root -d test_joomla_53

Out of scope of this PR: parallel tested one spec with custom database commands administrator/components/com_users/Users.cy.js
✅ for the tests 1. – 5.
Untested for 7. using IPv6 address + non-standard port
🟥 failed for 6. using database Unix socket -> which might be worth raising as a separate issue

     CypressError: `cy.task('queryDB')` failed with the following error:

> getaddrinfo ENOTFOUND unix:/jbt/run/mysql-socket/mysqld.sock

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.

4 participants