Fix report transient cache key instability caused by DateTime microsecond serialization

[shsajalchowdhury]

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.

Changes proposed in this Pull Request:

When get_cache_key() serialized DateTime objects via wp_json_encode(), the resulting cache key included microsecond precision from WC_DateTime. This produced a different transient key on every page load, completely defeating the report cache.

Root cause: get_default_query_vars() returns TimeInterval::default_before() which creates a WC_DateTime at the current moment with microseconds. Since wp_json_encode preserves sub-second precision, each request generates a unique MD5 hash, so wc_report_customers_* transients are written but never hit.

Fix: Normalize DateTimeInterface objects to ISO 8601 strings (DATE_ATOM format) in get_cache_key() before hashing. This strips microseconds and produces stable, deterministic cache keys while preserving second-level precision.

The normalization happens on a local copy of $params (PHP passes arrays by value), so the original $query_args with their DateTime objects remain intact for downstream SQL query building.

Closes #64177

How to test the changes in this Pull Request:

1. Create a WooCommerce store with some completed orders (200K+ amplifies the issue, but any store will show the behavior)
2. Enable transient logging (e.g., Query Monitor or a debugging plugin that tracks transients)
3. Go to WooCommerce → Orders and open any order in WP Admin
4. Note the wc_report_customers_* transient key
5. Refresh the page
6. Before fix: Each refresh creates a new wc_report_customers_* transient with a different key (due to microsecond variation in the before parameter)
7. After fix: The same transient key is reused on every refresh, cache is properly hit
8. Verify unit tests pass: test that DateTime objects with same second but different microseconds produce identical cache keys

Testing that has already taken place:

• Unit tests added in tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php verifying:
  • Cache keys are identical when DateTime objects differ only in microseconds
  • Cache keys differ when DateTime objects represent different seconds
  • Both before and after params produce stable keys
  • Original params array is not modified by get_cache_key()
• PHP lint passes clean via bin/lint-branch.sh

Milestone

• Automatically assign milestone for the next WooCommerce version

Changelog entry

• This Pull Request does not require a changelog entry. (Comment required below)

Changelog Entry Comment

Comment

A changelog file has been manually added to plugins/woocommerce/changelog/fix-64177-transient-cache-key-datetime as the automated changelog job cannot run on PRs from forks.

[github-actions]

Testing Guidelines

Hi ,

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:

• 🖼️ Screenshots or screen recordings.
• 📝 List of functionality tested / steps followed.
• 🌐 Site details (environment attributes such as hosting type, plugins, theme, store size, store age, and relevant settings).
• 🔍 Any analysis performed, such as assessing potential impacts on environment attributes and other plugins, conducting performance profiling, or using LLM/AI-based analysis.

⚠️ Within the testing details you provide, please ensure that no sensitive information (such as API keys, passwords, user data, etc.) is included in this public issue.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 946738f9-979e-4971-8d80-a3314c3644cf

📥 Commits

Reviewing files that changed from the base of the PR and between ba2b03c and faf23cf.

📒 Files selected for processing (1)

• plugins/woocommerce/tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php

🚧 Files skipped from review as they are similar to previous changes (1)

• plugins/woocommerce/tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php

---

📝 Walkthrough

Walkthrough

DateTime parameters used in WooCommerce Reports DataStore cache-key generation are normalized to ISO 8601 strings (DATE_ATOM) before encoding, preventing transient cache key instability caused by microsecond differences.

Changes

