Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing JS Arrays to Java object methods seem always to be returned as TruffleMap #3

Closed
pmlopes opened this issue May 4, 2018 · 12 comments
Assignees

Comments

@pmlopes
Copy link

pmlopes commented May 4, 2018

Imagine the following Java class:

public class CallMe {
  public void call(Object name) {
    System.out.println(name);
    System.out.println(name.getClass());
  }
}

Bootstrap the JS engine and add an instance of this class to the context bindings:

Context ctx = Context.newBuilder("js").allowAllAccess(true).build();
ctx.getBindings("js").putMember("cb", new CallMe());

Now when working with this interop between JS and java I observe the following when passing a JS Object:

ctx.eval("js", "cb.call({'foo': 'bar'});");
    // prints:
    // {foo=bar}
    // class com.oracle.truffle.api.interop.java.TruffleMap

Which feels correct, the JS Object is mapped to a java.util.Map which resembles good enough to the original source JS type.

Then I execute the same test with an JS array:

ctx.eval("js", "cb.call(['foo', 'bar']);");
    // prints:
    // {0=foo, 1=bar}
    // class com.oracle.truffle.api.interop.java.TruffleMap

I understand that a JS Array by nature is a JS Object however I'd expect that in this case there would be some logic to wrap it in a type that implements java.util.List.

The issue of always having a Map is that on the java side there is no way to know what was the source original type, as for example, the keys on TruffleMap are always of type String so no distinction can be made there, as well there are no high level helpers like:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray

That could be used from the java side.

@wirthi wirthi self-assigned this May 4, 2018
@pmlopes
Copy link
Author

pmlopes commented May 4, 2018

There are some workarounds ;) but it does feel like an hack. For example use the following code at start up:

    Value isObject = ctx.eval("js", "function (obj) { return typeof obj === 'object'; }");
    Value isArray = ctx.eval("js", "function () { return Array.isArray; }").execute();

And then when values get passed the target object needs to be aware of those 2 functions and perform the 2 checks to decide what is the underlying type.

This only detects the type it does not cast it to the desired type though.

@chumer
Copy link
Member

chumer commented May 4, 2018

If you are using an Object type signature then the polyglot API tries to map to a reasonable Java type. The semantics are defined here: http://www.graalvm.org/truffle/javadoc/org/graalvm/polyglot/Value.html#as-java.lang.Class-
See section "Object target type mapping".

JavaScript arrays are values with members and array elements. According to the semantics they are mapped to Maps. We would really have liked to map to something that is List and Map at the same time, unfortunately you cannot implement Map and List at the same time (a design flaw in the Java collections API).

I can think of four ways to check for that its an array:

  1. The one you proposed
  2. Map your signature to org.graalvm.polyglot.Value. Which supports to be called with any polyglot Value and has fine-grained APIs.
public class CallMe {
  public void call(Value v) {
    if (v.hasArrayElements()) {
      List list = v.as(List.class);
    }
  }
}
  1. You can convert your Object to a polyglot value using the Context like this:
public class CallMe {
  public void call(Object v) {
    System.out.println(context.asValue(v).hasArrayElements());
  }
}
  1. Use overloads (broken in rc1 sorry):
  public void call(List v) {
    // used if its a JS array
  }
  public void call(Object v) {
   // used for everything else
  }

Personally I think solution 4 looks the cleanest if you don't want to use the polyglot API for your APIs. Otherwise 2 probably provides the highest flexibility.

Hope this helps.

@chumer
Copy link
Member

chumer commented May 4, 2018

Sorry solution 4 seems to be broken in rc1. We will fix ASAP.

@pmlopes
Copy link
Author

pmlopes commented May 4, 2018

For my specific case option 3 (which i wasn't aware of) seems the best. The reason is that the API exposing the method is external to my project (so I can't change it) but my project provides a functional interface that is called after the object is received on that library. So I can bind the script context to that closure and use it there.

And it sure looks less hacky than what I was attempting to achieve!

dougxc pushed a commit that referenced this issue May 25, 2018
Remove a pointless adapter frame  by fixing up the function's formal
parameter count.  Before:

    frame #0: 0x000033257ea446d5 onParserExecute(...)
    frame #1: 0x000033257ea3b93f <adaptor>
    frame #2: 0x000033257ea41959 <internal>
    frame #3: 0x000033257e9840ff <entry>

After:

    frame #0: 0x00000956287446d5 onParserExecute(...)
    frame #1: 0x0000095628741959 <internal>
    frame #2: 0x00000956286840ff <entry>

PR-URL: nodejs/node#17693
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
@woess
Copy link
Member

woess commented Jun 4, 2018

Option 4 has been fixed (oracle/graal@edd49fa) and will work in 1.0.0-rc2.

@woess
Copy link
Member

woess commented Jun 4, 2018

@pmlopes are you satisfied with the above-mentioned solution? If yes, can we close this issue?

@pmlopes
Copy link
Author

pmlopes commented Jun 5, 2018

@woess 👍

@eduveks
Copy link

eduveks commented Nov 19, 2019

I have situations like this:

  public void call(List v) {
    // used if its a JS array
  }
  public void call(Map v) {
   // used if its a JS object
  }

And GraalVM raises this error below:

