[3.3.6] Problem with Database::getInstance()

[Zeromax]

There is a Problem with the Database::getInstance() when the config is not complete (no SQL settings)

This Error occured while trying to install MetaModels in a blank Contao with no localconfig.php.

Fatal error: Class 'Database\' not found in D:\www\metamodels-contao\system\modules\core\library\Contao\Database.php on line 161
Call Stack
#   Time    Memory  Function    Location
1   0.2100  348928  {main}( )   ..\install.php:0
2   0.2114  385264  require( 'D:\www\metamodels-contao\system\initialize.php' ) ..\install.php:24
3   0.3742  2036720 MetaModels\BackendIntegration\Boot::perform( )  ..\initialize.php:244
4   0.3745  2037240 MetaModels\BackendIntegration\Boot::initializeContaoObjectStack( )  ..\Boot.php:160
5   0.3765  2037360 MetaModels\BackendIntegration\Boot::getUser( )  ..\Boot.php:72
6   0.3797  2259864 Contao\User::getInstance( ) ..\Boot.php:45
7   0.3798  2260656 Contao\BackendUser->__construct( )  ..\User.php:151
8   0.3798  2260672 Contao\User->__construct( ) ..\BackendUser.php:103
9   0.3812  2276664 Contao\System->import( ???, ???, ??? )  ..\User.php:89
10  0.3826  2354040 call_user_func ( ??? )  ..\System.php:122
11  0.3826  2354192 Contao\Database::getInstance( ??? ) ..\System.php:122

See this line:

core/system/modules/core/library/Contao/Database.php

Line 160 in ac32cb4

$strClass = 'Database\\' . str_replace(' ', '_', ucwords(str_replace('_', ' ', strtolower($arrConfig['dbDriver']))));

$strClass = 'Database\\' . str_replace(' ', '_', ucwords(str_replace('_', ' ', strtolower($arrConfig['dbDriver']))));

There will be instanciate a none existing Class Database\ if the $arrConfig['dbDriver'] is not set.

Metamodels Fix: MetaModels/core@ac126a0

[Zeromax]

Same with a clean 3.3.6 installation -> no modules, only the default core modules:

Fatal error: Class 'Database\' not found in D:\www\metamodels-contao\system\modules\core\library\Contao\Database.php on line 161
Call Stack
#   Time    Memory  Function    Location
1   0.0015  200064  {main}( )   ..\index.php:0
2   0.0484  1563440 Index->__construct( )   ..\index.php:443
3   0.0508  1645736 Contao\System->import( )    ..\index.php:47
4   0.0539  1819568 call_user_func ( )  ..\System.php:122
5   0.0539  1819608 Contao\User::getInstance( ) ..\System.php:122
6   0.0540  1820640 Contao\FrontendUser->__construct( ) ..\User.php:151
7   0.0540  1820656 Contao\User->__construct( ) ..\FrontendUser.php:79
8   0.0546  1836656 Contao\System->import( )    ..\User.php:89
9   0.0561  1914048 call_user_func ( )  ..\System.php:122
10  0.0561  1914200 Contao\Database::getInstance( ) ..\System.php:122

[aschempp]

Why would we need a workaround for that? Just run the install tool?

[discordier]

Andreas, the problem is that this is not catch able as it is a fatal error.
The install tool has a check if there is a driver set and ignores it if not. The config bootstrapping is however continued and this imposes to perform the very same check in all extensions that are using the DB, the user class or anything else that retrieves the DB. Moving the check into the database and throwing an exception when the db is not configured would allow developers to catch the problem instead of always dying with a fatal error.
As you can see in the stack trace, he IS trying to use the install tool which is dying and rendering the installation unusable.

[aschempp]

Are you using the initializeSystem hook in MM? Maybe it should not be run if the installation is not yet successful?

[discordier]

Yes I am. However I strictly am against trying to work around a bug by cutting functionality. It is a bug to raise a fatal error instead of an exception from which can be recovered from.

[aschempp]

Not saying you’re wrong, but the standalone install tool will solve this problem in Contao 4 …=

[discordier]

Yep, I know. But waiting until Contao 4 is lts for such a problem to disappear is a bit harsh. ;)

[aschempp]

