Repository navigation
Add support for the new SQLite storage - #36
Conversation
Test MySQL backslash escapes with the default SQL mode, since the current driver explicitly rejects NO_BACKSLASH_ESCAPES. Keep assertions for every escape case and expect literal backslashes in quoted identifiers.
8b5567d to
70383ff
Compare
Match the PHPCompatibility target to the existing Composer requirement. The PHP 5.6 target incorrectly rejects supported features such as Throwable and void return types.
1a73811 to
5824b36
Compare
5824b36 to
539df2f
Compare
cf150e4 to
c5b3b7f
Compare
c5b3b7f to
976d605
Compare
Resolve DB_PATH, db-path.php, or a legacy .ht.sqlite database at runtime, retaining FQDB compatibility for older integration plugin versions. Export and table listing check that the database exists before opening it. Import uses the configured location and initializes secret-path storage only when no location has been recorded. Report invalid plugin database settings as command errors. Keep storage initialization out of the plugin loader and cover the storage behavior with Behat scenarios. WordPress/sqlite-database-integration#502 WordPress/sqlite-database-integration#512
An in-memory database is discarded when the command exits. Import reported success without keeping any data, and export and table listing reported a missing database.
976d605 to
5c08fcf
Compare
|
As part of reviewing this, I had Claude tell me what it thinks, and I'll have it post here because some its points sound reasonable. |
brandonpayton
left a comment
There was a problem hiding this comment.
🤖 This review was written by Claude (Claude Code), posted at @brandonpayton's request.
The direction is good. The resolution order (DB_PATH → FQDB → db-path.php → .ht.sqlite → initialize on import) makes sense, read commands no longer create storage as a side effect, and features/sqlite-storage.feature covers a lot of ground.
The main risk is that get_database_path() re-implements path resolution the plugin already owns, and a few edge cases come out differently. Details are in the inline comments. Roughly in priority order:
- The legacy
WP_SQLite_Translatorbranch ignores the resolved$database_path. - Import initializes storage in the constructor, before the dump file is opened.
- Exceptions from driver construction aren't wrapped in
WP_CLI::error. - A relative path from
db-path.phpresolves against the working directory, notFQDBDIR. - Smaller items:
DB_FILEis ignored,DB_PATHtakes precedence even on older plugins, the test helper readsFQDBwithout adefined()guard, andNO_BACKSLASH_ESCAPEScoverage was removed.
Longer term: a read-only "resolve without initializing" method on WP_SQLite_Storage would let the CLI drop its own copy of the resolution logic, so it can't drift from the plugin. That would make items 1, 4, and most of 5 go away.
Minor: AGENTS.md asks for an add/, update/ or fix/ branch prefix and for commits to reference a PR or issue number. Neither is a blocker.
Opening the database can fail, for example when the database directory does not exist. Show the reason as a WP-CLI error instead of an uncaught exception with a stack trace.
A missing or unreadable dump file no longer creates or initializes the database before the import fails.
|
Thanks for the review, @brandonpayton! I forgot to mention the bigger plan to move this repo into the SQLite integration so that import/export and the plugin can ship together, and all this cross-version logic can go away. Here, I just need it to work for the 3.1 release. I made a few improvements based on your feedback and replied to the rest. |
brandonpayton
left a comment
There was a problem hiding this comment.
@JanJakes I left one question, but this reads well to me.
|
Thanks for the thorough review! I'll go ahead and merge this one to prepare the 3.1 release. I will also try to make the migration of this repo to https://github.com/WordPress/sqlite-database-integration/ happen soon. |
Summary
Add support for the SQLite integration plugin's new database storage while keeping compatibility with older plugin releases. The commands now find the same database that WordPress uses:
DB_PATH, the legacyFQDBsetting, the recorded secret path (db-path.php), or an existing.ht.sqlitefile, in that order..ht.sqlitefile is used as is. WordPress moves it to the new storage on its next load.Behat covers the storage behavior. Separate commits correct the existing import fixtures for the current driver and align PHP compatibility checks with the project's PHP 7.4 requirement.
Why
These commands run before WordPress loads its database drop-in. With the new storage,
FQDBis no longer always defined at that point, so the commands need to find the configured database themselves.Testing
CI runs against the released plugin, which doesn't include the new storage yet.
Related: WordPress/sqlite-database-integration#502, WordPress/sqlite-database-integration#512