From 3108bd3d6cc18fde8089df8bfaf624c5242a0e88 Mon Sep 17 00:00:00 2001 From: G-XD Date: Sat, 14 Oct 2023 23:05:59 +0800 Subject: [PATCH] feat(binding/java): make Metadata a POJO (#3277) --- bindings/java/src/blocking_operator.rs | 12 ++- bindings/java/src/lib.rs | 54 +++++++++- .../org/apache/opendal/BlockingOperator.java | 4 +- .../java/org/apache/opendal/Metadata.java | 66 ++++++++++-- .../java/org/apache/opendal/Operator.java | 3 +- bindings/java/src/metadata.rs | 61 ----------- bindings/java/src/operator.rs | 8 +- .../org/apache/opendal/test/MetadataTest.java | 100 ++++++++++++++++++ .../test/behavior/AbstractBehaviorTest.java | 48 ++++----- 9 files changed, 247 insertions(+), 109 deletions(-) delete mode 100644 bindings/java/src/metadata.rs create mode 100644 bindings/java/src/test/java/org/apache/opendal/test/MetadataTest.java diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index fd7fffa23550..6f84a2fea1d2 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -19,12 +19,14 @@ use jni::objects::JByteArray; use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; -use jni::sys::{jbyteArray, jlong}; +use jni::sys::jbyteArray; +use jni::sys::jobject; use jni::JNIEnv; use opendal::BlockingOperator; use crate::jstring_to_string; +use crate::make_metadata; use crate::Result; /// # Safety @@ -98,17 +100,17 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_stat( _: JClass, op: *mut BlockingOperator, path: JString, -) -> jlong { +) -> jobject { intern_stat(&mut env, &mut *op, path).unwrap_or_else(|e| { e.throw(&mut env); - 0 + JObject::default().into_raw() }) } -fn intern_stat(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result { +fn intern_stat(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result { let path = jstring_to_string(env, &path)?; let metadata = op.stat(&path)?; - Ok(Box::into_raw(Box::new(metadata)) as jlong) + Ok(make_metadata(env, metadata)?.into_raw()) } /// # Safety diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index d46f6d715076..63adbb35b74a 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -32,13 +32,14 @@ use jni::JavaVM; use once_cell::sync::OnceCell; use opendal::raw::PresignedRequest; use opendal::Capability; +use opendal::EntryMode; +use opendal::Metadata; use opendal::OperatorInfo; use tokio::runtime::Builder; use tokio::runtime::Runtime; mod blocking_operator; mod error; -mod metadata; mod operator; pub(crate) type Result = std::result::Result; @@ -222,6 +223,57 @@ fn make_capability<'a>(env: &mut JNIEnv<'a>, cap: Capability) -> Result(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result> { + let mode = match metadata.mode() { + EntryMode::FILE => 0, + EntryMode::DIR => 1, + EntryMode::Unknown => 2, + }; + + let last_modified = metadata.last_modified().map_or_else( + || Ok::, Error>(JObject::null()), + |v| { + Ok(env.new_object( + "java/util/Date", + "(J)V", + &[JValue::Long(v.timestamp_millis())], + )?) + }, + )?; + + let cache_control = string_to_jstring(env, metadata.cache_control())?; + let content_disposition = string_to_jstring(env, metadata.content_disposition())?; + let content_md5 = string_to_jstring(env, metadata.content_md5())?; + let content_type = string_to_jstring(env, metadata.content_type())?; + let etag = string_to_jstring(env, metadata.etag())?; + let version = string_to_jstring(env, metadata.version())?; + + let result = env + .new_object( + "org/apache/opendal/Metadata", + "(IJLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Date;Ljava/lang/String;)V", + &[ + JValue::Int(mode as jint), + JValue::Long(metadata.content_length() as jlong), + JValue::Object(&content_disposition), + JValue::Object(&content_md5), + JValue::Object(&content_type), + JValue::Object(&cache_control), + JValue::Object(&etag), + JValue::Object(&last_modified), + JValue::Object(&version), + ], + )?; + Ok(result) +} + +fn string_to_jstring<'a>(env: &mut JNIEnv<'a>, s: Option<&str>) -> Result> { + s.map_or_else( + || Ok(JObject::null()), + |v| Ok(env.new_string(v.to_string())?.into()), + ) +} + /// # Safety /// /// The caller must guarantee that the Object passed in is an instance diff --git a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java index 03e8ba8b7458..96f4446412c9 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -67,7 +67,7 @@ public void delete(String path) { } public Metadata stat(String path) { - return new Metadata(stat(nativeHandle, path)); + return stat(nativeHandle, path); } public void createDir(String path) { @@ -91,7 +91,7 @@ public void rename(String sourcePath, String targetPath) { private static native void delete(long nativeHandle, String path); - private static native long stat(long nativeHandle, String path); + private static native Metadata stat(long nativeHandle, String path); private static native long createDir(long nativeHandle, String path); diff --git a/bindings/java/src/main/java/org/apache/opendal/Metadata.java b/bindings/java/src/main/java/org/apache/opendal/Metadata.java index 7713adafa837..6d886a7802a0 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Metadata.java +++ b/bindings/java/src/main/java/org/apache/opendal/Metadata.java @@ -19,26 +19,70 @@ package org.apache.opendal; +import java.util.Date; +import lombok.Data; + /** * Metadata carries all metadata associated with a path. */ -public class Metadata extends NativeObject { - protected Metadata(long nativeHandle) { - super(nativeHandle); +@Data +public class Metadata { + public final EntryMode mode; + public final long contentLength; + public final String contentDisposition; + public final String contentMd5; + public final String contentType; + public final String cacheControl; + public final String etag; + public final Date lastModified; + public final String version; + + public Metadata( + int mode, + long contentLength, + String contentDisposition, + String contentMd5, + String contentType, + String cacheControl, + String etag, + Date lastModified, + String version) { + this.mode = EntryMode.of(mode); + this.contentLength = contentLength; + this.contentDisposition = contentDisposition; + this.contentMd5 = contentMd5; + this.contentType = contentType; + this.cacheControl = cacheControl; + this.etag = etag; + this.lastModified = lastModified; + this.version = version; } public boolean isFile() { - return isFile(nativeHandle); + return mode == EntryMode.FILE; } - public long getContentLength() { - return getContentLength(nativeHandle); + public boolean isDir() { + return mode == EntryMode.DIR; } - @Override - protected native void disposeInternal(long handle); + public enum EntryMode { + /// FILE means the path has data to read. + FILE, + /// DIR means the path can be listed. + DIR, + /// Unknown means we don't know what we can do on this path. + UNKNOWN; - private static native boolean isFile(long nativeHandle); - - private static native long getContentLength(long nativeHandle); + public static EntryMode of(int mode) { + switch (mode) { + case 0: + return EntryMode.FILE; + case 1: + return EntryMode.DIR; + default: + return EntryMode.UNKNOWN; + } + } + } } diff --git a/bindings/java/src/main/java/org/apache/opendal/Operator.java b/bindings/java/src/main/java/org/apache/opendal/Operator.java index 7ca7bccd204d..54273ee1adff 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -148,8 +148,7 @@ public CompletableFuture append(String path, byte[] content) { public CompletableFuture stat(String path) { final long requestId = stat(nativeHandle, path); - final CompletableFuture f = AsyncRegistry.take(requestId); - return f.thenApply(Metadata::new); + return AsyncRegistry.take(requestId); } public CompletableFuture read(String path) { diff --git a/bindings/java/src/metadata.rs b/bindings/java/src/metadata.rs deleted file mode 100644 index c4f8510d7eb5..000000000000 --- a/bindings/java/src/metadata.rs +++ /dev/null @@ -1,61 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use jni::objects::JClass; -use jni::objects::JObject; -use jni::sys::jboolean; -use jni::sys::jlong; -use jni::JNIEnv; -use opendal::Metadata; - -/// # Safety -/// -/// This function should not be called before the Stat are ready. -#[no_mangle] -pub unsafe extern "system" fn Java_org_apache_opendal_Metadata_isFile( - _: JNIEnv, - _: JClass, - ptr: *mut Metadata, -) -> jboolean { - let metadata = &mut *ptr; - metadata.is_file() as jboolean -} - -/// # Safety -/// -/// This function should not be called before the Stat are ready. -#[no_mangle] -pub unsafe extern "system" fn Java_org_apache_opendal_Metadata_getContentLength( - _: JNIEnv, - _: JClass, - ptr: *mut Metadata, -) -> jlong { - let metadata = &mut *ptr; - metadata.content_length() as jlong -} - -/// # Safety -/// -/// This function should not be called before the Stat are ready. -#[no_mangle] -pub unsafe extern "system" fn Java_org_apache_opendal_Metadata_disposeInternal( - _: JNIEnv, - _: JObject, - ptr: *mut Metadata, -) { - drop(Box::from_raw(ptr)); -} diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 97fbaaa9a1e8..ccc06edf466a 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -35,6 +35,7 @@ use crate::get_current_env; use crate::get_global_runtime; use crate::jmap_to_hashmap; use crate::jstring_to_string; +use crate::make_metadata; use crate::make_operator_info; use crate::make_presigned_request; use crate::Result; @@ -181,15 +182,16 @@ fn intern_stat(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result Result { +async fn do_stat<'local>(op: &mut Operator, path: String) -> Result> { let metadata = op.stat(&path).await?; - Ok(Box::into_raw(Box::new(metadata)) as jlong) + let mut env = unsafe { get_current_env() }; + make_metadata(&mut env, metadata) } /// # Safety diff --git a/bindings/java/src/test/java/org/apache/opendal/test/MetadataTest.java b/bindings/java/src/test/java/org/apache/opendal/test/MetadataTest.java new file mode 100644 index 000000000000..69dbe6979347 --- /dev/null +++ b/bindings/java/src/test/java/org/apache/opendal/test/MetadataTest.java @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.opendal.test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; +import org.apache.opendal.BlockingOperator; +import org.apache.opendal.Metadata; +import org.apache.opendal.Operator; +import org.apache.opendal.test.behavior.AbstractBehaviorTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class MetadataTest { + @TempDir + private static Path tempDir; + + @Test + public void testAsyncMetadata() { + final Map conf = new HashMap<>(); + conf.put("root", tempDir.toString()); + + try (final Operator op = Operator.of("fs", conf)) { + final String dir = String.format("%s/", UUID.randomUUID().toString()); + op.createDir(dir).join(); + final Metadata dirMetadata = op.stat(dir).join(); + assertTrue(dirMetadata.isDir()); + + final String path = UUID.randomUUID().toString(); + final byte[] content = AbstractBehaviorTest.generateBytes(); + op.write(path, content).join(); + + final Metadata metadata = op.stat(path).join(); + assertTrue(metadata.isFile()); + assertThat(metadata.contentLength).isEqualTo(content.length); + assertThat(metadata.lastModified).isNotNull(); + assertThat(metadata.cacheControl).isNull(); + assertThat(metadata.contentDisposition).isNull(); + assertThat(metadata.contentMd5).isNull(); + assertThat(metadata.contentType).isNull(); + assertThat(metadata.etag).isNull(); + assertThat(metadata.version).isNull(); + + op.delete(dir).join(); + op.delete(path).join(); + } + } + + @Test + public void testBlockingMetadata() { + final Map conf = new HashMap<>(); + conf.put("root", tempDir.toString()); + + try (final BlockingOperator op = BlockingOperator.of("fs", conf)) { + final String dir = String.format("%s/", UUID.randomUUID().toString()); + op.createDir(dir); + final Metadata dirMetadata = op.stat(dir); + assertTrue(dirMetadata.isDir()); + + final String path = UUID.randomUUID().toString(); + final byte[] content = AbstractBehaviorTest.generateBytes(); + op.write(path, content); + + final Metadata metadata = op.stat(path); + assertTrue(metadata.isFile()); + assertThat(metadata.contentLength).isEqualTo(content.length); + assertThat(metadata.lastModified).isNotNull(); + assertThat(metadata.cacheControl).isNull(); + assertThat(metadata.contentDisposition).isNull(); + assertThat(metadata.contentMd5).isNull(); + assertThat(metadata.contentType).isNull(); + assertThat(metadata.etag).isNull(); + assertThat(metadata.version).isNull(); + + op.delete(dir); + op.delete(path); + } + } +} diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AbstractBehaviorTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AbstractBehaviorTest.java index b88290f65ec3..b8bbcdb2ccbe 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AbstractBehaviorTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AbstractBehaviorTest.java @@ -129,10 +129,10 @@ public void testStatFile() { final String path = UUID.randomUUID().toString(); final byte[] content = generateBytes(); operator.write(path, content).join(); - try (final Metadata meta = operator.stat(path).join()) { - assertThat(meta.isFile()).isTrue(); - assertThat(meta.getContentLength()).isEqualTo(content.length); - } + final Metadata meta = operator.stat(path).join(); + assertThat(meta.isFile()).isTrue(); + assertThat(meta.getContentLength()).isEqualTo(content.length); + operator.delete(path).join(); } @@ -144,10 +144,10 @@ public void testWriteFileWithNonAsciiName() { final String path = "βŒπŸ˜±δΈ­ζ–‡.test"; final byte[] content = generateBytes(); operator.write(path, content).join(); - try (final Metadata meta = operator.stat(path).join()) { - assertThat(meta.isFile()).isTrue(); - assertThat(meta.getContentLength()).isEqualTo(content.length); - } + final Metadata meta = operator.stat(path).join(); + assertThat(meta.isFile()).isTrue(); + assertThat(meta.getContentLength()).isEqualTo(content.length); + operator.delete(path).join(); } } @@ -197,9 +197,9 @@ public void testCreateDir() { final String path = String.format("%s/", UUID.randomUUID().toString()); operator.createDir(path).join(); - try (final Metadata meta = operator.stat(path).join()) { - assertThat(meta.isFile()).isFalse(); - } + final Metadata meta = operator.stat(path).join(); + assertThat(meta.isFile()).isFalse(); + operator.delete(path).join(); } @@ -212,9 +212,9 @@ public void testCreateDirExisting() { operator.createDir(path).join(); operator.createDir(path).join(); - try (final Metadata meta = operator.stat(path).join()) { - assertThat(meta.isFile()).isFalse(); - } + final Metadata meta = operator.stat(path).join(); + assertThat(meta.isFile()).isFalse(); + operator.delete(path).join(); } } @@ -567,10 +567,10 @@ public void testBlockingStatFile() { final String path = UUID.randomUUID().toString(); final byte[] content = generateBytes(); blockingOperator.write(path, content); - try (final Metadata meta = blockingOperator.stat(path)) { - assertThat(meta.isFile()).isTrue(); - assertThat(meta.getContentLength()).isEqualTo(content.length); - } + final Metadata meta = blockingOperator.stat(path); + assertThat(meta.isFile()).isTrue(); + assertThat(meta.getContentLength()).isEqualTo(content.length); + blockingOperator.delete(path); } } @@ -592,9 +592,9 @@ public void testBlockingCreateDir() { final String path = String.format("%s/", UUID.randomUUID().toString()); blockingOperator.createDir(path); - try (final Metadata meta = blockingOperator.stat(path)) { - assertThat(meta.isFile()).isFalse(); - } + final Metadata meta = blockingOperator.stat(path); + assertThat(meta.isFile()).isFalse(); + blockingOperator.delete(path); } @@ -607,9 +607,9 @@ public void testBlockingDirExisting() { blockingOperator.createDir(path); blockingOperator.createDir(path); - try (final Metadata meta = blockingOperator.stat(path)) { - assertThat(meta.isFile()).isFalse(); - } + final Metadata meta = blockingOperator.stat(path); + assertThat(meta.isFile()).isFalse(); + blockingOperator.delete(path); } }