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

19.2.0 Date interop not working #200

Open
hashtag-smashtag opened this issue Aug 27, 2019 · 5 comments
Open

19.2.0 Date interop not working #200

hashtag-smashtag opened this issue Aug 27, 2019 · 5 comments

Comments

@hashtag-smashtag
Copy link

Hi,

I'm trying to use the new date interop in 19.2.0, but having issues in both directions across the Javascript/Java boundary:

  • When I create a JS Date in Javascript, it's only seen as a Java Date if I explicitly use value.as(Date.class). However, value.as(Object.class) instead gives me an empty Map. The workaround for this is to use the HostAccessBuilder#targetTypeMapping feature.
  • When I create a Date in java, it's not seen as a JS Date in Javascript. Implementing ProxyInstant/ProxyDate does not change this. And there is no Java-JS type mapping (see Custom Java -> JS Conversion #144). Thus I still need to continue to use my own DateProxy (a ProxyObject that exposes a getTime member as a ProxyExecutable).

Here is a testng test in Groovy that demonstrates the issues:

// In Groovy
@Test
void "date interop troubles"() {
    // Java -> JS ----------------------------------------------------------
    def hostAccess = HostAccess.newBuilder()
            .allowAccessAnnotatedBy(HostAccess.Export.class)
            .allowArrayAccess(true)
            .allowListAccess(true)
            // we could use .targetTypeMapping, but are showing how date interop doesn't work without it
            .build()
    context = Context.newBuilder("js")
            .allowHostAccess(hostAccess)
            .allowExperimentalOptions(true)
            .option("js.experimental-foreign-object-prototype", "true")
            .build()

    def val = context.eval("js", "new Date();")

    that "asDate on js-date works", {
        def asResult = val.as(Date.class)
        assert asResult instanceof Date
        assertThat asResult isInstanceOf Date
    }

    that "asObject on js-date does NOT work", {
        def asResult = val.as(Object.class)
        assertThat asResult isNotInstanceOf Date
        assertThat asResult isInstanceOf Map
    }

    that "asMap on js-object-with-date does NOT work", {
        val = context.eval("js", "var abc = { str: 'hi', num: 123, date: new Date() }; abc;")
        def asResult = val.as(Map.class)
        assertThat asResult.date isNotInstanceOf Date
        assertThat asResult.date isInstanceOf Map
        asResult = val.as(new TypeLiteral<Map<String,Object>>() {})
        assertThat asResult.date isNotInstanceOf Date
        assertThat asResult.date isInstanceOf Map
    }

    // Java -> JS ----------------------------------------------------------
    context.getBindings("js").putMember("binding", new Date())

    that "java Date does NOT become or act like a js-date", {
        val = context.eval("js", "binding instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "binding.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }

    that "does NOT work for a POJO containing an explicit Date field in js", {
        context.getBindings("js").putMember("testClass", new TestClass(testDate: new Date()))
        val = context.eval("js", "testClass.testDate instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "testClass.testDate.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }

    // Not testing fancier things like Dates as values on Object/Serializable fields, within Collections, etc

    that "does NOT work for ProxyInstant", {
        context.getBindings("js").putMember("binding", ProxyInstant.from(new Date().toInstant()))
        val = context.eval("js", "binding instanceof Date")
        assert val.isBoolean() && !val.asBoolean()

        try {
            context.eval("js", "binding.getTime();")
            assert false
        } catch (PolyglotException ignored) { /* expected */}
    }
}

public class TestClass {
    @Export
    public Date testDate;
}
@Biglr
Copy link

Biglr commented Aug 27, 2020

Hi @hashtag-smashtag

I'm currently facing the same issue and was wondering if you have an example of your targetTypeMappings for JS Dates to Java Dates.

Cheers and thanks,
David

@hashtag-smashtag
Copy link
Author

Hi @Biglr ,

For the JS->Java direction, we use something like this (note DateProxy references are our own proxy for Java->JS):

// Wherever you're setting up your HostAccess, ours is static:
private static void addTypeMappings(HostAccess.Builder builder) {
    //...
    builder.targetTypeMapping(Value.class, Object.class, Values::isDate, Values::toDateOrClassCastException);
    builder.targetTypeMapping(Value.class, Date.class, Values::isDate, Values::toDateOrClassCastException);
    builder.targetTypeMapping(Value.class, DateProxy.class, Values::isDate, Values::toDateOrClassCastException);
   //...
}

// Use the HostAccess when you're constructing your Contexts:
Builder contextBuilder = Context.newBuilder("js)
    .allowHostAccess(hostAccess)
    //...
    ;

public final class Values {
    public static boolean isDate(@Nullable Value value) {
        if (value == null || value.isNull() || value.isHostObject()) {
            return false;
        }

        // Could consider other formats like long epoch
        // Currently just looking for js Date class or DateProxy

        if (value.isProxyObject() && (value.asProxyObject() instanceof DateProxy)) {
            return true;
        }

        Value metaObject = value.getMetaObject();
        if ((metaObject == null) || !Objects.equals(metaObject.getMetaSimpleName(), "Date")) {
            return false;
        }

        return value.hasMember("getTime") && value.getMember("getTime").canExecute();
    }

    public static Date toDateOrClassCastException(@Nullable Value value) {
        Date date = toDate(value);
        if (value == null) {
            // So mapper can move onto other mappings
            throw new ClassCastException();
        }
    }

    @Nullable
    public static Date toDate(@Nullable Value value) {
        if (value == null || value.isNull()) {
            return null;
        }

        if (value.isProxyObject() && value.asProxyObject() instanceof DateProxy) {
            return (DateProxy) value.asProxyObject();
        }

        checkArgument(value.hasMember("getTime"), "Value [%s] is not a Date.", value);
        Value time = value.getMember("getTime").execute();
        checkArgument((time != null) && time.isNumber() && time.fitsInLong(),
                      "Value [%s] is not a Date.", value);

        return new DateProxy(time.asLong());
    }
}

Cheers,
Matt

@Biglr
Copy link

Biglr commented Aug 27, 2020

Hi @hashtag-smashtag

Thank you very much for your reply!

I was fiddling around with targetTypeMaps in the mean time and I came up with this (found something similar in a close GitHub issue somewhere, there really is not much in terms of examples for this around...):

.targetTypeMapping(Value.class, Date.class, v -> v.hasMember("getTime"), v -> new Date(v.getMember("getTime").execute().asLong()))

however, this never worked and just ignored the mapping no matter what i set for accepts.

I guess the critical point was to not only specify Date.class or LocalDate.class but Object.class as well (which you did gladly, otherwise I might have never come to that conclusion).

I'll stick to your implementation though, since it offers much more extensibility 😉

Thank you once again very much, you saved me a lot time and headaches 😀

Cheers,
David

@steventamm
Copy link

steventamm commented Sep 3, 2020

Java->JS interop is still mostly broken. Using 20.2.0, it looks like JSRuntime.toPrimitiveFromForeign doesn't handle Date/Instance correctly and calls toString and tries to coerce it to a double. When I try to use the Intl library for formatting a straight ProxyDate, it gives a rangeError. If my ProxyDate implements ProxyObject and implements toString as the same as getTime, it works, but it's incorrect. Date.toString() should return a formatted date and not the epoch date.

JSRuntime.toPrimitiveFromForeign should check InteropLibrary.isDate/isInstant/etc. and then call the appropiate method.

You can validate it by running js --jvm --js.intl-402 then trying this

var pdate = Java.type('org.graalvm.polyglot.proxy.ProxyDate').from(Java.type('java.time.LocalDate').now())
new Intl.DateTimeFormat('en-US').format(pdate)

It returns

RangeError: Provided date is not in valid range.
	at <js> null(Unknown)
	at <js> :program(<shell>:2:1:0-45)

instead of properly coercing as a JSDate

@hashtag-smashtag
Copy link
Author

Hi @steventamm

For Java -> JS we use something like this as a workaround:

public class DateProxy extends Date implements ProxyObject {

    private static final Set<String> PROTOTYPE_FUNCTIONS = ImmutableSet.of(
            "getTime",
            "toISOString",
            "toJSON",
            "toString"
    );

    public DateProxy(Date date) {
        super(date.getTime());
    }

    public DateProxy(long epoch) {
        super(epoch);
    }

    @Override
    public Object getMember(String key) {
        switch (key) {
            case "getTime":
                return (ProxyExecutable) arguments -> getTime();
            case "toJSON":
            case "toISOString":
                return (ProxyExecutable) arguments -> ISO8601Utils.format(this, true);
            case "toString":
                // Currently defaulting to Date.toString, but could improve
                return (ProxyExecutable) arguments -> toString();
            default:
                throw new UnsupportedOperationException("This date does not support: " + key);
        }
    }

    @Override
    public Object getMemberKeys() {
        return PROTOTYPE_FUNCTIONS.toArray();
    }

    @Override
    public boolean hasMember(String key) {
        return PROTOTYPE_FUNCTIONS.contains(key);
    }

    @Override
    public void putMember(String key, Value value) {
        throw new UnsupportedOperationException("This date does not support adding new properties/functions.");
    }
}

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

3 participants