RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v6]

Patrick Concannon pconcannon at openjdk.java.net
Mon Nov 16 15:54:19 UTC 2020


On Fri, 13 Nov 2020 17:16:21 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 171:
>> 
>>> 169:         while (iter1.hasNext() && iter2.hasNext())
>>> 170:             assertEquals(iter1.next(), iter2.next());
>>> 171:     }
>> 
>> This code doesn't seem to be correct. It won't work if the `BiPredicate` vetoes only 1 value for a multi-valued request. This lets me think that this scenario is not tested, or if it is tested, then it's not tested with this method.
>> A simpler implementation would be to rely on map equality. That should work as both map are case insensitive.
>> That is something like:
>> (r1.headers().map(), HttpHeaders.of(r2.headers().map(), filter));```
>> assuming r1 is the request in which headers have been removed according to `filter`.
>
> Or if you don't want to trust `HttpHeaders.of` then you should probably use a `record` and `mapMulti` to build a stream of `(name, value)` pairs and then reassemble the map in the end (possibly using `groupingBy`).

I removed the stream check and replaced it with an assert based on map equality. I've also added more test cases to check the scenarios suggested above. See https://github.com/openjdk/jdk/pull/1059/commits/03b13e7aa9ff7bc222bd0c466b223ec522ff2973

-------------

PR: https://git.openjdk.java.net/jdk/pull/1059


More information about the net-dev mailing list