lambdaisland / uri

A pure Clojure/ClojureScript URI library
Mozilla Public License 2.0
243 stars 21 forks source link

Provide custom target data structure for query-string->map #37

Closed ilmoraunio closed 1 year ago

ilmoraunio commented 1 year ago

This is a minimal approach to fix an ordering issue which query-string parsing will experience when there are more than 8 query-string arguments.

Fixes #35 .

plexus commented 1 year ago

@borkdude @alysbrooks any idea why the bb tests are failing here? It seems like these same tests ran fine in the past. And I don't think this PR should be causing this kind of failure...

----- Error --------------------------------------------------------------------
Type:     clojure.lang.ExceptionInfo
Message:  Unable to resolve classname: uri/URI
Data:     {:type :sci/error, :line 12, :column 1, :file "/root/project/test/lambdaisland/uri_test.cljc", :phase "analysis"}
Location: /root/project/test/lambdaisland/uri_test.cljc:16:7
Phase:    analysis

----- Context ------------------------------------------------------------------
12: (deftest parsing
13:   (testing "happy path"
14:     (are [x y] (= y (uri/parse x))
15:       "http://user:password@example.com:8080/path?query=value#fragment"
16:       (uri/URI. "http" "user" "password" "example.com" "8080" "/path" "query=value" "fragment")
          ^--- Unable to resolve classname: uri/URI
17: 
18:       "/happy/path"
19:       (uri/URI. nil nil nil nil nil "/happy/path" nil nil)
20: 
21:       "relative/path"

----- Stack trace --------------------------------------------------------------
lambdaisland.uri-test                     - /root/project/test/lambdaisland/uri_test.cljc:16:7
clojure.core/list                         - <built-in>
clojure.core/let                          - <built-in>
lambdaisland.uri-test                     - /root/project/test/lambdaisland/uri_test.cljc:14:5
clojure.template/do-template              - <built-in>
... (run with --debug to see elided elements)
clojure.core/fn                           - <built-in>
lambdaisland.uri-test                     - /root/project/test/lambdaisland/uri_test.cljc:12:1
lambdaisland.uri-test                     - /root/project/test/lambdaisland/uri_test.cljc:12:1
lambdaisland.uri-test                     - /root/project/test/lambdaisland/uri_test.cljc:12:1
user-d53c38a3-3638-444d-84e0-f59c948e4f9d - <expr>:4:47

Exited with code exit status 1
plexus commented 1 year ago

Looks good @ilmoraunio , a simple and minimal solution! Thanks!

plexus commented 1 year ago

Released in v1.15.125

[lambdaisland/uri "1.15.125"]                 ;; deps.edn
{lambdaisland/uri {:mvn/version "1.15.125"}}  ;; project.clj
borkdude commented 1 year ago

@plexus Yes, details here: https://github.com/babashka/babashka/issues/1502

plexus commented 1 year ago

Thank you! Sorry for the noise, I wasn't aware this was already reported.

borkdude commented 1 year ago

No problem!

alysbrooks commented 1 year ago

I opened #42 to remove this from our code base.