From b1417126b507e3fa744fdaee9a6ca2565b24363b Mon Sep 17 00:00:00 2001 From: G-XD Date: Mon, 16 Oct 2023 15:22:47 +0800 Subject: [PATCH 01/20] feat(binding/java): add Entry for list --- .../main/java/org/apache/opendal/Entry.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 bindings/java/src/main/java/org/apache/opendal/Entry.java diff --git a/bindings/java/src/main/java/org/apache/opendal/Entry.java b/bindings/java/src/main/java/org/apache/opendal/Entry.java new file mode 100644 index 00000000000..d8518776cbd --- /dev/null +++ b/bindings/java/src/main/java/org/apache/opendal/Entry.java @@ -0,0 +1,33 @@ +/* + * 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; + +import lombok.Data; + +@Data +public class Entry { + public final String path; + public final Metadata metadata; + + public Entry(String path, Metadata metadata) { + this.path = path; + this.metadata = metadata; + } +} From a9bb79e8a1356a719c02c86a9f9d4b2f29de8cb5 Mon Sep 17 00:00:00 2001 From: G-XD Date: Mon, 16 Oct 2023 15:26:13 +0800 Subject: [PATCH 02/20] feat(binding/java): support list for blocking --- bindings/java/src/blocking_operator.rs | 55 +++++++++++++++++++ bindings/java/src/lib.rs | 12 ++++ .../org/apache/opendal/BlockingOperator.java | 12 ++++ 3 files changed, 79 insertions(+) diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index 6f84a2fea1d..d9c5a56927c 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -20,12 +20,15 @@ use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::sys::jbyteArray; +use jni::sys::jlong; use jni::sys::jobject; use jni::JNIEnv; use opendal::BlockingOperator; +use opendal::Metakey; use crate::jstring_to_string; +use crate::make_entry; use crate::make_metadata; use crate::Result; @@ -208,3 +211,55 @@ fn intern_rename( Ok(op.rename(&source_path, &target_path)?) } + +/// # Safety +/// +/// This function should not be called before the Operator are ready. +#[no_mangle] +pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_listWith( + mut env: JNIEnv, + _: JClass, + op: *mut BlockingOperator, + path: JString, + limit: jlong, + start_after: JString, + delimiter: JString, +) -> jobject { + intern_list_with(&mut env, &mut *op, path, limit, start_after, delimiter).unwrap_or_else(|e| { + e.throw(&mut env); + JObject::default().into_raw() + }) +} + +fn intern_list_with( + env: &mut JNIEnv, + op: &mut BlockingOperator, + path: JString, + limit: jlong, + start_after: JString, + delimiter: JString, +) -> Result { + let path = jstring_to_string(env, &path)?; + let mut op = op.list_with(&path).metakey(Metakey::Complete); + if limit > 0 { + op = op.limit(limit as usize); + } + if !start_after.is_null() { + op = op.start_after(jstring_to_string(env, &start_after)?.as_str()); + } + if !delimiter.is_null() { + op = op.delimiter(jstring_to_string(env, &delimiter)?.as_str()); + } + + let list = env.new_object("java/util/ArrayList", "()V", &[])?; + let jmap = env.get_list(&list)?; + + let obs = op.call()?; + + for entry in obs { + let entry = make_entry(env, entry)?; + jmap.add(env, &entry)?; + } + + Ok(list.into_raw()) +} diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index 97c6d0c5338..fe376ea0afc 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -32,6 +32,7 @@ use jni::JavaVM; use once_cell::sync::OnceCell; use opendal::raw::PresignedRequest; use opendal::Capability; +use opendal::Entry; use opendal::EntryMode; use opendal::Metadata; use opendal::OperatorInfo; @@ -269,6 +270,17 @@ fn make_metadata<'a>(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result(env: &mut JNIEnv<'a>, entry: Entry) -> Result> { + let path = env.new_string(entry.path())?; + let metadata = make_metadata(env, entry.metadata().to_owned())?; + + Ok(env.new_object( + "org/apache/opendal/Entry", + "(Ljava/lang/String;Lorg/apache/opendal/Metadata;)V", + &[JValue::Object(&path), JValue::Object(&metadata)], + )?) +} + fn string_to_jstring<'a>(env: &mut JNIEnv<'a>, s: Option<&str>) -> Result> { s.map_or_else( || Ok(JObject::null()), 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 96f4446412c..aca52415b22 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -20,6 +20,7 @@ package org.apache.opendal; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.Map; /** @@ -82,6 +83,14 @@ public void rename(String sourcePath, String targetPath) { rename(nativeHandle, sourcePath, targetPath); } + public List list(String path) { + return listWith(path, -1, null, null); + } + + public List listWith(String path, long limit, String startAfter, String delimiter) { + return listWith(nativeHandle, path, limit, startAfter, delimiter); + } + @Override protected native void disposeInternal(long handle); @@ -98,4 +107,7 @@ public void rename(String sourcePath, String targetPath) { private static native long copy(long nativeHandle, String sourcePath, String targetPath); private static native long rename(long nativeHandle, String sourcePath, String targetPath); + + private static native List listWith( + long nativeHandle, String path, long limit, String startAfter, String delimiter); } From d3036ae3ef5f97e9729b262a5c2050461a1a1f8a Mon Sep 17 00:00:00 2001 From: G-XD Date: Tue, 17 Oct 2023 00:28:08 +0800 Subject: [PATCH 03/20] feat(binding/java): support list & remove_all --- bindings/java/src/blocking_operator.rs | 28 +++- .../org/apache/opendal/BlockingOperator.java | 6 + .../java/org/apache/opendal/Operator.java | 20 +++ bindings/java/src/operator.rs | 124 ++++++++++++++++++ 4 files changed, 174 insertions(+), 4 deletions(-) diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index d9c5a56927c..8bef859f116 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -212,6 +212,27 @@ fn intern_rename( Ok(op.rename(&source_path, &target_path)?) } +/// # Safety +/// +/// This function should not be called before the Operator are ready. +#[no_mangle] +pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_removeAll( + mut env: JNIEnv, + _: JClass, + op: *mut BlockingOperator, + path: JString, +) { + intern_remove_all(&mut env, &mut *op, path).unwrap_or_else(|e| { + e.throw(&mut env); + }) +} + +fn intern_remove_all(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result<()> { + let path = jstring_to_string(env, &path)?; + + Ok(op.remove_all(&path)?) +} + /// # Safety /// /// This function should not be called before the Operator are ready. @@ -241,7 +262,7 @@ fn intern_list_with( ) -> Result { let path = jstring_to_string(env, &path)?; let mut op = op.list_with(&path).metakey(Metakey::Complete); - if limit > 0 { + if limit >= 0 { op = op.limit(limit as usize); } if !start_after.is_null() { @@ -252,13 +273,12 @@ fn intern_list_with( } let list = env.new_object("java/util/ArrayList", "()V", &[])?; - let jmap = env.get_list(&list)?; + let jlist = env.get_list(&list)?; let obs = op.call()?; - for entry in obs { let entry = make_entry(env, entry)?; - jmap.add(env, &entry)?; + jlist.add(env, &entry)?; } Ok(list.into_raw()) 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 aca52415b22..5eb207ce3ad 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -83,6 +83,10 @@ public void rename(String sourcePath, String targetPath) { rename(nativeHandle, sourcePath, targetPath); } + public void removeAll(String path) { + removeAll(nativeHandle, path); + } + public List list(String path) { return listWith(path, -1, null, null); } @@ -108,6 +112,8 @@ public List listWith(String path, long limit, String startAfter, String d private static native long rename(long nativeHandle, String sourcePath, String targetPath); + private static native void removeAll(long nativeHandle, String path); + private static native List listWith( long nativeHandle, String path, long limit, String startAfter, String delimiter); } 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 54273ee1adf..d5317d8a9b2 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -191,6 +192,20 @@ public CompletableFuture rename(String sourcePath, String targetPath) { return AsyncRegistry.take(requestId); } + public CompletableFuture removeAll(String path) { + final long requestId = removeAll(nativeHandle, path); + return AsyncRegistry.take(requestId); + } + + public CompletableFuture> list(String path) { + return listWith(path, -1, null, null); + } + + public CompletableFuture> listWith(String path, long limit, String startAfter, String delimiter) { + final long requestId = listWith(nativeHandle, path, limit, startAfter, delimiter); + return AsyncRegistry.take(requestId); + } + @Override protected native void disposeInternal(long handle); @@ -221,4 +236,9 @@ public CompletableFuture rename(String sourcePath, String targetPath) { private static native long copy(long nativeHandle, String sourcePath, String targetPath); private static native long rename(long nativeHandle, String sourcePath, String targetPath); + + private static native long removeAll(long nativeHandle, String path); + + private static native long listWith( + long nativeHandle, String path, long limit, String startAfter, String delimiter); } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index ccc06edf466..1ac230f27bf 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -17,6 +17,7 @@ use std::str::FromStr; use std::time::Duration; +use std::usize; use jni::objects::JByteArray; use jni::objects::JClass; @@ -28,6 +29,7 @@ use jni::sys::{jlong, jobject}; use jni::JNIEnv; use opendal::layers::BlockingLayer; use opendal::raw::PresignedRequest; +use opendal::Metakey; use opendal::Operator; use opendal::Scheme; @@ -35,6 +37,7 @@ use crate::get_current_env; use crate::get_global_runtime; use crate::jmap_to_hashmap; use crate::jstring_to_string; +use crate::make_entry; use crate::make_metadata; use crate::make_operator_info; use crate::make_presigned_request; @@ -415,6 +418,127 @@ async fn do_rename(op: &mut Operator, source_path: String, target_path: String) Ok(op.rename(&source_path, &target_path).await?) } +/// # Safety +/// +/// This function should not be called before the Operator are ready. +#[no_mangle] +pub unsafe extern "system" fn Java_org_apache_opendal_Operator_removeAll( + mut env: JNIEnv, + _: JClass, + op: *mut Operator, + path: JString, +) -> jlong { + intern_remove_all(&mut env, op, path).unwrap_or_else(|e| { + e.throw(&mut env); + 0 + }) +} + +fn intern_remove_all(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result { + let op = unsafe { &mut *op }; + let id = request_id(env)?; + + let path = jstring_to_string(env, &path)?; + + unsafe { get_global_runtime() }.spawn(async move { + let result = do_remove_all(op, path).await; + complete_future(id, result.map(|_| JValueOwned::Void)) + }); + + Ok(id) +} + +async fn do_remove_all(op: &mut Operator, path: String) -> Result<()> { + Ok(op.remove_all(&path).await?) +} + +/// # Safety +/// +/// This function should not be called before the Operator are ready. +#[no_mangle] +pub unsafe extern "system" fn Java_org_apache_opendal_Operator_listWith( + mut env: JNIEnv, + _: JClass, + op: *mut Operator, + path: JString, + limit: jlong, + start_after: JString, + delimiter: JString, +) -> jlong { + intern_list_with(&mut env, op, path, limit, start_after, delimiter).unwrap_or_else(|e| { + e.throw(&mut env); + 0 + }) +} + +fn intern_list_with( + env: &mut JNIEnv, + op: *mut Operator, + path: JString, + limit: jlong, + start_after: JString, + delimiter: JString, +) -> Result { + let op = unsafe { &mut *op }; + let id = request_id(env)?; + + let path = jstring_to_string(env, &path)?; + let limit = if limit < 0 { + None + } else { + Some(limit as usize) + }; + let start_after = if start_after.is_null() { + None + } else { + Some(jstring_to_string(env, &start_after)?) + }; + let delimiter = if delimiter.is_null() { + None + } else { + Some(jstring_to_string(env, &delimiter)?) + }; + + unsafe { get_global_runtime() }.spawn(async move { + let result = do_list_with(op, path, limit, start_after, delimiter).await; + complete_future(id, result.map(JValueOwned::Object)) + }); + + Ok(id) +} + +async fn do_list_with<'local>( + op: &mut Operator, + path: String, + limit: Option, + start_after: Option, + delimiter: Option, +) -> Result> { + let mut op = op.list_with(&path).metakey(Metakey::Complete); + if let Some(limit) = limit { + op = op.limit(limit); + } + if let Some(start_after) = start_after { + op = op.start_after(start_after.as_str()); + } + if let Some(delimiter) = delimiter { + op = op.delimiter(delimiter.as_str()); + } + let obs = op.await?; + + let mut env = unsafe { get_current_env() }; + + let list = env.new_object("java/util/ArrayList", "()V", &[])?; + let jlist = env.get_list(&list)?; + + for entry in obs { + let entry = make_entry(&mut env, entry)?; + jlist.add(&mut env, &entry)?; + } + + Ok(list) +} + /// # Safety /// /// This function should not be called before the Operator are ready. From fb960f54548f5a11f63745cfa19009430ffd248f Mon Sep 17 00:00:00 2001 From: G-XD Date: Tue, 17 Oct 2023 00:29:29 +0800 Subject: [PATCH 04/20] test(binding/java): add list test --- .../test/behavior/AbstractBehaviorTest.java | 437 +++++++++++++++++- 1 file changed, 426 insertions(+), 11 deletions(-) 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 b8bbcdb2ccb..3d7f8df3514 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 @@ -22,16 +22,23 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import io.github.cdimascio.dotenv.Dotenv; import io.github.cdimascio.dotenv.DotenvEntry; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Random; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import org.apache.opendal.BlockingOperator; import org.apache.opendal.Capability; +import org.apache.opendal.Entry; import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; import org.apache.opendal.Operator; @@ -527,6 +534,282 @@ public void testRenameOverwrite() { } } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + @Nested + class AsyncListTest { + @BeforeAll + public void precondition() { + final Capability capability = operator.info.fullCapability; + assumeTrue(capability.read && capability.write && capability.list); + } + + /** + * List dir should return newly created file. + */ + @Test + public void testListDir() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + operator.write(path, content).join(); + + final List entries = operator.list(parent + "/").join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + + found = true; + } + } + assertTrue(found); + operator.delete(path).join(); + } + + /** + * List dir with metakey + */ + @Test + public void testListDirWithMetakey() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + operator.write(path, content).join(); + + final List entries = operator.list(parent + "/").join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + assertTrue(found); + operator.delete(path).join(); + } + + /** + * listing a directory, which contains more objects than a single page can take. + */ + @Test + public void testListRichDir() { + final String parent = "test_list_rich_dir"; + operator.createDir(parent + "/").join(); + final List expected = new ArrayList<>(); + for (int i = 0; i < 100; i++) { + expected.add(String.format("%s/file-%d", parent, i)); + } + + for (String path : expected) { + operator.write(path, parent).join(); + } + + final List entries = operator.list(parent + "/").join(); + final List actual = + entries.stream().map(Entry::getPath).sorted().collect(Collectors.toList()); + + Collections.sort(expected); + assertThat(actual).isEqualTo(expected); + operator.removeAll(parent + "/").join(); + } + + /** + * List empty dir should return nothing. + */ + @Test + public void testListEmptyDir() { + final String dir = String.format("%s/", UUID.randomUUID().toString()); + operator.createDir(dir).join(); + + final List entries = operator.list(dir).join(); + assertThat(entries).isEmpty(); + + operator.delete(dir).join(); + } + + /** + * List non exist dir should return nothing. + */ + @Test + public void testListNotExistDir() { + final String dir = String.format("%s/", UUID.randomUUID().toString()); + + final List entries = operator.list(dir).join(); + assertThat(entries).isEmpty(); + } + + /** + * List dir should return correct sub dir. + */ + @Test + public void testListSubDir() { + final String path = String.format("%s/", UUID.randomUUID().toString()); + operator.createDir(path).join(); + + final List entries = operator.list("/").join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isDir()); + found = true; + } + } + assertTrue(found); + + operator.delete(path).join(); + } + + /** + * List dir should also to list nested dir. + */ + @Test + public void testListNestedDir() { + final String dir = String.format( + "%s/%s/", UUID.randomUUID().toString(), UUID.randomUUID().toString()); + final String fileName = UUID.randomUUID().toString(); + final String filePath = String.format("%s/%s", dir, fileName); + final String dirName = String.format("%s/", UUID.randomUUID().toString()); + final String dirPath = String.format("%s/%s", dir, dirName); + final String content = "test_list_nested_dir"; + + operator.createDir(dir).join(); + operator.write(filePath, content).join(); + operator.createDir(dirPath).join(); + + final List entries = operator.list(dir).join(); + assertThat(entries).hasSize(2); + + for (Entry entry : entries) { + // check file + if (entry.getPath().equals(filePath)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length()); + // check dir + } else if (entry.getPath().equals(dirPath)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isDir()); + } + } + + operator.removeAll(dir).join(); + } + + /** + * List with start after should start listing after the specified key + */ + @Test + public void testListWithStartAfter() { + if (!operator.info.fullCapability.listWithStartAfter) { + return; + } + final String dir = String.format("%s/", UUID.randomUUID().toString()); + operator.createDir(dir).join(); + + final String[] given = new String[] { + String.format("%s%s-0", dir, UUID.randomUUID().toString()), + String.format("%s%s-1", dir, UUID.randomUUID().toString()), + String.format("%s%s-2", dir, UUID.randomUUID().toString()), + String.format("%s%s-3", dir, UUID.randomUUID().toString()), + String.format("%s%s-4", dir, UUID.randomUUID().toString()), + String.format("%s%s-5", dir, UUID.randomUUID().toString()), + }; + + for (String path : given) { + operator.write(path, "content").join(); + } + + final List entries = + operator.listWith(dir, -1, given[2], null).join(); + final List expected = entries.stream().map(Entry::getPath).collect(Collectors.toList()); + + assertThat(expected).isEqualTo(Arrays.asList(given[3], given[4], given[5])); + + operator.removeAll(dir).join(); + } + + @Test + public void testScanRoot() { + final List entries = blockingOperator.listWith("", -1, null, ""); + final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertTrue(!actual.contains("/"), "empty root shouldn't return itself"); + assertTrue(!actual.contains(""), "empty root shouldn't return empty"); + } + + /** + * Walk top down should output as expected + */ + @Test + public void testScan() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + operator.createDir(String.format("%s/%s", parent, path)).join(); + } else { + operator.write(String.format("%s/%s", parent, path), "test_scan") + .join(); + } + } + final List entries = operator.listWith(String.format("%s/x/", parent), -1, null, "") + .join(); + final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertThat(actual).contains(parent + "/x/y", parent + "/x/x/y", parent + "/x/x/x/y"); + + operator.removeAll(parent + "/").join(); + } + + /** + * Remove all should remove all in this path. + */ + @Test + public void testRemoveAll() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + operator.createDir(String.format("%s/%s", parent, path)); + } else { + operator.write(String.format("%s/%s", parent, path), "test_scan"); + } + } + + operator.removeAll(parent + "/x/").join(); + + for (String path : expected) { + if (path.endsWith("/")) { + continue; + } + assertThatThrownBy(() -> operator.stat(String.format("%s/%s", parent, path)) + .join()) + .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + } + + operator.removeAll(parent + "/").join(); + } + } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Nested class BlockingWriteTest { @@ -666,7 +949,7 @@ public void testBlockingCopySourceDir() { blockingOperator.createDir(sourcePath); assertThatThrownBy(() -> blockingOperator.copy(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOperator.delete(sourcePath); } @@ -685,8 +968,8 @@ public void testBlockingCopyTargetDir() { blockingOperator.createDir(targetPath); - assertThatThrownBy(() -> operator.copy(sourcePath, targetPath).join()) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + assertThatThrownBy(() -> blockingOperator.copy(sourcePath, targetPath)) + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOperator.delete(sourcePath); blockingOperator.delete(targetPath); @@ -703,7 +986,7 @@ public void testBlockingCopySelf() { blockingOperator.write(sourcePath, sourceContent); assertThatThrownBy(() -> blockingOperator.copy(sourcePath, sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsSameFile)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsSameFile)); blockingOperator.delete(sourcePath); } @@ -781,7 +1064,7 @@ public void testBlockingRenameFile() { blockingOperator.rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOperator.stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOperator.stat(targetPath).getContentLength()).isEqualTo(sourceContent.length); @@ -798,7 +1081,7 @@ public void testBlockingRenameNonExistingSource() { final String targetPath = UUID.randomUUID().toString(); assertThatThrownBy(() -> blockingOperator.rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); } /** @@ -812,7 +1095,7 @@ public void testBlockingRenameSourceDir() { blockingOperator.createDir(sourcePath); assertThatThrownBy(() -> blockingOperator.rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); } /** @@ -830,7 +1113,7 @@ public void testBlockingRenameTargetDir() { blockingOperator.createDir(targetPath); assertThatThrownBy(() -> blockingOperator.rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOperator.delete(sourcePath); blockingOperator.delete(targetPath); @@ -847,7 +1130,7 @@ public void testBlockingRenameSelf() { blockingOperator.write(sourcePath, sourceContent); assertThatThrownBy(() -> blockingOperator.rename(sourcePath, sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsSameFile)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsSameFile)); blockingOperator.delete(sourcePath); } @@ -871,7 +1154,7 @@ public void testBlockingRenameNested() { blockingOperator.rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOperator.stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOperator.read(targetPath)).isEqualTo(sourceContent); @@ -899,7 +1182,7 @@ public void testBlockingRenameOverwrite() { blockingOperator.rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOperator.stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOperator.read(targetPath)).isEqualTo(sourceContent); @@ -908,6 +1191,138 @@ public void testBlockingRenameOverwrite() { } } + @TestInstance(TestInstance.Lifecycle.PER_CLASS) + @Nested + class BlockingListTest { + @BeforeAll + public void precondition() { + final Capability capability = blockingOperator.info.fullCapability; + assumeTrue( + capability.read && capability.write && capability.copy && capability.blocking && capability.list); + } + + @Test + public void testBlockingListDir() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + blockingOperator.write(path, content); + + final List list = blockingOperator.list(parent + "/"); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + found = true; + + assertThat(metadata.getContentLength()).isEqualTo(content.length); + assertTrue(metadata.isFile()); + } + } + assertTrue(found); + + blockingOperator.delete(path); + } + + @Test + public void testBlockingListDirWithMetakey() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + blockingOperator.write(path, content); + + final List list = blockingOperator.list(parent + "/"); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + assertTrue(metadata.isFile()); + + // We don't care about the value, we just to check there is no panic. + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + + assertTrue(found); + blockingOperator.delete(path); + } + + @Test + public void testBlockingListNonExistDir() { + final String dir = String.format("%s/", UUID.randomUUID().toString()); + + final List list = blockingOperator.list(dir); + assertTrue(list.isEmpty()); + } + + @Test + public void testBlockingScan() { + final String parent = UUID.randomUUID().toString(); + + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + + for (String path : expected) { + final byte[] content = generateBytes(); + if (path.endsWith("/")) { + blockingOperator.createDir(String.format("%s/%s", parent, path)); + } else { + blockingOperator.write(String.format("%s/%s", parent, path), content); + } + } + + final List list = blockingOperator.listWith(String.format("%s/x/", parent), -1, null, ""); + final Set paths = list.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertTrue(paths.contains(parent + "/x/y")); + assertTrue(paths.contains(parent + "/x/x/y")); + assertTrue(paths.contains(parent + "/x/x/x/y")); + + blockingOperator.removeAll(parent + "/"); + } + + /** + * Remove all should remove all in this path. + */ + @Test + public void testBlockingRemoveAll() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + blockingOperator.createDir(String.format("%s/%s", parent, path)); + } else { + blockingOperator.write(String.format("%s/%s", parent, path), "test_scan"); + } + } + + blockingOperator.removeAll(parent + "/x/"); + + for (String path : expected) { + if (path.endsWith("/")) { + continue; + } + assertThatThrownBy(() -> blockingOperator.stat(String.format("%s/%s", parent, path))) + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); + } + + blockingOperator.removeAll(parent + "/"); + } + } + /** * Generates a byte array of random content. */ From 7662b9c41a9fa0031a2e6f61c55db52602ea8af3 Mon Sep 17 00:00:00 2001 From: G-XD Date: Tue, 17 Oct 2023 22:30:21 +0800 Subject: [PATCH 05/20] feat(binding/java): add OpList args --- .../org/apache/opendal/BlockingOperator.java | 17 +++- .../java/org/apache/opendal/Operator.java | 17 +++- .../java/org/apache/opendal/args/OpList.java | 77 +++++++++++++++++++ .../test/behavior/AbstractBehaviorTest.java | 16 +++- 4 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 bindings/java/src/main/java/org/apache/opendal/args/OpList.java 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 5eb207ce3ad..e3dcdd8da85 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -22,6 +22,8 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import org.apache.opendal.args.OpList; +import org.apache.opendal.args.OpList.OpListBuilder; /** * BlockingOperator represents an underneath OpenDAL operator that @@ -88,11 +90,20 @@ public void removeAll(String path) { } public List list(String path) { - return listWith(path, -1, null, null); + return listWith(path).build().call(); } - public List listWith(String path, long limit, String startAfter, String delimiter) { - return listWith(nativeHandle, path, limit, startAfter, delimiter); + public OpListBuilder> listWith(String path) { + return OpList.builder(path, this::internListWith); + } + + private List internListWith(OpList> opList) { + return listWith( + nativeHandle, + opList.getPath(), + opList.getLimit(), + opList.getStartAfter().orElse(null), + opList.getDelimiter().orElse(null)); } @Override 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 d5317d8a9b2..21d933d88cb 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -26,6 +26,8 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import org.apache.opendal.args.OpList; +import org.apache.opendal.args.OpList.OpListBuilder; /** * Operator represents an underneath OpenDAL operator that @@ -198,11 +200,20 @@ public CompletableFuture removeAll(String path) { } public CompletableFuture> list(String path) { - return listWith(path, -1, null, null); + return listWith(path).build().call(); } - public CompletableFuture> listWith(String path, long limit, String startAfter, String delimiter) { - final long requestId = listWith(nativeHandle, path, limit, startAfter, delimiter); + public OpListBuilder>> listWith(String path) { + return OpList.builder(path, this::internListWith); + } + + private CompletableFuture> internListWith(OpList>> opList) { + final long requestId = listWith( + nativeHandle, + opList.getPath(), + opList.getLimit(), + opList.getStartAfter().orElse(null), + opList.getDelimiter().orElse(null)); return AsyncRegistry.take(requestId); } diff --git a/bindings/java/src/main/java/org/apache/opendal/args/OpList.java b/bindings/java/src/main/java/org/apache/opendal/args/OpList.java new file mode 100644 index 00000000000..3bcbc96ba24 --- /dev/null +++ b/bindings/java/src/main/java/org/apache/opendal/args/OpList.java @@ -0,0 +1,77 @@ +package org.apache.opendal.args; + +import java.util.Optional; +import java.util.function.Function; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NonNull; +import lombok.ToString; + +@Getter +@ToString +@EqualsAndHashCode +public class OpList { + private final String path; + private final long limit; + private final Optional startAfter; + private final Optional delimiter; + private final Function, T> func; + + public T call() { + return func.apply(this); + } + + private OpList( + @NonNull String path, + long limit, + Optional startAfter, + Optional delimiter, + @NonNull Function, T> func) { + this.path = path; + this.limit = limit; + this.startAfter = startAfter; + this.delimiter = delimiter; + this.func = func; + } + + public static OpListBuilder builder(String path, Function, T> func) { + return new OpListBuilder<>(path, func); + } + + @ToString + public static class OpListBuilder { + private final String path; + private final Function, T> func; + + private long limit; + private Optional startAfter; + private Optional delimiter; + + private OpListBuilder(String path, @NonNull Function, T> func) { + this.limit = -1L; + this.startAfter = Optional.empty(); + this.delimiter = Optional.empty(); + this.path = path; + this.func = func; + } + + public OpListBuilder limit(long limit) { + this.limit = limit; + return this; + } + + public OpListBuilder startAfter(String startAfter) { + this.startAfter = Optional.ofNullable(startAfter); + return this; + } + + public OpListBuilder delimiter(String delimiter) { + this.delimiter = Optional.ofNullable(delimiter); + return this; + } + + public OpList build() { + return new OpList<>(path, limit, startAfter, delimiter, func); + } + } +} 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 3d7f8df3514..678f723ae01 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 @@ -735,7 +735,7 @@ public void testListWithStartAfter() { } final List entries = - operator.listWith(dir, -1, given[2], null).join(); + operator.listWith(dir).startAfter(given[2]).build().call().join(); final List expected = entries.stream().map(Entry::getPath).collect(Collectors.toList()); assertThat(expected).isEqualTo(Arrays.asList(given[3], given[4], given[5])); @@ -745,7 +745,8 @@ public void testListWithStartAfter() { @Test public void testScanRoot() { - final List entries = blockingOperator.listWith("", -1, null, ""); + final List entries = + operator.listWith("").delimiter("").build().call().join(); final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); assertTrue(!actual.contains("/"), "empty root shouldn't return itself"); @@ -769,7 +770,10 @@ public void testScan() { .join(); } } - final List entries = operator.listWith(String.format("%s/x/", parent), -1, null, "") + final List entries = operator.listWith(String.format("%s/x/", parent)) + .delimiter("") + .build() + .call() .join(); final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); @@ -1282,7 +1286,11 @@ public void testBlockingScan() { } } - final List list = blockingOperator.listWith(String.format("%s/x/", parent), -1, null, ""); + final List list = blockingOperator + .listWith(String.format("%s/x/", parent)) + .delimiter("") + .build() + .call(); final Set paths = list.stream().map(Entry::getPath).collect(Collectors.toSet()); assertTrue(paths.contains(parent + "/x/y")); From deae37d6f1fcdeba3095609592fd67e36f4ab931 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 01:38:26 +0800 Subject: [PATCH 06/20] feat(binding/java): support metakey args --- Cargo.lock | 1 + bindings/java/Cargo.toml | 1 + bindings/java/src/blocking_operator.rs | 26 ++- bindings/java/src/lib.rs | 124 ++++++++++--- .../org/apache/opendal/BlockingOperator.java | 7 +- .../java/org/apache/opendal/Operator.java | 7 +- .../java/org/apache/opendal/args/OpList.java | 169 ++++++++++++++++-- bindings/java/src/operator.rs | 31 +++- .../test/behavior/AbstractBehaviorTest.java | 132 ++++++++++++-- 9 files changed, 434 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c0f7a71b28d..af14ab61f18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4154,6 +4154,7 @@ name = "opendal-java" version = "0.41.0" dependencies = [ "anyhow", + "flagset", "jni", "num_cpus", "once_cell", diff --git a/bindings/java/Cargo.toml b/bindings/java/Cargo.toml index 09364f2683f..cf0d7a4fcc1 100644 --- a/bindings/java/Cargo.toml +++ b/bindings/java/Cargo.toml @@ -129,6 +129,7 @@ anyhow = "1.0.71" jni = "0.21.1" num_cpus = "1.15.0" once_cell = "1.17.1" +flagset = "0.4" tokio = { version = "1.28.1", features = ["full"] } opendal = { workspace = true } diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index 8bef859f116..7973fc0a99c 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -20,6 +20,7 @@ use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::sys::jbyteArray; +use jni::sys::jintArray; use jni::sys::jlong; use jni::sys::jobject; use jni::JNIEnv; @@ -30,6 +31,7 @@ use opendal::Metakey; use crate::jstring_to_string; use crate::make_entry; use crate::make_metadata; +use crate::metakey_to_flagset; use crate::Result; /// # Safety @@ -113,7 +115,7 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_stat( 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(make_metadata(env, metadata)?.into_raw()) + Ok(make_metadata(env, metadata, Metakey::Complete.into())?.into_raw()) } /// # Safety @@ -245,8 +247,18 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_listWith( limit: jlong, start_after: JString, delimiter: JString, + metakeys: jintArray, ) -> jobject { - intern_list_with(&mut env, &mut *op, path, limit, start_after, delimiter).unwrap_or_else(|e| { + intern_list_with( + &mut env, + &mut *op, + path, + limit, + start_after, + delimiter, + metakeys, + ) + .unwrap_or_else(|e| { e.throw(&mut env); JObject::default().into_raw() }) @@ -259,9 +271,10 @@ fn intern_list_with( limit: jlong, start_after: JString, delimiter: JString, + metakeys: jintArray, ) -> Result { let path = jstring_to_string(env, &path)?; - let mut op = op.list_with(&path).metakey(Metakey::Complete); + let mut op = op.list_with(&path); if limit >= 0 { op = op.limit(limit as usize); } @@ -272,12 +285,17 @@ fn intern_list_with( op = op.delimiter(jstring_to_string(env, &delimiter)?.as_str()); } + let metakey = metakey_to_flagset(env, metakeys)?; + if let Some(metakey) = metakey { + op = op.metakey(metakey); + } + let list = env.new_object("java/util/ArrayList", "()V", &[])?; let jlist = env.get_list(&list)?; let obs = op.call()?; for entry in obs { - let entry = make_entry(env, entry)?; + let entry = make_entry(env, entry, metakey.unwrap_or(Metakey::Mode.into()))?; jlist.add(env, &entry)?; } diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index fe376ea0afc..d44baa5b497 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -20,11 +20,14 @@ use std::collections::HashMap; use std::ffi::c_void; use crate::error::Error; +use flagset::FlagSet; use jni::objects::JObject; +use jni::objects::JPrimitiveArray; use jni::objects::JString; use jni::objects::{JMap, JValue}; use jni::sys::jboolean; use jni::sys::jint; +use jni::sys::jintArray; use jni::sys::jlong; use jni::sys::JNI_VERSION_1_8; use jni::JNIEnv; @@ -35,6 +38,7 @@ use opendal::Capability; use opendal::Entry; use opendal::EntryMode; use opendal::Metadata; +use opendal::Metakey; use opendal::OperatorInfo; use tokio::runtime::Builder; use tokio::runtime::Runtime; @@ -226,30 +230,73 @@ fn make_capability<'a>(env: &mut JNIEnv<'a>, cap: Capability) -> Result(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result> { +fn make_metadata<'a>( + env: &mut JNIEnv<'a>, + metadata: Metadata, + metakey: FlagSet, +) -> 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 last_modified = + if metakey.contains(Metakey::LastModified) || metakey.contains(Metakey::Complete) { + 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())], + )?) + }, + )? + } else { + JObject::null() + }; - 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 cache_control = + if metakey.contains(Metakey::CacheControl) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.cache_control())? + } else { + JObject::null() + }; + let content_disposition = + if metakey.contains(Metakey::ContentDisposition) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.content_disposition())? + } else { + JObject::null() + }; + let content_md5 = + if metakey.contains(Metakey::ContentMd5) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.content_md5())? + } else { + JObject::null() + }; + let content_type = + if metakey.contains(Metakey::ContentType) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.content_type())? + } else { + JObject::null() + }; + let etag = if metakey.contains(Metakey::Etag) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.etag())? + } else { + JObject::null() + }; + let version = if metakey.contains(Metakey::Version) || metakey.contains(Metakey::Complete) { + string_to_jstring(env, metadata.version())? + } else { + JObject::null() + }; + let content_length = + if metakey.contains(Metakey::ContentLength) || metakey.contains(Metakey::Complete) { + metadata.content_length() as jlong + } else { + -1 + }; let result = env .new_object( @@ -257,7 +304,7 @@ fn make_metadata<'a>(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result(env: &mut JNIEnv<'a>, entry: Entry) -> Result> { +fn make_entry<'a>( + env: &mut JNIEnv<'a>, + entry: Entry, + metakey: FlagSet, +) -> Result> { let path = env.new_string(entry.path())?; - let metadata = make_metadata(env, entry.metadata().to_owned())?; + let metadata = make_metadata(env, entry.metadata().to_owned(), metakey)?; Ok(env.new_object( "org/apache/opendal/Entry", @@ -296,3 +347,36 @@ fn jstring_to_string(env: &mut JNIEnv, s: &JString) -> Result { let res = unsafe { env.get_string_unchecked(s)? }; Ok(res.into()) } + +fn metakey_to_flagset(env: &mut JNIEnv, metakeys: jintArray) -> Result>> { + let metakeys = unsafe { JPrimitiveArray::from_raw(metakeys) }; + let len = env.get_array_length(&metakeys)?; + let mut buf: Vec = vec![0; len as usize]; + env.get_int_array_region(metakeys, 0, &mut buf)?; + + let mut metakey: Option> = None; + for key in buf { + let m = match key { + 0 => Metakey::Complete, + 1 => Metakey::Mode, + 2 => Metakey::CacheControl, + 3 => Metakey::ContentDisposition, + 4 => Metakey::ContentLength, + 5 => Metakey::ContentMd5, + 6 => Metakey::ContentRange, + 7 => Metakey::ContentType, + 8 => Metakey::Etag, + 9 => Metakey::LastModified, + 10 => Metakey::Version, + _ => Err(opendal::Error::new( + opendal::ErrorKind::Unexpected, + "Invalid metakey", + ))?, + }; + metakey = match metakey { + None => Some(m.into()), + Some(metakey) => Some(metakey | m), + } + } + Ok(metakey) +} 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 e3dcdd8da85..049000a6045 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -22,7 +22,9 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.apache.opendal.args.OpList; +import org.apache.opendal.args.OpList.Metakey; import org.apache.opendal.args.OpList.OpListBuilder; /** @@ -103,7 +105,8 @@ private List internListWith(OpList> opList) { opList.getPath(), opList.getLimit(), opList.getStartAfter().orElse(null), - opList.getDelimiter().orElse(null)); + opList.getDelimiter().orElse(null), + Stream.of(opList.getMetakeys()).mapToInt(Metakey::getId).toArray()); } @Override @@ -126,5 +129,5 @@ private List internListWith(OpList> opList) { private static native void removeAll(long nativeHandle, String path); private static native List listWith( - long nativeHandle, String path, long limit, String startAfter, String delimiter); + long nativeHandle, String path, long limit, String startAfter, String delimiter, int... metakeys); } 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 21d933d88cb..e0a8e6f39da 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -26,7 +26,9 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Stream; import org.apache.opendal.args.OpList; +import org.apache.opendal.args.OpList.Metakey; import org.apache.opendal.args.OpList.OpListBuilder; /** @@ -213,7 +215,8 @@ private CompletableFuture> internListWith(OpList> internListWith(OpList { private final String path; + /** + * The limit passed to underlying service to specify the max results that could return. + */ private final long limit; + /** + * The start_after passes to underlying service to specify the specified key to start listing from. + */ private final Optional startAfter; + /** + * The delimiter used to for the list operation. Default to be `/` + */ private final Optional delimiter; - private final Function, T> func; + /** + * The metakey of the metadata that be queried. Default to be {@link Metakey#Mode}. + */ + private final Metakey[] metakeys; + /** + * The list function. + */ + private final Function, T> listFunc; + /** + * Calls the list function and returns the result. + * + * @return the result of calling the list + */ public T call() { - return func.apply(this); + return listFunc.apply(this); } private OpList( @NonNull String path, + @NonNull Function, T> listFunc, long limit, Optional startAfter, Optional delimiter, - @NonNull Function, T> func) { + Metakey... metakeys) { this.path = path; this.limit = limit; this.startAfter = startAfter; this.delimiter = delimiter; - this.func = func; + this.listFunc = listFunc; + this.metakeys = metakeys; } - public static OpListBuilder builder(String path, Function, T> func) { - return new OpListBuilder<>(path, func); + public static OpListBuilder builder(String path, Function, T> listFunc) { + return new OpListBuilder<>(path, listFunc); + } + + /** + * Metakey describes the metadata keys that can be queried. + * + * For query: + * + * At user side, we will allow user to query the metadata. If + * the meta has been stored, we will return directly. If no, we will + * call `stat` internally to fetch the metadata. + */ + public enum Metakey { + /** + * The special metadata key that used to mark this entry + * already contains all metadata. + */ + Complete(0), + /** + * + * Key for mode. + */ + Mode(1), + /** + * Key for cache control. + */ + CacheControl(2), + /** + * Key for content disposition. + */ + ContentDisposition(3), + /** + * Key for content length. + */ + ContentLength(4), + /** + * Key for content md5. + */ + ContentMd5(5), + /** + * Key for content range. + */ + ContentRange(6), + /** + * Key for content type. + */ + ContentType(7), + /** + * Key for etag. + */ + Etag(8), + /** + * Key for last last modified. + */ + LastModified(9), + /** + * Key for version. + */ + Version(10), + ; + + private final int id; + + Metakey(int id) { + this.id = id; + } + + public int getId() { + return id; + } } @ToString public static class OpListBuilder { private final String path; - private final Function, T> func; - + private final Function, T> listFunc; + private long limit; private Optional startAfter; private Optional delimiter; + private Metakey[] metakeys; - private OpListBuilder(String path, @NonNull Function, T> func) { + private OpListBuilder(String path, @NonNull Function, T> listFunc) { this.limit = -1L; this.startAfter = Optional.empty(); this.delimiter = Optional.empty(); + this.metakeys = new Metakey[] {Metakey.Mode}; this.path = path; - this.func = func; + this.listFunc = listFunc; } + /** + * Change the limit of this list operation. + * @param limit the delimiter to be set + * @return the updated {@link OpListBuilder} object + */ public OpListBuilder limit(long limit) { this.limit = limit; return this; } + /** + * Change the start_after of this list operation. + * + * @param startAfter the value to set the "startAfter" parameter to + * @return the updated {@link OpListBuilder} object + */ public OpListBuilder startAfter(String startAfter) { this.startAfter = Optional.ofNullable(startAfter); return this; } - + /** + * Change the delimiter. The default delimiter is "/" + * + * @param delimiter the delimiter to be set + * @return the updated {@link OpListBuilder} object + */ public OpListBuilder delimiter(String delimiter) { this.delimiter = Optional.ofNullable(delimiter); return this; } + /** + * Change the metakey of this list operation. The default metakey is `Metakey::Mode`. + * + * @param metakeys the metakeys to be set + * @return the updated {@link OpListBuilder} object + */ + public OpListBuilder metakeys(Metakey... metakeys) { + this.metakeys = metakeys; + return this; + } + + /** + * Build the OpList object with the provided parameters. + * + * @return The newly created {@link OpList} object. + */ public OpList build() { - return new OpList<>(path, limit, startAfter, delimiter, func); + return new OpList<>(path, listFunc, limit, startAfter, delimiter, metakeys); } } } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 1ac230f27bf..67d15ed5dba 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -19,12 +19,14 @@ use std::str::FromStr; use std::time::Duration; use std::usize; +use flagset::FlagSet; use jni::objects::JByteArray; use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::objects::JValue; use jni::objects::JValueOwned; +use jni::sys::jintArray; use jni::sys::{jlong, jobject}; use jni::JNIEnv; use opendal::layers::BlockingLayer; @@ -41,6 +43,7 @@ use crate::make_entry; use crate::make_metadata; use crate::make_operator_info; use crate::make_presigned_request; +use crate::metakey_to_flagset; use crate::Result; #[no_mangle] @@ -194,7 +197,7 @@ fn intern_stat(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result(op: &mut Operator, path: String) -> Result> { let metadata = op.stat(&path).await?; let mut env = unsafe { get_current_env() }; - make_metadata(&mut env, metadata) + make_metadata(&mut env, metadata, Metakey::Complete.into()) } /// # Safety @@ -464,11 +467,14 @@ pub unsafe extern "system" fn Java_org_apache_opendal_Operator_listWith( limit: jlong, start_after: JString, delimiter: JString, + metakeys: jintArray, ) -> jlong { - intern_list_with(&mut env, op, path, limit, start_after, delimiter).unwrap_or_else(|e| { - e.throw(&mut env); - 0 - }) + intern_list_with(&mut env, op, path, limit, start_after, delimiter, metakeys).unwrap_or_else( + |e| { + e.throw(&mut env); + 0 + }, + ) } fn intern_list_with( @@ -478,6 +484,7 @@ fn intern_list_with( limit: jlong, start_after: JString, delimiter: JString, + metakeys: jintArray, ) -> Result { let op = unsafe { &mut *op }; let id = request_id(env)?; @@ -498,9 +505,10 @@ fn intern_list_with( } else { Some(jstring_to_string(env, &delimiter)?) }; + let metakey = metakey_to_flagset(env, metakeys)?; unsafe { get_global_runtime() }.spawn(async move { - let result = do_list_with(op, path, limit, start_after, delimiter).await; + let result = do_list_with(op, path, limit, start_after, delimiter, metakey).await; complete_future(id, result.map(JValueOwned::Object)) }); @@ -513,8 +521,9 @@ async fn do_list_with<'local>( limit: Option, start_after: Option, delimiter: Option, + metakey: Option>, ) -> Result> { - let mut op = op.list_with(&path).metakey(Metakey::Complete); + let mut op = op.list_with(&path); if let Some(limit) = limit { op = op.limit(limit); } @@ -524,15 +533,19 @@ async fn do_list_with<'local>( if let Some(delimiter) = delimiter { op = op.delimiter(delimiter.as_str()); } + + if let Some(metakey) = metakey { + op = op.metakey(metakey); + } + let obs = op.await?; let mut env = unsafe { get_current_env() }; - let list = env.new_object("java/util/ArrayList", "()V", &[])?; let jlist = env.get_list(&list)?; for entry in obs { - let entry = make_entry(&mut env, entry)?; + let entry = make_entry(&mut env, entry, metakey.unwrap_or(Metakey::Mode.into()))?; jlist.add(&mut env, &entry)?; } 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 678f723ae01..7463deebd96 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 @@ -42,6 +42,7 @@ import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; import org.apache.opendal.Operator; +import org.apache.opendal.args.OpList.Metakey; import org.apache.opendal.test.condition.OpenDALExceptionCondition; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -558,9 +559,9 @@ public void testListDir() { boolean found = false; for (Entry entry : entries) { if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - assertTrue(metadata.isFile()); - assertThat(metadata.getContentLength()).isEqualTo(content.length); + Metadata meta = operator.stat(path).join(); + assertTrue(meta.isFile()); + assertThat(meta.getContentLength()).isEqualTo(content.length); found = true; } @@ -580,14 +581,64 @@ public void testListDirWithMetakey() { operator.write(path, content).join(); - final List entries = operator.list(parent + "/").join(); + final List entries = operator.listWith(parent + "/") + .metakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified) + .build() + .call() + .join(); boolean found = false; for (Entry entry : entries) { if (entry.getPath().equals(path)) { Metadata metadata = entry.getMetadata(); assertTrue(metadata.isFile()); assertThat(metadata.getContentLength()).isEqualTo(content.length); + // We don't care about the value, we just to check there is no panic. + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + assertTrue(found); + operator.delete(path).join(); + } + /** + * List dir with metakey complete + */ + @Test + public void testListDirWithMetakeyComplete() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + operator.write(path, content).join(); + + final List entries = operator.listWith(parent + "/") + .metakeys(Metakey.Complete) + .build() + .call() + .join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + // We don't care about the value, we just to check there is no panic. metadata.getCacheControl(); metadata.getContentDisposition(); metadata.getContentMd5(); @@ -722,12 +773,12 @@ public void testListWithStartAfter() { operator.createDir(dir).join(); final String[] given = new String[] { - String.format("%s%s-0", dir, UUID.randomUUID().toString()), - String.format("%s%s-1", dir, UUID.randomUUID().toString()), - String.format("%s%s-2", dir, UUID.randomUUID().toString()), - String.format("%s%s-3", dir, UUID.randomUUID().toString()), - String.format("%s%s-4", dir, UUID.randomUUID().toString()), - String.format("%s%s-5", dir, UUID.randomUUID().toString()), + String.format("%sfile-0", dir), + String.format("%sfile-1", dir), + String.format("%sfile-2", dir), + String.format("%sfile-3", dir), + String.format("%sfile-4", dir), + String.format("%sfile-5", dir), }; for (String path : given) { @@ -1217,11 +1268,11 @@ public void testBlockingListDir() { boolean found = false; for (Entry entry : list) { if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - found = true; + Metadata meta = blockingOperator.stat(path); + assertTrue(meta.isFile()); + assertThat(meta.getContentLength()).isEqualTo(content.length); - assertThat(metadata.getContentLength()).isEqualTo(content.length); - assertTrue(metadata.isFile()); + found = true; } } assertTrue(found); @@ -1237,7 +1288,58 @@ public void testBlockingListDirWithMetakey() { blockingOperator.write(path, content); - final List list = blockingOperator.list(parent + "/"); + final List list = blockingOperator + .listWith(parent + "/") + .metakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified) + .build() + .call(); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + assertTrue(metadata.isFile()); + + // We don't care about the value, we just to check there is no panic. + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + + assertTrue(found); + blockingOperator.delete(path); + } + + /** + * List dir with metakey complete + */ + public void testBlockingListDirWithMetakeyComplete() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final byte[] content = generateBytes(); + + blockingOperator.write(path, content); + + final List list = blockingOperator + .listWith(parent + "/") + .metakeys(Metakey.Complete) + .build() + .call(); boolean found = false; for (Entry entry : list) { if (entry.getPath().equals(path)) { From b72ccc176bd7839e45ecd0ff17d3824221b670f5 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 11:24:41 +0800 Subject: [PATCH 07/20] chore(binding/java): use Set for Metakey args --- bindings/java/src/blocking_operator.rs | 5 +++-- bindings/java/src/lib.rs | 11 +++++++++++ .../org/apache/opendal/BlockingOperator.java | 3 +-- .../main/java/org/apache/opendal/Operator.java | 3 +-- .../java/org/apache/opendal/args/OpList.java | 17 ++++++++++------- bindings/java/src/operator.rs | 13 +++---------- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index 7973fc0a99c..51a67d91b6f 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -28,6 +28,7 @@ use jni::JNIEnv; use opendal::BlockingOperator; use opendal::Metakey; +use crate::jstring_to_option_string; use crate::jstring_to_string; use crate::make_entry; use crate::make_metadata; @@ -278,10 +279,10 @@ fn intern_list_with( if limit >= 0 { op = op.limit(limit as usize); } - if !start_after.is_null() { + if jstring_to_option_string(env, &start_after)?.is_some() { op = op.start_after(jstring_to_string(env, &start_after)?.as_str()); } - if !delimiter.is_null() { + if jstring_to_option_string(env, &delimiter)?.is_some() { op = op.delimiter(jstring_to_string(env, &delimiter)?.as_str()); } diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index d44baa5b497..882915151ef 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -339,6 +339,14 @@ fn string_to_jstring<'a>(env: &mut JNIEnv<'a>, s: Option<&str>) -> Result Result> { + if s.is_null() { + Ok(None) + } else { + Ok(Some(jstring_to_string(env, s)?)) + } +} + /// # Safety /// /// The caller must guarantee that the Object passed in is an instance @@ -349,6 +357,9 @@ fn jstring_to_string(env: &mut JNIEnv, s: &JString) -> Result { } fn metakey_to_flagset(env: &mut JNIEnv, metakeys: jintArray) -> Result>> { + if metakeys.is_null() { + return Ok(None); + } let metakeys = unsafe { JPrimitiveArray::from_raw(metakeys) }; let len = env.get_array_length(&metakeys)?; let mut buf: Vec = vec![0; len as usize]; 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 049000a6045..45f0456721e 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; -import java.util.stream.Stream; import org.apache.opendal.args.OpList; import org.apache.opendal.args.OpList.Metakey; import org.apache.opendal.args.OpList.OpListBuilder; @@ -106,7 +105,7 @@ private List internListWith(OpList> opList) { opList.getLimit(), opList.getStartAfter().orElse(null), opList.getDelimiter().orElse(null), - Stream.of(opList.getMetakeys()).mapToInt(Metakey::getId).toArray()); + opList.getMetakeys().stream().mapToInt(Metakey::getId).toArray()); } @Override 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 e0a8e6f39da..7e3ba0f9a04 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -26,7 +26,6 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Stream; import org.apache.opendal.args.OpList; import org.apache.opendal.args.OpList.Metakey; import org.apache.opendal.args.OpList.OpListBuilder; @@ -216,7 +215,7 @@ private CompletableFuture> internListWith(OpList { /** * The metakey of the metadata that be queried. Default to be {@link Metakey#Mode}. */ - private final Metakey[] metakeys; + private final Set metakeys; /** * The list function. */ @@ -67,7 +70,7 @@ private OpList( long limit, Optional startAfter, Optional delimiter, - Metakey... metakeys) { + Set metakeys) { this.path = path; this.limit = limit; this.startAfter = startAfter; @@ -157,15 +160,15 @@ public static class OpListBuilder { private long limit; private Optional startAfter; private Optional delimiter; - private Metakey[] metakeys; + private Set metakeys; private OpListBuilder(String path, @NonNull Function, T> listFunc) { + this.path = path; + this.listFunc = listFunc; this.limit = -1L; this.startAfter = Optional.empty(); this.delimiter = Optional.empty(); - this.metakeys = new Metakey[] {Metakey.Mode}; - this.path = path; - this.listFunc = listFunc; + this.metakeys = new HashSet<>(); } /** @@ -206,7 +209,7 @@ public OpListBuilder delimiter(String delimiter) { * @return the updated {@link OpListBuilder} object */ public OpListBuilder metakeys(Metakey... metakeys) { - this.metakeys = metakeys; + this.metakeys.addAll(Arrays.asList(metakeys)); return this; } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 67d15ed5dba..ece95d21adf 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -38,6 +38,7 @@ use opendal::Scheme; use crate::get_current_env; use crate::get_global_runtime; use crate::jmap_to_hashmap; +use crate::jstring_to_option_string; use crate::jstring_to_string; use crate::make_entry; use crate::make_metadata; @@ -495,16 +496,8 @@ fn intern_list_with( } else { Some(limit as usize) }; - let start_after = if start_after.is_null() { - None - } else { - Some(jstring_to_string(env, &start_after)?) - }; - let delimiter = if delimiter.is_null() { - None - } else { - Some(jstring_to_string(env, &delimiter)?) - }; + let start_after = jstring_to_option_string(env, &start_after)?; + let delimiter = jstring_to_option_string(env, &delimiter)?; let metakey = metakey_to_flagset(env, metakeys)?; unsafe { get_global_runtime() }.spawn(async move { From 9893be8234efb5f77bd72cead4cea4a875be09b1 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 11:57:47 +0800 Subject: [PATCH 08/20] chore(binding/java): fix code --- .../apache/opendal/test/behavior/AbstractBehaviorTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 7463deebd96..d26ba9b7c7f 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 @@ -844,9 +844,10 @@ public void testRemoveAll() { }; for (String path : expected) { if (path.endsWith("/")) { - operator.createDir(String.format("%s/%s", parent, path)); + operator.createDir(String.format("%s/%s", parent, path)).join(); } else { - operator.write(String.format("%s/%s", parent, path), "test_scan"); + operator.write(String.format("%s/%s", parent, path), "test_scan") + .join(); } } From 9df26e9bf6653551bc461a39c2c7496d073161a4 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 12:56:49 +0800 Subject: [PATCH 09/20] chore(binding/java): fix code --- .../test/behavior/AbstractBehaviorTest.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) 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 35eb4e1a868..a8b1993cd3e 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 @@ -24,9 +24,9 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; @@ -70,6 +70,7 @@ public void setup() { + "\n--------------------------------------------------------------------------------", getClass().getCanonicalName(), scheme); + config.entrySet().forEach(e -> log.info("{}: {}", e.getKey(), e.getValue())); this.operator = Operator.of(scheme, config); this.blockingOperator = BlockingOperator.of(scheme, config); } @@ -543,7 +544,7 @@ public void precondition() { @Test public void testListDir() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); operator.write(path, content).join(); @@ -569,7 +570,7 @@ public void testListDir() { @Test public void testListDirWithMetakey() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); operator.write(path, content).join(); @@ -615,7 +616,7 @@ public void testListDirWithMetakey() { @Test public void testListDirWithMetakeyComplete() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); operator.write(path, content).join(); @@ -676,7 +677,7 @@ public void testListRichDir() { */ @Test public void testListEmptyDir() { - final String dir = String.format("%s/", UUID.randomUUID().toString()); + final String dir = String.format("%s/", UUID.randomUUID()); operator.createDir(dir).join(); final List entries = operator.list(dir).join(); @@ -690,7 +691,7 @@ public void testListEmptyDir() { */ @Test public void testListNotExistDir() { - final String dir = String.format("%s/", UUID.randomUUID().toString()); + final String dir = String.format("%s/", UUID.randomUUID()); final List entries = operator.list(dir).join(); assertThat(entries).isEmpty(); @@ -701,7 +702,7 @@ public void testListNotExistDir() { */ @Test public void testListSubDir() { - final String path = String.format("%s/", UUID.randomUUID().toString()); + final String path = String.format("%s/", UUID.randomUUID()); operator.createDir(path).join(); final List entries = operator.list("/").join(); @@ -723,11 +724,10 @@ public void testListSubDir() { */ @Test public void testListNestedDir() { - final String dir = String.format( - "%s/%s/", UUID.randomUUID().toString(), UUID.randomUUID().toString()); + final String dir = String.format("%s/%s/", UUID.randomUUID(), UUID.randomUUID()); final String fileName = UUID.randomUUID().toString(); final String filePath = String.format("%s/%s", dir, fileName); - final String dirName = String.format("%s/", UUID.randomUUID().toString()); + final String dirName = String.format("%s/", UUID.randomUUID()); final String dirPath = String.format("%s/%s", dir, dirName); final String content = "test_list_nested_dir"; @@ -762,7 +762,7 @@ public void testListWithStartAfter() { if (!operator.info.fullCapability.listWithStartAfter) { return; } - final String dir = String.format("%s/", UUID.randomUUID().toString()); + final String dir = String.format("%s/", UUID.randomUUID()); operator.createDir(dir).join(); final String[] given = new String[] { @@ -1247,7 +1247,7 @@ public void precondition() { @Test public void testBlockingListDir() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); blockingOperator.write(path, content); @@ -1271,7 +1271,7 @@ public void testBlockingListDir() { @Test public void testBlockingListDirWithMetakey() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); blockingOperator.write(path, content); @@ -1318,7 +1318,7 @@ public void testBlockingListDirWithMetakey() { */ public void testBlockingListDirWithMetakeyComplete() { final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID().toString()); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); final byte[] content = generateBytes(); blockingOperator.write(path, content); @@ -1353,7 +1353,7 @@ public void testBlockingListDirWithMetakeyComplete() { @Test public void testBlockingListNonExistDir() { - final String dir = String.format("%s/", UUID.randomUUID().toString()); + final String dir = String.format("%s/", UUID.randomUUID()); final List list = blockingOperator.list(dir); assertTrue(list.isEmpty()); From dc6f0d392fc25605223165b0b06606b2d2dce0ec Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 13:08:32 +0800 Subject: [PATCH 10/20] chore(binding/java): use Data for OpList --- .../java/src/main/java/org/apache/opendal/args/OpList.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bindings/java/src/main/java/org/apache/opendal/args/OpList.java b/bindings/java/src/main/java/org/apache/opendal/args/OpList.java index e13155874ec..745cd86e30a 100644 --- a/bindings/java/src/main/java/org/apache/opendal/args/OpList.java +++ b/bindings/java/src/main/java/org/apache/opendal/args/OpList.java @@ -24,14 +24,11 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; -import lombok.EqualsAndHashCode; -import lombok.Getter; +import lombok.Data; import lombok.NonNull; import lombok.ToString; -@Getter -@ToString -@EqualsAndHashCode +@Data public class OpList { private final String path; /** From e788c685d0a4c05b027f408af2d2a56fb5c523a7 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 18 Oct 2023 22:56:20 +0800 Subject: [PATCH 11/20] test(binding/java): add test for list --- .../opendal/test/behavior/AsyncListTest.java | 370 ++++++++++++++++++ .../test/behavior/BlockingCopyTest.java | 8 +- .../test/behavior/BlockingListTest.java | 223 +++++++++++ .../test/behavior/BlockingRenameTest.java | 14 +- 4 files changed, 604 insertions(+), 11 deletions(-) create mode 100644 bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java create mode 100644 bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java new file mode 100644 index 00000000000..8c6fcb5921f --- /dev/null +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java @@ -0,0 +1,370 @@ +/* + * 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.behavior; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; +import org.apache.opendal.Capability; +import org.apache.opendal.Entry; +import org.apache.opendal.Metadata; +import org.apache.opendal.OpenDALException; +import org.apache.opendal.args.OpList.Metakey; +import org.apache.opendal.test.condition.OpenDALExceptionCondition; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class AsyncListTest extends BehaviorTestBase { + + @BeforeAll + public void precondition() { + final Capability capability = op().info.fullCapability; + assumeTrue(capability.read && capability.write && capability.list); + } + + /** + * List dir should return newly created file. + */ + @Test + public void testListDir() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + op().write(path, content).join(); + + final List entries = op().list(parent + "/").join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata meta = op().stat(path).join(); + assertTrue(meta.isFile()); + assertThat(meta.getContentLength()).isEqualTo(content.length); + + found = true; + } + } + assertTrue(found); + op().delete(path).join(); + } + + /** + * List dir with metakey + */ + @Test + public void testListDirWithMetakey() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + op().write(path, content).join(); + + final List entries = op().listWith(parent + "/") + .metakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified) + .build() + .call() + .join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + assertTrue(found); + op().delete(path).join(); + } + + /** + * List dir with metakey complete + */ + @Test + public void testListDirWithMetakeyComplete() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + op().write(path, content).join(); + + final List entries = op().listWith(parent + "/") + .metakeys(Metakey.Complete) + .build() + .call() + .join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + assertTrue(found); + op().delete(path).join(); + } + + /** + * listing a directory, which contains more objects than a single page can take. + */ + @Test + public void testListRichDir() { + final String parent = "test_list_rich_dir"; + op().createDir(parent + "/").join(); + final List expected = new ArrayList<>(); + for (int i = 0; i < 100; i++) { + expected.add(String.format("%s/file-%d", parent, i)); + } + + for (String path : expected) { + op().write(path, parent).join(); + } + + final List entries = op().list(parent + "/").join(); + final List actual = + entries.stream().map(Entry::getPath).sorted().collect(Collectors.toList()); + + Collections.sort(expected); + assertThat(actual).isEqualTo(expected); + op().removeAll(parent + "/").join(); + } + + /** + * List empty dir should return nothing. + */ + @Test + public void testListEmptyDir() { + final String dir = String.format("%s/", UUID.randomUUID()); + op().createDir(dir).join(); + + final List entries = op().list(dir).join(); + assertThat(entries).isEmpty(); + + op().delete(dir).join(); + } + + /** + * List non exist dir should return nothing. + */ + @Test + public void testListNotExistDir() { + final String dir = String.format("%s/", UUID.randomUUID()); + + final List entries = op().list(dir).join(); + assertThat(entries).isEmpty(); + } + + /** + * List dir should return correct sub dir. + */ + @Test + public void testListSubDir() { + final String path = String.format("%s/", UUID.randomUUID()); + op().createDir(path).join(); + + final List entries = op().list("/").join(); + boolean found = false; + for (Entry entry : entries) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isDir()); + found = true; + } + } + assertTrue(found); + + op().delete(path).join(); + } + + /** + * List dir should also to list nested dir. + */ + @Test + public void testListNestedDir() { + final String dir = String.format("%s/%s/", UUID.randomUUID(), UUID.randomUUID()); + final String fileName = UUID.randomUUID().toString(); + final String filePath = String.format("%s/%s", dir, fileName); + final String dirName = String.format("%s/", UUID.randomUUID()); + final String dirPath = String.format("%s/%s", dir, dirName); + final String content = "test_list_nested_dir"; + + op().createDir(dir).join(); + op().write(filePath, content).join(); + op().createDir(dirPath).join(); + + final List entries = op().list(dir).join(); + assertThat(entries).hasSize(2); + + for (Entry entry : entries) { + // check file + if (entry.getPath().equals(filePath)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isFile()); + assertThat(metadata.getContentLength()).isEqualTo(content.length()); + // check dir + } else if (entry.getPath().equals(dirPath)) { + Metadata metadata = entry.getMetadata(); + assertTrue(metadata.isDir()); + } + } + + op().removeAll(dir).join(); + } + + /** + * List with start after should start listing after the specified key + */ + @Test + public void testListWithStartAfter() { + if (!op().info.fullCapability.listWithStartAfter) { + return; + } + final String dir = UUID.randomUUID().toString() + "/"; + op().createDir(dir).join(); + + final String[] given = new String[] { + String.format("%sfile-0", dir), + String.format("%sfile-1", dir), + String.format("%sfile-2", dir), + String.format("%sfile-3", dir), + String.format("%sfile-4", dir), + String.format("%sfile-5", dir), + }; + + for (String path : given) { + op().write(path, "content").join(); + } + + final List entries = + op().listWith(dir).startAfter(given[2]).build().call().join(); + final List expected = entries.stream().map(Entry::getPath).collect(Collectors.toList()); + + Assertions.assertThat(expected).isEqualTo(Arrays.asList(given[3], given[4], given[5])); + + op().removeAll(dir).join(); + } + + @Test + public void testScanRoot() { + final List entries = + op().listWith("").delimiter("").build().call().join(); + final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertTrue(!actual.contains("/"), "empty root shouldn't return itself"); + assertTrue(!actual.contains(""), "empty root shouldn't return empty"); + } + + /** + * Walk top down should output as expected + */ + @Test + public void testScan() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + op().createDir(String.format("%s/%s", parent, path)).join(); + } else { + op().write(String.format("%s/%s", parent, path), "test_scan").join(); + } + } + final List entries = op().listWith(String.format("%s/x/", parent)) + .delimiter("") + .build() + .call() + .join(); + final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertThat(actual).contains(parent + "/x/y", parent + "/x/x/y", parent + "/x/x/x/y"); + + op().removeAll(parent + "/").join(); + } + + /** + * Remove all should remove all in this path. + */ + @Test + public void testRemoveAll() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + op().createDir(String.format("%s/%s", parent, path)).join(); + } else { + op().write(String.format("%s/%s", parent, path), "test_scan").join(); + } + } + + op().removeAll(parent + "/x/").join(); + + for (String path : expected) { + if (path.endsWith("/")) { + continue; + } + assertThatThrownBy(() -> + op().stat(String.format("%s/%s", parent, path)).join()) + .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + } + + op().removeAll(parent + "/").join(); + } +} diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingCopyTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingCopyTest.java index bba304e7a41..9d60b194773 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingCopyTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingCopyTest.java @@ -83,7 +83,7 @@ public void testBlockingCopySourceDir() { blockingOp().createDir(sourcePath); assertThatThrownBy(() -> blockingOp().copy(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOp().delete(sourcePath); } @@ -102,8 +102,8 @@ public void testBlockingCopyTargetDir() { blockingOp().createDir(targetPath); - assertThatThrownBy(() -> op().copy(sourcePath, targetPath).join()) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + assertThatThrownBy(() -> blockingOp().copy(sourcePath, targetPath)) + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOp().delete(sourcePath); blockingOp().delete(targetPath); @@ -120,7 +120,7 @@ public void testBlockingCopySelf() { blockingOp().write(sourcePath, sourceContent); assertThatThrownBy(() -> blockingOp().copy(sourcePath, sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsSameFile)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsSameFile)); blockingOp().delete(sourcePath); } diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java new file mode 100644 index 00000000000..d9645ae2b82 --- /dev/null +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java @@ -0,0 +1,223 @@ +/* + * 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.behavior; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; +import org.apache.opendal.Capability; +import org.apache.opendal.Entry; +import org.apache.opendal.Metadata; +import org.apache.opendal.OpenDALException; +import org.apache.opendal.args.OpList.Metakey; +import org.apache.opendal.test.condition.OpenDALExceptionCondition; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class BlockingListTest extends BehaviorTestBase { + @BeforeAll + public void precondition() { + final Capability capability = blockingOp().info.fullCapability; + assumeTrue(capability.read && capability.write && capability.copy && capability.blocking && capability.list); + } + + @Test + public void testBlockingListDir() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + blockingOp().write(path, content); + + final List list = blockingOp().list(parent + "/"); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata meta = blockingOp().stat(path); + assertTrue(meta.isFile()); + assertThat(meta.getContentLength()).isEqualTo(content.length); + + found = true; + } + } + assertTrue(found); + + blockingOp().delete(path); + } + + @Test + public void testBlockingListDirWithMetakey() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + blockingOp().write(path, content); + + final List list = blockingOp() + .listWith(parent + "/") + .metakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified) + .build() + .call(); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + assertTrue(metadata.isFile()); + + // We don't care about the value, we just to check there is no panic. + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + + assertTrue(found); + blockingOp().delete(path); + } + + /** + * List dir with metakey complete + */ + public void testBlockingListDirWithMetakeyComplete() { + final String parent = UUID.randomUUID().toString(); + final String path = String.format("%s/%s", parent, UUID.randomUUID()); + final byte[] content = generateBytes(); + + blockingOp().write(path, content); + + final List list = blockingOp() + .listWith(parent + "/") + .metakeys(Metakey.Complete) + .build() + .call(); + boolean found = false; + for (Entry entry : list) { + if (entry.getPath().equals(path)) { + Metadata metadata = entry.getMetadata(); + assertThat(metadata.getContentLength()).isEqualTo(content.length); + assertTrue(metadata.isFile()); + + // We don't care about the value, we just to check there is no panic. + metadata.getCacheControl(); + metadata.getContentDisposition(); + metadata.getContentMd5(); + metadata.getContentType(); + metadata.getEtag(); + metadata.getVersion(); + metadata.getLastModified(); + found = true; + } + } + + assertTrue(found); + blockingOp().delete(path); + } + + @Test + public void testBlockingListNonExistDir() { + final String dir = String.format("%s/", UUID.randomUUID()); + + final List list = blockingOp().list(dir); + assertTrue(list.isEmpty()); + } + + @Test + public void testBlockingScan() { + final String parent = UUID.randomUUID().toString(); + + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + + for (String path : expected) { + final byte[] content = generateBytes(); + if (path.endsWith("/")) { + blockingOp().createDir(String.format("%s/%s", parent, path)); + } else { + blockingOp().write(String.format("%s/%s", parent, path), content); + } + } + + final List list = blockingOp() + .listWith(String.format("%s/x/", parent)) + .delimiter("") + .build() + .call(); + final Set paths = list.stream().map(Entry::getPath).collect(Collectors.toSet()); + + assertTrue(paths.contains(parent + "/x/y")); + assertTrue(paths.contains(parent + "/x/x/y")); + assertTrue(paths.contains(parent + "/x/x/x/y")); + + blockingOp().removeAll(parent + "/"); + } + + /** + * Remove all should remove all in this path. + */ + @Test + public void testBlockingRemoveAll() { + final String parent = UUID.randomUUID().toString(); + final String[] expected = new String[] { + "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", + }; + for (String path : expected) { + if (path.endsWith("/")) { + blockingOp().createDir(String.format("%s/%s", parent, path)); + } else { + blockingOp().write(String.format("%s/%s", parent, path), "test_scan"); + } + } + + blockingOp().removeAll(parent + "/x/"); + + for (String path : expected) { + if (path.endsWith("/")) { + continue; + } + assertThatThrownBy(() -> blockingOp().stat(String.format("%s/%s", parent, path))) + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); + } + + blockingOp().removeAll(parent + "/"); + } +} diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingRenameTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingRenameTest.java index 0993f15ff3e..925c95532e8 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingRenameTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingRenameTest.java @@ -54,7 +54,7 @@ public void testBlockingRenameFile() { blockingOp().rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOp().stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOp().stat(targetPath).getContentLength()).isEqualTo(sourceContent.length); @@ -71,7 +71,7 @@ public void testBlockingRenameNonExistingSource() { final String targetPath = UUID.randomUUID().toString(); assertThatThrownBy(() -> blockingOp().rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); } /** @@ -85,7 +85,7 @@ public void testBlockingRenameSourceDir() { blockingOp().createDir(sourcePath); assertThatThrownBy(() -> blockingOp().rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); } /** @@ -103,7 +103,7 @@ public void testBlockingRenameTargetDir() { blockingOp().createDir(targetPath); assertThatThrownBy(() -> blockingOp().rename(sourcePath, targetPath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsADirectory)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsADirectory)); blockingOp().delete(sourcePath); blockingOp().delete(targetPath); @@ -120,7 +120,7 @@ public void testBlockingRenameSelf() { blockingOp().write(sourcePath, sourceContent); assertThatThrownBy(() -> blockingOp().rename(sourcePath, sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.IsSameFile)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.IsSameFile)); blockingOp().delete(sourcePath); } @@ -140,7 +140,7 @@ public void testBlockingRenameNested() { blockingOp().rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOp().stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOp().read(targetPath)).isEqualTo(sourceContent); @@ -168,7 +168,7 @@ public void testBlockingRenameOverwrite() { blockingOp().rename(sourcePath, targetPath); assertThatThrownBy(() -> blockingOp().stat(sourcePath)) - .is(OpenDALExceptionCondition.ofAsync(OpenDALException.Code.NotFound)); + .is(OpenDALExceptionCondition.ofSync(OpenDALException.Code.NotFound)); assertThat(blockingOp().read(targetPath)).isEqualTo(sourceContent); From 450771494b032a28592928e7575e29ebbf6773e9 Mon Sep 17 00:00:00 2001 From: G-XD Date: Thu, 19 Oct 2023 23:43:59 +0800 Subject: [PATCH 12/20] refactor(binding/java): update list api --- bindings/java/src/blocking_operator.rs | 6 +- .../org/apache/opendal/BlockingOperator.java | 26 +- .../java/org/apache/opendal/Operator.java | 28 +-- .../java/org/apache/opendal/args/OpList.java | 222 ------------------ .../org/apache/opendal/args/OpListArgs.java | 127 ++++++++++ bindings/java/src/operator.rs | 18 +- .../opendal/test/behavior/AsyncListTest.java | 56 +++-- .../test/behavior/BlockingListTest.java | 45 ++-- 8 files changed, 206 insertions(+), 322 deletions(-) delete mode 100644 bindings/java/src/main/java/org/apache/opendal/args/OpList.java create mode 100644 bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index b8ca2c68732..f7789678063 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -251,7 +251,7 @@ fn intern_remove_all(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) /// /// This function should not be called before the Operator are ready. #[no_mangle] -pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_listWith( +pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_list( mut env: JNIEnv, _: JClass, op: *mut BlockingOperator, @@ -261,7 +261,7 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_listWith( delimiter: JString, metakeys: jintArray, ) -> jobject { - intern_list_with( + intern_list( &mut env, &mut *op, path, @@ -276,7 +276,7 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_listWith( }) } -fn intern_list_with( +fn intern_list( env: &mut JNIEnv, op: &mut BlockingOperator, path: JString, 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 b0e70d89a05..b1da3105701 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -22,9 +22,7 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; -import org.apache.opendal.args.OpList; -import org.apache.opendal.args.OpList.Metakey; -import org.apache.opendal.args.OpList.OpListBuilder; +import org.apache.opendal.args.OpListArgs; /** * BlockingOperator represents an underneath OpenDAL operator that @@ -101,21 +99,17 @@ public void removeAll(String path) { } public List list(String path) { - return listWith(path).build().call(); + return list(new OpListArgs(path)); } - public OpListBuilder> listWith(String path) { - return OpList.builder(path, this::internListWith); - } - - private List internListWith(OpList> opList) { - return listWith( + public List list(OpListArgs args) { + return list( nativeHandle, - opList.getPath(), - opList.getLimit(), - opList.getStartAfter().orElse(null), - opList.getDelimiter().orElse(null), - opList.getMetakeys().stream().mapToInt(Metakey::getId).toArray()); + args.getPath(), + args.getLimit(), + args.getStartAfter(), + args.getDelimiter(), + args.getMetakeys()); } @Override @@ -139,6 +133,6 @@ private List internListWith(OpList> opList) { private static native void removeAll(long nativeHandle, String path); - private static native List listWith( + private static native List list( long nativeHandle, String path, long limit, String startAfter, String delimiter, int... metakeys); } 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 85aa92e0131..e845229e962 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -26,9 +26,7 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import org.apache.opendal.args.OpList; -import org.apache.opendal.args.OpList.Metakey; -import org.apache.opendal.args.OpList.OpListBuilder; +import org.apache.opendal.args.OpListArgs; /** * Operator represents an underneath OpenDAL operator that @@ -215,22 +213,18 @@ public CompletableFuture removeAll(String path) { } public CompletableFuture> list(String path) { - return listWith(path).build().call(); + return list(new OpListArgs(path)); } - public OpListBuilder>> listWith(String path) { - return OpList.builder(path, this::internListWith); - } - - private CompletableFuture> internListWith(OpList>> opList) { - final long requestId = listWith( + public CompletableFuture> list(OpListArgs args) { + final long requestid = list( nativeHandle, - opList.getPath(), - opList.getLimit(), - opList.getStartAfter().orElse(null), - opList.getDelimiter().orElse(null), - opList.getMetakeys().stream().mapToInt(Metakey::getId).toArray()); - return AsyncRegistry.take(requestId); + args.getPath(), + args.getLimit(), + args.getStartAfter(), + args.getDelimiter(), + args.getMetakeys()); + return AsyncRegistry.take(requestid); } @Override @@ -268,6 +262,6 @@ private CompletableFuture> internListWith(OpList { - private final String path; - /** - * The limit passed to underlying service to specify the max results that could return. - */ - private final long limit; - /** - * The start_after passes to underlying service to specify the specified key to start listing from. - */ - private final Optional startAfter; - /** - * The delimiter used to for the list operation. Default to be `/` - */ - private final Optional delimiter; - /** - * The metakey of the metadata that be queried. Default to be {@link Metakey#Mode}. - */ - private final Set metakeys; - /** - * The list function. - */ - private final Function, T> listFunc; - - /** - * Calls the list function and returns the result. - * - * @return the result of calling the list - */ - public T call() { - return listFunc.apply(this); - } - - private OpList( - @NonNull String path, - @NonNull Function, T> listFunc, - long limit, - Optional startAfter, - Optional delimiter, - Set metakeys) { - this.path = path; - this.limit = limit; - this.startAfter = startAfter; - this.delimiter = delimiter; - this.listFunc = listFunc; - this.metakeys = metakeys; - } - - public static OpListBuilder builder(String path, Function, T> listFunc) { - return new OpListBuilder<>(path, listFunc); - } - - /** - * Metakey describes the metadata keys that can be queried. - * - * For query: - * - * At user side, we will allow user to query the metadata. If - * the meta has been stored, we will return directly. If no, we will - * call `stat` internally to fetch the metadata. - */ - public enum Metakey { - /** - * The special metadata key that used to mark this entry - * already contains all metadata. - */ - Complete(0), - /** - * - * Key for mode. - */ - Mode(1), - /** - * Key for cache control. - */ - CacheControl(2), - /** - * Key for content disposition. - */ - ContentDisposition(3), - /** - * Key for content length. - */ - ContentLength(4), - /** - * Key for content md5. - */ - ContentMd5(5), - /** - * Key for content range. - */ - ContentRange(6), - /** - * Key for content type. - */ - ContentType(7), - /** - * Key for etag. - */ - Etag(8), - /** - * Key for last last modified. - */ - LastModified(9), - /** - * Key for version. - */ - Version(10), - ; - - private final int id; - - Metakey(int id) { - this.id = id; - } - - public int getId() { - return id; - } - } - - @ToString - public static class OpListBuilder { - private final String path; - private final Function, T> listFunc; - - private long limit; - private Optional startAfter; - private Optional delimiter; - private Set metakeys; - - private OpListBuilder(String path, @NonNull Function, T> listFunc) { - this.path = path; - this.listFunc = listFunc; - this.limit = -1L; - this.startAfter = Optional.empty(); - this.delimiter = Optional.empty(); - this.metakeys = new HashSet<>(); - } - - /** - * Change the limit of this list operation. - * @param limit the delimiter to be set - * @return the updated {@link OpListBuilder} object - */ - public OpListBuilder limit(long limit) { - this.limit = limit; - return this; - } - - /** - * Change the start_after of this list operation. - * - * @param startAfter the value to set the "startAfter" parameter to - * @return the updated {@link OpListBuilder} object - */ - public OpListBuilder startAfter(String startAfter) { - this.startAfter = Optional.ofNullable(startAfter); - return this; - } - /** - * Change the delimiter. The default delimiter is "/" - * - * @param delimiter the delimiter to be set - * @return the updated {@link OpListBuilder} object - */ - public OpListBuilder delimiter(String delimiter) { - this.delimiter = Optional.ofNullable(delimiter); - return this; - } - - /** - * Change the metakey of this list operation. The default metakey is `Metakey::Mode`. - * - * @param metakeys the metakeys to be set - * @return the updated {@link OpListBuilder} object - */ - public OpListBuilder metakeys(Metakey... metakeys) { - this.metakeys.addAll(Arrays.asList(metakeys)); - return this; - } - - /** - * Build the OpList object with the provided parameters. - * - * @return The newly created {@link OpList} object. - */ - public OpList build() { - return new OpList<>(path, listFunc, limit, startAfter, delimiter, metakeys); - } - } -} diff --git a/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java b/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java new file mode 100644 index 00000000000..a8bb2cecddb --- /dev/null +++ b/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java @@ -0,0 +1,127 @@ +/* + * 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.args; + +import java.util.Objects; +import java.util.stream.Stream; +import lombok.AllArgsConstructor; +import lombok.Data; + +@Data +@AllArgsConstructor +public class OpListArgs { + private String path; + /** + * The limit passed to underlying service to specify the max results that could return. + */ + private long limit; + /** + * The start_after passes to underlying service to specify the specified key to start listing from. + */ + private String startAfter; + /** + * The delimiter used to for the list operation. Default to be `/` + */ + private String delimiter; + /** + * The metakey of the metadata that be queried. Default to be {@link Metakey#Mode}. + */ + private int[] metakeys; + + public OpListArgs(String path) { + this(path, -1, null, null, new int[] {}); + } + + public void setMetakeys(Metakey... metakeys) { + this.metakeys = Stream.of(metakeys) + .filter(Objects::nonNull) + .mapToInt(Metakey::getId) + .toArray(); + } + + /** + * Metakey describes the metadata keys that can be queried. + * + * For query: + * + * At user side, we will allow user to query the metadata. If + * the meta has been stored, we will return directly. If no, we will + * call `stat` internally to fetch the metadata. + */ + public enum Metakey { + /** + * The special metadata key that used to mark this entry + * already contains all metadata. + */ + Complete(0), + /** + * + * Key for mode. + */ + Mode(1), + /** + * Key for cache control. + */ + CacheControl(2), + /** + * Key for content disposition. + */ + ContentDisposition(3), + /** + * Key for content length. + */ + ContentLength(4), + /** + * Key for content md5. + */ + ContentMd5(5), + /** + * Key for content range. + */ + ContentRange(6), + /** + * Key for content type. + */ + ContentType(7), + /** + * Key for etag. + */ + Etag(8), + /** + * Key for last last modified. + */ + LastModified(9), + /** + * Key for version. + */ + Version(10), + ; + + private int id; + + Metakey(int id) { + this.id = id; + } + + public int getId() { + return id; + } + } +} diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index c59a43cd468..c51d3719039 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -473,7 +473,7 @@ async fn do_remove_all(op: &mut Operator, path: String) -> Result<()> { /// /// This function should not be called before the Operator are ready. #[no_mangle] -pub unsafe extern "system" fn Java_org_apache_opendal_Operator_listWith( +pub unsafe extern "system" fn Java_org_apache_opendal_Operator_list( mut env: JNIEnv, _: JClass, op: *mut Operator, @@ -483,15 +483,13 @@ pub unsafe extern "system" fn Java_org_apache_opendal_Operator_listWith( delimiter: JString, metakeys: jintArray, ) -> jlong { - intern_list_with(&mut env, op, path, limit, start_after, delimiter, metakeys).unwrap_or_else( - |e| { - e.throw(&mut env); - 0 - }, - ) + intern_list(&mut env, op, path, limit, start_after, delimiter, metakeys).unwrap_or_else(|e| { + e.throw(&mut env); + 0 + }) } -fn intern_list_with( +fn intern_list( env: &mut JNIEnv, op: *mut Operator, path: JString, @@ -514,14 +512,14 @@ fn intern_list_with( let metakey = metakey_to_flagset(env, metakeys)?; unsafe { get_global_runtime() }.spawn(async move { - let result = do_list_with(op, path, limit, start_after, delimiter, metakey).await; + let result = do_list(op, path, limit, start_after, delimiter, metakey).await; complete_future(id, result.map(JValueOwned::Object)) }); Ok(id) } -async fn do_list_with<'local>( +async fn do_list<'local>( op: &mut Operator, path: String, limit: Option, diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java index 8c6fcb5921f..f44c1cd7496 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java @@ -34,7 +34,8 @@ import org.apache.opendal.Entry; import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; -import org.apache.opendal.args.OpList.Metakey; +import org.apache.opendal.args.OpListArgs; +import org.apache.opendal.args.OpListArgs.Metakey; import org.apache.opendal.test.condition.OpenDALExceptionCondition; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -87,20 +88,18 @@ public void testListDirWithMetakey() { op().write(path, content).join(); - final List entries = op().listWith(parent + "/") - .metakeys( - Metakey.Mode, - Metakey.CacheControl, - Metakey.ContentDisposition, - Metakey.ContentLength, - Metakey.ContentMd5, - Metakey.ContentType, - Metakey.Etag, - Metakey.Version, - Metakey.LastModified) - .build() - .call() - .join(); + final OpListArgs args = new OpListArgs(parent + "/"); + args.setMetakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified); + final List entries = op().list(args).join(); boolean found = false; for (Entry entry : entries) { if (entry.getPath().equals(path)) { @@ -133,11 +132,9 @@ public void testListDirWithMetakeyComplete() { op().write(path, content).join(); - final List entries = op().listWith(parent + "/") - .metakeys(Metakey.Complete) - .build() - .call() - .join(); + final OpListArgs args = new OpListArgs(parent + "/"); + args.setMetakeys(Metakey.Complete); + final List entries = op().list(args).join(); boolean found = false; for (Entry entry : entries) { if (entry.getPath().equals(path)) { @@ -290,8 +287,9 @@ public void testListWithStartAfter() { op().write(path, "content").join(); } - final List entries = - op().listWith(dir).startAfter(given[2]).build().call().join(); + final OpListArgs args = new OpListArgs(dir); + args.setStartAfter(given[2]); + final List entries = op().list(args).join(); final List expected = entries.stream().map(Entry::getPath).collect(Collectors.toList()); Assertions.assertThat(expected).isEqualTo(Arrays.asList(given[3], given[4], given[5])); @@ -301,8 +299,10 @@ public void testListWithStartAfter() { @Test public void testScanRoot() { - final List entries = - op().listWith("").delimiter("").build().call().join(); + final OpListArgs args = new OpListArgs(""); + args.setDelimiter(""); + + final List entries = op().list(args).join(); final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); assertTrue(!actual.contains("/"), "empty root shouldn't return itself"); @@ -325,11 +325,9 @@ public void testScan() { op().write(String.format("%s/%s", parent, path), "test_scan").join(); } } - final List entries = op().listWith(String.format("%s/x/", parent)) - .delimiter("") - .build() - .call() - .join(); + final OpListArgs args = new OpListArgs(parent + "/x/"); + args.setDelimiter(""); + final List entries = op().list(args).join(); final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); assertThat(actual).contains(parent + "/x/y", parent + "/x/x/y", parent + "/x/x/x/y"); diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java index d9645ae2b82..cf7e915d6f4 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java @@ -31,7 +31,8 @@ import org.apache.opendal.Entry; import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; -import org.apache.opendal.args.OpList.Metakey; +import org.apache.opendal.args.OpListArgs; +import org.apache.opendal.args.OpListArgs.Metakey; import org.apache.opendal.test.condition.OpenDALExceptionCondition; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -77,20 +78,18 @@ public void testBlockingListDirWithMetakey() { blockingOp().write(path, content); - final List list = blockingOp() - .listWith(parent + "/") - .metakeys( - Metakey.Mode, - Metakey.CacheControl, - Metakey.ContentDisposition, - Metakey.ContentLength, - Metakey.ContentMd5, - Metakey.ContentType, - Metakey.Etag, - Metakey.Version, - Metakey.LastModified) - .build() - .call(); + final OpListArgs args = new OpListArgs(parent + "/"); + args.setMetakeys( + Metakey.Mode, + Metakey.CacheControl, + Metakey.ContentDisposition, + Metakey.ContentLength, + Metakey.ContentMd5, + Metakey.ContentType, + Metakey.Etag, + Metakey.Version, + Metakey.LastModified); + final List list = blockingOp().list(args); boolean found = false; for (Entry entry : list) { if (entry.getPath().equals(path)) { @@ -124,11 +123,9 @@ public void testBlockingListDirWithMetakeyComplete() { blockingOp().write(path, content); - final List list = blockingOp() - .listWith(parent + "/") - .metakeys(Metakey.Complete) - .build() - .call(); + final OpListArgs args = new OpListArgs(parent + "/"); + args.setMetakeys(Metakey.Complete); + final List list = blockingOp().list(args); boolean found = false; for (Entry entry : list) { if (entry.getPath().equals(path)) { @@ -177,11 +174,9 @@ public void testBlockingScan() { } } - final List list = blockingOp() - .listWith(String.format("%s/x/", parent)) - .delimiter("") - .build() - .call(); + final OpListArgs args = new OpListArgs(parent + "/x/"); + args.setDelimiter(""); + final List list = blockingOp().list(args); final Set paths = list.stream().map(Entry::getPath).collect(Collectors.toSet()); assertTrue(paths.contains(parent + "/x/y")); From e6fb20719053a995a30b1369c7a9626803095423 Mon Sep 17 00:00:00 2001 From: G-XD Date: Thu, 19 Oct 2023 23:52:28 +0800 Subject: [PATCH 13/20] ci(binding/java): update root path of fs test --- .github/workflows/bindings_java.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bindings_java.yml b/.github/workflows/bindings_java.yml index fef27ca4505..d465def6671 100644 --- a/.github/workflows/bindings_java.yml +++ b/.github/workflows/bindings_java.yml @@ -90,6 +90,6 @@ jobs: ./mvnw test -Dtest="behavior.*Test" export OPENDAL_TEST=fs - export OPENDAL_FS_ROOT=/tmp + export OPENDAL_FS_ROOT=${{ runner.temp }}/ ./mvnw test -Dtest="behavior.*Test" From 145ff8d140cc879555ba91e1d08135446478876e Mon Sep 17 00:00:00 2001 From: G-XD Date: Fri, 20 Oct 2023 22:07:21 +0800 Subject: [PATCH 14/20] refactor(binding/java): refactor Metakey name --- .../java/org/apache/opendal/Metadata.java | 64 ++++++++++++++++++- .../org/apache/opendal/args/OpListArgs.java | 33 ++++++---- .../opendal/test/behavior/AsyncListTest.java | 20 +++--- .../test/behavior/BlockingListTest.java | 20 +++--- 4 files changed, 100 insertions(+), 37 deletions(-) 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 6d886a7802a..564ad8560d4 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Metadata.java +++ b/bindings/java/src/main/java/org/apache/opendal/Metadata.java @@ -21,6 +21,7 @@ import java.util.Date; import lombok.Data; +import org.apache.opendal.args.OpListArgs.Metakey; /** * Metadata carries all metadata associated with a path. @@ -28,13 +29,64 @@ @Data public class Metadata { public final EntryMode mode; + /** + * Content Length of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#CONTENT_LENGTH}, otherwise it will be -1. + */ public final long contentLength; + /** + * Content-Disposition of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#CONTENT_DISPOSITION}, otherwise it will be null. + */ public final String contentDisposition; + /** + * Content MD5 of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#CONTENT_MD5}, otherwise it will be null. + */ public final String contentMd5; + /** + * Content Type of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#CONTENT_TYPE}, otherwise it will be null. + */ public final String contentType; + /** + * Cache Control of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#CACHE_CONTROL}, otherwise it will be null. + */ public final String cacheControl; + /** + * Etag of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#ETAG}, otherwise it will be null. + */ public final String etag; + /** + * Last Modified of the entry. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#LAST_MODIFIED}, otherwise it will be null. + */ public final Date lastModified; + /** + * Version of the entry. + * Version is a string that can be used to identify the version of this entry. + * This field may come out from the version control system, like object + * versioning in AWS S3. + * + * This value is only available when calling on result of `stat` or `list` with + * {@link Metakey#VERSION}, otherwise it will be null. + */ public final String version; public Metadata( @@ -67,11 +119,17 @@ public boolean isDir() { } public enum EntryMode { - /// FILE means the path has data to read. + /** + * FILE means the path has data to read. + */ FILE, - /// DIR means the path can be listed. + /** + * DIR means the path can be listed. + */ DIR, - /// Unknown means we don't know what we can do on this path. + /** + * Unknown means we don't know what we can do on this path. + */ UNKNOWN; public static EntryMode of(int mode) { diff --git a/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java b/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java index a8bb2cecddb..d1468b20e93 100644 --- a/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java +++ b/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java @@ -29,11 +29,13 @@ public class OpListArgs { private String path; /** - * The limit passed to underlying service to specify the max results that could return. + * The limit passed to underlying service to specify the max results that could + * return. */ private long limit; /** - * The start_after passes to underlying service to specify the specified key to start listing from. + * The start_after passes to underlying service to specify the specified key to + * start listing from. */ private String startAfter; /** @@ -41,7 +43,8 @@ public class OpListArgs { */ private String delimiter; /** - * The metakey of the metadata that be queried. Default to be {@link Metakey#Mode}. + * The metakey of the metadata that be queried. Default to be + * {@link Metakey#MODE}. */ private int[] metakeys; @@ -70,48 +73,50 @@ public enum Metakey { * The special metadata key that used to mark this entry * already contains all metadata. */ - Complete(0), + COMPLETE(0), /** * * Key for mode. */ - Mode(1), + MODE(1), /** * Key for cache control. */ - CacheControl(2), + CACHE_CONTROL(2), /** * Key for content disposition. */ - ContentDisposition(3), + CONTENT_DISPOSITION(3), /** * Key for content length. */ - ContentLength(4), + CONTENT_LENGTH(4), /** * Key for content md5. */ - ContentMd5(5), + CONTENT_MD5(5), /** * Key for content range. + * + * Note: this is not supported yet. */ - ContentRange(6), + CONTENT_RANGE(6), /** * Key for content type. */ - ContentType(7), + CONTENT_TYPE(7), /** * Key for etag. */ - Etag(8), + ETAG(8), /** * Key for last last modified. */ - LastModified(9), + LAST_MODIFIED(9), /** * Key for version. */ - Version(10), + VERSION(10), ; private int id; diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java index f44c1cd7496..134694932a0 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java @@ -90,15 +90,15 @@ public void testListDirWithMetakey() { final OpListArgs args = new OpListArgs(parent + "/"); args.setMetakeys( - Metakey.Mode, - Metakey.CacheControl, - Metakey.ContentDisposition, - Metakey.ContentLength, - Metakey.ContentMd5, - Metakey.ContentType, - Metakey.Etag, - Metakey.Version, - Metakey.LastModified); + Metakey.MODE, + Metakey.CACHE_CONTROL, + Metakey.CONTENT_DISPOSITION, + Metakey.CONTENT_LENGTH, + Metakey.CONTENT_MD5, + Metakey.CONTENT_TYPE, + Metakey.ETAG, + Metakey.VERSION, + Metakey.LAST_MODIFIED); final List entries = op().list(args).join(); boolean found = false; for (Entry entry : entries) { @@ -133,7 +133,7 @@ public void testListDirWithMetakeyComplete() { op().write(path, content).join(); final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys(Metakey.Complete); + args.setMetakeys(Metakey.COMPLETE); final List entries = op().list(args).join(); boolean found = false; for (Entry entry : entries) { diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java index cf7e915d6f4..71eaafddfc9 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java @@ -80,15 +80,15 @@ public void testBlockingListDirWithMetakey() { final OpListArgs args = new OpListArgs(parent + "/"); args.setMetakeys( - Metakey.Mode, - Metakey.CacheControl, - Metakey.ContentDisposition, - Metakey.ContentLength, - Metakey.ContentMd5, - Metakey.ContentType, - Metakey.Etag, - Metakey.Version, - Metakey.LastModified); + Metakey.MODE, + Metakey.CACHE_CONTROL, + Metakey.CONTENT_DISPOSITION, + Metakey.CONTENT_LENGTH, + Metakey.CONTENT_MD5, + Metakey.CONTENT_TYPE, + Metakey.ETAG, + Metakey.VERSION, + Metakey.LAST_MODIFIED); final List list = blockingOp().list(args); boolean found = false; for (Entry entry : list) { @@ -124,7 +124,7 @@ public void testBlockingListDirWithMetakeyComplete() { blockingOp().write(path, content); final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys(Metakey.Complete); + args.setMetakeys(Metakey.COMPLETE); final List list = blockingOp().list(args); boolean found = false; for (Entry entry : list) { From 00dd58c493542efbcded2cc4f6124a0cf616edaa Mon Sep 17 00:00:00 2001 From: G-XD Date: Mon, 23 Oct 2023 19:07:36 +0800 Subject: [PATCH 15/20] refactor(binding/java): remove list with args support --- bindings/java/src/blocking_operator.rs | 48 +----- bindings/java/src/lib.rs | 46 ------ .../org/apache/opendal/BlockingOperator.java | 16 +- .../java/org/apache/opendal/Metadata.java | 28 ++-- .../java/org/apache/opendal/Operator.java | 16 +- .../org/apache/opendal/args/OpListArgs.java | 132 --------------- bindings/java/src/operator.rs | 59 +------ .../opendal/test/behavior/AsyncListTest.java | 156 ------------------ .../test/behavior/BlockingListTest.java | 112 ------------- 9 files changed, 26 insertions(+), 587 deletions(-) delete mode 100644 bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index f7789678063..d5675c84670 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -20,17 +20,15 @@ use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::sys::jobject; -use jni::sys::{jbyteArray, jintArray, jlong}; +use jni::sys::{jbyteArray, jlong}; use jni::JNIEnv; use opendal::BlockingOperator; use opendal::Metakey; -use crate::jstring_to_option_string; use crate::jstring_to_string; use crate::make_entry; use crate::make_metadata; -use crate::metakey_to_flagset; use crate::Result; /// # Safety @@ -256,58 +254,22 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_list( _: JClass, op: *mut BlockingOperator, path: JString, - limit: jlong, - start_after: JString, - delimiter: JString, - metakeys: jintArray, ) -> jobject { - intern_list( - &mut env, - &mut *op, - path, - limit, - start_after, - delimiter, - metakeys, - ) - .unwrap_or_else(|e| { + intern_list(&mut env, &mut *op, path).unwrap_or_else(|e| { e.throw(&mut env); JObject::default().into_raw() }) } -fn intern_list( - env: &mut JNIEnv, - op: &mut BlockingOperator, - path: JString, - limit: jlong, - start_after: JString, - delimiter: JString, - metakeys: jintArray, -) -> Result { +fn intern_list(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result { let path = jstring_to_string(env, &path)?; - let mut op = op.list_with(&path); - if limit >= 0 { - op = op.limit(limit as usize); - } - if jstring_to_option_string(env, &start_after)?.is_some() { - op = op.start_after(jstring_to_string(env, &start_after)?.as_str()); - } - if jstring_to_option_string(env, &delimiter)?.is_some() { - op = op.delimiter(jstring_to_string(env, &delimiter)?.as_str()); - } - - let metakey = metakey_to_flagset(env, metakeys)?; - if let Some(metakey) = metakey { - op = op.metakey(metakey); - } + let obs = op.list(&path)?; let list = env.new_object("java/util/ArrayList", "()V", &[])?; let jlist = env.get_list(&list)?; - let obs = op.call()?; for entry in obs { - let entry = make_entry(env, entry, metakey.unwrap_or(Metakey::Mode.into()))?; + let entry = make_entry(env, entry, Metakey::Mode.into())?; jlist.add(env, &entry)?; } diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index 9e510bcc0e9..6e30fa15ca6 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -22,12 +22,10 @@ use std::ffi::c_void; use crate::error::Error; use flagset::FlagSet; use jni::objects::JObject; -use jni::objects::JPrimitiveArray; use jni::objects::JString; use jni::objects::{JMap, JValue}; use jni::sys::jboolean; use jni::sys::jint; -use jni::sys::jintArray; use jni::sys::jlong; use jni::sys::JNI_VERSION_1_8; use jni::JNIEnv; @@ -340,14 +338,6 @@ fn string_to_jstring<'a>(env: &mut JNIEnv<'a>, s: Option<&str>) -> Result Result> { - if s.is_null() { - Ok(None) - } else { - Ok(Some(jstring_to_string(env, s)?)) - } -} - /// # Safety /// /// The caller must guarantee that the Object passed in is an instance @@ -356,39 +346,3 @@ fn jstring_to_string(env: &mut JNIEnv, s: &JString) -> Result { let res = unsafe { env.get_string_unchecked(s)? }; Ok(res.into()) } - -fn metakey_to_flagset(env: &mut JNIEnv, metakeys: jintArray) -> Result>> { - if metakeys.is_null() { - return Ok(None); - } - let metakeys = unsafe { JPrimitiveArray::from_raw(metakeys) }; - let len = env.get_array_length(&metakeys)?; - let mut buf: Vec = vec![0; len as usize]; - env.get_int_array_region(metakeys, 0, &mut buf)?; - - let mut metakey: Option> = None; - for key in buf { - let m = match key { - 0 => Metakey::Complete, - 1 => Metakey::Mode, - 2 => Metakey::CacheControl, - 3 => Metakey::ContentDisposition, - 4 => Metakey::ContentLength, - 5 => Metakey::ContentMd5, - 6 => Metakey::ContentRange, - 7 => Metakey::ContentType, - 8 => Metakey::Etag, - 9 => Metakey::LastModified, - 10 => Metakey::Version, - _ => Err(opendal::Error::new( - opendal::ErrorKind::Unexpected, - "Invalid metakey", - ))?, - }; - metakey = match metakey { - None => Some(m.into()), - Some(metakey) => Some(metakey | m), - } - } - Ok(metakey) -} 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 b1da3105701..f0b0a5facc5 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; -import org.apache.opendal.args.OpListArgs; /** * BlockingOperator represents an underneath OpenDAL operator that @@ -99,17 +98,7 @@ public void removeAll(String path) { } public List list(String path) { - return list(new OpListArgs(path)); - } - - public List list(OpListArgs args) { - return list( - nativeHandle, - args.getPath(), - args.getLimit(), - args.getStartAfter(), - args.getDelimiter(), - args.getMetakeys()); + return list(nativeHandle, path); } @Override @@ -133,6 +122,5 @@ public List list(OpListArgs args) { private static native void removeAll(long nativeHandle, String path); - private static native List list( - long nativeHandle, String path, long limit, String startAfter, String delimiter, int... metakeys); + private static native List list(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 564ad8560d4..0a35735d15f 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Metadata.java +++ b/bindings/java/src/main/java/org/apache/opendal/Metadata.java @@ -21,61 +21,56 @@ import java.util.Date; import lombok.Data; -import org.apache.opendal.args.OpListArgs.Metakey; /** * Metadata carries all metadata associated with a path. */ @Data public class Metadata { + /** + * Mode of the entry. + */ public final EntryMode mode; /** * Content Length of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#CONTENT_LENGTH}, otherwise it will be -1. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be -1. */ public final long contentLength; /** * Content-Disposition of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#CONTENT_DISPOSITION}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String contentDisposition; /** * Content MD5 of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#CONTENT_MD5}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String contentMd5; /** * Content Type of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#CONTENT_TYPE}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String contentType; /** * Cache Control of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#CACHE_CONTROL}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String cacheControl; /** * Etag of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#ETAG}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String etag; /** * Last Modified of the entry. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#LAST_MODIFIED}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final Date lastModified; /** @@ -84,8 +79,7 @@ public class Metadata { * This field may come out from the version control system, like object * versioning in AWS S3. * - * This value is only available when calling on result of `stat` or `list` with - * {@link Metakey#VERSION}, otherwise it will be null. + * Note: For now, this value is only available when calling on result of `stat`, otherwise it will be null. */ public final String version; 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 e845229e962..8c6ecfa0210 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -26,7 +26,6 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import org.apache.opendal.args.OpListArgs; /** * Operator represents an underneath OpenDAL operator that @@ -213,17 +212,7 @@ public CompletableFuture removeAll(String path) { } public CompletableFuture> list(String path) { - return list(new OpListArgs(path)); - } - - public CompletableFuture> list(OpListArgs args) { - final long requestid = list( - nativeHandle, - args.getPath(), - args.getLimit(), - args.getStartAfter(), - args.getDelimiter(), - args.getMetakeys()); + final long requestid = list(nativeHandle, path); return AsyncRegistry.take(requestid); } @@ -262,6 +251,5 @@ public CompletableFuture> list(OpListArgs args) { private static native long removeAll(long nativeHandle, String path); - private static native long list( - long nativeHandle, String path, long limit, String startAfter, String delimiter, int... metakeys); + private static native long list(long nativeHandle, String path); } diff --git a/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java b/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java deleted file mode 100644 index d1468b20e93..00000000000 --- a/bindings/java/src/main/java/org/apache/opendal/args/OpListArgs.java +++ /dev/null @@ -1,132 +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. - */ - -package org.apache.opendal.args; - -import java.util.Objects; -import java.util.stream.Stream; -import lombok.AllArgsConstructor; -import lombok.Data; - -@Data -@AllArgsConstructor -public class OpListArgs { - private String path; - /** - * The limit passed to underlying service to specify the max results that could - * return. - */ - private long limit; - /** - * The start_after passes to underlying service to specify the specified key to - * start listing from. - */ - private String startAfter; - /** - * The delimiter used to for the list operation. Default to be `/` - */ - private String delimiter; - /** - * The metakey of the metadata that be queried. Default to be - * {@link Metakey#MODE}. - */ - private int[] metakeys; - - public OpListArgs(String path) { - this(path, -1, null, null, new int[] {}); - } - - public void setMetakeys(Metakey... metakeys) { - this.metakeys = Stream.of(metakeys) - .filter(Objects::nonNull) - .mapToInt(Metakey::getId) - .toArray(); - } - - /** - * Metakey describes the metadata keys that can be queried. - * - * For query: - * - * At user side, we will allow user to query the metadata. If - * the meta has been stored, we will return directly. If no, we will - * call `stat` internally to fetch the metadata. - */ - public enum Metakey { - /** - * The special metadata key that used to mark this entry - * already contains all metadata. - */ - COMPLETE(0), - /** - * - * Key for mode. - */ - MODE(1), - /** - * Key for cache control. - */ - CACHE_CONTROL(2), - /** - * Key for content disposition. - */ - CONTENT_DISPOSITION(3), - /** - * Key for content length. - */ - CONTENT_LENGTH(4), - /** - * Key for content md5. - */ - CONTENT_MD5(5), - /** - * Key for content range. - * - * Note: this is not supported yet. - */ - CONTENT_RANGE(6), - /** - * Key for content type. - */ - CONTENT_TYPE(7), - /** - * Key for etag. - */ - ETAG(8), - /** - * Key for last last modified. - */ - LAST_MODIFIED(9), - /** - * Key for version. - */ - VERSION(10), - ; - - private int id; - - Metakey(int id) { - this.id = id; - } - - public int getId() { - return id; - } - } -} diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index c51d3719039..694533d6e2b 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -17,16 +17,13 @@ use std::str::FromStr; use std::time::Duration; -use std::usize; -use flagset::FlagSet; use jni::objects::JByteArray; use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::objects::JValue; use jni::objects::JValueOwned; -use jni::sys::jintArray; use jni::sys::{jlong, jobject}; use jni::JNIEnv; use opendal::layers::BlockingLayer; @@ -38,13 +35,11 @@ use opendal::Scheme; use crate::get_current_env; use crate::get_global_runtime; use crate::jmap_to_hashmap; -use crate::jstring_to_option_string; use crate::jstring_to_string; use crate::make_entry; use crate::make_metadata; use crate::make_operator_info; use crate::make_presigned_request; -use crate::metakey_to_flagset; use crate::Result; #[no_mangle] @@ -478,78 +473,36 @@ pub unsafe extern "system" fn Java_org_apache_opendal_Operator_list( _: JClass, op: *mut Operator, path: JString, - limit: jlong, - start_after: JString, - delimiter: JString, - metakeys: jintArray, ) -> jlong { - intern_list(&mut env, op, path, limit, start_after, delimiter, metakeys).unwrap_or_else(|e| { + intern_list(&mut env, op, path).unwrap_or_else(|e| { e.throw(&mut env); 0 }) } -fn intern_list( - env: &mut JNIEnv, - op: *mut Operator, - path: JString, - limit: jlong, - start_after: JString, - delimiter: JString, - metakeys: jintArray, -) -> Result { +fn intern_list(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result { let op = unsafe { &mut *op }; let id = request_id(env)?; let path = jstring_to_string(env, &path)?; - let limit = if limit < 0 { - None - } else { - Some(limit as usize) - }; - let start_after = jstring_to_option_string(env, &start_after)?; - let delimiter = jstring_to_option_string(env, &delimiter)?; - let metakey = metakey_to_flagset(env, metakeys)?; unsafe { get_global_runtime() }.spawn(async move { - let result = do_list(op, path, limit, start_after, delimiter, metakey).await; + let result = do_list(op, path).await; complete_future(id, result.map(JValueOwned::Object)) }); Ok(id) } -async fn do_list<'local>( - op: &mut Operator, - path: String, - limit: Option, - start_after: Option, - delimiter: Option, - metakey: Option>, -) -> Result> { - let mut op = op.list_with(&path); - if let Some(limit) = limit { - op = op.limit(limit); - } - if let Some(start_after) = start_after { - op = op.start_after(start_after.as_str()); - } - if let Some(delimiter) = delimiter { - op = op.delimiter(delimiter.as_str()); - } - - if let Some(metakey) = metakey { - op = op.metakey(metakey); - } - - let obs = op.await?; +async fn do_list<'local>(op: &mut Operator, path: String) -> Result> { + let obs = op.list(&path).await?; let mut env = unsafe { get_current_env() }; let list = env.new_object("java/util/ArrayList", "()V", &[])?; let jlist = env.get_list(&list)?; for entry in obs { - let entry = make_entry(&mut env, entry, metakey.unwrap_or(Metakey::Mode.into()))?; + let entry = make_entry(&mut env, entry, Metakey::Mode.into())?; jlist.add(&mut env, &entry)?; } diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java index 134694932a0..bf6e5b0f43b 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/AsyncListTest.java @@ -24,20 +24,15 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import org.apache.opendal.Capability; import org.apache.opendal.Entry; import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; -import org.apache.opendal.args.OpListArgs; -import org.apache.opendal.args.OpListArgs.Metakey; import org.apache.opendal.test.condition.OpenDALExceptionCondition; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; @@ -77,85 +72,6 @@ public void testListDir() { op().delete(path).join(); } - /** - * List dir with metakey - */ - @Test - public void testListDirWithMetakey() { - final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID()); - final byte[] content = generateBytes(); - - op().write(path, content).join(); - - final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys( - Metakey.MODE, - Metakey.CACHE_CONTROL, - Metakey.CONTENT_DISPOSITION, - Metakey.CONTENT_LENGTH, - Metakey.CONTENT_MD5, - Metakey.CONTENT_TYPE, - Metakey.ETAG, - Metakey.VERSION, - Metakey.LAST_MODIFIED); - final List entries = op().list(args).join(); - boolean found = false; - for (Entry entry : entries) { - if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - assertTrue(metadata.isFile()); - assertThat(metadata.getContentLength()).isEqualTo(content.length); - - metadata.getCacheControl(); - metadata.getContentDisposition(); - metadata.getContentMd5(); - metadata.getContentType(); - metadata.getEtag(); - metadata.getVersion(); - metadata.getLastModified(); - found = true; - } - } - assertTrue(found); - op().delete(path).join(); - } - - /** - * List dir with metakey complete - */ - @Test - public void testListDirWithMetakeyComplete() { - final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID()); - final byte[] content = generateBytes(); - - op().write(path, content).join(); - - final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys(Metakey.COMPLETE); - final List entries = op().list(args).join(); - boolean found = false; - for (Entry entry : entries) { - if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - assertTrue(metadata.isFile()); - assertThat(metadata.getContentLength()).isEqualTo(content.length); - - metadata.getCacheControl(); - metadata.getContentDisposition(); - metadata.getContentMd5(); - metadata.getContentType(); - metadata.getEtag(); - metadata.getVersion(); - metadata.getLastModified(); - found = true; - } - } - assertTrue(found); - op().delete(path).join(); - } - /** * listing a directory, which contains more objects than a single page can take. */ @@ -263,78 +179,6 @@ public void testListNestedDir() { op().removeAll(dir).join(); } - /** - * List with start after should start listing after the specified key - */ - @Test - public void testListWithStartAfter() { - if (!op().info.fullCapability.listWithStartAfter) { - return; - } - final String dir = UUID.randomUUID().toString() + "/"; - op().createDir(dir).join(); - - final String[] given = new String[] { - String.format("%sfile-0", dir), - String.format("%sfile-1", dir), - String.format("%sfile-2", dir), - String.format("%sfile-3", dir), - String.format("%sfile-4", dir), - String.format("%sfile-5", dir), - }; - - for (String path : given) { - op().write(path, "content").join(); - } - - final OpListArgs args = new OpListArgs(dir); - args.setStartAfter(given[2]); - final List entries = op().list(args).join(); - final List expected = entries.stream().map(Entry::getPath).collect(Collectors.toList()); - - Assertions.assertThat(expected).isEqualTo(Arrays.asList(given[3], given[4], given[5])); - - op().removeAll(dir).join(); - } - - @Test - public void testScanRoot() { - final OpListArgs args = new OpListArgs(""); - args.setDelimiter(""); - - final List entries = op().list(args).join(); - final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); - - assertTrue(!actual.contains("/"), "empty root shouldn't return itself"); - assertTrue(!actual.contains(""), "empty root shouldn't return empty"); - } - - /** - * Walk top down should output as expected - */ - @Test - public void testScan() { - final String parent = UUID.randomUUID().toString(); - final String[] expected = new String[] { - "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", - }; - for (String path : expected) { - if (path.endsWith("/")) { - op().createDir(String.format("%s/%s", parent, path)).join(); - } else { - op().write(String.format("%s/%s", parent, path), "test_scan").join(); - } - } - final OpListArgs args = new OpListArgs(parent + "/x/"); - args.setDelimiter(""); - final List entries = op().list(args).join(); - final Set actual = entries.stream().map(Entry::getPath).collect(Collectors.toSet()); - - assertThat(actual).contains(parent + "/x/y", parent + "/x/x/y", parent + "/x/x/x/y"); - - op().removeAll(parent + "/").join(); - } - /** * Remove all should remove all in this path. */ diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java index 71eaafddfc9..feb2913297b 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BlockingListTest.java @@ -24,15 +24,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.util.List; -import java.util.Set; import java.util.UUID; -import java.util.stream.Collectors; import org.apache.opendal.Capability; import org.apache.opendal.Entry; import org.apache.opendal.Metadata; import org.apache.opendal.OpenDALException; -import org.apache.opendal.args.OpListArgs; -import org.apache.opendal.args.OpListArgs.Metakey; import org.apache.opendal.test.condition.OpenDALExceptionCondition; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -70,85 +66,6 @@ public void testBlockingListDir() { blockingOp().delete(path); } - @Test - public void testBlockingListDirWithMetakey() { - final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID()); - final byte[] content = generateBytes(); - - blockingOp().write(path, content); - - final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys( - Metakey.MODE, - Metakey.CACHE_CONTROL, - Metakey.CONTENT_DISPOSITION, - Metakey.CONTENT_LENGTH, - Metakey.CONTENT_MD5, - Metakey.CONTENT_TYPE, - Metakey.ETAG, - Metakey.VERSION, - Metakey.LAST_MODIFIED); - final List list = blockingOp().list(args); - boolean found = false; - for (Entry entry : list) { - if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - assertThat(metadata.getContentLength()).isEqualTo(content.length); - assertTrue(metadata.isFile()); - - // We don't care about the value, we just to check there is no panic. - metadata.getCacheControl(); - metadata.getContentDisposition(); - metadata.getContentMd5(); - metadata.getContentType(); - metadata.getEtag(); - metadata.getVersion(); - metadata.getLastModified(); - found = true; - } - } - - assertTrue(found); - blockingOp().delete(path); - } - - /** - * List dir with metakey complete - */ - public void testBlockingListDirWithMetakeyComplete() { - final String parent = UUID.randomUUID().toString(); - final String path = String.format("%s/%s", parent, UUID.randomUUID()); - final byte[] content = generateBytes(); - - blockingOp().write(path, content); - - final OpListArgs args = new OpListArgs(parent + "/"); - args.setMetakeys(Metakey.COMPLETE); - final List list = blockingOp().list(args); - boolean found = false; - for (Entry entry : list) { - if (entry.getPath().equals(path)) { - Metadata metadata = entry.getMetadata(); - assertThat(metadata.getContentLength()).isEqualTo(content.length); - assertTrue(metadata.isFile()); - - // We don't care about the value, we just to check there is no panic. - metadata.getCacheControl(); - metadata.getContentDisposition(); - metadata.getContentMd5(); - metadata.getContentType(); - metadata.getEtag(); - metadata.getVersion(); - metadata.getLastModified(); - found = true; - } - } - - assertTrue(found); - blockingOp().delete(path); - } - @Test public void testBlockingListNonExistDir() { final String dir = String.format("%s/", UUID.randomUUID()); @@ -157,35 +74,6 @@ public void testBlockingListNonExistDir() { assertTrue(list.isEmpty()); } - @Test - public void testBlockingScan() { - final String parent = UUID.randomUUID().toString(); - - final String[] expected = new String[] { - "x/", "x/y", "x/x/", "x/x/y", "x/x/x/", "x/x/x/y", "x/x/x/x/", - }; - - for (String path : expected) { - final byte[] content = generateBytes(); - if (path.endsWith("/")) { - blockingOp().createDir(String.format("%s/%s", parent, path)); - } else { - blockingOp().write(String.format("%s/%s", parent, path), content); - } - } - - final OpListArgs args = new OpListArgs(parent + "/x/"); - args.setDelimiter(""); - final List list = blockingOp().list(args); - final Set paths = list.stream().map(Entry::getPath).collect(Collectors.toSet()); - - assertTrue(paths.contains(parent + "/x/y")); - assertTrue(paths.contains(parent + "/x/x/y")); - assertTrue(paths.contains(parent + "/x/x/x/y")); - - blockingOp().removeAll(parent + "/"); - } - /** * Remove all should remove all in this path. */ From 30941bc5e837b9bc9164b3a97500974c08fb95fa Mon Sep 17 00:00:00 2001 From: G-XD Date: Mon, 23 Oct 2023 19:45:48 +0800 Subject: [PATCH 16/20] refactor(binding/java): align the pattern of convert list --- .../main/java/org/apache/opendal/OpenDAL.java | 6 +++--- bindings/java/src/utility.rs | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java b/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java index 3c7d358557b..93eac878e52 100644 --- a/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java +++ b/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java @@ -19,10 +19,10 @@ package org.apache.opendal; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import lombok.experimental.UtilityClass; @@ -33,7 +33,7 @@ public class OpenDAL { static { NativeLibrary.loadLibrary(); - final Set enabledServices = new HashSet<>(Arrays.asList(loadEnabledServices())); + final Set enabledServices = new HashSet<>(loadEnabledServices()); ENABLED_SERVICES = Collections.unmodifiableSet(enabledServices); } @@ -43,5 +43,5 @@ public static Collection enabledServices() { return ENABLED_SERVICES; } - private static native String[] loadEnabledServices(); + private static native List loadEnabledServices(); } diff --git a/bindings/java/src/utility.rs b/bindings/java/src/utility.rs index eed9f62d30b..1eaad390972 100644 --- a/bindings/java/src/utility.rs +++ b/bindings/java/src/utility.rs @@ -16,7 +16,7 @@ // under the License. use jni::objects::{JClass, JObject}; -use jni::sys::{jobjectArray, jsize}; +use jni::sys::jobject; use jni::JNIEnv; use opendal::Scheme; @@ -29,21 +29,23 @@ use crate::{string_to_jstring, Result}; pub unsafe extern "system" fn Java_org_apache_opendal_OpenDAL_loadEnabledServices( mut env: JNIEnv, _: JClass, -) -> jobjectArray { +) -> jobject { intern_load_enabled_services(&mut env).unwrap_or_else(|e| { e.throw(&mut env); JObject::default().into_raw() }) } -fn intern_load_enabled_services(env: &mut JNIEnv) -> Result { +fn intern_load_enabled_services(env: &mut JNIEnv) -> Result { let services = Scheme::enabled(); - let res = env.new_object_array(services.len() as jsize, "java/lang/String", JObject::null())?; - for (idx, service) in services.iter().enumerate() { + let list = env.new_object("java/util/ArrayList", "()V", &[])?; + let jlist = env.get_list(&list)?; + + for service in services { let srv = string_to_jstring(env, Some(&service.to_string()))?; - env.set_object_array_element(&res, idx as jsize, srv)?; + jlist.add(env, &srv)?; } - Ok(res.into_raw()) + Ok(list.into_raw()) } From 8e0374c73129037059a7d9d7bc4b777252b55477 Mon Sep 17 00:00:00 2001 From: G-XD Date: Mon, 23 Oct 2023 23:57:59 +0800 Subject: [PATCH 17/20] refactor(binding/java): align the pattern of convert list --- bindings/java/src/blocking_operator.rs | 21 ++++++++++++------- .../org/apache/opendal/BlockingOperator.java | 6 ++++-- .../main/java/org/apache/opendal/OpenDAL.java | 6 +++--- .../java/org/apache/opendal/Operator.java | 14 ++++++++++++- bindings/java/src/operator.rs | 18 +++++++++------- bindings/java/src/utility.rs | 16 +++++++------- 6 files changed, 51 insertions(+), 30 deletions(-) diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index d5675c84670..f1883981663 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -20,6 +20,8 @@ use jni::objects::JClass; use jni::objects::JObject; use jni::objects::JString; use jni::sys::jobject; +use jni::sys::jobjectArray; +use jni::sys::jsize; use jni::sys::{jbyteArray, jlong}; use jni::JNIEnv; @@ -254,24 +256,27 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_list( _: JClass, op: *mut BlockingOperator, path: JString, -) -> jobject { +) -> jobjectArray { intern_list(&mut env, &mut *op, path).unwrap_or_else(|e| { e.throw(&mut env); JObject::default().into_raw() }) } -fn intern_list(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result { +fn intern_list(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Result { let path = jstring_to_string(env, &path)?; let obs = op.list(&path)?; - let list = env.new_object("java/util/ArrayList", "()V", &[])?; - let jlist = env.get_list(&list)?; + let jarray = env.new_object_array( + obs.len() as jsize, + "org/apache/opendal/Entry", + JObject::null(), + )?; - for entry in obs { - let entry = make_entry(env, entry, Metakey::Mode.into())?; - jlist.add(env, &entry)?; + for (idx, entry) in obs.iter().enumerate() { + let entry = make_entry(env, entry.to_owned(), Metakey::Mode.into())?; + env.set_object_array_element(&jarray, idx as jsize, entry)?; } - Ok(list.into_raw()) + Ok(jarray.into_raw()) } 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 f0b0a5facc5..7d8cd041c03 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -20,6 +20,8 @@ package org.apache.opendal; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -98,7 +100,7 @@ public void removeAll(String path) { } public List list(String path) { - return list(nativeHandle, path); + return new ArrayList<>(Arrays.asList(list(nativeHandle, path))); } @Override @@ -122,5 +124,5 @@ public List list(String path) { private static native void removeAll(long nativeHandle, String path); - private static native List list(long nativeHandle, String path); + private static native Entry[] list(long nativeHandle, String path); } diff --git a/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java b/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java index 93eac878e52..3c7d358557b 100644 --- a/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java +++ b/bindings/java/src/main/java/org/apache/opendal/OpenDAL.java @@ -19,10 +19,10 @@ package org.apache.opendal; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; import lombok.experimental.UtilityClass; @@ -33,7 +33,7 @@ public class OpenDAL { static { NativeLibrary.loadLibrary(); - final Set enabledServices = new HashSet<>(loadEnabledServices()); + final Set enabledServices = new HashSet<>(Arrays.asList(loadEnabledServices())); ENABLED_SERVICES = Collections.unmodifiableSet(enabledServices); } @@ -43,5 +43,5 @@ public static Collection enabledServices() { return ENABLED_SERVICES; } - private static native List loadEnabledServices(); + private static native String[] loadEnabledServices(); } 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 8c6ecfa0210..e20365f5ab8 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -21,6 +21,9 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.UUID; @@ -213,7 +216,16 @@ public CompletableFuture removeAll(String path) { public CompletableFuture> list(String path) { final long requestid = list(nativeHandle, path); - return AsyncRegistry.take(requestid); + final CompletableFuture result = AsyncRegistry.take(requestid); + if (result != null) { + return result.thenApplyAsync(e -> { + if (e == null) { + return Collections.emptyList(); + } + return new ArrayList<>(Arrays.asList(e)); + }); + } + return null; } @Override diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 694533d6e2b..8f8217a169b 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -24,6 +24,7 @@ use jni::objects::JObject; use jni::objects::JString; use jni::objects::JValue; use jni::objects::JValueOwned; +use jni::sys::jsize; use jni::sys::{jlong, jobject}; use jni::JNIEnv; use opendal::layers::BlockingLayer; @@ -498,15 +499,18 @@ async fn do_list<'local>(op: &mut Operator, path: String) -> Result jobject { +) -> jobjectArray { intern_load_enabled_services(&mut env).unwrap_or_else(|e| { e.throw(&mut env); JObject::default().into_raw() }) } -fn intern_load_enabled_services(env: &mut JNIEnv) -> Result { +fn intern_load_enabled_services(env: &mut JNIEnv) -> Result { let services = Scheme::enabled(); + let res = env.new_object_array(services.len() as jsize, "java/lang/String", JObject::null())?; - let list = env.new_object("java/util/ArrayList", "()V", &[])?; - let jlist = env.get_list(&list)?; - - for service in services { + for (idx, service) in services.iter().enumerate() { let srv = string_to_jstring(env, Some(&service.to_string()))?; - jlist.add(env, &srv)?; + env.set_object_array_element(&res, idx as jsize, srv)?; } - Ok(list.into_raw()) + Ok(res.into_raw()) } From 8c43cc11984ad546f90e733268ddbaf0995a8499 Mon Sep 17 00:00:00 2001 From: G-XD Date: Wed, 25 Oct 2023 23:48:46 +0800 Subject: [PATCH 18/20] refactor(binding/java): use Metadata::metakey() api for make entry --- Cargo.lock | 1 - bindings/java/Cargo.toml | 1 - bindings/java/src/blocking_operator.rs | 5 +- bindings/java/src/lib.rs | 107 ++++++++---------- .../org/apache/opendal/BlockingOperator.java | 3 +- .../java/org/apache/opendal/Operator.java | 14 +-- bindings/java/src/operator.rs | 5 +- 7 files changed, 56 insertions(+), 80 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4223fb71401..2466fc2c5c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4342,7 +4342,6 @@ name = "opendal-java" version = "0.41.0" dependencies = [ "anyhow", - "flagset", "jni", "num_cpus", "once_cell", diff --git a/bindings/java/Cargo.toml b/bindings/java/Cargo.toml index 82b8a4bd841..df1ca7e66dd 100644 --- a/bindings/java/Cargo.toml +++ b/bindings/java/Cargo.toml @@ -135,7 +135,6 @@ anyhow = "1.0.71" jni = "0.21.1" num_cpus = "1.15.0" once_cell = "1.17.1" -flagset = "0.4" tokio = { version = "1.28.1", features = ["full"] } opendal = { workspace = true } diff --git a/bindings/java/src/blocking_operator.rs b/bindings/java/src/blocking_operator.rs index f1883981663..4713e8894f7 100644 --- a/bindings/java/src/blocking_operator.rs +++ b/bindings/java/src/blocking_operator.rs @@ -26,7 +26,6 @@ use jni::sys::{jbyteArray, jlong}; use jni::JNIEnv; use opendal::BlockingOperator; -use opendal::Metakey; use crate::jstring_to_string; use crate::make_entry; @@ -127,7 +126,7 @@ pub unsafe extern "system" fn Java_org_apache_opendal_BlockingOperator_stat( 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(make_metadata(env, metadata, Metakey::Complete.into())?.into_raw()) + Ok(make_metadata(env, metadata)?.into_raw()) } /// # Safety @@ -274,7 +273,7 @@ fn intern_list(env: &mut JNIEnv, op: &mut BlockingOperator, path: JString) -> Re )?; for (idx, entry) in obs.iter().enumerate() { - let entry = make_entry(env, entry.to_owned(), Metakey::Mode.into())?; + let entry = make_entry(env, entry.to_owned())?; env.set_object_array_element(&jarray, idx as jsize, entry)?; } diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index 6e30fa15ca6..a70642ae8d5 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -20,7 +20,6 @@ use std::collections::HashMap; use std::ffi::c_void; use crate::error::Error; -use flagset::FlagSet; use jni::objects::JObject; use jni::objects::JString; use jni::objects::{JMap, JValue}; @@ -229,73 +228,67 @@ fn make_capability<'a>(env: &mut JNIEnv<'a>, cap: Capability) -> Result( - env: &mut JNIEnv<'a>, - metadata: Metadata, - metakey: FlagSet, -) -> Result> { +fn make_metadata<'a>(env: &mut JNIEnv<'a>, metadata: Metadata) -> Result> { let mode = match metadata.mode() { EntryMode::FILE => 0, EntryMode::DIR => 1, EntryMode::Unknown => 2, }; - let last_modified = - if metakey.contains(Metakey::LastModified) || metakey.contains(Metakey::Complete) { - 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())], - )?) - }, - )? - } else { - JObject::null() - }; + let metakey = metadata.metakey(); - let cache_control = - if metakey.contains(Metakey::CacheControl) || metakey.contains(Metakey::Complete) { - string_to_jstring(env, metadata.cache_control())? - } else { - JObject::null() - }; - let content_disposition = - if metakey.contains(Metakey::ContentDisposition) || metakey.contains(Metakey::Complete) { - string_to_jstring(env, metadata.content_disposition())? - } else { - JObject::null() - }; - let content_md5 = - if metakey.contains(Metakey::ContentMd5) || metakey.contains(Metakey::Complete) { - string_to_jstring(env, metadata.content_md5())? - } else { - JObject::null() - }; - let content_type = - if metakey.contains(Metakey::ContentType) || metakey.contains(Metakey::Complete) { - string_to_jstring(env, metadata.content_type())? - } else { - JObject::null() - }; - let etag = if metakey.contains(Metakey::Etag) || metakey.contains(Metakey::Complete) { + let contains_metakey = |k| metakey.contains(k) || metakey.contains(Metakey::Complete); + + let last_modified = if contains_metakey(Metakey::LastModified) { + 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())], + )?) + }, + )? + } else { + JObject::null() + }; + + let cache_control = if contains_metakey(Metakey::CacheControl) { + string_to_jstring(env, metadata.cache_control())? + } else { + JObject::null() + }; + let content_disposition = if contains_metakey(Metakey::ContentDisposition) { + string_to_jstring(env, metadata.content_disposition())? + } else { + JObject::null() + }; + let content_md5 = if contains_metakey(Metakey::ContentMd5) { + string_to_jstring(env, metadata.content_md5())? + } else { + JObject::null() + }; + let content_type = if contains_metakey(Metakey::ContentType) { + string_to_jstring(env, metadata.content_type())? + } else { + JObject::null() + }; + let etag = if contains_metakey(Metakey::Etag) { string_to_jstring(env, metadata.etag())? } else { JObject::null() }; - let version = if metakey.contains(Metakey::Version) || metakey.contains(Metakey::Complete) { + let version = if contains_metakey(Metakey::Version) { string_to_jstring(env, metadata.version())? } else { JObject::null() }; - let content_length = - if metakey.contains(Metakey::ContentLength) || metakey.contains(Metakey::Complete) { - metadata.content_length() as jlong - } else { - -1 - }; + let content_length = if contains_metakey(Metakey::ContentLength) { + metadata.content_length() as jlong + } else { + -1 + }; let result = env .new_object( @@ -316,13 +309,9 @@ fn make_metadata<'a>( Ok(result) } -fn make_entry<'a>( - env: &mut JNIEnv<'a>, - entry: Entry, - metakey: FlagSet, -) -> Result> { +fn make_entry<'a>(env: &mut JNIEnv<'a>, entry: Entry) -> Result> { let path = env.new_string(entry.path())?; - let metadata = make_metadata(env, entry.metadata().to_owned(), metakey)?; + let metadata = make_metadata(env, entry.metadata().to_owned())?; Ok(env.new_object( "org/apache/opendal/Entry", 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 7d8cd041c03..276043a2871 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -20,7 +20,6 @@ package org.apache.opendal; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -100,7 +99,7 @@ public void removeAll(String path) { } public List list(String path) { - return new ArrayList<>(Arrays.asList(list(nativeHandle, path))); + return Arrays.asList(list(nativeHandle, path)); } @Override 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 e20365f5ab8..7b891c68357 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -21,11 +21,10 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; @@ -217,15 +216,8 @@ public CompletableFuture removeAll(String path) { public CompletableFuture> list(String path) { final long requestid = list(nativeHandle, path); final CompletableFuture result = AsyncRegistry.take(requestid); - if (result != null) { - return result.thenApplyAsync(e -> { - if (e == null) { - return Collections.emptyList(); - } - return new ArrayList<>(Arrays.asList(e)); - }); - } - return null; + Objects.requireNonNull(result); + return result.thenApplyAsync(Arrays::asList); } @Override diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 8f8217a169b..9b9b79982ce 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -29,7 +29,6 @@ use jni::sys::{jlong, jobject}; use jni::JNIEnv; use opendal::layers::BlockingLayer; use opendal::raw::PresignedRequest; -use opendal::Metakey; use opendal::Operator; use opendal::Scheme; @@ -207,7 +206,7 @@ fn intern_stat(env: &mut JNIEnv, op: *mut Operator, path: JString) -> Result(op: &mut Operator, path: String) -> Result> { let metadata = op.stat(&path).await?; let mut env = unsafe { get_current_env() }; - make_metadata(&mut env, metadata, Metakey::Complete.into()) + make_metadata(&mut env, metadata) } /// # Safety @@ -506,7 +505,7 @@ async fn do_list<'local>(op: &mut Operator, path: String) -> Result Date: Thu, 26 Oct 2023 00:13:17 +0800 Subject: [PATCH 19/20] ci(binding/java): remove behavior tests --- .github/workflows/bindings_java.yml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/.github/workflows/bindings_java.yml b/.github/workflows/bindings_java.yml index d465def6671..077f28627aa 100644 --- a/.github/workflows/bindings_java.yml +++ b/.github/workflows/bindings_java.yml @@ -81,15 +81,3 @@ jobs: run: | ./mvnw clean install -DskipTests ./mvnw verify artifact:compare - - name: Behavior tests - working-directory: bindings/java - shell: bash - run: | - export OPENDAL_TEST=memory - export OPENDAL_MEMORY_ROOT=/opendal - ./mvnw test -Dtest="behavior.*Test" - - export OPENDAL_TEST=fs - export OPENDAL_FS_ROOT=${{ runner.temp }}/ - ./mvnw test -Dtest="behavior.*Test" - From 67181481aacc5576aa7548e07b5805e01e8da4ef Mon Sep 17 00:00:00 2001 From: G-XD Date: Thu, 26 Oct 2023 00:14:00 +0800 Subject: [PATCH 20/20] refactor(binding/java): update code style --- bindings/java/src/main/java/org/apache/opendal/Operator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 7b891c68357..79f9799e5be 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -216,8 +216,7 @@ public CompletableFuture removeAll(String path) { public CompletableFuture> list(String path) { final long requestid = list(nativeHandle, path); final CompletableFuture result = AsyncRegistry.take(requestid); - Objects.requireNonNull(result); - return result.thenApplyAsync(Arrays::asList); + return Objects.requireNonNull(result).thenApplyAsync(Arrays::asList); } @Override