ffang commented on PR #3620: URL: https://github.com/apache/cxf/pull/3620#issuecomment-6444976345
> Thanks a lot @ffang , I like this version much better, a few comments to make implementation a bit cleaner (in my opinion), thank you for working it through! Thanks @reta for the review, the suggestions made it cleaner. I've pushed 7e68f3b on top of cb45590 (no history rewrite this time): - **Immutable view**: the `ServletResponseHeaders` constructor is now private and a `ServletResponseHeaders.of(message, response)` factory always returns an unmodifiable view, so it no longer relies on the caller to wrap it. The test now also checks that `remove` and `clear` throw `UnsupportedOperationException`. - **Static method**: `Headers.getResponseHeaders(Message, HttpServletResponse)` is now a single static method, and the helpers it uses (`getContentTypeFromMessage`, `isResponseBodyAvailable`, `isSingleHeader`, `headerObjectToString`) are static too. - **Constructor removed**: the private `Headers(Message, Map)` constructor and the instance `getResponseHeaders` are gone. - **`new Headers(message)`**: I kept away from this one on purpose. That constructor calls `getSetProtocolHeaders(message)`, which writes to the message (it stores `PROTOCOL_HEADERS` or replaces a `HashMap` with a `TreeMap` copy). The view is read during logging and shouldn't change the message, so it only reads `PROTOCOL_HEADERS`. I added a Javadoc note explaining why. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
