Java: Support most RESP3 types (valkey-io#804)

* Implement support for receiving Redis arrays

* Add support for simpler RESP3 types

* Add some unit tests for FFI

* Add more tests and support for VerbatimStrings

* Fix and test Map and Set types

* Add TODO about using conditional compilation flag in a separate test Gradle task

* Remove dependency on ordered_float

* Fix spotless errors

* Attempt to fix CI test failures by updating Gradle task to build Rust

* Update build.gradle to try to fix CI tests again

* Change assertTrue nilValue == null to assertNull

Co-authored-by: Yury-Fridlyand <[email protected]>

* Remove init and teardown

* Parameterize tests

* Update client/build.gradle

* Remove TODO about using primitive integers

* Add tests for casting Redis Ints to Java Integers

* Move FFI testing code to a separate module

* Update build.gradle to use a new task to run FfiTests

* Apply spotless changes

* Address PR comments on FFI tests

* Run spotless with new whitespace changes

---------

Co-authored-by: Yury-Fridlyand <[email protected]>

java/client/build.gradle

@@ -50,22 +50,34 @@ tasks.register('cleanProtobuf') {
     }
 }
 
+tasks.register('cleanRust') {
+    doFirst {
+        project.delete(Paths.get(project.projectDir.path, '../target').toString())
+    }
+}
+
 tasks.register('buildRustRelease', Exec) {
     commandLine 'cargo', 'build', '--release'
     workingDir project.rootDir
-    environment 'CARGO_TERM_COLOR', 'always'
+    environment CARGO_TERM_COLOR: 'always'
 }
 
 tasks.register('buildRustReleaseStrip', Exec) {
     commandLine 'cargo', 'build', '--release', '--strip'
     workingDir project.rootDir
-    environment 'CARGO_TERM_COLOR', 'always'
+    environment CARGO_TERM_COLOR: 'always'
 }
 
 tasks.register('buildRust', Exec) {
     commandLine 'cargo', 'build'
     workingDir project.rootDir
-    environment 'CARGO_TERM_COLOR', 'always'
+    environment CARGO_TERM_COLOR: 'always'
+}
+
+tasks.register('buildRustFfi', Exec) {
+    commandLine 'cargo', 'build'
+    workingDir project.rootDir
+    environment CARGO_TERM_COLOR: 'always', CARGO_BUILD_RUSTFLAGS: '--cfg ffi_test'
 }
 
 tasks.register('buildWithRust') {
@@ -88,19 +100,31 @@ tasks.register('buildWithProto') {
     finalizedBy 'build'
 }
 
+tasks.register('testFfi', Test) {
+    dependsOn 'buildRustFfi'
+    include "glide/ffi/FfiTest.class"
+}
+
 tasks.register('buildAll') {
-    dependsOn 'protobuf', 'buildRust'
+    dependsOn 'protobuf', 'buildRust', 'testFfi'
     finalizedBy 'build'
 }
 
-compileJava.dependsOn('protobuf')
-clean.dependsOn('cleanProtobuf')
+compileJava.dependsOn('protobuf', 'buildRustRelease')
+clean.dependsOn('cleanProtobuf', 'cleanRust')
+test.dependsOn('buildRust')
+testFfi.dependsOn('buildRust')
+
+test {
+    exclude "glide/ffi/FfiTest.class"
+}
 
 tasks.withType(Test) {
     testLogging {
         exceptionFormat "full"
         events "started", "skipped", "passed", "failed"
         showStandardStreams true
     }
-    jvmArgs "-Djava.library.path=${projectDir}/../target/release:${projectDir}/../target/debug"
+    jvmArgs "-Djava.library.path=${projectDir}/../target/debug"
 }
+

java/client/src/test/java/glide/ffi/FfiTest.java

@@ -0,0 +1,143 @@
+package glide.ffi;
+
+import static org.junit.jupiter.api.Assertions.assertAll;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import glide.ffi.resolvers.RedisValueResolver;
+import java.util.HashMap;
+import java.util.HashSet;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class FfiTest {
+
+    static {
+        System.loadLibrary("glide_rs");
+    }
+
+    public static native long createLeakedNil();
+
+    public static native long createLeakedSimpleString(String value);
+
+    public static native long createLeakedOkay();
+
+    public static native long createLeakedInt(long value);
+
+    public static native long createLeakedBulkString(byte[] value);
+
+    public static native long createLeakedLongArray(long[] value);
+
+    public static native long createLeakedMap(long[] keys, long[] values);
+
+    public static native long createLeakedDouble(double value);
+
+    public static native long createLeakedBoolean(boolean value);
+
+    public static native long createLeakedVerbatimString(String value);
+
+    public static native long createLeakedLongSet(long[] value);
+
+    @Test
+    public void redisValueToJavaValue_Nil() {
+        long ptr = FfiTest.createLeakedNil();
+        Object nilValue = RedisValueResolver.valueFromPointer(ptr);
+        assertNull(nilValue);
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"hello", "cat", "dog"})
+    public void redisValueToJavaValue_SimpleString(String input) {
+        long ptr = FfiTest.createLeakedSimpleString(input);
+        Object simpleStringValue = RedisValueResolver.valueFromPointer(ptr);
+        assertEquals(input, simpleStringValue);
+    }
+
+    @Test
+    public void redisValueToJavaValue_Okay() {
+        long ptr = FfiTest.createLeakedOkay();
+        Object okayValue = RedisValueResolver.valueFromPointer(ptr);
+        assertEquals("OK", okayValue);
+    }
+
+    @ParameterizedTest
+    @ValueSource(longs = {0L, 100L, 774L, Integer.MAX_VALUE + 1, Integer.MIN_VALUE - 1})
+    public void redisValueToJavaValue_Int(Long input) {
+        long ptr = FfiTest.createLeakedInt(input);
+        Object longValue = RedisValueResolver.valueFromPointer(ptr);
+        assertTrue(longValue instanceof Long);
+        assertEquals(input, longValue);
+    }
+
+    @Test
+    public void redisValueToJavaValue_BulkString() {
+        String input = "😀\n💎\n🗿";
+        byte[] bulkString = input.getBytes();
+        long ptr = FfiTest.createLeakedBulkString(bulkString);
+        Object bulkStringValue = RedisValueResolver.valueFromPointer(ptr);
+        assertEquals(input, bulkStringValue);
+    }
+
+    @Test
+    public void redisValueToJavaValue_Array() {
+        long[] array = {1L, 2L, 3L};
+        long ptr = FfiTest.createLeakedLongArray(array);
+        Object longArrayValue = RedisValueResolver.valueFromPointer(ptr);
+        assertTrue(longArrayValue instanceof Object[]);
+        Object[] result = (Object[]) longArrayValue;
+        assertArrayEquals(new Object[] {1L, 2L, 3L}, result);
+    }
+
+    @Test
+    public void redisValueToJavaValue_Map() {
+        long[] keys = {12L, 14L, 23L};
+        long[] values = {1L, 2L, 3L};
+        long ptr = FfiTest.createLeakedMap(keys, values);
+        Object mapValue = RedisValueResolver.valueFromPointer(ptr);
+        assertTrue(mapValue instanceof HashMap);
+        HashMap<Object, Object> result = (HashMap<Object, Object>) mapValue;
+        assertAll(
+                () -> assertEquals(1L, result.get(12L)),
+                () -> assertEquals(2L, result.get(14L)),
+                () -> assertEquals(3L, result.get(23L)));
+    }
+
+    @ParameterizedTest
+    @ValueSource(doubles = {1.0d, 25.2d, 103.5d})
+    public void redisValueToJavaValue_Double(Double input) {
+        long ptr = FfiTest.createLeakedDouble(input);
+        Object doubleValue = RedisValueResolver.valueFromPointer(ptr);
+        assertEquals(input, doubleValue);
+    }
+
+    @Test
+    public void redisValueToJavaValue_Boolean() {
+        long ptr = FfiTest.createLeakedBoolean(true);
+        Object booleanValue = RedisValueResolver.valueFromPointer(ptr);
+        assertTrue((Boolean) booleanValue);
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"hello", "cat", "dog"})
+    public void redisValueToJavaValue_VerbatimString(String input) {
+        long ptr = FfiTest.createLeakedVerbatimString(input);
+        Object verbatimStringValue = RedisValueResolver.valueFromPointer(ptr);
+        assertEquals(input, verbatimStringValue);
+    }
+
+    @Test
+    public void redisValueToJavaValue_Set() {
+        long[] array = {1L, 2L, 2L};
+        long ptr = FfiTest.createLeakedLongSet(array);
+        Object longSetValue = RedisValueResolver.valueFromPointer(ptr);
+        assertTrue(longSetValue instanceof HashSet);
+        HashSet<Long> result = (HashSet<Long>) longSetValue;
+        assertAll(
+                () -> assertTrue(result.contains(1L)),
+                () -> assertTrue(result.contains(2L)),
+                () -> assertEquals(result.size(), 2));
+    }
+}

