Fix #82: Fully honor Guzzle 'sink' option on cache hits and revalidations

[Kevinrob]

Description

This PR fixes issue #82 by ensuring that Guzzle's sink request option is fully respected when a response is served from the cache.

Changes

• Sink Support: Implemented manual body streaming to the provided sink destination during Cache HITs, STALE responses, and 304 Not Modified revalidations.
• Stream Robustness: Added checks for stream seekability before rewinding to prevent crashes with non-seekable sinks (like pipes).
  Architecture: Centralized sink application logic into a protected applySink method.
  PHPUnit Maintenance: Updated phpunit.xml.dist to use /tmp for the results cache, fixing a permission warning when running tests in Docker.
  Regression Tests: Added a comprehensive SinkTest suite covering file paths, resources, and stream interfaces as sinks.

Verification

• Verified that files are correctly populated on cache hits.
• Verified that revalidation (304) correctly writes the cached body to the new sink.
• All tests passed on PHP 8.5.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the Guzzle cache middleware to fully support the sink request option for cached responses. Previously, the sink option was not honored when a response was served from the cache or during revalidation. This change ensures that response bodies from cache hits, stale responses, and 304 Not Modified revalidations are correctly streamed to the specified sink, improving consistency and preventing unexpected behavior for applications relying on this Guzzle feature.

Highlights

• Sink Support: Implemented manual body streaming to the provided 'sink' destination during Cache HITs, STALE responses, and '304 Not Modified' revalidations.
• Stream Robustness: Added checks for stream seekability before rewinding to prevent crashes with non-seekable sinks (like pipes).
• Architecture: Centralized sink application logic into a protected 'applySink' method.
• PHPUnit Maintenance: Updated 'phpunit.xml.dist' to use '/tmp' for the results cache, fixing a permission warning when running tests in Docker.
• Regression Tests: Added a comprehensive 'SinkTest' suite covering file paths, resources, and stream interfaces as sinks.

Changelog

• phpunit.xml.dist
  • Added cacheResultFile attribute to store PHPUnit results in /tmp.
• src/CacheEntry.php
  • Imported GuzzleHttp\Psr7\Utils directly.
  • Updated restoreStreamBody to use the imported Utils class.
• src/CacheMiddleware.php
  • Imported GuzzleHttp\Psr7\Utils and Psr\Http\Message\StreamInterface.
  • Refactored cache hit, stale, and revalidation paths to apply the sink option via a new applySink method.
  • Modified the then callback for promises to also apply sink to stale responses on errors.
  • Updated addToCache to use the imported Utils class for stream creation.
  • Introduced protected function applySink(ResponseInterface $response, array $options): ResponseInterface to centralize sink application logic.
  • Introduced protected function writeToSink(StreamInterface $body, $sink) to handle writing stream content to various sink types (string path, resource, StreamInterface) with seekability checks.
  • Changed the return type of getStaleResponse from void to null for consistency.
• tests/SinkTest.php
  • Added a new test file SinkTest.php.
  • Included testSinkWithCacheHit to verify sink functionality with cached responses for file paths, resources, and stream interfaces.
  • Included testSinkWithRevalidation304 to verify sink functionality during 304 revalidations.
  • Added a helper method rrmdir for directory cleanup in tests.

Activity

• No human activity (comments, reviews, etc.) has been detected on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request successfully implements support for Guzzle's sink request option when serving responses from the cache. A security audit found no significant vulnerabilities introduced by these changes, confirming the correct handling of various sink types and stream seekability. However, I've identified a couple of areas for improvement: the new writeToSink method could be more robust with explicit type checking for the sink parameter, and a helper function in the new test file (SinkTest.php) has a minor bug and doesn't adhere to coding standards.

[shadowhand]

This is intentionally missing declare(strict_types = 1); or does this project not use strict types?

[Kevinrob]

The project does not use declare(strict_types=1); in any of its existing files (src or tests). I omitted it here to maintain consistency with the rest of the codebase. If we decide to adopt strict types, it should be done project-wide in a separate PR.