Conflict between Record and IPersistentMap

[martinraison]

I have a record like this:

(defrecord Foo [a b])

and I would like to make it serializable, like this:

(extend-msgpack
  Foo
  21
  [v] (msgpack/pack (into {} v))
  [bytes] (map->Foo (msgpack/unpack bytes)))

However, this causes undefined behavior because a Foo record is also an IPersistentMap. Sometimes the record gets serialized correctly, sometimes it gets serialized as a simple map.

What's the best solution to this problem?

[martinraison]

For now, I managed to hack my way around the issue by overriding the implementation of Packable for IPersistentMap (and duplicating a bunch of code along the way):

(require '[msgpack.core :as msgpack])
(require '[msgpack.macros :refer [extend-msgpack]])
(require msgpack.clojure-extensions)

(defonce record-type-map (atom {}))

(defmacro extend-msgpack-record [cl type map->cl]
  `(let [type# ~type]
     (swap! record-type-map assoc (.getName ~cl) type#)
     (defmethod msgpack/refine-ext type# [{:keys [~'data]}]
       (~map->cl (msgpack/unpack ~'data)))))

(defn- pack-coll
  [coll ^DataOutput s opts]
  (doseq [item coll] (msgpack/packable-pack item s opts)))

(extend-protocol msgpack/Packable
  IPersistentMap
  (packable-pack [map ^DataOutput s opts]
    (if-let [t (@record-type-map (.getName (class map)))]
      (msgpack/packable-pack (msgpack/->Ext t (msgpack/pack (into {} map))) s opts)
      (msgpack/cond-let [len (count map)
                         pairs (interleave (keys map) (vals map))]
                        (<= len 0xf)
                        (do (.writeByte s (bit-or 2r10000000 len)) (pack-coll pairs s opts))

                        (<= len 0xffff)
                        (do (.writeByte s 0xde) (.writeShort s len) (pack-coll pairs s opts))

                        (<= len 0xffffffff)
                        (do (.writeByte s 0xdf) (.writeInt s len) (pack-coll pairs s opts))))))

And then I can define an extension for a record by doing this:

(extend-msgpack-record Foo 21 map->Foo)

Since Records are a feature of the language, it would be nice if this use-case was supported by the library itself. With this workaround, I have to be very careful not to accidentally reload the msgpack.core namespace, which would reset the Packable implementation for IPersistentMap to its original state.

[edma2]

Thanks for the bug report. I'll have a look sometime this week.

[skrat]

It also puzzled me how can you extend-protocol a protocol. I thought you can only extend a particular type, not all types that implement particular protocol (IPersistentMap). I couldn't find any docs on how this is possible. It's not possible in ClojureScript. You should instead extend particular types (clojure.lang.PersistentHashMap, clojure.lang.PersistentArrayMap, etc.)

I tried to implement generic record packing/unpacking in #27 , with some success on the CLJS side. I got it working, but it was relying on record's FQN, however in CLJS advanced compilation, the names are mangled. However, I believe that you can easily do this in CLJ, on JVM. What I did was that I replaced (in msgpack.core) every call to packable-pack, with pack-value, which would check if the value is record?, and if so, would invoke pack-record (else packable-pack), which in turns packs an Ext, where the data is a pair of [(rec-fqn rec) (into {} rec)]. You can look at EDN reader for inspiration, Clojure makes this possible.