Initial implementation of Ray.FakeQuery#1
Merged
Merged
Conversation
- FakeQueryModule: Ray.Di module that scans interfaceDir and binds #[DbQuery] interfaces to JSON-backed fake implementations - FakeQueryInterceptor: intercepts #[DbQuery] method calls, loads JSON, hydrates to return type; void methods are no-ops - JsonHydrator: hydrates JSON data to entities (camelCase and snake_case key support, constructor and property-based hydration) - FakeQueryConfig: value object holding fakeDir path - FakeJsonNotFoundException: thrown when JSON fixture file not found
- FakeQueryModuleTest: 6 tests covering entity hydration, list hydration, explicit row type, void no-op, and missing JSON exception - Test fixtures: TodoQueryInterface, TodoCommandInterface, TodoEntity - JSON fixtures: todo_item.json, todo_list.json (camelCase keys) - Fix: explicit JsonHydrator binding in FakeQueryModule - Fix: exclude tests/tmp from PHPStan analysis - Fix: migrate phpunit.xml.dist schema
- Support .jsonl (JSON Lines) for collection queries, .json for single rows - Add UnknownFakeJsonException: validate fakeDir files against #[DbQuery] IDs - Nullable return type with missing file returns null - Fix exception hierarchy: RuntimeException is no longer final - Fix nullable union type (TodoEntity|null) handling in isNullable() - Extract resolveArgument() to eliminate else (coding rule compliance) - Add domain types (Types.php): JsonRow, JsonRowList, DbQueryIdSet - Remove skeleton FakeQuery class and test - Fix composer scripts to use ./vendor/bin/phpunit
Add UserEntity with constructor hydration and UserQueryInterface with union type nullable return. Test camelCase key matching and optional parameter defaults in resolveArgument(). Mark unreachable union type branch in isNullable() and defensive edge cases with codeCoverageIgnore.
Reviewer's GuideIntroduces the initial Ray.FakeQuery implementation that wires Ray.Di DbQuery interfaces to a fake JSON/JSONL-backed interceptor, adds JSON hydrator and configuration, validates fixture files against query IDs, updates docs and composer scripts, loosens RuntimeException inheritance, and replaces old FakeQuery entrypoint/tests with a module-based design and comprehensive PHPUnit coverage. Sequence diagram for FakeQueryInterceptor handling a DbQuery method callsequenceDiagram
actor Client
participant DbQueryInterface
participant FakeQueryInterceptor
participant FakeQueryConfig
participant Filesystem
participant ReturnEntity
participant JsonHydrator
Client->>DbQueryInterface: callMethod()
DbQueryInterface->>FakeQueryInterceptor: invoke(invocation)
FakeQueryInterceptor->>FakeQueryInterceptor: getMethod() and DbQuery annotation
FakeQueryInterceptor->>FakeQueryInterceptor: inspect return type
alt return type is void
FakeQueryInterceptor-->>DbQueryInterface: return null
DbQueryInterface-->>Client: null
else non void
FakeQueryInterceptor->>FakeQueryConfig: read fakeDir
FakeQueryInterceptor->>FakeQueryInterceptor: decide extension .json or .jsonl
FakeQueryInterceptor->>Filesystem: file_exists(fakeDir/id.ext)
alt file missing and nullable
FakeQueryInterceptor-->>DbQueryInterface: return null
DbQueryInterface-->>Client: null
else file missing and not nullable
FakeQueryInterceptor-->>Client: throw FakeJsonNotFoundException
else file exists
FakeQueryInterceptor->>Filesystem: file_get_contents(path)
Filesystem-->>FakeQueryInterceptor: content
alt row (single entity or scalar)
FakeQueryInterceptor->>FakeQueryInterceptor: json_decode(content)
FakeQueryInterceptor->>ReturnEntity: __invoke(method)
ReturnEntity-->>FakeQueryInterceptor: entityClass or null
FakeQueryInterceptor->>JsonHydrator: hydrate(data, entityClass, true)
else row list (collection)
FakeQueryInterceptor->>FakeQueryInterceptor: parseJsonl(content)
FakeQueryInterceptor->>ReturnEntity: __invoke(method)
ReturnEntity-->>FakeQueryInterceptor: entityClass
FakeQueryInterceptor->>JsonHydrator: hydrate(dataList, entityClass, false)
end
JsonHydrator-->>FakeQueryInterceptor: hydratedResult
FakeQueryInterceptor-->>DbQueryInterface: hydratedResult
DbQueryInterface-->>Client: hydratedResult
end
end
Class diagram for initial Ray.FakeQuery module-based designclassDiagram
direction LR
class FakeQueryModule {
-string fakeDir
-string interfaceDir
+__construct(string fakeDir, string interfaceDir, AbstractModule module)
+configure() void
-validateFakeFiles(array classes) void
-collectDbQueryIds(array classes) array
}
class FakeQueryConfig {
+string fakeDir
+__construct(string fakeDir)
}
class JsonHydrator {
+hydrate(mixed data, string entityClass, bool isRow) mixed
-hydrateRow(mixed data, string entityClass) mixed
-hydrateRowList(mixed data, string entityClass) mixed
-hydrateOne(array row, string entityClass) object
-hydrateProperties(ReflectionClass refClass, array row) object
-hydrateConstructor(ReflectionClass refClass, array row) object
-resolveArgument(ReflectionParameter param, array row) mixed
}
class FakeQueryInterceptor {
-FakeQueryConfig config
-JsonHydrator hydrator
-ReturnEntityInterface returnEntity
+__construct(FakeQueryConfig config, JsonHydrator hydrator, ReturnEntityInterface returnEntity)
+invoke(MethodInvocation invocation) mixed
-parseJsonl(string content) array
-isNullable(ReflectionType returnType) bool
}
class Types {
<<final>>
%% @psalm-type JsonRow = array
%% @psalm-type JsonRowList = list
%% @psalm-type DbQueryIdSet = array
}
class RuntimeException {
}
class FakeJsonNotFoundException {
+__construct(string filename, string fakeDir)
}
class UnknownFakeJsonException {
+__construct(string filename, string fakeDir)
}
class AbstractModule {
<<external>>
+configure() void
}
class MethodInterceptor {
<<external>>
+invoke(MethodInvocation invocation) mixed
}
class MethodInvocation {
<<external>>
+getMethod() ReflectionMethod
}
class ReturnEntityInterface {
<<external>>
+__invoke(ReflectionMethod method) string
}
class ReturnEntity {
<<external>>
}
class Queries {
<<external>>
+fromDir(string interfaceDir) Queries
+classes list~class-string~
}
class DbQuery {
<<external annotation>>
+string id
+string type
}
class DocBlockFactoryInterface {
<<external>>
}
class DocBlockFactory {
<<external>>
+createInstance() DocBlockFactoryInterface
}
%% Inheritance and implementation
FakeQueryModule --|> AbstractModule
FakeQueryInterceptor ..|> MethodInterceptor
FakeJsonNotFoundException --|> RuntimeException
UnknownFakeJsonException --|> RuntimeException
ReturnEntity ..|> ReturnEntityInterface
%% Composition and usage
FakeQueryModule --> FakeQueryConfig
FakeQueryModule --> JsonHydrator
FakeQueryModule --> FakeQueryInterceptor
FakeQueryModule --> DocBlockFactoryInterface
FakeQueryModule --> ReturnEntityInterface
FakeQueryModule ..> Queries
FakeQueryModule ..> DbQuery
FakeQueryModule ..> ReflectionClass
FakeQueryInterceptor --> FakeQueryConfig
FakeQueryInterceptor --> JsonHydrator
FakeQueryInterceptor --> ReturnEntityInterface
FakeQueryInterceptor ..> DbQuery
JsonHydrator ..> ReflectionClass
JsonHydrator ..> ReflectionParameter
FakeJsonNotFoundException --> RuntimeException
UnknownFakeJsonException --> RuntimeException
FakeQueryModule ..> UnknownFakeJsonException
FakeQueryInterceptor ..> FakeJsonNotFoundException
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a fake-query system that intercepts DbQuery-annotated methods to load JSON/JSONL fixtures, hydrate results into entities, introduce config and module wiring, new exceptions, and updated tests/docs; nullable return types return null when fixtures are missing, non-nullable throw. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Interceptor as FakeQueryInterceptor
participant Config as FakeQueryConfig
participant FS as Filesystem
participant Hydrator as JsonHydrator
participant ReturnEntity as ReturnEntityInterface
Client->>Interceptor: invoke(method)
Interceptor->>Interceptor: read DbQuery attribute, inspect return type
Interceptor->>Config: get fakeDir
Interceptor->>FS: read {fakeDir}/{queryId}.{json|jsonl}
alt file exists
FS-->>Interceptor: content
Interceptor->>Interceptor: parse (.json -> object, .jsonl -> lines)
Interceptor->>ReturnEntity: determine entity class/type
Interceptor->>Hydrator: hydrate(data, entityClass, isRow)
Hydrator-->>Client: entity or list
else file missing & nullable
Interceptor-->>Client: return null
else file missing & non-nullable
Interceptor-->>Client: throw FakeJsonNotFoundException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
FakeQueryModule::validateFakeFiles, a failedglob(e.g., unreadable or non-existent directory) is treated the same as "no files", which can hide misconfiguration; consider detecting this case and surfacing a configuration error instead of silently skipping validation. - In
FakeQueryInterceptor::parseJsonl,json_decoderesults are used without checking for decode errors, so malformed JSONL lines will yieldnullrows that then flow into hydration; you may want to validate each line (e.g., viajson_last_error()or strict decoding) and throw a dedicated exception with filename and line number. - In
JsonHydrator::resolveArgument, falling back tonullfor missing non-optional constructor parameters can lead to a less clearTypeErrorwhen instantiating entities; consider throwing a domain-specific exception when a required parameter has no matching JSON key instead of silently passingnull.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `FakeQueryModule::validateFakeFiles`, a failed `glob` (e.g., unreadable or non-existent directory) is treated the same as "no files", which can hide misconfiguration; consider detecting this case and surfacing a configuration error instead of silently skipping validation.
- In `FakeQueryInterceptor::parseJsonl`, `json_decode` results are used without checking for decode errors, so malformed JSONL lines will yield `null` rows that then flow into hydration; you may want to validate each line (e.g., via `json_last_error()` or strict decoding) and throw a dedicated exception with filename and line number.
- In `JsonHydrator::resolveArgument`, falling back to `null` for missing non-optional constructor parameters can lead to a less clear `TypeError` when instantiating entities; consider throwing a domain-specific exception when a required parameter has no matching JSON key instead of silently passing `null`.
## Individual Comments
### Comment 1
<location path="src/FakeQueryInterceptor.php" line_range="65" />
<code_context>
+ throw new FakeJsonNotFoundException($dbQuery->id . $ext, $this->config->fakeDir);
+ }
+
+ $content = (string) file_get_contents($jsonFile);
+ /** @psalm-suppress MixedAssignment */
+ $data = $isRow
</code_context>
<issue_to_address>
**issue (bug_risk):** Casting `file_get_contents` result to string hides read failures and can lead to confusing downstream behavior.
`file_get_contents` returns `false` on failure, which becomes `''` when cast to `string`. That makes an unreadable file indistinguishable from a legitimately empty file, and then `json_decode('')` / `parseJsonl('')` silently yields `null`/empty data. Please check explicitly for `false` and either throw a dedicated exception or reuse `FakeJsonNotFoundException` (or similar) so IO failures are not masked as empty content.
</issue_to_address>
### Comment 2
<location path="src/FakeQueryInterceptor.php" line_range="67-69" />
<code_context>
+
+ $content = (string) file_get_contents($jsonFile);
+ /** @psalm-suppress MixedAssignment */
+ $data = $isRow
+ ? json_decode($content, true)
+ : $this->parseJsonl($content);
+
+ $entityClass = ($this->returnEntity)($method);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Lack of JSON error handling means malformed JSON will fail late and non-obviously.
Both the row and JSONL paths call `json_decode(..., true)` without checking for failures. On malformed input this returns `null`, which then either triggers `JsonHydrator`’s `assert(is_array($data))` or produces `null` entries in the JSONL list. Consider using `JSON_THROW_ON_ERROR` and mapping the exception to a domain-specific error, or checking `json_last_error()` / `json_last_error_msg()` and throwing a clear "invalid fixture JSON" exception at this point.
Suggested implementation:
```
/** @psalm-suppress MixedAssignment */
$data = $isRow
? json_decode($content, true, 512, JSON_THROW_ON_ERROR)
: $this->parseJsonl($content);
```
To fully implement the suggestion consistently across both row and JSONL paths, you should also:
1. Update all `json_decode` calls inside `parseJsonl()` to use `JSON_THROW_ON_ERROR`, for example:
- Change `json_decode($line, true)` to `json_decode($line, true, 512, JSON_THROW_ON_ERROR)`.
2. Optionally, if you want a domain-specific error:
- Wrap the call site above in a `try/catch (\JsonException $e)` block and rethrow a custom exception (e.g. `FakeInvalidJsonFixtureException`) with a clear message including `$jsonFile`.
- Implement that custom exception class (likely under `src/Exception/` or wherever your other domain exceptions live).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
phpunit.xml.dist (1)
3-7: Optional: Restrict discovery to test files only.If helper/fixture PHP files exist under
tests/, adding a suffix filter avoids accidental test pickup.Suggested tweak
- <directory>tests</directory> + <directory suffix="Test.php">tests</directory>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@phpunit.xml.dist` around lines 3 - 7, Restrict PHPUnit test discovery by adding a suffix filter on the <directory> element so only test files are picked up (e.g., set the directory's suffix to "Test.php" or another project convention) inside the existing <testsuite name="Ray.FakeQuery test suite"> block; update the <directory> element in phpunit.xml.dist to include the suffix attribute to avoid picking up helper/fixture files under tests/.README.md (1)
70-72: Align the later mapping explanation with the new JSONL convention.Since Line 71 introduces
.jsonlfor collections, update the “How It Works” mapping text (Line 97) so it doesn’t imply.jsonis always used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 70 - 72, Update the “How It Works” mapping text so it reflects the new JSONL convention: change any wording that implies collections use “.json” to state that single-entity files use “.json” (e.g., todo_item.json) while collections use “.jsonl” (e.g., todo_list.jsonl), and adjust examples and phrasing in the mapping paragraph to mention both “.json” and “.jsonl” as appropriate rather than always “.json”.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@DESIGN.md`:
- Around line 144-147: Update the earlier flow/behavior sections to match the
nullable-file rule described in the storage format block: for "Single entity
(`?Entity`): `{queryId}.json`", "Collection (`array<Entity>`):
`{queryId}.jsonl`", "Nullable" and "void methods" semantics, change any
statements that say missing files always throw to instead state that missing
files return null for nullable return types (and only throw for non-nullable
returns), and clarify that void methods require no file; ensure the wording in
those earlier sections mirrors the exact behavior described in the storage
format lines so implementers are not given conflicting rules.
In `@phpstan.neon`:
- Line 8: Remove the accidental "(?)" from the excludePaths pattern: locate the
excludePaths entry that currently reads with "tests/tmp (?)" and replace it with
a valid pattern such as "tests/tmp" (to exclude the directory) or "tests/tmp/*"
(to exclude all contents). Ensure the change is made in the phpstan
configuration where the excludePaths array is defined so PHPStan's fnmatch
semantics are respected.
In `@README.md`:
- Around line 76-89: The README contains JSON/JSONL code fences that include
JavaScript-style `//` comments, making the examples invalid for copy-paste; edit
the JSON and JSONL example blocks (the single-object example and the multi-line
JSONL example) to remove the `//` comment lines and instead place the
descriptive text outside the fenced blocks (e.g., a plain sentence like
"`var/fake/todo_item.json` (single JSON object):" before the ```json``` fence
and "`var/fake/todo_list.jsonl` (one JSON object per line):" before the
```jsonl``` fence) so the fenced content contains only valid JSON/JSONL.
In `@src/FakeQueryConfig.php`:
- Around line 9-12: FakeQueryConfig's constructor currently accepts a fakeDir
without validating it, so add a fail-fast check in FakeQueryConfig::__construct
(or FakeQueryModule::__construct if config isn't constructed there) that
verifies the path exists and is readable (use is_dir($fakeDir) and
is_readable($fakeDir) or equivalent) and throw an InvalidArgumentException (or
RuntimeException used elsewhere) with a clear message if the check fails; this
ensures FakeQueryInterceptor and FakeQueryModule::validateFakeFiles will never
attempt to read from a non-existent/unreadable directory.
In `@src/FakeQueryInterceptor.php`:
- Around line 65-69: The code currently casts file_get_contents to string and
calls json_decode without checking for read or parse errors; update the block
around $content/$isRow/$data so that you first check
file_get_contents($jsonFile) !== false (or use is_readable) and throw a clear
exception if reading fails, then decode using json_decode with error handling
(either JSON_THROW_ON_ERROR in a try/catch or check json_last_error() after
decode) and throw a descriptive exception if decoding fails; also update
parseJsonl($content) to validate each line's json_decode and surface parse
errors similarly so both the $isRow branch and the parseJsonl path fail fast
with clear messages.
---
Nitpick comments:
In `@phpunit.xml.dist`:
- Around line 3-7: Restrict PHPUnit test discovery by adding a suffix filter on
the <directory> element so only test files are picked up (e.g., set the
directory's suffix to "Test.php" or another project convention) inside the
existing <testsuite name="Ray.FakeQuery test suite"> block; update the
<directory> element in phpunit.xml.dist to include the suffix attribute to avoid
picking up helper/fixture files under tests/.
In `@README.md`:
- Around line 70-72: Update the “How It Works” mapping text so it reflects the
new JSONL convention: change any wording that implies collections use “.json” to
state that single-entity files use “.json” (e.g., todo_item.json) while
collections use “.jsonl” (e.g., todo_list.jsonl), and adjust examples and
phrasing in the mapping paragraph to mention both “.json” and “.jsonl” as
appropriate rather than always “.json”.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
DESIGN.mdREADME.mdcomposer.jsonphpstan.neonphpunit.xml.distsrc/Exception/FakeJsonNotFoundException.phpsrc/Exception/RuntimeException.phpsrc/Exception/UnknownFakeJsonException.phpsrc/FakeQuery.phpsrc/FakeQueryConfig.phpsrc/FakeQueryInterceptor.phpsrc/FakeQueryModule.phpsrc/JsonHydrator.phpsrc/Types.phptests/Fake/Entity/TodoEntity.phptests/Fake/Entity/UserEntity.phptests/Fake/Query/TodoCommandInterface.phptests/Fake/Query/TodoQueryInterface.phptests/Fake/Query/UserQueryInterface.phptests/Fake/todo_item.jsontests/Fake/todo_list.jsontests/Fake/todo_list.jsonltests/Fake/user_item.jsontests/Fake/user_list.jsonltests/FakeQueryModuleTest.phptests/FakeQueryTest.phptests/FakeUnknown/stray_query.json
💤 Files with no reviewable changes (2)
- tests/FakeQueryTest.php
- src/FakeQuery.php
- FakeQueryInterceptor: check file_get_contents for false, add JSON_THROW_ON_ERROR - FakeQueryConfig: validate fakeDir exists with InvalidFakeDirException - phpstan.neon: fix excludePaths pattern (tests/tmp/*) - DESIGN.md: align missing-file behavior for nullable vs non-nullable - README.md: remove comments from JSON blocks, simplify
Member
Author
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DESIGN.md (1)
90-97:⚠️ Potential issue | 🟡 MinorUpdate the flow step to reflect
.jsonvs.jsonlselection.The flow still implies a fixed
.jsonload path, but the conventions now specify.jsonlfor collections. Please make the flow wording match the new extension rules.Suggested doc patch
-4. Load `{fakeDir}/{queryId}.json` +4. Resolve fixture file by return shape: + - single row/entity → `{fakeDir}/{queryId}.json` + - collection (`array<Entity>`) → `{fakeDir}/{queryId}.jsonl`Also applies to: 146-149
♻️ Duplicate comments (1)
src/FakeQueryConfig.php (1)
16-18:⚠️ Potential issue | 🟡 MinorAlso validate directory readability at construction time.
Current check catches non-directories, but unreadable directories still pass and later surface as misleading fixture-not-found failures.
Proposed fail-fast check
use function is_dir; +use function is_readable; @@ - if (! is_dir($fakeDir)) { + if (! is_dir($fakeDir) || ! is_readable($fakeDir)) { throw new InvalidFakeDirException($fakeDir); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/FakeQueryConfig.php` around lines 16 - 18, The constructor check that currently only verifies is_dir($fakeDir) should also assert the directory is readable; update the validation in FakeQueryConfig (the block that throws InvalidFakeDirException($fakeDir)) to fail-fast when either the path is not a directory or is not readable (use is_readable on $fakeDir) and throw InvalidFakeDirException with the same identifying $fakeDir so unreadable directories are rejected at construction time.
🧹 Nitpick comments (2)
src/Exception/InvalidFakeDirException.php (1)
11-11: Make the exception message more generic to avoid false diagnostics.
$fakeDircan fail validation for reasons beyond non-existence (e.g., path exists but is not a directory), so the message is slightly misleading.Proposed message tweak
- parent::__construct("Fake directory does not exist: {$fakeDir}"); + parent::__construct("Invalid fake directory: {$fakeDir}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Exception/InvalidFakeDirException.php` at line 11, The exception message in InvalidFakeDirException is too specific ("Fake directory does not exist: {$fakeDir}") and can mislead when other validation failures occur; update the message passed to parent::__construct in the InvalidFakeDirException constructor to a more generic wording such as "Invalid fake directory: {$fakeDir}" or "Invalid fake directory provided" so it covers non-existence and other invalid states while preserving the $fakeDir value for context.tests/FakeQueryModuleTest.php (1)
112-120: Optional cleanup: extract repeated injector wiring into a helper.The repeated anonymous module setup appears in several tests; consolidating it into a small factory/helper will make failure-focused tests easier to read and maintain.
Also applies to: 132-140, 155-163, 183-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/FakeQueryModuleTest.php` around lines 112 - 120, Extract the repeated anonymous Injector wiring into a helper method (e.g., createInjector or makeInjector) on the test class so tests reuse it instead of duplicating the anonymous class; the helper should accept the two path arguments and return new Injector(new class extends AbstractModule { protected function configure(): void { $this->install(new FakeQueryModule($basePath, $queryPath)); } }, $tmpDir) (use the actual parameter names you prefer), then replace the repeated blocks in tests that construct Injector directly (the occurrences using Injector, AbstractModule, and FakeQueryModule around lines 112-120, 132-140, 155-163, 183-191) with calls to that helper for clearer, DRY test setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 5-8: The fenced code block containing the lines "Ray.MediaQuery
SQL files → DB execution → Entity" and "Ray.FakeQuery JSON files → hydration
→ Entity (same interface)" lacks a language identifier and triggers markdownlint
MD040; update that opening fence from ``` to include a language identifier
(e.g., use ```text) so the block is explicitly labeled and the linter passes,
keeping the block content unchanged.
---
Duplicate comments:
In `@src/FakeQueryConfig.php`:
- Around line 16-18: The constructor check that currently only verifies
is_dir($fakeDir) should also assert the directory is readable; update the
validation in FakeQueryConfig (the block that throws
InvalidFakeDirException($fakeDir)) to fail-fast when either the path is not a
directory or is not readable (use is_readable on $fakeDir) and throw
InvalidFakeDirException with the same identifying $fakeDir so unreadable
directories are rejected at construction time.
---
Nitpick comments:
In `@src/Exception/InvalidFakeDirException.php`:
- Line 11: The exception message in InvalidFakeDirException is too specific
("Fake directory does not exist: {$fakeDir}") and can mislead when other
validation failures occur; update the message passed to parent::__construct in
the InvalidFakeDirException constructor to a more generic wording such as
"Invalid fake directory: {$fakeDir}" or "Invalid fake directory provided" so it
covers non-existence and other invalid states while preserving the $fakeDir
value for context.
In `@tests/FakeQueryModuleTest.php`:
- Around line 112-120: Extract the repeated anonymous Injector wiring into a
helper method (e.g., createInjector or makeInjector) on the test class so tests
reuse it instead of duplicating the anonymous class; the helper should accept
the two path arguments and return new Injector(new class extends AbstractModule
{ protected function configure(): void { $this->install(new
FakeQueryModule($basePath, $queryPath)); } }, $tmpDir) (use the actual parameter
names you prefer), then replace the repeated blocks in tests that construct
Injector directly (the occurrences using Injector, AbstractModule, and
FakeQueryModule around lines 112-120, 132-140, 155-163, 183-191) with calls to
that helper for clearer, DRY test setup.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
DESIGN.mdREADME.mdphpstan.neonsrc/Exception/InvalidFakeDirException.phpsrc/Exception/LogicException.phpsrc/FakeQueryConfig.phpsrc/FakeQueryInterceptor.phptests/FakeEmpty/.gitkeeptests/FakeQueryModuleTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- phpstan.neon
✅ Actions performedComments resolved. Auto-approval is disabled; enable |
- FakeQueryConfig: also check is_readable($fakeDir) - InvalidFakeDirException: generalize message for non-existence and unreadable - README.md: add text language identifier to code fence
Member
Author
|
@coderabbitai approve |
✅ Actions performedComments resolved. Auto-approval is disabled; enable |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FakeQueryModulethat scans interfaces with#[DbQuery]and binds them toFakeQueryInterceptorFakeQueryInterceptorloads JSON/JSONL fixture files instead of executing SQL, with entity hydration (snake_case → camelCase)voidreturn (no-op), single entity (?Entity), entity list (array<Entity>), and raw array returnsFakeJsonNotFoundExceptionwith descriptive message when fixture file is missingFakeQueryConfig::assertValidFakeDir()Architecture
Test plan
Summary by Sourcery
Introduce a Ray.Di module and interceptor for serving DbQuery interfaces from JSON/JSONL fixtures instead of a database, with type-aware hydration and validation of fake data files.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores