fix: Strict JSON value to fail on duplicate keys when ingesting #1334
Conversation
Implements a `StrictValue` for JSON deserialization that flags duplicate keys in JSON map instead of accepting the last key (like the default implementation). Added unit tests for verifying deserialization. Resolves: parseablehq#1217
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPHandler
participant StrictValue
participant ProcessingLogic
Client->>HTTPHandler: Send JSON payload
HTTPHandler->>StrictValue: Deserialize JSON (strict mode)
alt No duplicate keys
StrictValue->>HTTPHandler: Return parsed Value
HTTPHandler->>ProcessingLogic: Pass Value for ingestion
ProcessingLogic->>HTTPHandler: Ingestion result
HTTPHandler->>Client: Respond success/failure
else Duplicate keys found
StrictValue->>HTTPHandler: Return error
HTTPHandler->>Client: Respond with error
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/utils/json/strict.rs (1)
32-32: Fix grammar in comments.Minor grammatical corrections needed in the comments.
Apply this diff to fix the grammar:
- // Number is a i64 + // Number is an i64And at line 41:
- // Number is a u64 + // Number is a u64Also applies to: 41-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/handlers/http/ingest.rs(12 hunks)src/utils/json/mod.rs(1 hunks)src/utils/json/strict.rs(1 hunks)
🔇 Additional comments (9)
src/utils/json/mod.rs (1)
31-31: LGTM!The module declaration correctly exposes the new strict JSON functionality.
src/utils/json/strict.rs (4)
6-17: Well-designed intermediate representation!The
InterimValueenum appropriately covers all JSON types including extended integers, and the use ofVec<(String, InterimValue)>for objects enables duplicate key detection.
54-62: Excellent duplicate key detection!The implementation correctly detects duplicate keys during object conversion and returns an appropriate error message.
186-216: Clean and effective StrictValue implementation!The two-phase deserialization approach (InterimValue → serde_json::Value) effectively enforces strict JSON parsing while maintaining compatibility with the existing serde_json ecosystem.
218-370: Comprehensive test coverage!The test suite thoroughly covers all JSON types, nested structures, and most importantly, validates the duplicate key detection behavior that differentiates
StrictValuefromserde_json::Value.src/handlers/http/ingest.rs (4)
41-41: Correct import update!The import properly includes
StrictValuefrom the strict module alongside the existingJsonFlattenError.
53-53: Clean integration of StrictValue!The handler correctly receives JSON as
StrictValueand extracts the inner value for processing, ensuring strict validation without disrupting the existing logic flow.Also applies to: 85-86
159-159: Consistent StrictValue integration across all OTEL handlers!All three OTEL ingestion handlers (logs, metrics, traces) follow the same pattern of receiving
StrictValueand extracting the inner value, maintaining consistency across the codebase.Also applies to: 209-215, 225-225, 273-279, 289-289, 338-344
131-132: Complete and consistent StrictValue adoption!The remaining handlers (
ingest_internal_streamandpost_event) also correctly implement the StrictValue pattern, ensuring all ingestion endpoints enforce strict JSON validation.Also applies to: 137-137, 355-355, 391-391
The same change can be made there, right? I'll add another commit here. |
|
Spoke too soon, I would need a bit more idea on how the ingestion through kafka/kinesis works in code. |
We can take up Kafka and Kinesis flows separately. If you're fine, I will merge this @vrongmeal |
Of course! Can go ahead and merge this in |
Fixes #1217.
Description
Implements a
StrictValuefor JSON deserialization that flags duplicate keys in JSON map instead of accepting the last key (like the default implementation).Added unit tests for verifying deserialization.
This PR has:
Summary by CodeRabbit