allow custom converters from and to java

[pmlopes]

When working with Java interoperability, basic conversions happen between types that cross across languages. A js object becomes a java Map for example.

There are cases where it would be interesting to allow user defined conversions. For example:

• js Date <=> java Instant
• js object <=> java POJO

Having a interface that could be:

Function<F, T> convert (Class<F> from, Class<T> to);

In order to keep performance acceptable this functions could be stored on a identity hash map and only one converter per specific type allowed.

Plus matching should be the exact class so avoid to much reflection or runtime type information.

The example above is just for illustration, an alternative could be looking at nashorn dynamic linker API that allows amongst other things this exact scenario.

You can see a simple usage here:

https://github.com/reactiverse/es4x/blob/develop/es4x/src/main/java-9/io/reactiverse/es4x/dynalink/ES4XJSONLinker.java

[chumer]

Thanks for the suggestion!

Yes Nashorn exposed that through the dynalink API. That is the equivalent to exposing the full Truffle API to the embedder in terms of complexity. I agree we should have a mechanisms like this, but hopefully much simpler. The challenge will be to not sacrifice performance for this (if not used and if used only for the affected types).

I'd suggest to add following:

Context.Builder.hostConverter(Class<T> toClass, Function<Value, T> converter)
Context.Engine.hostConverter(Class<T> toClass, Function<Value, T> converter)

Example from https://github.com/reactiverse/es4x/blob/develop/es4x/src/main/java-9/io/reactiverse/es4x/dynalink/ES4XJSONLinker.java

