Add JSON validation for channel and merchant events #233
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds JSON validation for channel and merchant events to ensure data integrity and proper field requirements. The validation covers both JSON parsing and required field checks for different event types.
- Implements content validation for channel events (ChannelCreateEvent and ChannelMetadataEvent) with required field checks
- Adds validation for merchant events (CreateOrUpdateProductEvent and CreateOrUpdateStallEvent) with specific field requirements
- Includes comprehensive unit tests for invalid JSON and missing required fields scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| JsonContentValidationTest.java | New test file covering validation scenarios for invalid JSON and missing required fields |
| MerchantEvent.java | Adds base validation for merchant entities with ID field requirement |
| CreateOrUpdateStallEvent.java | Implements stall-specific validation for name and currency fields |
| CreateOrUpdateProductEvent.java | Implements product-specific validation for name, currency, and price fields |
| ChannelMetadataEvent.java | Adds validation for channel profile with name, about, and picture field requirements |
| ChannelCreateEvent.java | Implements identical validation to ChannelMetadataEvent for channel profile fields |
| if (profile.getPicture() == null) { | ||
| throw new AssertionError("Invalid `content`: `picture` field is required."); | ||
| } | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
This catch block catches all exceptions including AssertionError, which masks validation errors. Consider catching only JSON parsing exceptions (e.g., JsonProcessingException) and allowing AssertionErrors to propagate naturally.
| } catch (Exception e) { | |
| } catch (JsonProcessingException e) { |
Copilot uses AI. Check for mistakes.
| throw new AssertionError("Invalid `content`: `picture` field is required."); | ||
| } | ||
|
|
||
| } catch (Exception e) { |
There was a problem hiding this comment.
This catch block catches all exceptions including AssertionError, which masks validation errors. Consider catching only JSON parsing exceptions (e.g., JsonProcessingException) and allowing AssertionErrors to propagate naturally.
| } catch (Exception e) { | |
| } catch (JsonProcessingException e) { |
Copilot uses AI. Check for mistakes.
| } catch (AssertionError e) { | ||
| throw e; |
There was a problem hiding this comment.
The try-catch block that re-throws AssertionError is redundant. AssertionErrors should be allowed to propagate naturally without explicit catching and re-throwing.
| } catch (AssertionError e) { | |
| throw e; |
Copilot uses AI. Check for mistakes.
| } catch (AssertionError e) { | ||
| throw e; |
There was a problem hiding this comment.
The try-catch block that re-throws AssertionError is redundant. AssertionErrors should be allowed to propagate naturally without explicit catching and re-throwing.
| } catch (AssertionError e) { | |
| throw e; |
Copilot uses AI. Check for mistakes.
| } catch (AssertionError e) { | ||
| throw e; |
There was a problem hiding this comment.
The try-catch block that re-throws AssertionError is redundant. AssertionErrors should be allowed to propagate naturally without explicit catching and re-throwing.
| } catch (AssertionError e) { | |
| throw e; |
Copilot uses AI. Check for mistakes.
Summary
Testing
mvn -q verifymvn -DskipTests installhttps://chatgpt.com/codex/tasks/task_b_688a8503ca2c83319e47e1b7c88d7c8e