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

Database migration uses wrong database when initialising migration classes #1532

Closed
hwiesmann opened this issue Nov 24, 2018 · 4 comments
Closed
Labels
database Issues or pull requests that affect the database layer

Comments

@hwiesmann
Copy link


name: Bug report
about: Database migration uses wrong database when initialising migration classes (similar as #1531)


The following code crashes or is writing data into a completely different database under the assumption that the passed connection interface $db is not equivalent to a connection interface that is defined in any configuration file and therefore is not associated to any group:

$migrate = \Config\Services::migrations(null,$db); // migration configuration must allow/enable migration! $migrate->current($group); // the content of $group determines if the program crashes ($group does relate to a valid connection interface) or if data may be written to a wrong database ($group relates to a valid connection interface)

The root cause of this problem is this line in MigrationRunner.php version():

$instance = new $class(\Config\Database::forge($this->group));

The migration class is initialised with a wrong forge class object. The correct statement should be:

$instance = new $class(new \CodeIgniter\Database\Forge($this->db));

PS: It seems to be that in MigrationRunner the relation between database and group variables is not well separated (see also bug #1531).

CodeIgniter 4 version
CodeIgniter 4.0.0 Alpha 2

Affected module(s)
MigrationRunner.php

@jim-parry jim-parry added the database Issues or pull requests that affect the database layer label Dec 10, 2018
@lonnieezell
Copy link
Member

Your fix isn't quite right. Simply creating a new instance of the Forge class completely skips loading the database-specific forge class. The fix looks to be a little more complicated, but will get it fixed.

@hwiesmann
Copy link
Author

Sorry, I did not even know that there is a database-specific forge class.

But why do we have then a public (and not protected) constructor? It seems to be that it does not make sense to instantiate a non-database specific forge class anyway.

PS: It would be nice to have the inheritance diagram of classes somewhere listed in the documentation.

@lonnieezell
Copy link
Member

This should be fixed now with #1660 I believe. @hwiesmann can you check and verify for me please?

@hwiesmann
Copy link
Author

@lonnieezell seems to be fixed, works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Issues or pull requests that affect the database layer
Projects
None yet
Development

No branches or pull requests

3 participants