Fix: Extended ResourcePool never updates stale cached entries

[Wanne Van Camp (wannevancamp)]

Summary

The save() method in Extended ResourcePool has two issues that prevent cached entries from ever being updated once initially stored in the PSR-6 cache.

Bug 1: parent::save() gates the cache write

Standard::save() returns false when the resource already exists in the in-memory pool — which is the case whenever warmUp() has loaded it from cache earlier in the same request. Extended::save() treats this false as "nothing to do" and returns before reaching the PSR-6 cache write:

if (!parent::save($resource)) {
    return false; // ← Never reaches Redis/cache write
}

Bug 2: isHit() prevents overwriting

Even if bug 1 didn't exist, the isHit() guard explicitly prevents overwriting existing cache entries:

if (!$cacheItem->isHit()) { // ← Skips write for existing entries
    $cacheItem->set(guzzle_json_encode($resource));
    $this->cacheItemPool->save($cacheItem);
}

Combined effect

Once an Entry or Asset is cached in the PSR-6 pool, the SDK can never update it even when a fresh version is fetched directly from the Contentful API. The only way to get fresh data is to flush the entire cache store.

Reproduction

1. Configure the client with cacheContent: true and autoWarmup: true
2. Fetch an entry via getEntries() it gets cached in the PSR-6 pool
3. Update the entry in Contentful
4. Fetch the same entry again via getEntries() the API returns fresh data
5. On the same request: the in-memory pool has the fresh version (mapper mutates in-place)
6. On the next request: warmUp() loads the stale version from the PSR-6 cache, because save() never wrote the fresh version

Fix

• Ignore parent::save()'s return value. The in-memory pool is still updated by the parent call, but we always proceed to the cache write
• Remove the isHit() guard so stale cache entries are overwritten with fresh data

Summary by Bito

The PR fixes a bug in the Extended ResourcePool where cached entries were never updated once stored in the PSR-6 cache. The save() method had two issues: it gated cache writes based on parent::save() return value and prevented overwriting existing cache entries with an isHit() check. This combined effect meant stale data persisted indefinitely. The fix ignores the parent return value and removes the isHit() guard to ensure fresh API data always updates the cache.

Detailed Changes

• Removes the condition that checks parent::save() return value, ensuring the method always proceeds to cache operations in Extended.php
• Eliminates the isHit() guard before setting and saving cache items, allowing overwrites of existing entries in Extended.php

[bito-code-review]

Code Review Agent Run #d14bb7

Actionable Suggestions - 0

Review Details

• Files reviewed - 1 · Commit Range: fd8aad1..fd8aad1

  • src/ResourcePool/Extended.php
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at jared.jolton@contentful.com.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted	Summary
Bug Fix - Fix Extended ResourcePool Cache Update Issue	Extended.php	Modified the save method to ignore parent::save() return value and remove isHit() guard, ensuring cached entries are always updated with fresh data.

[bito-code-review]

Impact Analysis by Bito

Interaction Diagram

sequenceDiagram
participant Client
participant ResourceBuilder
participant Extended as Extended ResourcePool<br/>🔄 Updated | ●●○ Medium
participant Standard as Standard ResourcePool
participant Cache as PSR Cache Pool
Client->>ResourceBuilder: build(resource data)
ResourceBuilder->>Extended: save(resource)
Extended->>Standard: parent::save(resource)
Standard-->>Extended: return result
alt [autoWarmup enabled && type in warmupTypes]
Extended->>Cache: getItem(cacheKey)
Cache-->>Extended: return CacheItem
Extended->>Cache: set(encoded resource)
Extended->>Cache: save(CacheItem)
    end
Extended-->>ResourceBuilder: return true
ResourceBuilder-->>Client: return built resource
Note over Extended: Always performs cache update<br/>removed hit check for freshness

Loading

---

The Extended ResourcePool's save method now always updates the cache for auto-warmup resource types, even if the cache item was already present, ensuring data freshness but potentially increasing cache write operations. This change affects resource fetching in the Contentful PHP SDK by modifying cache behavior during resource building.

Code Paths Analyzed

Impact:
The change alters the save method in the Extended ResourcePool class to ignore the result of the parent save operation, always perform caching warmup for eligible resource types, and return true unconditionally, potentially affecting caching behavior for resources not stored in the pool.

Flow:
Resource save request -> Call parent::save (result ignored) -> Generate cache key -> Check autoWarmup and warmupTypes -> Retrieve cache item -> Set cache item with JSON-encoded resource -> Save cache item -> Return true

Direct Changes (Diff Files):
• src/ResourcePool/Extended.php [106-124] — Modified the save method to remove conditional checks on parent save result and cache hit status, ensuring caching always occurs for warmup-eligible types and method always returns true.

Repository Impact:
• Resource Pool and Caching Logic: Changes the behavior to cache resources even when the parent pool does not store them, impacting memory usage and cache consistency for non-standard resource types.

Cross-Repository Dependencies:
• None.

Database/Caching Impact:
• None

API Contract Violations:
• None.

Infrastructure Dependencies:
• None.

Testing Recommendations

Frontend Impact:
• None.

Service Integration:
• Test the save method with resource types not included in the parent pool's saved types (e.g., types other than 'ContentType', 'Environment', 'Space') to verify that caching warmup occurs and the method returns true.
• Verify that resources are correctly cached in the cache pool when autoWarmup is enabled and the resource type is in warmupTypes, regardless of parent save outcome.

Data Serialization:
• None.

Privacy Compliance:
• None.

Backward Compatibility:
• Test existing integrations that may rely on the save method returning false for non-stored resource types, as the method now always returns true.

OAuth Functionality:
• None

Reliability Testing:
• None

Additional Insights:
• Test cache behavior when a cache item already exists (isHit returns true) to ensure the cache is still updated with the new resource data.
• Run unit tests for the Extended ResourcePool class to ensure no regressions in resource management and caching logic.

_{Analysis based on known dependency patterns and edges. Actual impact may vary.}