Merge branch 'master' into fixed-array-paging

.github/FUNDING.yml

@@ -7,5 +7,5 @@ github: [jamesmudd]
 liberapay: jamesmudd
 #issuehunt: # Replace with a single IssueHunt username
 #otechie: # Replace with a single Otechie username
-custom: ['https://www.paypal.me/jamesmudd/10']
 buy_me_a_coffee: jamesmudd
+custom: ['https://www.paypal.me/jamesmudd/10', 'https://strike.me/jamesmudd/']

CHANGES.md

@@ -1,5 +1,15 @@
 # jHDF Change Log
 
+## v0.8.4 - October 2024
+- Fix incorrectly written string attributes. https://github.com/jamesmudd/jhdf/issues/641
+- Dependency updates
+
+## v0.8.3 - October 2024
+- Add support for accessing decompressed chunks individually. Thanks to [@marcobitplane](https://github.com/marcobitplane) https://github.com/jamesmudd/jhdf/pull/626
+- Fix OSGi headers, and autogenerate them during the build. Thanks to [@mailaender](https://github.com/Mailaender) https://github.com/jamesmudd/jhdf/pull/625 https://github.com/jamesmudd/jhdf/pull/632
+- Delete temporary file when closing a file read from an input stream. Thanks to [@ivanwick](https://github.com/ivanwick) https://github.com/jamesmudd/jhdf/issues/262 https://github.com/jamesmudd/jhdf/pull/636
+- Build and dependency updates
+
 ## v0.8.2 - August 2024
 - Add support for writing `boolean` datasets and attributes as Bitfield
 

jhdf/build.gradle

@@ -21,13 +21,13 @@ plugins {
 
     // Community plugins (need versions)
     id 'org.sonarqube' version '4.0.0.2929' // Code quality
-    id "com.github.spotbugs" version "6.0.23" // Static analysis
+    id "com.github.spotbugs" version "6.0.26" // Static analysis
     id "me.champeau.jmh" version "0.7.2" // JMH support
 }
 
 // Variables
 group = 'io.jhdf'
-version = '0.8.2'
+version = '0.8.4'
 
 compileJava {
     sourceCompatibility = "1.8"
@@ -55,7 +55,7 @@ dependencies {
     implementation group: 'org.lz4', name: 'lz4-java', version: '1.8.0'
 
     // Use JUnit 5 test framework
-    testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter', version: '5.11.1'
+    testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter', version: '5.11.3'
     testRuntimeOnly group: 'org.slf4j', name: 'slf4j-simple', version: '1.7.36'
 
     // Mocking
@@ -67,8 +67,8 @@ dependencies {
     // Alternative bitshuffle impl to check results against
     testImplementation 'org.xerial.snappy:snappy-java:1.1.10.7'
     // For parsing h5dump XML output
-    testImplementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.18.0'
-    testImplementation 'commons-io:commons-io:2.17.0'
+    testImplementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-xml:2.18.1'
+    testImplementation 'commons-io:commons-io:2.18.0'
 }
 
 test {
@@ -105,7 +105,7 @@ jar {
             'Bundle-Vendor': 'James Mudd',
             'Bundle-Version': project.version,
             'Export-Package': 'io.jhdf,io.jhdf.*,io.jhdf.api,io.jhdf.api.*',
-            'Require-Bundle': 'slf4j.api;bundle-version="1.7.36",org.apache.commons.lang3;bundle-version="3.17.0",com.ning.compress-lzf;bundle-version="1.1.2",lz4-java;bundle-version="1.8.0"',
+            'Require-Bundle': buildRequireBundleHeader(),
             // Build data
             'Build-JDK': System.getProperty('java.vendor') + ' ' + System.getProperty('java.version'),
             'Build-OS': System.getProperty('os.name') + ' ' + System.getProperty('os.version'),
@@ -190,6 +190,7 @@ publishing {
 }
 
 signing {
+    required { System.getenv("SIGNING_KEY") != null }
     def signingKey = System.getenv("SIGNING_KEY")
     def signingPassword = System.getenv("SIGNING_PASSWORD")
     useInMemoryPgpKeys(signingKey, signingPassword)
@@ -198,6 +199,9 @@ signing {
 
 import com.github.spotbugs.snom.Confidence
 import com.github.spotbugs.snom.Effort
+
+import java.util.stream.Collectors
+
 spotbugs {
     ignoreFailures = true // Allow build to continue with errors
     effort = Effort.valueOf('MAX')
@@ -222,3 +226,28 @@ sonarqube {
         property "sonar.java.checkstyle.reportPaths", "build/reports/checkstyle/main.xml,build/reports/checkstyle/test.xml"
     }
 }
+
+String buildRequireBundleHeader() {
+    def runtimeDependencies = configurations.runtimeClasspath.getAllDependencies()
+    def bundleToModule = [
+        "slf4j.api": "org.slf4j:slf4j-api",
+        "org.apache.commons.lang3": "org.apache.commons:commons-lang3",
+        "com.ning.compress-lzf": "com.ning:compress-lzf",
+        "lz4-java": "org.lz4:lz4-java"
+    ]
+
+    def moduleToVersion = runtimeDependencies.collectEntries {
+        [it.module.toString(), it.version]
+    }
+
+    //  Check here to catch when dependencies are added/removed
+    if(!bundleToModule.values().containsAll(moduleToVersion.keySet())) {
+        throw new IllegalStateException("Runtime dependencies not mapped for OSGi")
+    }
+
+    def header = bundleToModule.entrySet().stream()
+        .map {"${it.key};bundle-version=\"${moduleToVersion.get(it.value)}\"" }
+        .collect(Collectors.joining(","))
+
+    return header
+}

jhdf/src/main/java/io/jhdf/HdfFile.java

@@ -36,10 +36,8 @@
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.file.FileSystems;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -156,24 +154,15 @@ private HdfFile(HdfBackingStorage hdfBackingStorage) {
 
 	/**
 	 * Opens an {@link HdfFile} from an {@link InputStream}. The stream will be read fully into a temporary file. The
-	 * file will be cleaned up at application exit.
+	 * file will be deleted when the HdfFile is closed, or at application exit if it was never closed.
 	 *
 	 * @param inputStream the {@link InputStream} to read
 	 * @return HdfFile instance
 	 * @see HdfFile#fromBytes(byte[])
 	 * @see HdfFile#fromByteBuffer(ByteBuffer)
 	 */
 	public static HdfFile fromInputStream(InputStream inputStream) {
-		try {
-			Path tempFile = Files.createTempFile(null, "-stream.hdf5"); // null random file name
-			logger.info("Creating temp file [{}]", tempFile.toAbsolutePath());
-			tempFile.toFile().deleteOnExit(); // Auto cleanup
-			Files.copy(inputStream, tempFile, StandardCopyOption.REPLACE_EXISTING);
-			logger.debug("Read stream to temp file [{}]", tempFile.toAbsolutePath());
-			return new HdfFile(tempFile);
-		} catch (IOException e) {
-			throw new HdfException("Failed to open input stream", e);
-		}
+		return TempHdfFile.fromInputStream(inputStream);
 	}
 
 	public HdfFile(Path hdfFile) {

jhdf/src/main/java/io/jhdf/TempHdfFile.java

@@ -0,0 +1,51 @@
+/*
+ * This file is part of jHDF. A pure Java library for accessing HDF5 files.
+ *
+ * https://jhdf.io
+ *
+ * Copyright (c) 2024 James Mudd
+ *
+ * MIT License see 'LICENSE' file
+ */
+package io.jhdf;
+
+import io.jhdf.exceptions.HdfException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+
+class TempHdfFile extends HdfFile {
+	private static final Logger logger = LoggerFactory.getLogger(TempHdfFile.class);
+
+	private TempHdfFile(Path tempFile) {
+		super(tempFile);
+	}
+
+	@Override
+	public void close() {
+		super.close();
+		logger.info("Deleting temp file on close [{}]", getFileAsPath().toAbsolutePath());
+		boolean deleteSuccess = getFile().delete();
+		if (!deleteSuccess) {
+			logger.warn("Could not delete temp file [{}]", getFileAsPath().toAbsolutePath());
+		}
+	}
+
+	public static TempHdfFile fromInputStream(InputStream inputStream) {
+		try {
+			Path tempFile = Files.createTempFile(null, "-stream.hdf5"); // null random file name
+			logger.info("Creating temp file [{}]", tempFile.toAbsolutePath());
+			tempFile.toFile().deleteOnExit(); // Auto cleanup in case close() is never called
+			Files.copy(inputStream, tempFile, StandardCopyOption.REPLACE_EXISTING);
+			logger.debug("Read stream to temp file [{}]", tempFile.toAbsolutePath());
+			return new TempHdfFile(tempFile);
+		} catch (IOException e) {
+			throw new HdfException("Failed to open input stream", e);
+		}
+	}
+}

jhdf/src/main/java/io/jhdf/object/datatype/StringData.java

@@ -252,9 +252,14 @@ private void encodeDataInternal(Object data, int[] dims, ByteBuffer buffer) {
 				encodeDataInternal(newArray, stripLeadingIndex(dims), buffer);
 			}
 		} else {
-			for (String str : (String[]) data) {
-				buffer.put(this.charset.encode(str)).put(NULL);
-			}
+			final int offset = buffer.position();
+            String[] strings = (String[]) data;
+            for (int i = 0; i < strings.length; i++) {
+                String str = strings[i];
+                buffer.put(this.charset.encode(str))
+                        .put(NULL)
+                        .position(offset + (i + 1) * getSize());
+            }
 		}
 	}
 

jhdf/src/test/java/io/jhdf/HdfFileTest.java

@@ -48,7 +48,9 @@
 import static org.hamcrest.Matchers.sameInstance;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class HdfFileTest {
 
@@ -316,6 +318,17 @@ void testReadingFromStreamThrowsWhenStreamCantBeRead() throws IOException {
 		}
 	}
 
+	@Test
+	void testReadingFromStreamDeletesTempFileOnClose() throws IOException {
+		File tempFile;
+		try (InputStream inputStream = this.getClass().getResource(HDF5_TEST_FILE_PATH).openStream();
+			 HdfFile hdfFile = HdfFile.fromInputStream(inputStream)) {
+			tempFile = hdfFile.getFile();
+			assertTrue(tempFile.exists());
+		}
+		assertFalse(tempFile.exists());
+	}
+
 	@Test
 	void testLoadingInMemoryFile() throws IOException, URISyntaxException {
 		Path path = Paths.get(this.getClass().getResource(HDF5_TEST_FILE_PATH).toURI());

jhdf/src/test/java/io/jhdf/writing/StringWritingTest.java

@@ -13,12 +13,14 @@
 import io.jhdf.HdfFile;
 import io.jhdf.TestUtils;
 import io.jhdf.WritableHdfFile;
+import io.jhdf.api.Dataset;
 import io.jhdf.api.Node;
 import io.jhdf.api.WritiableDataset;
 import io.jhdf.examples.TestAllFilesBase;
 import io.jhdf.h5dump.EnabledIfH5DumpAvailable;
 import io.jhdf.h5dump.H5Dump;
 import io.jhdf.h5dump.HDF5FileXml;
+import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Order;
@@ -96,7 +98,8 @@ void writeStrings() throws Exception {
 			{"element 2,1", "element 2,2", "element 2,3", "element 2,4", "element 2,5"}
 		});
 
-		writableHdfFile.putDataset("prose", StringUtils.split(prose));
+		WritiableDataset proseDataset = writableHdfFile.putDataset("prose", StringUtils.split(prose));
+		proseDataset.putAttribute("prose_attr", StringUtils.split(prose));
 
 		// Actually flush and write everything
 		writableHdfFile.close();
@@ -131,4 +134,62 @@ void readStringDatasetsWithH5Dump() throws Exception {
 			H5Dump.assetXmlAndHdfFileMatch(hdf5FileXml, hdfFile);
 		}
 	}
+
+	@Test
+	@Order(3)
+	// https://github.com/jamesmudd/jhdf/issues/641
+	void writeVarStringAttributes() throws Exception {
+		Path tempFile = Files.createTempFile(this.getClass().getSimpleName(), ".hdf5");
+		WritableHdfFile writableHdfFile = HdfFile.write(tempFile);
+
+		// Write a dataset with string attributes
+		WritiableDataset writiableDataset = writableHdfFile.putDataset("dataset", new String[] {"vv", "xx", "abcdef"});
+		writiableDataset.putAttribute("labels", new String[] {"vv", "xx", "abcdef"});
+		writiableDataset.putAttribute("units", new String[] {"", "1", "mm2"});
+		writableHdfFile.close();
+
+		// Now read it back
+		try (HdfFile hdfFile = new HdfFile(tempFile)) {
+			Dataset dataset = hdfFile.getDatasetByPath("dataset");
+			assertThat(dataset.getData()).isEqualTo(new String[] {"vv", "xx", "abcdef"});
+
+			// Expected :["vv", "xx", "abcdef"]
+			// Actual   :["vv", "cdedf", ""]
+			assertThat(dataset.getAttribute("labels").getData()).isEqualTo(new String[] {"vv", "xx", "abcdef"});
+
+			// Expected :["", "1", "mm2"]
+			// Actual   :["", "m2", ""]
+			assertThat(dataset.getAttribute("units").getData()).isEqualTo(new String[] {"", "1", "mm2"});
+		} finally {
+			tempFile.toFile().delete();
+		}
+	}
+
+	@Test()
+	@Order(4)
+	void writeReallyLongStrings() throws Exception {
+		Path tempFile = Files.createTempFile(this.getClass().getSimpleName(), ".hdf5");
+		WritableHdfFile writableHdfFile = HdfFile.write(tempFile);
+
+		// Write a dataset with string attributes
+		String[] randomLongStringData = {
+			RandomStringUtils.insecure().nextAlphanumeric(234, 456),
+			RandomStringUtils.insecure().nextAlphanumeric(234, 456),
+			RandomStringUtils.insecure().nextAlphanumeric(234, 456),
+			RandomStringUtils.insecure().nextAlphanumeric(234, 456),
+			RandomStringUtils.insecure().nextAlphanumeric(234, 456),
+		};
+		WritiableDataset writiableDataset = writableHdfFile.putDataset("dataset", randomLongStringData);
+		writiableDataset.putAttribute("attr", randomLongStringData);
+		writableHdfFile.close();
+
+		// Now read it back
+		try (HdfFile hdfFile = new HdfFile(tempFile)) {
+			Dataset dataset = hdfFile.getDatasetByPath("dataset");
+			assertThat(dataset.getData()).isEqualTo(randomLongStringData);
+			assertThat(dataset.getAttribute("attr").getData()).isEqualTo(randomLongStringData);
+		} finally {
+			tempFile.toFile().delete();
+		}
+	}
 }