fix: close QA P1 findings on auto-schema-upgrade (#4)

Closes three P1 findings from the QA baseline review of a8a0ce2:
- uninstall.php deletes psa_db_version
- PSA_Upgrade class with unit tests (is_needed is pure, maybe_run is the driver)
- ARCHITECTURE.md documents the schema-versioning mechanism

Also extracts the logic into includes/class-psa-upgrade.php so it is cleanly testable without bootstrapping the whole plugin file.

Agent-Session: cowork-qa-p1-fixes-20260414
This commit is contained in:
peptiderepo
2026-04-14 20:47:30 +07:00
committed by GitHub
parent 3820bdfd28
commit 7587a1cebe
6 changed files with 190 additions and 24 deletions

View File

@@ -356,6 +356,22 @@ $psa_settings = array(
);
```
### Schema versioning & automatic upgrades
The plugin stores the schema version it last migrated to in a separate option, `psa_db_version`. This is distinct from `PSA_VERSION` (the running code's semver), and lets the plugin detect whether the database needs to catch up after an in-place file update.
- **Option name:** `PSA_Upgrade::OPTION_NAME` (`psa_db_version`), string, semver.
- **Default:** `PSA_Upgrade::DEFAULT_STORED_VERSION` (`'0.0.0'`) when the option has never been written — strictly less than any released version so fresh installs always trigger the upgrade path.
- **Driver:** `PSA_Upgrade::maybe_run()` in `includes/class-psa-upgrade.php`, called on every `admin_init` from `psa_admin_init()`. The method:
1. Reads `psa_db_version`.
2. Calls the pure helper `PSA_Upgrade::is_needed( $stored, PSA_VERSION )`, a thin wrapper over `version_compare` (stored < current → upgrade).
3. If an upgrade is needed, re-runs `PSA_Cost_Tracker::create_table()` so `dbDelta` picks up any additive schema changes (new columns, new indexes).
4. Writes `update_option( 'psa_db_version', PSA_VERSION, false )` so the upgrade doesn't re-run on the next request.
- **Why on `admin_init`:** dbDelta is safe-to-repeat but not free. Running it once when an admin visits after an update — rather than on every frontend page load — keeps the cost near zero without requiring users to deactivate/reactivate the plugin.
- **Downgrade / rollback:** if the running code is older than `psa_db_version` (e.g., deploy rolled back), `is_needed()` returns false and the driver is a no-op. The newer schema is left intact; old code continues to work against it as long as changes are additive.
- **Cleanup:** `uninstall.php` deletes `psa_db_version` alongside the other plugin options.
- **Testability:** `PSA_Upgrade::is_needed()` is a pure static method (no WordPress API calls) and is covered by `tests/unit/UpgradeTest.php` across fresh-install, same-version, future-version, and older-version scenarios, plus constant-stability checks on `OPTION_NAME` and `DEFAULT_STORED_VERSION`.
## Frontend Architecture
### JavaScript (jQuery-based)

View File

@@ -0,0 +1,83 @@
<?php
/**
* Schema upgrade helpers for Peptide Search AI.
*
* What: Pure-function helpers that decide whether a database schema upgrade
* is required, plus the driver that runs the upgrade when the installed
* code version is newer than the stored psa_db_version option.
*
* Who triggers it: psa_admin_init() in peptide-search-ai.php calls
* PSA_Upgrade::maybe_run() on every WordPress admin_init hook.
*
* Dependencies: PSA_Cost_Tracker (for re-running dbDelta on the
* cost-tracker table), WordPress options API (get_option / update_option),
* and the PSA_VERSION constant defined in peptide-search-ai.php.
*
* @package PeptideSearchAI
*/
declare( strict_types=1 );
if ( ! defined( 'ABSPATH' ) ) {
exit;
}
/**
* PSA_Upgrade — thin wrapper around version_compare + dbDelta.
*
* The class exists so the decision logic (is_needed) can be unit-tested as a
* pure function, while the impure driver (maybe_run) stays isolated.
*/
class PSA_Upgrade {
/**
* Option name used to persist the schema version last migrated to.
*
* @var string
*/
const OPTION_NAME = 'psa_db_version';
/**
* Default stored version for fresh installs. Strictly less than any
* real released version so version_compare treats it as "older".
*
* @var string
*/
const DEFAULT_STORED_VERSION = '0.0.0';
/**
* Decide whether a schema upgrade is needed given a stored version and
* the currently-installed code version. Pure function — no side effects,
* no WordPress API calls — so it can be exercised in isolation.
*
* @param string $stored_version Version string recorded in psa_db_version.
* @param string $current_version Version string of the running code.
* @return bool True if an upgrade must run; false otherwise.
*/
public static function is_needed( string $stored_version, string $current_version ): bool {
return version_compare( $stored_version, $current_version, '<' );
}
/**
* Run database upgrades when the installed code version is newer than
* the stored psa_db_version option. Keeps users from needing to
* deactivate/reactivate the plugin after a schema change.
*
* Side effects: reads psa_db_version option; on upgrade, calls
* PSA_Cost_Tracker::create_table() and writes psa_db_version.
*
* @return void
*/
public static function maybe_run(): void {
$stored = get_option( self::OPTION_NAME, self::DEFAULT_STORED_VERSION );
if ( ! self::is_needed( $stored, PSA_VERSION ) ) {
return;
}
// Re-run CREATE TABLE via dbDelta — additive schema changes (new
// columns, new indexes) are applied automatically.
PSA_Cost_Tracker::create_table();
update_option( self::OPTION_NAME, PSA_VERSION, false );
}
}

View File

@@ -33,6 +33,7 @@ require_once PSA_PLUGIN_DIR . 'includes/class-psa-admin.php';
require_once PSA_PLUGIN_DIR . 'includes/class-psa-batch-enrichment.php';
require_once PSA_PLUGIN_DIR . 'includes/class-psa-template.php';
require_once PSA_PLUGIN_DIR . 'includes/class-psa-directory.php';
require_once PSA_PLUGIN_DIR . 'includes/class-psa-upgrade.php';
function psa_init() {
PSA_Post_Type::register_peptide_post_type();
@@ -48,33 +49,10 @@ add_action( 'init', 'psa_init' );
function psa_admin_init() {
PSA_Post_Type::init_admin();
psa_maybe_run_upgrades();
PSA_Upgrade::maybe_run();
}
add_action( 'admin_init', 'psa_admin_init' );
/**
* Run database upgrades when the installed code version is newer than the
* stored `psa_db_version` option. Keeps users from needing to
* deactivate/reactivate the plugin after a schema change.
*
* Triggered on admin_init so dbDelta runs once per upgrade, not on every
* frontend page load.
*
* @return void
*/
function psa_maybe_run_upgrades() {
$stored = get_option( 'psa_db_version', '0.0.0' );
if ( version_compare( $stored, PSA_VERSION, '>=' ) ) {
return;
}
// Re-run CREATE TABLE via dbDelta — additive schema changes (new columns)
// will be applied automatically.
PSA_Cost_Tracker::create_table();
update_option( 'psa_db_version', PSA_VERSION, false );
}
/**
* Check if current page is the Echo KB main page.
* Uses is_page() which is reliable even in block templates.

View File

@@ -130,3 +130,4 @@ require_once ABSPATH . 'includes/class-psa-cost-tracker.php';
require_once ABSPATH . 'includes/class-psa-openrouter.php';
require_once ABSPATH . 'includes/class-psa-kb-builder.php';
require_once ABSPATH . 'includes/class-psa-ai-generator.php';
require_once ABSPATH . 'includes/class-psa-upgrade.php';

View File

@@ -0,0 +1,87 @@
<?php
/**
* Tests for the schema-upgrade decision logic in PSA_Upgrade.
*
* Covers PSA_Upgrade::is_needed() across the four version-comparison
* scenarios: fresh install, already-upgraded, future-version (downgrade),
* and older stored version.
*
* @package PeptideSearchAI
*/
declare( strict_types=1 );
use PHPUnit\Framework\TestCase;
// PSA_Upgrade is already loaded by tests/bootstrap.php via class-psa-upgrade.php.
// No plugin-file bootstrap needed — the decision helper is a pure function.
/**
* Test case for PSA_Upgrade::is_needed().
*
* The helper is a pure static method (no WordPress API calls, no side
* effects), so it can be exercised directly without a live option store.
*/
class UpgradeTest extends TestCase {
/**
* Fresh install: stored version defaults to '0.0.0', which is strictly
* less than any real released version, so an upgrade must run.
*/
public function test_fresh_install_triggers_upgrade(): void {
$this->assertTrue(
PSA_Upgrade::is_needed( '0.0.0', '4.4.3' ),
'Fresh install (stored = 0.0.0) must trigger an upgrade against any released version.'
);
}
/**
* Already-upgraded: stored version equals current. No upgrade needed.
*/
public function test_same_version_skips_upgrade(): void {
$this->assertFalse(
PSA_Upgrade::is_needed( '4.4.3', '4.4.3' ),
'Stored version equal to current version must not trigger an upgrade.'
);
}
/**
* Future-version: stored version is ahead of the running code (e.g., a
* downgrade, or a user running multiple sites out of sync). Do not run
* upgrade — it could apply an older schema over a newer one.
*/
public function test_future_stored_version_skips_upgrade(): void {
$this->assertFalse(
PSA_Upgrade::is_needed( '5.0.0', '4.4.3' ),
'Stored version greater than current version must not trigger an upgrade.'
);
}
/**
* Belt-and-braces: a strictly newer code version against an older
* stored version must trigger an upgrade. Guards against an accidental
* operator flip in version_compare() (e.g., '>=' vs '<').
*/
public function test_older_stored_version_triggers_upgrade(): void {
$this->assertTrue(
PSA_Upgrade::is_needed( '4.4.2', '4.4.3' ),
'Stored version older than current version must trigger an upgrade.'
);
}
/**
* Constants used by the driver should be stable across releases so
* existing installs keep pointing at the same option key.
*/
public function test_option_name_constant_is_stable(): void {
$this->assertSame( 'psa_db_version', PSA_Upgrade::OPTION_NAME );
}
/**
* Default stored version must be strictly less than any future release
* so fresh-install detection keeps working.
*/
public function test_default_stored_version_is_zero(): void {
$this->assertSame( '0.0.0', PSA_Upgrade::DEFAULT_STORED_VERSION );
}
}

View File

@@ -42,6 +42,7 @@ if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) {
// 3. Delete plugin options.
delete_option( 'psa_settings' );
delete_option( 'psa_search_cache_gen' );
delete_option( 'psa_db_version' );
// 4. Delete all plugin transients (validation cache, rate limits).
$wpdb->query(