clojurewerkz / spyglass

A Clojure Memcached client (also: Couchbase, Kestrel). Built on top of SpyMemcached, supports ASCII and binary protocols, strives to be 100% feature complete.
http://clojurememcached.info
67 stars 17 forks source link

Booleans deserialized as new instances of java.lang.Boolean - breaks if, when etc #15

Closed rsslldnphy closed 9 years ago

rsslldnphy commented 9 years ago

Hi, I'm pretty sure this is a bug in the upstream project rather than this one, but I'm raising an issue here as I think you may be able to help as it seems Clojure specific.

If you persist an EDN value that contains a boolean into memcached, when you read it in, the boolean is deserialized as a new instance of java.lang.Boolean - so a false value is not identical? to false, which breaks if, when etc. This doesn't happen when you persist just a boolean by itself - so it seems to be something to do with the serialization/deserialization of EDN.

Here's a REPL session to illustrate what I mean:

user=> (.set c "abc" 3000 {:superuser false})
#<OperationFuture net.spy.memcached.internal.OperationFuture@2b66969c>
user=> (def user (.get c "abc"))
#'user/user
user=> user
{:superuser false}
user=> (when (:superuser user) (println "Do very secure thing!"))
Do very secure thing!
nil
user=> (class (:superuser user))
java.lang.Boolean
user=> (identical? (:superuser user) false)
false

user=> (.set c "abc" 3000 false)
#<OperationFuture net.spy.memcached.internal.OperationFuture@3bba6d59>
user=> (.get c "abc" )
false
user=> (identical? (.get c "abc" ) false)
true
michaelklishin commented 9 years ago

You are correct, Spyglass performs no serialisation.

rsslldnphy commented 9 years ago

Yes I see that. As I said I created an issue here as I thought you might be able to offer some help? It doesn't happen for plain booleans, only for Clojure datastructures that contain them. Have you never run into any Clojure-specific serialization problems that might provide a hint as to where to look for the problem?

I also asked here because it's Clojure users that will be affected by this bug - I imagined posting an issue on the Java project would elicit a shrug. But I will post there as well.

michaelklishin commented 9 years ago

SpyMemcached has transcoders and you should be able to provide your own. Let me know if you find a solution as I'd be happy to use a different default transcoder in Spyglass.

rsslldnphy commented 9 years ago

Aye, I think I've tracked it down in SerializingTranscoder.

Would there be anything wrong with just doing a pr-str before sending to spymemcached and an edn/read-string after reading back? I can't think of any obvious sensible cases where you'd want to cache, from Clojure, data that couldn't be serialized as EDN (but there may well be cases I haven't thought of of course).

Upgrading to a version that did this would invalidate any data currently in memcached, but I think that would happen with almost any transcoder change.

I can't think of any other sensible approaches, so if this doesn't sound like an option I'll at least get together a pull request to add a warning to the docs to say you should make sure to apply your choice of serialization for compound values.