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.

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: Support most RESP3 types #804

Merged
merged 23 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
84c89ca
Implement support for receiving Redis arrays
jonathanl-bq Jan 3, 2024
f9527d8
Add support for simpler RESP3 types
jonathanl-bq Jan 5, 2024
4c142bc
Add some unit tests for FFI
jonathanl-bq Jan 9, 2024
3e6802b
Add more tests and support for VerbatimStrings
jonathanl-bq Jan 9, 2024
3158900
Fix and test Map and Set types
jonathanl-bq Jan 10, 2024
4f5216e
Add TODO about using conditional compilation flag in a separate test …
jonathanl-bq Jan 10, 2024
a36baba
Remove dependency on ordered_float
jonathanl-bq Jan 10, 2024
0bf715c
Fix spotless errors
jonathanl-bq Jan 10, 2024
c76ae6c
Attempt to fix CI test failures by updating Gradle task to build Rust
jonathanl-bq Jan 10, 2024
487c0b2
Update build.gradle to try to fix CI tests again
jonathanl-bq Jan 10, 2024
fcbafc5
Change assertTrue nilValue == null to assertNull
jonathanl-bq Jan 10, 2024
cb930ca
Remove init and teardown
jonathanl-bq Jan 10, 2024
e612941
Parameterize tests
jonathanl-bq Jan 12, 2024
0539531
Update client/build.gradle
jonathanl-bq Jan 12, 2024
5893289
Remove TODO about using primitive integers
jonathanl-bq Jan 12, 2024
9832bbb
Add tests for casting Redis Ints to Java Integers
jonathanl-bq Jan 12, 2024
3649335
Move FFI testing code to a separate module
jonathanl-bq Jan 13, 2024
e5d678e
Update build.gradle to use a new task to run FfiTests
jonathanl-bq Jan 15, 2024
38caf3b
Apply spotless changes
jonathanl-bq Jan 15, 2024
d9bf353
Merge pull request #55 from Bit-Quill/java/dev_lotjonat_resp3_pt1
jonathanl-bq Jan 16, 2024
1cf634b
Address PR comments on FFI tests
jonathanl-bq Jan 19, 2024
6e2e89d
Merge branch 'main' into java/integ_lotjonat_resp3_pt1
jonathanl-bq Jan 22, 2024
51c3d9d
Run spotless with new whitespace changes
jonathanl-bq Jan 22, 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
54 changes: 14 additions & 40 deletions java/client/src/test/java/glide/ffi/FfiTest.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package glide.ffi;

