search

taoensso / nippy

The fastest serialization library for Clojure
https://www.taoensso.com/nippy
Eclipse Public License 1.0
1.04k stars 60 forks source link

Switch reader to safe EDN reader #25

Closed weavejester closed 11 years ago

weavejester commented 11 years ago

Fixes issue #15 (and probably #23 as well) by replacing the unsafe Clojure reader with the EDN reader from the Clojure tools.reader project.

weavejester commented 11 years ago

Actually, I've just realized this will break reader fallback for serialization and deserialization of records, so don't merge yet.

weavejester commented 11 years ago

Some investigation reveals that, as per #24, Nippy is not currently able to serialize records anyway via the reader fallback. Raw types from deftype are also unserializable by default unless one of the print multimethods is implemented, or unless a custom Nippy serialization is designed.

So unless I'm missing something, this shouldn't break any existing functionality... a second opinion would be very welcome, however.

ptaoussanis commented 11 years ago

Hi James, thanks a lot for this (and #24)!

Busy heading to bed now, but will definitely take a look at this tomorrow and get back to you ASAP.

ptaoussanis commented 11 years ago

Looks good to me. Going to merge to test the changes locally. One thing we might want to add is a :readers option for passing to edn/read-string since that doesn't seem to check the *data-readers* binding.

weavejester commented 11 years ago

Now that I look into it, that's very true. Should the option be called :readers or the more specific :edn-readers?

ptaoussanis commented 11 years ago

Okay just pushed to the master branch with all your changes and a couple of tweaks (notably the IPersistentCollection change).

Now that I look into it, that's very true. Should the option be called :readers or the more specific :edn-readers?

Another alternative: how would you feel if we just use *data-readers* as the :readers arg for edn/read-string? Not sure how often people are going to be wanting to adjust readers on an ad-hoc basis...

weavejester commented 11 years ago

Seems reasonable, though to be honest it doesn't affect me much. If I have a custom tagged literal I want to serialize, I'd probably just write a custom freeze/thaw for it as well.