Fix API Key Status Cache Synchronization and Expired State Handling

[the-hercules]

Closes https://github.com/rtCamp/godam-core/issues/1070

Overview

This PR improves API key status handling by ensuring cached user data remains consistent with the persisted API key state, particularly during expiration and grace-period flows.

Changes

1. Force EXPIRED State After Grace Period

When verification is skipped after the grace period ($skip_verification), cached user data is now explicitly updated with:

• valid_api_key = false
• api_key_status = EXPIRED
• Updated timestamp

and persisted back to rtgodam_user_data.

Why: Ensures stale verification results cannot persist in cache once the grace period has ended.

---

2. Clear Cache When API Key Expires

When a previously valid API key is determined to be expired:

rtgodam_mark_api_key_expired();
    delete_option( 'rtgodam_user_data' ); 

the cached user data is immediately removed.

Why: Prevents requests from using data generated while the key was still valid and forces regeneration on the next request.

---

Impact

• Prevents stale cache after API key expiration.
• Keeps cached data aligned with persisted API key status.
• Ensures deterministic behavior after grace-period expiry.
• Improves overall cache consistency and reliability.

Video Demo

Screen.Recording.2026-06-24.at.4.02.39.PM.mov

[Copilot]

Pull request overview

This PR tightens API key status/cache consistency by (1) forcing cached rtgodam_user_data into an explicit EXPIRED state once the grace period has ended (when automatic verification is skipped), and (2) clearing cached user data immediately when a previously-valid key is detected as expired.

Changes:

• Update cached/persisted rtgodam_user_data to valid_api_key=false and api_key_status=EXPIRED when skipping verification post–grace period.
• Delete the rtgodam_user_data option when a previously valid key transitions to expired during verification.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
inc/helpers/custom-functions.php	Syncs cached rtgodam_user_data to an explicit EXPIRED state when verification is skipped after grace period.
admin/godam-transcoder-functions.php	Clears cached rtgodam_user_data when verification determines a previously valid key is expired.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[KMchaudhary]

Code Review

Traced the full flow across both files and their callers. Overall the fix is sound and the write-avoidance guard is genuinely well-designed — a few notes below.

What's good

• The $cache_needs_correction guard is the highlight. Without it, the skip-verification path would update_option() on every admin page load for the entire post-grace lifetime of an expired key (the timestamp always differs → a real DB write each time). Writing only when the cached status isn't already EXPIRED/false is the right call, and the inline comment explaining it is excellent.
• Defensive normalization of $rtgodam_user_data / $user_data (is_array/is_object coercion) is appropriate given the option can be false or hold legacy shapes.
• The base array is preserved and only specific keys overwritten, so usage fields (storage_used, etc.) survive the correction.

Issues & suggestions

1. The delete_option('rtgodam_user_data') in the transcoder function is a no-op in the primary flow (medium).
rtgodam_verify_api_key() is called from rtgodam_get_user_data() (custom-functions.php:364). When the expired branch fires it deletes the option and returns a WP_Error, but execution then continues in rtgodam_get_user_data() and unconditionally rewrites rtgodam_user_data at line 449 in the same request. So the delete only actually sticks when verify is called from the manual settings path (class-settings.php:216), where the REST handler returns early on the error without rebuilding the cache.

The change isn't harmful, but the PR description's claim that the cache "is immediately removed … forces regeneration on the next request" only holds for the manual-verify path, not the automatic one. Worth a code comment so a future maintainer doesn't assume the delete is authoritative.

2. Possible undefined-index notice on timestamp (low).
The new block sets $rtgodam_user_data['timestamp'] only inside if ( $cache_needs_correction ). The localize path reads $rtgodam_user_data['timestamp'] unconditionally at line 464. In normal flows this is safe (the first correcting pass always sets it), but a cached array that is already EXPIRED + valid_api_key=false yet missing timestamp would skip correction and then emit a PHP notice. Cheap to harden — set timestamp outside the guard, or $rtgodam_user_data['timestamp'] ??= time();.

3. Stale storageBandwidthError persists into EXPIRED state (low / cosmetic).
Because the base array is preserved, a previously-set storageBandwidthError ("Bandwidth limit exceeded…") carries through into the forced-EXPIRED data. Once the key is expired, the API-key status message should arguably take precedence over a usage banner — consider clearing it in the correction block.

4. Unrelated cosmetic change. The removed blank line in rtgodam_filter_input() is outside this PR's scope — harmless, but ideally not mixed into a behavioral fix.

Tests

No tests added. This is cache-consistency logic with several branches (expired+grace, expired+post-grace, missing cache, already-corrected cache) and there's no existing coverage for rtgodam_get_user_data(). A couple of regression tests would de-risk future changes to this area.

Security / performance

• No security concerns; masked-key handling is unchanged in spirit.
• Performance is improved by the $cache_needs_correction guard — it specifically prevents per-pageload DB writes. 👍

Verdict

Approve with minor changes. Core fix is correct. Before merge I'd suggest: (a) clarify/justify the transcoder delete_option() given it's overwritten in the automatic path, (b) move the timestamp assignment out of the correction guard to avoid the notice, and optionally (c) clear storageBandwidthError on expiry and add a regression test.

_{🤖 Generated with Claude Code}

[the-hercules]

Above issues addressed
Issue 1 -> Fixed
Issue 2 -> Every code path that writes rtgodam_user_data always sets timestamp before the update_option call. A missing key can only arise from a manually crafted DB row, which is outside the plugin's responsibility.
Issue 3 -> Fixed

cc - @KMchaudhary