Context.newBuilder().hostConverter(JsonObject.class, (v) -> {
   if (v.isNull()) return v;
   if (v.isHostObject() && v.asHostObject() instanceof JsonObject) return v;
    return new JsonObject(v.as(Map.class));
}

This is slightly different to your suggestion. I am not sure what we could provide a reasonable fromType Class. Also, what would that be useful for?
The advantage of a statically provided type is that we don't need to specify mutability of the converter factory. What if the class mapping suddenly changes? Some hard to specify parts of the application would use the old converter function and others would use a different one.

@pmlopes Would that fit your use-cases?

[pmlopes]

@chumer this would be great as a user I could map my own js types to java types before interop! This would cover all the current use cases I have:

• js Object -> vertx JsonObject
• js Array -> vertx JsonArray
• js Date -> java.util.Date
• js Date -> java.time.Instant

Plus allow users to register custom converters e.g.:

• js Object -> user POJO

👍

[woess]

I'm afraid that this increases the chance for ambiguities in the method overload selection, even if there's only one converter per type allowed. If there are multiple host converters involved, we can't decide which one's the best. Dynalink has a solution for this, called ConversionComparator, which allows the user to resolve this ambiguity. While not absolutely necessary, we might need something similar.

@pmlopes You don't need a source type guard, do you?

[pmlopes]

@woess in my comment I was saying that matching would be on exact class (no hierarchy) to reduce the complexity and I believe that only allow only 1 converter per class is enough. Perhaps the API should return the previous converter if any or null.

In this case I me as user really needs multiple converters, then I'd write the chain myself. WDYT?

[chumer]

There is typically a single entity, naming the embedder, deciding conversion. A comparator is not needed, right? The embedder can add conversion comparator on top if necessary.

[woess]

If host converters accepted all source types, that would make things simpler, as you'd only need to look at the target (parameter) types to select method and converter. The downside is lack of flexiblity.

Say you have two method overloads with different custom converters:

• method(JsonObject)
• method(Instant)

Of course, you could define a type ranking, but then one type always get chosen over the other.
You probably want to dispatch by source type (JS Date -> Instant, JS Object -> JsonObject). For that to work, you need a dynamic type check (guard). The converter either needs some sort of accepts(Value) method, or return a result that reveals that it couldn't convert. A negative result could mean that a better method overload is available, so you need to reevaluate the choice of method overload and conversions.
Oh, and a JS Date is also an Object, so you'll want to make sure you always call the Instant method if possible, either using a not-a-Date guard in the JsonObject converter, or:
When multiple methods are applicable with different custom converters, you'll get an exception, unless you provide a conversion comparator that resolves the ambiguity.
I'm afraid that also means we should automatically insert negative guards for higher-ranking converters, if there's a possibility their supported types overlap.

[pmlopes]

I think the example above also relates to another topic that has been discussed in gitter, say we have the following interface from the comment above:

interface InterOpTest {
  void method(JsonObject arg);
  void method(Instant arg);
}

When calling this form the host (graaljs) the method arity is the same so it is not obvious which method to call, for this we need something like we had on nashorn to explicitly select the desired method e.g.:

// assume myInstanceOfInterOpTest is an instance of InterOpTest
myInstanceOfInterOpTest.method['java.time.Instant'](new Date())

With such a construct in place the choice of the method is not a issue, so we can now continue with the convertion which allows us to look just to the target type which is clearly Instant. With this we can then lookup for a users custom converter and if present, invoke, otherwise fallback to the regular conversion chain...

[chumer]

Here is the new API that is work in progress. Feedback appreciated:

public class Test {

    public static class MyClass {
    
        @HostAccess.Export
        public void foo(JsonObject c) {
        }
    }
    
    public static class JsonObject {
        JsonObject(Value v) {
        }
        // omitted
    }
    
    public static void main(String[] args) {
        HostAccess customMapping = HostAccess.newBuilder().//
                        allowAccessAnnotatedBy(HostAccess.Export.class).//
                        typeMapping(JsonObject.class,
                                        (v) -> v.hasMembers() || v.hasArrayElements(),
                                        (v) -> new JsonObject(v)).build();
    
        try (Context c = Context.newBuilder().allowHostAccess(customMapping).build()) {
            c.getBindings("js").putMember("javaObject", new MyClass());
            c.eval("js", "javaObject.foo({})"); // works!
            c.eval("js", "javaObject.foo([])"); // works!
            c.eval("js", "javaObject.foo(42)"); // fails!
        }
    }

}

Precedence will be determined by the order of the typeMapping specification. So the earlier declared mappings get precedence. Note that the standard type mappings except the Object.class type mapping (as this fits all values) always gets precedence. For example, a method with int signature that is passed a number that fits int will not use any mappings for int.

@pmlopes your issue about the method overload selection is a separate issue we are working on and we hope to get to it soon.

[pmlopes]

@chumer I think this is great, just one question, I assume we can we have multiple:

                      typeMapping(JsonObject.class,
                                        (v) -> v.hasMembers() || v.hasArrayElements(),
                                        (v) -> new JsonObject(v)).build();

right? If the anwser is yes then 👍

[chumer]

@pmlopes Of course. As many as you like :-)
Thanks for the feedback.

[ispringer]

@chumer, will the mappings also get applied to elements of Collections and Maps? For example:

@HostAccess.Export
public void foo(List<JsonObject> list) {
}

@HostAccess.Export
public void foo(Map<String, JsonObject> map) {
}

@HostAccess.Export
public void foo(List<Map<String, JsonObject>> listOfMaps) {
}

[chumer]

@ispringer Yes. That is the plan. Will also work for arrays. Need more tests for that, good reminder.

[ispringer]

@chumer, thanks, that's great to hear.

Do you plan on adding additional built-in conversions for JDK types that have obvious mappings? For example, the JavaScript Date class has a straightforward conversion to a Java Date or Instant class. It would be nice if that and similar cases "just worked".

The below test summarizes the current lack of support for Date conversion:

