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 all commits
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
253 changes: 63 additions & 190 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 @@ -40,227 +42,98 @@
*/
public final class TestUtil {

private static final String userDir = System.getProperty("user.dir");

/**
* TestNG group constants
*/
public static final String GENERATE_JAVA_FILES = "generate_java_files";
public static final String CHECK_CPP_FILES = "check_cpp_files";
public static final String CHECK_GO_FILES = "check_go_files";
public static final String CHECK_RUST_FILES = "check_rust_files";
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;

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

/**
* 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.
* The project relative Path for Rust serialized sketches to be tested by Java.
*/
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;
}

public static final Path rustPath = Path.of(".", "serialization_test_data", "rust_generated_files");

/**
* 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.
* The project relative Path for /src/test/resources
*/
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 resPath = Path.of(".","src","test","resources");

/**
* 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
*/
public static byte[] readAllBytesFromInputStream(final InputStream in) {
return readBytesFromInputStream(Integer.MAX_VALUE, in);
}

/**
* 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.

* Gets all the bytes of a file as a byte array.
* If the file is missing, this writes a warning message to the console.
* @param basePath the base directory path where the file is located
* @param fileName the simple file name of the file
* @return a byte array
* @throws RuntimeException for IO errors, or if resolved path is not a file or not readable.
*/
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;
public static byte[] getFileBytes(final Path basePath, final 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 error output
byte[] bytes = new byte[0]; //or null
if (Files.notExists(path)) { //In this specific case, just issue warning.
System.err.println("File disappeared or not found: " + absPath);
return bytes; //empty
}

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;
if (!Files.isRegularFile(path) || !Files.isReadable(path)) {
throw new RuntimeException("Path is not a regular file or not readable: " + absPath);
}
return result;
}

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);
bytes = Files.readAllBytes(path);
return bytes;
} catch (IOException e) {
throw new RuntimeException("System IO Error reading file: " + absPath + " " + e);
}
}

/**
* 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
* Puts all the bytes of the given byte array to a file with the given fileName.
* This assumes that the base directory path is {@link #javaPath javaPath}.
* @param fileName the name of the target file
* @param bytes the given byte array
*/
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);
}
final File file;
public static void putBytesToJavaPathFile(final String fileName, final byte[] bytes) {
putBytesToFile(javaPath, fileName, bytes);
}

/**
* Puts all the bytes of the given byte array to a basePath file with the given fileName.
* @param basePath the directory path for the given fileName
* @param fileName the name of the target file
* @param bytes the given byte array
* @throws RuntimeException for IO errors,
*/
public static void putBytesToFile(final Path basePath, final String fileName, final byte[] bytes) {
Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null.");
Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null.");
Objects.requireNonNull(bytes, "input parameter 'byte[] bytes' cannot be null.");
Path filePath = null;
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");
}

} catch (final IOException e) { throw new RuntimeException(e); }
return file;
Files.createDirectories(basePath); //create the directory if it doesn't exist.
filePath = basePath.resolve(fileName);
Files.write(filePath, bytes);
} catch (IOException e) {
throw new RuntimeException("System IO Error writing file: " + filePath.toString() + " " + e);

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning test

Variable
filePath
Loading
may be null at this access because of
this
Loading
assignment.

Copilot Autofix

AI about 14 hours ago

To fix the problem, we must ensure that the catch block does not dereference filePath when it might still be null. The behavior we want to preserve is: throw a RuntimeException that includes as much path information as is safely available, but never trigger a NullPointerException in the error handling itself.

The best minimal change is to avoid preinitializing filePath to null, move its declaration down so that it is definitely assigned before use when possible, and in the catch block handle the case where filePath was never set. Concretely:

  • Replace Path filePath = null; with Path filePath; and move it inside the try block, immediately before the resolve call, so that the variable is only in scope within the try/catch where it’s needed.
  • In the catch block, construct the error message defensively: if filePath is non-null, use filePath.toString(), otherwise fall back to a generic "unknown" or something based on basePath and fileName (both are guaranteed non-null by Objects.requireNonNull).

Because filePath is used only in this method and no external contracts depend on its exact error message format, this change preserves functionality while removing the null-dereference risk. No new imports or helper methods are required.

Suggested changeset 1
src/test/java/org/apache/datasketches/common/TestUtil.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/TestUtil.java b/src/test/java/org/apache/datasketches/common/TestUtil.java
--- a/src/test/java/org/apache/datasketches/common/TestUtil.java
+++ b/src/test/java/org/apache/datasketches/common/TestUtil.java
@@ -127,13 +127,13 @@
     Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null.");
     Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null.");
     Objects.requireNonNull(bytes, "input parameter 'byte[] bytes' cannot be null.");
-    Path filePath = null;
     try {
       Files.createDirectories(basePath); //create the directory if it doesn't exist.
-      filePath = basePath.resolve(fileName);
+      final Path filePath = basePath.resolve(fileName);
       Files.write(filePath, bytes);
     } catch (IOException e) {
-      throw new RuntimeException("System IO Error writing file: " + filePath.toString() + " " + e);
+      final String baseInfo = "basePath=" + basePath + ", fileName=" + fileName;
+      throw new RuntimeException("System IO Error writing file (" + baseInfo + "): " + e);
     }
   }
 }
EOF
@@ -127,13 +127,13 @@
Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null.");
Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null.");
Objects.requireNonNull(bytes, "input parameter 'byte[] bytes' cannot be null.");
Path filePath = null;
try {
Files.createDirectories(basePath); //create the directory if it doesn't exist.
filePath = basePath.resolve(fileName);
final Path filePath = basePath.resolve(fileName);
Files.write(filePath, bytes);
} catch (IOException e) {
throw new RuntimeException("System IO Error writing file: " + filePath.toString() + " " + e);
final String baseInfo = "basePath=" + basePath + ", fileName=" + fileName;
throw new RuntimeException("System IO Error writing file (" + baseInfo + "): " + e);
}
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}

}
69 changes: 69 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,69 @@
/*
* 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 {
getFileBytes(resPath, "");
} 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
Loading
Loading