refactor(binding/java): read should return bytes (#3153)

Signed-off-by: tison <[email protected]>

bindings/java/src/blocking_operator.rs

@@ -21,9 +21,9 @@ use jni::objects::JByteArray;
 use jni::objects::JClass;
 use jni::objects::JObject;
 use jni::objects::JString;
-use jni::sys::jlong;
-use jni::sys::jstring;
+use jni::sys::{jbyteArray, jlong};
 use jni::JNIEnv;
+
 use opendal::layers::BlockingLayer;
 use opendal::BlockingOperator;
 use opendal::Operator;
@@ -78,17 +78,18 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_read(
     _: JClass,
     op: *mut BlockingOperator,
     path: JString,
-) -> jstring {
+) -> jbyteArray {
     intern_read(&mut env, &mut *op, path).unwrap_or_else(|e| {
         e.throw(&mut env);
-        JObject::null().into_raw()
+        JByteArray::default().into_raw()
     })
 }
 
-fn intern_read(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result<jstring> {
+fn intern_read(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result<jbyteArray> {
     let path = env.get_string(&path)?;
-    let content = String::from_utf8(op.read(path.to_str()?)?)?;
-    Ok(env.new_string(content)?.into_raw())
+    let content = op.read(path.to_str()?)?;
+    let result = env.byte_array_from_slice(content.as_slice())?;
+    Ok(result.into_raw())
 }
 
 /// # Safety

bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java

@@ -49,7 +49,7 @@ public void write(String path, byte[] content) {
         write(nativeHandle, path, content);
     }
 
-    public String read(String path) {
+    public byte[] read(String path) {
         return read(nativeHandle, path);
     }
 
@@ -68,7 +68,7 @@ public Metadata stat(String path) {
 
     private static native void write(long nativeHandle, String path, byte[] content);
 
-    private static native String read(long nativeHandle, String path);
+    private static native byte[] read(long nativeHandle, String path);
 
     private static native void delete(long nativeHandle, String path);
 

bindings/java/src/main/java/org/apache/opendal/Operator.java

@@ -137,7 +137,7 @@ public CompletableFuture<Metadata> stat(String path) {
         return f.thenApply(Metadata::new);
     }
 
-    public CompletableFuture<String> read(String path) {
+    public CompletableFuture<byte[]> read(String path) {
         final long requestId = read(nativeHandle, path);
         return AsyncRegistry.take(requestId);
     }

bindings/java/src/operator.rs

@@ -222,10 +222,9 @@ fn intern_read(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result<jlo
 
 async fn do_read<'local>(op: &mut Operator, path: String) -> Result<JObject<'local>> {
     let content = op.read(&path).await?;
-    let content = String::from_utf8(content)?;
 
     let env = unsafe { get_current_env() };
-    let result = env.new_string(content)?;
+    let result = env.byte_array_from_slice(content.as_slice())?;
     Ok(result.into())
 }
 

bindings/java/src/test/java/org/apache/opendal/AsyncStepsTest.java

@@ -19,13 +19,15 @@
 
 package org.apache.opendal;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import io.cucumber.java.en.Given;
 import io.cucumber.java.en.Then;
 import io.cucumber.java.en.When;
+import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
@@ -67,8 +69,8 @@ public void the_async_file_test_content_length_must_be_13(String path, int lengt
 
     @Then("The async file {string} must have content {string}")
     public void the_async_file_test_must_have_content_hello_world(String path, String content) {
-        String readContent = op.read(path).join();
-        assertEquals(content, readContent);
+        byte[] readContent = op.read(path).join();
+        assertThat(readContent).isEqualTo(content.getBytes(StandardCharsets.UTF_8));
     }
 
     @Then("The presign operation should success or raise exception Unsupported")

bindings/java/src/test/java/org/apache/opendal/BlockingOperatorTest.java

@@ -21,6 +21,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.jupiter.api.AfterEach;
@@ -53,7 +54,7 @@ public void testStatNotExistFile() {
     @Test
     public void testCreateAndDelete() {
         op.write("testCreateAndDelete", "Odin");
-        assertThat(op.read("testCreateAndDelete")).isEqualTo("Odin");
+        assertThat(op.read("testCreateAndDelete")).isEqualTo("Odin".getBytes(StandardCharsets.UTF_8));
         op.delete("testCreateAndDelete");
         assertThatExceptionOfType(OpenDALException.class)
                 .isThrownBy(() -> op.stat("testCreateAndDelete"))

bindings/java/src/test/java/org/apache/opendal/OperatorTest.java

@@ -21,6 +21,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -42,7 +43,7 @@ public void testCreateAndDelete() {
         @Cleanup Operator op = new Operator("Memory", params);
 
         op.write("testCreateAndDelete", "Odin").join();
-        assertThat(op.read("testCreateAndDelete").join()).isEqualTo("Odin");
+        assertThat(op.read("testCreateAndDelete").join()).isEqualTo("Odin".getBytes(StandardCharsets.UTF_8));
         op.delete("testCreateAndDelete").join();
         assertThatThrownBy(() -> op.stat("testCreateAndDelete").join())
                 .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound));
@@ -59,17 +60,18 @@ public void testAppendManyTimes() {
         for (int i = 0; i < trunks.length; i++) {
             op.append("testAppendManyTimes", trunks[i]).join();
             String expected = Arrays.stream(trunks).limit(i + 1).collect(Collectors.joining());
-            assertThat(op.read("testAppendManyTimes").join()).isEqualTo(expected);
+            assertThat(op.read("testAppendManyTimes").join()).isEqualTo(expected.getBytes(StandardCharsets.UTF_8));
         }
 
         // write overwrite existing content
         op.write("testAppendManyTimes", "new attempt").join();
-        assertThat(op.read("testAppendManyTimes").join()).isEqualTo("new attempt");
+        assertThat(op.read("testAppendManyTimes").join()).isEqualTo("new attempt".getBytes(StandardCharsets.UTF_8));
 
         for (int i = 0; i < trunks.length; i++) {
             op.append("testAppendManyTimes", trunks[i]).join();
             String expected = Arrays.stream(trunks).limit(i + 1).collect(Collectors.joining());
-            assertThat(op.read("testAppendManyTimes").join()).isEqualTo("new attempt" + expected);
+            assertThat(op.read("testAppendManyTimes").join())
+                    .isEqualTo(("new attempt" + expected).getBytes(StandardCharsets.UTF_8));
         }
     }
 }

bindings/java/src/test/java/org/apache/opendal/RedisServiceTest.java

@@ -22,6 +22,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 import lombok.Cleanup;
@@ -48,7 +49,7 @@ public void testAccessRedisService() {
         @Cleanup final Operator op = new Operator("Redis", params);
 
         op.write("testAccessRedisService", "Odin").join();
-        assertThat(op.read("testAccessRedisService").join()).isEqualTo("Odin");
+        assertThat(op.read("testAccessRedisService").join()).isEqualTo("Odin".getBytes(StandardCharsets.UTF_8));
         op.delete("testAccessRedisService").join();
         assertThatThrownBy(() -> op.stat("testAccessRedisService").join())
                 .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound));
@@ -64,7 +65,7 @@ public void testAccessRedisServiceBlocking() {
         @Cleanup final BlockingOperator op = new BlockingOperator("Redis", params);
 
         op.write("testAccessRedisServiceBlocking", "Odin");
-        assertThat(op.read("testAccessRedisServiceBlocking")).isEqualTo("Odin");
+        assertThat(op.read("testAccessRedisServiceBlocking")).isEqualTo("Odin".getBytes(StandardCharsets.UTF_8));
         op.delete("testAccessRedisServiceBlocking");
         assertThatExceptionOfType(OpenDALException.class)
                 .isThrownBy(() -> op.stat("testAccessRedisServiceBlocking"))

bindings/java/src/test/java/org/apache/opendal/StepsTest.java

@@ -19,12 +19,14 @@
 
 package org.apache.opendal;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import io.cucumber.java.en.Given;
 import io.cucumber.java.en.Then;
 import io.cucumber.java.en.When;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 import lombok.Cleanup;
@@ -64,7 +66,7 @@ public void the_blocking_file_test_content_length_must_be_13(String path, int le
 
     @Then("The blocking file {string} must have content {string}")
     public void the_blocking_file_test_must_have_content_hello_world(String path, String content) {
-        String readContent = op.read(path);
-        assertEquals(content, readContent);
+        byte[] readContent = op.read(path);
+        assertThat(readContent).isEqualTo(content.getBytes(StandardCharsets.UTF_8));
     }
 }