Re: RFR: 8266250: WebSocketTest and WebSocketProxyTest call assertEquals(List, List)
On Wed, 28 Apr 2021 21:22:23 GMT, Daniel Fuchs wrote: > It also avoids relying on an unspecified behavior of `Assert.assertEquals`. Which behavior is that? If I recall correctly, the test was written for TestNG 6.9.5, which provides a [method to compare collections](https://github.com/cbeust/testng/blob/ef2d1199abff4e1b8fa4b1148c1314e776d7a044/src/main/java/org/testng/Assert.java#L508-L517). - PR: https://git.openjdk.java.net/jdk/pull/3776
RFR: 8266250: WebSocketTest and WebSocketProxyTest call assertEquals(List, List)
Please find here an almost trivial test fix that should improve diagnostic in case of failures. It also avoids relying on an unspecified behavior of `Assert.assertEquals`. - Commit messages: - 8266250: WebSocketTest and WebSocketProxyTest call assertEquals(List, List) Changes: https://git.openjdk.java.net/jdk/pull/3776/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3776&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266250 Stats: 50 lines in 2 files changed: 48 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3776.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3776/head:pull/3776 PR: https://git.openjdk.java.net/jdk/pull/3776
Re: RFR: 8266155: Convert java.base to use Stream.toList() [v2]
On Wed, 28 Apr 2021 16:57:25 GMT, Ian Graves wrote: >> 8266155: Convert java.base to use Stream.toList() > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Removing redundant imports Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3734
Re: RFR: 8266155: Convert java.base to use Stream.toList() [v2]
On Wed, 28 Apr 2021 16:57:25 GMT, Ian Graves wrote: >> 8266155: Convert java.base to use Stream.toList() > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Removing redundant imports Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3734
Re: RFR: 8266155: Convert java.base to use Stream.toList() [v2]
> 8266155: Convert java.base to use Stream.toList() Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Removing redundant imports - Changes: - all: https://git.openjdk.java.net/jdk/pull/3734/files - new: https://git.openjdk.java.net/jdk/pull/3734/files/3fb335e8..a0242a13 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3734&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3734&range=00-01 Stats: 7 lines in 7 files changed: 0 ins; 7 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3734.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3734/head:pull/3734 PR: https://git.openjdk.java.net/jdk/pull/3734
Re: RFR: 8266155: Convert java.base to use Stream.toList()
On Wed, 28 Apr 2021 15:41:31 GMT, Chris Hegarty wrote: >> 8266155: Convert java.base to use Stream.toList() > > src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 6788: > >> 6786: } >> 6787: >> 6788: /** > > I think the import of Collectors can be dropped in this file? And maybe a few > other files too? The import can be dropped from the jdk.internal.module.* classes too. jdk.internal.module.IllegalAccessLogger will likely be removed as part of JEP 403. - PR: https://git.openjdk.java.net/jdk/pull/3734
Re: RFR: 8266155: Convert java.base to use Stream.toList()
On Tue, 27 Apr 2021 21:34:02 GMT, Ian Graves wrote: > 8266155: Convert java.base to use Stream.toList() Marked as reviewed by chegar (Reviewer). src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 6788: > 6786: } > 6787: > 6788: /** I think the import of Collectors can be dropped in this file? And maybe a few other files too? - PR: https://git.openjdk.java.net/jdk/pull/3734
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v6]
> Add two static factory methods to com.sun.net.httpserver.Filter that
> facilitate the creation of pre- and post-processing Filters:
>
> `public static Filter beforeResponse(String description,
> Consumer filterImpl) {}`
> `public static Filter afterResponse(String description,
> Consumer filterImpl) {}`
Julia Boes 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 ten additional commits since the
last revision:
- update @return description
- Merge branch 'master' into 8265123
- some more spec updates and small test cleanup
- update specs and small fix in test
- fix empty response body
- update specs and add test
- update specs and expand test
- initial commit
- (drive-by) add detail msg for AssertionError
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/3468/files
- new: https://git.openjdk.java.net/jdk/pull/3468/files/0e76acd4..cba4b885
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=05
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=04-05
Stats: 563626 lines in 5254 files changed: 44584 ins; 511080 del; 7962 mod
Patch: https://git.openjdk.java.net/jdk/pull/3468.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3468/head:pull/3468
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v6]
On Wed, 28 Apr 2021 14:32:30 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes 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 ten additional commits since
> the last revision:
>
> - update @return description
> - Merge branch 'master' into 8265123
> - some more spec updates and small test cleanup
> - update specs and small fix in test
> - fix empty response body
> - update specs and add test
> - update specs and expand test
> - initial commit
> - (drive-by) add detail msg for AssertionError
Marked as reviewed by dfuchs (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v5]
On Wed, 28 Apr 2021 13:20:23 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> some more spec updates and small test cleanup
I wonder if instead of simply saying:
* @return a filter
we should say:
* @return a filter whose operation is invoked before the exchange is handled
* @return a filter whose operation is invoked after the exchange is handled
Otherwise, LGTM!
No need for a new review if you take on these changes.
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v5]
On Wed, 28 Apr 2021 13:20:23 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> some more spec updates and small test cleanup
Marked as reviewed by michaelm (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v5]
> Add two static factory methods to com.sun.net.httpserver.Filter that
> facilitate the creation of pre- and post-processing Filters:
>
> `public static Filter beforeResponse(String description,
> Consumer filterImpl) {}`
> `public static Filter afterResponse(String description,
> Consumer filterImpl) {}`
Julia Boes has updated the pull request incrementally with one additional
commit since the last revision:
some more spec updates and small test cleanup
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/3468/files
- new: https://git.openjdk.java.net/jdk/pull/3468/files/8751f654..0e76acd4
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=03-04
Stats: 24 lines in 2 files changed: 1 ins; 2 del; 21 mod
Patch: https://git.openjdk.java.net/jdk/pull/3468.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3468/head:pull/3468
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v4]
On Wed, 28 Apr 2021 11:29:18 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update specs and small fix in test
Changes requested by michaelm (Reviewer).
src/jdk.httpserver/share/classes/com/sun/net/httpserver/Filter.java line 149:
> 147: * operation.
> 148: *
> 149: * The {@link #description() description} describes the returned
> filter.
Maybe, delete the first sentence here. I don't think it adds much value. Then
the @param for description could say: The string to be returned from {@link
#description()}
-
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v4]
On Wed, 28 Apr 2021 11:29:18 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update specs and small fix in test
src/jdk.httpserver/share/classes/com/sun/net/httpserver/Filter.java line 164:
> 162: * since this is commonly done by the exchange handler.
> 163: *
> 164: * Example of adding the Foo response header to all responses:
Maybe `the {@code "Foo"}` ? Or `the {@code "Foo: Bar"}`?
src/jdk.httpserver/share/classes/com/sun/net/httpserver/Filter.java line 212:
> 210: * exchange or {@linkplain HttpExchange#sendResponseHeaders(int,
> long) send the response headers}.
> 211: * Doing so is likely to fail, since this is commonly done by the
> exchange
> 212: * handler.
maybe `... since the request is expected to have already been handled before
the operation is executed`?
src/jdk.httpserver/share/classes/com/sun/net/httpserver/Filter.java line 222:
> 220: * Example of adding a sequence of afterHandler filters to a
> context:
> 221: * The order in which the filters are invoked is reverse to the
> order in
> 222: * which they are added to the context's filter-list.
May need to be a bit more precise. The filters are invoked in the order they
are added, but their operations are invoked in the reverse order. I'd suggest:
* The order in which the filter's operations are invoked is reverse to the
order in
* which the filters are added to the context's filter-list.
-
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v3]
On Mon, 26 Apr 2021 17:27:13 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix empty response body
Thanks for your comments, Daniel. I incorporated them and tightened the specs
and examples.
API docs updated in place:
http://cr.openjdk.java.net/~jboes/webrevs/8265123/api/jdk.httpserver/com/sun/net/httpserver/Filter.html
-
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v4]
> Add two static factory methods to com.sun.net.httpserver.Filter that
> facilitate the creation of pre- and post-processing Filters:
>
> `public static Filter beforeResponse(String description,
> Consumer filterImpl) {}`
> `public static Filter afterResponse(String description,
> Consumer filterImpl) {}`
Julia Boes has updated the pull request incrementally with one additional
commit since the last revision:
update specs and small fix in test
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/3468/files
- new: https://git.openjdk.java.net/jdk/pull/3468/files/823bce14..8751f654
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3468&range=02-03
Stats: 54 lines in 2 files changed: 5 ins; 9 del; 40 mod
Patch: https://git.openjdk.java.net/jdk/pull/3468.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/3468/head:pull/3468
PR: https://git.openjdk.java.net/jdk/pull/3468
Re: RFR: 8265123: Add static factory methods to com.sun.net.httpserver.Filter [v4]
On Wed, 28 Apr 2021 11:26:18 GMT, Julia Boes wrote:
>> Add two static factory methods to com.sun.net.httpserver.Filter that
>> facilitate the creation of pre- and post-processing Filters:
>>
>> `public static Filter beforeResponse(String description,
>> Consumer filterImpl) {}`
>> `public static Filter afterResponse(String description,
>> Consumer filterImpl) {}`
>
> Julia Boes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> update specs and small fix in test
Marked as reviewed by chegar (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/3468
