Response rendering type dispatch bug in 0.5.0

35 views
Skip to first unread message

Constantine Vetoshev

unread,
Sep 28, 2010, 12:20:15 PM9/28/10
to comp...@googlegroups.com
Some of my Compojure handlers were unexpectedly returning nil. Here's
an example:

(defroutes cv-routes
(GET "/" [] "hi")
(GET "/request1" request
{:status 200
:headers {"Content-Type" "text/plain"}
:body "request 1"})
(GET "/request2" request
(fn [request] {:status 200
:headers {"Content-Type" "text/plain"}
:body "request 2"}))
(GET "/request3" request "request 3"))

/ and /request3 always worked, whereas /request1 and /request2
sometimes worked, and sometimes returned nil.

compojure/response.clj extends several types to include a render
method, among them, java.util.Map. Note that the two failing routes
return Ring response maps. It turned out that that the exact type of
the Renderable produced by these response maps is either
PersistentArrayMap or PersistentHashMap, and neither matched the
render method specialized on java.util.Map. Instead, the executing
render method specialized on IFn.

Both PersistentArrayMap and PersistentHashMap extend APersistentMap,
which does indeed implement Map, but directly extends AFn. AFn
implements IFn. Although I didn't dig deeply into the Clojure compiler
source, I suspect that the extend mechanism searches depth-first
through all superclasses before it starts looking through interfaces.

I wish I knew why the whole problem happens intermittently — the
unpredictable behavior made it much more difficult to track down. In
any case, changing the extend-type to specialize on
clojure.lang.APersistentMap fixed it.

Here is a patch:

diff --git a/src/compojure/response.clj b/src/compojure/response.clj
index 0636289..4beebf1 100644
--- a/src/compojure/response.clj
+++ b/src/compojure/response.clj
@@ -1,9 +1,8 @@
(ns compojure.response
"Methods for generating Ring response maps"
(:use [ring.util.response :only (response header)])
- (:import java.util.Map
- [java.io File InputStream]
- [clojure.lang IDeref IFn ISeq]))
+ (:import [java.io File InputStream]
+ [clojure.lang APersistentMap IDeref IFn ISeq]))

(defprotocol Renderable
(render [this request]
@@ -19,7 +18,7 @@
(-> (response this)
(header "Content-Type" "text/html"))))

-(extend-type Map
+(extend-type APersistentMap
Renderable
(render [this _]
(merge (response "") this)))

James Reeves

unread,
Sep 28, 2010, 6:58:46 PM9/28/10
to comp...@googlegroups.com
Good catch! I've released 0.5.1 to fix this.

- James

> --
> You received this message because you are subscribed to the Google Groups "Compojure" group.
> To post to this group, send email to comp...@googlegroups.com.
> To unsubscribe from this group, send email to compojure+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/compojure?hl=en.
>
>

Reply all
Reply to author
Forward
0 new messages