Allow arbitrary extra data in API objects that support this loose format #104
Conversation
This file contains 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
rokob
force-pushed
the
allow-arbitrary-keys
branch
from
99f68b1
to
0cc0199
Compare
|
This is so weird. The test that is failing on the push should ALWAYS be failing but it passes because the JVM is pretty fast. We are comparing two different calls to the current system time for equality, so I'll fix that. |
basoko
reviewed
Feb 16, 2018
| @@ -25,6 +25,8 @@ | |||
|
|
|||
| public class Factory { | |||
|
|
|||
| private static final long CURRENT_TIME = System.currentTimeMillis(); | |||
mattdrees
reviewed
Feb 21, 2018
| private Person(Builder builder) { | ||
| this.id = builder.id; | ||
| this.username = builder.username; | ||
| this.email = builder.email; | ||
| this.metadata = builder.metadata != null ? unmodifiableMap(builder.metadata) : null; |
There was a problem hiding this comment.
Might not be a big deal, but unmodifiableMap() doesn't guarantee that the map is immutable. So this change actually changes Person from an immutable object to a mutable one (though it can only be mutated by mutating the map passed to the builder).
FWIW.
|
@rokob is there anything I can do to help here? I am guessing you're wanting to solve the NPath complexity warnings that Codacy raised. If that's true, I can work on a solution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In the API docs some of the elements in the payload body support arbitrary extra data with a comment that starts like
// Can contain any arbitrary keys.. This PR adds back the ability to include this extra data with the various objects that the API understands, namely:Server,Message,Request.Clientalready supports this. Additionally, the docs don't mention it but we have always supported arbitrary extra data withPersonso this adds that capacity as well.The general form is to use the Builder for each type and to pass in a
Map<String, Object>to themetadatafunction. This map will get merged into the final JSON for that object before the standard keys are added. That means that keys that are standard such asbodyfor theMessagetype will be overwritten if they exist in this metadata object. See the tests added herein for examples of this behaviour.Fixes #86