import static java.lang.Math.toIntExact;
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.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import glide.ffi.resolvers.RedisValueResolver;
Expand Down Expand Up @@ -32,7 +31,7 @@ public class FfiTest {

public static native long createLeakedLongArray(long[] value);

public static native long createLeakedMap();
public static native long createLeakedMap(long[] keys, long[] values);

public static native long createLeakedDouble(double value);

Expand Down Expand Up @@ -65,44 +64,17 @@ public void redisValueToJavaValue_Okay() {
}

@ParameterizedTest
@ValueSource(longs = {0L, 100L, 774L})
@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_CastToIntegerMax() {
long ptr = FfiTest.createLeakedInt(Integer.MAX_VALUE);
Object longValue = RedisValueResolver.valueFromPointer(ptr);
toIntExact((Long) longValue);
}

@Test
public void redisValueToJavaValue_CastToIntegerTooLarge() {
long ptr = FfiTest.createLeakedInt(((long) Integer.MAX_VALUE) + 1);
Object longValue = RedisValueResolver.valueFromPointer(ptr);
assertThrows(ArithmeticException.class, () -> toIntExact((Long) longValue));
}

@Test
public void redisValueToJavaValue_CastToIntegerMin() {
long ptr = FfiTest.createLeakedInt(Integer.MIN_VALUE);
Object longValue = RedisValueResolver.valueFromPointer(ptr);
toIntExact((Long) longValue);
}

@Test
public void redisValueToJavaValue_CastToIntegerTooSmall() {
long ptr = FfiTest.createLeakedInt(((long) Integer.MIN_VALUE) - 1);
Object longValue = RedisValueResolver.valueFromPointer(ptr);
assertThrows(ArithmeticException.class, () -> toIntExact((Long) longValue));
}

@ParameterizedTest
@ValueSource(strings = {"hello", "cat", "dog"})
public void redisValueToJavaValue_BulkString(String input) {
public void redisValueToJavaValue_BulkString() {
String input = "😀\n💎\n🗿";
Copy link
Contributor

Choose a reason for hiding this comment

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

impeccable choice of emojis!

byte[] bulkString = input.getBytes();
long ptr = FfiTest.createLeakedBulkString(bulkString);
Object bulkStringValue = RedisValueResolver.valueFromPointer(ptr);
Expand All @@ -116,19 +88,21 @@ public void redisValueToJavaValue_Array() {
Object longArrayValue = RedisValueResolver.valueFromPointer(ptr);
assertTrue(longArrayValue instanceof Object[]);
Object[] result = (Object[]) longArrayValue;
assertAll(
() -> assertEquals(1L, result[0]),
() -> assertEquals(2L, result[1]),
() -> assertEquals(3L, result[2]));
assertArrayEquals(new Object[] {1L, 2L, 3L}, result);
}

@Test
public void redisValueToJavaValue_Map() {
long ptr = FfiTest.createLeakedMap();
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(2L, result.get(1L)), () -> assertEquals("hi", result.get(3L)));
assertAll(
() -> assertEquals(1L, result.get(12L)),
() -> assertEquals(2L, result.get(14L)),
() -> assertEquals(3L, result.get(23L)));
}

@ParameterizedTest
Expand Down
51 changes: 28 additions & 23 deletions java/src/ffi_test.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use jni::{objects::JClass, sys::jlong, JNIEnv};
use jni::{
objects::{JClass, JLongArray},
sys::jlong,
JNIEnv,
};
use redis::Value;

#[no_mangle]
Expand Down Expand Up @@ -56,29 +60,23 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBulkString<'local>(
pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedLongArray<'local>(
mut env: JNIEnv<'local>,
_class: JClass<'local>,
value: jni::objects::JLongArray<'local>,
value: JLongArray<'local>,
) -> jlong {
use jni::objects::ReleaseMode;
let value = unsafe {
env.get_array_elements(&value, ReleaseMode::NoCopyBack)
.unwrap()
};
let array = value
.iter()
.map(|val| Value::Int(*val))
.collect::<Vec<Value>>();
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>(
_env: JNIEnv<'local>,
shachlanAmazon marked this conversation as resolved.
Show resolved Hide resolved
mut env: JNIEnv<'local>,
_class: JClass<'local>,
keys: JLongArray<'local>,
values: JLongArray<'local>,
) -> jlong {
let mut map: Vec<(Value, Value)> = Vec::new();
map.push((Value::Int(1i64), Value::Int(2i64)));
map.push((Value::Int(3i64), Value::SimpleString("hi".to_string())));
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
}
Expand Down Expand Up @@ -122,17 +120,24 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedVerbatimString<'local>
pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedLongSet<'local>(
mut env: JNIEnv<'local>,
_class: JClass<'local>,
value: jni::objects::JLongArray<'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 value = unsafe {
env.get_array_elements(&value, ReleaseMode::NoCopyBack)
let elements = unsafe {
env.get_array_elements(array, ReleaseMode::NoCopyBack)
.unwrap()
};
let set = value
elements
.iter()
.map(|val| Value::Int(*val))
.collect::<Vec<Value>>();
let redis_value = Value::Set(set);
Box::leak(Box::new(redis_value)) as *mut Value as jlong
.map(|value| Value::Int(*value))
.collect::<Vec<Value>>()
}
Loading