Cache Store API products last modified timestamp#63228
Conversation
…r request The ProductQuery::get_last_modified() method ran a MAX() query against the posts table on every Store API /products request. This caches the result in the object cache and invalidates it via the clean_post_cache hook in WC_Post_Data, eliminating the query on subsequent requests. The cached value uses a dedicated key/group rather than WordPress core's wp_cache_*_last_changed() pattern because the value is exposed to clients via the Last-Modified header. Core's pattern auto-seeds with the current time on cache miss, which would force unnecessary client-side cache invalidation. Instead, on a miss we fall back to the DB for the real last modification time. Also normalizes the Last-Modified header to RFC 7232 HTTP-date format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Testing GuidelinesHi @opr @ObliviousHarmony @Copilot @woocommerce/flux, Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughAdds caching for the products "last modified" value in the WordPress object cache and a cache invalidation hook tied to post cache cleanup; ProductQuery now returns an HTTP-date formatted last-modified string (or null) and seeds/reads that value from cache. Changes
Sequence DiagramssequenceDiagram
participant Client
participant Cache as Object Cache (wc_products)
participant DB as Database
participant Invalidator as WC_Post_Data (clean_post_cache)
Note over Client,Cache: Initial request (cache miss)
Client->>Cache: wp_cache_get('last_modified')
Cache-->>Client: null
Client->>DB: SELECT MAX(post_modified_gmt) WHERE type IN (product, product_variation)
DB-->>Client: post_modified_gmt
Client->>Cache: wp_cache_set('last_modified', HTTP-date)
Cache-->>Client: stored
Note over Client,Cache: Subsequent request (cache hit)
Client->>Cache: wp_cache_get('last_modified')
Cache-->>Client: HTTP-date (no DB query)
Note over Invalidator,Cache: Product updated -> cache invalidated
Invalidator->>Cache: wp_cache_delete('last_modified')
Cache-->>Invalidator: deleted
Note over Client,DB: Next request after invalidation (reseeding)
Client->>Cache: wp_cache_get('last_modified')
Cache-->>Client: null
Client->>DB: SELECT MAX(post_modified_gmt) ...
DB-->>Client: updated post_modified_gmt
Client->>Cache: wp_cache_set('last_modified', new HTTP-date)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/ProductQueryTest.php`:
- Line 8: The test class ProductQueryTest currently extends
Yoast\PHPUnitPolyfills\TestCases\TestCase and declares a protected setUp();
change the class to extend WC_Unit_Test_Case (so WooCommerce test utilities and
cleanup run) and change the setUp() method signature to public function setUp():
void; also add or update the class docblock to reflect it's a WooCommerce unit
test (mention WC_Unit_Test_Case) to match project conventions.
🧹 Nitpick comments (4)
plugins/woocommerce/src/StoreApi/Utilities/ProductQuery.php (2)
337-353: Add a@sincetag documenting the return-type change.The method's return type changed from
int(Unix timestamp) tostring|null(HTTP-date). This is a public method on a non-internal class, so the signature change should be annotated with@since 10.6.0to signal the breaking change to extension developers.📝 Proposed docblock addition
* `@return` string|null HTTP-date formatted string, or null if no products exist. + * `@since` 10.6.0 Return type changed from int (Unix timestamp) to string|null (HTTP-date). */
360-366:nullreturn is not cached — repeated DB queries when no products exist.When
$last_modified_gmtis falsy (no products), the method returnsnullwithout caching, so every subsequent call will re-query the DB. For most stores this is fine since having zero products is an edge case. If this path ever becomes hot, consider caching a sentinel value.plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/ProductQueryTest.php (2)
32-56: Fragile DB manipulation — consider wrapping in try/finally for safety.The test temporarily renames all product post types in the DB. If an assertion or error occurs between lines 39 and 53, the DB is left in a corrupt state for subsequent tests. Wrapping the assert + restore in a
try/finallywould be safer.Also, the
REPLACEquery on line 47 is redundant — it setspost_typeto an empty string, and lines 51–53 overwrite that with the correct values anyway. Consider removing lines 46–48 entirely.♻️ Proposed safer cleanup
$wpdb->query( "UPDATE {$wpdb->posts} SET post_type = '_tmp_hidden' WHERE post_type IN ('product', 'product_variation')" ); - $result = $this->product_query->get_last_modified(); - - // Restore original posts. - $wpdb->query( - "UPDATE {$wpdb->posts} SET post_type = REPLACE(post_type, '_tmp_hidden', '') WHERE post_type = '_tmp_hidden'" - ); - - // Restore correct post types from the saved data. - foreach ( $original_posts as $post ) { - $wpdb->update( $wpdb->posts, array( 'post_type' => $post->post_type ), array( 'ID' => $post->ID ) ); + try { + $result = $this->product_query->get_last_modified(); + } finally { + foreach ( $original_posts as $post ) { + $wpdb->update( $wpdb->posts, array( 'post_type' => $post->post_type ), array( 'ID' => $post->ID ) ); + } } $this->assertNull( $result );
166-181: Minor: unused$first_resultvariable.The call on line 171 is needed to seed the cache, but the assignment to
$first_resultis unused. This can be cleaned up to avoid the PHPMD warning.🧹 Quick fix
- $first_result = $this->product_query->get_last_modified(); + $this->product_query->get_last_modified();
f8bd3ca to
632ed87
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
632ed87 to
9d19429
Compare
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
Pull request overview
This PR implements caching for the Store API products' last modified timestamp to reduce database overhead. Previously, every /wc/store/v1/products request executed a SELECT MAX(post_modified_gmt) query to populate the Last-Modified response header. The new implementation caches this value in the object cache and invalidates it when products are modified, eliminating unnecessary database queries on cache hits.
Changes:
- Added object cache support for
ProductQuery::get_last_modified()with explicit invalidation on product changes - Normalized the
Last-Modifiedheader format from Unix timestamp to RFC 7232 HTTP-date format - Added comprehensive unit tests covering caching behavior, cache invalidation, and edge cases
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
plugins/woocommerce/src/StoreApi/Utilities/ProductQuery.php |
Implements caching logic with database fallback on cache miss and converts return format to HTTP-date string |
plugins/woocommerce/includes/class-wc-post-data.php |
Adds cache invalidation hook that clears the cached timestamp when product posts are modified |
plugins/woocommerce/tests/php/src/Blocks/StoreApi/Utilities/ProductQueryTest.php |
Adds 8 unit tests covering HTTP-date format, caching, invalidation for products/variations, and non-product changes |
plugins/woocommerce/phpstan-baseline.neon |
Removes two resolved PHPStan errors related to the return type and header value type changes |
plugins/woocommerce/changelog/wooplug-6264-cache-products-last-modified |
Documents the performance improvement in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Cache Store API products last modified timestamp to avoid DB query per request The ProductQuery::get_last_modified() method ran a MAX() query against the posts table on every Store API /products request. This caches the result in the object cache and invalidates it via the clean_post_cache hook in WC_Post_Data, eliminating the query on subsequent requests. The cached value uses a dedicated key/group rather than WordPress core's wp_cache_*_last_changed() pattern because the value is exposed to clients via the Last-Modified header. Core's pattern auto-seeds with the current time on cache miss, which would force unnecessary client-side cache invalidation. Instead, on a miss we fall back to the DB for the real last modification time. Also normalizes the Last-Modified header to RFC 7232 HTTP-date format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add changelog entry for products last modified caching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Changes proposed in this Pull Request:
ProductQuery::get_last_modified()runs aSELECT MAX(post_modified_gmt)query against the posts table on every Store API/wc/store/v1/productsrequest to populate theLast-Modifiedresponse header. This adds unnecessary database overhead on every request since products change infrequently relative to how often they are read.This PR caches the result in the object cache (
last_modifiedkey inwc_productsgroup) and invalidates it via theclean_post_cachehook inWC_Post_Data. On cache hit, the DB query is skipped entirely. On cache miss (first request or after a product change), the query runs and the result is cached.The implementation uses
wp_cache_delete()/wp_cache_get()/wp_cache_set()rather than WordPress core'swp_cache_*_last_changed()pattern. Core'slast_changedpattern auto-seeds with the current time on cache miss, which is fine for opaque cache-key salts but would cause incorrect behavior here — the value is exposed to clients via theLast-Modifiedheader, and auto-seeding with "now" would force all clients to unnecessarily invalidate their local caches. Instead, on a cache miss we fall back to the database for the real last modification time.Also normalizes the
Last-Modifiedheader value to RFC 7232 HTTP-date format (previously a raw unix timestamp).Closes WOOPLUG-6264 / #63164
How to test the changes in this Pull Request:
/wp-json/wc/store/v1/productsdirectly).Last-Modifiedheader should now be in HTTP-date format, e.g.,Wed, 09 Oct 2024 08:28:00 GMT.SELECT MAX(post_modified_gmt)query is no longer present.Last-Modifiedheader value has updated to reflect the change. The DB query should run once on this first request after the change, then be cached again.Last-Modifiedshould update.Last-Modifiedshould update.Last-Modifiedheader on the products endpoint does NOT change.Testing that has already taken place:
pnpm test:php:env -- --filter ProductQueryTest)Milestone
Changelog entry
Changelog Entry Comment
Comment
Changelog entry is included in this PR at
plugins/woocommerce/changelog/wooplug-6264-cache-products-last-modified.