Add options to EdnPacker to pass through to read-string.

[ericnormand]

It should be possible to add custom tag handlers and other options for read-string.

[ptaoussanis]

Hi Eric, this patch unfortunately produces compilation warnings since cljs.reader/read-string is single-arity only. What options did you have in mind to use?

[ericnormand]

Upon further research into ClojureScript, I realize that I don't think what I wanted to do in the way I wanted it was possible.

I wanted to pass in {:readers {'my-project/custom-type #'my-project/read-custom-type}}, which is the way to get custom tagged literals in Clojure. Unfortunately, the interface is different in ClojureScript. You have to rebind a dynamic variable or something, so that's out for this use case. It would be way more complicated.

Sorry for the pull request.

[ptaoussanis]

Unfortunately, the interface is different in ClojureScript.

Yeah, that is a bit unfortunate. Haven't used custom tagged literals in Cljs myself, but seems like something like this should hopefully do the trick, despite being a bit clumsy.

Have just pushed to Clojars under [com.taoensso/sente "1.3.0-SNAPSHOT"] if you felt like giving it a try (it's completely untested).

Re-opening for now until we've confirmed this works and/or decide to revert it.

Cheers! :-)

[ptaoussanis]

Closing for now. Have the change in the commit history; you're welcome to reopen if you still have interest in this going in. Cheers :-)

[gphilipp]

Hi Peter & Eric, I'm using cli-time and have joda-time dates in the structures I want to pass back and forth between my Clojurescript app and my Clojure server. How can I do this either with edn or transit packer ? It seems like only this pull request would allow that (for edn at least).

[ptaoussanis]

Hi Gilles,

PR welcome to reintroduce the previously-reverted commit if you can test it and confirm that it's working correctly?

Otherwise keep in mind that as a worst case you can always use pr-str to produce an edn string, send that over the wire, and manually read-string it back on the other end with whatever reader bindings you'd like like. Does that help?

[danielcompton]

@gphilipp here is some code that I'm using to add converters for Joda time dates into Transit and JSON. It's late here, I'll try and follow up with more details on it's use and whether it is another route to do what you're after.

(ns my-ns.utils
  (:require [cheshire.generate :as gen]
            [cognitect.transit :as transit]
            [clj-time.coerce :as coerce]
            [taoensso.sente :as sente]
            [taoensso.timbre :as timbre])
  (:import [org.joda.time DateTime ReadableInstant]
           [org.joda.time.format ISODateTimeFormat]))

(defn setup-encoders
  "Add converters to turn JODA Time objects into JSON dates"
  []
  (gen/add-encoder
    DateTime
    (fn [^DateTime dt ^com.fasterxml.jackson.core.json.WriterBasedJsonGenerator generator]
      (.writeString generator (.print (ISODateTimeFormat/dateTime) ^ReadableInstant dt)))))

;; TODO: Use native Java to avoid dep on clj-time?
(def joda-time-writer
  (transit/write-handler
    (constantly "m")
    (fn [v] (-> v coerce/to-date .getTime))
    (fn [v] (-> v coerce/to-date .getTime .toString))))

[danielcompton]

@gphilipp following up a few months later when I ran into this issue myself :)

(ns my-ns.app
  (:require [cognitect.transit :as transit]
            [taoensso.sente.packers.transit :as sente-transit])
  (:import [org.joda.time DateTime ReadableInstant]))

;; From http://increasinglyfunctional.com/2014/09/02/custom-transit-writers-clojure-joda-time/
(def joda-time-writer
  (transit/write-handler
    (constantly "m")
    (fn [v] (-> ^ReadableInstant v .getMillis))
    (fn [v] (-> ^ReadableInstant v .getMillis .toString))))

(def custom-transit-writers {DateTime joda-time-writer})

(def packer (sente-transit/->TransitPacker :json {:handlers utils/custom-transit-writers} {}))

[danielsz]

A little contribution to the discussion, especially to the attention of boot users. :-)