@Test
void "conversion of JS Date to Java Date or Instant"() {
    Context context = Context.create("js")

    Value jsDate = context.eval("js", "new Date();")
    println "Converted JS Date to polyglot Value with toString [${jsDate}] and className [${jsDate.metaObject.getMember('className')}]"

    // Try to convert to a Java Date.
    try {
        Date javaDate = jsDate.as(Date.class)
    } catch (e) {
        System.err.println("Failed to convert js Date to java Date :-( : ${e}")
    }

    // No dice. Try to convert to a Java Instant.
    try {
        Instant javaInstant = jsDate.as(Instant.class)
    } catch (e) {
        System.err.println("Failed to convert js Date to java Instant :-( : ${e}")
    }

    // No dice. See what type we get back if we call value.as(Object.class).
    Object javaObject = jsDate.as(Object.class)
    println "Converted JS Date to Java ${javaObject.getClass().name}"
    // A com.oracle.truffle.polyglot.PolyglotMap with size 0 ??? Pretty useless.
    // I would think it would make more sense for it to just return the original polyglot Value.

    // Manually convert the polyglot Value to a Java Date.
    assert jsDate.hasMember("getTime")
    def timestamp = jsDate.getMember("getTime").execute().asLong()
    Date javaDate = new Date(timestamp)
    println "Manually converted JS Date to java Date with toString [${javaDate}]"
}

[johnwatts]

@chumer This looks great!

We're already doing our own makeshift type mapping for List and Map in Java-land.

From "JavaScript Usage Examples" in http://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/Value.html#as-java.lang.Class- I would like way to configure the Context so that instead of:

assert context.eval("js", "[]").as(Object.class) instanceof Map;

We get:
assert context.eval("js", "[]").as(Object.class) instanceof List;

I'm thinking this could work

typeMapping(Object.class,
        (v) -> v.hasArrayElements(),
        (v) -> v.as(List.class)).build();

but I have the following questions:

1. Would I need to repeat the exact same typeMapping for Collection.class (and maybe again for Iterable.class) if I want to ensure those are mapped as Lists because of allow custom converters from and to java #94 (comment) ?

2. In addition to elements of Collections which @ispringer mentioned, I trust sub-objects will use typeMapping, right? E.g.

public class CustomType {
         public Object json;
}

typeMapping(Object.class,
        (v) -> v.hasArrayElements(),
        (v) -> new LinkedList(v.as(List.class))).build();

CustomType customType = context.eval("js", "{json: [1]}").as(CustomRequest.class);
assert customType.json instanceof LinkedList;

3. If a type-mapper delegates to graal for type-mapping by calling Value.as, will custom type-mappers still be called within that delegated call for a subfield or collection element?

public class CustomRequest {
         public Object json;
}

typeMapping(Object.class,
        (v) -> v.hasArrayElements(),
        (v) -> new LinkedList(v.as(List.class))).build();

CustomType customType = context.eval("js", "{json: [[]]}").as(CustomRequest.class);
assert customType.json[0] instanceof LinkedList;

Sorry for the long-winded questions and thanks in advance.

[chumer]

@ispringer we are thinking about a few more standard mappings. Date/Instant is one of them. Currently, our focus is on getting the converters right and removing lossy conversions that we currently do for String -> number and number -> string.

@johnwatts Thank you. These are excellent questions.

Would I need to repeat the exact same typeMapping for Collection.class (and maybe again for Iterable.class) if I want to ensure those are mapped as Lists because of #94 (comment) ?

Yes you need to add them for all the base types manually. The type mappings work with exact target types only.

In addition to elements of Collections which @ispringer mentioned, I trust sub-objects will use typeMapping, right? E.g.

Yes. The type mappings work on any kind of recursive mapping. This includes fields, method parameters, proxy/functional interface return values and varargs.

BTW.: your example in this form does not work. The CustomRequest class needs to be an interface to be used like you want it for Value.as(CustomRequest).

If a type-mapper delegates to graal for type-mapping by calling Value.as, will custom type-mappers still be called within that delegated call for a subfield or collection element?

If you call type mapping recursively in a custom type mapping, then only standard mappings will be applied for those values. Do you see any problems with that?

[chumer]

Here is the latest javadoc and method signature:

