fix/duplicate-queries-vendor-dashboard-dokan-wpml #111
Conversation
📝 WalkthroughWalkthroughAdds an in-memory option cache to Dokan_WPML: a private static cache, hooks registered to clear specific cached options on option changes, a public cache-clearing method, and get_raw_option updated to return cached values and populate the cache on misses. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Dokan_WPML::get_raw_option
participant Cache as In-memory Cache (self::$cached_options)
participant WPML as WPML_Multilingual_Options_Utils
participant WP as WordPress Options
rect rgb(230, 247, 255)
Caller->>Cache: check [section][option]
alt cache hit
Cache-->>Caller: return cached value
else cache miss
Caller->>WPML: get_option_without_filtering(option, section, default)
WPML->>WP: read option from DB (or WP option API)
WP-->>WPML: option value
WPML-->>Caller: raw option value
Caller->>Cache: store value at [section][option]
Cache-->>Caller: return stored value
end
end
note over WP,Cache: Hooks (updated_option/added_option/deleted_option)\ncall Dokan_WPML::clear_option_cache(option) to remove cached entry
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dokan-wpml.php (1)
52-57: Minor: Use PHPDoc format for the docblock.The comment uses
/*instead of/**, which means IDEs and documentation generators won't recognize it as a proper PHPDoc block.Suggested fix
- /* + /** * Cached options * * @var array */ private $cached_options = [];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dokan-wpml.php(2 hunks)
🔇 Additional comments (1)
dokan-wpml.php (1)
1013-1033: Caching implementation looks good.The per-request cache correctly prevents duplicate database queries to
WPML_Multilingual_Options_Utils->get_option_without_filtering(). Since the class uses a singleton pattern, the cache persists across all calls within the same request.One minor note: if
WPML_Multilingual_Options_Utilsclass is not loaded (line 1020-1022), the fallback path doesn't cache its result. This is acceptable since WPML is a required dependency, but worth being aware of if debugging caching behavior in edge cases.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
dokan-wpml.php (1)
221-235: Cache invalidation works correctly for the current use case.The unused parameter warnings from PHPMD are false positives—
$param2and$param3are required because WordPress hooks pass different numbers of arguments (updated_optionpasses 3,added_optionpasses 2,deleted_optionpasses 1), and this method must be compatible with all three.However, the invalidation logic assumes only the
dokan_pagesoption is cached. While this matches current usage (lines 590, 785, 813), theget_raw_optionmethod signature is generic and could cache other sections. If the method is used for other sections in the future, those cached values won't be invalidated properly.🔎 Optional: More defensive cache invalidation
If you want to future-proof the cache invalidation for any section, consider checking if the updated option matches any cached section:
public function clear_option_cache( $option, $param2 = null, $param3 = null ) { - // Clear only if dokan_pages option is affected - if ( $option === 'dokan_pages' ) { - $this->cached_options = []; + // Clear cache entries for the affected option/section + if ( empty( $this->cached_options ) ) { + return; } + + foreach ( array_keys( $this->cached_options ) as $cache_key ) { + // Cache keys are formatted as: {section}_{option} + // If the section matches the updated option name, clear it + if ( strpos( $cache_key, $option . '_' ) === 0 ) { + unset( $this->cached_options[ $cache_key ] ); + } + } }This approach clears only the cache entries for the affected section rather than the entire cache, making it safer if the method is used for caching other WordPress options in the future.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dokan-wpml.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
dokan-wpml.php
230-230: Avoid unused parameters such as '$param2'. (undefined)
(UnusedFormalParameter)
230-230: Avoid unused parameters such as '$param3'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (3)
dokan-wpml.php (3)
52-57: LGTM!Clean addition of the cache storage property to hold option values in memory.
92-96: Cache invalidation hooks correctly registered.The three hooks properly cover all option lifecycle events (add, update, delete) to ensure cache coherency.
1033-1053: Caching implementation correctly eliminates duplicate queries.The cache-check-before-fetch pattern is correctly implemented and directly addresses the PR objective to eliminate duplicate calls to
WPML_Multilingual_Options_Utils->get_option_without_filtering().The cache key construction (
{section}_{option}) is straightforward and appropriate for the use case. While theoretically there could be key collisions with specially crafted section/option names (e.g.,section="foo_bar"+option="baz"producing the same key assection="foo"+option="bar_baz"), this is extremely unlikely given the controlled nature of WordPress option names.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dokan-wpml.php
🔇 Additional comments (3)
dokan-wpml.php (3)
52-57: LGTM! Cache property is appropriately scoped.The static property is correctly used for in-memory caching across multiple method calls during a single request.
93-96: LGTM! Hook registration correctly uses only the needed parameter.The cache clearing hooks are properly registered and only accept the
$optionparameter, which is all that's needed for cache invalidation.
1038-1056: LGTM! Caching implementation is well-structured.The cache-check-compute-store pattern is correctly implemented, and the use of the null coalesce operator addresses previous feedback.
|
All problems are fixed. Please check. |
|
Tested and found good, waiting for dev review done |
All Submissions:
Changes proposed in this Pull Request:
Added a caching layer to avoid redundant database queries. The query now executes only once, with subsequent calls fetching the cached result instead of hitting the database.
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
*Title: Bug/Problem - Duplicate Queries Detected on Vendor Dashboard When Dokan WPML Is Enabled
Before Changes
When Dokan WPML is active, Query Monitor reports duplicate database queries on the vendor dashboard. The duplicate calls originate from the method WPML_Multilingual_Options_Utils->get_option_without_filtering().
After Changes
The vendor dashboard avoid unnecessary duplicate option queries, especially those triggered through WPML, to ensure optimal performance.
Feature Video (optional)
Video reference: https://d.pr/v/Gaa2R9
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Performance Improvements
Reliability / Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.