I was trying to achieve exactly the same: passing custom edn tags for Joda. I'm not using transit. Not that I have anything against transit, but I'm in full control of the data passing through my system. Since my system is 100% Clojure, what flows through the various pieces (server, client) is edn. Blissful simplicity (I know transit is faster, but it's meaningless in my use case).

I've defined custom reader tags like so: https://gist.github.com/Deraen/eb3f650c472fb1abe970#file-edn-cljx

With sente, I have not defined any flexipacker, so if my understanding is correct the default is used, which happens to be :edn.

So far so good.

I was debugging however an instance of

clojure.lang.ExceptionInfo: No reader function for tag DateTime
    data: {:type :reader-exception}

The problem was that data-readers was not populated. I tried both with writing data_readers.clj and via rebinding data-readers. Long story short, the issue was with boot: boot-clj/boot#47

[ptaoussanis]

Thanks Daniel, that'll be super helpful to have here as a reference for others!

[danielsz]

Thank you, @ptaoussanis!

And the good new is that the issue is fixed in system as of 0.2.1-SNAPSHOT.

https://github.com/danielsz/system

[danielsz]

It was a bit too soon to claim victory.

I'm sending the following payload from the client.

15-Dec-06 09:34:28 daniel-MacBookAir DEBUG [taoensso.sente] - Bad package: [[:twitter-fu/api {:update-preferences {:shop-name "test", :accounts {:twitter "bellybag1" :legacy {:end #DateTime "20151205T000000.000Z", :provider "shopify"}}, :total-items 5}}] "3cc733"] (clojure.lang.ExceptionInfo: No reader function for tag DateTime {:type :reader-exception})

Sente is unaware of the custom tag which has been registered here:

(cljs.reader/register-tag-parser! "DateTime" reader-str->datetime)

Can you please advise?

[ptaoussanis]

Hi Daniel,

Please make sure that you're using [com.taoensso/encore "2.26.1"] or newer; some work went in there recently to try pave over issues with ClojureScript's reader tag support. The next Sente release will auto-bump this dependency but it's not included with Sente 1.7.0-RC1 yet.

If that's no help, could you possibly try confirm whether the issue still exists when not using Boot?

Thanks!

[danielsz]

Hi Peter,

Yes, that was it. No such problem with [com.taoensso/encore "2.26.1"]. Thank you!

Am I allowed to rant?

Having old versions of the two auxiliary libraries encore and timbre in my classpath is something of a fixture. I remember old versions of Timbre giving me trouble (although that may have been in conjunction with Carmine). Besides being a potential source of problems, it is a difficult one to debug from a user's perspective. I never think of checking the latest versions, because these are no part of my project dependencies, and my expectation is that the latest Sente is really the latest (including dependencies). I see Sente ships with [com.taoensso/encore "2.18.0"]. Do you expect users to track versions of the project's dependencies themselves? I mean, is there a reason for this state of affairs?

End of rant. :-)

[ptaoussanis]

Yes, that was it. No such problem with [com.taoensso/encore "2.26.1"]. Thank you!

No problem. Will try cut a new Sente release with the fix in the next few days, just have my hands a bit full atm.

Having old versions of the two auxiliary libraries encore and timbre in my classpath is something of a fixture.

Fixture?

I never think of checking the latest versions, because these are no part of my project dependencies, and my expectation is that the latest Sente is really the latest (including dependencies).

You don't need to think of checking the latest versions (that'd be documented otherwise), your expectation is correct that libs should pull in the necessary dependencies.

In this case: you brought a Sente issue to my attention for the first time. I suspected that maybe a new version of Encore would help. You just confirmed that. So my intention is to bump the Encore dependency for Sente's next overdue release, fixing the issue. This way you get the fix sooner than if you'd have had to wait for the next Sente release so not sure what state of affairs here warrants ranting?

Does that make sense / seem reasonable?

Edit: just to clarify a possible source of confusion - I wasn't intending to suggest that you're running into this issue because you failed to check for the latest dependencies. You shouldn't need to. Rather: was asking you to try with the latest dependencies to see if that might solve the problem.

[danielsz]

Yes, absolutely. I'm sorry for the tone of the rant.
If I had any trouble with your otherwise excellent libraries, it was with the auxiliary ones (timbre and encore). It is my responsibility to report it when it happens, not as a result of past frustrations.

[ptaoussanis]

No problem, happy you reported this :-)