On Tue, 21 Oct 2025 15:14:48 GMT, Josiah Noel <[email protected]> wrote:
>> Now ExchangeImpl will default to having a separate attribute map for the
>> request duration.
>
> Josiah Noel has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Refactor module-info.java for imports and notes
>
> Updated import statement and modified implementation notes.
Additional feedback - do we really need two tests? I hadn't realised that they
were basically the same. I suggest to merge them (and keep the old name)
test/jdk/com/sun/net/httpserver/ExchangeAttributeTest.java line 107:
> 105: exchange.setAttribute("attr", "val");
> 106: assertEquals("val", exchange.getAttribute("attr"));
> 107: assertEquals("val",
> exchange.getHttpContext().getAttributes().get("attr"));
Suggestion:
assertEquals("context-val",
exchange.getHttpContext().getAttributes().get("attr"));
-------------
Changes requested by dfuchs (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/27652#pullrequestreview-3361503304
PR Review Comment: https://git.openjdk.org/jdk/pull/27652#discussion_r2448845528