fix(check): orphan-venues count_refreshed action persists the refreshed cache via direct wpdb update + cache invalidation#285
Open
chubes4 wants to merge 1 commit into
Open
Conversation
…date + cache invalidation wp_update_term_count_now() was observed to not persist the refreshed count on production: 99 stale-cache venue terms were correctly identified (wp_term_taxonomy.count=0 with real wp_term_relationships rows), the function was called against each, and zero of them had their cached count actually move. term_id 30968 (real=2) and 40065 (real=1) both stayed cached=0 after the audit run. Replace the call with a direct $wpdb->update against wp_term_taxonomy plus explicit clean_term_cache() + wp_cache_delete() to invalidate the surrounding object cache. This bypasses any custom update_count_callback indirection and any post-status filtering applied by the default WP counter — defensive against all three failure modes hypothesized in issue #284 in a single change. The diagnostic logic (stale-cache detection) is unchanged; only the write step is fixed. The count_refreshed branch is the only one that needed cache invalidation — protected_by_flow / flagged / deleted already write through update_term_meta or wp_delete_term and do not touch wp_term_taxonomy.count. Adds two regression tests: - test_count_refreshed_actually_persists_in_term_taxonomy asserts via a fresh $wpdb->get_var query (not get_term()) that the DB column itself is updated. - test_count_refresh_invalidates_object_cache primes the term cache with the stale value and asserts get_term() returns the refreshed value, proving DB write + cache invalidation work together. Fixes #284
Contributor
Homeboy Results —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #284
Summary
wp data-machine-events check orphan-venues --applycorrectly detects venue terms whosewp_term_taxonomy.count = 0is stale (real relationship rows exist inwp_term_relationships), reports them with actioncount_refreshed, and previously calledwp_update_term_count_now( array($term_taxonomy_id), 'venue' ). The function call returned without error but did not actually persist the refreshed count.Production repro (from #284)
After a full
--applyrun today:99 stale-cache terms were detected in today's audit; none of their cached counts actually got refreshed.
Operationally harmless (the
count_refreshedaction's effect is "skip deletion" — which is still correct, the term was not really an orphan), but a real correctness bug in the diagnostic and an admin-UI cosmetic mismatch.Fix layer applied: Layer 3 (direct
$wpdb->update+ explicit cache invalidation)Investigation findings:
CheckOrphanVenuesCommandcallswp_update_term_count_now()at the count-refresh branch (was line 232).Venue_Taxonomy::register_venue_taxonomy()does not register a customupdate_count_callback, so the default WP counter is in play. Layer 2 (fix a misconfigured custom callback) is not applicable.clean_term_cache()+wp_cache_delete()after the existing helper call) might have been sufficient, but the production failure mode is ambiguous — could be Redis intercepting the cachedterms:Nvalue, could be the default counter'spublish/private/inheritpost-status filter excluding rows we already detected, could be something else entirely. The most defensive shape — and the one explicitly recommended in the issue's "Fix direction" Calendar block expands multi-date events to all dates instead of respecting occurrence dates #3 — is to bypasswp_update_term_count_now()entirely.The new helper
persist_refreshed_count()does both halves of the fix:wp_term_taxonomy.countvia$wpdb->update(). Bypasses any customupdate_count_callbackindirection and any post-status filtering applied by the default counter.clean_term_cache( $term_id, 'venue', true )andwp_cache_delete( $term_id, 'terms' )so the nextget_term()call reads fresh from the database.The diagnostic logic (stale-cache detection via
real_relationship_count()) is unchanged — only the write step is fixed. The other action branches (protected_by_flow,flagged,deleted) do not touchwp_term_taxonomy.countand do not need cache invalidation; they were intentionally left alone.Tests
Two new regression tests in
tests/Unit/CheckOrphanVenuesCommandTest.php:test_count_refreshed_actually_persists_in_term_taxonomy— seeds a venue term withwp_term_taxonomy.count=0and 2 realwp_term_relationshipsrows against publish-statusdata_machine_eventsposts; runs--apply; assertswp_term_taxonomy.countis now2via a fresh$wpdb->get_varquery (deliberately not viaget_term(), which can return a cached value).test_count_refresh_invalidates_object_cache— same seed; primes the term cache with the stale 0 value; runs--apply; asserts that a subsequentget_term($term_id, 'venue')returns->count = 2. Proves both the DB write AND the cache invalidation work together.Existing
test_refreshes_stale_count_cache_before_decidingcontinues to pass — its assertions hold under the new persistence path.Verification
php -lclean on both modified files.homeboy auditreturns only pre-existing drift findings unrelated to this change.Bootstrap blocker
The plugin has no
tests/bootstrap.phpand nocomposer.jsonrequire-devforphpunit/wp-phpunit. I could not execute the test suite locally in this worktree — the tests are syntactically valid and follow the sameWP_UnitTestCaseshape as the surrounding test file but were not run. Tests should run in CI if a workflow exists, or against a properly bootstrapped WP test harness.Post-merge operator action
After this lands and deploys, re-run against the existing 99 stale-cache terms to actually persist them:
This will re-traverse the same candidates (their cached count is still 0), hit the
count_refreshedbranch, and this time the count column will move.Alternative one-liner if the operator wants to fix every venue cache network-wide outside this command:
Out of scope
check missing-venue-addresses— separate concern.wp_update_term_count_now()— same hypothesized failure modes may apply, but each call site has its own context. This PR fixes only the count_refreshed branch inCheckOrphanVenuesCommand.