Skip to content
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.

Sign up for GitHub

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

Java: JSON.MGET. #2514

Merged
merged 6 commits into from
Nov 14, 2024
Merged

Java: JSON.MGET. #2514

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7653331
`JSON.MGET`.
Yury-Fridlyand Oct 24, 2024
f380a34
update
Yury-Fridlyand Nov 7, 2024
288ef6c
Merge remote-tracking branch 'upstream/release-1.2' into java/yuryf-j…
Yury-Fridlyand Nov 7, 2024
ea14d71
update
Yury-Fridlyand Nov 7, 2024
efc3934
Update java/client/src/main/java/glide/api/commands/servermodules/Jso…
Yury-Fridlyand Nov 13, 2024
a196606
Update java/client/src/main/java/glide/api/commands/servermodules/Jso…
Yury-Fridlyand Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* Java: Added `FT.AGGREGATE` ([#2466](https://github.com/valkey-io/valkey-glide/pull/2466))
* Java: Added `FT.PROFILE` ([#2473](https://github.com/valkey-io/valkey-glide/pull/2473))
* Java: Added `JSON.SET` and `JSON.GET` ([#2462](https://github.com/valkey-io/valkey-glide/pull/2462))
* Java: Added `JSON.MGET` ([#2514](https://github.com/valkey-io/valkey-glide/pull/2514))
* Node: Added `FT.CREATE` ([#2501](https://github.com/valkey-io/valkey-glide/pull/2501))
* Node: Added `FT.INFO` ([#2540](https://github.com/valkey-io/valkey-glide/pull/2540))
* Node: Added `FT.AGGREGATE` ([#2554](https://github.com/valkey-io/valkey-glide/pull/2554))
Expand Down
81 changes: 81 additions & 0 deletions java/client/src/main/java/glide/api/commands/servermodules/Json.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package glide.api.commands.servermodules;

import static glide.api.models.GlideString.gs;
import static glide.utils.ArrayTransformUtils.castArray;
import static glide.utils.ArrayTransformUtils.concatenateArrays;

import glide.api.BaseClient;
Expand All @@ -22,6 +23,7 @@ public class Json {
private static final String JSON_PREFIX = "JSON.";
private static final String JSON_SET = JSON_PREFIX + "SET";
private static final String JSON_GET = JSON_PREFIX + "GET";
private static final String JSON_MGET = JSON_PREFIX + "MGET";
private static final String JSON_NUMINCRBY = JSON_PREFIX + "NUMINCRBY";
private static final String JSON_NUMMULTBY = JSON_PREFIX + "NUMMULTBY";
private static final String JSON_ARRAPPEND = JSON_PREFIX + "ARRAPPEND";
Expand Down Expand Up @@ -411,6 +413,85 @@ public static CompletableFuture<GlideString> get(
new ArgsBuilder().add(gs(JSON_GET)).add(key).add(options.toArgs()).add(paths).toArray());
}

/**
* Retrieves the JSON values at the specified <code>path</code> stored at multiple <code>keys
* </code>.
*
* @apiNote When in cluster mode, if keys in <code>keys</code> map to different hash slots, the
* command will be split across these slots and executed separately for each. This means the
* command is atomic only at the slot level. If one or more slot-specific requests fail, the
* entire call will return the first encountered error, even though some requests may have
* succeeded while others did not. If this behavior impacts your application logic, consider
* splitting the request into sub-requests per slot to ensure atomicity.
* @param client The client to execute the command.
* @param keys The keys of the JSON documents.
* @param path The path within the JSON documents.
* @return An array with requested values for each key.
* <ul>
* <li>For JSONPath (path starts with <code>$</code>): Returns a stringified JSON list
* replies for every possible path, or a string representation of an empty array, if
* path doesn't exist.
* <li>For legacy path (path doesn't start with <code>$</code>): Returns a string
* representation of the value in <code>path</code>. If <code>path</code> doesn't exist,
* the corresponding array element will be <code>null</code>.
* </ul>
* If a <code>key</code> doesn't exist, the corresponding array element will be <code>null
* </code>.
* @example
* <pre>{@code
* Json.set(client, "doc1", "$", "{\"a\": 1, \"b\": [\"one\", \"two\"]}").get();
* Json.set(client, "doc2", "$", "{\"a\": 1, \"c\": false}").get();
* var res = Json.mget(client, new String[] { "doc1", "doc2", "doc3" }, "$.c").get();
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved
* assert Arrays.equals(res, new String[] { "[]", "[false]", null });
* }</pre>
*/
public static CompletableFuture<String[]> mget(
@NonNull BaseClient client, @NonNull String[] keys, @NonNull String path) {
return Json.<Object[]>executeCommand(
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
client, concatenateArrays(new String[] {JSON_MGET}, keys, new String[] {path}))
.thenApply(res -> castArray(res, String.class));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of confusion here. So 3 things:

  1. the thenApply() call here on line 452
  2. the <Object[]> type hint before invoking executeCommand() on line 450
  3. the final convertion in executeCommand(), which will convert to whatever the method return type is (.thenApply(r -> (T) r)) on line 2709 (for mget(), T is String[])

Not sure why all 3 are needed here. Isn't 3 alone good enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib.rs returns an arbitrary Object[], it cannot be casted to String[], so we have to use castArray.
So what happens here:

  1. libglide returns Object[]
  2. customCommand returns Object (which is actually array, yes)
  3. with Json.<Object[]> prefix we instruct that we expect Object[]
  4. with .thenApply(r -> (T) r) we safely cast this Object to Object[]
  5. with castArray we convert Object[] to String[]

So in total there is a chained Object -> Object[] -> String[] conversion distrubuted over multiple places of code 🤷
I'd like to refactor this if you give me an idea.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note: we kind of have to go through Object -> Object[] -> String[].
This is because castArray requires the Object[] type. We cannot cast Object[] to String[] directly, but we need to cast all inner elements (thank you Java).

}

/**
* Retrieves the JSON values at the specified <code>path</code> stored at multiple <code>keys
* </code>.
*
* @apiNote When in cluster mode, if keys in <code>keys</code> map to different hash slots, the
* command will be split across these slots and executed separately for each. This means the
* command is atomic only at the slot level. If one or more slot-specific requests fail, the
* entire call will return the first encountered error, even though some requests may have
* succeeded while others did not. If this behavior impacts your application logic, consider
* splitting the request into sub-requests per slot to ensure atomicity.
* @param client The client to execute the command.
* @param keys The keys of the JSON documents.
* @param path The path within the JSON documents.
* @return An array with requested values for each key.
* <ul>
* <li>For JSONPath (path starts with <code>$</code>): Returns a stringified JSON list
* replies for every possible path, or a string representation of an empty array, if
* path doesn't exist.
* <li>For legacy path (path doesn't start with <code>$</code>): Returns a string
* representation of the value in <code>path</code>. If <code>path</code> doesn't exist,
* the corresponding array element will be <code>null</code>.
* </ul>
* If a <code>key</code> doesn't exist, the corresponding array element will be <code>null
* </code>.
* @example
* <pre>{@code
* Json.set(client, "doc1", "$", "{\"a\": 1, \"b\": [\"one\", \"two\"]}").get();
* Json.set(client, "doc2", "$", "{\"a\": 1, \"c\": false}").get();
* var res = Json.mget(client, new GlideString[] { gs("doc1"), gs("doc2"), gs("doc3") }, gs("$.c")).get();
* assert Arrays.equals(res, new GlideString[] { gs("[]"), gs("[false]"), null });
* }</pre>
*/
public static CompletableFuture<GlideString[]> mget(
@NonNull BaseClient client, @NonNull GlideString[] keys, @NonNull GlideString path) {
return Json.<Object[]>executeCommand(
client,
concatenateArrays(new GlideString[] {gs(JSON_MGET)}, keys, new GlideString[] {path}))
.thenApply(res -> castArray(res, GlideString.class));
}

/**
* Appends one or more <code>values</code> to the JSON array at the specified <code>path</code>
* within the JSON document stored at <code>key</code>.
Expand Down
23 changes: 23 additions & 0 deletions java/integTest/src/test/java/glide/modules/JsonTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import glide.api.models.commands.FlushMode;
import glide.api.models.commands.InfoOptions.Section;
import glide.api.models.commands.json.JsonGetOptions;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import lombok.SneakyThrows;
Expand Down Expand Up @@ -849,6 +850,28 @@ public void objkeys() {
assertArrayEquals(new Object[] {gs("a"), gs("b")}, res);
}

@Test
@SneakyThrows
public void mget() {
String key1 = UUID.randomUUID().toString();
String key2 = UUID.randomUUID().toString();
var data =
Map.of(
key1, "{\"a\": 1, \"b\": [\"one\", \"two\"]}",
key2, "{\"a\": 1, \"c\": false}");

for (var entry : data.entrySet()) {
assertEquals("OK", Json.set(client, entry.getKey(), "$", entry.getValue()).get());
}

var res1 =
Json.mget(client, new String[] {key1, key2, UUID.randomUUID().toString()}, "$.c").get();
assertArrayEquals(new String[] {"[]", "[false]", null}, res1);

var res2 = Json.mget(client, new GlideString[] {gs(key1), gs(key2)}, gs(".b[*]")).get();
assertArrayEquals(new GlideString[] {gs("\"one\""), null}, res2);
}

@Test
@SneakyThrows
public void json_forget() {
Expand Down