On Mon, 16 Nov 2020 17:23:42 GMT, Daniel Fuchs <dfu...@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