Yep, I know. But waiting until Contao 4 is lts for such a problem to disappear is a bit harsh. ;)

Absolutly

I strictly am against trying to work around a bug by cutting functionality

I just thought removing the hook would fix this and potentially other issues, what would be your solution?

[Zeromax]

Just for a further explanation, that the core itself can not handle a incomplete installation:

1. Remove the localconfig.php -> or use a blank installation
2. call the Frontend -> get the message pleas install Contao
3. call the install tool and acceppt the license -> stop here
4. call the frontend or backend -> Fatal Error

Removing the initilize Hook is not a solution.
There is a Config::isComplete() method. But this method is useless, because it returns true, if the config is incomplete. Or am I getting something wrong?

[aschempp]

It just checks if the localconfig file is there.

[Zeromax]

So the method naming is wrong ;) or the config file should not be created, when I accept the license.
If there is a localconfig.php, it does not mean, that the config is complete.

Isn't there a minimum Config Setup, which is neccesary to get Contao running? Some DB Settings, license, encryptionKey, etc. If they aren't set, the Installation is not complete. Or do i missunderstand something?
IMHO this definition can be part of the Config::isComplete() method.

[leofeyer]

Please try replacing line 158 of system/modules/core/library/Database.php with:

if (!isset(static::$arrInstances[$strKey]) && !empty($arrConfig['dbDriver']))

It does not fix the underlying problem (which will only be fixed with a stand-alone installer), but it improves the situation.

[leofeyer]

Of course, we could also check for a database driver in System::isComplete(), but that is not better a fix than the one above IMHO.

[Zeromax]

@leofeyer your Suggestion fixes the error.

IMHO than better remove the function Config::isComplete(), because it does only check if there is a localconfig, like @aschempp already said.
I think the best way for know is to check a minimum Needed Config Setup, to get Contao running. And if those are set, then the config is complete.

[aschempp]

Even if a DB driver is selected, that does not mean Contao can run (e.g. incorrect DB access details)=

[Zeromax]

Those are the config params from my localconfig after a clean installation:

$GLOBALS['TL_CONFIG']['licenseAccepted'] = true;
$GLOBALS['TL_CONFIG']['installPassword'] = 'somepasswordhash';
$GLOBALS['TL_CONFIG']['encryptionKey'] = 'somekey';
$GLOBALS['TL_CONFIG']['dbDriver'] = 'MySQLi';
$GLOBALS['TL_CONFIG']['dbHost'] = 'localhost';
$GLOBALS['TL_CONFIG']['dbUser'] = 'root';
$GLOBALS['TL_CONFIG']['dbPass'] = ''; // important, could be empty ;)
$GLOBALS['TL_CONFIG']['dbDatabase'] = 'contao-3.3.5';
$GLOBALS['TL_CONFIG']['dbPconnect'] = false;
$GLOBALS['TL_CONFIG']['dbCharset'] = 'UTF8';
$GLOBALS['TL_CONFIG']['dbPort'] = 3306;
$GLOBALS['TL_CONFIG']['dbSocket'] = ''; // important, could be empty ;)
$GLOBALS['TL_CONFIG']['adminEmail'] = '[email protected]';

If all of these options are set, the config is complete and Contao is able to run.
Not proofed if the Config Setting it self does work ;).

Maybe with contao 4 a stand-alone installer is comming ;)

[leofeyer]

That's wrong. Most of the database parameters (e.g. dbUser or dbDatabase) can actually be empty, therefore we must not check these. Also, it does not make sense to check for values which are defined in the default configuration (e.g. dbCharset or dbPort).

If we want to add some checks to the Config::isComplete() method, we should only check the following:

$GLOBALS['TL_CONFIG']['licenseAccepted'] === true;
$GLOBALS['TL_CONFIG']['encryptionKey'] !== '';
$GLOBALS['TL_CONFIG']['dbDriver'] !== '';
$GLOBALS['TL_CONFIG']['adminEmail'] !== '';

@contao/developers Does this make sense to you?

[leofeyer]

Changed in 108d724. We must not check for adminEmail either, because if someone imports a theme, they are likely to have an admin user without ever having to have to enter an admin e-mail address.

[Zeromax]

@leofeyer sounds perfect. thanks ;)