Problem/Motivation
The BC layer introduced in Drupal 9.4 to deal with old style database settings fails on database replicas. It triggers a warning:
Warning: Undefined array key "driver" in Drupal\Core\Site\Settings::initialize() (line 169 of /var/www/html/web/core/lib/Drupal/Core/Site/Settings.php).
Drupal\Core\Site\Settings::initialize('/var/www/html/web', 'sites/default', Object) (Line: 1065)
Drupal\Core\DrupalKernel->initializeSettings(Object) (Line: 699)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
The critical part is that drivers or namespace settings specifically for one or more replicas are ignored and a potentially required autoloading doesn't happen.
In addition the still existing Drupal 7 settings BC code is now dead code because the Drupal 8 BC code causes the condition to be always FALSE.
Steps to reproduce
Add a replica according to the docs and you'll get the warning. Add a custom driver for the replica and the autoloader will not be triggered.
Proposed resolution
Re-organize the code as it was in 9.3 and apply the BC logic after the replica settings have been parsed and converted.
Another advantage will be that database related code is back in Database.php instead of Settings.php.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#16 | interdiff.3294695.13-16.txt | 826 bytes | longwave |
#16 | 3294695-16-10.0.x.patch | 9.27 KB | longwave |
#16 | 3294695-16-9.5.x.patch | 9.21 KB | longwave |
#13 | 3294695-10-10.0.x.patch | 9.31 KB | longwave |
| |||
#9 | 3294695-9.patch | 9.35 KB | daffie |
Comments
Comment #2
mkalkbrennerComment #3
xjmComment #4
hkirsman CreditAttribution: hkirsman at Wunder for Trimble Solutions Corporation commentedThe patch worked for me.
Comment #5
pandaski CreditAttribution: pandaski commentedI confirmed this is raising the warning
$databases['default']['replica'][]
[warning] Undefined array key "driver" Settings.php:169
[warning] Undefined array key "namespace" Settings.php:187
I also tested the patch in the local environment, it can fix the issue.
Comment #6
catchThe patch looks fine, but is this something we could add a test for?
Comment #7
longwaveI think it should be possible to extend ExistingDrupal8StyleDatabaseConnectionInSettingsPhpTest to do this, though it would need to add a replica connection instead of just replacing existing settings.
Comment #8
daffie CreditAttribution: daffie at Finalist commentedAdded the testing to ExistingDrupal8StyleDatabaseConnectionInSettingsPhpTest.
The only change is the added testing.
Comment #9
daffie CreditAttribution: daffie at Finalist commentedStyle guide fix.
Comment #11
longwaveThank you @daffie.
Comment #12
catchLooks like we also need a 10.x version of the patch.
Comment #13
longwaveComment #14
longwaveComment #15
daffie CreditAttribution: daffie at Finalist commentedI made the test by mistake to only work for MySQL. It should also work for PostgreSQL and SQLite.
Comment #16
longwaveAddressed #15.
Comment #17
daffie CreditAttribution: daffie at Finalist commentedIt a bit strange to me that the test works for PostgreSQL and SQLite with the driver hardcoded to "mysql." Maybe we should change it. It also can go in as is, therefor for me it is RTBC.
Comment #19
catchCommitted/pushed/cherry-picked the respective patches to the respective branches for 10.1.x down to 9.4.x, thanks!
Comment #21
longwaveRe #17 oh I was so busy looking at the namespace line I didn't even see that! But it doesn't seem to matter so this will do for the test.
Comment #23
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI don't yet know why moving this line caused a regression for people updating from 9.4.6 to 9.4.7 or higher, but it appears to have.
With 9.4.8, if my $databases info does not contain an 'autoload' key and I run
drush status
, I get:I suspect the reason to be that the
class_exists($driver_class)
line ingetDriverClass()
fails due to the autoloader not containing the mysql module's directory, so therefore this falls back to the abstract Schema class that can't be instantiated.If I
patch -p1 -R < 3294695-16-9.5.x.patch
the patch in #16, thendrush status
works as expected. See also #3290924-24: [regression] With Drupal 9.4, can no longer call Database::getConnection() from within settings.php due to driver classes not yet in autoloader where this is being reported.I have not yet traced why Drush's bootstrap doesn't reach the code in the location that this issue moved it to.
Comment #24
effulgentsia CreditAttribution: effulgentsia at Acquia commentedThe reason is that the new location is hidden in an
if
block.Before, in Settings::initialize():
After, in Database::addConnectionInfo():
What this means is that if code within settings.php called
Database::addConnectionInfo()
orDatabase::setMultipleConnectionInfo()
first, without passing in $class_loader, then later when Settings::initialize() continues, prior to this issue it would invoke the ->addPsr4() method, but now thatif (empty(self::$databaseInfo[$key][$target])) {
stops that.That makes this regression a slightly different variant of #3290924: [regression] With Drupal 9.4, can no longer call Database::getConnection() from within settings.php due to driver classes not yet in autoloader and can be dealt with there rather than re-opening this issue or creating a new one.