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.

Sign up for GitHub

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

Jump to bottom

Backport the SQLite rewrite from the standalone plugin #677

Closed
wants to merge 19 commits into from
Closed

Backport the SQLite rewrite from the standalone plugin #677

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
590a89a
Backport the SQLite rewrite
aristath Mar 16, 2023
93ae4dc
Update modules/database/sqlite/db.copy.php
aristath Mar 20, 2023
64df26d
backport more changes
aristath Apr 5, 2023
1bc9ee0
Update "package" file docs
aristath Apr 10, 2023
26f72ab
Update since docs
aristath Apr 10, 2023
073ef34
fix more since docs
aristath Apr 10, 2023
8ad6a79
add back module headers
aristath Apr 10, 2023
4652949
Add back the can-load file
aristath Apr 10, 2023
68ec7a1
add fallbacks for constants
aristath Apr 10, 2023
5cfa86d
more backward-compatibility fixes
aristath Apr 10, 2023
8948766
Plugin is no longer based on Kojima Toshiyasu's implementation
aristath Apr 10, 2023
6489f32
remove more file headers which no longer apply
aristath Apr 10, 2023
ac0dad1
Fix activation/deactivation
aristath Apr 10, 2023
d7e7538
Revert plugin changes
aristath Apr 10, 2023
35abe35
rename file to what it used to be
aristath Apr 10, 2023
560f4df
remove debug-crosscheck
aristath Apr 10, 2023
710eddf
Update modules/database/sqlite/constants.php
aristath Apr 11, 2023
8dc116f
Update modules/database/sqlite/can-load.php
aristath Apr 11, 2023
5e6d474
Merge branch 'trunk' into sqlite-rewrite-backport
felixarntz Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions admin/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function perflab_render_modules_page_field( $module_slug, $module_data, $module_
<?php
if ( $is_standalone_plugin_loaded ) {
esc_html_e( 'The module cannot be managed with Performance Lab since it is already active as a standalone plugin.', 'performance-lab' );
} elseif ( 'database/sqlite' === $module_slug && file_exists( WP_CONTENT_DIR . '/db.php' ) && ! defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) ) {
} elseif ( 'database/sqlite' === $module_slug && file_exists( WP_CONTENT_DIR . '/db.php' ) && ! defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) && ! defined( 'SQLITE_DB_DROPIN_VERSION' ) ) {
printf(
/* translators: %s: db.php drop-in path */
esc_html__( 'The SQLite module cannot be activated because a different %s drop-in already exists.', 'performance-lab' ),
Expand Down Expand Up @@ -192,7 +192,7 @@ function perflab_render_modules_page_field( $module_slug, $module_data, $module_
</p>
<?php if ( 'database/sqlite' === $module_slug ) : ?>
<?php if ( $enabled ) : ?>
<?php if ( defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) ) : ?>
<?php if ( defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) || defined( 'SQLITE_DB_DROPIN_VERSION' ) ) : ?>
<?php // Don't use the WP notice classes here, as that makes them move to the top of the page. ?>
<p class="notice notice-warning" style="padding:1em;max-width:50em;">
<?php esc_html_e( 'Your site is currently using an SQLite database. You can disable this module to get back to your previous MySQL database, with all your previous data intact.', 'performance-lab' ); ?>
Expand Down
5 changes: 3 additions & 2 deletions modules/database/sqlite/can-load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
*/
return function() {

// If the PERFLAB_SQLITE_DB_DROPIN_VERSION constant is defined, then the module is already active.
if ( defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) ) {
// If the PERFLAB_SQLITE_DB_DROPIN_VERSION or SQLITE_DB_DROPIN_VERSION constants
aristath marked this conversation as resolved.
Show resolved Hide resolved
// are defined, then the module is already active.
if ( defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) || defined( 'SQLITE_DB_DROPIN_VERSION' ) ) {
return true;
}

Expand Down
19 changes: 15 additions & 4 deletions modules/database/sqlite/constants.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,23 @@
* @package performance-lab
*/

// Backwards-compatibility.
if ( ! defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) && defined( 'SQLITE_DB_DROPIN_VERSION' ) ) {
define( 'PERFLAB_SQLITE_DB_DROPIN_VERSION', SQLITE_DB_DROPIN_VERSION );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my other comment, we also need to ensure backward compatibility the other direction, especially as I'm assuming SQLITE_DB_DROPIN_VERSION will be our main constant going forward.

Suggested change
}
}
if ( ! defined( 'SQLITE_DB_DROPIN_VERSION' ) && defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) ) {
define( 'SQLITE_DB_DROPIN_VERSION', PERFLAB_SQLITE_DB_DROPIN_VERSION );
}


