Optimized Uri.withQuery

[davidik1]

Uri.withQuery receives a Query object, converts it to a string, and then calls the Uri.copy method which creates a new Uri object with the given query (The Uri class holds the query as an un-parsed string).
The copy method ends up in queryString.map(new UriParser(_).parseRawQueryString()) which percent-encodes the query string.
As in this flow, the query string is converted from a Query object, it is always percent-encoded, so the additional encoding operation is unnecessary. We can skip it by replacing the call to copy with createUnsafe.

[raboof]

Logic-wise makes sense to me.

UriSpec.scala already has some tests such as:

uri.withQuery(Query("param1" -> "val\"ue1")) shouldEqual Uri("http://host/path?param1=val%22ue1#fragment")

This is a good start, but perhaps it would be good to add some more test cases with Query instances that are created with the other Query constructors to validate that these also produce the correct results. I'm pretty convinced that this implementation will indeed behave correctly, but good to codify.

[YoniKF]

Consider adding a copyUnsafe method and calling it here and also in all the other with... methods (this assumes that the query stored inside Uri is also already encoded).

[He-Pin]

lgtm, thanks

[pjfanning]

lgtm

[pjfanning]

fails to compile in scala 2.12 build

[error] /home/runner/work/pekko-http/pekko-http/http-core/src/test/scala/org/apache/pekko/http/scaladsl/model/UriSpec.scala:824:36: value addOne is not a member of scala.collection.mutable.Builder[(String, String),org.apache.pekko.http.scaladsl.model.Uri.Query]
[error]       val query = Query.newBuilder.addOne("param1" -> "val\"ue1").result()
[error]                                    ^
Error: value addOne is not a member of scala.collection.mutable.Builder[(String, String),org.apache.pekko.http.scaladsl.model.Uri.Query]

You need to avoid the addOne function.

[pjfanning]

Thanks for the fast turnaround on the compile issue.