/**
 * Adds a custom source to target type mapping for Java host calls, host field assignments
 * and {@link Value#as(Class) explicit value conversions}. The source type specifies the
 * static source type for the conversion. The target type specifies the exact and static
 * target type of the mapping. Sub or base target types won't trigger the mapping. Custom
 * target type mappings always have precedence over default mappings specified in
 * {@link Value#as(Class)}, therefore allow to customize their behavior. The provided
 * converter takes a value of the source type and converts it to the target type. If the
 * mapping is only conditionally applicable then an accepts predicate may be specified. If
 * the mapping is applicable for all source values with the specified source type then a
 * <code>null</code> accepts predicate should be specified. The converter must return a non
 * <code>null</code> value. The converter may throw a {@link ClassCastException} if the
 * mapping is not applicable. It is recommended to return <code>false</code> in the accepts
 * predicate if the mapping is not applicable instead of throwing an exception.
 * <p>
 * All type mappings are applied recursively to generic types. A type mapping with the
 * target type <code>String.class</code> will also be applied to the elements of a
 * <code>List<String></code> mapping. This works for lists, maps, arrays and varargs
 * parameters.
 * <p>
 * The source type uses the semantics of {@link Value#as(Class)} to convert to the source
 * value. Custom type mappings are not applied there. If the source type is not applicable
 * to a value then the mapping will not be applied. For conversions that may accept any
 * value the {@link Value} should be used as source type.
 * <p>
 * Multiple mappings may be added for a source or target class. Multiple mappings are
 * applied in the order they were added. The first mapping that accepts the source value
 * will be used. Custom target type mappings all use the same precedence when an overloaded
 * method is selected. This means that if two methods with a custom target type mapping are
 * applicable for a set of arguments, an {@link IllegalArgumentException} is thrown at
 * runtime.
 * <p>
 * Primitive boxed target types will be applied to the primitive and boxed values. It is
 * therefore enough to specify a target mapping to {@link Integer} to also map to the target
 * type <code>int.class</code>. Primitive target types can not be used as target types. They
 * throw an {@link IllegalArgumentException} if used.
 * <p>
 * If the converter function or the accepts predicate calls {@link Value#as(Class)}
 * recursively then custom target mappings are not applied. It is strongly discouraged that
 * accept predicates or converter cause any side-effects or escape values for permanent
 * storage.
 * <p>
 *
 * Usage example:
 *
 * TODO 
 *
 *
 * @param sourceType the static source type to convert from with this mapping. The source
 *            type must be applicable for a mapping to be accepted.
 * @param targetType the exact and static target type to convert to with this mapping.
 * @param accepts the predicate to check whether a mapping is applicable. Returns
 *            <code>true</code> if the mapping is applicable else false. If set to
 *            <code>null</code> then all values of a given source type is applicable.
 * @param converter a function that produces the converted value of the mapping. Must not
 *            return <code>null</code>. May throw {@link ClassCastException} if the source
 *            value is not convertible.
 * @throws IllegalArgumentException for primitive target types.
 * @since 1.0
 */
public <S, T> Builder targetTypeMapping(Class<S> sourceType, Class<T> targetType, Predicate<S> accepts, Function<S, T> converter) 

I've added a source type to simplify recursive mappings. So you can now use any source type including Value.class.

[chumer]

We plan to remove some standard builtin coercions that we were not happy with. You will be able to reconfigure the mappings like this:

HostAccess.Builder builder = HostAccess.newBuilder();
// character coercion
builder.targetTypeMapping(Integer.class, Character.class, (v) -> v >= 0 && v < 65536, (v) -> (char) (int) v);

// String to primitive coercion
builder.targetTypeMapping(String.class, Byte.class, null, (v) -> parseByteOrThrow(v));
builder.targetTypeMapping(String.class, Short.class, null, (v) -> parseShortOrThrow(v));
builder.targetTypeMapping(String.class, Integer.class, null, (v) -> parseIntOrThrow(v));
builder.targetTypeMapping(String.class, Long.class, null, (v) -> parseLongOrThrow(v));
builder.targetTypeMapping(String.class, Float.class, null, (v) -> parseFloatOrThrow(v));
builder.targetTypeMapping(String.class, Double.class, null, (v) -> parseDoubleOrThrow(v));
builder.targetTypeMapping(String.class, Boolean.class, null, (v) -> parseBooleanOrThrow(v));