Cohort / File(s)	Summary
Changelog Entry plugins/woocommerce/changelog/fix-64177-transient-cache-key-datetime	Patch-level changelog entry documenting the transient cache key instability fix related to DateTime serialization.
Cache Key Normalization plugins/woocommerce/src/Admin/API/Reports/DataStore.php	Normalize values implementing \DateTimeInterface to DATE_ATOM strings prior to ksort and md5(wp_json_encode($params)), ensuring deterministic cache keys.
Test Coverage plugins/woocommerce/tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php	New PHPUnit tests verifying cache key stability across DateTime microsecond differences, sensitivity to second-level differences, stability with both before/after, and that original DateTime objects are not mutated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix report transient cache key instability caused by DateTime microsecond serialization' accurately and specifically describes the main change—normalizing DateTime objects in cache key generation to resolve cache instability.
Description check	✅ Passed	The description is well-detailed and directly related to the changeset. It explains the root cause (DateTime microseconds in cache keys), the fix (normalizing to ISO 8601 strings), testing approach, and verification steps.
Linked Issues check	✅ Passed	The PR fully addresses issue #64177. It normalizes DateTime objects to ISO 8601 strings in get_cache_key() to eliminate microsecond variance, implements comprehensive unit tests verifying stable cache keys and immutability, and provides manual testing steps aligned with the issue's requirements.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fix #64177: a changelog entry, DateTime normalization in DataStore.php, and comprehensive test coverage. No extraneous or out-of-scope modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@plugins/woocommerce/tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php`:
- Around line 69-78: The test currently only checks that $params['before'] is
still a DateTime instance; instead, capture a clone of the original DateTime
(e.g., $original = clone $dt) before calling $this->invoke_get_cache_key($store,
$params) and assert that the original and the post-call DateTime are equal in
both timestamp and timezone (for example compare formatted string or both
getTimestamp() and getTimezone()->getName()) so
CustomersDataStore::get_cache_key (invoked via invoke_get_cache_key) is proven
not to mutate the DateTime value.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ac6e651d-5d38-432a-b049-2271d0122f38

📥 Commits

Reviewing files that changed from the base of the PR and between cf5f683 and ba2b03c.

📒 Files selected for processing (3)

• plugins/woocommerce/changelog/fix-64177-transient-cache-key-datetime
• plugins/woocommerce/src/Admin/API/Reports/DataStore.php
• plugins/woocommerce/tests/php/src/Admin/API/Reports/DataStore/CacheKeyTest.php

[prettyboymp]

Heads up: test_user_creation failure traces to a latent cache-invalidation bug your fix has surfaced — not a regression in your change

Nice work tracking down the microsecond-instability bug. The fix itself is correct, but it's exposing a separate latent bug in WC_Cache_Helper::get_transient_version() that's causing WC_Admin_Tests_API_Reports_Customers::test_user_creation to fail.

What's happening

The test does three things in the same wall-clock second:

1. REST call #1 (no orders) → caches []
2. Create order + WC_Helper_Queue::run_all_pending('wc-admin-data') → OrdersScheduler calls ReportsCache::invalidate() (correctly)
3. REST call #2 → expects 1 customer

Pre-fix, this passed because every request had unique microseconds in the cache key, so call #2 always missed the cache. Post-fix, calls #1 and #2 produce identical cache keys (correct second-precision behavior). But Cache::invalidate() calls WC_Cache_Helper::get_transient_version( 'woocommerce_reports', true ), which stamps the version with (string) time(). Within the same second, the "new" version equals the old one — invalidation becomes a silent no-op, and call #2 reads the stale [] from cache.

Reproducer:

$v1 = WC_Cache_Helper::get_transient_version( 'woocommerce_reports' );      // "1777776460"
WC_Cache_Helper::get_transient_version( 'woocommerce_reports', true );      // refresh
$v2 = WC_Cache_Helper::get_transient_version( 'woocommerce_reports' );      // still "1777776460"
// $v1 === $v2 → invalidation no-op

Suggested path

The cache-helper bug is its own concern and shouldn't gate this PR — it's tracked at #64557.

For this PR, fix the failing test by bypassing the cache in this test class (it's a query-semantics test, not a cache test):

Preferred — disable caching for the whole test class via the existing filter:

public function setUp(): void {
    parent::setUp();
    add_filter( 'woocommerce_analytics_report_should_use_cache', '__return_false' );
}

public function tearDown(): void {
    remove_filter( 'woocommerce_analytics_report_should_use_cache', '__return_false' );
    parent::tearDown();
}

This is a one-place declaration that the test class is exercising query semantics, not the cache layer — the filter at DataStore.php:307 exists for exactly this kind of caller.

Alternative — pass force_cache_refresh on the post-mutation reads:

$request->set_query_params( array( 'per_page' => 10, 'force_cache_refresh' => true ) );

Equivalent for the failing assertion, but more verbose if multiple tests in the file have the same shape.

Either preserves the test's intent (verifying that admins-with-orders and customers appear in the customers list) without depending on cache-invalidation timing — please avoid sleep(1) since it slows the suite and hides the real timing assumption.

Otherwise the PR fix itself looks good — small, focused, well-tested, and addresses the cache-key instability accurately. Once the test is updated, this should be ready to merge.

[prettyboymp]

@shsajalchowdhury, changes look good. Thank you.