From 1339761c5030dc6d7e7c7b9c84c5fb8d3cd0a7b1 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 25 Jun 2023 13:24:05 -0700 Subject: [PATCH 1/6] Update snappy-java and migrate mjson to org.json to address CVEs --- build.gradle | 6 +- .../htsjdk/beta/io/bundle/BundleJSON.java | 70 +++++++++++-------- .../util/htsget/HtsgetErrorResponse.java | 14 ++-- .../util/htsget/HtsgetPOSTRequest.java | 29 ++++---- .../samtools/util/htsget/HtsgetResponse.java | 38 +++++----- .../samtools/util/HtsgetRequestUnitTest.java | 30 ++++---- 6 files changed, 101 insertions(+), 86 deletions(-) diff --git a/build.gradle b/build.gradle index 70a58414bf..3b6ff88624 100644 --- a/build.gradle +++ b/build.gradle @@ -30,12 +30,10 @@ jacocoTestReport { dependencies { implementation "commons-logging:commons-logging:1.1.1" - implementation "org.xerial.snappy:snappy-java:1.1.8.4" + implementation "org.xerial.snappy:snappy-java:1.1.10.1" implementation "org.apache.commons:commons-compress:1.22" implementation 'org.tukaani:xz:1.8' - implementation ('org.sharegov:mjson:1.4.1') { - exclude group: "junit" - } + implementation "org.json:json:20230227" implementation 'org.openjdk.nashorn:nashorn-core:15.4' diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java index 4419371501..9a60af934e 100644 --- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java +++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java @@ -4,7 +4,8 @@ import htsjdk.io.IOPath; import htsjdk.samtools.util.Log; import htsjdk.utils.ValidationUtils; -import mjson.Json; +import org.json.JSONException; +import org.json.JSONObject; import java.util.ArrayList; import java.util.Collections; @@ -50,10 +51,10 @@ public class BundleJSON { * @throws IllegalArgumentException if any resource in bundle is not an IOPathResources. */ public static String toJSON(final Bundle bundle) { - final Json outerJSON = Json.object() - .set(JSON_PROPERTY_SCHEMA_NAME, JSON_SCHEMA_NAME) - .set(JSON_PROPERTY_SCHEMA_VERSION, JSON_SCHEMA_VERSION) - .set(JSON_PROPERTY_PRIMARY, bundle.getPrimaryContentType()); + final JSONObject outerJSON = new JSONObject(); + outerJSON.put(JSON_PROPERTY_SCHEMA_NAME, JSON_SCHEMA_NAME); + outerJSON.put(JSON_PROPERTY_SCHEMA_VERSION, JSON_SCHEMA_VERSION); + outerJSON.put(JSON_PROPERTY_PRIMARY, bundle.getPrimaryContentType()); bundle.forEach(bundleResource -> { final Optional resourcePath = bundleResource.getIOPath(); @@ -62,11 +63,11 @@ public static String toJSON(final Bundle bundle) { } // generate JSON for each bundle resource - final Json resourceJSON = Json.object().set(JSON_PROPERTY_PATH, resourcePath.get().getURIString()); + final JSONObject resourceJSON = new JSONObject().put(JSON_PROPERTY_PATH, resourcePath.get().getURIString()); if (bundleResource.getFileFormat().isPresent()) { - resourceJSON.set(JSON_PROPERTY_FORMAT, bundleResource.getFileFormat().get()); + resourceJSON.put(JSON_PROPERTY_FORMAT, bundleResource.getFileFormat().get()); } - outerJSON.set(bundleResource.getContentType(), resourceJSON); + outerJSON.put(bundleResource.getContentType(), resourceJSON); }); return prettyPrintJSON(outerJSON); @@ -100,7 +101,7 @@ public static Bundle toBundle( String primaryContentType; try { - final Json jsonDocument = Json.read(jsonString); + final JSONObject jsonDocument = new JSONObject(jsonString); if (jsonDocument == null || jsonString.length() < 1) { throw new IllegalArgumentException( String.format("JSON file parsing failed %s", jsonString)); @@ -121,9 +122,13 @@ public static Bundle toBundle( } primaryContentType = getPropertyAsString(JSON_PROPERTY_PRIMARY, jsonDocument); - jsonDocument.asJsonMap().forEach((String contentType, Json jsonDoc) -> { + jsonDocument.toMap().forEach((String contentType, Object jsonDocObj) -> { + if (! (jsonDocObj instanceof JSONObject jsonDoc)) { + throw new IllegalStateException("Not a JSONObject"); + } + if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { - final Json format = jsonDoc.at(JSON_PROPERTY_FORMAT); + final String format = jsonDoc.optString(JSON_PROPERTY_FORMAT); final IOPathResource ioPathResource = new IOPathResource( ioPathConstructor.apply(getPropertyAsString(JSON_PROPERTY_PATH, jsonDoc)), contentType, @@ -136,7 +141,7 @@ public static Bundle toBundle( if (resources.isEmpty()) { LOG.warn("Empty resource bundle found: ", jsonString); } - } catch (Json.MalformedJsonException | java.lang.UnsupportedOperationException e) { + } catch (JSONException | UnsupportedOperationException e) { throw new IllegalArgumentException(e); } @@ -145,7 +150,7 @@ public static Bundle toBundle( // Simple pretty-printer to produce indented JSON strings from a Json document. Note that // this is not generalized and will only work on Json documents produced by BundleJSON::toJSON. - private static String prettyPrintJSON(final Json jsonDocument) { + private static String prettyPrintJSON(final JSONObject jsonDocument) { final StringBuilder sb = new StringBuilder(); final String TOP_LEVEL_PROPERTY_FORMAT = " \"%s\":\"%s\""; @@ -168,9 +173,13 @@ private static String prettyPrintJSON(final Json jsonDocument) { sb.append(",\n"); final List formattedResources = new ArrayList<>(); - jsonDocument.asJsonMap().forEach((String contentType, Json jsonDoc) -> { + jsonDocument.toMap().forEach((String contentType, Object jsonDocObj) -> { + if (! (jsonDocObj instanceof JSONObject jsonDoc)) { + throw new IllegalStateException("Not a JSONObject"); + } + if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { - final Json format = jsonDoc.at(JSON_PROPERTY_FORMAT); + final JSONObject format = jsonDoc.getJSONObject(JSON_PROPERTY_FORMAT); final StringBuilder resSB = new StringBuilder(); if (format != null) { resSB.append(String.format("{\"%s\":\"%s\",\"%s\":\"%s\"}", @@ -188,7 +197,7 @@ private static String prettyPrintJSON(final Json jsonDocument) { }); sb.append(formattedResources.stream().collect(Collectors.joining(",\n", "", "\n"))); sb.append("}\n"); - } catch (Json.MalformedJsonException | java.lang.UnsupportedOperationException e) { + } catch (JSONException | UnsupportedOperationException e) { throw new IllegalArgumentException(e); } @@ -196,20 +205,21 @@ private static String prettyPrintJSON(final Json jsonDocument) { } // return the value of propertyName from jsonDocument as a String value - private static String getPropertyAsString(final String propertyName, final Json jsonDocument) { - final Json propertyValue = jsonDocument.at(propertyName); - if (propertyValue == null) { - throw new IllegalArgumentException( - String.format("JSON bundle is missing the required property %s (%s)", - propertyName, - jsonDocument.toString())); - } else if (!propertyValue.isString()) { - throw new IllegalArgumentException( - String.format("Expected string value for bundle property %s but found %s", - propertyName, - propertyValue.toString())); - } - return propertyValue.asString(); + private static String getPropertyAsString(final String propertyName, final JSONObject jsonDocument) { + return jsonDocument.getString(propertyName); +// final JSONObject propertyValue = jsonDocument.at(propertyName); +// if (propertyValue == null) { +// throw new IllegalArgumentException( +// String.format("JSON bundle is missing the required property %s (%s)", +// propertyName, +// jsonDocument.toString())); +// } else if (!propertyValue.isString()) { +// throw new IllegalArgumentException( +// String.format("Expected string value for bundle property %s but found %s", +// propertyName, +// propertyValue.toString())); +// } +// return propertyValue.asString(); } } diff --git a/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java b/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java index 7269fb41b6..fe6145c0a4 100644 --- a/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java +++ b/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java @@ -1,6 +1,8 @@ package htsjdk.samtools.util.htsget; +import org.json.JSONObject; + /** * Class allowing deserialization from json htsget error response, as defined in https://samtools.github.io/hts-specs/htsget.html * @@ -30,17 +32,15 @@ public String getMessage() { } public static HtsgetErrorResponse parse(final String s) { - final mjson.Json j = mjson.Json.read(s); - final mjson.Json htsget = j.at("htsget"); + final JSONObject j = new JSONObject(s); + final JSONObject htsget = j.optJSONObject("htsget"); if (htsget == null) { throw new IllegalStateException(new HtsgetMalformedResponseException("No htsget key found in response")); } - final mjson.Json errorJson = htsget.at("error"); - final mjson.Json messageJson = htsget.at("message"); + final String errorJson = htsget.optString("error"); + final String messageJson = htsget.optString("message"); - return new HtsgetErrorResponse( - errorJson == null ? null : errorJson.asString(), - messageJson == null ? null : messageJson.asString()); + return new HtsgetErrorResponse(errorJson, messageJson); } } diff --git a/src/main/java/htsjdk/samtools/util/htsget/HtsgetPOSTRequest.java b/src/main/java/htsjdk/samtools/util/htsget/HtsgetPOSTRequest.java index 11d89f41d5..7457491419 100644 --- a/src/main/java/htsjdk/samtools/util/htsget/HtsgetPOSTRequest.java +++ b/src/main/java/htsjdk/samtools/util/htsget/HtsgetPOSTRequest.java @@ -3,7 +3,8 @@ import htsjdk.samtools.SAMRecord; import htsjdk.samtools.util.Locatable; import htsjdk.samtools.util.RuntimeIOException; -import mjson.Json; +import org.json.JSONArray; +import org.json.JSONObject; import java.io.IOException; import java.net.HttpURLConnection; @@ -134,41 +135,41 @@ public URI toURI() { return this.getEndpoint(); } - public mjson.Json queryBody() { - final mjson.Json postBody = Json.object(); + public JSONObject queryBody() { + final JSONObject postBody = new JSONObject(); if (this.format != null) { - postBody.set("format", this.getFormat().toString()); + postBody.put("format", this.getFormat().toString()); } if (this.dataClass != null) { - postBody.set("class", this.getDataClass().toString()); + postBody.put("class", this.getDataClass().toString()); } if (!this.fields.isEmpty()) { - postBody.set( + postBody.put( "fields", - Json.array(this.getFields().stream() + new JSONArray(this.getFields().stream() .map(HtsgetRequestField::toString) .toArray()) ); } if (!this.tags.isEmpty()) { - postBody.set("tags", Json.array(this.getTags().toArray())); + postBody.put("tags", new JSONArray(this.getTags().toArray())); } if (!this.notags.isEmpty()) { - postBody.set("notags", Json.array(this.getNoTags().toArray())); + postBody.put("notags", new JSONArray(this.getNoTags().toArray())); } if (!this.intervals.isEmpty()) { - postBody.set("regions", Json.array( + postBody.put("regions", new JSONArray( this.intervals.stream() .map(interval -> { - final mjson.Json intervalJson = Json.object(); + final JSONObject intervalJson = new JSONObject(); if (interval != null && interval.getContig() != null) { - intervalJson.set("referenceName", interval.getContig()); + intervalJson.put("referenceName", interval.getContig()); // Do not insert start and end for unmapped reads or if we are requesting the entire contig if (!interval.getContig().equals(SAMRecord.NO_ALIGNMENT_REFERENCE_NAME)) { // getStart() - 1 is necessary as GA4GH standards use 0-based coordinates while Locatables are 1-based - intervalJson.set("start", interval.getStart() - 1); + intervalJson.put("start", interval.getStart() - 1); if (interval.getEnd() != Integer.MAX_VALUE && interval.getEnd() != -1) { - intervalJson.set("end", interval.getEnd()); + intervalJson.put("end", interval.getEnd()); } } } diff --git a/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java b/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java index 908bda1253..1e79d1c622 100644 --- a/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java +++ b/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java @@ -1,6 +1,8 @@ package htsjdk.samtools.util.htsget; import htsjdk.samtools.util.RuntimeIOException; +import org.json.JSONArray; +import org.json.JSONObject; import java.io.*; import java.net.HttpURLConnection; @@ -98,32 +100,32 @@ public InputStream getData() { * @param blockJson json value representing a block * @return parsed block object */ - public static Block parse(final mjson.Json blockJson) { - final mjson.Json uriJson = blockJson.at("url"); + public static Block parse(final JSONObject blockJson) { + final String uriJson = blockJson.optString("url"); if (uriJson == null) { throw new HtsgetMalformedResponseException("No URI found in Htsget data block: " + blockJson.toString().substring(0, Math.min(100, blockJson.toString().length()))); } final URI uri; try { - uri = new URI(uriJson.asString()); + uri = new URI(uriJson); } catch (final URISyntaxException e) { - throw new HtsgetMalformedResponseException("Could not parse URI in Htsget data block: " + uriJson.asString(), e); + throw new HtsgetMalformedResponseException("Could not parse URI in Htsget data block: " + uriJson, e); } - final mjson.Json dataClassJson = blockJson.at("class"); + final String dataClassJson = blockJson.optString("class"); final HtsgetClass dataClass = dataClassJson == null ? null - : HtsgetClass.valueOf(dataClassJson.asString().toLowerCase()); + : HtsgetClass.valueOf(dataClassJson.toLowerCase()); - final mjson.Json headersJson = blockJson.at("headers"); + final JSONObject headersJson = blockJson.optJSONObject("headers"); final Map headers = headersJson == null ? null - : headersJson.asJsonMap().entrySet().stream() + : headersJson.toMap().entrySet().stream() .collect(Collectors.toMap( Map.Entry::getKey, - e -> e.getValue().asString() + e -> e.getValue().toString() )); return new Block(uri, headers, dataClass); @@ -161,28 +163,30 @@ public String getMd5() { * @return parsed HtsgetResponse object */ public static HtsgetResponse parse(final String s) { - final mjson.Json j = mjson.Json.read(s); - final mjson.Json htsget = j.at("htsget"); + final JSONObject j = new JSONObject(s); + final JSONObject htsget = j.optJSONObject("htsget"); if (htsget == null) { throw new HtsgetMalformedResponseException("No htsget key found in response"); } - final mjson.Json md5Json = htsget.at("md5"); - final mjson.Json formatJson = htsget.at("format"); + final String md5Json = htsget.optString("md5"); + final String formatJson = htsget.optString("format"); - final mjson.Json blocksJson = htsget.at("urls"); + final JSONArray blocksJson = htsget.optJSONArray("urls"); if (blocksJson == null) { throw new HtsgetMalformedResponseException("No urls field found in Htsget Response"); } - final List blocks = blocksJson.asJsonList().stream() + final List blocks = blocksJson.toList().stream() + .filter(JSONObject.class::isInstance) + .map(JSONObject.class::cast) .map(Block::parse) .collect(Collectors.toList()); return new HtsgetResponse( - formatJson == null ? null : HtsgetFormat.valueOf(formatJson.asString().toUpperCase()), + formatJson == null ? null : HtsgetFormat.valueOf(formatJson.toUpperCase()), blocks, - md5Json == null ? null : md5Json.asString() + md5Json ); } diff --git a/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java b/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java index fc5819c19d..7ef3e28401 100644 --- a/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java +++ b/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java @@ -2,6 +2,8 @@ import htsjdk.HtsjdkTest; import htsjdk.samtools.util.htsget.*; +import org.json.JSONArray; +import org.json.JSONObject; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -115,33 +117,33 @@ public void testPOSTjsonBody() { new Interval("chrM", 17, 32)) ); - final mjson.Json postBody = req.queryBody(); + final JSONObject postBody = req.queryBody(); - Assert.assertEquals(postBody.at("format").asString(), "BAM"); - Assert.assertEquals(postBody.at("class").asString(), "header"); + Assert.assertEquals(postBody.getString("format"), "BAM"); + Assert.assertEquals(postBody.getString("class"), "header"); Assert.assertEqualsNoOrder( - postBody.at("fields").asList().stream().map(Object::toString).toArray(String[]::new), + postBody.getJSONArray("fields").toList().stream().map(Object::toString).toArray(String[]::new), new String[]{"QNAME", "CIGAR"} ); Assert.assertEqualsNoOrder( - postBody.at("tags").asList().stream().map(Object::toString).toArray(String[]::new), + postBody.getJSONArray("tags").toList().stream().map(Object::toString).toArray(String[]::new), new String[]{"tag1", "tag3"} ); Assert.assertEqualsNoOrder( - postBody.at("notags").asList().stream().map(Object::toString).toArray(String[]::new), + postBody.getJSONArray("notags").toList().stream().map(Object::toString).toArray(String[]::new), new String[]{"tag2"} ); - final mjson.Json[] expectedRegions = new mjson.Json[]{ - mjson.Json.object("referenceName", "chr1", "start", 0, "end", 16), - mjson.Json.object("referenceName", "chr1", "start", 16, "end", 32), - mjson.Json.object("referenceName", "chrM", "start", 0, "end", 16), - mjson.Json.object("referenceName", "chrM", "start", 16, "end", 32), - }; + final JSONArray expectedRegions = new JSONArray(); + expectedRegions.put(new JSONObject("{referenceName: \"chr1\", start: 0, end: 16}")); + expectedRegions.put(new JSONObject("{referenceName: \"chr1\", start: 16, end: 32}")); + expectedRegions.put(new JSONObject("{referenceName: \"chrM\", start: 0, end: 16}")); + expectedRegions.put(new JSONObject("{referenceName: \"chrM\", start: 16, end: 32}")); + Assert.assertEqualsNoOrder( - postBody.at("regions").asJsonList().toArray(new mjson.Json[0]), - expectedRegions + postBody.getJSONArray("regions").toList(), + expectedRegions.toList() ); } } \ No newline at end of file From a249e196f58aa01dbb1ad6f1ef1a0f6726de6bf3 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 25 Jun 2023 17:48:40 -0700 Subject: [PATCH 2/6] Test fixes --- .../htsjdk/beta/io/bundle/BundleJSON.java | 2 +- .../util/htsget/HtsgetErrorResponse.java | 4 ++-- .../samtools/util/htsget/HtsgetResponse.java | 19 +++++++++++-------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java index 9a60af934e..fe6c324f1b 100644 --- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java +++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java @@ -128,7 +128,7 @@ public static Bundle toBundle( } if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { - final String format = jsonDoc.optString(JSON_PROPERTY_FORMAT); + final String format = jsonDoc.optString(JSON_PROPERTY_FORMAT, null); final IOPathResource ioPathResource = new IOPathResource( ioPathConstructor.apply(getPropertyAsString(JSON_PROPERTY_PATH, jsonDoc)), contentType, diff --git a/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java b/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java index fe6145c0a4..9cf10bc4ad 100644 --- a/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java +++ b/src/main/java/htsjdk/samtools/util/htsget/HtsgetErrorResponse.java @@ -38,8 +38,8 @@ public static HtsgetErrorResponse parse(final String s) { throw new IllegalStateException(new HtsgetMalformedResponseException("No htsget key found in response")); } - final String errorJson = htsget.optString("error"); - final String messageJson = htsget.optString("message"); + final String errorJson = htsget.optString("error", null); + final String messageJson = htsget.optString("message", null); return new HtsgetErrorResponse(errorJson, messageJson); } diff --git a/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java b/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java index 1e79d1c622..e73de907b3 100644 --- a/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java +++ b/src/main/java/htsjdk/samtools/util/htsget/HtsgetResponse.java @@ -4,12 +4,16 @@ import org.json.JSONArray; import org.json.JSONObject; -import java.io.*; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.SequenceInputStream; import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; import java.util.*; import java.util.stream.Collectors; +import java.util.stream.IntStream; /** * Class allowing deserialization from json htsget response, as defined in https://samtools.github.io/hts-specs/htsget.html @@ -101,7 +105,7 @@ public InputStream getData() { * @return parsed block object */ public static Block parse(final JSONObject blockJson) { - final String uriJson = blockJson.optString("url"); + final String uriJson = blockJson.optString("url", null); if (uriJson == null) { throw new HtsgetMalformedResponseException("No URI found in Htsget data block: " + blockJson.toString().substring(0, Math.min(100, blockJson.toString().length()))); @@ -113,7 +117,7 @@ public static Block parse(final JSONObject blockJson) { throw new HtsgetMalformedResponseException("Could not parse URI in Htsget data block: " + uriJson, e); } - final String dataClassJson = blockJson.optString("class"); + final String dataClassJson = blockJson.optString("class", null); final HtsgetClass dataClass = dataClassJson == null ? null : HtsgetClass.valueOf(dataClassJson.toLowerCase()); @@ -169,17 +173,16 @@ public static HtsgetResponse parse(final String s) { throw new HtsgetMalformedResponseException("No htsget key found in response"); } - final String md5Json = htsget.optString("md5"); - final String formatJson = htsget.optString("format"); + final String md5Json = htsget.optString("md5", null); + final String formatJson = htsget.optString("format", null); final JSONArray blocksJson = htsget.optJSONArray("urls"); if (blocksJson == null) { throw new HtsgetMalformedResponseException("No urls field found in Htsget Response"); } - final List blocks = blocksJson.toList().stream() - .filter(JSONObject.class::isInstance) - .map(JSONObject.class::cast) + final List blocks = IntStream.range(0, blocksJson.length()) + .mapToObj(blocksJson::getJSONObject) .map(Block::parse) .collect(Collectors.toList()); From 921a19e9402271fb24c941893d3208dc10a2f083 Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 25 Jun 2023 17:55:11 -0700 Subject: [PATCH 3/6] Minor code cleanup --- .../htsjdk/beta/io/bundle/BundleJSON.java | 51 ++++++------------- .../samtools/util/HtsgetRequestUnitTest.java | 10 ++-- 2 files changed, 21 insertions(+), 40 deletions(-) diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java index fe6c324f1b..49a5c06319 100644 --- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java +++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java @@ -51,10 +51,10 @@ public class BundleJSON { * @throws IllegalArgumentException if any resource in bundle is not an IOPathResources. */ public static String toJSON(final Bundle bundle) { - final JSONObject outerJSON = new JSONObject(); - outerJSON.put(JSON_PROPERTY_SCHEMA_NAME, JSON_SCHEMA_NAME); - outerJSON.put(JSON_PROPERTY_SCHEMA_VERSION, JSON_SCHEMA_VERSION); - outerJSON.put(JSON_PROPERTY_PRIMARY, bundle.getPrimaryContentType()); + final JSONObject outerJSON = new JSONObject() + .put(JSON_PROPERTY_SCHEMA_NAME, JSON_SCHEMA_NAME) + .put(JSON_PROPERTY_SCHEMA_VERSION, JSON_SCHEMA_VERSION) + .put(JSON_PROPERTY_PRIMARY, bundle.getPrimaryContentType()); bundle.forEach(bundleResource -> { final Optional resourcePath = bundleResource.getIOPath(); @@ -102,25 +102,25 @@ public static Bundle toBundle( try { final JSONObject jsonDocument = new JSONObject(jsonString); - if (jsonDocument == null || jsonString.length() < 1) { + if (jsonString.length() < 1) { throw new IllegalArgumentException( String.format("JSON file parsing failed %s", jsonString)); } // validate the schema name - final String schemaName = getPropertyAsString(JSON_PROPERTY_SCHEMA_NAME, jsonDocument); + final String schemaName = jsonDocument.optString(JSON_PROPERTY_SCHEMA_NAME, null); if (!schemaName.equals(JSON_SCHEMA_NAME)) { throw new IllegalArgumentException( String.format("Expected bundle schema name %s but found %s", JSON_SCHEMA_NAME, schemaName)); } // validate the schema version - final String schemaVersion = getPropertyAsString(JSON_PROPERTY_SCHEMA_VERSION, jsonDocument); + final String schemaVersion = jsonDocument.optString(JSON_PROPERTY_SCHEMA_VERSION, null); if (!schemaVersion.equals(JSON_SCHEMA_VERSION)) { throw new IllegalArgumentException(String.format("Expected bundle schema version %s but found %s", JSON_SCHEMA_VERSION, schemaVersion)); } - primaryContentType = getPropertyAsString(JSON_PROPERTY_PRIMARY, jsonDocument); + primaryContentType = jsonDocument.optString(JSON_PROPERTY_PRIMARY, null); jsonDocument.toMap().forEach((String contentType, Object jsonDocObj) -> { if (! (jsonDocObj instanceof JSONObject jsonDoc)) { @@ -130,11 +130,11 @@ public static Bundle toBundle( if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { final String format = jsonDoc.optString(JSON_PROPERTY_FORMAT, null); final IOPathResource ioPathResource = new IOPathResource( - ioPathConstructor.apply(getPropertyAsString(JSON_PROPERTY_PATH, jsonDoc)), + ioPathConstructor.apply(jsonDoc.optString(JSON_PROPERTY_PATH, null)), contentType, format == null ? null : - getPropertyAsString(JSON_PROPERTY_FORMAT, jsonDoc)); + jsonDoc.optString(JSON_PROPERTY_FORMAT, null)); resources.add(ioPathResource); } }); @@ -158,17 +158,17 @@ private static String prettyPrintJSON(final JSONObject jsonDocument) { sb.append("{\n"); // schema name - final String schemaName = getPropertyAsString(JSON_PROPERTY_SCHEMA_NAME, jsonDocument); + final String schemaName = jsonDocument.optString(JSON_PROPERTY_SCHEMA_NAME, null); sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_SCHEMA_NAME, schemaName)); sb.append(",\n"); // schema version - final String schemaVersion = getPropertyAsString(JSON_PROPERTY_SCHEMA_VERSION, jsonDocument); + final String schemaVersion = jsonDocument.optString(JSON_PROPERTY_SCHEMA_VERSION, null); sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_SCHEMA_VERSION, schemaVersion)); sb.append(",\n"); // primary - final String primary = getPropertyAsString(JSON_PROPERTY_PRIMARY, jsonDocument); + final String primary = jsonDocument.optString(JSON_PROPERTY_PRIMARY, null); sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_PRIMARY, primary)); sb.append(",\n"); @@ -184,13 +184,13 @@ private static String prettyPrintJSON(final JSONObject jsonDocument) { if (format != null) { resSB.append(String.format("{\"%s\":\"%s\",\"%s\":\"%s\"}", JSON_PROPERTY_PATH, - getPropertyAsString(JSON_PROPERTY_PATH, jsonDoc), + jsonDoc.optString(JSON_PROPERTY_PATH, null), JSON_PROPERTY_FORMAT, - getPropertyAsString(JSON_PROPERTY_FORMAT, jsonDoc))); + jsonDoc.optString(JSON_PROPERTY_FORMAT, null))); } else { resSB.append(String.format("{\"%s\":\"%s\"}", JSON_PROPERTY_PATH, - getPropertyAsString(JSON_PROPERTY_PATH, jsonDoc))); + jsonDoc.optString(JSON_PROPERTY_PATH, null))); } formattedResources.add(String.format(" \"%s\":%s", contentType, resSB.toString())); } @@ -203,23 +203,4 @@ private static String prettyPrintJSON(final JSONObject jsonDocument) { return sb.toString(); } - - // return the value of propertyName from jsonDocument as a String value - private static String getPropertyAsString(final String propertyName, final JSONObject jsonDocument) { - return jsonDocument.getString(propertyName); -// final JSONObject propertyValue = jsonDocument.at(propertyName); -// if (propertyValue == null) { -// throw new IllegalArgumentException( -// String.format("JSON bundle is missing the required property %s (%s)", -// propertyName, -// jsonDocument.toString())); -// } else if (!propertyValue.isString()) { -// throw new IllegalArgumentException( -// String.format("Expected string value for bundle property %s but found %s", -// propertyName, -// propertyValue.toString())); -// } -// return propertyValue.asString(); - } - } diff --git a/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java b/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java index 7ef3e28401..a53be24f89 100644 --- a/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java +++ b/src/test/java/htsjdk/samtools/util/HtsgetRequestUnitTest.java @@ -135,11 +135,11 @@ public void testPOSTjsonBody() { new String[]{"tag2"} ); - final JSONArray expectedRegions = new JSONArray(); - expectedRegions.put(new JSONObject("{referenceName: \"chr1\", start: 0, end: 16}")); - expectedRegions.put(new JSONObject("{referenceName: \"chr1\", start: 16, end: 32}")); - expectedRegions.put(new JSONObject("{referenceName: \"chrM\", start: 0, end: 16}")); - expectedRegions.put(new JSONObject("{referenceName: \"chrM\", start: 16, end: 32}")); + final JSONArray expectedRegions = new JSONArray() + .put(new JSONObject("{referenceName: \"chr1\", start: 0, end: 16}")) + .put(new JSONObject("{referenceName: \"chr1\", start: 16, end: 32}")) + .put(new JSONObject("{referenceName: \"chrM\", start: 0, end: 16}")) + .put(new JSONObject("{referenceName: \"chrM\", start: 16, end: 32}")); Assert.assertEqualsNoOrder( postBody.getJSONArray("regions").toList(), From 95a722e3a2ef81aa1dd029e2bf17358c4cc287df Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 25 Jun 2023 21:24:31 -0700 Subject: [PATCH 4/6] Test fix --- .../htsjdk/beta/io/bundle/BundleJSON.java | 64 ++----------------- 1 file changed, 4 insertions(+), 60 deletions(-) diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java index 49a5c06319..c6358ea862 100644 --- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java +++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java @@ -70,7 +70,7 @@ public static String toJSON(final Bundle bundle) { outerJSON.put(bundleResource.getContentType(), resourceJSON); }); - return prettyPrintJSON(outerJSON); + return outerJSON.toString(1); } /** @@ -122,9 +122,9 @@ public static Bundle toBundle( } primaryContentType = jsonDocument.optString(JSON_PROPERTY_PRIMARY, null); - jsonDocument.toMap().forEach((String contentType, Object jsonDocObj) -> { - if (! (jsonDocObj instanceof JSONObject jsonDoc)) { - throw new IllegalStateException("Not a JSONObject"); + jsonDocument.keySet().forEach((String contentType) -> { + if (! (jsonDocument.get(contentType) instanceof JSONObject jsonDoc)) { + return; } if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { @@ -147,60 +147,4 @@ public static Bundle toBundle( return new Bundle(primaryContentType, resources); } - - // Simple pretty-printer to produce indented JSON strings from a Json document. Note that - // this is not generalized and will only work on Json documents produced by BundleJSON::toJSON. - private static String prettyPrintJSON(final JSONObject jsonDocument) { - final StringBuilder sb = new StringBuilder(); - final String TOP_LEVEL_PROPERTY_FORMAT = " \"%s\":\"%s\""; - - try { - sb.append("{\n"); - - // schema name - final String schemaName = jsonDocument.optString(JSON_PROPERTY_SCHEMA_NAME, null); - sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_SCHEMA_NAME, schemaName)); - sb.append(",\n"); - - // schema version - final String schemaVersion = jsonDocument.optString(JSON_PROPERTY_SCHEMA_VERSION, null); - sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_SCHEMA_VERSION, schemaVersion)); - sb.append(",\n"); - - // primary - final String primary = jsonDocument.optString(JSON_PROPERTY_PRIMARY, null); - sb.append(String.format(TOP_LEVEL_PROPERTY_FORMAT, JSON_PROPERTY_PRIMARY, primary)); - sb.append(",\n"); - - final List formattedResources = new ArrayList<>(); - jsonDocument.toMap().forEach((String contentType, Object jsonDocObj) -> { - if (! (jsonDocObj instanceof JSONObject jsonDoc)) { - throw new IllegalStateException("Not a JSONObject"); - } - - if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { - final JSONObject format = jsonDoc.getJSONObject(JSON_PROPERTY_FORMAT); - final StringBuilder resSB = new StringBuilder(); - if (format != null) { - resSB.append(String.format("{\"%s\":\"%s\",\"%s\":\"%s\"}", - JSON_PROPERTY_PATH, - jsonDoc.optString(JSON_PROPERTY_PATH, null), - JSON_PROPERTY_FORMAT, - jsonDoc.optString(JSON_PROPERTY_FORMAT, null))); - } else { - resSB.append(String.format("{\"%s\":\"%s\"}", - JSON_PROPERTY_PATH, - jsonDoc.optString(JSON_PROPERTY_PATH, null))); - } - formattedResources.add(String.format(" \"%s\":%s", contentType, resSB.toString())); - } - }); - sb.append(formattedResources.stream().collect(Collectors.joining(",\n", "", "\n"))); - sb.append("}\n"); - } catch (JSONException | UnsupportedOperationException e) { - throw new IllegalArgumentException(e); - } - - return sb.toString(); - } } From fb0e2048d1f8993ff0ab6b0a7a2ceb72591fd9ab Mon Sep 17 00:00:00 2001 From: bbimber Date: Sun, 25 Jun 2023 22:18:47 -0700 Subject: [PATCH 5/6] Test fix --- .../htsjdk/beta/io/bundle/BundleJSON.java | 20 +++++++++++++++---- .../htsjdk/beta/io/bundle/BundleJSONTest.java | 5 +++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java index c6358ea862..324e2b8bed 100644 --- a/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java +++ b/src/main/java/htsjdk/beta/io/bundle/BundleJSON.java @@ -108,19 +108,19 @@ public static Bundle toBundle( } // validate the schema name - final String schemaName = jsonDocument.optString(JSON_PROPERTY_SCHEMA_NAME, null); + final String schemaName = getRequiredPropertyAsString(jsonDocument, JSON_PROPERTY_SCHEMA_NAME); if (!schemaName.equals(JSON_SCHEMA_NAME)) { throw new IllegalArgumentException( String.format("Expected bundle schema name %s but found %s", JSON_SCHEMA_NAME, schemaName)); } // validate the schema version - final String schemaVersion = jsonDocument.optString(JSON_PROPERTY_SCHEMA_VERSION, null); + final String schemaVersion = getRequiredPropertyAsString(jsonDocument, JSON_PROPERTY_SCHEMA_VERSION); if (!schemaVersion.equals(JSON_SCHEMA_VERSION)) { throw new IllegalArgumentException(String.format("Expected bundle schema version %s but found %s", JSON_SCHEMA_VERSION, schemaVersion)); } - primaryContentType = jsonDocument.optString(JSON_PROPERTY_PRIMARY, null); + primaryContentType = getRequiredPropertyAsString(jsonDocument, JSON_PROPERTY_PRIMARY); jsonDocument.keySet().forEach((String contentType) -> { if (! (jsonDocument.get(contentType) instanceof JSONObject jsonDoc)) { @@ -130,7 +130,7 @@ public static Bundle toBundle( if (!TOP_LEVEL_PROPERTIES.contains(contentType)) { final String format = jsonDoc.optString(JSON_PROPERTY_FORMAT, null); final IOPathResource ioPathResource = new IOPathResource( - ioPathConstructor.apply(jsonDoc.optString(JSON_PROPERTY_PATH, null)), + ioPathConstructor.apply(getRequiredPropertyAsString(jsonDoc, JSON_PROPERTY_PATH)), contentType, format == null ? null : @@ -147,4 +147,16 @@ public static Bundle toBundle( return new Bundle(primaryContentType, resources); } + + private static String getRequiredPropertyAsString(JSONObject jsonDocument, String propertyName) { + final String propertyValue = jsonDocument.optString(propertyName, null); + if (propertyValue == null) { + throw new IllegalArgumentException( + String.format("JSON bundle is missing the required property %s (%s)", + propertyName, + jsonDocument)); + } + + return propertyValue; + } } diff --git a/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java b/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java index 43a9377251..54eadf14eb 100644 --- a/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java +++ b/src/test/java/htsjdk/beta/io/bundle/BundleJSONTest.java @@ -3,6 +3,7 @@ import htsjdk.HtsjdkTest; import htsjdk.io.HtsPath; import htsjdk.io.IOPath; +import org.json.JSONObject; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -157,7 +158,7 @@ public Object[][] getRoundTripJSON() { public void testRoundTripJSON(final String jsonString, final String primaryKey, final List resources) { final Bundle bundleFromResources = new Bundle(primaryKey, resources); final String actualJSONString = BundleJSON.toJSON(bundleFromResources); - Assert.assertEquals(actualJSONString, jsonString); + Assert.assertEquals(actualJSONString, new JSONObject(jsonString).toString(1)); // now recreate the bundle from JSON final Bundle bundleFromJSON = BundleJSON.toBundle(jsonString); @@ -227,7 +228,7 @@ public Object[][] getInvalidBundleJSON() { // syntax error (missing quote in before schemaName { "{\"schemaVersion\":\"0.1.0\",schemaName\":\"htsbundle\",\"ALIGNED_READS\":{\"path\":\"myreads" + ".bam\",\"format\":\"BAM\"},\"primary\":\"ALIGNED_READS\"}", - "Invalid JSON near position: 25" }, + "Expected a ':' after a key at 36" }, // no enclosing {} -> UnsupportedOperationException (no text message) {"\"schemaName\":\"htsbundle\", \"schemaVersion\":\"0.1.0\"", "", }, From e6b92c6c8b3b00ded79145db18d168c21591af2d Mon Sep 17 00:00:00 2001 From: bbimber Date: Fri, 28 Jul 2023 09:06:56 -0700 Subject: [PATCH 6/6] Update org.json --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 3b6ff88624..72100b864f 100644 --- a/build.gradle +++ b/build.gradle @@ -33,7 +33,7 @@ dependencies { implementation "org.xerial.snappy:snappy-java:1.1.10.1" implementation "org.apache.commons:commons-compress:1.22" implementation 'org.tukaani:xz:1.8' - implementation "org.json:json:20230227" + implementation "org.json:json:20230618" implementation 'org.openjdk.nashorn:nashorn-core:15.4'