Throw EmbedException when a _self-embedded resource returns no body#374
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #374 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 687 688 +1
===========================================
Files 96 96
Lines 1722 1726 +4
===========================================
+ Hits 1722 1726 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
87cc22d to
044b16a
Compare
#[CacheableResponse]/#[DonutCache] restore only the view on a cache hit, not the body, so a resource embedded as _self (whose body linkSelf reads) breaks silently: dev dies on assert, prod does an empty foreach(null) merge. Replace the assert with a domain exception whose message names the fix (use #[Cacheable] value cache). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The tests (HttpResourceObjectTest, HttpRequestCurlTest) use Koriym\PhpServer\PhpServer directly, but it was only pulled in indirectly via bear/devtools (^1.0). On --prefer-lowest that resolved to 1.0.0, which predates the PhpServer class, causing a 'Class not found' fatal. Require it directly at ^1.0.1 (the version that introduced PhpServer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0f36c85 to
7ab16b7
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
A resource embedded as
_selfhas its body flattened into the parent byEmbedInterceptor::linkSelf, which reads$result->body. But#[CacheableResponse]/#[DonutCache]restore only the view on a cache hit, not the body. So a_self-embedded resource with those cache attributes breaks silently on the second (cache-hit) request:assert(is_array($result->body))with no explanationforeach(null)merge (a warning, then a blank body)This PR replaces the assert with a domain exception whose message names the fix, turning a confusing failure into a fail-fast with actionable guidance:
Test
Added
testSelfLinkNullBodyThrowsEmbedException. Independently verified that reverting the guard back to the oldassertmakes the test fail — confirming the test truly depends on the new behavior rather than passing incidentally. Full suite green: 404 tests, 704 assertions.🤖 Generated with Claude Code