Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Use InputStream.transferTo #2669

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions solr/core/src/java/org/apache/solr/cli/PostTool.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.DeprecatedAttributes;
import org.apache.commons.cli.Option;
import org.apache.commons.io.output.NullOutputStream;
import org.apache.solr.client.api.util.SolrVersion;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrServerException;
Expand Down Expand Up @@ -1054,16 +1055,12 @@ public static InputStream stringToStream(String s) {
* source and thrown away.
*/
private static void pipe(InputStream source, OutputStream dest) throws IOException {
byte[] buf = new byte[1024];
int read = 0;
while ((read = source.read(buf)) >= 0) {
if (null != dest) {
dest.write(buf, 0, read);
}
}
if (null != dest) {
dest.flush();
if (dest == null) {
dest = NullOutputStream.INSTANCE;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A commons-io utility

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

}
// copy source to dest
source.transferTo(dest);
dest.flush();
}

public FileFilter getFileFilterFromFileTypes(String fileTypes) {
Expand Down
16 changes: 14 additions & 2 deletions solr/core/src/java/org/apache/solr/handler/BlobHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.lang.invoke.MethodHandles;
import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import java.security.MessageDigest;
import java.util.ArrayList;
Expand All @@ -34,6 +35,7 @@
import java.util.List;
import java.util.Map;
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.io.input.BoundedInputStream;
import org.apache.lucene.document.Document;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.index.Term;
Expand Down Expand Up @@ -111,8 +113,8 @@ public void handleRequestBody(final SolrQueryRequest req, SolrQueryResponse rsp)

for (ContentStream stream : req.getContentStreams()) {
ByteBuffer payload;
try (InputStream is = stream.getStream()) {
payload = Utils.toByteArray(is, maxSize);
try (InputStream is = boundedInputStream(stream.getStream(), maxSize)) {
payload = Utils.toByteArray(is);
}
MessageDigest m = MessageDigest.getInstance("MD5");
m.update(payload.array(), payload.arrayOffset() + payload.position(), payload.limit());
Expand Down Expand Up @@ -261,6 +263,16 @@ public void write(OutputStream os) throws IOException {
}
}

private static InputStream boundedInputStream(final InputStream is, final long maxLength)
throws IOException {
return new BoundedInputStream(is, maxLength) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A commons-io utility

@Override
protected void onMaxLength(long maxLength, long count) {
throw new BufferOverflowException();
}
};
}

private void verifyWithRealtimeGet(
String blobName, long version, SolrQueryRequest req, Map<String, Object> doc) {
for (; ; ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.apache.solr.handler.loader.CSVLoaderBase.SEPARATOR;

import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
Expand Down Expand Up @@ -65,14 +64,11 @@ public class DefaultSampleDocumentsLoader implements SampleDocumentsLoader {
private static final int MAX_STREAM_SIZE = (5 * 1024 * 1024);
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

public static byte[] streamAsBytes(final InputStream in) throws IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
byte[] buf = new byte[1024];
int r;
/** Reads input completely into a byte array. Closes the stream. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels super trappy to have this method close the stream and Utils.toByteArray leave it open, but not something we can fix here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; it's unusual for a method to accept an InputStream and also close it. I at least documented it. And I reduced its scope from public. I was tempted to rename the method to something like streamAsBytesAndClose so that the closing is super apparent at the caller. Can still do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have the andClose pattern in other places...

static byte[] streamAsBytes(final InputStream in) throws IOException {
try (in) {
while ((r = in.read(buf)) != -1) baos.write(buf, 0, r);
return in.readAllBytes();
}
return baos.toByteArray();
}

@Override
Expand Down
28 changes: 4 additions & 24 deletions solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -1754,14 +1754,7 @@ private static void zipWithForbiddenEndings(File fileOrDirectory, File zipfile)
zout.putNextEntry(new ZipEntry("test." + fileType));

try (InputStream in = new FileInputStream(fileOrDirectory)) {
byte[] buffer = new byte[1024];
while (true) {
int readCount = in.read(buffer);
if (readCount < 0) {
break;
}
zout.write(buffer, 0, readCount);
}
in.transferTo(zout);
}

zout.closeEntry();
Expand All @@ -1783,8 +1776,7 @@ private static void zip(File directory, File zipfile) throws IOException {
Deque<File> queue = new ArrayDeque<>();
queue.push(directory);
OutputStream out = new FileOutputStream(zipfile);
ZipOutputStream zout = new ZipOutputStream(out);
try {
try (ZipOutputStream zout = new ZipOutputStream(out)) {
while (!queue.isEmpty()) {
directory = queue.pop();
for (File kid : directory.listFiles()) {
Expand All @@ -1796,26 +1788,14 @@ private static void zip(File directory, File zipfile) throws IOException {
} else {
zout.putNextEntry(new ZipEntry(name));

InputStream in = new FileInputStream(kid);
try {
byte[] buffer = new byte[1024];
while (true) {
int readCount = in.read(buffer);
if (readCount < 0) {
break;
}
zout.write(buffer, 0, readCount);
}
} finally {
in.close();
try (InputStream in = new FileInputStream(kid)) {
in.transferTo(zout);
}

zout.closeEntry();
}
}
}
} finally {
zout.close();
}
}

Expand Down
13 changes: 2 additions & 11 deletions solr/core/src/test/org/apache/solr/util/TestCborDataFormat.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,22 +139,13 @@ private byte[] runQuery(String testCollection, CloudSolrClient client, String wt
request.setResponseParser(new InputStreamResponseParser(wt));
}
result = client.request(request, testCollection);
byte[] b = copyStream((InputStream) result.get("stream"));
InputStream inputStream = (InputStream) result.get("stream");
byte[] b = inputStream.readAllBytes();
System.out.println(wt + "_time : " + timer.getTime());
System.out.println(wt + "_size : " + b.length);
return b;
}

private static byte[] copyStream(InputStream inputStream) throws IOException {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
byte[] buffer = new byte[4096];
int bytesRead;
while ((bytesRead = inputStream.read(buffer)) != -1) {
outputStream.write(buffer, 0, bytesRead);
}
return outputStream.toByteArray();
}

private void modifySchema(String testCollection, CloudSolrClient client)
throws SolrServerException, IOException {
GenericSolrRequest req =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.io.UnsupportedEncodingException;
import java.util.Arrays;

@Deprecated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we add the since tag to help us remember when to remove this deprecated method? There are so many deprecated methods that just kind of hang around because we don't have good mechanisms for deciding/remembering to remove them!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted it; we don't need to wait for internal utilities

public class BytesOutputStream extends OutputStream {
private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;

Expand Down
23 changes: 2 additions & 21 deletions solr/solrj/src/java/org/apache/solr/common/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -1175,29 +1175,10 @@ public byte[] getbuf() {
}
}

/** Reads an input stream into a byte array. Does not close the input. */
public static ByteBuffer toByteArray(InputStream is) throws IOException {
return toByteArray(is, Integer.MAX_VALUE);
}

/**
* Reads an input stream into a byte array
*
* @param is the input stream
* @return the byte array
* @throws IOException If there is a low-level I/O error.
*/
public static ByteBuffer toByteArray(InputStream is, long maxSize) throws IOException {
try (BAOS bos = new BAOS()) {
long sz = 0;
int next = is.read();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code was very sad, reading one char at a time!

while (next > -1) {
if (++sz > maxSize) {
throw new BufferOverflowException();
}
bos.write(next);
next = is.read();
}
bos.flush();
is.transferTo(bos);
return bos.getByteBuffer();
}
}
Expand Down
Loading