garydgregory commented on code in PR #706:
URL: https://github.com/apache/commons-compress/pull/706#discussion_r2346896096
##########
src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java:
##########
@@ -88,6 +89,20 @@ public Iterator<E> unwrap() {
}
+ /**
+ * Generic builder for ArchiveInputStream instances.
+ *
+ * @param <T> The type of {@link ArchiveInputStream} to build.
+ * @param <B> The type of the concrete Builder.
+ * @since 1.29.0
+ */
+ public abstract static class Builder<T extends ArchiveInputStream<?>, B
extends Builder<T, B>> extends AbstractStreamBuilder<T, B> {
Review Comment:
If a Builder class is abstract, then please prefix its name with "Abstract"
(like its superclass in this case).
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
Review Comment:
Javadoc: "Sets..."
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
Review Comment:
Javadoc.
##########
src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java:
##########
@@ -39,6 +39,30 @@
*/
public class ArArchiveInputStream extends ArchiveInputStream<ArArchiveEntry> {
+ /**
+ * Builds a new {@link ArArchiveInputStream}.
+ * <p>
+ * For example:
Review Comment:
There is no need to indent text in paragraph blocks, especially since these
spaces will be stripped out.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
+ * {@code false} to stop
at the compressed size.
+ * @return this
Review Comment:
Other builders we have are less terse for these types of setters: `@return
{@code this} instance.`
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -320,14 +445,32 @@ public ZipArchiveInputStream(final InputStream
inputStream, final String encodin
* this to true if you want to
read a split archive.
* @since 1.20
*/
- public ZipArchiveInputStream(final InputStream inputStream, final String
encoding, final boolean useUnicodeExtraFields,
- final boolean allowStoredEntriesWithDataDescriptor, final boolean
skipSplitSig) {
- super(inputStream, encoding);
+ public ZipArchiveInputStream(
+ final InputStream inputStream,
+ final String encoding,
+ final boolean useUnicodeExtraFields,
+ final boolean allowStoredEntriesWithDataDescriptor,
+ final boolean skipSplitSig) {
+ this(
+ inputStream,
+ builder()
+ .setCharset(encoding)
+ .setUseUnicodeExtraFields(useUnicodeExtraFields)
+
.setAllowStoredEntriesWithDataDescriptor(allowStoredEntriesWithDataDescriptor)
+ .setSkipSplitSig(skipSplitSig));
+ }
+
+ protected ZipArchiveInputStream(final AbstractBuilder<?, ?> builder)
throws IOException {
Review Comment:
Javadoc.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
+ * {@code false} to stop
at the compressed size.
+ * @return this
+ */
+
Review Comment:
Remove extra blank line.
##########
src/main/java/org/apache/commons/compress/archivers/ArchiveInputStream.java:
##########
@@ -88,6 +89,20 @@ public Iterator<E> unwrap() {
}
+ /**
+ * Generic builder for ArchiveInputStream instances.
+ *
+ * @param <T> The type of {@link ArchiveInputStream} to build.
+ * @param <B> The type of the concrete Builder.
+ * @since 1.29.0
+ */
+ public abstract static class Builder<T extends ArchiveInputStream<?>, B
extends Builder<T, B>> extends AbstractStreamBuilder<T, B> {
+
+ protected Builder() {
Review Comment:
New protected methods should also have a Javadoc, in this case: "Constructs
a new instance."
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
+ * {@code false} to stop
at the compressed size.
+ * @return this
+ */
+
+ public B setAllowStoredEntriesWithDataDescriptor(final boolean
allowStoredEntriesWithDataDescriptor) {
+ this.allowStoredEntriesWithDataDescriptor =
allowStoredEntriesWithDataDescriptor;
+ return asThis();
+ }
+
+ protected boolean isAllowStoredEntriesWithDataDescriptor() {
Review Comment:
Javadoc
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
Review Comment:
The "allow" prefix seems redundant to me and "with" is kinda wierd (to me).
How about "storedDescribedEntries"?
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
+ * {@code false} to stop
at the compressed size.
+ * @return this
+ */
+
+ public B setAllowStoredEntriesWithDataDescriptor(final boolean
allowStoredEntriesWithDataDescriptor) {
+ this.allowStoredEntriesWithDataDescriptor =
allowStoredEntriesWithDataDescriptor;
+ return asThis();
+ }
+
+ protected boolean isAllowStoredEntriesWithDataDescriptor() {
+ return allowStoredEntriesWithDataDescriptor;
+ }
+
+ /**
+ * Configures whether the stream should skip the ZIP split signature
Review Comment:
The convention I've been using is that a setter method "Sets...:", a getter
method "Gets...", and an "isFoo()" method "Tests..." or "Tests whether...".
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
Review Comment:
Javadoc.
##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java:
##########
@@ -77,6 +77,112 @@
*/
public class ZipArchiveInputStream extends ArchiveInputStream<ZipArchiveEntry>
implements InputStreamStatistics {
+ /**
+ * Abstract builder for derived classes of {@link ZipArchiveInputStream}.
+ *
+ * @param <T> The type of the {@link ZipArchiveInputStream}.
+ * @param <B> The type of the builder itself.
+ * @since 1.29.0
+ */
+ public abstract static class AbstractBuilder<T extends
ZipArchiveInputStream, B extends AbstractBuilder<T, B>>
+ extends ArchiveInputStream.Builder<T, B> {
+ private boolean useUnicodeExtraFields = true;
+ private boolean allowStoredEntriesWithDataDescriptor;
+ private boolean skipSplitSig;
+
+ protected AbstractBuilder() {
+ setCharset(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Controls whether to use InfoZIP Unicode Extra Fields (if present)
to set the file names.
+ *
+ * <p>This feature is enabled by default.</p>
+ *
+ * @param useUnicodeExtraFields If {@code true} Unicode Extra Fields
should be used.
+ * @return this
+ */
+ public B setUseUnicodeExtraFields(final boolean useUnicodeExtraFields)
{
+ this.useUnicodeExtraFields = useUnicodeExtraFields;
+ return asThis();
+ }
+
+ protected boolean isUseUnicodeExtraFields() {
+ return useUnicodeExtraFields;
+ }
+
+ /**
+ * Controls whether the stream attempts to read STORED entries that
use a data descriptor.
+ *
+ * <p>If set to {@code true}, the stream will not stop reading an
entry at the
+ * declared compressed size. Instead, it will continue until a data
descriptor
+ * is encountered (by detecting the Data Descriptor Signature). This
may cause
+ * issues in certain cases, such as JARs embedded in WAR files.</p>
+ *
+ * <p>See <a
href="https://issues.apache.org/jira/browse/COMPRESS-555";>COMPRESS-555</a>
+ * for details.</p>
+ *
+ * <p>This feature is disabled by default.</p>
+ *
+ * @param allowStoredEntriesWithDataDescriptor {@code true} to read
STORED entries with data descriptors,
+ * {@code false} to stop
at the compressed size.
+ * @return this
+ */
+
+ public B setAllowStoredEntriesWithDataDescriptor(final boolean
allowStoredEntriesWithDataDescriptor) {
+ this.allowStoredEntriesWithDataDescriptor =
allowStoredEntriesWithDataDescriptor;
+ return asThis();
+ }
+
+ protected boolean isAllowStoredEntriesWithDataDescriptor() {
+ return allowStoredEntriesWithDataDescriptor;
+ }
+
+ /**
+ * Configures whether the stream should skip the ZIP split signature
+ * ({@code 08074B50}) at the beginning of the input.
+ *
+ * <p>Disabled by default.</p>
+ *
+ * @param skipSplitSig {@code true} to skip the ZIP split signature,
{@code false} otherwise
+ * @return this
+ */
+ public B setSkipSplitSig(final boolean skipSplitSig) {
+ this.skipSplitSig = skipSplitSig;
+ return asThis();
+ }
+
+ protected boolean isSkipSplitSig() {
Review Comment:
Using an abbreviation like "Sig" could be confusing; I'd just say
"Signtature", assuming that is what we're talking about here. Also: Javadoc
which would remove any confusion at all.
--
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]