// Temporary - This will be in wp-config.php once SQLite is merged in Core.
if ( ! defined( 'DATABASE_TYPE' ) ) {
if ( defined( 'PERFLAB_SQLITE_DB_DROPIN_VERSION' ) ) {
define( 'DATABASE_TYPE', 'sqlite' );
if ( ! defined( 'DB_ENGINE' ) ) {
if ( defined( 'SQLITE_DB_DROPIN_VERSION' ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the code above in line 10-12, we would need to use PERFLAB_SQLITE_DB_DROPIN_VERSION instead, as above you set that constant based on the other one. Which one is considered the source of truth? Given the standalone plugin uses SQLITE_DB_DROPIN_VERSION, we should probably use that.

define( 'DB_ENGINE', 'sqlite' );
} elseif ( defined( 'DATABASE_ENGINE' ) ) {
// backwards compatibility with previous versions of the standalone plugin.
define( 'DB_ENGINE', DATABASE_ENGINE );
} elseif ( defined( 'DATABASE_TYPE' ) ) {
// backwards compatibility with previous versions of the performance-lab plugin.
aristath marked this conversation as resolved.
Show resolved Hide resolved
define( 'DB_ENGINE', DATABASE_TYPE );
} else {
define( 'DATABASE_TYPE', 'mysql' );
define( 'DB_ENGINE', 'mysql' );
}
}

Expand Down
50 changes: 32 additions & 18 deletions modules/database/sqlite/site-health.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,34 @@
<?php
/**
* Adds and filters data in the site-health screen.
* Tweaks for the health-check screens.
*
* @package performance-lab
* @since 1.8.0
* @package performance-lab
*/

// Require the constants file.
require_once __DIR__ . '/constants.php';

/**
* Filter debug data in site-health screen.
*
* When the plugin gets merged in wp-core, these should be merged in src/wp-admin/includes/class-wp-debug-data.php
* See https://github.com/WordPress/wordpress-develop/pull/3220/files
*
* @since 1.8.0
*
* @param array $info The debug data.
* @return array The filtered debug data.
*/
function perflab_sqlite_plugin_filter_debug_data( $info ) {
$database_type = defined( 'DATABASE_TYPE' ) && 'sqlite' === DATABASE_TYPE ? 'sqlite' : 'mysql';
function sqlite_plugin_filter_debug_data( $info ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the doc block here? Not having a doc block is worse than having one :)

Additionally, we should be careful about prefixing our code properly. sqlite_plugin_ works well as a prefix, since that makes it very clear it's for the plugin and won't conflict when that code is eventually part of WordPress core. But we shouldn't use just sqlite_ or wp_sqlite_, because those are at risk for conflicts.

$db_engine = defined( 'DB_ENGINE' ) && 'sqlite' === DB_ENGINE ? 'sqlite' : 'mysql';

$info['wp-constants']['fields']['DATABASE_TYPE'] = array(
'label' => 'DATABASE_TYPE',
'value' => ( defined( 'DATABASE_TYPE' ) ? DATABASE_TYPE : __( 'Undefined', 'performance-lab' ) ),
'debug' => ( defined( 'DATABASE_TYPE' ) ? DATABASE_TYPE : 'undefined' ),
$info['wp-constants']['fields']['DB_ENGINE'] = array(
'label' => 'DB_ENGINE',
'value' => ( defined( 'DB_ENGINE' ) ? DB_ENGINE : __( 'Undefined', 'performance-lab' ) ),
'debug' => ( defined( 'DB_ENGINE' ) ? DB_ENGINE : 'undefined' ),
);

$info['wp-database']['fields']['database_type'] = array(
$info['wp-database']['fields']['db_engine'] = array(
'label' => __( 'Database type', 'performance-lab' ),
'value' => 'sqlite' === $database_type ? 'SQLite' : 'MySQL/MariaDB',
'value' => 'sqlite' === $db_engine ? 'SQLite' : 'MySQL/MariaDB',
);

if ( 'sqlite' === $database_type ) {
if ( 'sqlite' === $db_engine ) {
$info['wp-database']['fields']['database_version'] = array(
'label' => __( 'SQLite version', 'performance-lab' ),
'value' => class_exists( 'SQLite3' ) ? SQLite3::version()['versionString'] : null,
Expand Down Expand Up @@ -65,4 +59,24 @@ function perflab_sqlite_plugin_filter_debug_data( $info ) {

return $info;
}
add_filter( 'debug_information', 'perflab_sqlite_plugin_filter_debug_data' ); // Filter debug data in site-health screen.
add_filter( 'debug_information', 'sqlite_plugin_filter_debug_data' ); // Filter debug data in site-health screen.

/**
* Filter site_status tests in site-health screen.
*
* When the plugin gets merged in wp-core, these should be merged in src/wp-admin/includes/class-wp-site-health.php
*
* @param array $tests The tests.
* @return array
*/
function sqlite_plugin_filter_site_status_tests( $tests ) {
$db_engine = defined( 'DB_ENGINE' ) && 'sqlite' === DB_ENGINE ? 'sqlite' : 'mysql';

if ( 'sqlite' === $db_engine ) {
unset( $tests['direct']['utf8mb4_support'] );
unset( $tests['direct']['sql_server'] );
}

return $tests;
}
add_filter( 'site_status_tests', 'sqlite_plugin_filter_site_status_tests' );
Loading