Re: RFR: 8263506: Make sun.net.httpserver.UnmodifiableHeaders unmodifiable [v5]
> The fix makes the map in sun.net.httpserver.UnmodifiableHeaders unmodifiable > by wrapping it in an unmodifiable view. 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 12 additional commits since the last revision: - Merge branch 'master' into 8263506 - fix get method and constructor, add tests - Account for null values and change type in constructor - Merge branch 'master' into 8263506 - Merge branch 'master' into 8263506 - move constructor call in ExchangeImpl and fix indentation - wrap List with unmodifiable map and update test - remove map wrapping - fix imports - Merge branch 'master' into 8263506 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/0e53c3d5...4f2247fc - Changes: - all: https://git.openjdk.java.net/jdk/pull/3032/files - new: https://git.openjdk.java.net/jdk/pull/3032/files/ee8da732..4f2247fc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3032&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3032&range=03-04 Stats: 16876 lines in 229 files changed: 13190 ins; 2440 del; 1246 mod Patch: https://git.openjdk.java.net/jdk/pull/3032.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3032/head:pull/3032 PR: https://git.openjdk.java.net/jdk/pull/3032
Re: RFR: 8263506: Make sun.net.httpserver.UnmodifiableHeaders unmodifiable [v3]
On Tue, 6 Apr 2021 13:12:46 GMT, Julia Boes wrote: >> LGTM > > I included a null check in the constructor to account for null values of > `List`, which `Headers` allows. Fixed the indentation. > > Testing: Tier 1-3 all clear. It turns out I wasn't right in the last commit. The null check is not needed in the constructor, but in the `get` method, where `Headers::get` can return null, which can cause `Collections.unmodifiableList` to throw NPE. I updated the get method and added some more test cases to confirm equality of Headers and UnmodifiableHeaders instances. Testing: Tier 1-3 all clear. - PR: https://git.openjdk.java.net/jdk/pull/3032
Re: RFR: 8263506: Make sun.net.httpserver.UnmodifiableHeaders unmodifiable [v5]
On Wed, 7 Apr 2021 13:28:32 GMT, Julia Boes wrote: >> The fix makes the map in sun.net.httpserver.UnmodifiableHeaders unmodifiable >> by wrapping it in an unmodifiable view. > > 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 12 additional commits since > the last revision: > > - Merge branch 'master' into 8263506 > - fix get method and constructor, add tests > - Account for null values and change type in constructor > - Merge branch 'master' into 8263506 > - Merge branch 'master' into 8263506 > - move constructor call in ExchangeImpl and fix indentation > - wrap List with unmodifiable map and update test > - remove map wrapping > - fix imports > - Merge branch 'master' into 8263506 > - ... and 2 more: > https://git.openjdk.java.net/jdk/compare/eed6b83a...4f2247fc src/jdk.httpserver/share/classes/sun/net/httpserver/UnmodifiableHeaders.java line 42: > 40: h.forEach((k, v) -> unmodHeaders.put(k, > Collections.unmodifiableList(v))); > 41: this.map = Collections.unmodifiableMap(unmodHeaders); > 42: this.headers = h; Now that we have `unmodHeaders` we could simply keep it: this.headers = unmodHeaders; we no longer need to keep the original `headers` around. src/jdk.httpserver/share/classes/sun/net/httpserver/UnmodifiableHeaders.java line 56: > 54: List l = headers.get(key); > 55: return l == null ? null : Collections.unmodifiableList(l); > 56: } You could simply return `map.get(key)` here - since map is an unmodifiable map wrapping `unmodHeaders`; Or even `headers.get(key)` if `headers` is `unmodHeaders` as suggested below... - PR: https://git.openjdk.java.net/jdk/pull/3032
Re: RFR: 8263506: Make sun.net.httpserver.UnmodifiableHeaders unmodifiable [v6]
> The fix makes the map in sun.net.httpserver.UnmodifiableHeaders unmodifiable > by wrapping it in an unmodifiable view. Julia Boes has updated the pull request incrementally with one additional commit since the last revision: assign unmodHeaders instead of original headers - Changes: - all: https://git.openjdk.java.net/jdk/pull/3032/files - new: https://git.openjdk.java.net/jdk/pull/3032/files/4f2247fc..6fce0d2d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3032&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3032&range=04-05 Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3032.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3032/head:pull/3032 PR: https://git.openjdk.java.net/jdk/pull/3032
Re: RFR: 8263506: Make sun.net.httpserver.UnmodifiableHeaders unmodifiable [v6]
On Wed, 7 Apr 2021 17:02:28 GMT, Julia Boes wrote: >> The fix makes the map in sun.net.httpserver.UnmodifiableHeaders unmodifiable >> by wrapping it in an unmodifiable view. > > Julia Boes has updated the pull request incrementally with one additional > commit since the last revision: > > assign unmodHeaders instead of original headers Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3032
