sporkmonger
Round-tripping query_values unnecessarily changes order.
puts Addressable::URI.parse('/foo?d=1&b=2&e=3&a=4&c=5').tap do |uri|
uri.query_values = uri.query_values
end.normalize
#=> /foo?a=4&b=2&c=5&d=1&e=3
The 'problem' with this is that if I am cleaning an existing URI by removing specific query string values, the URI almost always changes, even when no actual changes occurred.
The culprit is the following code:
# Useful default for OAuth and caching.
# Only to be used for non-Array inputs. Arrays should preserve order.
new_query_values.sort!
Ruby's hashes are intentionally ordered. IMHO the order should be preserved. If I wanted it sorted, I could sort it before passing to the method. With this code in place I cannot as-easily round-trip a URI's parameters through this and have them unchanged. (I'd have to get the values, check to see if any of the keys to be removed existed, and only set if the hash did not change.)
If this sort! is not removed wholesale, I request that an option be added to not use it, or a separate method (uri.sorted_query_values= ?) that does not sort.
normalized_query(:sorted) and normalized_query() behave the way you'd expect, but there's no analog for query_values. Ruby's hashes have not always been ordered, and until very recently, Addressable had explicit support for 1.8.x where they aren't ordered. Round-tripping a hash assignment there would have dropped your ordering independent of underlying implementation.
That said, we no longer support 1.8.x and I'm not sure that optimizing for the cache/OAuth use-case here offers any real-world value, so I think you're right that this needs to come out. However, it's also a breaking change so it'll need to wait at least until I cut the 2.4.0 release.
