fix: store cache transient-version markers with an expiration#3266
Open
MdAsifHossainNadim wants to merge 5 commits into
Open
fix: store cache transient-version markers with an expiration#3266MdAsifHossainNadim wants to merge 5 commits into
MdAsifHossainNadim wants to merge 5 commits into
Conversation
Cache group version markers were written via set_transient() without an expiration, so WordPress stores them with autoload=yes and never clears them — bloating autoloaded options on sites without a persistent object cache (5,000+ rows reported). Pass a filterable expiration (dokan_cache_transient_version_expiration, default MONTH_IN_SECONDS) so WordPress stores the marker autoload=no with a timeout and garbage-collects it. The default TTL outlives the longest-lived data transient to avoid premature group invalidation. Ref: getdokan/plugin-internal-tasks#1965 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a configurable expiration for transient cache version markers and adds a new Admin Tools dashboard page with functionality to manage default Dokan pages and clear caches. The Tools page is available in both React (new) and Vue (legacy) with extensible sections for Pro features. Backend logic is centralized in a ChangesTransient cache optimization
Admin Tools feature
Sequence DiagramsequenceDiagram
participant Admin as Admin User
participant UI as React/Vue UI
participant REST as REST/AJAX
participant ToolsActions as ToolsActions Service
participant DB as WordPress DB
participant Cache as WP Object Cache
Admin->>UI: Click "Create Pages" or "Clear Caches"
UI->>REST: POST /dokan/v1/admin/tools/create-pages or clear-caches
REST->>ToolsActions: call create_default_pages() or clear_dokan_caches()
alt Create Default Pages
ToolsActions->>DB: Query wp_posts by page slug
DB-->>ToolsActions: Return existing pages
ToolsActions->>DB: Insert missing pages
ToolsActions->>DB: Update dokan_pages option
ToolsActions->>DB: Flush rewrite rules
DB-->>ToolsActions: Success
else Clear Caches
ToolsActions->>DB: DELETE FROM wp_options WHERE option_name LIKE '_transient_dokan_%'
ToolsActions->>Cache: wp_cache_flush()
Cache-->>ToolsActions: Cache cleared
ToolsActions->>REST: Fire dokan_caches_cleared action
end
ToolsActions-->>REST: Return { success, message }
REST-->>UI: JSON response
UI->>Admin: Show success/error toast, update button state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/Traits/TransientCache.php`:
- Line 51: Replace the placeholder docblock tag "`@since` DOKAN_SINCE" in the
TransientCache trait with the concrete release version string (e.g., "`@since`
3.4.0" or the appropriate current release) so the method/hook documentation is
accurate; locate the docblock inside the includes/Traits/TransientCache.php file
and update the `@since` annotation accordingly for the associated method/hook.
- Around line 56-58: The filtered expiration value from
apply_filters('dokan_cache_transient_version_expiration', MONTH_IN_SECONDS,
$group) must be validated before calling set_transient to avoid non-expiring
markers; update the TransientCache logic to coerce and guard $expiration (ensure
it is a positive integer greater than zero, e.g. (int)$expiration and if <= 0
fallback to MONTH_IN_SECONDS) and then pass the validated value into
set_transient($transient_name, $transient_value, $expiration).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2a46952-3382-4c10-b5ad-f512f99ed9f4
📒 Files selected for processing (1)
includes/Traits/TransientCache.php
…default pages and cache
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/Admin/Tools/ToolsActions.php`:
- Around line 97-99: The `update_option( 'dokan_pages_created', 1 )` call is
executed unconditionally regardless of whether the page creation operations
succeeded or failed. Add conditional logic to only set `dokan_pages_created` to
1 after verifying that all required pages were actually created successfully.
Check the return values or results from the preceding page creation operations
(before the `update_option( 'dokan_pages', $dokan_pages )` call) and only
proceed with marking `dokan_pages_created` as complete if all operations were
successful, otherwise leave the flag unset or set it to 0 to indicate incomplete
setup.
- Around line 142-154: Replace the global wp_cache_flush() call in the
ToolsActions.php file with targeted cache deletion that only removes
Dokan-specific cache keys. Instead of clearing all WordPress caches, use
wp_cache_delete() to selectively clear only the Dokan-related cache entries that
need to be invalidated. This prevents clearing unrelated site caches from other
plugins and services. Identify all critical Dokan cache keys (such as those for
products, settings, vendor data, etc.) and call wp_cache_delete() for each of
them individually.
- Around line 113-116: The method check_all_dokan_pages_exists() is returning
string values '1' and '0' instead of proper booleans, which causes the frontend
contract to break because in JavaScript '0' is truthy. Additionally, the method
only checks a stale dokan_pages_created option rather than verifying actual page
state. Modify the method to return real boolean values (true and false) instead
of strings, and update the logic to verify the actual existence of dokan pages
by querying current page records instead of only relying on the potentially
outdated get_option call.
In `@includes/Ajax.php`:
- Around line 73-80: The create_pages() method is missing nonce verification
which is required to prevent CSRF attacks. Even though the method has a
capability check with current_user_can(), nonce validation is necessary for
state-changing operations. Add nonce verification at the beginning of the
create_pages() method using wp_verify_nonce() to check for a nonce parameter in
the request, and return an error response if verification fails. Follow the
pattern used in the clear_caches() method (referenced at Lines 108-110) which
already implements nonce validation correctly.
In `@src/admin/dashboard/components/Tools/HeaderImage.tsx`:
- Line 1: The HeaderImage function parameter has implicit typing which violates
strict TypeScript mode. Define an explicit props interface or type for the
function parameters. Create a type that specifies the structure of the props
object, explicitly declaring that className is a string type with a default
value of an empty string. Apply this type annotation to the destructured
parameter in the HeaderImage function signature to ensure all component props
are strictly typed.
In `@src/admin/dashboard/components/Tools/sections/InstallationGuide.tsx`:
- Around line 45-54: The useEffect hook that calls apiFetch for the
'/dokan/v1/admin/tools/check-all-dokan-pages-exists' endpoint has a .then
handler but is missing a .catch handler, which can cause unhandled promise
rejections if the request fails. Add a .catch block after the .then to handle
rejection cases appropriately, such as logging any errors that occur during the
API call and optionally setting a fallback state to ensure the component behaves
predictably regardless of whether the request succeeds or fails.
In `@src/admin/pages/Tools.vue`:
- Around line 103-137: The `createPages()` and `checkAllPages()` methods send
AJAX requests without CSRF nonce protection, making them vulnerable to CSRF
attacks. Additionally, `createPages()` only clears the loading flag on success,
which locks the UI if the request fails. Fix this by adding nonce parameters to
both jQuery.post calls (the nonce should be available from the dokan object) and
ensure the loading flag in `createPages()` is cleared in both success and error
scenarios by using error handlers or wrapping the callback logic to guarantee
cleanup on all outcomes.
In `@src/components/ToolsSection.tsx`:
- Line 62: The className string in ToolsSection.tsx contains Tailwind CSS
important modifiers using the legacy prefix syntax (exclamation mark before the
utility class). Update all important modifiers in the className from prefix
placement (like !h-[28px]) to the Tailwind v4 recommended suffix placement
(h-[28px]!) by moving the exclamation mark to the end of each utility class.
This affects all four utility classes in the style definition: h-[28px],
font-[500], text-[12px], and leading-[16px].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a44b1dea-fc10-4c7d-a3a0-d475e30bafd5
📒 Files selected for processing (18)
includes/Admin/Dashboard/Pages/Tools.phpincludes/Admin/Menu.phpincludes/Admin/Tools/ToolsActions.phpincludes/Ajax.phpincludes/Assets.phpincludes/DependencyManagement/Providers/AdminDashboardServiceProvider.phpincludes/REST/Manager.phpincludes/REST/ToolsController.phpsrc/admin/dashboard/components/Dashboard.tsxsrc/admin/dashboard/components/Tools/HeaderImage.tsxsrc/admin/dashboard/components/Tools/Tools.tsxsrc/admin/dashboard/components/Tools/sections/ClearDokanCaches.tsxsrc/admin/dashboard/components/Tools/sections/InstallationGuide.tsxsrc/admin/dashboard/components/Tools/types.tssrc/admin/pages/Tools.vuesrc/admin/router/index.jssrc/components/ToolsSection.tsxsrc/components/index.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/index.tsx
Comment on lines
+142
to
+154
| wp_cache_flush(); | ||
|
|
||
| /** | ||
| * Fires after all Dokan caches have been cleared from the admin Tools page. | ||
| * | ||
| * @since DOKAN_SINCE | ||
| */ | ||
| do_action( 'dokan_caches_cleared' ); | ||
|
|
||
| return [ | ||
| 'process' => 'success', | ||
| 'message' => __( 'Dokan caches have been cleared successfully.', 'dokan-lite' ), | ||
| ]; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Avoid full object-cache flush for a Dokan-scoped action.
At Line 142, wp_cache_flush() clears all groups, not just Dokan keys. This can evict unrelated site caches and cause broad performance degradation after a single Tools click.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/Admin/Tools/ToolsActions.php` around lines 142 - 154, Replace the
global wp_cache_flush() call in the ToolsActions.php file with targeted cache
deletion that only removes Dokan-specific cache keys. Instead of clearing all
WordPress caches, use wp_cache_delete() to selectively clear only the
Dokan-related cache entries that need to be invalidated. This prevents clearing
unrelated site caches from other plugins and services. Identify all critical
Dokan cache keys (such as those for products, settings, vendor data, etc.) and
call wp_cache_delete() for each of them individually.
MdAsifHossainNadim
added
Needs: Testing
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
2 participants
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.
Summary
Cache group transient-version markers are written via
set_transient()with no expiration, so WordPress stores them withautoload = yesand never clears them. On sites without a persistent object cache this bloats the autoloaded options (a client reported 5,000+ autoloaded rows, mostly_transient_dokan_cache_*-transient-version).This adds a filterable expiration to the marker so WordPress stores it
autoload = no(with a_transient_timeout_companion) and garbage-collects it like any transient.Related PR: https://github.com/getdokan/dokan-pro/pull/5765
Closes: https://github.com/getdokan/plugin-internal-tasks/issues/1965
Change
includes/Traits/TransientCache.php—get_transient_version()now sets the marker with:The default TTL (1 month) comfortably outlives the longest-lived data transient (≤ 1 week), so cache groups are not invalidated prematurely.
How to test
autoload = yes, no_transient_timeout_companion.autoload = no(oroff) with a matching_transient_timeout_…row ~30 days out.Verified
Local site, forcing the DB transient path: marker stored
autoload = offwith a~2592000s(30-day) timeout; cache read-back unaffected.Ref: getdokan/plugin-internal-tasks#1965 · Companion Pro PR adds a Clear Dokan Caches admin tool.
🤖 Generated with Claude Code
Summary by CodeRabbit
Summary