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

Error when encoding map with metadata on symbol key #47

Closed
mk opened this issue Mar 10, 2020 · 7 comments
Closed

Error when encoding map with metadata on symbol key #47

mk opened this issue Mar 10, 2020 · 7 comments

Comments

@mk
Copy link

mk commented Mar 10, 2020

transit-clj errors when trying to encode a map with a symbol key with metadata on it:

(require 'cognitect.transit)
(in-ns 'cognitect.transit)

(import [java.io File ByteArrayInputStream ByteArrayOutputStream OutputStreamWriter])

(def out (ByteArrayOutputStream. 2000))
(def w (writer out :json {:transform write-meta}))

;; meta on val works
(write w {'key (with-meta 'val {:foo 'bar})})

;; but writing a map with metadata on the key errors
(write w {(with-meta 'key {:foo 'bar}) 'val})

;; Execution error at com.cognitect.transit.impl.AbstractEmitter/emitEncoded (AbstractEmitter.java:1).
;;   Cannot be used as a map key cognitect.transit.WithMeta@5f483ec3

;; writing the same with a second `{}` key works:
(write w {(with-meta 'key {:foo 'bar}) 'val {} 'val2})

I think the issue is that transit should pick a cmap for a symbol with metadata on it.

The same works with transit-cljs:

(require [cognitect.transit :as t]
(def w (t/writer :json {:transform t/write-meta}))
(cognitect.transit/write w {(with-meta 'key {:foo 'bar}) 'val})
;; => "["^ ",["~#with-meta",["~$key",["^ ","~:foo","~$bar"]]],"~$val"]"
@djblue
Copy link

djblue commented Aug 12, 2020

I just ran into this issue using transit-clj in babashka writing maps with symbol keys. It appears that symbols contain line / column metadata by default.

@borkdude
Copy link

borkdude commented Nov 2, 2021

A workaround is to choose a different (longer than 1 character) tag for clojure.lang.Symbol and override the read and write handler for this:

(ns workaround
  (:require [cognitect.transit :as t])
  (:import [java.io ByteArrayOutputStream]))

(def out (ByteArrayOutputStream. 2000))

(def w (t/writer out :json
                 {:handlers {clojure.lang.Symbol (t/write-handler "$$" t/nsed-name)}
                  :transform t/write-meta}))

(t/write w {(with-meta 'SYMBOL {:foo 'bar}) 'val})

(def ba (.toByteArray out))

(def transit (String. ba))

(prn transit)
;;=> "[\"~#cmap\",[[\"~#with-meta\",[[\"~#$$\",\"SYMBOL\"],[\"^ \",\"~:foo\",[\"^2\",\"bar\"]]]],[\"^2\",\"val\"]]]"

(require '[clojure.java.io :as io])

(def r (t/reader (io/input-stream ba) :json
                 {:handlers (t/read-handler-map {"$$" (t/read-handler symbol)})}))

(prn (map meta (keys (t/read r))))
;;=> ({:foo bar})

I'm not entirely sure what a proper solution for this is, but I noticed there are several tag.length() == 1 checks in https://github.com/cognitect/transit-java/blob/master/src/main/java/com/cognitect/transit/impl/AbstractEmitter.java so a tag length of 1 seems to have a magic meaning.

@fogus
Copy link
Contributor

fogus commented Nov 3, 2021

The map with symbols having metadata passes a stringableKeys check but those symbols need WithMeta encoding meaning that they're not useable in "map" contexts. Maybe symbols with metadata should fail a stringableKeys check. More investigation needed for the right approach here, but it explains the failure and why the $$ workaround works and why adding another non string able key (i.e. {}) fixes it also.

@borkdude
Copy link

borkdude commented Nov 5, 2021

I've got another workaround here which surfaces another interesting bit. When using a conditional tag that returns a custom tag when there's metadata on the symbol, but a single dollar when there's not, the transit appears normal and the special tag doesn't even appear.

I'm not sure why this workaround works, but the interesting bit is that it preserves the original transit format which can be read without any modifications on consumers.

(ns workaround
  (:require [cognitect.transit :as t]
            [clojure.java.io :as io])
  (:import [java.io ByteArrayOutputStream]))

(defn obj->str [v]
  (let [out (ByteArrayOutputStream. 2000)
        w (t/writer out :json
                    {:handlers {clojure.lang.Symbol
                                (t/write-handler (fn [v]
                                                   (if (meta v)
                                                     "<SPECIAL_TAG>"
                                                     "$"))
                                                t/nsed-name)}
                     :transform t/write-meta})]
    (t/write w v)
    (String. (.toByteArray out))))

;; look ma, no special read handler
(defn str->obj [s]
  (let [rdr (t/reader (io/input-stream (.getBytes s)) :json)
        v (t/read rdr)]
    v))

(let [map-w-symbol-meta {(with-meta 'SYMBOL {:foo 'bar}) 'val}
      transit-w-meta (obj->str map-w-symbol-meta)
      v (str->obj transit-w-meta)]
  (prn transit-w-meta)
  ;;=> "[\"~#cmap\",[[\"~#with-meta\",[[\"~#$$\",\"SYMBOL\"],[\"^ \",\"~:foo\",[\"^2\",\"bar\"]]]],[\"^2\",\"val\"]]]"

  (prn v) ;;=> {SYMBOL val}
  (prn (map meta (keys v))) ;;=> ({:foo bar})
  )

(let [map-wo-symbol-meta {'SYMBOL 'val}
      transit-w-meta (obj->str map-wo-symbol-meta)
      v (str->obj transit-w-meta)]
  (prn transit-w-meta)
  ;;=> "[\"^ \",\"~$SYMBOL\",\"~$val\"]"
  (prn v) ;;=> {SYMBOL val}
  (prn (map meta (keys v))) ;;=> (nil)
  )

fogus added a commit to cognitect/transit-java that referenced this issue Dec 14, 2021
…g resolution when present. This handles the case where a transformer may convert an object into a form that the marhsaling phase may not be able to handle. This bifurcation between the classifying tag and the marshaled form was specifically causing issues where symbol map keys with metadata did not marshal into a stringable form but was classified as stringable earlier in the write phase.
@puredanger
Copy link
Contributor

We have a fix for this prepped that you can try, it uses a git dep for transit-java that requires prepping (there is no change in transit-clj):

In your deps.edn:

{:deps
 {com.cognitect/transit-clj {:mvn/version "1.0.324"}
  com.cognitect/transit-java {:git/url "https://github.com/cognitect/transit-java.git" :git/sha "f84ca6290b7a0489a8671bc96bd76ede8b391415"}}}

And then you will need to prep with:

clj -X:deps prep

And then you should be able to test this fix. Please let us know what you find!

@mk
Copy link
Author

mk commented Feb 2, 2022

@puredanger @fogus looks good to me running first tests on the repl, thank you! Will also plug this into our CI and report back if we encounter any other issues.

@puredanger
Copy link
Contributor

Released in transit-java 1.0.362 and transit-clj 1.0.329

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

5 participants