fix(rest): support json equality filtering #2088
Conversation
📝 WalkthroughWalkthroughThe changes update the server's REST API request handling logic to improve type coercion, particularly for fields of JSON type or custom type definitions. The coercion method now receives full field metadata, enabling it to parse JSON strings for relevant fields and provide more robust error handling for invalid JSON input. Corresponding updates are made to filter construction logic to support equality filtering on JSON fields. The test suite is expanded with new schema definitions and test cases to verify correct behavior for JSON field filtering and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant REST API (RequestHandler)
participant Database
Client->>REST API (RequestHandler): Sends request with JSON field/filter
REST API (RequestHandler)->>REST API (RequestHandler): Calls coerce(fieldInfo, value)
alt fieldInfo is JSON or type definition
REST API (RequestHandler)->>REST API (RequestHandler): Parse value as JSON
alt Parsing fails
REST API (RequestHandler)->>Client: Return 400 error (invalid-value)
else Parsing succeeds
REST API (RequestHandler)->>Database: Query with parsed JSON value
Database-->>REST API (RequestHandler): Return result
REST API (RequestHandler)-->>Client: Return result
end
else fieldInfo is primitive
REST API (RequestHandler)->>REST API (RequestHandler): Coerce value to primitive type
REST API (RequestHandler)->>Database: Query with coerced value
Database-->>REST API (RequestHandler): Return result
REST API (RequestHandler)-->>Client: Return result
end
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/server/src/api/rest/index.ts (1)
1749-1750: TODO for extending JSON filtering.
There's a placeholder note for adding inequality filters. Let me know if you need help implementing them, as they can get tricky for structured or nested JSON values.packages/server/tests/api/rest.test.ts (1)
758-773: Good error handling test for invalid JSONThe test properly verifies that the server returns an appropriate error response when invalid JSON syntax is provided in the filter. This is critical for providing good developer feedback.
Consider adding additional error test cases for:
- Type mismatches (e.g., passing a number when an object is expected)
- Filtering with null values or empty objects
- Using complex nested properties in filters
These would further strengthen the robustness of your JSON filtering implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/src/api/rest/index.ts(8 hunks)packages/server/tests/api/rest.test.ts(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/server/src/api/rest/index.ts (1)
packages/runtime/src/cross/model-meta.ts (1)
FieldInfo(26-107)
🪛 Biome (1.9.4)
packages/server/src/api/rest/index.ts
[error] 1358-1358: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 1389-1389: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (11)
packages/server/src/api/rest/index.ts (6)
1352-1352: Implementation for single ID field scenario looks correct.
No issues found here; the code effectively handles a single field ID by coercing it according to its field type.
1385-1385: Implementation for single ID field scenario looks correct.
The code snippet properly sets the object key for an ID field and handles type coercion.
1439-1448: Good JSON parsing for type definitions and JSON-typed fields.
Throwing anInvalidValueErrorensures invalid JSON values are promptly identified and handled. This change solidly supports parsing stringified JSON input, aligning with the PR's goal of improved JSON equality filtering.
1775-1775: Bulk coercion for array values looks correct.
Applying.mapwith thecoercemethod for each item is straightforward and consistent.
1786-1789: Equality filtering support for JSON fields is a great addition.
Returning{ equals: coerced }correctly handles direct equality comparisons on JSON-typed fields.
1794-1794: Bulk coercion for comma-separated values looks good.
This is consistent with existing logic for filtering multiple possible values.packages/server/tests/api/rest.test.ts (5)
25-27: Good implementation of the Address type definitionThe Address type with a city field is well-defined and provides a good test case for the JSON field filtering functionality.
37-38: Good addition of JSON fields to User modelThe two different JSON field types provide comprehensive test coverage:
addressas a typed JSON field using the Address type definitionsomeJsonas a generic JSON fieldThis allows testing both structured and unstructured JSON filtering.
437-438: Good test data setup for JSON fieldsThe test data is appropriately set up with:
- A structured JSON object for the address field
- A simple string value for the someJson field
This provides a good foundation for testing the JSON filtering functionality.
726-740: Thorough test coverage for typed JSON equality filteringThe test correctly verifies JSON equality filtering on the typed
addressfield with:
- A positive test case that should match one user (Seattle)
- A negative test case that should match no users (Tokyo)
The use of
JSON.stringify()to properly format the filter value is a good practice.
742-756: Good test coverage for plain JSON equality filteringThe test properly verifies equality filtering for the untyped
someJsonfield with:
- A positive test case matching the string value 'foo'
- A negative test case with a non-matching string value 'bar'
This confirms that JSON filtering works with primitive values as well as objects.
No description provided.