url-request-format now supports jetty, yada, rails and php through th…

…e :vec-strategy option

.gitignore

@@ -1,4 +1,5 @@
 pom.xml
+/lein.ps1
 pom.xml.asc
 .lein-failures
 *jar

CHANGES.md

@@ -1,7 +1,41 @@
-release 0.6.0
+## Changes since 0.6.0
 
-- [close the apache closeable client](https://github.com/JulianBirch/cljs-ajax/pull/178)
+* Submitting a `GET` with `:params {:a [10 20]}` used to produce `?a=10&a=20` in 0.3, it now does so again. I'd like to apologise to everyone for this particular breakage, and for the long time it's taken to fix. I know a lot of people have had to work around this issue.
+* If you want to get `?a[]=10&a[]=20` instead, add `:vec-strategy :rails` to your map.
+
+## Changes since 0.5.0
+
+- Bug fix: [close the apache closeable client](https://github.com/JulianBirch/cljs-ajax/pull/178)
 - [better error messages with keywords as formats](https://github.com/JulianBirch/cljs-ajax/pull/161)
-- [PURGE method](https://github.com/JulianBirch/cljs-ajax/pull/169)
-- [support url encoding](https://github.com/JulianBirch/cljs-ajax/pull/163)
-- [Support nodejs](https://github.com/JulianBirch/cljs-ajax/pull/166)
+- The [PURGE method](https://github.com/JulianBirch/cljs-ajax/pull/169) is now supported. Whilst not part of the HTTP standard, it's sufficiently common on things like Varnish that it seems worth supporting.
+- The handling of `+` under [url encoding](https://github.com/JulianBirch/cljs-ajax/pull/163) got fixed. This was a breaking change, but too important not to fix.
+- Experimental [nodejs support](https://github.com/JulianBirch/cljs-ajax/pull/166). Whilst this works (albeit requiring you to also include @pupeno/xmlhttprequest from npm), it's not part of our test suite. We'd welcome further contributions on this.
+
+## Version 0.4.0
+
+cljs-ajax never had a stable 0.4.0 release, so there's no breaking changes.
+
+## Breaking Changes Since 0.3
+
+* EDN support is now in its own namespace: `ajax.edn`
+* The `:edn` keyword no longer works.
+* The definition of the `AjaxImpl` protocol has changed.
+* Submitting a `GET` with `:params {:a [10 20]}` used to produce `?a=10&a=20`. It now produces `?a[0]=10&a[1]=20`.
+* `js/FormData` and `js/ArrayBuffer` &c are now submitted using a `:body` tag, not the `:params` tag
+* [Interceptors](docs/interceptors.md) were added. Whilst not strictly speaking a breaking change, the optimal way of solving certain problems has definitely changed.
+* Keywords that are used as request parameter values are stringified using `(str my-keyword)` instead of `(name my-keyword)` causing leading colons to be preserved.
+
+## Breaking Changes Since 0.2
+
+* The default response format is now `:transit`.
+* The default request format is now `:transit`.
+* Format detection is now "opt in" with `ajax-request`.  See [formats.md](docs/formats.md).  It remains the default with `GET` and `POST`.  This means that code using `ajax-request` will be smaller with advanced optimizations.
+* `:is-parse-error`, `:timeout?` and `:aborted?` have been removed, in favour of `:failure`
+* `ajax-request` now has `:format` and `:response-format` parameters, same as `POST`
+* The functions that returned merged request/response formats have been removed.
+
+## Breaking Changes Since 0.1
+
+* `ajax-request`'s API has significantly changed.  It used to be pretty equivalent to GET and POST.
+* `:keywordize-keys` is now `:keywords?`
+* `:format` used to be the response format.  The request format used to always to be `:url`

README.md

@@ -46,6 +46,12 @@ The following settings affect the interpretation of JSON responses:  (You must s
 * `:keywords?` - true/false specifies whether keys in maps will be keywordized
 * `:prefix` - the prefix to be stripped from the start of the JSON response. e.g. `while(1);` which is added by some APIs to [prevent JSON hijacking](https://stackoverflow.com/questions/2669690/why-does-google-prepend-while1-to-their-json-responses).  You should *always* use this if you've got a GET request.
 
+### GET specific settings
+
+The `:vec-strategy` setting affects how sequences are written out. 
+A `:vec-strategy` of `:java` will render `{:a [1 2]}` as `a=1&a=2`.
+A `:vec-strategy` of `:rails` will render `{:a [1 2]}` as `a[]=1&a[]=2`. This is also the correct setting for working with HTTP.
+
 ### GET/POST examples
 
 ```clojure
@@ -66,7 +72,15 @@ The following settings affect the interpretation of JSON responses:  (You must s
                         :b [1 2]
                         :c {:d 3 :e 4}
                         "f" 5}})
-;;; writes "a=0&b[0]=1&b[1]=2&c[d]=3&c[e]=4&f=5"
+;;; writes "a=0&b=1&b=2&c[d]=3&c[e]=4&f=5"
+
+(GET "/hello" {:params {:a 0
+                        :b [1 2]
+                        :c {:d 3 :e 4}
+                        "f" 5}
+               :vec-strategy :rails})
+;;; writes "a=0&b[]=1&b[]=2&c[d]=3&c[e]=4&f=5"
+
 
 (GET "/hello" {:handler handler
                :error-handler error-handler})

docs/formats.md

@@ -40,6 +40,13 @@ There are functions that return request and response formats.  Most of these fun
 * `:keywords?`, which if true returns the keys as keywords and if false or unprovided returns them as strings.
 * `:raw`, if true, returns a JS object rather than a CLJS object.
 
+### URL parameters
+
+`url-request-format` takes one parameter: `vec-strategy`.
+* `:java` will render `{:a [1 2]}` as `a=1&a=2`. This works with yada, ASP and Jetty (ring). It also matches the behaviour of superagent.
+* `:rails` will render `{:a [1 2]}` as `a[]=1&a[]=2`. This is also the correct setting for working with PHP and matches the behaviour of jQuery.
+* `:indexed` will render `{:a [1 2]}` as `a[0]=1&a[1]=2`. This is mostly kept for backwards compatibility and shouldn't be used in new code.
+
 ### Detect parameters
 
 `detect-response-format` has one parameter: `:defaults`, which is a list of pairs.  The first item in the pair is a substring that starts the content type.  The second item is the response format function to call.  It will be passed the options in.  So, you can, for instance, have `:raw` set to `true` and content detection available at the same time.  If you use the zero-arity version, `:defaults` is set to `default-formats`.

src/ajax/core.cljc

@@ -1,6 +1,7 @@
 (ns ajax.core
   (:require [clojure.string :as str]
             [cognitect.transit :as t]
+            [ajax.url :as url]
             [ajax.protocols :refer
              [-body -process-request -process-response -abort -status
               -get-response-header -status-text -js-ajax-request
@@ -138,70 +139,33 @@
 
 ;;; Request Format Record
 
-#? (:cljs
-    (defn params-to-str-alt [params]
-      (if params
-        (-> params
-            clj->js
-            structs/Map.
-            query-data/createFromMap
-            .toString))))
-
-(declare param-to-str)
-
-(p/defn-curried vec-param-to-str [prefix key value]
-  (param-to-str prefix [key value]))
-
-(p/defn-curried param-to-str [prefix [key value]]
-  (let [k1 (if (keyword? key) (name key) key)
-        new-key (if prefix (str prefix "[" k1 "]") k1)]
-    (cond (string? value)
-          [[new-key value]]
-
-          (map? value)
-          (mapcat (param-to-str new-key) (seq value))
-
-          (sequential? value)
-          (apply concat (map-indexed (vec-param-to-str new-key)
-                                     (seq value)))
-
-          :else [[new-key value]])))
-
 (defn to-utf8-writer [to-str]
   #? (:cljs to-str
       :clj (fn write-utf8 [stream params]
              (doto (OutputStreamWriter. stream)
                (.write ^String (to-str params))
                (.flush)))))
 
-(defn params-to-str [params]
-  (let [url-encode-fn #? (:clj (fn [u] (java.net.URLEncoder/encode (str u) "UTF-8"))
-                          :cljs js/encodeURIComponent)]
-    (->> (seq params)
-         (mapcat (param-to-str nil))
-         (map (fn [[k v]] (str k "=" (url-encode-fn v))))
-         (str/join "&"))))
-
-(p/defn-curried uri-with-params [params params-to-str uri]
-  (if params
-    (str uri
-         (if (re-find #"\?" uri) "&" "?") ; add & if uri contains ?
-         (params-to-str params))
-    uri))
-
 (defn get-request-format [format]
   (cond
    (map? format) format
    (keyword? format) (throw-error ["keywords are not allowed as request formats in ajax calls: " format])
    (ifn? format) {:write format :content-type "text/plain"}
    :else {}))
 
-(defrecord ProcessGet [params-to-str]
+(p/defn-curried uri-with-params [{:keys [vec-strategy params]} uri]
+  (if params
+    (str uri
+         (if (re-find #"\?" uri) "&" "?") ; add & if uri contains ?
+         (url/params-to-str vec-strategy params))
+    uri))
+
+(defrecord ProcessGet []
   Interceptor
   (-process-request [_ {:keys [method] :as request}]
     (if (= method "GET")
       (reduced (update request :uri
-                       (uri-with-params (:params request) params-to-str)))
+                       (uri-with-params request)))
       request))
   (-process-response [_ response] response))
 
@@ -286,9 +250,11 @@
            :clj ["application/transit+msgpack"
                  "application/transit+json"])})))
 
-(defn url-request-format []
-  {:write (to-utf8-writer params-to-str)
-   :content-type "application/x-www-form-urlencoded; charset=utf-8"})
+(defn url-request-format
+  ([] (url-request-format {})) 
+  ([{:keys [vec-strategy]}]
+   {:write (to-utf8-writer (url/params-to-str vec-strategy))
+    :content-type "application/x-www-form-urlencoded; charset=utf-8"}))
 
 (defn raw-response-format
   ([] (map->ResponseFormat {:read -body
@@ -477,7 +443,7 @@
     (js-handler handler interceptors)
     (throw-error "No ajax handler provided.")))
 
-(def request-interceptors [(ProcessGet. params-to-str) (DirectSubmission.) (ApplyRequestFormat.)])
+(def request-interceptors [(ProcessGet.) (DirectSubmission.) (ApplyRequestFormat.)])
 
 (def default-interceptors (atom []))
 
@@ -514,8 +480,8 @@
            :transit (transit-request-format format-params)
            :json (json-request-format)
            :text (text-request-format)
-           :raw (url-request-format)
-           :url (url-request-format)
+           :raw (url-request-format format-params)
+           :url (url-request-format format-params)
            nil)))
 
 (defn keyword-response-format-element [format format-params]

src/ajax/url.cljc

@@ -0,0 +1,117 @@
+(ns ajax.url
+
+"At first blush, it's pretty bizarre that an entire file is devoted to one  
+ function, namely params-to-str, which just takes a map and converts it to
+ a querystring. However, it turns out that people sometimes want to encode
+ fairly complex maps and the behaviour in the presence of vectors/arrays
+ is controversial.
+
+ The basic question is: what {:a [1 2]} be encoded as? The correct answer
+ as far as ring is concerned is a=1&a=2. This is also true of most Java
+ implementations, ASP.NET, Angular, Haskell and even old-school ASP. This 
+ is called vec-strategy :java in the code. Rails and PHP, however, 
+ prefer a[]=1&a[]=2, which has an obvious implementation in a dynamic 
+ language. This is called vec-strategy :rails. Finally, there's what 
+ cljs-ajax (mistakenly) did between versions 0.4.0 and 0.6.x: 
+ a[0]=1&a[2]=1, which is called vec-strategy :indexed. This is retained 
+ mostly for people who need to keep compatibility with the previous behaviour.
+
+ None of these are the \"correct answer\": the HTTP standards are
+ silent on the subject, so you're left with what your server accepts, and
+ different servers have different conventions. Worse, if you send the
+ wrong convention it gets misinterpreted. Send strategy :rails to a :java
+ server and you get { \"a[]\" [1 2]}. Worse, send strategy :java to a :rails
+ server and you get { \"a\" 2 }. So it's important to know what your server's
+ convention is.
+
+ The situation for maps is simpler, pretty much everyone encodes
+ {:a {:b 1}} as \"a[b]=1\". That is, assuming they process it at all.
+ The HTTP spec is similarly silent on this and your server may get your
+ language's equivalent of { \"a[b]\" 1 }. In cases like this, you have two
+ choices 1) write your own server-side decoder or 2) don't ever send
+ nested maps.
+
+ If you ever wanted to consider exactly how bad the effect of supporting
+ a wide range of use cases, consider that this was the original code:
+
+ (defn params-to-str [params]
+    (if params
+        (-> params      
+            clj->js
+            structs/Map.
+            query-data/createFromMap
+            .toString)))
+
+ This code remains completely correct for at least 90% of actual users
+ of cljs-ajax. Now we have ~50 SLOCs acheiving much the same result.
+"
+
+#?@ (:clj  ((:require 
+             [poppea :as p]
+             [clojure.string :as str]))
+     :cljs ((:require 
+             [clojure.string :as str])
+             (:require-macros [poppea :as p])))
+
+)
+
+(defn- key-encode [key]
+  (if (keyword? key) (name key) key))
+
+(def ^:private value-encode ; why doesn't def- exist?
+    #? (:clj (fn value-encode [u] (java.net.URLEncoder/encode (str u) "UTF-8"))
+        :cljs js/encodeURIComponent))
+
+(defn- key-value-pair-to-str [[k v]] 
+       (str (key-encode k) "=" (value-encode v)))
+
+(p/defn-curried- vec-key-transform-fn [vec-key-encode k v]
+    [(vec-key-encode k) v])
+
+(defn- to-vec-key-transform [vec-strategy]
+    (let [vec-key-encode (case (or vec-strategy :java)
+                               :java (fn [k] nil) ; no subscript
+                               :rails (fn [k] "") ; [] subscript
+                               :indexed identity)] ; [1] subscript
+        (vec-key-transform-fn vec-key-encode)))
+
+
+(p/defn-curried- param-to-key-value-pairs [vec-key-transform prefix [key value]]
+    "Takes a parameter and turns it into a sequence of key-value pairs suitable
+     for passing to `key-value-pair-to-str`. Since we can have nested maps and
+     vectors, we need a vec-key-transform function and the current query key
+     prefix as well as the key and value to be analysed. Ultimately, this 
+     function walks the structure and flattens it." 
+    (let [k1 (key-encode key)
+          new-key (if prefix 
+                      (if key 
+                          (str prefix "[" k1 "]")
+                          prefix)
+                      k1)
+          recurse (param-to-key-value-pairs vec-key-transform new-key)]
+        (cond 
+            (string? value) ; string is sequential so we have to handle it separately
+            [[new-key value]]  ; ("a" 1) should be ["a" 1]
+
+            (keyword? value)
+            [[new-key (name value)]] ; (:a 1) should be ["a" 1]
+
+            (map? value)
+            (mapcat recurse (seq value)) ; {:b {:a 1}} should be ["b[a]" 1]
+
+            (sequential? value) ; behaviour depends on vec-key-transform
+            (->> (seq value)
+                 (map-indexed vec-key-transform)
+                 (mapcat recurse))
+
+            :else [[new-key value]])))
+
+(p/defn-curried params-to-str [vec-strategy params]
+    "vec-strategy is one of :rails (a[]=3&a[]=4)
+                            :java (a=3&a=4) (this is the correct behaviour and the default)
+                            :indexed (a[3]=1&a[4]=1)
+     params is an arbitrary clojure map"
+    (->> [nil params]
+         (param-to-key-value-pairs (to-vec-key-transform vec-strategy) nil)
+         (map key-value-pair-to-str)
+         (str/join "&")))

test/ajax/test/core.cljc

@@ -25,7 +25,6 @@
                       transform-opts
                       get-request-format
                       get-response-format
-                      params-to-str
                       apply-request-format
                       json-read
                       POST GET
@@ -36,18 +35,6 @@
                      [java.lang String]
                      [java.io ByteArrayInputStream])))
 
-(deftest complex-params-to-str
-  (is (= "a=0" (params-to-str {:a 0})))
-  (is (= "b[0]=1&b[1]=2" (params-to-str {:b [1 2]})))
-  (is (= "c[d]=3&c[e]=4" (params-to-str {:c {:d 3 :e 4}})))
-  (is (= "d=5" (params-to-str {"d" 5})))
-  (is (= "a=0&b[0]=1&b[1]=2&c[d]=3&c[e]=4&f=5"
-         (params-to-str {:a 0
-                         :b [1 2]
-                         :c {:d 3 :e 4}
-                         "f" 5})))
-  (is (= "a=b%2Bc" (params-to-str {:a "b+c"}))))
-
 (deftest normalize
   (is (= "GET" (normalize-method :get)))
   (is (= "POST" (normalize-method "POST"))))
@@ -131,14 +118,15 @@
                          :response-format (keyword-response-format nil {})})]
     (is (= "application/transit+json, application/transit+transit, application/json, text/plain, text/html, */*" (get headers "Accept")))))
 
+; NB This also tests that the vec-strategy has reverted to :java
 (deftest can-add-to-query-string
   (let [{:keys [uri]}
-        (process-inputs {:params {:a 3 :b "hello"}
+        (process-inputs {:params {:a [3 4] :b "hello"} 
                          :headers nil
                          :uri "/test?extra=true"
                          :method "GET"
                          :response-format (edn-response-format)})]
-    (is (= "/test?extra=true&a=3&b=hello" uri))))
+    (is (= "/test?extra=true&a=3&a=4&b=hello" uri))))
 
 (deftest use-interceptor
   (let [interceptor (to-interceptor