From 7948b575e1b6bbcda4fa3e736105ad80034c73f5 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 2 Jul 2024 15:11:32 -0700 Subject: [PATCH] Get protocol string constants from Rust instead of hard-coding them --- .../java/glide/api/RedisClusterClient.java | 5 +- .../api/models/commands/scan/ScanOptions.java | 14 +- .../resolvers/ClusterScanCursorResolver.java | 11 ++ .../ffi/resolvers/ObjectTypeResolver.java | 37 ++++ java/src/lib.rs | 158 +++++++++++++++++- 5 files changed, 212 insertions(+), 13 deletions(-) create mode 100644 java/client/src/main/java/glide/ffi/resolvers/ObjectTypeResolver.java diff --git a/java/client/src/main/java/glide/api/RedisClusterClient.java b/java/client/src/main/java/glide/api/RedisClusterClient.java index 19689594e7..800c57045e 100644 --- a/java/client/src/main/java/glide/api/RedisClusterClient.java +++ b/java/client/src/main/java/glide/api/RedisClusterClient.java @@ -1002,10 +1002,9 @@ public CompletableFuture sortStore( return commandManager.submitNewCommand(Sort, arguments, this::handleLongResponse); } + /** A {@link ClusterScanCursor} implementation for interacting with the Rust layer. */ private static final class NativeClusterScanCursor implements CommandManager.ClusterScanCursorDetail { - // TODO: This should be made a constant in Rust. - private static final String FINISHED_CURSOR_MARKER = "finished"; private String cursorHandle; private boolean isFinished; @@ -1014,7 +1013,7 @@ private static final class NativeClusterScanCursor // This is for internal use only. public NativeClusterScanCursor(@NonNull String cursorHandle) { this.cursorHandle = cursorHandle; - this.isFinished = FINISHED_CURSOR_MARKER.equals(cursorHandle); + this.isFinished = ClusterScanCursorResolver.FINISHED_CURSOR_HANDLE.equals(cursorHandle); } @Override diff --git a/java/client/src/main/java/glide/api/models/commands/scan/ScanOptions.java b/java/client/src/main/java/glide/api/models/commands/scan/ScanOptions.java index c08c38dfac..153de3f1c4 100644 --- a/java/client/src/main/java/glide/api/models/commands/scan/ScanOptions.java +++ b/java/client/src/main/java/glide/api/models/commands/scan/ScanOptions.java @@ -3,6 +3,7 @@ import glide.api.commands.GenericClusterCommands; import glide.api.commands.GenericCommands; +import glide.ffi.resolvers.ObjectTypeResolver; import glide.utils.ArrayTransformUtils; import lombok.experimental.SuperBuilder; @@ -25,13 +26,12 @@ public class ScanOptions extends BaseScanOptions { private final ObjectType type; public enum ObjectType { - // TODO: Get names from Rust rather than hard-coding them. - STRING("String"), - LIST("List"), - SET("Set"), - ZSET("ZSet"), - HASH("Hash"), - STREAM("Stream"); + STRING(ObjectTypeResolver.OBJECT_TYPE_STRING_NATIVE_NAME), + LIST(ObjectTypeResolver.OBJECT_TYPE_LIST_NATIVE_NAME), + SET(ObjectTypeResolver.OBJECT_TYPE_SET_NATIVE_NAME), + ZSET(ObjectTypeResolver.OBJECT_TYPE_ZSET_NATIVE_NAME), + HASH(ObjectTypeResolver.OBJECT_TYPE_HASH_NATIVE_NAME), + STREAM(ObjectTypeResolver.OBJECT_TYPE_STREAM_NATIVE_NAME); /** * @return the name of the enum when communicating with the native layer. diff --git a/java/client/src/main/java/glide/ffi/resolvers/ClusterScanCursorResolver.java b/java/client/src/main/java/glide/ffi/resolvers/ClusterScanCursorResolver.java index 289a96377e..8657bfa0f4 100644 --- a/java/client/src/main/java/glide/ffi/resolvers/ClusterScanCursorResolver.java +++ b/java/client/src/main/java/glide/ffi/resolvers/ClusterScanCursorResolver.java @@ -1,11 +1,22 @@ /** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ package glide.ffi.resolvers; +import glide.managers.CommandManager; + +/** + * Helper class for invoking JNI resources for {@link CommandManager.ClusterScanCursorDetail} + * implementations. + */ public final class ClusterScanCursorResolver { + public static final String FINISHED_CURSOR_HANDLE; + // TODO: consider lazy loading the glide_rs library static { NativeUtils.loadGlideLib(); + FINISHED_CURSOR_HANDLE = getFinishedCursorHandleConstant(); } public static native void releaseNativeCursor(String cursor); + + public static native String getFinishedCursorHandleConstant(); } diff --git a/java/client/src/main/java/glide/ffi/resolvers/ObjectTypeResolver.java b/java/client/src/main/java/glide/ffi/resolvers/ObjectTypeResolver.java new file mode 100644 index 0000000000..deaaa54b68 --- /dev/null +++ b/java/client/src/main/java/glide/ffi/resolvers/ObjectTypeResolver.java @@ -0,0 +1,37 @@ +/** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ +package glide.ffi.resolvers; + +import glide.api.models.commands.scan.ScanOptions; + +/** Helper class for invoking JNI resources for the {@link ScanOptions.ObjectType} enum. */ +public class ObjectTypeResolver { + public static final String OBJECT_TYPE_STRING_NATIVE_NAME; + public static final String OBJECT_TYPE_LIST_NATIVE_NAME; + public static final String OBJECT_TYPE_SET_NATIVE_NAME; + public static final String OBJECT_TYPE_ZSET_NATIVE_NAME; + public static final String OBJECT_TYPE_HASH_NATIVE_NAME; + public static final String OBJECT_TYPE_STREAM_NATIVE_NAME; + + // TODO: consider lazy loading the glide_rs library + static { + NativeUtils.loadGlideLib(); + OBJECT_TYPE_STRING_NATIVE_NAME = getTypeStringConstant(); + OBJECT_TYPE_LIST_NATIVE_NAME = getTypeListConstant(); + OBJECT_TYPE_SET_NATIVE_NAME = getTypeSetConstant(); + OBJECT_TYPE_ZSET_NATIVE_NAME = getTypeZSetConstant(); + OBJECT_TYPE_HASH_NATIVE_NAME = getTypeHashConstant(); + OBJECT_TYPE_STREAM_NATIVE_NAME = getTypeStreamConstant(); + } + + public static native String getTypeStringConstant(); + + public static native String getTypeListConstant(); + + public static native String getTypeSetConstant(); + + public static native String getTypeZSetConstant(); + + public static native String getTypeHashConstant(); + + public static native String getTypeStreamConstant(); +} diff --git a/java/src/lib.rs b/java/src/lib.rs index 46b253b898..3e13db1387 100644 --- a/java/src/lib.rs +++ b/java/src/lib.rs @@ -2,7 +2,16 @@ * Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ use glide_core::start_socket_listener as start_socket_listener_core; + +// Protocol constants to expose to Java. +use glide_core::client::FINISHED_SCAN_CURSOR; +use glide_core::HASH as TYPE_HASH; +use glide_core::LIST as TYPE_LIST; use glide_core::MAX_REQUEST_ARGS_LENGTH as MAX_REQUEST_ARGS_LENGTH_IN_BYTES; +use glide_core::SET as TYPE_SET; +use glide_core::STREAM as TYPE_STREAM; +use glide_core::STRING as TYPE_STRING; +use glide_core::ZSET as TYPE_ZSET; use bytes::Bytes; use jni::errors::Error as JniError; @@ -420,16 +429,25 @@ pub extern "system" fn Java_glide_ffi_resolvers_LoggerResolver_initInternal<'loc } #[no_mangle] -pub extern "system" fn Java_glide_ffi_resolvers_ClusterScanCursorResolver_releaseNativeCursor<'local>( +/// Releases a ClusterScanCursor handle allocated in Rust. +/// +/// This function is meant to be invoked by Java using JNI. +/// +/// * `_env` - The JNI environment. Not used. +/// * `_class` - The class object. Not used. +/// * cursor - The cursor handle to release. +pub extern "system" fn Java_glide_ffi_resolvers_ClusterScanCursorResolver_releaseNativeCursor< + 'local, +>( mut env: JNIEnv<'local>, _class: JClass<'local>, - cursor: JString, + cursor: JString<'local>, ) { handle_panics( move || { fn release_native_cursor( env: &mut JNIEnv<'_>, - cursor: JString, + cursor: JString<'_>, ) -> Result<(), FFIError> { let cursor_str: String = env.get_string(&cursor)?.into(); glide_core::cluster_scan_container::remove_scan_state_cursor(cursor_str); @@ -442,3 +460,137 @@ pub extern "system" fn Java_glide_ffi_resolvers_ClusterScanCursorResolver_releas ) .unwrap_or(()) } + +/// Returns the String representing a finished cursor handle. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ClusterScanCursorResolver_getFinishedCursorHandleConstant< + 'local, +>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, FINISHED_SCAN_CURSOR, "getFinishedCursorHandleConstant") +} + +/// Returns the String representing the name of the ObjectType String. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeStringConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_STRING, "getTypeStringConstant") +} + +/// Returns the String representing the name of the ObjectType List. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeListConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_LIST, "getTypeListConstant") +} + +/// Returns the String representing the name of the ObjectType Set. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeSetConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_SET, "getTypeSetConstant") +} + +/// Returns the String representing the name of the ObjectType ZSet. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeZSetConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_ZSET, "getTypeZSetConstant") +} + +/// Returns the String representing the name of the ObjectType Hash. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeHashConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_HASH, "getTypeHashConstant") +} + +/// Returns the String representing the name of the ObjectType Set. +/// +/// This function is meant to be invoked by Java using JNI. This is used to ensure +/// that this constant is consistent with the Rust client. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +#[no_mangle] +pub extern "system" fn Java_glide_ffi_resolvers_ObjectTypeResolver_getTypeStreamConstant<'local>( + env: JNIEnv<'local>, + _class: JClass<'local>, +) -> JString<'local> { + safe_create_jstring(env, TYPE_STREAM, "getTypeStreamConstant") +} + +/// Convert a Rust string to a Java String and handle errors. +/// +/// * `env` - The JNI environment. +/// * `_class` - The class object. Not used. +/// * `input` - The String to convert. +/// * `functionName` - The name of the calling function. +fn safe_create_jstring<'local>( + mut env: JNIEnv<'local>, + input: &str, + function_name: &str, +) -> JString<'local> { + handle_panics( + move || { + fn create_jstring<'a>( + env: &mut JNIEnv<'a>, + input: &str, + ) -> Result, FFIError> { + Ok(env.new_string(input)?) + } + let result = create_jstring(&mut env, input); + handle_errors(&mut env, result) + }, + function_name, + ) + .unwrap_or(JString::<'_>::default()) +}