Xuanwo commented on code in PR #1894:
URL:
https://github.com/apache/incubator-opendal/pull/1894#discussion_r1161803444
##########
bindings/java/src/lib.rs:
##########
@@ -43,52 +44,34 @@ pub extern "system" fn
Java_org_apache_opendal_Operator_getOperator(
let map = convert_map(&mut env, ¶ms);
if let Ok(operator) = build_operator(scheme, map) {
- Box::into_raw(Box::new(operator)) as *const i32
+ Box::into_raw(Box::new(operator)) as jlong
} else {
env.exception_clear().expect("Couldn't clear exception");
env.throw_new(
"java/lang/IllegalArgumentException",
"Unsupported operator.",
)
.expect("Couldn't throw exception");
- std::ptr::null()
+ 0 as jlong
}
}
-fn convert_map(env: &mut JNIEnv, params: &JObject) -> HashMap<String, String> {
- let mut result: HashMap<String, String> = HashMap::new();
- let _ = JMap::from_env(env, params)
- .unwrap()
- .iter(env)
- .and_then(|mut iter| {
- while let Some(e) = iter.next(env)? {
- let key = JString::from(e.0);
- let value = JString::from(e.1);
- let key: String = env
- .get_string(&key)
- .expect("Couldn't get java string!")
- .into();
- let value: String = env
- .get_string(&value)
- .expect("Couldn't get java string!")
- .into();
- result.insert(key, value);
- }
- Ok(())
- });
- result
-}
-
/// # Safety
///
/// This function should not be called before the Operator are ready.
#[no_mangle]
pub unsafe extern "system" fn Java_org_apache_opendal_Operator_freeOperator(
mut _env: JNIEnv,
_class: JClass,
- ptr: *mut Operator,
+ _ptr: *mut Operator,
Review Comment:
please don't make arg as `_xxx` if we are using it.
##########
bindings/java/src/main/java/org/apache/opendal/Stat.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 io.questdb.jar.jni.JarJniLoader;
+
+public class Stat {
+
+ public static final String ORG_APACHE_OPENDAL_RUST_LIBS =
"/org/apache/opendal/rust/libs";
+
+ public static final String OPENDAL_JAVA = "opendal_java";
Review Comment:
What's this for?
##########
bindings/java/src/main/java/org/apache/opendal/Stat.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 io.questdb.jar.jni.JarJniLoader;
+
+public class Stat {
Review Comment:
Class should be named following the same style like opendal does. How about
using `Metadata`?
##########
bindings/java/src/main/java/org/apache/opendal/Stat.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 io.questdb.jar.jni.JarJniLoader;
+
+public class Stat {
+
+ public static final String ORG_APACHE_OPENDAL_RUST_LIBS =
"/org/apache/opendal/rust/libs";
+
+ public static final String OPENDAL_JAVA = "opendal_java";
+
+ long statPtr;
+ String fileName;
+
+ private native void freeStat(long statPtr);
+ private native boolean isFile(long statPtr);
+ private native long contentLength(long statPtr);
+
+ static {
+ JarJniLoader.loadLib(
+ Operator.class,
+ ORG_APACHE_OPENDAL_RUST_LIBS,
+ OPENDAL_JAVA);
+ }
+
+ public Stat(long statPtr, String fileName) {
Review Comment:
Please don't change opendal's public API.
##########
bindings/java/src/main/java/org/apache/opendal/Stat.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 io.questdb.jar.jni.JarJniLoader;
+
+public class Stat {
+
+ public static final String ORG_APACHE_OPENDAL_RUST_LIBS =
"/org/apache/opendal/rust/libs";
+
+ public static final String OPENDAL_JAVA = "opendal_java";
+
+ long statPtr;
+ String fileName;
+
+ private native void freeStat(long statPtr);
+ private native boolean isFile(long statPtr);
+ private native long contentLength(long statPtr);
Review Comment:
How about naming as `getContentLength`?
##########
bindings/java/src/main/java/org/apache/opendal/Stat.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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 io.questdb.jar.jni.JarJniLoader;
+
+public class Stat {
+
+ public static final String ORG_APACHE_OPENDAL_RUST_LIBS =
"/org/apache/opendal/rust/libs";
+
+ public static final String OPENDAL_JAVA = "opendal_java";
+
+ long statPtr;
+ String fileName;
Review Comment:
Metadata should not store filename.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
