Validation::run() using Database\MySQL\Connection

[ciadavid]

PHP Version

7.3

CodeIgniter4 Version

4.2.7

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows, Linux

Which server did you use?

apache

Database

Mysql and MariaDB

What happened?

Whe I try to run() validation using directly a Database\MySQLi\Connection y get Error not text|null parameter.

I want to send $validation->run($fieldsData,null,$this->db) because I get connections from database configuration, I send directly the connection, insted $dbGroup.

Validation works if I update the file vendor.....\system\Calidation\Validation.php line 109 (run() declaration), and remove ?string from $dbGroup = null, to allow other data, for example directly send Database|MySQLi\Connection.

Steps to Reproduce

configure 2 databases, directly, not use dbGroups.
Send validation run() and add a Database\MySQLi\Connnection insted $dbGroup.

If you delete ?string from validation::run() declaration of $dbGroup it works and not get error.

Expected Output

allow to validetion::run() to send Database\MySQLi\Connection and $dbGroups.

Anything else?

No response

[kenjis]

We use GitHub issues to track BUGS and to track approved DEVELOPMENT work packages. We use our forum to provide SUPPORT and to discuss FEATURE REQUESTS.

[michalsn]

Sadly, the suggested changes would cause the need to change the interface class (and we need really good reasons to do that), so personally, I don't think it's going to happen anytime soon.

I'm curious about the use case you presented. Most likely, after getting the connection information from the database table, you're connecting to only one additional database simultaneously.

I would consider creating a DB group like dynamic and filling in the necessary information for connection after getting them from the database (see the Settings package). Then you can use the database group dynamic for connection and in the validation class.

Another option would be to use a feature that already exists. Every connection (even dynamic) has a group name. With something like array_keys(Database::getConnections()) you would get the current list of groups. I'm sure you can filter out what you need and use it.

[kenjis]

It seems if we fix this, it is a breaking change.

If you delete ?string from validation::run() declaration of $dbGroup it works and not get error.

If it is true, maybe we would accept the change in 4.3 branch.

@michalsn $dbGroup does not exist in ValidationInterface::run().
But it seems we should add it. This is a bug in the interface.

P.S. Oh, the interface bug was fixed in 4.3.

[michalsn]

It seems if we fix this, it is a breaking change.

If you delete ?string from validation::run() declaration of $dbGroup it works and not get error.

If it is true, maybe we would accept the change in 4.3 branch.

Well... this is not a bug. Changing this would break other people's code if they extend the Validation class, and this can be a fairly common practice. Besides, there are ways to achieve what OP wants. My vote would go to "no".

@michalsn $dbGroup does not exist in ValidationInterface::run(). But it seems we should add it. This is a bug in the interface.

Lol... I'm so good at checking things.

P.S. Oh, the interface bug was fixed in 4.3.

That's a relief.

[kenjis]

@michalsn Thank you for your opinion.
Maybe I am too loose on BC changes, so more strict persons may need.

However, bug fixes may still be needed to make the current interface realistically usable.

[michalsn]

However, bug fixes may still be needed to make the current interface realistically usable.

What do you mean? Isn't this fixed in 4.3?

[kenjis]

ValidationInterface is fixed in 4.3.
I meant bugs may still remain in other interfaces.

[ciadavid]

Sorry but, if I look how is DBgroup used in Validation, I can see thet it is used at is_unique, is_not_unique.

If I follow the code I can see that DBGroup is used at Database:;connect that it allow to use string, array, instanceof BaseConnection...

I think it is not good to limit on Validation->run() that the dbgroup must be ?string, because how it is used it can be a instaceof BaseConnection.

is it not, true?

[kenjis]

@ciadavid Why don't you send a PR to 4.3 branch?
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

v4.3.0 will be released soon. Once released, ValidationInterface cannot be changed until the next major version.

[kenjis]

I think it is not good to limit on Validation->run() that the dbgroup must be ?string, because how it is used it can be a instaceof BaseConnection.

Yes.

But if we remove ?string for $dbGroup (also remove in the ValidationInterface) in 4.3, the existing class which extends Validation will break. So we cannot do it.

CodeIgniter4/system/Validation/Validation.php

Line 109 in 558db23

public function run(?array $data = null, ?string $group = null, ?string $dbGroup = null): bool