Review comments.

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 @@
  * <p>
  * 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<BundleResource>, 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<String, BundleResource> resources = new HashMap<>(); // content type -> resource
+    // Note that this uses LinkedHashMap to preserve the input order of the resources,
+    // but that order is not preserved when round tripping through JSON.
+    private final Map<String, BundleResource> resources = new LinkedHashMap<>(); // content type -> resource
     private final String primaryContentType;
 
     /**
@@ -55,7 +53,9 @@ public Bundle(final String primaryContentType, final Collection<BundleResource>
         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<BundleResource> bundle1Set = new HashSet<>(bundle1.getResources());
+        final HashSet<BundleResource> bundle2Set = new HashSet<>(bundle2.getResources());
+        return bundle1Set.equals(bundle2Set);
+    }
 }

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<String> 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<String> 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<Bundle> bundles) {
+    public static String toJSON(final List<Bundle> 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 <T extends IOPath> 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<Bundle> bundles = toBundleCollection(jsonString, ioPathConstructor);
+                final List<Bundle> 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<Bundle> from a jsonString, using a custom class that implements {@link IOPath} for all
+     * Create a List<Bundle> 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<Bundle>
+     * @return List<Bundle>
      * @param <T> IOPath-derived class to use for IOPathResources
      */
-    public static <T extends IOPath> Collection<Bundle> toBundleCollection(
+    public static <T extends IOPath> List<Bundle> toBundleList(
             final String jsonString,
             final Function<String, T> ioPathConstructor) {
         ValidationUtils.nonEmpty(jsonString, "json bundle string");
@@ -177,20 +168,20 @@ public static <T extends IOPath> Collection<Bundle> 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<Bundle> from a jsonString.
+     * Create a List<Bundle> 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<Bundle>} created from a Collection of jsonStrings
+     * @return a {@link List<Bundle>} created from a Collection of jsonStrings
      */
-    public static Collection<Bundle> toBundleCollection(final String jsonString) {
-        return toBundleCollection(jsonString, HtsPath::new);
+    public static List<Bundle> toBundleList(final String jsonString) {
+        return toBundleList(jsonString, HtsPath::new);
     }
 
     private static <T extends IOPath> Bundle toBundle(
@@ -213,23 +204,20 @@ private static <T extends IOPath> Bundle toBundle(
 
             final String primaryContentType = getRequiredPropertyAsString(jsonObject, JSON_PROPERTY_PRIMARY);
             final Collection<BundleResource> 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  <T extends IOPath> IOPathResource toBundleResource(
+    private static <T extends IOPath> IOPathResource toBundleResource(
             final String contentType,
             final JSONObject jsonObject,
             final Function<String, T> ioPathConstructor) {
         final String format = jsonObject.optString(JSON_PROPERTY_FORMAT, null);
         return new IOPathResource(
                 ioPathConstructor.apply(getRequiredPropertyAsString(jsonObject, JSON_PROPERTY_PATH)),
                 contentType,
-                format == null ? null : format);
+                format);
     }
     private static <T extends IOPath> Collection<BundleResource> toBundleResources(
             final JSONObject jsonResources,
@@ -249,7 +237,7 @@ private static <T extends IOPath> Collection<BundleResource> toBundleResources(
         return bundleResources;
     }
 
-    private static String getRequiredPropertyAsString(JSONObject jsonDocument, String propertyName) {
+    private static String getRequiredPropertyAsString(final JSONObject jsonDocument, final String propertyName) {
         final String propertyValue = jsonDocument.optString(propertyName, null);
         if (propertyValue == null) {
             throw new IllegalArgumentException(

src/main/java/htsjdk/beta/plugin/reads/ReadsBundle.java

@@ -195,7 +195,7 @@ private static <T extends IOPath> IOPathResource toInputResource(final String pr
 
     // Try to infer the contentType/format, i.e., READS/BAM from an IOPath. Currently this
     // exists purely to check for logical inconsistencies. It can detect cases that are illogical
-    // (an IOPath that has format CRAM, but file extension BAM), but it can't determinstically
+    // (an IOPath that has format CRAM, but file extension BAM), but it can't deterministically
     // and correctly infer the types in all cases without reproducing all the logic embedded in all the
     // codecs (i.e., an htsget IOPath ends in ".bam", but has format HTSGET_BAM, not BAM - detecting
     // that here would require parsing the entire IOPath structure, which is best left to the codecs

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<BundleResource> resources) {
-        final Collection<Bundle> expectedBundleCollection = Arrays.asList(new Bundle(primaryKey, resources));
-        final Collection<Bundle> actualBundleCollection = BundleJSON.toBundleCollection(jsonString);
+        final List<Bundle> expectedBundleCollection = Arrays.asList(new Bundle(primaryKey, resources));
+        final List<Bundle> 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<Bundle> bundles) {
         // create a bundle collection from the input JSON, make sure if equals the test collection
-        final Collection<Bundle> bundlesFromJSON = BundleJSON.toBundleCollection(jsonString);
+        final List<Bundle> 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<Bundle> bundlesFromRoundtripJSON = BundleJSON.toBundleCollection(actualJSONString);
+        final List<Bundle> 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;

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);
+    }
+
 }

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<IOPath> expectedReadsBundle)  {
         final ReadsBundle<IOPath> 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<IOPath> 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());
     }