cognitect / transit-java

transit-format implementation for Java
Apache License 2.0
62 stars 21 forks source link

Java 10 compatibility: cannot decode verbose time (javax/xml/bind/DatatypeConverter) #28

Closed timothypratley closed 6 years ago

timothypratley commented 6 years ago

When using Java 10 (or 9), transit cannot decode verbose time. such as ~t2018-12-31T00:00:00.000Z

Symptom:

  actual: java.lang.RuntimeException: java.lang.NoClassDefFoundError: javax/xml/bind/DatatypeConverter
 at com.cognitect.transit.impl.ReaderFactory$ReaderImpl.read (ReaderFactory.java:114)
    cognitect.transit$read.invokeStatic (transit.clj:319)

Minimal repro example:

(ns ttt.core
  (:require [cognitect.transit :as transit]
            [clojure.java.io :as io]))

(defn transit-decode [body]
  (let [in (io/input-stream (.getBytes body))
        reader (transit/reader in :json)]
    (transit/read reader)))

(defn foo []
  (transit-decode "{\"hi\":\"~t2018-12-31T00:00:00.000Z\"}"))

Cause:

com.cognitect.transit.impl/ReaderHandlers.java, line 248 uses javax.xml.bind.DatatypeConverter.parseDateTime:

    public static class VerboseTimeReadHandler implements ReadHandler<Object, String> {

        @Override
        public Object fromRep(String rep) {
            Calendar t = javax.xml.bind.DatatypeConverter.parseDateTime(rep);
            t.setTimeZone(TimeZone.getTimeZone("Zulu"));
            return t.getTime();
        }
    }

Java 10 (and 9) introduces modules, and javax.xml.bind is no longer included by default. https://www.deps.co/blog/how-to-upgrade-clojure-projects-to-use-java-9/

Suggestion:

Use a different method to read the time which is compatible with Java 8 and 10? (to avoid having to use Java flags to enable the javax.xml.bind module).

puredanger commented 6 years ago

Thanks for the issue - I didn't realize that was buried in there. I wonder if there is any difference between this and clojure.instant/parse-timestamp.

puredanger commented 6 years ago

I think practically speaking, the answer is no, just this is in Java. :)

puredanger commented 6 years ago

I'll take a look at this when I get a chance.

timothypratley commented 6 years ago

:) Thank you Alex @puredanger

timothypratley commented 6 years ago

Hi @puredanger, I think you are correct that clojure.instant/parse-timestamp can be used as a replacement; shall I make a pull-request to that effect?

puredanger commented 6 years ago

Sorry, we don't take PRs on this project, but I will get to it.

timothypratley commented 6 years ago

Okidoki thank you :)

ghadishayban commented 6 years ago

as a workaround during java or jlink you can --add-modules java.xml.bind

timothypratley commented 6 years ago

Hi, I am shamelessly checking in to see if you have time to address this... no big deal if not.

puredanger commented 6 years ago

Added explicit dep to resolve this in transit-java 0.8.337 (also available via transit-clj 0.8.313).

timothypratley commented 6 years ago

Thank you Alex!!! :) :+1:

ghadishayban commented 6 years ago

Another solution is to use java.time and kill a dependency

https://github.com/ghadishayban/transit-java/commit/4868bb4aef7db0eabd0aac6855f998aa0aa90eca