-
Notifications
You must be signed in to change notification settings - Fork 9
chore: Adds FDv2 PayloadProcessor #95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| * | ||
| * @throws IllegalArgumentException if any required field is missing | ||
| */ | ||
| public void validate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, we have to do this because JSON makes an empty one and then fills things in?
| return; | ||
| } | ||
| for (Event event : events) { | ||
| if (event == null || event.getEvent() == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the process by which this could happen? And if it cannot happen, but does happen, then should it be silent?
|
|
||
| // if there's no payloads, return | ||
| List<PayloadIntent> payloads = serverIntentData.getPayloads(); | ||
| if (payloads == null || payloads.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like this may also be something we would want to know if we are diagnosing an issue.
| } | ||
| putObject.validate(); | ||
| // if server intent hasn't been received yet, ignore the event | ||
| if (tempId == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was treated as an error in the Go implementation, and I also treated as an error. It feels to violate what should be a requirement of the protocol.
| } | ||
| payloadTransferred.validate(); | ||
| // if server intent hasn't been received yet, we should reset | ||
| if (tempId == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be logged?
No description provided.