Skip to content

Commit e693cfa

Browse files
leerholeerho
committed
This replaces a bunch of methods attempting to read file bytes with one
method. They were not robust, especially in a Windows OS environment.
1 parent 29837cc commit e693cfa

16 files changed

+161
-238
lines changed

‎src/test/java/org/apache/datasketches/common/TestUtil.java‎

Lines changed: 29 additions & 195 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import java.net.URISyntaxException;
2929
import java.net.URL;
3030
import java.nio.file.Files;
31+
import java.nio.file.LinkOption;
32+
import java.nio.file.NoSuchFileException;
3133
import java.nio.file.Path;
3234
import java.nio.file.Paths;
3335
import java.util.ArrayList;
@@ -51,216 +53,48 @@ public final class TestUtil {
5153
public static final String CHECK_CPP_HISTORICAL_FILES = "check_cpp_historical_files";
5254

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

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

6365
/**
64-
* The full target Path for Go serialized sketches to be tested by Java.
66+
* The project relative Path for Go serialized sketches to be tested by Java.
6567
*/
66-
public static final Path goPath = createPath("serialization_test_data/go_generated_files");
67-
68-
private static Path createPath(final String projectLocalDir) {
69-
try {
70-
return Files.createDirectories(Paths.get(userDir, projectLocalDir));
71-
} catch (IOException e) { throw new SketchesArgumentException(e.getCause().toString()); }
72-
}
73-
74-
//Get Resources
75-
76-
private static final int BUF_SIZE = 1 << 13;
77-
78-
/**
79-
* Gets the file defined by the given resource file's shortFileName.
80-
* @param shortFileName the last name in the pathname's name sequence.
81-
* @return the file defined by the given resource file's shortFileName.
82-
*/
83-
public static File getResourceFile(final String shortFileName) {
84-
Objects.requireNonNull(shortFileName, "input parameter 'String shortFileName' cannot be null.");
85-
final String slashName = (shortFileName.charAt(0) == '/') ? shortFileName : '/' + shortFileName;
86-
final URL url = TestUtil.class.getResource(slashName);
87-
Objects.requireNonNull(url, "resource " + slashName + " returns null URL.");
88-
File file;
89-
file = createTempFile(slashName);
90-
if (url.getProtocol().equals("jar")) { //definitely a jar
91-
try (final InputStream input = TestUtil.class.getResourceAsStream(slashName);
92-
final OutputStream out = new FileOutputStream(file)) {
93-
Objects.requireNonNull(input, "InputStream is null.");
94-
int numRead = 0;
95-
final byte[] buf = new byte[1024];
96-
while ((numRead = input.read(buf)) != -1) { out.write(buf, 0, numRead); }
97-
} catch (final IOException e ) { throw new RuntimeException(e); }
98-
} else { //protocol says resource is not a jar, must be a file
99-
file = new File(getResourcePath(url));
100-
}
101-
if (!file.setReadable(false, true)) {
102-
throw new IllegalStateException("Failed to set owner only 'Readable' on file");
103-
}
104-
if (!file.setWritable(false, false)) {
105-
throw new IllegalStateException("Failed to set everyone 'Not Writable' on file");
106-
}
107-
return file;
108-
}
109-
110-
/**
111-
* Returns a byte array of the contents of the file defined by the given resource file's shortFileName.
112-
* @param shortFileName the last name in the pathname's name sequence.
113-
* @return a byte array of the contents of the file defined by the given resource file's shortFileName.
114-
* @throws IllegalArgumentException if resource cannot be read.
115-
*/
116-
public static byte[] getResourceBytes(final String shortFileName) {
117-
Objects.requireNonNull(shortFileName, "input parameter 'String shortFileName' cannot be null.");
118-
final String slashName = (shortFileName.charAt(0) == '/') ? shortFileName : '/' + shortFileName;
119-
final URL url = TestUtil.class.getResource(slashName);
120-
Objects.requireNonNull(url, "resource " + slashName + " returns null URL.");
121-
final byte[] out;
122-
if (url.getProtocol().equals("jar")) { //definitely a jar
123-
try (final InputStream input = TestUtil.class.getResourceAsStream(slashName)) {
124-
out = readAllBytesFromInputStream(input);
125-
} catch (final IOException e) { throw new RuntimeException(e); }
126-
} else { //protocol says resource is not a jar, must be a file
127-
try {
128-
out = Files.readAllBytes(Paths.get(getResourcePath(url)));
129-
} catch (final IOException e) { throw new RuntimeException(e); }
130-
}
131-
return out;
132-
}
133-
68+
public static final Path goPath = Path.of(".", "serialization_test_data", "go_generated_files");
69+
13470
/**
135-
* Note: This is only needed in Java 8 as it is part of Java 9+.
136-
* Read all bytes from the given <i>InputStream</i>.
137-
* This is limited to streams that are no longer than the maximum allocatable byte array determined by the VM.
138-
* This may be a little smaller than <i>Integer.MAX_VALUE</i>.
139-
* @param in the Input Stream
140-
* @return byte array
71+
* The project relative Path for /src/test/resources
14172
*/
142-
public static byte[] readAllBytesFromInputStream(final InputStream in) {
143-
return readBytesFromInputStream(Integer.MAX_VALUE, in);
144-
}
73+
public static final Path resPath = Path.of(".","src","test","resources");
14574

146-
/**
147-
* Note: This is only needed in Java 8 as is part of Java 9+.
148-
* Read <i>numBytesToRead</i> bytes from an input stream into a single byte array.
149-
* This is limited to streams that are no longer than the maximum allocatable byte array determined by the VM.
150-
* This may be a little smaller than <i>Integer.MAX_VALUE</i>.
151-
* @param numBytesToRead number of bytes to read
152-
* @param in the InputStream
153-
* @return the filled byte array from the input stream
154-
* @throws IllegalArgumentException if array size grows larger than what can be safely allocated by some VMs.
155-
156-
*/
157-
public static byte[] readBytesFromInputStream(final int numBytesToRead, final InputStream in) {
158-
if (numBytesToRead < 0) { throw new IllegalArgumentException("numBytesToRead must be positive or zero."); }
159-
160-
List<byte[]> buffers = null;
161-
byte[] result = null;
162-
int totalBytesRead = 0;
163-
int remaining = numBytesToRead;
164-
int chunkCnt;
165-
do {
166-
final byte[] partialBuffer = new byte[Math.min(remaining, BUF_SIZE)];
167-
int numRead = 0;
168-
169-
try {
170-
// reads input stream in chunks of partial buffers, stops at EOF or when remaining is zero.
171-
while ((chunkCnt =
172-
in.read(partialBuffer, numRead, Math.min(partialBuffer.length - numRead, remaining))) > 0) {
173-
numRead += chunkCnt;
174-
remaining -= chunkCnt;
175-
}
176-
} catch (final IOException e) { throw new RuntimeException(e); }
17775

178-
if (numRead > 0) {
179-
if (Integer.MAX_VALUE - Long.BYTES - totalBytesRead < numRead) {
180-
throw new IllegalArgumentException(
181-
"Input stream is larger than what can be safely allocated as a byte[] in some VMs."); }
182-
totalBytesRead += numRead;
183-
if (result == null) {
184-
result = partialBuffer;
185-
} else {
186-
if (buffers == null) {
187-
buffers = new ArrayList<>();
188-
buffers.add(result);
189-
}
190-
buffers.add(partialBuffer);
191-
}
192-
}
193-
} while (chunkCnt >= 0 && remaining > 0);
194-
195-
final byte[] out;
196-
if (buffers == null) {
197-
if (result == null) {
198-
out = new byte[0];
199-
} else {
200-
out = result.length == totalBytesRead ? result : Arrays.copyOf(result, totalBytesRead);
201-
}
202-
return out;
203-
}
76+
//Get Resources
20477

205-
result = new byte[totalBytesRead];
206-
int offset = 0;
207-
remaining = totalBytesRead;
208-
for (byte[] b : buffers) {
209-
final int count = Math.min(b.length, remaining);
210-
System.arraycopy(b, 0, result, offset, count);
211-
offset += count;
212-
remaining -= count;
213-
}
214-
return result;
215-
}
78+
private static final int BUF_SIZE = 1 << 13;
21679

217-
private static String getResourcePath(final URL url) { //must not be null
218-
try {
219-
final URI uri = url.toURI();
220-
//decodes any special characters
221-
final String path = uri.isAbsolute() ? Paths.get(uri).toAbsolutePath().toString() : uri.getPath();
222-
return path;
223-
} catch (final URISyntaxException e) {
224-
throw new IllegalArgumentException("Cannot find resource: " + url.toString() + Util.LS + e);
80+
public static byte[] getFileBytes(Path basePath, String fileName) throws RuntimeException {
81+
Objects.requireNonNull(basePath, "input parameter 'Path basePath' cannot be null.");
82+
Objects.requireNonNull(fileName, "input parameter 'String fileName' cannot be null.");
83+
Path path = Path.of(basePath.toString(), fileName);
84+
Path absPath = path.toAbsolutePath(); //for debugging
85+
byte[] bytes = new byte[0]; //or null
86+
if (Files.notExists(path)) {
87+
System.err.println("File disappeared or not found: " + absPath);
88+
return bytes; //or null
22589
}
226-
}
227-
228-
/**
229-
* Create an empty temporary file.
230-
* On a Mac these files are stored at the system variable $TMPDIR. They should be cleared on a reboot.
231-
* @param shortFileName the name before prefixes and suffixes are added here and by the OS.
232-
* The final extension will be the current extension. The prefix "temp_" is added here.
233-
* @return a temp file,which will be eventually deleted by the OS
234-
*/
235-
private static File createTempFile(final String shortFileName) {
236-
//remove any leading slash
237-
final String resName = (shortFileName.charAt(0) == '/') ? shortFileName.substring(1) : shortFileName;
238-
final String suffix;
239-
final String name;
240-
final int lastIdx = resName.length() - 1;
241-
final int lastIdxOfDot = resName.lastIndexOf('.');
242-
if (lastIdxOfDot == -1) {
243-
suffix = ".tmp";
244-
name = resName;
245-
} else if (lastIdxOfDot == lastIdx) {
246-
suffix = ".tmp";
247-
name = resName.substring(0, lastIdxOfDot);
248-
} else { //has a real suffix
249-
suffix = resName.substring(lastIdxOfDot);
250-
name = resName.substring(0, lastIdxOfDot);
90+
if (!Files.isRegularFile(path) || !Files.isReadable(path)) {
91+
throw new RuntimeException("Path is not a regular file or not readable: " + absPath);
25192
}
252-
final File file;
25393
try {
254-
file = File.createTempFile("temp_" + name, suffix);
255-
if (!file.setReadable(false, true)) {
256-
throw new IllegalStateException("Failed to set only owner 'Readable' on file");
257-
}
258-
if (!file.setWritable(false, true)) {
259-
throw new IllegalStateException("Failed to set only owner 'Writable' on file");
94+
bytes = Files.readAllBytes(path);
95+
return bytes;
96+
} catch (IOException e) {
97+
throw new RuntimeException("System Error reading file: " + absPath + " " + e);
26098
}
261-
262-
} catch (final IOException e) { throw new RuntimeException(e); }
263-
return file;
264-
}
265-
99+
}
266100
}

‎src/test/java/org/apache/datasketches/common/TestUtilTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.datasketches.common;
21+
22+
import static org.apache.datasketches.common.TestUtil.getFileBytes;
23+
import static org.apache.datasketches.common.TestUtil.resPath;
24+
25+
import static org.testng.Assert.assertEquals;
26+
import static org.testng.Assert.assertTrue;
27+
//import static org.testng.internal.EclipseInterface.ASSERT_LEFT; // Ignore, standard imports
28+
import static org.testng.Assert.assertNotNull;
29+
30+
import org.testng.annotations.AfterMethod;
31+
import org.testng.annotations.BeforeMethod;
32+
import org.testng.annotations.Test;
33+
34+
import java.io.IOException;
35+
import java.nio.file.Files;
36+
import java.nio.file.Path;
37+
import java.util.Arrays;
38+
import static java.nio.charset.StandardCharsets.UTF_8;
39+
40+
public class TestUtilTest {
41+
42+
@Test
43+
public void testGetFileBytes_Success() throws IOException {
44+
byte[] resultBytes = getFileBytes(resPath, "GettysburgAddress.txt");
45+
assertNotNull(resultBytes);
46+
String resultString = new String(resultBytes, UTF_8);
47+
assertTrue(resultString.startsWith("Abraham Lincoln's Gettysburg Address:"));
48+
}
49+
50+
@Test
51+
public void testGetFileBytes_MissingFile() {
52+
byte[] resultBytes = getFileBytes(resPath, "NonExistentFile");
53+
assertNotNull(resultBytes);
54+
assertEquals(resultBytes.length, 0, "Should return empty array for missing file.");
55+
}
56+
57+
@Test
58+
public void testGetFileBytes_NotRegular_NotReadable() throws IOException {
59+
try {
60+
byte[] resultBytes = getFileBytes(resPath, "");
61+
} catch (RuntimeException e) {
62+
System.out.println(e.toString());
63+
}
64+
}
65+
66+
}

‎src/test/java/org/apache/datasketches/cpc/CpcSketchCrossLanguageTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.apache.datasketches.common.TestUtil.CHECK_CPP_FILES;
2323
import static org.apache.datasketches.common.TestUtil.CHECK_GO_FILES;
2424
import static org.apache.datasketches.common.TestUtil.GENERATE_JAVA_FILES;
25+
import static org.apache.datasketches.common.TestUtil.getFileBytes;
2526
import static org.apache.datasketches.common.TestUtil.cppPath;
2627
import static org.apache.datasketches.common.TestUtil.goPath;
2728
import static org.apache.datasketches.common.TestUtil.javaPath;
@@ -75,7 +76,7 @@ public void allFlavors() throws IOException {
7576
final Flavor[] flavorArr = {Flavor.EMPTY, Flavor.SPARSE, Flavor.HYBRID, Flavor.PINNED, Flavor.SLIDING};
7677
int flavorIdx = 0;
7778
for (final int n: nArr) {
78-
final byte[] bytes = Files.readAllBytes(cppPath.resolve("cpc_n" + n + "_cpp.sk"));
79+
final byte[] bytes = getFileBytes(cppPath, "cpc_n" + n + "_cpp.sk");
7980
final CpcSketch sketch = CpcSketch.heapify(MemorySegment.ofArray(bytes));
8081
assertEquals(sketch.getFlavor(), flavorArr[flavorIdx++]);
8182
assertEquals(sketch.getEstimate(), n, n * 0.02);
@@ -88,7 +89,7 @@ public void checkAllFlavorsGo() throws IOException {
8889
final Flavor[] flavorArr = {Flavor.EMPTY, Flavor.SPARSE, Flavor.HYBRID, Flavor.PINNED, Flavor.SLIDING};
8990
int flavorIdx = 0;
9091
for (final int n: nArr) {
91-
final byte[] bytes = Files.readAllBytes(goPath.resolve("cpc_n" + n + "_go.sk"));
92+
final byte[] bytes = getFileBytes(goPath, "cpc_n" + n + "_go.sk");
9293
final CpcSketch sketch = CpcSketch.heapify(MemorySegment.ofArray(bytes));
9394
assertEquals(sketch.getFlavor(), flavorArr[flavorIdx++]);
9495
assertEquals(sketch.getEstimate(), n, n * 0.02);

‎src/test/java/org/apache/datasketches/filters/bloomfilter/BloomFilterCrossLanguageTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import static org.apache.datasketches.common.TestUtil.CHECK_CPP_FILES;
2323
import static org.apache.datasketches.common.TestUtil.GENERATE_JAVA_FILES;
24+
import static org.apache.datasketches.common.TestUtil.getFileBytes;
2425
import static org.apache.datasketches.common.TestUtil.cppPath;
2526
import static org.apache.datasketches.common.TestUtil.javaPath;
2627
import static org.testng.Assert.assertEquals;
@@ -65,7 +66,7 @@ public void readBloomFilterBinariesForCompatibilityTesting() throws IOException
6566
final short[] hArr = {3, 5};
6667
for (final int n : nArr) {
6768
for (final short numHashes : hArr) {
68-
final byte[] bytes = Files.readAllBytes(cppPath.resolve("bf_n" + n + "_h" + numHashes + "_cpp.sk"));
69+
final byte[] bytes = getFileBytes(cppPath,"bf_n" + n + "_h" + numHashes + "_cpp.sk");
6970
final BloomFilter bf = BloomFilter.heapify(MemorySegment.ofArray(bytes));
7071
assertEquals(bf.isEmpty(), n == 0);
7172
assertTrue(bf.isEmpty() || (bf.getBitsUsed() > (n / 10)));

0 commit comments

Comments
 (0)