// Primitive to String coercion
builder.targetTypeMapping(Number.class, String.class, null, (v) -> v.toString());
builder.targetTypeMapping(Boolean.class, String.class, null, (v) -> v.toString());

[johnwatts]

Thanks for the preliminary Javadoc. That prompts one new question:

The converter must return a non null value.

What is the effect of returning null? Is it the same as a converter throwing an exception or returning false in the accept predicate? I think this would block me from converting a JS number to Java Integer while mapping JS NaN to Java null.

And following up on my earlier questions:

The CustomRequest class needs to be an interface...

Yup. Thanks for the reminder. I incompletely refactored my example plus I was suffering from a little bit of wishful thinking. Sorry.

If you call type mapping recursively in a custom type mapping, then only standard mappings will be applied for those values. Do you see any problems with that?

The custom type mapping will have to take responsibility for type mapping all the nested sub-objects if I want it to apply recursively. My overall goal is to be able to accept arbitrary JS objects (typically constructed from JSON). The reason it doesn't work currently is because JS arrays have members so they become Maps, not Lists if their target type isn't sufficiently constrained.

I would like to be able to fix that with a custom type mapping

builder.targetTypeMapping(Object.class, Object.class,
        (v) -> v.hasArrayElements(),
        (v) -> v.as(List.class)).build();

Which will work for this example

assert context.eval("js", "[]").as(Object.class) instanceof List;

But because Value.as doesn't apply custom typemapping it won't work for

assert ((List) context.eval("js", "[[]]").as(Object.class)).get(0) instanceof List;

My choices are either to use Value.as and not have my custom type mapping apply recursively or walk the object graph myself which leads to a lot more code in the typemapper that is basically reimplementing the standard typemapping behavior (probably poorly). I need that decision (JS [] -> Java List) to apply recursively at all levels but that is the only behavior I want this typemapper to be concerned with.

I can see why you might not want Value.as to invoke custom typemapping but could there be a way to explicitly delegate the creation of nested objects to full typemapping instead of the standard typemapping only? Another example where this might matter is that you said you plan to "remove some standard builtin coercions". If someone puts those standard coercions in a custom typemappings, I would again think that a custom typemapper should at least have the option to call an API similar to Value.as and have those previously standard coercions apply rather than having to reimplement that logic for subfields inside one or more custom typemappers.

[chumer]

@johnwatts
The implementation just landed on tip:
https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/HostAccess.Builder.html#targetTypeMapping-java.lang.Class-java.lang.Class-java.util.function.Predicate-java.util.function.Function-

The converter must return a non null value.

Null values are now supported.
Also, recursive conversions are now supported.

Here is your full example:

HostAccess.Builder builder = HostAccess.newBuilder();
builder.targetTypeMapping(Value.class, Object.class, (v) -> v.hasArrayElements(), (v) -> v.as(List.class));
HostAccess access = builder.build();

try (Context context = Context.newBuilder().allowHostAccess(access).build()) {
    System.out.println(context.eval("js", "[]").as(Object.class) instanceof List);
    System.out.println(((List) context.eval("js", "[[]]").as(Object.class)).get(0) instanceof List);
}

This prints on tip:

true 
true

This feature will be part of RC 15. Closing.

[chumer]

@johnwatts Thinking again. Actually, this version should be a tiny bit faster than my previous suggestion:

 HostAccess.Builder builder = HostAccess.newBuilder();
 builder.targetTypeMapping(List.class, Object.class, null, (v) -> v);
 HostAccess access = builder.build();

[mikehearn]

Am I right in assuming this can't be used with the NodeJS interop, as there's no context access?