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

[Patrick Concannon]

On Mon, 16 Nov 2020 17:23:42 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
>> 
>>  - 8252304: Added more test cases to check header filter
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - 8252304: BiPredicate parameter added to newBuilder
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - Merge remote-tracking branch 'origin/master' into JDK-8252304
>>  - 8252304: Removed catch block from newBuilder(HttpRequest)
>>  - ... and 5 more: https://git.openjdk.java.net/jdk/compare/2415139d...03b13e7a
>
> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 323:
> 
>> 321: 
>> 322:         var r = HttpRequest.newBuilder(request, filter).build();
>> 323:         assertFalse(r.headers().map().containsKey("testName1"));
> 
> You could also additionally:
> 
> assertTrue(r.headers().firstValue("testName1").isEmpty());

Check added as requested. See https://github.com/openjdk/jdk/pull/1059/commits/68ea89e1c55735adc9230b8abc07e1df3ffc03ca

> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 330:
> 
>> 328:     public void testRemoveSingleHeaderValue(HttpRequest request) {
>> 329:         BiPredicate<String, String> filter = (n, v) ->
>> 330:                 n.equalsIgnoreCase("testName1") && !v.equalsIgnoreCase("testValue1");
> 
> Well same comment here - we need some kind of guarantee that at least one request has that config.

Check as discussed in comment above added in commit https://github.com/openjdk/jdk/pull/1059/commits/68ea89e1c55735adc9230b8abc07e1df3ffc03ca

> test/jdk/java/net/httpclient/HttpRequestNewBuilderTest.java line 341:
> 
>> 339:         BiPredicate<String, String> filter = (n, v) ->
>> 340:                 !(((n.equalsIgnoreCase("testName1") && v.equalsIgnoreCase("testValue1")))
>> 341:                 || ((n.equalsIgnoreCase("testName2") && v.equalsIgnoreCase("testValue2"))));
> 
> It would probably be much clearer if you used `BiPredicate.negate` and `BiPredicate.or` here to combine the two BiPredicates.

`Bipredicate.negate` and `BiPredicate.or` now used instead of boolean operators. See https://github.com/openjdk/jdk/pull/1059/commits/68ea89e1c55735adc9230b8abc07e1df3ffc03ca

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

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