PostgreSQL support - proposal n.1: without change in database schema

[xlii-chl]

Those patches add support for PostgreSQL database : they doesn't modify the database schema but, instead, forcibly convert the booleans attributes' values of the entity to integers (see the 2 setters).

Tests added to run on MySQL and PostgreSQL (on top of already running on SQLite). Since it seems that CodeIgniter 4.1.x sadly doesn't support PHP 7.3 anymore, I removed it from the tests' matrix.

Those patches also fix a couple of typos.

[MGatner]

CodeIgniter 4.1.x will still support PHP 7.3 but the develop branch has dropped it as prep for 4.2. If you want to run against 7.3 you will need to change the Composer dev requirement to codeigniter4/framework: ^4.1.

This looks like a good PR; I am surprised that a library needs modification to support a supported database driver though. I have no experience with Postgres - is this something that the framework is handling poorly? Or some niche case here?

[xlii-chl]

Thanks for reviewing it.
Sorry about the failed test: it seems to hang on some transient error in the translations. I couldn't debug it yet but it shouldn't be caused by the patch, maybe even not by MythAuth.

Glad to hear that PHP 7.3 will still be supported a little bit longer. This is often noted by big organizations which struggle to keep the pace of other frameworks.

About the problem, well... both boolean fields and PostgreSQL are kinda rarely used in the PHP ecosystem, so you have a choice between :

• using integers as booleans, but beware of inserting TRUE inside a integer field as MySQL will implicitly cast it, while the strongly typed PostgreSQL will throw an error,
• effectively using booleans, but some low-level library (PDO or libpq I guess) makes those fields come up like (string)f and (string)t with PostgreSQL, instead of (int)0 and (int)1 with MySQL and SQLite, which, everyonce in a while, comes to bite someone in the *** :)

Actually, CodeIgniter may have this kind of a bug (see #495 where I have to avoid 'f' begin cast as (bool)true). I'll dig it a little more and maybe open an issue there.
And by the way, I forgot to mention but there is an even simpler solution to #324 :

--- a/src/Controllers/AuthController.php
+++ b/src/Controllers/AuthController.php
@@ -345,7 +345,7 @@ public function attemptReset()
                $user->reset_hash               = null;
                $user->reset_at                 = date('Y-m-d H:i:s');
                $user->reset_expires    = null;
-               $user->force_pass_reset = false;
+               $user->force_pass_reset = 0;
                $users->save($user);
 
                return redirect()->route('login')->with('message', lang('Auth.resetSuccess'));

However, this pull-request and its setters may help the absent-minded developers :)
I'll let you decide which is best. Don't hesitate to ping me to adapt the pull-request.

[lonnieezell]

I'm not opposed to this change, but I'm surprised having those fields cast as booleans isn't already converting that. Been a long time since I've looked at that code in the model, though.

And a huge thank you for updating the tests to run on MySQL and Postgres also.

[xlii-chl]

Update on the failed tests: it seems to come from some other test calling Services::reset(). By calling Services::reset(true) in the failing class LocalAuthenticateValidateTest, the lang() function works correctly again.

Ponctual correction in f94f4c5 but a more global approach, covering all the test classes, may be preferable:

--- a/tests/_support/AuthTestCase.php
+++ b/tests/_support/AuthTestCase.php
@@ -7,6 +7,7 @@
 use Faker\Generator;
 use Myth\Auth\Authorization\GroupModel;
 use Myth\Auth\Authorization\PermissionModel;
+use Myth\Auth\Config\Services;
 use Myth\Auth\Entities\User;
 use Myth\Auth\Models\UserModel;
 use Myth\Auth\Test\AuthTestTrait;
@@ -67,6 +68,15 @@ class AuthTestCase extends CIUnitTestCase
         */
        protected $faker;
 
+       public static function setUpBeforeClass(): void
+       {
+               parent::setUpBeforeClass();
+
+               // Some tests call Services::reset() which, contrary to
+               // Services::reset(true), discards a lot of services.
+               Services::reset(true);
+       }
+
        protected function setUp(): void
        {
                parent::setUp();

Note to self: it was transient because we use executionOrder="random" in phpunit.xml. To reproduce more easily, pick the seed of a failed run and launch php -d xdebug.mode=coverage ./vendor/bin/phpunit --random-order-seed 1645919175

[MGatner]

@xlii-chl I think this one is still good, but I would like to run it again after #517. Are you around to rebase in the next few days?

[xlii-chl]

@xlii-chl I think this one is still good, but I would like to run it again after #517. Are you around to rebase in the next few days?

Rebased.