You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/11/26 23:15:44 UTC
[GitHub] [commons-io] maxxedev opened a new pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
maxxedev opened a new pull request #171:
URL: https://github.com/apache/commons-io/pull/171
Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
PipedInput/OutputStream in JDK can have surprisingly complex behavior with respect
to how threads need to be arranged. QueueInput/OutputStream are much simpler
alternatives that is easier to use correctly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] coveralls edited a comment on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-734509980
[](https://coveralls.io/builds/35311229)
Coverage decreased (-0.03%) to 88.561% when pulling **d16b67c50d57a17feabc0359d7661adfca1af19c on maxxedev:master** into **cd5a7d7be8d4a071774ce4566231bf6f8c4f808d on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] maxxedev commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
maxxedev commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538586279
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
Review comment:
This Queue**Input**Stream method creates a Queue**Output**Stream instance. Just saying "an instance" might be confusing?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r537510489
##########
File path: src/main/java/org/apache/commons/io/output/QueueOutputStream.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.output;
+
+import org.apache.commons.io.input.QueueInputStream;
+
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedOutputStream}; queue input stream provides what's
+ * written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueOutputStream outputStream = new QueueOutputStream();
+ * QueueInputStream inputStream = outputStream.newPipeInputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueOutputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueInputStream
+ * @since 2.9.0
+ */
+public class QueueOutputStream extends OutputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueOutputStream} with no limit to internal buffer size
+ */
+ public QueueOutputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueOutputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueOutputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueInputStream connected to this. Writes to this output stream will be visible to the input stream.
+ *
+ * @return QueueInputStream connected to this stream
+ */
+ public QueueInputStream newQueueInputStream() {
+ return new QueueInputStream(queue);
+ }
+
+ /**
+ * Write a single byte.
+ *
+ * @throws InterruptedIOException if the thread is interrupted while writing to the queue.
+ */
+ @Override
+ public void write(final int b) throws InterruptedIOException {
+ try {
+ queue.put(0xFF & b);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new InterruptedIOException();
Review comment:
Is there a reason, the catch block creates a new exception instead of rethrowing the one it caught?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538613626
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
Review comment:
Ah, yes of course!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538463992
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
Review comment:
`Create` -> `Constructs` since a constructor constructs ;-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] coveralls edited a comment on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-734509980
[](https://coveralls.io/builds/35490067)
Coverage increased (+0.1%) to 88.683% when pulling **ebf67bcf9f4c560a94887e42127dd7c983143655 on maxxedev:master** into **cd5a7d7be8d4a071774ce4566231bf6f8c4f808d on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r537141093
##########
File path: src/main/java/org/apache/commons/io/output/QueueOutputStream.java
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.output;
+
+import org.apache.commons.io.input.QueueInputStream;
+
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import static java.util.Objects.requireNonNull;
Review comment:
We don't use static imports for this kind of call in the rest of the main code base. In unit tests, we use statics for JUnit methods.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-735422984
IMO, you need better tests with edge cases like input size of 0, 1, 2, 4, 8, and the "usual" buffer sizes from other streams like 4096 or the more recent 8192 (private java.io.BufferedInputStream.DEFAULT_BUFFER_SIZE).
You can parameterize the tests with normal JUnit features. Someone (like me ;-) will invariably look at the current implementation and not like the one byte at a time implementations since normal streams usually work on buffers, but that's a different story, let's start with better tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory merged pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #171:
URL: https://github.com/apache/commons-io/pull/171
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] coveralls commented on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-734509980
[](https://coveralls.io/builds/35275127)
Coverage increased (+0.03%) to 88.62% when pulling **03e6220d32f244d2fd78e59e44021b7497945d72 on maxxedev:master** into **cd5a7d7be8d4a071774ce4566231bf6f8c4f808d on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] coveralls edited a comment on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-734509980
[](https://coveralls.io/builds/35545868)
Coverage increased (+0.09%) to 88.672% when pulling **2f8e47f02d98883592b81cca539ce6c4096f7cb6 on maxxedev:master** into **cd5a7d7be8d4a071774ce4566231bf6f8c4f808d on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538462823
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
+ *
+ * @return QueueOutputStream connected to this stream
+ */
+ public QueueOutputStream newQueueOutputStream() {
+ return new QueueOutputStream(queue);
+ }
+
+ /**
+ * Read a single byte.
+ *
+ * @return either the byte read or <code>-1</code> if the end of the stream has been reached
Review comment:
You can abbreviate ` <code>-1</code>` as `{@code -1}`.
You could also avoid using a magic number and do a static import of `IOUtils.EOF` in the code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538467520
##########
File path: src/main/java/org/apache/commons/io/output/QueueOutputStream.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.output;
+
+import org.apache.commons.io.input.QueueInputStream;
+
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedOutputStream}; queue input stream provides what's
+ * written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueOutputStream outputStream = new QueueOutputStream();
+ * QueueInputStream inputStream = outputStream.newPipeInputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueOutputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueInputStream
+ * @since 2.9.0
+ */
+public class QueueOutputStream extends OutputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueOutputStream} with no limit to internal buffer size
Review comment:
-> `Creates a new instance...`
I am not a fan of creating a hyperlink to the class we are already in, it feels misleading, like we are sending someone elsewhere, when we are not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538464842
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
Review comment:
`an QueueOutputStream` -> `an instance`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538465265
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
+ *
+ * @return QueueOutputStream connected to this stream
+ */
+ public QueueOutputStream newQueueOutputStream() {
+ return new QueueOutputStream(queue);
+ }
+
+ /**
+ * Read a single byte.
Review comment:
`Read` -> `Reads`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] maxxedev commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
maxxedev commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538583703
##########
File path: src/main/java/org/apache/commons/io/input/QueueInputStream.java
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.input;
+
+import org.apache.commons.io.output.QueueOutputStream;
+
+import java.io.InputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedInputStream}; queue input stream provides what's written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueInputStream inputStream = new QueueInputStream();
+ * QueueOutputStream outputStream = inputStream.newQueueOutputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueInputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueOutputStream
+ * @since 2.9.0
+ */
+public class QueueInputStream extends InputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueInputStream} with no limit to internal buffer size
+ */
+ public QueueInputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueInputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueInputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueOutputStream connected to this. Writes to the output stream will be visible to this input stream.
+ *
+ * @return QueueOutputStream connected to this stream
+ */
+ public QueueOutputStream newQueueOutputStream() {
+ return new QueueOutputStream(queue);
+ }
+
+ /**
+ * Read a single byte.
+ *
+ * @return either the byte read or <code>-1</code> if the end of the stream has been reached
Review comment:
-1 as magic number comes from [InputStream::read](https://docs.oracle.com/javase/8/docs/api/java/io/InputStream.html#read--). I think it'd better to keep it that way to be consistent with super method documentation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r538459087
##########
File path: src/main/java/org/apache/commons/io/output/QueueOutputStream.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.output;
+
+import org.apache.commons.io.input.QueueInputStream;
+
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedOutputStream}; queue input stream provides what's
+ * written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueOutputStream outputStream = new QueueOutputStream();
+ * QueueInputStream inputStream = outputStream.newPipeInputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueOutputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueInputStream
+ * @since 2.9.0
+ */
+public class QueueOutputStream extends OutputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueOutputStream} with no limit to internal buffer size
+ */
+ public QueueOutputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueOutputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueOutputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueInputStream connected to this. Writes to this output stream will be visible to the input stream.
+ *
+ * @return QueueInputStream connected to this stream
+ */
+ public QueueInputStream newQueueInputStream() {
+ return new QueueInputStream(queue);
+ }
+
+ /**
+ * Write a single byte.
+ *
+ * @throws InterruptedIOException if the thread is interrupted while writing to the queue.
+ */
+ @Override
+ public void write(final int b) throws InterruptedIOException {
+ try {
+ queue.put(0xFF & b);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new InterruptedIOException();
Review comment:
Then we shouldn't we still preserve the original exception by calling `initCause()` on the new exception?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #171:
URL: https://github.com/apache/commons-io/pull/171#issuecomment-741819142
I will review all of this tomorrow (Thursday).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] maxxedev commented on a change in pull request #171: Add QueueInput/OutputStream as simpler alternatives to PipedInput/OutputStream
Posted by GitBox <gi...@apache.org>.
maxxedev commented on a change in pull request #171:
URL: https://github.com/apache/commons-io/pull/171#discussion_r537657059
##########
File path: src/main/java/org/apache/commons/io/output/QueueOutputStream.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.io.output;
+
+import org.apache.commons.io.input.QueueInputStream;
+
+import java.io.InterruptedIOException;
+import java.io.OutputStream;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
+import java.util.Objects;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Simple alternative to JDK {@link java.io.PipedOutputStream}; queue input stream provides what's
+ * written in queue output stream.
+ *
+ * Example usage:
+ * <pre>
+ * QueueOutputStream outputStream = new QueueOutputStream();
+ * QueueInputStream inputStream = outputStream.newPipeInputStream();
+ *
+ * outputStream.write("hello world".getBytes(UTF_8));
+ * inputStream.read();
+ * </pre>
+ *
+ * Unlike JDK {@link PipedInputStream} and {@link PipedOutputStream}, queue input/output streams may be
+ * used safely in a single thread or multiple threads. Also, unlike JDK classes, no special meaning is
+ * attached to initial or current thread. Instances can be used longer after initial threads exited.
+ *
+ * Closing a {@code QueueOutputStream} has no effect. The methods in this class can be called after
+ * the stream has been closed without generating an {@code IOException}.
+ *
+ * @see QueueInputStream
+ * @since 2.9.0
+ */
+public class QueueOutputStream extends OutputStream {
+
+ private final BlockingQueue<Integer> queue;
+
+ /**
+ * Create a {@link QueueOutputStream} with no limit to internal buffer size
+ */
+ public QueueOutputStream() {
+ this(new LinkedBlockingQueue<>());
+ }
+
+ /**
+ * Create a {@link QueueOutputStream} with given buffer
+ *
+ * @param queue backing queue for the stream
+ */
+ public QueueOutputStream(final BlockingQueue<Integer> queue) {
+ this.queue = Objects.requireNonNull(queue, "queue is required");
+ }
+
+ /**
+ * Create an QueueInputStream connected to this. Writes to this output stream will be visible to the input stream.
+ *
+ * @return QueueInputStream connected to this stream
+ */
+ public QueueInputStream newQueueInputStream() {
+ return new QueueInputStream(queue);
+ }
+
+ /**
+ * Write a single byte.
+ *
+ * @throws InterruptedIOException if the thread is interrupted while writing to the queue.
+ */
+ @Override
+ public void write(final int b) throws InterruptedIOException {
+ try {
+ queue.put(0xFF & b);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new InterruptedIOException();
Review comment:
InterruptedException does not extend IOException but InterruptedIOException does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org