Skip to content

This replaces a bunch of methods attempting to read file bytes with one method. #727

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.

Sign up for GitHub

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

Draft
leerho wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

This replaces a bunch of methods attempting to read file bytes with one method. #727

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e693cfa
This replaces a bunch of methods attempting to read file bytes with one
leerho Feb 18, 2026
31e5942
Fix unread var.
leerho Feb 18, 2026
d6a9d04
Add putBytesToFile methods
leerho Feb 18, 2026
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
224 changes: 29 additions & 195 deletions src/test/java/org/apache/datasketches/common/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
Expand All @@ -51,216 +53,48 @@ public final class TestUtil {
public static final String CHECK_CPP_HISTORICAL_FILES = "check_cpp_historical_files";

/**
* The full target Path for Java serialized sketches to be tested by other languages.
* The project relative Path for Java serialized sketches to be tested by other languages.
*/
public static final Path javaPath = createPath("serialization_test_data/java_generated_files");
public static final Path javaPath = Path.of(".", "serialization_test_data", "java_generated_files");

/**
* The full target Path for C++ serialized sketches to be tested by Java.
* The project relative Path for C++ serialized sketches to be tested by Java.
*/
public static final Path cppPath = createPath("serialization_test_data/cpp_generated_files");
public static final Path cppPath = Path.of(".", "serialization_test_data", "cpp_generated_files");

/**
* The full target Path for Go serialized sketches to be tested by Java.
* The project relative Path for Go serialized sketches to be tested by Java.
*/
public static final Path goPath = createPath("serialization_test_data/go_generated_files");

private static Path createPath(final String projectLocalDir) {
try {
return Files.createDirectories(Paths.get(userDir, projectLocalDir));
} catch (IOException e) { throw new SketchesArgumentException(e.getCause().toString()); }
}

//Get Resources

private static final int BUF_SIZE = 1 << 13;

/**
* Gets the file defined by the given resource file's shortFileName.
* @param shortFileName the last name in the pathname's name sequence.
* @return the file defined by the given resource file's shortFileName.
*/
public static File getResourceFile(final String shortFileName) {
Objects.requireNonNull(shortFileName, "input parameter 'String shortFileName' cannot be null.");
final String slashName = (shortFileName.charAt(0) == '/') ? shortFileName : '/' + shortFileName;
final URL url = TestUtil.class.getResource(slashName);
Objects.requireNonNull(url, "resource " + slashName + " returns null URL.");
File file;
file = createTempFile(slashName);
if (url.getProtocol().equals("jar")) { //definitely a jar
try (final InputStream input = TestUtil.class.getResourceAsStream(slashName);
final OutputStream out = new FileOutputStream(file)) {
Objects.requireNonNull(input, "InputStream is null.");
int numRead = 0;
final byte[] buf = new byte[1024];
while ((numRead = input.read(buf)) != -1) { out.write(buf, 0, numRead); }
} catch (final IOException e ) { throw new RuntimeException(e); }
} else { //protocol says resource is not a jar, must be a file
file = new File(getResourcePath(url));
}
if (!file.setReadable(false, true)) {
throw new IllegalStateException("Failed to set owner only 'Readable' on file");
}
if (!file.setWritable(false, false)) {
throw new IllegalStateException("Failed to set everyone 'Not Writable' on file");
}
return file;
}

/**
* Returns a byte array of the contents of the file defined by the given resource file's shortFileName.
* @param shortFileName the last name in the pathname's name sequence.
* @return a byte array of the contents of the file defined by the given resource file's shortFileName.
* @throws IllegalArgumentException if resource cannot be read.
*/
public static byte[] getResourceBytes(final String shortFileName) {
Objects.requireNonNull(shortFileName, "input parameter 'String shortFileName' cannot be null.");
final String slashName = (shortFileName.charAt(0) == '/') ? shortFileName : '/' + shortFileName;
final URL url = TestUtil.class.getResource(slashName);
Objects.requireNonNull(url, "resource " + slashName + " returns null URL.");
final byte[] out;
if (url.getProtocol().equals("jar")) { //definitely a jar
try (final InputStream input = TestUtil.class.getResourceAsStream(slashName)) {
out = readAllBytesFromInputStream(input);
} catch (final IOException e) { throw new RuntimeException(e); }
} else { //protocol says resource is not a jar, must be a file
try {
out = Files.readAllBytes(Paths.get(getResourcePath(url)));
} catch (final IOException e) { throw new RuntimeException(e); }
}
return out;
}

public static final Path goPath = Path.of(".", "serialization_test_data", "go_generated_files");

/**
* Note: This is only needed in Java 8 as it is part of Java 9+.
* Read all bytes from the given <i>InputStream</i>.
* This is limited to streams that are no longer than the maximum allocatable byte array determined by the VM.
* This may be a little smaller than <i>Integer.MAX_VALUE</i>.
* @param in the Input Stream
* @return byte array
* The project relative Path for /src/test/resources
*/
public static byte[] readAllBytesFromInputStream(final InputStream in) {
return readBytesFromInputStream(Integer.MAX_VALUE, in);
}
public static final Path resPath = Path.of(".","src","test","resources");

/**
* Note: This is only needed in Java 8 as is part of Java 9+.
* Read <i>numBytesToRead</i> bytes from an input stream into a single byte array.
* This is limited to streams that are no longer than the maximum allocatable byte array determined by the VM.
* This may be a little smaller than <i>Integer.MAX_VALUE</i>.
* @param numBytesToRead number of bytes to read
* @param in the InputStream
* @return the filled byte array from the input stream
* @throws IllegalArgumentException if array size grows larger than what can be safely allocated by some VMs.

*/
public static byte[] readBytesFromInputStream(final int numBytesToRead, final InputStream in) {
if (numBytesToRead < 0) { throw new IllegalArgumentException("numBytesToRead must be positive or zero."); }

List<byte[]> buffers = null;
byte[] result = null;
int totalBytesRead = 0;
int remaining = numBytesToRead;
int chunkCnt;
do {
final byte[] partialBuffer = new byte[Math.min(remaining, BUF_SIZE)];
int numRead = 0;

try {
// reads input stream in chunks of partial buffers, stops at EOF or when remaining is zero.
while ((chunkCnt =
in.read(partialBuffer, numRead, Math.min(partialBuffer.length - numRead, remaining))) > 0) {
numRead += chunkCnt;
remaining -= chunkCnt;
}
} catch (final IOException e) { throw new RuntimeException(e); }

if (numRead > 0) {
if (Integer.MAX_VALUE - Long.BYTES - totalBytesRead < numRead) {
throw new IllegalArgumentException(
"Input stream is larger than what can be safely allocated as a byte[] in some VMs."); }
totalBytesRead += numRead;
if (result == null) {
result = partialBuffer;
} else {
if (buffers == null) {
buffers = new ArrayList<>();
buffers.add(result);
}
buffers.add(partialBuffer);
}
}
} while (chunkCnt >= 0 && remaining > 0);

final byte[] out;
if (buffers == null) {
if (result == null) {
out = new byte[0];
} else {
out = result.length == totalBytesRead ? result : Arrays.copyOf(result, totalBytesRead);
}
return out;
}
//Get Resources

result = new byte[totalBytesRead];
int offset = 0;
remaining = totalBytesRead;
for (byte[] b : buffers) {
final int count = Math.min(b.length, remaining);
System.arraycopy(b, 0, result, offset, count);
offset += count;
remaining -= count;
}
return result;
}
private static final int BUF_SIZE = 1 << 13;

private static String getResourcePath(final URL url) { //must not be null
try {
final URI uri = url.toURI();
//decodes any special characters
final String path = uri.isAbsolute() ? Paths.get(uri).toAbsolutePath().toString() : uri.getPath();
return path;
} catch (final URISyntaxException e) {
throw new IllegalArgumentException("Cannot find resource: " + url.toString() + Util.LS + e);
public static byte[] getFileBytes(Path basePath, String fileName) throws RuntimeException {
Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null.");
Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null.");
Path path = Path.of(basePath.toString(), fileName);
Path absPath = path.toAbsolutePath(); //for debugging
byte[] bytes = new byte[0]; //or null
if (Files.notExists(path)) {
System.err.println("File disappeared or not found: " + absPath);
return bytes; //or null
}
}

/**
* Create an empty temporary file.
* On a Mac these files are stored at the system variable $TMPDIR. They should be cleared on a reboot.
* @param shortFileName the name before prefixes and suffixes are added here and by the OS.
* The final extension will be the current extension. The prefix "temp_" is added here.
* @return a temp file,which will be eventually deleted by the OS
*/
private static File createTempFile(final String shortFileName) {
//remove any leading slash
final String resName = (shortFileName.charAt(0) == '/') ? shortFileName.substring(1) : shortFileName;
final String suffix;
final String name;
final int lastIdx = resName.length() - 1;
final int lastIdxOfDot = resName.lastIndexOf('.');
if (lastIdxOfDot == -1) {
suffix = ".tmp";
name = resName;
} else if (lastIdxOfDot == lastIdx) {
suffix = ".tmp";
name = resName.substring(0, lastIdxOfDot);
} else { //has a real suffix
suffix = resName.substring(lastIdxOfDot);
name = resName.substring(0, lastIdxOfDot);
if (!Files.isRegularFile(path) || !Files.isReadable(path)) {
throw new RuntimeException("Path is not a regular file or not readable: " + absPath);
}
final File file;
try {
file = File.createTempFile("temp_" + name, suffix);
if (!file.setReadable(false, true)) {
throw new IllegalStateException("Failed to set only owner 'Readable' on file");
}
if (!file.setWritable(false, true)) {
throw new IllegalStateException("Failed to set only owner 'Writable' on file");
bytes = Files.readAllBytes(path);
return bytes;
} catch (IOException e) {
throw new RuntimeException("System Error reading file: " + absPath + " " + e);
}

} catch (final IOException e) { throw new RuntimeException(e); }
return file;
}

}
}
66 changes: 66 additions & 0 deletions src/test/java/org/apache/datasketches/common/TestUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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.datasketches.common;

import static org.apache.datasketches.common.TestUtil.getFileBytes;
import static org.apache.datasketches.common.TestUtil.resPath;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
//import static org.testng.internal.EclipseInterface.ASSERT_LEFT; // Ignore, standard imports
import static org.testng.Assert.assertNotNull;

import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import static java.nio.charset.StandardCharsets.UTF_8;

public class TestUtilTest {

@Test
public void testGetFileBytes_Success() throws IOException {
byte[] resultBytes = getFileBytes(resPath, "GettysburgAddress.txt");
assertNotNull(resultBytes);
String resultString = new String(resultBytes, UTF_8);
assertTrue(resultString.startsWith("Abraham Lincoln's Gettysburg Address:"));
}

@Test
public void testGetFileBytes_MissingFile() {
byte[] resultBytes = getFileBytes(resPath, "NonExistentFile");
assertNotNull(resultBytes);
assertEquals(resultBytes.length, 0, "Should return empty array for missing file.");
}

@Test
public void testGetFileBytes_NotRegular_NotReadable() throws IOException {
try {
byte[] resultBytes = getFileBytes(resPath, "");

Check notice

Code scanning / CodeQL

Unread local variable Note test

Variable 'byte[] resultBytes' is never read.

Copilot Autofix

AI 1 day ago

To fix the problem, remove the unused local variable while preserving the side effect of calling getFileBytes. This means we should keep the method invocation but drop the assignment to resultBytes.

The best minimal change is within testGetFileBytes_NotRegular_NotReadable in src/test/java/org/apache/datasketches/common/TestUtilTest.java: replace byte[] resultBytes = getFileBytes(resPath, ""); with just getFileBytes(resPath, "");. This does not alter any test logic (the test still triggers the call that may throw a RuntimeException), but eliminates the unread local variable. No new imports, methods, or other definitions are required.

Suggested changeset 1
src/test/java/org/apache/datasketches/common/TestUtilTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/test/java/org/apache/datasketches/common/TestUtilTest.java b/src/test/java/org/apache/datasketches/common/TestUtilTest.java
--- a/src/test/java/org/apache/datasketches/common/TestUtilTest.java
+++ b/src/test/java/org/apache/datasketches/common/TestUtilTest.java
@@ -57,7 +57,7 @@
   @Test
   public void testGetFileBytes_NotRegular_NotReadable() throws IOException {
     try {
-      byte[] resultBytes = getFileBytes(resPath, "");
+      getFileBytes(resPath, "");
     } catch (RuntimeException e) {
       System.out.println(e.toString());
     }
EOF
@@ -57,7 +57,7 @@
@Test
public void testGetFileBytes_NotRegular_NotReadable() throws IOException {
try {
byte[] resultBytes = getFileBytes(resPath, "");
getFileBytes(resPath, "");
} catch (RuntimeException e) {
System.out.println(e.toString());
}
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Copy link
Member Author

@leerho leerho Feb 18, 2026

Choose a reason for hiding this comment

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

Fixed.

} catch (RuntimeException e) {
System.out.println(e.toString());
}
}

}
5 changes: 3 additions & 2 deletions src/test/java/org/apache/datasketches/cpc/CpcSketchCrossLanguageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.apache.datasketches.common.TestUtil.CHECK_CPP_FILES;
import static org.apache.datasketches.common.TestUtil.CHECK_GO_FILES;
import static org.apache.datasketches.common.TestUtil.GENERATE_JAVA_FILES;
import static org.apache.datasketches.common.TestUtil.getFileBytes;
import static org.apache.datasketches.common.TestUtil.cppPath;
import static org.apache.datasketches.common.TestUtil.goPath;
import static org.apache.datasketches.common.TestUtil.javaPath;
Expand Down Expand Up @@ -75,7 +76,7 @@ public void allFlavors() throws IOException {
final Flavor[] flavorArr = {Flavor.EMPTY, Flavor.SPARSE, Flavor.HYBRID, Flavor.PINNED, Flavor.SLIDING};
int flavorIdx = 0;
for (final int n: nArr) {
final byte[] bytes = Files.readAllBytes(cppPath.resolve("cpc_n" + n + "_cpp.sk"));
final byte[] bytes = getFileBytes(cppPath, "cpc_n" + n + "_cpp.sk");
final CpcSketch sketch = CpcSketch.heapify(MemorySegment.ofArray(bytes));
assertEquals(sketch.getFlavor(), flavorArr[flavorIdx++]);
assertEquals(sketch.getEstimate(), n, n * 0.02);
Expand All @@ -88,7 +89,7 @@ public void checkAllFlavorsGo() throws IOException {
final Flavor[] flavorArr = {Flavor.EMPTY, Flavor.SPARSE, Flavor.HYBRID, Flavor.PINNED, Flavor.SLIDING};
int flavorIdx = 0;
for (final int n: nArr) {
final byte[] bytes = Files.readAllBytes(goPath.resolve("cpc_n" + n + "_go.sk"));
final byte[] bytes = getFileBytes(goPath, "cpc_n" + n + "_go.sk");
final CpcSketch sketch = CpcSketch.heapify(MemorySegment.ofArray(bytes));
assertEquals(sketch.getFlavor(), flavorArr[flavorIdx++]);
assertEquals(sketch.getEstimate(), n, n * 0.02);
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/org/apache/datasketches/filters/bloomfilter/BloomFilterCrossLanguageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import static org.apache.datasketches.common.TestUtil.CHECK_CPP_FILES;
import static org.apache.datasketches.common.TestUtil.GENERATE_JAVA_FILES;
import static org.apache.datasketches.common.TestUtil.getFileBytes;
import static org.apache.datasketches.common.TestUtil.cppPath;
import static org.apache.datasketches.common.TestUtil.javaPath;
import static org.testng.Assert.assertEquals;
Expand Down Expand Up @@ -65,7 +66,7 @@ public void readBloomFilterBinariesForCompatibilityTesting() throws IOException
final short[] hArr = {3, 5};
for (final int n : nArr) {
for (final short numHashes : hArr) {
final byte[] bytes = Files.readAllBytes(cppPath.resolve("bf_n" + n + "_h" + numHashes + "_cpp.sk"));
final byte[] bytes = getFileBytes(cppPath,"bf_n" + n + "_h" + numHashes + "_cpp.sk");
final BloomFilter bf = BloomFilter.heapify(MemorySegment.ofArray(bytes));
assertEquals(bf.isEmpty(), n == 0);
assertTrue(bf.isEmpty() || (bf.getBitsUsed() > (n / 10)));
Expand Down
Loading
Loading