java/src/ffi_test.rs

@@ -0,0 +1,143 @@
+use jni::{
+    objects::{JClass, JLongArray},
+    sys::jlong,
+    JNIEnv,
+};
+use redis::Value;
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedNil<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+) -> jlong {
+    let redis_value = Value::Nil;
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedSimpleString<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jni::objects::JString<'local>,
+) -> jlong {
+    let value: String = env.get_string(&value).unwrap().into();
+    let redis_value = Value::SimpleString(value);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedOkay<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+) -> jlong {
+    let redis_value = Value::Okay;
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedInt<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jlong,
+) -> jlong {
+    let redis_value = Value::Int(value);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBulkString<'local>(
+    env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jni::objects::JByteArray<'local>,
+) -> jlong {
+    let value = env.convert_byte_array(&value).unwrap();
+    let value = value.into_iter().collect::<Vec<u8>>();
+    let redis_value = Value::BulkString(value);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedLongArray<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: JLongArray<'local>,
+) -> jlong {
+    let array = java_long_array_to_value(&mut env, &value);
+    let redis_value = Value::Array(array);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedMap<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    keys: JLongArray<'local>,
+    values: JLongArray<'local>,
+) -> jlong {
+    let keys_vec = java_long_array_to_value(&mut env, &keys);
+    let values_vec = java_long_array_to_value(&mut env, &values);
+    let map: Vec<(Value, Value)> = keys_vec.into_iter().zip(values_vec).collect();
+    let redis_value = Value::Map(map);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedDouble<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jni::sys::jdouble,
+) -> jlong {
+    let redis_value = Value::Double(value.into());
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBoolean<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jni::sys::jboolean,
+) -> jlong {
+    let redis_value = Value::Boolean(value != 0);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedVerbatimString<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: jni::objects::JString<'local>,
+) -> jlong {
+    use redis::VerbatimFormat;
+    let value: String = env.get_string(&value).unwrap().into();
+    let redis_value = Value::VerbatimString {
+        format: VerbatimFormat::Text,
+        text: value,
+    };
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedLongSet<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    value: JLongArray<'local>,
+) -> jlong {
+    let set = java_long_array_to_value(&mut env, &value);
+    let redis_value = Value::Set(set);
+    Box::leak(Box::new(redis_value)) as *mut Value as jlong
+}
+
+fn java_long_array_to_value<'local>(
+    env: &mut JNIEnv<'local>,
+    array: &JLongArray<'local>,
+) -> Vec<Value> {
+    use jni::objects::ReleaseMode;
+    let elements = unsafe {
+        env.get_array_elements(array, ReleaseMode::NoCopyBack)
+            .unwrap()
+    };
+    elements
+        .iter()
+        .map(|value| Value::Int(*value))
+        .collect::<Vec<Value>>()
+}