dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.77k stars 408 forks source link

allow nested array query params #629

Open GordianNaught opened 1 year ago

GordianNaught commented 1 year ago

The only issue I foresee is the handling of empty dictionary query params. I'm assuming, based on the lack of test coverage, that this isn't a real use case?

dakrone commented 1 year ago

@GordianNaught it looks like this causes a number of test failures (with lein test :all), with regard to changed multi-value query params:

lein test :only clj-http.test.client-test/multi-valued-query-params

FAIL in (multi-valued-query-params) (client_test.clj:1698)
multi-valued query params in indexed-style
a[1]=2&a[2]=3&b[0]=x&b[2]=z&a[0]=1&b[1]=y
expected: (.contains query-string "b[0]=x&b[1]=y&b[2]=z")
  actual: false

(It also changes things with empty offsets like a[]=1&a[]=2)

GordianNaught commented 1 year ago

I just looked at those tests. I have to make the wrap-nested-params middleware aware of the multi-param-style and the encoding. I can do that, but this gets wonky pretty quickly as I can't recur on multi-param-entries. As it is currently written, multi-param-entries will not look deeper into the nested structure. This looks like it will require a larger refactor than I was anticipating to support these other parameter styles.