Move resource ownership into utopia-php/di#220
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization 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 |
ChiragAgg5k
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates HTTP “resource” ownership out of utopia-php/http and into utopia-php/di by injecting a DI container into Http and using it for request-scoped resources and adapter-provided raw resources.
Changes:
- Removed
Http::setResource()/Http::getResource()and delegated resource resolution/cleanup toUtopia\DI\Container. - Updated FPM/Swoole adapters to pass adapter-specific raw resources into
Http’s request lifecycle. - Updated tests and docs to register custom resources directly on the DI container, and bumped minimum PHP version to 8.2.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/Http/Http.php |
Introduces DI container integration for resource wiring, resolution, refresh/purge lifecycle, and removes legacy resource APIs. |
src/Http/Adapter/FPM/Server.php |
Passes adapter raw resources through the onRequest callback to be registered per request. |
src/Http/Adapter/Swoole/Server.php |
Same as FPM: provides raw Swoole request/response as request-scoped resources. |
tests/HttpTest.php |
Refactors tests to use the DI container for resource registration/resolution instead of Http. |
README.md |
Updates docs/examples to use the DI container and the new Http constructor signature. |
docs/Getting-Starting-Guide.md |
Attempts to update the getting-started snippet and PHP requirement. |
composer.json |
Adds utopia-php/di dependency, VCS repository entry, and raises PHP constraint to >=8.2. |
composer.lock |
Locks the new DI dependency and updates platform PHP to >=8.2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| namespace Utopia\Http; | ||
|
|
||
| use Utopia\DI\Container as DIContainer; |
| @@ -82,21 +87,21 @@ public function testCanGetEnvironmentVariable(): void | |||
|
|
|||
| public function testCanGetResources(): void | |||
There was a problem hiding this comment.
We should just remove this test instead right?
| @@ -119,11 +124,11 @@ public function testCanGetResources(): void | |||
|
|
|||
| public function testCanGetDefaultValueWithFunction(): void | |||
Summary
Utopia\DI\ContainerHttpand use it internally for request-scoped framework resources likerequest,response,route,error, and adapter-specific raw resourcesHttp::setResource()->inject()semantics in HTTP, but delegate resolution and lifecycle cleanup to DIVerification
vendor/bin/phpunit --configuration phpunit.xml --do-not-cache-result tests/HttpTest.phpvendor/bin/phpstan analyse -c phpstan.neon --memory-limit 512MNotes
Http::setResource()/Http::getResource()usage in favor of explicit DI ownershipvendor/bin/phpunit --configuration phpunit.xml --do-not-cache-resultrun still hits the existing e2e environment failures because this machine cannot resolve thefpmandswooleservice hosts