Add URI::Params#merge, #merge! and URI#update_query_params

[skinnyjames]

This PR adds 3 new methods in the URI namespace.

2 utility methods on URI::Params for merging other URI::Params, destructively and non-destructively.

• merge(other : URI::Params, *, replace : Bool) : URI::Params
• merge!(other : URI::Params, *, replace : Bool) : URI::Params

It also adds a method on URI to mutate query_params on the URI object in place.

• update_query_params(&) : URI::Params

Happy to speak more to it, modify, or close if needed.

closes #13399

[straight-shoota]

Just a note about the implementation: I think it would be cleaner and more efficient to iterate params.raw_params. That avoids the extra complexity for keeping track of whether a key was already assigned from params.
I figure this could be quite simple using Hash#merge!. The yielding variant would be great for replace: false.

[skinnyjames]

Just a note about the implementation: I think it would be cleaner and more efficient to iterate params.raw_params. That avoids the extra complexity for keeping track of whether a key was already assigned from params. I figure this could be quite simple using Hash#merge!. The yielding variant would be great for replace: false.

Great call, I wasn't aware of block form of Hash#merge!. Thanks.

Would it be more performant for the non-destructive URI::Params#merge to just use Hash#merge as well? or is dup.merge! more concise?

Nevermind. It's been a bit since I looked at this PR, this doesn't make sense

[straight-shoota]

There's a bug in merge! where the array value needs to be duped.

I also made a couple of suggestions for improving the API docs.

[straight-shoota]

Suggested change

	@raw_params.merge!(params.raw_params)
	@raw_params.merge!(params.raw_params) { |_, first, second| second.dup }

The array needs to be duped to prevent mutations on either self or params to reflect on the other.

a = URI::Params.parse("foo=bar")
b = URI::Params.parse("foo=baz")
b.merge!(a)
b.add("foo", "qux")
a.add("foo", "qax")
a # => URI::Params{"foo" => ["bar", "qux", "qax]}
b # => URI::Params{"foo" => ["bar", "qux", "qax]}

Could you add a spec for this please?

[skinnyjames]

If Hash#merge! is causing state to leak, would the solve using reduce be more explicit?

I'll add these changes + spec, but I personally find it confusing to follow nested #dups

[straight-shoota]

Using Hash#merge! is fine and safe. The only reason this dup is necessary is that the Array representation is an implementation detail of URI::Params and not exposed.