Add a hotfix for missing DB_NAME constant

[JanJakes]

A hotfix for:

Fatal error: Uncaught Error: Undefined constant "DB_NAME" in /var/www/html/wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/db.php:64 Stack trace: #0 /var/www/html/wp-content/db.php(37): require_once() #1 /var/www/html/wp-includes/load.php(683): require_once('/var/www/html/w...') #2 /var/www/html/wp-settings.php(133): require_wp_db() #3 /var/www/html/wp-config.php(85): require_once('/var/www/html/w...') #4 /var/www/html/wp-load.php(50): require_once('/var/www/html/w...') #5 /var/www/html/wp-blog-header.php(13): require_once('/var/www/html/w...') #6 /var/www/html/index.php(17): require('/var/www/html/w...') #7 {main} thrown in /var/www/html/wp-content/mu-plugins/sqlite-database-integration/wp-includes/sqlite/db.php on line 64

This error occurs when importing a site backup in Studio that doesn't define the DB_NAME constant.

The ideal solution is to address this in Playground, but in the meantime, we can use a hotfix.

Fixes #40.

[bgrgicak]

Should we just make this change in Playground in that case and skip adding temporary solutions to the SQLite plugin?

[JanJakes]

@bgrgicak I'm not sure what that will involve. Do you think it's a small and quick fix? I'm not yet familiar enough with the Playground codebase to have an idea of what needs to be done — do we do something similar in Playground already? Will it reuse a Blueprint step logic? Etc.

[bgrgicak]

It should be quick to do. We can update the bootWordPress function and define the constant if it's missing, similarly to how we add the WP_HOME constant.

Or we could add it to preloadSqliteIntegration, where we define SQLITE_MAIN_FILE.

[adamziel]

Agreed with Bero. It would be something like this in boot.ts:

	php.defineConstant('WP_HOME', options.siteUrl);
	php.defineConstant('WP_SITEURL', options.siteUrl);

	// Run "before database" hooks to mount/copy more files in
	if (options.hooks?.beforeDatabaseSetup) {
		await options.hooks.beforeDatabaseSetup(php);
	}

	// @TODO Assert WordPress core files are in place

	if (options.sqliteIntegrationPluginZip) {
+		if ( ! isConstantDefined('DB_NAME') ) {
+			php.defineConstant('DB_NAME', 'wordpress');
+		}
		await preloadSqliteIntegration(
			php,
			await options.sqliteIntegrationPluginZip
		);
	}

We don't have the isConstantDefined implementation. I don't think we can boot WP at this point yet and just call defined( "DB_NAME" ) so we might need to statically analyze wp-config.php. Here's one way to do it.

[adamziel]

Ah, we already have some static analysis PHP code in the Playground repo, we might need to refactor it to also have a separate function just for checking the constant's presence:

https://github.com/Automattic/wordpress-playground-private/blob/53d691faa17193c49a40aa2a747f032c505d34f2/packages/playground/blueprints/src/lib/steps/rewrite-wp-config-to-define-constants.php#L70

[adamziel]

@JanJakes we would need to check which constants are already defined find and only add the DB_NAME constant if it's not already a part of wp-config.php. rewrite_wp_config_to_define_constants has all the relevant logic in it, it just doesn't do the thing we need here. It seems like we need to split out get_constants_defined_in_wp_config( $code ) or is_constant_defined_in_wp_config( $code, $constant ).

An important aspect to consider is what will happen when we backup/export the site. Which solution would include the DB_NAME correctly set?

The only way to include the DB_NAME with the site export seems to be updating the wp-config.php.

[JanJakes]

@adamziel What if the wp-config.php already contains some conditional for defining the constant:

if ( ! getenv( ... ) ) {
    define( DB_NAME, ... );
}

Would is_constant_defined_in_wp_config return true or false, and how would we rewrite it? I guess we could assume the likely 99% scenario, and just say "if there's any define( DB_NAME, ... ) anywhere in the file, we don't inject it, otherwise we do".

[adamziel]

At first, I was tempted to say we could cover those cases with if(!defined('DB_NAME', ...)) before require ABSPATH . "wp-settings.php";. I wonder if that's true, though. If a site only defines a DB_NAME conditionally, they seem to have a very specific intention for what they want to do, e.g. run multiple WordPress instances from a single directory. Adding another definition would likely collide with that intent. I'd say, in those cases, we should just bale, throw a meaningful error, and let them handle it.

[JanJakes]

@adamziel I agree. To summarize, I think we should only inject the default DB_NAME value if there is no define( 'DB_NAME', ... ) at all in the config.

[JanJakes]

Let's continue the discussion in https://github.com/Automattic/wordpress-playground-private/pull/140.

[adamziel]

Actually, why do we need to require DB_NAME in SQLite?

[adamziel]

Nevermind, I just read the comment and immediately forgot what it said 🤦

avoid issues such as storing an incorrect database name in the information schema.

[JanJakes]

Closing in favor of https://github.com/Automattic/wordpress-playground-private/pull/140.