> Can I please get a review of this enhancement which proposes to enhance 
> `java.util.zip.Deflater/Inflater` classes to now implement `AutoCloseable`?
> 
> The actual work for this was done a few years back when we discussed the 
> proposed approaches and then I raised a RFR. At that time I couldn't take 
> this to completion. The current changes in this PR involve the implementation 
> that was discussed at that time and also have implemented the review 
> suggestions from that time. Here are those previous discussions and reviews:
> 
> https://mail.openjdk.org/pipermail/core-libs-dev/2019-June/061079.html
> https://mail.openjdk.org/pipermail/core-libs-dev/2019-July/061177.html
> https://mail.openjdk.org/pipermail/core-libs-dev/2019-July/061229.html
> 
> To summarize those discussions, we had concluded that:
> - `Deflater` and `Inflater` will implement the `AutoCloseable` interface
> -  In the `close()` implementation we will invoke the `end()` method (`end()` 
> can be potentially overridden by subclasses).
> - `close()` will be specified and implemented to be idempotent. Calling 
> `close()` a second time or more will be a no-op.
> - Calling `end()` and then `close()`, although uncommon, will also support 
> idempotency and that `close()` call will be a no-op.
> - However, calling `close()` and then `end()` will not guarantee idempotency 
> and depending on the implementing subclass, the `end()` may throw an 
> exception.
> 
> New tests have been included as part of these changes and they continue to 
> pass along with existing tests in tier1, tier2 and tier3. When I had 
> originally added these new tests, I hadn't used junit. I can convert them to 
> junit if that's preferable.
> 
> I'll file a CSR shortly.

Jaikiran Pai 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 18 additional commits since the 
last revision:

 - update the class level documentation of Inflater to match the updates in 
Deflater
 - merge latest from master branch
 - improve Deflater class level doc
 - Stuart's review - improve end() API doc
 - merge latest from master branch
 - missed a few methods for specifying IllegalStateException
 - Lance's review - specify methods that throw IllegalStateException if the 
Inflater/Deflater is closed
 - Roger's suggestion - Make Inflater.close() and Deflater.close() final, also 
update the new tests to match this change
 - merge latest from master branch
 - update tests to match the new specification
 - ... and 8 more: https://git.openjdk.org/jdk/compare/fa33f683...f155bf73

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/19675/files
  - new: https://git.openjdk.org/jdk/pull/19675/files/19f2db96..f155bf73

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19675&range=09
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19675&range=08-09

  Stats: 27066 lines in 378 files changed: 18357 ins; 7000 del; 1709 mod
  Patch: https://git.openjdk.org/jdk/pull/19675.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19675/head:pull/19675

PR: https://git.openjdk.org/jdk/pull/19675

Reply via email to