Introducing JSON to Lettuce

[tishun]

Granted, this review turned out much much much bigger than I anticipated.

Instructions for reviewing

Ideally please concentrate on the most important decisions:

• API surface as defined by src/main/templates/io/lettuce/core/api/RedisJsonCommands.java
• sample use cases in the src/test/java/io/lettuce/core/RedisJsonCommandBuilderUnitTests.java
• integration of the Jackson parser in the src/main/java/io/lettuce/core/json/DefaultJsonParser.java
• use the testcontainers fw to deploy docker-stack src/test/java/io/lettuce/core/RedisContainerIntegrationTests.java

General considerations

(for more details check the README.md with some more information)

This implementation aims to solve several general problems:

1. avoid any JSON processing as part of the Netty event loop
2. provide Lettuce-specific interfaces to handle JSON as DOM
3. provide the means to apply RedisCodec encoding and decoding
4. abstract from the RESP2 vs RESP3 specifics
5. support both JsonPath v1 and JsonPath v2

Three operating modes facilitating default (basic), advanced and power-user usage

Default mode

• [x] Uses Jackson behind the scenes, but provides a driver-specific API to avoid tight coupling with any 3rd party library
• [x] Deserializes lazily the data returned by the server to avoid slowing down the event loop
• [x] Serializes the data when sending JSON to the server before passing it to the event loop
• [x] Allows for the usage of RedisCodecs to enable usage of different data types without explicit conversion *(1)

Advanced mode (incomplete, but considered in the design)

• [ ] Allows the users of the driver to provide custom parsers utilizing other third party libraries or their own implementation

Power-user mode

• [x] Makes it possible to use the JSON data in raw format, for example work only with byte buffers without parsing

Notes

• The Jackson parser uses byte arrays and strings for parsing data. Although codec conversion is not enforced in cases when the Jackson parser modifies JSON documents in memory conversion might be required as it does not handle all data types, e.g. if the user is using some other representation of the data the driver would use the codec the user has supplied to convert to bytes and then the StringCodec to convert to String
• changes were required to the way the command enumeration as the the JSON.X commands have a dot in their name
• changes were required to the CommandBuilder to avoid making it too large and attempt to follow the single responsibility principle
• the stateful connection class was extended to facilitate the parser logic - I am not particularly fond of this and will try to change it

Leftovers

• [ ] consider better naming for the JsonParser abstraction
• [ ] sane mechanism for changing the currently used parser
• [ ] overloads of the methods with defaults, for easier usage
• [ ] object mapping functionality (e.g. provide a .class file to a method)
• [ ] >80% coverage with unit tests
• [ ] Implement an enumeration for the result of JSON.TYPE

Unrelated changes

There are a few unrelated changes such as adding missing imports in 3 other template files

Supported commands

Command	Status
JSON.ARRAPPEND	✅ Implemented
JSON.ARRINDEX	✅ Implemented
JSON.ARRINSERT	✅ Implemented
JSON.ARRLEN	✅ Implemented
JSON.ARRPOP	✅ Implemented
JSON.ARRTRIM	✅ Implemented
JSON.CLEAR	✅ Implemented
JSON.DEBUG	❌ Not planned
JSON.DEBUG MEMORY	❌ Not planned
JSON.DEL	✅ Implemented
JSON.FORGET	❌ Not planned
JSON.GET	✅ Implemented
JSON.MERGE	✅ Implemented
JSON.MGET	✅ Implemented
JSON.MSET	✅ Implemented
JSON.NUMINCRBY	✅ Implemented
JSON.NUMMULTBY	❌ Not planned
JSON.OBJKEYS	✅ Implemented
JSON.OBJLEN	✅ Implemented
JSON.RESP	❌ Not planned
JSON.SET	✅ Implemented
JSON.STRAPPEND	✅ Implemented
JSON.STRLEN	✅ Implemented
JSON.TOGGLE	✅ Implemented
JSON.TYPE	✅ Implemented

Make sure that:

• [x] You have read the contribution guidelines.
• [x] You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
• [x] You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
• [x] You submit test cases (unit or integration tests) that back your changes.

[tishun]

This looks really good! It will be a nice addition to Lettuce. Just a couple things:

• I did not see tests against Redis Cluster.
• There does not seem to be an implementation of key routing in case of JSON.MGET with the Cluster API.

Hey @jruaux , thanks a lot for taking a look!

Yeah these two points are very valid ones. Let me see if I can address them with an update.

[sazzad16]

Hey @tishun, ...tracking... stuff got in here. Is it intended?

[tishun]

Hey @tishun, ...tracking... stuff got in here. Is it intended?

No, but this is because the PR is fork/redis-json -> origin/redis-json and origin/redis-json is now behind origin/main

I will try to fix this.

[tishun]

Overall, this is a good first iteration, but binding all JSON interfaces to generic types of Lettuce codec seems unnecessary and just increases the complexity of the code.

There is no way to use the Codec system without having the generics, as we discussed offline. I do - however - agree that the key in a JSON file does not map to the redis key, used as part of most commands. As Such I will refactor the code to only consider the value generic.

Also, we need more developer-friendly overloads for frequently used commands like JSON.SET. See my inline comments for more details.

Agreed, this is one of the leftovers

[tishun]

Overall, this is a good first iteration, but binding all JSON interfaces to generic types of Lettuce codec seems unnecessary and just increases the complexity of the code.

There is no way to use the Codec system without having the generics, as we discussed offline.

After some more careful consideration I actually fully agree with you.

The latest version does not have any generics for the JsonValue / JsonParser abstractions as they did not help in any way.