Add conversions for JObject wrapper types

[uvlad7]

@giovanniberti I also added a fix for Box<[bool]> conversion here.

And I need your advice how to add tests for unchecked conversion too.

[uvlad7]

I implemented Object[] <=> Box<[JObject<'env>]> conversion, but I don't know how to make the following

       #[call_type(unchecked)]
        pub extern "jni" fn userArray() -> Box<[User<'env, 'borrow>]> {
            vec![].into_boxed_slice()
        }

work, because it's not possible to impl foreign trait for slices and it's also not possible to impl Signature for Box<[T]>.

@giovanniberti can you take a look, please? I really need this feature to implement some bindings (it actually doesn't matter if rust type is Box<[T]> or something else, but I need to be able to accept arrays as parameters and return them from rust)

UPD: I created conversions for one-dimensional arrays, but not for other types. I'm wondering how to impl TryFromJavaValue for structures like struct UserArray(Box<[User]>) where From<Box<[User]>> for UserArray is implemented. Obviously, in that case we can construct UserArray from java value, but I don't understand how to implement this.

UPD2: I created an example in the tests, but I think it'd be better to impl it with derive for such a wrapper types, and without constcat dependency and this additional ArrSignature trait.

[uvlad7]

@EliseChouleur So, I assume it's better to wait for #59, right?

[uvlad7]

@EliseChouleur something is really wrong with derived TryIntoJavaValue implementation, but I don't see what.

UPD: And it crashes in JavaVM::new(vm_args), but only when User::selfSignatureCheck call is present. May be this is related? But the problem reproduces on Microsoft Java (microsoft-21.0.1) too.

UPD2: JavaVM::new crashes when the debugger is attached, it doesn't depend on selfSignatureCheck, but ofk makes it much harder to debug. I'm on linux, but this seems relevant.

[giovanniberti]

Sorry @uvlad7, I did just see this PR! Right now the situation with the jni crate is tricky, as version 0.21 requires a bit of work for upgrading. I'll get to thiisk PR after the other PRs as this is still in draft. :)

[uvlad7]

@giovanniberti, so I fixed infinite recursion problem in [Try]IntoJavaValue, but only to face another issue with AutoLocals. I see workflows weren't executed, but as for the 22acb1e, tests fail with the following error:

thread 'vm_creation_and_object_usage' panicked at 'assertion failed: `(left == right)`
  left: `["User{username='user', password='password'}", "null", "[]", "[]"]`,
 right: `["User{username='user', password='password'}", "User{username='user', password='42'}", "[]", "[]"]`', tests/mod.rs:146:5

And after the "fix" in 849d892 works fine. Well, not actually, since if you try to pass non-empty vec or slice, they appear filled with nulls on the Java side (and cause a segfault without the fix, which is confusing, because it affects only first argument's conversion). Now I don't understand how to really fix that problem, all that local references and drop stuff is confusing, I'm not fluent in Rust's lifetimes and borrowing nor in Java local references.

as this is still in draft

With the problem above fixed it's ready for review. Wanna ask if that ArrSignature looks good to you (I don't like that solution actually, may be better add that stuff into the Signature itself, or find another way?) and discuss derive implementation for arrays, and how to reduce code duplication between safe and unchecked, but it can be done in separate MR(s), and MVP is already ready and useful.

[uvlad7]

I assume the problem is in the fact that user value is moved and then dropped by try_into - as long as the inner AutoLocal - before the actual env.call_method call.

the following works just fine

            let v: robusta_jni::jni::objects::JValue<'_> = ::std::convert::Into::into(
                <&User as ::robusta_jni::convert::TryIntoJavaValue>::try_into(
                    &user,
                    &env,
                )?,
            );

But AutoLocal was completely rewritten in 0.21.0, so may be we should put this on hold

JNI 0.21.0 Changelog says:

Trying to use an object reference after it has been deleted now causes a compile error instead of undefined behavior. As a result, it is now safe to use AutoLocal, JNIEnv::delete_local_ref, and `JNIEnv::with_local_fr...

So it covers this case

[uvlad7]

@giovanniberti , @EliseChouleur , take a look, please. There are still a lot of improvements to be added, but I assume it'd be better to merge this to fix bugs and to avoid conflicts in migration to jni-0.21, since this PR is really huge.

[giovanniberti]

Sorry for the very late review. (understatement of the century right there :) ) First of all: impressive work! You did manage to upgrade jni to 0.21, I tried that a while ago and gave up because of the API changes

Just a few comments:

1. There are a few commented lines here and there, I'd eliminate those if not needed
2. There are a couple new traits related to array handling but they don't seem to be documented, could you add a few lines on those?

Thank you again for all the effort!

[uvlad7]

Uh, sorry, I created a mess with branches, and unfortunately I didn't manage to update jni to 0.21.1, moved to another branch and gave up. This MR contains other useful fixes, though, and I can return it back into working condition - with jni 0.20 as a dependency - if you want to merge. I want to continue my efforts in migration to 0.21, but I won't have any time for that in the nearest future

[giovanniberti]

This MR contains other useful fixes, though, and I can return it back into working condition - with jni 0.20 as a dependency - if you want to merge.

Sure!

I want to continue my efforts in migration to 0.21, but I won't have any time for that in the nearest future

No problem, I myself have left this repo in a semi-abandoned state since last year — it's in the spirit of FOSS that people can contribute whenever they want and can :)

[uvlad7]

I reverted stuff back to jni 0.20.0 version, looks like it's ready to be merged.