refactor(java): refactor rust code to remove mostly unwrap and expect

[ShadowySpirits]

close #2208

[Xuanwo]

I don't like this PR because it adds too much complexity. Is it required to implement macro_rules in a binding?

[ShadowySpirits]

Otherwise, we need to write the following codes in all places where exceptions may occur:

let result = op.do_something();
if let Err(error) = result {
    let exception = build_java_exception();
    env.throw(exception).unwrap();
    return;
}
let result = ressult.unwrap();

With the macro, we can simplify them into:

let result = handle_opendal_result!(env, op.do_something());

[Xuanwo]

I prefer write them directly, maybe via map_err().

[ShadowySpirits]

I prefer write them directly, maybe via map_err().

The point is we can not return an error with ?:

let result = op.do_something().map_err()?;

But must throw an exception manually:

if let Err(error) = result {
    let exception = build_java_exception();
    env.throw(exception).unwrap();
    return;
}

Why not use the macro to avoid duplicated code?

[ShadowySpirits]

And this practice seems like a rust way: https://doc.rust-lang.org/rust-by-example/macros/dry.html

[Xuanwo]

The main issue is that Java uses exception differently from Rust's Result logic. To address this, we need to create a shim between the Rust and Java code. For example:

• xxx_inner() -> Result<T, Error>
• xxx() -> T: This would call xxx_inner() and raise an exception.

Additionally, why do we need to build exceptions in Rust code? Could we instead return a new object and raise an exception in Java?

j4rs returns a jobject for that:

#[call_from_java("io.github.astonbitecode.j4rs.example.RustFunctionCalls.addintegers")]
fn add_integers(integer_instance1: Instance, integer_instance2: Instance) -> Result<Instance, String> {
    let jvm: Jvm = Jvm::attach_thread().unwrap();
    let i1: i32 = jvm.to_rust(integer_instance1).unwrap();
    let i2: i32 = jvm.to_rust(integer_instance2).unwrap();
    let sum = i1 + i2;
    let ia = InvocationArg::try_from(sum).map_err(|error| format!("{}", error)).unwrap();
    Instance::try_from(ia).map_err(|error| format!("{}", error))
}

Underhood:

// The jni function return type
let ref jni_function_output = match &user_function_signature.output {
    ReturnType::Default => ReturnType::Default,
    _ => {
        let ret_type: ReturnType = syn::parse_str("-> jobject").unwrap();
        ret_type
    }
};
// The jni return value. This may be void or jobject
let return_value = match &user_function_signature.output {
    ReturnType::Default => {
        let ret_value: Expr = syn::parse_str("()").unwrap();
        ret_value
    }
    _ => {
        let ret_value: Expr = syn::parse_str(
            r#"match instance_to_return {
                Ok(i) => {
                    i.java_object()
                    // i.as_java_ptr_with_local_ref(jni_env).unwrap()
                },
                Err(error) => {
                    let message = format!("{}", error);
                    let _ = jvm.throw_invocation_exception(&message);
                    ptr::null_mut()
                },
            }"#).unwrap();
        ret_value
    }
};

---

Binding must be simple and easy to understand.

If we have to add those complex logic, I prefer to use j4rs directly.

[Xuanwo]

And this practice seems like a rust way: https://doc.rust-lang.org/rust-by-example/macros/dry.html

Most binding code doesn't change frequently, so it's better to optimize it for readability.

[ShadowySpirits]

To address this, we need to create a shim between the Rust and Java code. For example:
xxx_inner() -> Result<T, Error>
xxx() -> T: This would call xxx_inner() and raise an exception.

Ok, I'll do this to make the code clearer.

// generated by macros call_from_java
let ret_value: Expr = syn::parse_str(
    r#"match instance_to_return {
        Ok(i) => {
            i.java_object()
            // i.as_java_ptr_with_local_ref(jni_env).unwrap()
        },
        Err(error) => {
            let message = format!("{}", error);
            let _ = jvm.throw_invocation_exception(&message);
            ptr::null_mut()
        },
    }"#).unwrap();
ret_value

The j4rs way, to be honest, is exactly what I did.

[Xuanwo]

The j4rs way, to be honest, is exactly what I did.

You are right. But the difference from my point is we don't need (and don't want) to maintain the code itself.

[Xuanwo]

Hi, is this PR still valid?

[ShadowySpirits]

Hi, is this PR still valid?

Sorry, I'm kind of busy these days. I will finish it this weekend.

[Xuanwo]

Sorry, I'm kind of busy these days. I will finish it this weekend.

Take your time and have fun! There is no hurry.

[tisonkun]

The codebase has been changed a lot with #2276 #2288 #2291. The unwrap boundary is narrowed into three parts now:

1. When JNIOnLoad and thread start, I don't think we can handle the exception there.
2. When retrieving the thread local JNIEnv. It should be always success otherwise a fatal error. unwrap for simplifying the code.
3. When constructing exceptions in async callback - the current thread is a Rust thread, no result can be popped up.

Although, for case 3, we can still narrow a bit the error handling part to collect all exceptions before calling f.completeExceptionally to an exception to the future, or improving the log.

Closing this PR since any meaningful follow-up requires almost rewrite.

[tisonkun]

Sorry that I'm unaware of this PR in the first place. There are still improvements can be done for the java bindings.