diff --git a/src/main/java/htsjdk/beta/io/bundle/Bundle.java b/src/main/java/htsjdk/beta/io/bundle/Bundle.java
index 9efe52e45b..ad10339b42 100644
--- a/src/main/java/htsjdk/beta/io/bundle/Bundle.java
+++ b/src/main/java/htsjdk/beta/io/bundle/Bundle.java
@@ -4,11 +4,7 @@
import htsjdk.utils.ValidationUtils;
import java.io.Serializable;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
/**
* An immutable collection of related resources, including a (single, required) primary resource, such as "reads",
@@ -34,13 +30,15 @@
*
* Bundles that contain only serializable ({@link IOPathResource}) resources may be serialized to, and
* deserialized from JSON.
+ *
+ * Note that the order of resources in a bundle is not significant, and is not guaranteed to be preserved.
*/
public class Bundle implements Iterable, Serializable {
private static final long serialVersionUID = 1L;
- // don't use LinkedHashMap here; using HashMap resolves unnatural resource ordering issues that arise
- // when creating a bundle from serialized files or strings
- private final Map resources = new HashMap<>(); // content type -> resource
+ // content type -> resource. Note that the order of resources is not preserved when round tripping
+ // through JSON.
+ private final Map resources = new LinkedHashMap<>(); // content type -> resource
private final String primaryContentType;
/**
@@ -55,7 +53,9 @@ public Bundle(final String primaryContentType, final Collection
ValidationUtils.validateArg(primaryContentType.length() > 0,
"A non-zero length primary resource content type must be provided");
ValidationUtils.nonNull(resources, "resource collection");
- ValidationUtils.nonEmpty(resources,"resource collection");
+ if (resources.isEmpty()) {
+ throw new IllegalArgumentException("A bundle must contain at least one resource");
+ }
resources.forEach(r -> {
if (null != this.resources.putIfAbsent(r.getContentType(), r)) {
@@ -155,4 +155,18 @@ public int hashCode() {
public String toString() {
return String.format("%s/%d resource(s)", primaryContentType, resources.size());
}
+
+ // compare two bundles for equality without regard for resource order
+ public static boolean equalsIgnoreOrder(final Bundle bundle1, final Bundle bundle2) {
+ if (bundle1 == null || bundle2 == null) {
+ return false;
+ } else if (!bundle1.getPrimaryContentType().equals(bundle2.getPrimaryContentType())) {
+ return false;
+ } else if (bundle1.getResources().size() != bundle2.getResources().size()) {
+ return false;
+ }
+ final HashSet bundle1Set = new HashSet<>(bundle1.getResources());
+ final HashSet bundle2Set = new HashSet<>(bundle2.getResources());
+ return bundle1Set.equals(bundle2Set);
+ }
}
diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java
index c24254c8a7..101fd0ee3b 100644
--- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java
+++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java
@@ -10,8 +10,6 @@
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@@ -36,14 +34,8 @@ public class BundleJSON {
public static final String JSON_SCHEMA_NAME = "htsbundle";
public static final String JSON_SCHEMA_VERSION = "0.1.0"; // TODO: bump this to 1.0.0
- final private static Set TOP_LEVEL_PROPERTIES = Collections.unmodifiableSet(
- new HashSet<>() {
- private static final long serialVersionUID = 1L;
- {
- add(JSON_PROPERTY_SCHEMA_NAME);
- add(JSON_PROPERTY_SCHEMA_VERSION);
- add(JSON_PROPERTY_PRIMARY);
- }});
+ final private static Set TOP_LEVEL_PROPERTIES =
+ Set.of(JSON_PROPERTY_SCHEMA_NAME,JSON_PROPERTY_SCHEMA_VERSION, JSON_PROPERTY_PRIMARY);
/**
* Serialize a bundle to a JSON string representation. All resources in the bundle must
@@ -77,14 +69,14 @@ public static String toJSON(final Bundle bundle) {
}
/**
- * Convert a (non-empty) Collection of Bundles to a JSON array string representation.
- * @param bundles a Collection of Bundles to serialize to JSON
- * @return a JSON string (array) representation of the collection of bundles
- * @throw IllegalArgumentException if the collection is empty
+ * Convert a (non-empty) List of Bundles to a JSON array string representation.
+ * @param bundles a List of Bundles to serialize to JSON
+ * @return a JSON string (array) representation of the list of bundles
+ * @throw IllegalArgumentException if the list is empty
*/
- public static String toJSON(final Collection bundles) {
+ public static String toJSON(final List bundles) {
if (bundles.isEmpty()) {
- throw new IllegalArgumentException("A bundle collection must contain at least one bundle");
+ throw new IllegalArgumentException("A bundle list must contain at least one bundle");
}
return bundles.stream()
.map(BundleJSON::toJSON)
@@ -120,35 +112,34 @@ public static Bundle toBundle(
return toBundle(new JSONObject(jsonString), ioPathConstructor);
} catch (JSONException | UnsupportedOperationException e) {
// see if the user provided a collection instead of a single bundle, and if so, present it as
- // a Bundle as long as it only contains one Bundle
+ // a Bundle as long as it only contains one BundleResource
try {
- final Collection bundles = toBundleCollection(jsonString, ioPathConstructor);
+ final List bundles = toBundleList(jsonString, ioPathConstructor);
if (bundles.size() > 1) {
throw new IllegalArgumentException(
- String.format("A JSON string with more than one bundle was provided but only a single Bundle is allowed",
- e.getMessage(),
+ String.format("A JSON string with more than one bundle was provided but only a single bundle is allowed in this context (%s)",
e.getMessage()));
}
return bundles.stream().findFirst().get();
} catch (JSONException | UnsupportedOperationException e2) {
throw new IllegalArgumentException(
- String.format("JSON can be interpreted neither as an individual bundle (%s) nor as a bundle collection (%s)",
- e2.getMessage(),
- e.getMessage()),
+ String.format("The JSON can be interpreted neither as an individual bundle (%s) nor as a bundle collection (%s)",
+ e.getMessage(),
+ e2.getMessage()),
e);
}
}
}
/**
- * Create a Collection from a jsonString, using a custom class that implements {@link IOPath} for all
+ * Create a List from a jsonString, using a custom class that implements {@link IOPath} for all
* resources.
* @param jsonString the json string must conform to the bundle schema, and may contain an array or single object
* @param ioPathConstructor constructor to use to create the backing IOPath for all resources
- * @return Collection
+ * @return List
* @param IOPath-derived class to use for IOPathResources
*/
- public static Collection toBundleCollection(
+ public static List toBundleList(
final String jsonString,
final Function ioPathConstructor) {
ValidationUtils.nonEmpty(jsonString, "json bundle string");
@@ -177,20 +168,20 @@ public static Collection toBundleCollection(
e);
}
}
- if (bundles.size() < 1) {
+ if (bundles.isEmpty()) {
throw new IllegalArgumentException("JSON bundle collection must contain at least one bundle");
}
return bundles;
}
/**
- * Create a Collection from a jsonString.
+ * Create a List from a jsonString.
*
* @param jsonString a JSON strings that conform to the bundle schema; may be an array or single object
- * @return a {@link Collection} created from a Collection of jsonStrings
+ * @return a {@link List} created from a Collection of jsonStrings
*/
- public static Collection toBundleCollection(final String jsonString) {
- return toBundleCollection(jsonString, HtsPath::new);
+ public static List toBundleList(final String jsonString) {
+ return toBundleList(jsonString, HtsPath::new);
}
private static Bundle toBundle(
@@ -213,15 +204,12 @@ private static Bundle toBundle(
final String primaryContentType = getRequiredPropertyAsString(jsonObject, JSON_PROPERTY_PRIMARY);
final Collection bundleResources = toBundleResources(jsonObject, ioPathConstructor);
- if (bundleResources.isEmpty()) {
- LOG.warn("Empty resource bundle found in: ", jsonObject.toString());
- }
return new Bundle(primaryContentType, bundleResources);
} catch (JSONException | UnsupportedOperationException e) {
throw new IllegalArgumentException(e);
}
}
- private static IOPathResource toBundleResource(
+ private static IOPathResource toBundleResource(
final String contentType,
final JSONObject jsonObject,
final Function ioPathConstructor) {
@@ -229,7 +217,7 @@ private static IOPathResource toBundleResource(
return new IOPathResource(
ioPathConstructor.apply(getRequiredPropertyAsString(jsonObject, JSON_PROPERTY_PATH)),
contentType,
- format == null ? null : format);
+ format);
}
private static Collection toBundleResources(
final JSONObject jsonResources,
diff --git a/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java b/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java
index 1243debb74..ea2c65b8c0 100644
--- a/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java
+++ b/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java
@@ -12,7 +12,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
-import java.util.Collection;
import java.util.List;
import java.util.Optional;
@@ -202,8 +201,8 @@ public void testAcceptSingleBundleJSONAsCollection(
final String jsonString,
final String primaryKey,
final List resources) {
- final Collection expectedBundleCollection = Arrays.asList(new Bundle(primaryKey, resources));
- final Collection actualBundleCollection = BundleJSON.toBundleCollection(jsonString);
+ final List expectedBundleCollection = Arrays.asList(new Bundle(primaryKey, resources));
+ final List actualBundleCollection = BundleJSON.toBundleList(jsonString);
Assert.assertEquals(actualBundleCollection, expectedBundleCollection);
}
@@ -334,7 +333,7 @@ public void testRejectInvalidJSON(final String jsonString, final String expected
@Test(dataProvider = "invalidBundleJSON", expectedExceptions = IllegalArgumentException.class)
public void testRejectInvalidJSONAsCollection(final String jsonString, final String expectedMessageFragment) {
try {
- BundleJSON.toBundleCollection(jsonString);
+ BundleJSON.toBundleList(jsonString);
} catch (final IllegalArgumentException e) {
Assert.assertTrue(e.getMessage().contains(expectedMessageFragment));
throw e;
@@ -454,19 +453,19 @@ public void testRoundTripJSONCollection(
final String jsonString,
final List bundles) {
// create a bundle collection from the input JSON, make sure if equals the test collection
- final Collection bundlesFromJSON = BundleJSON.toBundleCollection(jsonString);
+ final List bundlesFromJSON = BundleJSON.toBundleList(jsonString);
Assert.assertEquals(bundlesFromJSON, bundles);
// now write the test collection of bundles to JSON, and then roundtrip it back to a bundle collection
final String actualJSONString = BundleJSON.toJSON(bundles);
- final Collection bundlesFromRoundtripJSON = BundleJSON.toBundleCollection(actualJSONString);
+ final List bundlesFromRoundtripJSON = BundleJSON.toBundleList(actualJSONString);
Assert.assertEquals(bundlesFromRoundtripJSON, bundles);
}
@Test(expectedExceptions = IllegalArgumentException.class)
public void testRejectEmptyCollection() {
try {
- BundleJSON.toBundleCollection("[]");
+ BundleJSON.toBundleList("[]");
} catch (final IllegalArgumentException e) {
Assert.assertTrue(e.getMessage().contains("JSON bundle collection must contain at least one bundle"));
throw e;
diff --git a/src/test/java/htsjdk/beta/io/bundle/BundleTest.java b/src/test/java/htsjdk/beta/io/bundle/BundleTest.java
index 4ec6f67f00..38bfaa1641 100644
--- a/src/test/java/htsjdk/beta/io/bundle/BundleTest.java
+++ b/src/test/java/htsjdk/beta/io/bundle/BundleTest.java
@@ -7,6 +7,7 @@
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Iterator;
// Example JSON :
@@ -84,4 +85,9 @@ public void testResourceIterator() {
}
}
+ @Test(expectedExceptions = IllegalArgumentException.class)
+ public void testRejectEmptyBundle() {
+ new Bundle(BundleResourceType.ALIGNED_READS, Collections.EMPTY_LIST);
+ }
+
}
\ No newline at end of file
diff --git a/src/test/java/htsjdk/beta/plugin/reads/ReadsBundleTest.java b/src/test/java/htsjdk/beta/plugin/reads/ReadsBundleTest.java
index a697808871..04b0271f10 100644
--- a/src/test/java/htsjdk/beta/plugin/reads/ReadsBundleTest.java
+++ b/src/test/java/htsjdk/beta/plugin/reads/ReadsBundleTest.java
@@ -2,11 +2,7 @@
import htsjdk.HtsjdkTest;
import htsjdk.beta.io.IOPathUtils;
-import htsjdk.beta.io.bundle.Bundle;
-import htsjdk.beta.io.bundle.BundleBuilder;
-import htsjdk.beta.io.bundle.BundleJSON;
-import htsjdk.beta.io.bundle.BundleResourceType;
-import htsjdk.beta.io.bundle.IOPathResource;
+import htsjdk.beta.io.bundle.*;
import htsjdk.io.HtsPath;
import htsjdk.io.IOPath;
import org.testng.Assert;
@@ -138,8 +134,7 @@ public void testReadWriteRoundTrip(
final String jsonString,
final ReadsBundle expectedReadsBundle) {
final ReadsBundle bundleFromJSON = ReadsBundle.getReadsBundleFromString(jsonString);
- Assert.assertEquals(bundleFromJSON, expectedReadsBundle);
- Assert.assertEquals(bundleFromJSON.getPrimaryContentType(), expectedReadsBundle.getPrimaryContentType());
+ Assert.assertTrue(Bundle.equalsIgnoreOrder(bundleFromJSON, expectedReadsBundle));
Assert.assertTrue(bundleFromJSON.getReads().getIOPath().isPresent());
Assert.assertEquals(bundleFromJSON.getReads().getIOPath().get(), expectedReadsBundle.getReads().getIOPath().get());
}
@@ -152,8 +147,7 @@ public void testGetReadsBundleFromPath(
IOPathUtils.writeStringToPath(jsonFilePath, jsonString);
final ReadsBundle bundleFromPath = ReadsBundle.getReadsBundleFromPath(jsonFilePath);
- Assert.assertEquals(bundleFromPath, expectedReadsBundle);
- Assert.assertEquals(bundleFromPath.getPrimaryContentType(), expectedReadsBundle.getPrimaryContentType());
+ Assert.assertTrue(Bundle.equalsIgnoreOrder(bundleFromPath, expectedReadsBundle));
Assert.assertTrue(bundleFromPath.getReads().getIOPath().isPresent());
Assert.assertEquals(bundleFromPath.getReads().getIOPath().get(), expectedReadsBundle.getReads().getIOPath().get());
}