Draft API for connection and template layers for Redis JSON commands#3327
Draft API for connection and template layers for Redis JSON commands#3327Dgramada wants to merge 4 commits intospring-projects:mainfrom
Conversation
…perations. Furthermore, added a JsonPath class for easier path creating for JSON methods in the template layer. These are draft classes and their purpose is to get feedback and an initial feeling of how the JSON API should look like. Signed-off-by: Yordan Tsintsov <yordan.tsintsov@redis.com>
spring-projects-issues
added
the
status: waiting-for-triage
…implementations for some of the overloaded classes. Signed-off-by: Yordan Tsintsov <yordan.tsintsov@redis.com>
Signed-off-by: Yordan Tsintsov <yordan.tsintsov@redis.com>
| * @see <a href="https://redis.io/docs/latest/commands/json.arrindex/">Redis JSON.ARRINDEX</a> | ||
| */ | ||
| @NullMarked | ||
| public final class JsonArrayRange { |
There was a problem hiding this comment.
We do have a Spring Data Range object org.springframework.data.domain.Range<T>. I wonder if we could that instead?
| * @see <a href="https://redis.io/docs/latest/commands/json.arrappend/">Redis Documentation: JSON.ARRAPPEND</a> | ||
| * @since 4.3 | ||
| */ | ||
| List<@Nullable Long> jsonArrAppend(byte[] key, String path, String... values); |
There was a problem hiding this comment.
I notice the commands use string but the operations use JsonPath. Even if JsonPath is just a wrapper around a string it may insulate us and give us a bit more flexibility for later extension. Wdyt?
| * @since 4.3 | ||
| */ | ||
| @NullMarked | ||
| public final class JsonPath { |
There was a problem hiding this comment.
I understand why you went w/ this DSL - to help build the JSON path. However, I think that providing the full JSON path string may be more readable than using the DSL.
For example:
JsonPath.root().child("address").child("city");
vs.
JsonPath.of("$.address.city")
Also, JSON path users are accustomed to providing the full path string. The full path string is also what is returned when users Google search "How to create a JSON path string to navigate to the ....".
This would also reduce quite a bit of API surface area to maintain.
Another concern is w/ the string concatenation - I am not sure how it would surface, but it reminds me of building up a SQL query via string concatenation that could lead to a CVE (not sure exactly how, but it just made me raise my eyebrow).
Wdyt about just providing a JsonPath.of(String) for V1?
There was a problem hiding this comment.
Yes you are right, this is more reasonable. I will keep it simple.
Signed-off-by: Yordan Tsintsov <yordan.tsintsov@redis.com>
3 participants
Provides a draft version of how the JSON template and connection layer API should look like. The purpose of this PR is to receive feedback and to get a better feeling of what should Spring Data Redis provide for users that would like to use Redis JSON.