From d089b799510dde6fb32e9088a810ddc15e2bc6cc Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 26 Oct 2023 20:55:18 +0800 Subject: [PATCH 01/11] feat(bindings/java): support consturct ops with native layers Signed-off-by: tison --- .../org/apache/opendal/BlockingOperator.java | 17 ++++++++--- .../java/org/apache/opendal/Operator.java | 17 +++++++++-- .../apache/opendal/layer/NativeLayerSpec.java | 11 +++++++ .../opendal/layer/RetryNativeLayerSpec.java | 29 +++++++++++++++++++ 4 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java create mode 100644 bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.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 276043a2871d..2e0bb37e0a03 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -21,8 +21,10 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; +import org.apache.opendal.layer.NativeLayerSpec; /** * BlockingOperator represents an underneath OpenDAL operator that @@ -31,6 +33,13 @@ public class BlockingOperator extends NativeObject { public final OperatorInfo info; + /** + * @see #of(String, Map, List) + */ + public static BlockingOperator of(String schema, Map map) { + return of(schema, map, Collections.emptyList()); + } + /** * Construct an OpenDAL blocking operator: * @@ -39,10 +48,11 @@ public class BlockingOperator extends NativeObject { * and see what config options each service supports. * * @param schema the name of the underneath service to access data from. - * @param map a map of properties to construct the underneath operator. + * @param map a map of properties to construct the underneath operator. + * @param specs a list of native layer specs to construct layers onto the op. */ - public static BlockingOperator of(String schema, Map map) { - try (final Operator operator = Operator.of(schema, map)) { + public static BlockingOperator of(String schema, Map map, List specs) { + try (final Operator operator = Operator.of(schema, map, specs)) { return operator.blocking(); } } @@ -54,7 +64,6 @@ public static BlockingOperator of(String schema, Map map) { /** * @return the cloned blocking operator. - * * @see Operator#duplicate() */ public BlockingOperator duplicate() { 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 79f9799e5be1..b962a7874ea9 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -22,12 +22,14 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; 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; +import org.apache.opendal.layer.NativeLayerSpec; /** * Operator represents an underneath OpenDAL operator that @@ -104,6 +106,14 @@ private static CompletableFuture take(long requestId) { public final OperatorInfo info; + + /** + * @see #of(String, Map, List) + */ + public static Operator of(String schema, Map map) { + return of(schema, map, Collections.emptyList()); + } + /** * Construct an OpenDAL operator: * @@ -113,9 +123,10 @@ private static CompletableFuture take(long requestId) { * * @param schema the name of the underneath service to access data from. * @param map a map of properties to construct the underneath operator. + * @param specs a list of native layer specs to construct layers onto the op. */ - public static Operator of(String schema, Map map) { - final long nativeHandle = constructor(schema, map); + public static Operator of(String schema, Map map, List specs) { + final long nativeHandle = constructor(schema, map, specs.toArray(new NativeLayerSpec[0])); final OperatorInfo info = makeOperatorInfo(nativeHandle); return new Operator(nativeHandle, info); } @@ -224,7 +235,7 @@ public CompletableFuture> list(String path) { private static native long duplicate(long nativeHandle); - private static native long constructor(String schema, Map map); + private static native long constructor(String schema, Map map, NativeLayerSpec[] specs); private static native long read(long nativeHandle, String path); diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java new file mode 100644 index 000000000000..36f0104061fe --- /dev/null +++ b/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java @@ -0,0 +1,11 @@ +package org.apache.opendal.layer; + +public abstract class NativeLayerSpec { + + /** + * This method is called from native code. It returns the pointer of the constructed native layer, + * which is immediately used (moved) to make the operator. + */ + @SuppressWarnings("unused") + protected abstract long makeNativeLayer(); +} diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java new file mode 100644 index 000000000000..de91d44b14bd --- /dev/null +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java @@ -0,0 +1,29 @@ +package org.apache.opendal.layer; + +import java.time.Duration; +import lombok.Builder; + +@Builder +public class RetryNativeLayerSpec extends NativeLayerSpec { + + private final boolean jitter; + + @Builder.Default + private final float factor = 2; + + @Builder.Default + private final Duration minDelay = Duration.ofSeconds(1); + + @Builder.Default + private final Duration maxDelay = Duration.ofSeconds(60); + + @Builder.Default + private final long maxTimes = 3; + + @Override + protected long makeNativeLayer() { + return makeNativeLayer(jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); + } + + private static native long makeNativeLayer(boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); +} From 73a11647b568d50cbdb03bed8bf58eb6f4b1f37a Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 26 Oct 2023 21:33:31 +0800 Subject: [PATCH 02/11] impl the native part Signed-off-by: tison --- bindings/java/src/layer.rs | 60 +++++++++++++++++++++++++++++++++++ bindings/java/src/lib.rs | 1 + bindings/java/src/operator.rs | 27 ++++++++++++++-- 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 bindings/java/src/layer.rs diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs new file mode 100644 index 000000000000..79ef71e8e952 --- /dev/null +++ b/bindings/java/src/layer.rs @@ -0,0 +1,60 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::time::Duration; + +use jni::objects::JClass; +use jni::sys::{jboolean, jfloat, jlong}; +use jni::JNIEnv; + +use opendal::layers::RetryLayer; +use opendal::raw::{FusedAccessor, Layer}; + +pub(crate) enum NativeLayer { + Retry(RetryLayer), +} + +impl NativeLayer { + pub(crate) fn into_inner(self) -> impl Layer { + match self { + NativeLayer::Retry(layer) => layer, + } + } +} + +#[no_mangle] +pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayerSpec_makeNativeLayer( + _: JNIEnv, + _: JClass, + jitter: jboolean, + factor: jfloat, + min_delay: jlong, + max_delay: jlong, + max_times: jlong, +) -> jlong { + let mut retry = RetryLayer::new(); + retry = retry.with_factor(factor); + retry = retry.with_min_delay(Duration::from_nanos(min_delay as u64)); + retry = retry.with_max_delay(Duration::from_nanos(max_delay as u64)); + if jitter != 0 { + retry = retry.with_jitter() + } + if max_times >= 0 { + retry = retry.with_max_times(max_times as usize); + } + Box::into_raw(Box::new(NativeLayer::Retry(retry))) as jlong +} diff --git a/bindings/java/src/lib.rs b/bindings/java/src/lib.rs index f472e7a5386e..d375d729cd74 100644 --- a/bindings/java/src/lib.rs +++ b/bindings/java/src/lib.rs @@ -41,6 +41,7 @@ use tokio::runtime::Runtime; mod blocking_operator; mod convert; mod error; +mod layer; mod operator; mod utility; diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 293ca4b33cf3..727b0190aca9 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -18,15 +18,16 @@ use std::str::FromStr; use std::time::Duration; -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::objects::{JByteArray, JObjectArray}; use jni::sys::jsize; use jni::sys::{jlong, jobject}; use jni::JNIEnv; + use opendal::layers::BlockingLayer; use opendal::raw::PresignedRequest; use opendal::Operator; @@ -36,6 +37,7 @@ use crate::convert::jmap_to_hashmap; use crate::convert::jstring_to_string; use crate::get_current_env; use crate::get_global_runtime; +use crate::layer::NativeLayer; use crate::make_entry; use crate::make_metadata; use crate::make_operator_info; @@ -48,21 +50,40 @@ pub extern "system" fn Java_org_apache_opendal_Operator_constructor( _: JClass, scheme: JString, map: JObject, + specs: JObjectArray, ) -> jlong { - intern_constructor(&mut env, scheme, map).unwrap_or_else(|e| { + intern_constructor(&mut env, scheme, map, specs).unwrap_or_else(|e| { e.throw(&mut env); 0 }) } -fn intern_constructor(env: &mut JNIEnv, scheme: JString, map: JObject) -> Result { +fn intern_constructor( + env: &mut JNIEnv, + scheme: JString, + map: JObject, + specs: JObjectArray, +) -> Result { let scheme = Scheme::from_str(jstring_to_string(env, &scheme)?.as_str())?; let map = jmap_to_hashmap(env, &map)?; + let mut op = Operator::via_map(scheme, map)?; + + // auto add the blocking layer on demand if !op.info().full_capability().blocking { let _guard = unsafe { get_global_runtime() }.enter(); op = op.layer(BlockingLayer::create()?); } + + // add user-specified layers + let specs_len = env.get_array_length(&specs)?; + for i in 0..specs_len { + let spec = env.get_object_array_element(&specs, i)?; + let layer = env.call_method(&spec, "makeNativeLayer", "()J", &[])?.j()?; + let layer = unsafe { Box::from_raw(layer as *mut NativeLayer) }; + op = op.layer(layer.into_inner()); + } + Ok(Box::into_raw(Box::new(op)) as jlong) } From 2b380609c5453b006886640ad2fdc657124dfc04 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 26 Oct 2023 21:33:58 +0800 Subject: [PATCH 03/11] license header Signed-off-by: tison --- .../apache/opendal/layer/NativeLayerSpec.java | 19 +++++++++++++++++++ .../opendal/layer/RetryNativeLayerSpec.java | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java index 36f0104061fe..b867ec653430 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java @@ -1,3 +1,22 @@ +/* + * 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.layer; public abstract class NativeLayerSpec { diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java index de91d44b14bd..9e0ef82b75e6 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java @@ -1,3 +1,22 @@ +/* + * 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.layer; import java.time.Duration; From c88c24c6cc46da4ea21700391a7c99ca96cf0a0f Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 26 Oct 2023 21:39:37 +0800 Subject: [PATCH 04/11] add a simple consturct test Signed-off-by: tison --- .../apache/opendal/test/NativeLayerTest.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java diff --git a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java new file mode 100644 index 000000000000..a97c2e4058b0 --- /dev/null +++ b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.opendal.test; + +import static org.assertj.core.api.Assertions.assertThat; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.opendal.Operator; +import org.apache.opendal.layer.NativeLayerSpec; +import org.apache.opendal.layer.RetryNativeLayerSpec; +import org.junit.jupiter.api.Test; + +public class NativeLayerTest { + @Test + void testOperatorWithRetryLayer() { + final Map conf = new HashMap<>(); + conf.put("root", "/opendal/"); + final NativeLayerSpec retryLayerSpec = RetryNativeLayerSpec.builder().build(); + final List nativeLayerSpecs = Collections.singletonList(retryLayerSpec); + try (final Operator op = Operator.of("memory", conf, nativeLayerSpecs)) { + assertThat(op.info).isNotNull(); + } + } +} From 10e218035ac3bb27a7bf682c4800bc8d022d210e Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 26 Oct 2023 21:43:29 +0800 Subject: [PATCH 05/11] spotless Signed-off-by: tison --- bindings/java/src/main/java/org/apache/opendal/Operator.java | 1 - .../java/org/apache/opendal/layer/RetryNativeLayerSpec.java | 3 ++- 2 files changed, 2 insertions(+), 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 b962a7874ea9..0c2788dae78b 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -106,7 +106,6 @@ private static CompletableFuture take(long requestId) { public final OperatorInfo info; - /** * @see #of(String, Map, List) */ diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java index 9e0ef82b75e6..8f10132fb0c2 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java @@ -44,5 +44,6 @@ protected long makeNativeLayer() { return makeNativeLayer(jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); } - private static native long makeNativeLayer(boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); + private static native long makeNativeLayer( + boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); } From 9ad0e41cee1beacddf709bfdd1268ee365d8eeaa Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 27 Oct 2023 00:30:27 +0800 Subject: [PATCH 06/11] change API Signed-off-by: tison --- bindings/java/src/layer.rs | 20 ++++---------- .../org/apache/opendal/BlockingOperator.java | 7 +++-- .../NativeLayerSpec.java => NativeLayer.java} | 12 +++------ .../java/org/apache/opendal/Operator.java | 23 ++++++++++------ ...veLayerSpec.java => RetryNativeLayer.java} | 11 ++++---- bindings/java/src/operator.rs | 26 +++---------------- .../apache/opendal/test/NativeLayerTest.java | 8 +++--- 7 files changed, 39 insertions(+), 68 deletions(-) rename bindings/java/src/main/java/org/apache/opendal/{layer/NativeLayerSpec.java => NativeLayer.java} (70%) rename bindings/java/src/main/java/org/apache/opendal/layer/{RetryNativeLayerSpec.java => RetryNativeLayer.java} (77%) diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs index 79ef71e8e952..653f39b461ed 100644 --- a/bindings/java/src/layer.rs +++ b/bindings/java/src/layer.rs @@ -22,30 +22,20 @@ use jni::sys::{jboolean, jfloat, jlong}; use jni::JNIEnv; use opendal::layers::RetryLayer; -use opendal::raw::{FusedAccessor, Layer}; - -pub(crate) enum NativeLayer { - Retry(RetryLayer), -} - -impl NativeLayer { - pub(crate) fn into_inner(self) -> impl Layer { - match self { - NativeLayer::Retry(layer) => layer, - } - } -} +use opendal::Operator; #[no_mangle] -pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayerSpec_makeNativeLayer( +pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( _: JNIEnv, _: JClass, + op: *mut Operator, jitter: jboolean, factor: jfloat, min_delay: jlong, max_delay: jlong, max_times: jlong, ) -> jlong { + let op = unsafe { Box::from_raw(op) }; let mut retry = RetryLayer::new(); retry = retry.with_factor(factor); retry = retry.with_min_delay(Duration::from_nanos(min_delay as u64)); @@ -56,5 +46,5 @@ pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayerSpec_makeNa if max_times >= 0 { retry = retry.with_max_times(max_times as usize); } - Box::into_raw(Box::new(NativeLayer::Retry(retry))) as jlong + Box::into_raw(Box::new(op.layer(retry))) as jlong } 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 2e0bb37e0a03..28c32d9caf34 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -24,7 +24,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import org.apache.opendal.layer.NativeLayerSpec; /** * BlockingOperator represents an underneath OpenDAL operator that @@ -49,10 +48,10 @@ public static BlockingOperator of(String schema, Map map) { * * @param schema the name of the underneath service to access data from. * @param map a map of properties to construct the underneath operator. - * @param specs a list of native layer specs to construct layers onto the op. + * @param layers a list of native layer specs to construct layers onto the op. */ - public static BlockingOperator of(String schema, Map map, List specs) { - try (final Operator operator = Operator.of(schema, map, specs)) { + public static BlockingOperator of(String schema, Map map, List layers) { + try (final Operator operator = Operator.of(schema, map, layers)) { return operator.blocking(); } } diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java similarity index 70% rename from bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java rename to bindings/java/src/main/java/org/apache/opendal/NativeLayer.java index b867ec653430..32b22376ad28 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/NativeLayerSpec.java +++ b/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java @@ -17,14 +17,8 @@ * under the License. */ -package org.apache.opendal.layer; +package org.apache.opendal; -public abstract class NativeLayerSpec { - - /** - * This method is called from native code. It returns the pointer of the constructed native layer, - * which is immediately used (moved) to make the operator. - */ - @SuppressWarnings("unused") - protected abstract long makeNativeLayer(); +public abstract class NativeLayer { + protected abstract long layer(long operatorNativeHandle); } 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 0c2788dae78b..aa203a207acd 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -29,7 +29,6 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; -import org.apache.opendal.layer.NativeLayerSpec; /** * Operator represents an underneath OpenDAL operator that @@ -122,15 +121,23 @@ public static Operator of(String schema, Map map) { * * @param schema the name of the underneath service to access data from. * @param map a map of properties to construct the underneath operator. - * @param specs a list of native layer specs to construct layers onto the op. + * @param layers a list of native layer specs to construct layers onto the op. */ - public static Operator of(String schema, Map map, List specs) { - final long nativeHandle = constructor(schema, map, specs.toArray(new NativeLayerSpec[0])); - final OperatorInfo info = makeOperatorInfo(nativeHandle); - return new Operator(nativeHandle, info); + public static Operator of(String schema, Map map, List layers) { + final long nativeHandle = constructor(schema, map); + if (layers.isEmpty()) { + final OperatorInfo info = makeOperatorInfo(nativeHandle); + return new Operator(nativeHandle, info); + } else { + long op = nativeHandle; + for (NativeLayer layer : layers) { + op = layer.layer(op); + } + return new Operator(op, makeOperatorInfo(op)); + } } - Operator(long nativeHandle, OperatorInfo info) { + private Operator(long nativeHandle, OperatorInfo info) { super(nativeHandle); this.info = info; } @@ -234,7 +241,7 @@ public CompletableFuture> list(String path) { private static native long duplicate(long nativeHandle); - private static native long constructor(String schema, Map map, NativeLayerSpec[] specs); + private static native long constructor(String schema, Map map); private static native long read(long nativeHandle, String path); diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java similarity index 77% rename from bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java rename to bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java index 8f10132fb0c2..0cdbf86eeaa8 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayerSpec.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java @@ -21,9 +21,10 @@ import java.time.Duration; import lombok.Builder; +import org.apache.opendal.NativeLayer; @Builder -public class RetryNativeLayerSpec extends NativeLayerSpec { +public class RetryNativeLayer extends NativeLayer { private final boolean jitter; @@ -40,10 +41,10 @@ public class RetryNativeLayerSpec extends NativeLayerSpec { private final long maxTimes = 3; @Override - protected long makeNativeLayer() { - return makeNativeLayer(jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); + protected long layer(long op) { + return doLayer(op, jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); } - private static native long makeNativeLayer( - boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); + private static native long doLayer( + long nativeHandle, boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index 727b0190aca9..e4a13430b780 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -18,12 +18,12 @@ use std::str::FromStr; use std::time::Duration; +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::objects::{JByteArray, JObjectArray}; use jni::sys::jsize; use jni::sys::{jlong, jobject}; use jni::JNIEnv; @@ -37,7 +37,6 @@ use crate::convert::jmap_to_hashmap; use crate::convert::jstring_to_string; use crate::get_current_env; use crate::get_global_runtime; -use crate::layer::NativeLayer; use crate::make_entry; use crate::make_metadata; use crate::make_operator_info; @@ -50,40 +49,21 @@ pub extern "system" fn Java_org_apache_opendal_Operator_constructor( _: JClass, scheme: JString, map: JObject, - specs: JObjectArray, ) -> jlong { - intern_constructor(&mut env, scheme, map, specs).unwrap_or_else(|e| { + intern_constructor(&mut env, scheme, map).unwrap_or_else(|e| { e.throw(&mut env); 0 }) } -fn intern_constructor( - env: &mut JNIEnv, - scheme: JString, - map: JObject, - specs: JObjectArray, -) -> Result { +fn intern_constructor(env: &mut JNIEnv, scheme: JString, map: JObject) -> Result { let scheme = Scheme::from_str(jstring_to_string(env, &scheme)?.as_str())?; let map = jmap_to_hashmap(env, &map)?; - let mut op = Operator::via_map(scheme, map)?; - - // auto add the blocking layer on demand if !op.info().full_capability().blocking { let _guard = unsafe { get_global_runtime() }.enter(); op = op.layer(BlockingLayer::create()?); } - - // add user-specified layers - let specs_len = env.get_array_length(&specs)?; - for i in 0..specs_len { - let spec = env.get_object_array_element(&specs, i)?; - let layer = env.call_method(&spec, "makeNativeLayer", "()J", &[])?.j()?; - let layer = unsafe { Box::from_raw(layer as *mut NativeLayer) }; - op = op.layer(layer.into_inner()); - } - Ok(Box::into_raw(Box::new(op)) as jlong) } diff --git a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java index a97c2e4058b0..141409bdbc9e 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java @@ -24,9 +24,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.opendal.NativeLayer; import org.apache.opendal.Operator; -import org.apache.opendal.layer.NativeLayerSpec; -import org.apache.opendal.layer.RetryNativeLayerSpec; +import org.apache.opendal.layer.RetryNativeLayer; import org.junit.jupiter.api.Test; public class NativeLayerTest { @@ -34,8 +34,8 @@ public class NativeLayerTest { void testOperatorWithRetryLayer() { final Map conf = new HashMap<>(); conf.put("root", "/opendal/"); - final NativeLayerSpec retryLayerSpec = RetryNativeLayerSpec.builder().build(); - final List nativeLayerSpecs = Collections.singletonList(retryLayerSpec); + final NativeLayer retryLayerSpec = RetryNativeLayer.builder().build(); + final List nativeLayerSpecs = Collections.singletonList(retryLayerSpec); try (final Operator op = Operator.of("memory", conf, nativeLayerSpecs)) { assertThat(op.info).isNotNull(); } From b0ce5fc27ff4a535375b253c7b2cb3971864710d Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 27 Oct 2023 00:40:04 +0800 Subject: [PATCH 07/11] change API Signed-off-by: tison --- bindings/java/src/layer.rs | 4 +-- .../org/apache/opendal/BlockingOperator.java | 13 ++------- .../java/org/apache/opendal/Operator.java | 28 ++++++------------- .../apache/opendal/test/NativeLayerTest.java | 12 ++++---- .../test/behavior/BehaviorExtension.java | 8 ++++-- 5 files changed, 23 insertions(+), 42 deletions(-) diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs index 653f39b461ed..0010f33b571b 100644 --- a/bindings/java/src/layer.rs +++ b/bindings/java/src/layer.rs @@ -35,7 +35,7 @@ pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( max_delay: jlong, max_times: jlong, ) -> jlong { - let op = unsafe { Box::from_raw(op) }; + let op = unsafe { &*op }; let mut retry = RetryLayer::new(); retry = retry.with_factor(factor); retry = retry.with_min_delay(Duration::from_nanos(min_delay as u64)); @@ -46,5 +46,5 @@ pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( if max_times >= 0 { retry = retry.with_max_times(max_times as usize); } - Box::into_raw(Box::new(op.layer(retry))) as jlong + Box::into_raw(Box::new(op.clone().layer(retry))) as jlong } 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 28c32d9caf34..ffb0b6042055 100644 --- a/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java +++ b/bindings/java/src/main/java/org/apache/opendal/BlockingOperator.java @@ -21,7 +21,6 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -32,13 +31,6 @@ public class BlockingOperator extends NativeObject { public final OperatorInfo info; - /** - * @see #of(String, Map, List) - */ - public static BlockingOperator of(String schema, Map map) { - return of(schema, map, Collections.emptyList()); - } - /** * Construct an OpenDAL blocking operator: * @@ -48,10 +40,9 @@ public static BlockingOperator of(String schema, Map map) { * * @param schema the name of the underneath service to access data from. * @param map a map of properties to construct the underneath operator. - * @param layers a list of native layer specs to construct layers onto the op. */ - public static BlockingOperator of(String schema, Map map, List layers) { - try (final Operator operator = Operator.of(schema, map, layers)) { + public static BlockingOperator of(String schema, Map map) { + try (final Operator operator = Operator.of(schema, map)) { return operator.blocking(); } } 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 aa203a207acd..02fa246fa02c 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -22,7 +22,6 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -105,13 +104,6 @@ private static CompletableFuture take(long requestId) { public final OperatorInfo info; - /** - * @see #of(String, Map, List) - */ - public static Operator of(String schema, Map map) { - return of(schema, map, Collections.emptyList()); - } - /** * Construct an OpenDAL operator: * @@ -121,20 +113,11 @@ public static Operator of(String schema, Map map) { * * @param schema the name of the underneath service to access data from. * @param map a map of properties to construct the underneath operator. - * @param layers a list of native layer specs to construct layers onto the op. */ - public static Operator of(String schema, Map map, List layers) { + public static Operator of(String schema, Map map) { final long nativeHandle = constructor(schema, map); - if (layers.isEmpty()) { - final OperatorInfo info = makeOperatorInfo(nativeHandle); - return new Operator(nativeHandle, info); - } else { - long op = nativeHandle; - for (NativeLayer layer : layers) { - op = layer.layer(op); - } - return new Operator(op, makeOperatorInfo(op)); - } + final OperatorInfo info = makeOperatorInfo(nativeHandle); + return new Operator(nativeHandle, info); } private Operator(long nativeHandle, OperatorInfo info) { @@ -156,6 +139,11 @@ public Operator duplicate() { return new Operator(nativeHandle, this.info); } + public Operator layer(NativeLayer layer) { + final long nativeHandle = layer.layer(this.nativeHandle); + return new Operator(nativeHandle, makeOperatorInfo(nativeHandle)); + } + public BlockingOperator blocking() { final long nativeHandle = makeBlockingOp(this.nativeHandle); final OperatorInfo info = this.info; diff --git a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java index 141409bdbc9e..f3b21d8ead22 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java @@ -20,10 +20,9 @@ package org.apache.opendal.test; import static org.assertj.core.api.Assertions.assertThat; -import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; +import lombok.Cleanup; import org.apache.opendal.NativeLayer; import org.apache.opendal.Operator; import org.apache.opendal.layer.RetryNativeLayer; @@ -34,10 +33,9 @@ public class NativeLayerTest { void testOperatorWithRetryLayer() { final Map conf = new HashMap<>(); conf.put("root", "/opendal/"); - final NativeLayer retryLayerSpec = RetryNativeLayer.builder().build(); - final List nativeLayerSpecs = Collections.singletonList(retryLayerSpec); - try (final Operator op = Operator.of("memory", conf, nativeLayerSpecs)) { - assertThat(op.info).isNotNull(); - } + final NativeLayer retryLayer = RetryNativeLayer.builder().build(); + @Cleanup final Operator op = Operator.of("memory", conf); + @Cleanup final Operator layeredOp = op.layer(retryLayer); + assertThat(layeredOp.info).isNotNull(); } } diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java index 465fda7b208e..5ec33424b393 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java @@ -25,9 +25,11 @@ import java.io.StringWriter; import java.util.HashMap; import java.util.Map; +import lombok.Cleanup; import lombok.extern.slf4j.Slf4j; import org.apache.opendal.BlockingOperator; import org.apache.opendal.Operator; +import org.apache.opendal.layer.RetryNativeLayer; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -53,8 +55,10 @@ public void beforeAll(ExtensionContext context) { config.put(key.substring(prefix.length()), entry.getValue()); } } - this.operator = Operator.of(scheme, config); - this.blockingOperator = BlockingOperator.of(scheme, config); + + @Cleanup final Operator op = Operator.of(scheme, config); + this.operator = op.layer(RetryNativeLayer.builder().build()); + this.blockingOperator = this.operator.blocking(); this.testName = String.format("%s(%s)", context.getDisplayName(), scheme); log.info( From 62f5e271644d249915896d5fbb69f9f495c27169 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 28 Oct 2023 10:14:38 +0800 Subject: [PATCH 08/11] save Signed-off-by: tison --- bindings/java/src/layer.rs | 39 ++++++++++++++----- .../opendal/{NativeLayer.java => Layer.java} | 4 +- .../java/org/apache/opendal/NativeObject.java | 4 ++ .../java/org/apache/opendal/Operator.java | 5 +-- ...{RetryNativeLayer.java => RetryLayer.java} | 11 +++--- bindings/java/src/operator.rs | 27 +++++++++++++ .../{NativeLayerTest.java => LayerTest.java} | 8 ++-- .../test/behavior/BehaviorExtension.java | 4 +- 8 files changed, 77 insertions(+), 25 deletions(-) rename bindings/java/src/main/java/org/apache/opendal/{NativeLayer.java => Layer.java} (89%) rename bindings/java/src/main/java/org/apache/opendal/layer/{RetryNativeLayer.java => RetryLayer.java} (81%) rename bindings/java/src/test/java/org/apache/opendal/test/{NativeLayerTest.java => LayerTest.java} (87%) diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs index 0010f33b571b..0459cc69ffce 100644 --- a/bindings/java/src/layer.rs +++ b/bindings/java/src/layer.rs @@ -17,25 +17,45 @@ use std::time::Duration; -use jni::objects::JClass; -use jni::sys::{jboolean, jfloat, jlong}; +use jni::objects::{JClass, JObject}; +use jni::sys::{jboolean, jfloat, jlong, jobject}; use jni::JNIEnv; use opendal::layers::RetryLayer; -use opendal::Operator; + +use crate::operator::{get_operator, make_operator}; +use crate::Result; #[no_mangle] -pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( - _: JNIEnv, +pub extern "system" fn Java_org_apache_opendal_layer_RetryLayer_doLayer<'local>( + mut env: JNIEnv<'local>, _: JClass, - op: *mut Operator, + op: JObject<'local>, + jitter: jboolean, + factor: jfloat, + min_delay: jlong, + max_delay: jlong, + max_times: jlong, +) -> jobject { + intern_retry_layer( + &mut env, op, jitter, factor, min_delay, max_delay, max_times, + ) + .unwrap_or_else(|e| { + e.throw(&mut env); + JObject::default().into_raw() + }) +} + +fn intern_retry_layer<'local>( + env: &mut JNIEnv<'local>, + op: JObject<'local>, jitter: jboolean, factor: jfloat, min_delay: jlong, max_delay: jlong, max_times: jlong, -) -> jlong { - let op = unsafe { &*op }; +) -> Result { + let op = get_operator(env, op)?; let mut retry = RetryLayer::new(); retry = retry.with_factor(factor); retry = retry.with_min_delay(Duration::from_nanos(min_delay as u64)); @@ -46,5 +66,6 @@ pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( if max_times >= 0 { retry = retry.with_max_times(max_times as usize); } - Box::into_raw(Box::new(op.clone().layer(retry))) as jlong + let op = op.clone().layer(retry); + Ok(make_operator(env, op)?.into_raw()) } diff --git a/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java b/bindings/java/src/main/java/org/apache/opendal/Layer.java similarity index 89% rename from bindings/java/src/main/java/org/apache/opendal/NativeLayer.java rename to bindings/java/src/main/java/org/apache/opendal/Layer.java index 32b22376ad28..43e96e7b910d 100644 --- a/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java +++ b/bindings/java/src/main/java/org/apache/opendal/Layer.java @@ -19,6 +19,6 @@ package org.apache.opendal; -public abstract class NativeLayer { - protected abstract long layer(long operatorNativeHandle); +public abstract class Layer { + protected abstract Operator layer(Operator op); } diff --git a/bindings/java/src/main/java/org/apache/opendal/NativeObject.java b/bindings/java/src/main/java/org/apache/opendal/NativeObject.java index c3496ea248f2..7f1416367b90 100644 --- a/bindings/java/src/main/java/org/apache/opendal/NativeObject.java +++ b/bindings/java/src/main/java/org/apache/opendal/NativeObject.java @@ -69,6 +69,10 @@ public void close() { disposeInternal(nativeHandle); } + private long getNativeHandle() { + return nativeHandle; + } + /** * Deletes underlying native object pointer. * 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 02fa246fa02c..0bc0310b63d8 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -139,9 +139,8 @@ public Operator duplicate() { return new Operator(nativeHandle, this.info); } - public Operator layer(NativeLayer layer) { - final long nativeHandle = layer.layer(this.nativeHandle); - return new Operator(nativeHandle, makeOperatorInfo(nativeHandle)); + public Operator layer(Layer layer) { + return layer.layer(this); } public BlockingOperator blocking() { diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java similarity index 81% rename from bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java rename to bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java index 0cdbf86eeaa8..d328cecdb931 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java @@ -21,10 +21,11 @@ import java.time.Duration; import lombok.Builder; -import org.apache.opendal.NativeLayer; +import org.apache.opendal.Layer; +import org.apache.opendal.Operator; @Builder -public class RetryNativeLayer extends NativeLayer { +public class RetryLayer extends Layer { private final boolean jitter; @@ -41,10 +42,10 @@ public class RetryNativeLayer extends NativeLayer { private final long maxTimes = 3; @Override - protected long layer(long op) { + protected Operator layer(Operator op) { return doLayer(op, jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); } - private static native long doLayer( - long nativeHandle, boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); + private static native Operator doLayer( + Operator nativeHandle, boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index e4a13430b780..c45c982ad341 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -721,3 +721,30 @@ fn get_future<'local>(env: &mut JNIEnv<'local>, id: jlong) -> Result( + env: &mut JNIEnv<'local>, + op: JObject<'local>, +) -> Result<&'local mut Operator> { + let op = env + .call_method(op, "getNativeHandle", "()J", &[]) + .expect("get_operator_call_method") + .j() + .expect("get_operator_j()"); + Ok(unsafe { &mut *(op as *mut Operator) }) +} + +pub(crate) fn make_operator<'local>( + env: &mut JNIEnv<'local>, + op: Operator, +) -> Result> { + let info = make_operator_info(env, op.info())?; + Ok(env.new_object( + "org/apache/opendal/Operator", + "(JLorg/apache/opendal/OperatorInfo;)V", + &[ + JValue::Long(Box::into_raw(Box::new(op)) as jlong), + JValue::Object(&info), + ], + )?) +} diff --git a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java similarity index 87% rename from bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java rename to bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java index f3b21d8ead22..e99e0b257185 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java @@ -23,17 +23,17 @@ import java.util.HashMap; import java.util.Map; import lombok.Cleanup; -import org.apache.opendal.NativeLayer; +import org.apache.opendal.Layer; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryNativeLayer; +import org.apache.opendal.layer.RetryLayer; import org.junit.jupiter.api.Test; -public class NativeLayerTest { +public class LayerTest { @Test void testOperatorWithRetryLayer() { final Map conf = new HashMap<>(); conf.put("root", "/opendal/"); - final NativeLayer retryLayer = RetryNativeLayer.builder().build(); + final Layer retryLayer = RetryLayer.builder().build(); @Cleanup final Operator op = Operator.of("memory", conf); @Cleanup final Operator layeredOp = op.layer(retryLayer); assertThat(layeredOp.info).isNotNull(); diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java index 5ec33424b393..ae95e7344f7e 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java @@ -29,7 +29,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.opendal.BlockingOperator; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryNativeLayer; +import org.apache.opendal.layer.RetryLayer; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -57,7 +57,7 @@ public void beforeAll(ExtensionContext context) { } @Cleanup final Operator op = Operator.of(scheme, config); - this.operator = op.layer(RetryNativeLayer.builder().build()); + this.operator = op.layer(RetryLayer.builder().build()); this.blockingOperator = this.operator.blocking(); this.testName = String.format("%s(%s)", context.getDisplayName(), scheme); From 59105e70a9921ef8d8c8d3d55d996f6ea1f22fd2 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 2 Nov 2023 16:59:03 +0800 Subject: [PATCH 09/11] Revert "save" This reverts commit 62f5e271644d249915896d5fbb69f9f495c27169. --- bindings/java/src/layer.rs | 39 +++++-------------- .../opendal/{Layer.java => NativeLayer.java} | 4 +- .../java/org/apache/opendal/NativeObject.java | 4 -- .../java/org/apache/opendal/Operator.java | 5 ++- ...{RetryLayer.java => RetryNativeLayer.java} | 11 +++--- bindings/java/src/operator.rs | 27 ------------- .../{LayerTest.java => NativeLayerTest.java} | 8 ++-- .../test/behavior/BehaviorExtension.java | 4 +- 8 files changed, 25 insertions(+), 77 deletions(-) rename bindings/java/src/main/java/org/apache/opendal/{Layer.java => NativeLayer.java} (89%) rename bindings/java/src/main/java/org/apache/opendal/layer/{RetryLayer.java => RetryNativeLayer.java} (81%) rename bindings/java/src/test/java/org/apache/opendal/test/{LayerTest.java => NativeLayerTest.java} (87%) diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs index 0459cc69ffce..0010f33b571b 100644 --- a/bindings/java/src/layer.rs +++ b/bindings/java/src/layer.rs @@ -17,45 +17,25 @@ use std::time::Duration; -use jni::objects::{JClass, JObject}; -use jni::sys::{jboolean, jfloat, jlong, jobject}; +use jni::objects::JClass; +use jni::sys::{jboolean, jfloat, jlong}; use jni::JNIEnv; use opendal::layers::RetryLayer; - -use crate::operator::{get_operator, make_operator}; -use crate::Result; +use opendal::Operator; #[no_mangle] -pub extern "system" fn Java_org_apache_opendal_layer_RetryLayer_doLayer<'local>( - mut env: JNIEnv<'local>, +pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( + _: JNIEnv, _: JClass, - op: JObject<'local>, - jitter: jboolean, - factor: jfloat, - min_delay: jlong, - max_delay: jlong, - max_times: jlong, -) -> jobject { - intern_retry_layer( - &mut env, op, jitter, factor, min_delay, max_delay, max_times, - ) - .unwrap_or_else(|e| { - e.throw(&mut env); - JObject::default().into_raw() - }) -} - -fn intern_retry_layer<'local>( - env: &mut JNIEnv<'local>, - op: JObject<'local>, + op: *mut Operator, jitter: jboolean, factor: jfloat, min_delay: jlong, max_delay: jlong, max_times: jlong, -) -> Result { - let op = get_operator(env, op)?; +) -> jlong { + let op = unsafe { &*op }; let mut retry = RetryLayer::new(); retry = retry.with_factor(factor); retry = retry.with_min_delay(Duration::from_nanos(min_delay as u64)); @@ -66,6 +46,5 @@ fn intern_retry_layer<'local>( if max_times >= 0 { retry = retry.with_max_times(max_times as usize); } - let op = op.clone().layer(retry); - Ok(make_operator(env, op)?.into_raw()) + Box::into_raw(Box::new(op.clone().layer(retry))) as jlong } diff --git a/bindings/java/src/main/java/org/apache/opendal/Layer.java b/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java similarity index 89% rename from bindings/java/src/main/java/org/apache/opendal/Layer.java rename to bindings/java/src/main/java/org/apache/opendal/NativeLayer.java index 43e96e7b910d..32b22376ad28 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Layer.java +++ b/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java @@ -19,6 +19,6 @@ package org.apache.opendal; -public abstract class Layer { - protected abstract Operator layer(Operator op); +public abstract class NativeLayer { + protected abstract long layer(long operatorNativeHandle); } diff --git a/bindings/java/src/main/java/org/apache/opendal/NativeObject.java b/bindings/java/src/main/java/org/apache/opendal/NativeObject.java index 7f1416367b90..c3496ea248f2 100644 --- a/bindings/java/src/main/java/org/apache/opendal/NativeObject.java +++ b/bindings/java/src/main/java/org/apache/opendal/NativeObject.java @@ -69,10 +69,6 @@ public void close() { disposeInternal(nativeHandle); } - private long getNativeHandle() { - return nativeHandle; - } - /** * Deletes underlying native object pointer. * 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 0bc0310b63d8..02fa246fa02c 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -139,8 +139,9 @@ public Operator duplicate() { return new Operator(nativeHandle, this.info); } - public Operator layer(Layer layer) { - return layer.layer(this); + public Operator layer(NativeLayer layer) { + final long nativeHandle = layer.layer(this.nativeHandle); + return new Operator(nativeHandle, makeOperatorInfo(nativeHandle)); } public BlockingOperator blocking() { diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java similarity index 81% rename from bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java rename to bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java index d328cecdb931..0cdbf86eeaa8 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java @@ -21,11 +21,10 @@ import java.time.Duration; import lombok.Builder; -import org.apache.opendal.Layer; -import org.apache.opendal.Operator; +import org.apache.opendal.NativeLayer; @Builder -public class RetryLayer extends Layer { +public class RetryNativeLayer extends NativeLayer { private final boolean jitter; @@ -42,10 +41,10 @@ public class RetryLayer extends Layer { private final long maxTimes = 3; @Override - protected Operator layer(Operator op) { + protected long layer(long op) { return doLayer(op, jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); } - private static native Operator doLayer( - Operator nativeHandle, boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); + private static native long doLayer( + long nativeHandle, boolean jitter, float factor, long minDelay, long maxDelay, long maxTimes); } diff --git a/bindings/java/src/operator.rs b/bindings/java/src/operator.rs index c45c982ad341..e4a13430b780 100644 --- a/bindings/java/src/operator.rs +++ b/bindings/java/src/operator.rs @@ -721,30 +721,3 @@ fn get_future<'local>(env: &mut JNIEnv<'local>, id: jlong) -> Result( - env: &mut JNIEnv<'local>, - op: JObject<'local>, -) -> Result<&'local mut Operator> { - let op = env - .call_method(op, "getNativeHandle", "()J", &[]) - .expect("get_operator_call_method") - .j() - .expect("get_operator_j()"); - Ok(unsafe { &mut *(op as *mut Operator) }) -} - -pub(crate) fn make_operator<'local>( - env: &mut JNIEnv<'local>, - op: Operator, -) -> Result> { - let info = make_operator_info(env, op.info())?; - Ok(env.new_object( - "org/apache/opendal/Operator", - "(JLorg/apache/opendal/OperatorInfo;)V", - &[ - JValue::Long(Box::into_raw(Box::new(op)) as jlong), - JValue::Object(&info), - ], - )?) -} diff --git a/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java similarity index 87% rename from bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java rename to bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java index e99e0b257185..f3b21d8ead22 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java @@ -23,17 +23,17 @@ import java.util.HashMap; import java.util.Map; import lombok.Cleanup; -import org.apache.opendal.Layer; +import org.apache.opendal.NativeLayer; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryLayer; +import org.apache.opendal.layer.RetryNativeLayer; import org.junit.jupiter.api.Test; -public class LayerTest { +public class NativeLayerTest { @Test void testOperatorWithRetryLayer() { final Map conf = new HashMap<>(); conf.put("root", "/opendal/"); - final Layer retryLayer = RetryLayer.builder().build(); + final NativeLayer retryLayer = RetryNativeLayer.builder().build(); @Cleanup final Operator op = Operator.of("memory", conf); @Cleanup final Operator layeredOp = op.layer(retryLayer); assertThat(layeredOp.info).isNotNull(); diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java index ae95e7344f7e..5ec33424b393 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java @@ -29,7 +29,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.opendal.BlockingOperator; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryLayer; +import org.apache.opendal.layer.RetryNativeLayer; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -57,7 +57,7 @@ public void beforeAll(ExtensionContext context) { } @Cleanup final Operator op = Operator.of(scheme, config); - this.operator = op.layer(RetryLayer.builder().build()); + this.operator = op.layer(RetryNativeLayer.builder().build()); this.blockingOperator = this.operator.blocking(); this.testName = String.format("%s(%s)", context.getDisplayName(), scheme); From 58377ee791ea2be652881150f0a548b5473d5103 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 2 Nov 2023 17:00:32 +0800 Subject: [PATCH 10/11] rename Signed-off-by: tison --- .../org/apache/opendal/{NativeLayer.java => Layer.java} | 4 ++-- .../java/src/main/java/org/apache/opendal/Operator.java | 2 +- .../layer/{RetryNativeLayer.java => RetryLayer.java} | 8 ++++---- .../opendal/test/{NativeLayerTest.java => LayerTest.java} | 8 ++++---- .../apache/opendal/test/behavior/BehaviorExtension.java | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) rename bindings/java/src/main/java/org/apache/opendal/{NativeLayer.java => Layer.java} (89%) rename bindings/java/src/main/java/org/apache/opendal/layer/{RetryNativeLayer.java => RetryLayer.java} (86%) rename bindings/java/src/test/java/org/apache/opendal/test/{NativeLayerTest.java => LayerTest.java} (87%) diff --git a/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java b/bindings/java/src/main/java/org/apache/opendal/Layer.java similarity index 89% rename from bindings/java/src/main/java/org/apache/opendal/NativeLayer.java rename to bindings/java/src/main/java/org/apache/opendal/Layer.java index 32b22376ad28..061c928c3eae 100644 --- a/bindings/java/src/main/java/org/apache/opendal/NativeLayer.java +++ b/bindings/java/src/main/java/org/apache/opendal/Layer.java @@ -19,6 +19,6 @@ package org.apache.opendal; -public abstract class NativeLayer { - protected abstract long layer(long operatorNativeHandle); +public abstract class Layer { + protected abstract long layer(long nativeOp); } 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 02fa246fa02c..da0384475702 100644 --- a/bindings/java/src/main/java/org/apache/opendal/Operator.java +++ b/bindings/java/src/main/java/org/apache/opendal/Operator.java @@ -139,7 +139,7 @@ public Operator duplicate() { return new Operator(nativeHandle, this.info); } - public Operator layer(NativeLayer layer) { + public Operator layer(Layer layer) { final long nativeHandle = layer.layer(this.nativeHandle); return new Operator(nativeHandle, makeOperatorInfo(nativeHandle)); } diff --git a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java b/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java similarity index 86% rename from bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java rename to bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java index 0cdbf86eeaa8..86c3bc66ee11 100644 --- a/bindings/java/src/main/java/org/apache/opendal/layer/RetryNativeLayer.java +++ b/bindings/java/src/main/java/org/apache/opendal/layer/RetryLayer.java @@ -21,10 +21,10 @@ import java.time.Duration; import lombok.Builder; -import org.apache.opendal.NativeLayer; +import org.apache.opendal.Layer; @Builder -public class RetryNativeLayer extends NativeLayer { +public class RetryLayer extends Layer { private final boolean jitter; @@ -41,8 +41,8 @@ public class RetryNativeLayer extends NativeLayer { private final long maxTimes = 3; @Override - protected long layer(long op) { - return doLayer(op, jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); + protected long layer(long nativeOp) { + return doLayer(nativeOp, jitter, factor, minDelay.toNanos(), maxDelay.toNanos(), maxTimes); } private static native long doLayer( diff --git a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java b/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java similarity index 87% rename from bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java rename to bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java index f3b21d8ead22..e99e0b257185 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/NativeLayerTest.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/LayerTest.java @@ -23,17 +23,17 @@ import java.util.HashMap; import java.util.Map; import lombok.Cleanup; -import org.apache.opendal.NativeLayer; +import org.apache.opendal.Layer; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryNativeLayer; +import org.apache.opendal.layer.RetryLayer; import org.junit.jupiter.api.Test; -public class NativeLayerTest { +public class LayerTest { @Test void testOperatorWithRetryLayer() { final Map conf = new HashMap<>(); conf.put("root", "/opendal/"); - final NativeLayer retryLayer = RetryNativeLayer.builder().build(); + final Layer retryLayer = RetryLayer.builder().build(); @Cleanup final Operator op = Operator.of("memory", conf); @Cleanup final Operator layeredOp = op.layer(retryLayer); assertThat(layeredOp.info).isNotNull(); diff --git a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java index 5ec33424b393..ae95e7344f7e 100644 --- a/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java +++ b/bindings/java/src/test/java/org/apache/opendal/test/behavior/BehaviorExtension.java @@ -29,7 +29,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.opendal.BlockingOperator; import org.apache.opendal.Operator; -import org.apache.opendal.layer.RetryNativeLayer; +import org.apache.opendal.layer.RetryLayer; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -57,7 +57,7 @@ public void beforeAll(ExtensionContext context) { } @Cleanup final Operator op = Operator.of(scheme, config); - this.operator = op.layer(RetryNativeLayer.builder().build()); + this.operator = op.layer(RetryLayer.builder().build()); this.blockingOperator = this.operator.blocking(); this.testName = String.format("%s(%s)", context.getDisplayName(), scheme); From 1f41a5c295a839c9813cbb524badce87c32ae995 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 2 Nov 2023 19:42:00 +0800 Subject: [PATCH 11/11] fixup Signed-off-by: tison --- bindings/java/src/layer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/java/src/layer.rs b/bindings/java/src/layer.rs index 0010f33b571b..3706d77f53d5 100644 --- a/bindings/java/src/layer.rs +++ b/bindings/java/src/layer.rs @@ -25,7 +25,7 @@ use opendal::layers::RetryLayer; use opendal::Operator; #[no_mangle] -pub extern "system" fn Java_org_apache_opendal_layer_RetryNativeLayer_doLayer( +pub extern "system" fn Java_org_apache_opendal_layer_RetryLayer_doLayer( _: JNIEnv, _: JClass, op: *mut Operator,