Add configurable Cache-Control header to command responses - #32
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughAdds a configurable ChangesCache-Control configuration and response handling
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant BundleConfiguration
participant StixxOpenApiCommandExtension
participant CommandController
participant Response
BundleConfiguration->>StixxOpenApiCommandExtension: provide validation.cache_control
StixxOpenApiCommandExtension->>CommandController: inject cache-control parameter
CommandController->>Response: set Cache-Control header when enabled
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Default every command response to Cache-Control: no-store to prevent browsers and intermediaries from caching sensitive API data. The value is configurable via stixx_openapi_command.cache_control and can be set to null to disable.
5e390ec to
f342eae
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/Unit/DependencyInjection/ConfigurationTest.php (1)
45-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd Configuration-level tests for custom and disabled
cache_controlvalues.The controller tests cover default, custom, and null scenarios, but the Configuration tests only verify the default. Per path instructions, aim for 100% coverage of logical scenarios — add cases asserting that
cache_control: ~yieldsnullandcache_control: 'no-cache, private'yields the custom string.As per coding guidelines, "Aim for 100% coverage of logical scenarios" and "Use
#[DataProvider]for multiple similar scenarios."♻️ Suggested test additions
public function testCustomConfig(): void { // ... existing code ... } + public function testCacheControlCanBeDisabled(): void + { + // Arrange + $configuration = new Configuration(); + $processor = new Processor(); + $customConfig = [ + 'cache_control' => null, + ]; + + // Act + $config = $processor->processConfiguration($configuration, [$customConfig]); + + // Assert + self::assertNull($config['cache_control']); + } + + public function testCacheControlAcceptsCustomValue(): void + { + // Arrange + $configuration = new Configuration(); + $processor = new Processor(); + $customConfig = [ + 'cache_control' => 'no-cache, private', + ]; + + // Act + $config = $processor->processConfiguration($configuration, [$customConfig]); + + // Assert + self::assertSame('no-cache, private', $config['cache_control']); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/DependencyInjection/ConfigurationTest.php` around lines 45 - 72, Add Configuration-level coverage for custom and null cache_control values alongside testCustomConfig, using a #[DataProvider] for the similar scenarios. Assert that 'no-cache, private' is preserved and a null (~) value resolves to null, while retaining the existing defaults for other configuration keys.Source: Path instructions
tests/Unit/Controller/CommandControllerTest.php (1)
154-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce test setup duplication with a data provider or helper.
The three new cache-control tests share nearly identical arrange blocks (mock setup, envelope, bus, resolver, responder). Per path instructions, "Use
#[DataProvider]for multiple similar scenarios." Extracting a shared factory or using#[DataProvider]would reduce maintenance burden and make adding future scenarios trivial.As per coding guidelines, "Use
#[DataProvider]for multiple similar scenarios."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/Controller/CommandControllerTest.php` around lines 154 - 245, Reduce duplication across testInvokeSetsDefaultCacheControlHeader, testInvokeOmitsCacheControlHeaderWhenDisabled, and testInvokeSetsCustomCacheControlHeader by extracting their shared command, request, mock, envelope, bus, resolver, and responder setup into a reusable helper or data provider. Prefer a PHPUnit #[DataProvider] to supply the cacheControl value and expected assertion for each scenario, while preserving the existing default, disabled, and custom header behaviors.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Unit/Controller/CommandControllerTest.php`:
- Around line 212-213: Strengthen the disabled-cache-control assertion in the
relevant controller test by replacing the exact-value check with a substring
assertion that verifies the response Cache-Control header does not contain
“no-store”, including when Symfony adds other directives.
---
Nitpick comments:
In `@tests/Unit/Controller/CommandControllerTest.php`:
- Around line 154-245: Reduce duplication across
testInvokeSetsDefaultCacheControlHeader,
testInvokeOmitsCacheControlHeaderWhenDisabled, and
testInvokeSetsCustomCacheControlHeader by extracting their shared command,
request, mock, envelope, bus, resolver, and responder setup into a reusable
helper or data provider. Prefer a PHPUnit #[DataProvider] to supply the
cacheControl value and expected assertion for each scenario, while preserving
the existing default, disabled, and custom header behaviors.
In `@tests/Unit/DependencyInjection/ConfigurationTest.php`:
- Around line 45-72: Add Configuration-level coverage for custom and null
cache_control values alongside testCustomConfig, using a #[DataProvider] for the
similar scenarios. Assert that 'no-cache, private' is preserved and a null (~)
value resolves to null, while retaining the existing defaults for other
configuration keys.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31e4b8ce-f2af-4770-84f7-5095c1414065
📒 Files selected for processing (7)
README.mdconfig/controller.phpsrc/Controller/CommandController.phpsrc/DependencyInjection/Configuration.phpsrc/DependencyInjection/StixxOpenApiCommandExtension.phptests/Unit/Controller/CommandControllerTest.phptests/Unit/DependencyInjection/ConfigurationTest.php
…ertion Cover default, disabled, and custom cache_control values through the full kernel request lifecycle. Replace assertNotSame with assertStringNotContainsString in the disabled-cache unit test so it holds even when Symfony appends extra directives.
1 participant
Summary
cache_controlconfiguration option (default:no-store) that setsCache-Controlon every command response, preventing browsers and intermediaries from caching sensitive API data.stixx_openapi_command.cache_control; set to~(null) to disable.defaults.cache_headers).Changed files
src/DependencyInjection/Configuration.phpcache_controlscalar node (default'no-store')src/DependencyInjection/StixxOpenApiCommandExtension.phpconfig/controller.php$cacheControlarg toCommandControllersrc/Controller/CommandController.php?string $cacheControl, setsCache-Controlheader on responsesREADME.mdtests/Unit/Controller/CommandControllerTest.phptests/Unit/DependencyInjection/ConfigurationTest.phpConfiguration
Test plan
no-storein Cache-Controlnull): controller does not override Symfony's computed defaultSummary by CodeRabbit
Cache-Controlheaders to command responses.no-store, with support for custom values or disabling the header.