fix: add strict_types, const visibility, and one-line docblocks #348
Conversation
koriym
commented
Jan 15, 2026
koriym
commented
- Add missing declare(strict_types=1)
- Add public visibility to GLOBAL_KEY constant
- Convert multi-line docblocks to one-line format
Add optional batch processing mechanism that reduces resource requests during Crawl traversal. - Add `batch` parameter to #[Link] annotation to specify BatchResolver - Add BatchResolverInterface for implementing custom batch logic - Add Requests class with helper methods (getQueryParam, mapResults) - Add Results class for URI-keyed result mapping - Add BatchResolverFactoryInterface for DI integration - Modify Linker to process batch-enabled links efficiently 100% backward compatible - existing code works without changes. Batch processing is opt-in via the new `batch` parameter.
Rename BatchResolver to DataLoader following GraphQL's DataLoader pattern: - src/Batch/ → src/DataLoader/ - BatchResolverInterface → DataLoaderInterface - BatchResolverFactory → DataLoaderFactory - LikeBatchResolver → LikeDataLoader The naming better reflects the pattern's purpose and origin.
- Add DataLoaderModule to src/Module for easy installation - Add edge case tests for Requests class (empty URIs, missing keys) - Add additional Linker tests (multiple invokes, empty comments) - Add demo for DataLoader usage pattern - Update Link annotation property to use dataLoader parameter
- Apply constructor property promotion in Requests class - Use early exit pattern for better code readability - Add psalm-suppress annotations for intentional mixed types - Fix fully qualified class name reference in Types.php - Format single-element arrays on single lines
- Extract DataLoader processing logic into separate DataLoaderProcessor class - Linker now delegates to DataLoaderProcessor instead of handling DataLoader logic directly - Reduces Linker's class complexity below PHPMD threshold of 50 - Add DataLoaderProcessorTest for 100% coverage of edge cases
- Remove DataLoaderFactory and DataLoaderFactoryInterface - DataLoaderProcessor now uses InjectorInterface directly - Simplifies the architecture by eliminating unnecessary abstraction
- Remove Requests and Results classes
- Simplify DataLoaderInterface to __invoke(array $queries): array
- Auto-infer key from URI template (no dataLoaderKey needed)
- Support both URI formats: {?var} and param={var}
- Add RowMustContainKeyInDataLoaderException
Document that $queries is passed as associative arrays (not just values) to support multiple key parameters in URI templates.
- Rename DataLoaderProcessor class to DataLoader - Rename process() method to load() - Add domain types to Types.php (DataLoaderQuery, DataLoaderRow, etc.) - Update DataLoaderInterface to use imported types
- Remove separate DataLoaderModule - DataLoader is now included by default in ResourceClientModule - Update demo to use simplified module setup
The class name already explains the context.
WalkthroughDataLoader機能を導入します。複数のリソースリクエストをバッチで処理するためのDataLoaderインターフェースと実装、Link annotationへの統合、Linkerの統合、型定義、包括的なテスト、およびデモアプリケーションを追加します。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Linker
participant DataLoader
participant DataLoaderImpl
participant Resource
Client->>Linker: invoke(get, article?id=1)
Linker->>Resource: fetch article
Resource-->>Linker: article data
Linker->>Linker: annotationCrawl (collect link annotations)
alt Link with DataLoader
Linker->>DataLoader: load(annotations, link, bodyList)
DataLoader->>DataLoader: extract keys from template
DataLoader->>DataLoader: build per-item queries
DataLoader->>DataLoaderImpl: __invoke(aggregated queries)
DataLoaderImpl-->>DataLoader: batch rows result
DataLoader->>DataLoader: group by keys
DataLoader->>Linker: distribute into bodyList
else Link without DataLoader
Linker->>Resource: fetch per resource
Resource-->>Linker: individual results
end
Linker-->>Client: crawled tree with loaded data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (16)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2026-01-09T18:36:49.791ZApplied to files:
📚 Learning: 2026-01-09T15:46:03.556ZApplied to files:
📚 Learning: 2026-01-09T18:36:49.791ZApplied to files:
📚 Learning: 2026-01-09T15:46:03.556ZApplied to files:
📚 Learning: 2026-01-09T19:38:54.994ZApplied to files:
📚 Learning: 2026-01-09T18:36:49.791ZApplied to files:
📚 Learning: 2026-01-09T19:38:54.994ZApplied to files:
📚 Learning: 2025-10-28T00:42:34.230ZApplied to files:
🧬 Code graph analysis (7)src/DataLoader/DataLoaderInterface.php (3)
tests/Fake/FakeVendor/Sandbox/DataLoader/LikeDataLoader.php (2)
tests/DataLoader/LinkerDataLoaderTest.php (4)
src/DataLoader/DataLoader.php (4)
src/Annotation/Link.php (2)
src/Module/ResourceClientModule.php (1)
demo/4.link-crawl-dataloader.php (3)
🔇 Additional comments (34)
✏️ Tip: You can disable this entire section by setting 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 #348 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 541 574 +33
===========================================
Files 84 86 +2
Lines 1370 1453 +83
===========================================
+ Hits 1370 1453 +83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add missing declare(strict_types=1) - Add public visibility to GLOBAL_KEY constant - Convert multi-line docblocks to one-line format
koriym
force-pushed
the
claude/fix-error-346-UZqAW
branch
from
68c841d to
20b61bb
Compare
- Fix parse_str null/false argument errors by separating type checks - Remove orphaned gh-pages submodule entry (no .gitmodules)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
