Introducing JSON to Lettuce (#2933) #2992
Conversation
* Most of the API layer * Add the JSON.TYPE command * Kotlin coroutines added; Started JSON README.md; Parser registry is now part of the Connection; formatting; chained all commands; extracted commands in their own unit RedisJsonCommandBuilder; * Implemented 90% of commands from top 10 * All but SET are implemented * Integrated Jackson, finished up the SET command * Left out a few files by mistake * Adding some JavaDoc * Formatting * Implemented all JSON commands * Introducing test containers to the testing fw * Complete coverage with integration tests, straight scenarios * Added Pathv1 tests * Added RedisCE cluster support for the JSON.MGET and JSON.MSET commands * Handle null values * No longer using K for the JSON object keys * Polishing * JsonType introduced to help typization * Remove the RedisCodec from the JsonValue/JsonParser abstraction, add configuration for custom parsers * Extend API surface with methods that reduce the amount of required arguments * Adding unit tests, addressing changes to README.md * Implemented object-mapping functionality * Addresses Ali's comments * Addressed last bunch of comments from Ali, changed ports to not colide with existing infra * Forgot to change ports and stop debug log * Polishing touches
* Most of the API layer * Add the JSON.TYPE command * Kotlin coroutines added; Started JSON README.md; Parser registry is now part of the Connection; formatting; chained all commands; extracted commands in their own unit RedisJsonCommandBuilder; * Implemented 90% of commands from top 10 * All but SET are implemented * Integrated Jackson, finished up the SET command * Left out a few files by mistake * Adding some JavaDoc * Formatting * Implemented all JSON commands * Introducing test containers to the testing fw * Complete coverage with integration tests, straight scenarios * Added Pathv1 tests * Added RedisCE cluster support for the JSON.MGET and JSON.MSET commands * Handle null values * No longer using K for the JSON object keys * Polishing * JsonType introduced to help typization * Remove the RedisCodec from the JsonValue/JsonParser abstraction, add configuration for custom parsers * Extend API surface with methods that reduce the amount of required arguments * Adding unit tests, addressing changes to README.md * Implemented object-mapping functionality * Addresses Ali's comments * Addressed last bunch of comments from Ali, changed ports to not colide with existing infra * Forgot to change ports and stop debug log * Polishing touches
| @@ -441,7 +441,7 @@ static BytesArgument of(ProtocolKeyword protocolKeyword) { | |||
|
|
|||
| @Override | |||
| public String toString() { | |||
| return protocolKeyword.name(); | |||
| return protocolKeyword.toString(); | |||
There was a problem hiding this comment.
@tishun
Why are you introducing a breaking change to an interface that is publicly available?
If there’s a valid reason, I think making the change is acceptable — but I don’t understand the intent behind this modification.
The loss of compatibility is causing issues.
There was a problem hiding this comment.
Hey @be-hase ,
can you help me understand what the impact is and why is it breaking?
The .name() method was not a great choice to begin with, because it limits us to use procotol keywords that follow the Java naming rules, and some of the protocol keywords use fullstops, for ex. FT.CREATE
For legacy cases it should yield the same results.
There was a problem hiding this comment.
@be-hase please file an issue if you are experiencing interface breakage with some small example.
I will make sure we address this retroactively in the releases that contain the JSON feature.
There was a problem hiding this comment.
@tishun
I have been using this interface to collect metrics with Micrometer.
I was using ProtocolKeyword#name as a label.
This code has been distributed internally as a library.
Although Lettuce now provides a similar mechanism, mine was implemented before that.
Now that ProtocolKeyword#name has suddenly been removed, it’s causing a NoSuchMethodError, which is a serious problem.
( sorry for late response
There was a problem hiding this comment.
No problem.
See #3368
(For more details on the change see #2933)
This PR merges the feature branch introducing the RedisJSON functionality
Make sure that:
mvn formatter:formattarget. Don’t submit any formatting related changes.