TypeError: invokeMember (call) on JavaObject[MyClass@2eadd8cc (MyClass)] failed due to: Multiple applicable overloads found for method name call (candidates: [Method[public void MyClass.call(java.util.List)], Method[public org.netuno.psamata.Values MyClass.call(java.util.Map)]], arguments: [DynamicObject<JSArray>@7276fac1 (DynamicObjectBasic)])

This behavior is very annoying and complicate a lot to use JS arrays with Java already existing methods.

And is impossible to change every situation like that to receive an Object instead of Map.

And even if change all my methods to the only Object will not be a solution because the object received is a PolyglotMap:

public void call(Object m) {
    String objType = "";
    if (m instanceof Iterable) {
        objType = "List";
    } else if (m instanceof Map) {
        objType = "Map";
    }
    System.out.println(objType);
}

It is impossible for to me change all situations that already exist to hack this behavior.

Somehow GraalVM needs to send a PolyglotList when it is a JS Array.

With situations like this, I don't have a polyglot Java implementation... with this I wonder if tomorrow I need to integrate Python or Ruby what will be the changes in Java source that will be necessary...?

In my case, my decision goes to not support JS arrays, because seem like a limitation of the GraalVM and hack Java methods aren't a good practice.

For me, the Java code is to be reused to any language and not tied to a scripting language.

JavaScript arrays are values with members and array elements. According to the semantics they are mapped to Maps. We would really have liked to map to something that is List and Map at the same time, unfortunately you cannot implement Map and List at the same time (a design flaw in the Java collections API).

I understand the "design flaw in the Java collections API", but for me, GraalVM needs to find a solution to create better compatibility with all existing Java code.

And GraalVM is awesome anyway, great job!

@ispringer
Copy link

@eduveks, I think you can solve the situation you described using a polyglot custom type mapping - something like:

HostAccess hostAccess = HostAccess.newBuilder()
    .targetTypeMapping(Value.class, List.class, (v) -> v.hasArrayElements(), (v) -> v.as(List.class));
    .build();

For more details, see #94 (comment).

@eduveks
Copy link

eduveks commented Nov 19, 2019

@ispringer thank you for these insights, really helped.

With your suggestion I'm finally able to make all working like a charm, this is my final code:

        HostAccess hostAccess = HostAccess.newBuilder()
                .allowPublicAccess(true)
                .targetTypeMapping(
                        Value.class, Object.class,
                        (v) -> v.hasArrayElements(),
                        (v) -> transformArray(v)
                ).targetTypeMapping(
                        Value.class, List.class,
                        (v) -> v.hasArrayElements(),
                        (v) -> transformArray(v)
                ).targetTypeMapping(
                        Value.class, Collection.class,
                        (v) -> v.hasArrayElements(),
                        (v) -> transformArray(v)
                ).targetTypeMapping(
                        Value.class, Iterable.class,
                        (v) -> v.hasArrayElements(),
                        (v) -> transformArray(v)
                )
                .build();

        context = Context.newBuilder(permittedLanguages)
                .allowAllAccess(true)
                .allowHostAccess(hostAccess)
                .build();

The trick here is mapping to all methods with inputs like List, Collection and Iterable, and to Object when is an array with nested JS object.

Bellow is my methods to transform into List and Map, Map because the JS objects inside arrays don't works if not transformed too.

    private Map transformMembers(Value v) {
        Map map = new HashMap();
        for (String key : v.getMemberKeys()) {
            Value member = v.getMember(key);
            if (member.hasArrayElements() && !member.isHostObject()) {
                map.put(key, transformArray(member));
            } else if (member.hasMembers() && !member.isHostObject()) {
                map.put(key, transformMembers(member));
            } else {
                map.put(key, valueToObject(member));
            }
        }
        return map;
    }

    private List transformArray(Value v) {
        List list = new ArrayList();
        for (int i = 0; i < v.getArraySize(); ++i) {
            Value element = v.getArrayElement(i);
            if (element.hasArrayElements() && !element.isHostObject()) {
                list.add(transformArray(element));
            } else if (element.hasMembers() && !element.isHostObject()) {
                list.add(transformMembers(element));
            } else {
                list.add(valueToObject(element));
            }
        }
        return list;
    }

Now I can finally support implementations like this:

const data = _val.init(['zp', 'kz', { up: [1, 3, 4] }]);
_out.json([
    {
        aa: [1, 2, 3, data],
        dt: data,
        bb: {ar: ['a', 'b', {sss: [1, 3, 4]}]}
    }
]);

Where the _val and _out are Java Objects.

And even with Java Objects inside JS Array or JS Object works well too, see the "data" variable.

Here is my final result:

[{"aa":[1,2,3,["zp","kz",{"up":[1,3,4]}]],"dt":["zp","kz",{"up":[1,3,4]}],"bb":{"ar":["a","b",{"sss":[1,3,4]}]}}]

@tailangankur
Copy link

@ispringer @eduveks,
I am having the same issue where i am returning an array of object from my javascript code and here on java side it is coming as empty polyglotmap, i tried the above solutions but none worked for me, can you guys help?

@eduveks
Copy link

eduveks commented Jun 30, 2021

@tailangankur,
I've shared my core integrations with GraalVM used in netuno.org, here:

May this help you.

If you test the Netuno Platform you can create a JS web service and will be able to try and see the results of my above sample code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants