From e4eca89202963f5f7c95ac002efd64fe0e79a25c Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Thu, 25 Oct 2018 15:41:21 +0100
Subject: [PATCH] JAL-3143 comma-delimited feature attributes for alleles,
 clinical_significance

---
 src/jalview/ext/ensembl/EnsemblFeatures.java |   15 +++++-------
 src/jalview/ext/ensembl/EnsemblSeqProxy.java |    8 ++++--
 src/jalview/util/JSONUtils.java              |   34 ++++++++++++++++++++++++++
 test/jalview/util/JSONUtilsTest.java         |   26 ++++++++++++++++++++
 4 files changed, 72 insertions(+), 11 deletions(-)
 create mode 100644 src/jalview/util/JSONUtils.java
 create mode 100644 test/jalview/util/JSONUtilsTest.java

diff --git a/src/jalview/ext/ensembl/EnsemblFeatures.java b/src/jalview/ext/ensembl/EnsemblFeatures.java
index a133381..9a45dda 100644
--- a/src/jalview/ext/ensembl/EnsemblFeatures.java
+++ b/src/jalview/ext/ensembl/EnsemblFeatures.java
@@ -26,6 +26,7 @@ import jalview.datamodel.Sequence;
 import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceI;
 import jalview.io.gff.SequenceOntologyI;
+import jalview.util.JSONUtils;
 
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -129,15 +130,11 @@ class EnsemblFeatures extends EnsemblRestClient
           int end = Integer.parseInt(obj.get("end").toString());
           String source = obj.get("source").toString();
           String strand = obj.get("strand").toString();
-          Object value = obj.get("consequence_type");
-          value = obj.get("alleles");
-          JSONArray allelesArray = (JSONArray) value;
-          String alleles = allelesArray == null ? null
-                  : allelesArray.toString(); // todo need as a List?
-          value = obj.get("clinical_significance");
-          JSONArray clinSigArray = (JSONArray) value;
-          String clinSig = clinSigArray == null ? null
-                  : clinSigArray.toString();
+          String alleles = JSONUtils
+                  .arrayToList((JSONArray) obj.get("alleles"));
+          String clinSig = JSONUtils
+                  .arrayToList(
+                          (JSONArray) obj.get("clinical_significance"));
 
           /*
            * convert 'variation' to 'sequence_variant', and 'cds' to 'CDS'
diff --git a/src/jalview/ext/ensembl/EnsemblSeqProxy.java b/src/jalview/ext/ensembl/EnsemblSeqProxy.java
index 7a37c8a..5dc701d 100644
--- a/src/jalview/ext/ensembl/EnsemblSeqProxy.java
+++ b/src/jalview/ext/ensembl/EnsemblSeqProxy.java
@@ -915,10 +915,14 @@ public abstract class EnsemblSeqProxy extends EnsemblRestClient
 
   /**
    * Answers true if the feature type is either 'NMD_transcript_variant' or
-   * 'transcript' or one of its sub-types in the Sequence Ontology. This is
-   * needed because NMD_transcript_variant behaves like 'transcript' in Ensembl
+   * 'transcript' (or one of its sub-types in the Sequence Ontology). This is
+   * because NMD_transcript_variant behaves like 'transcript' in Ensembl
    * although strictly speaking it is not (it is a sub-type of
    * sequence_variant).
+   * <p>
+   * (This test was needed when fetching transcript features as GFF. As we are
+   * now fetching as JSON, all features have type 'transcript' so the check for
+   * NMD_transcript_variant is redundant. Left in for any future case arising.)
    * 
    * @param featureType
    * @return
diff --git a/src/jalview/util/JSONUtils.java b/src/jalview/util/JSONUtils.java
new file mode 100644
index 0000000..cdfc88e
--- /dev/null
+++ b/src/jalview/util/JSONUtils.java
@@ -0,0 +1,34 @@
+package jalview.util;
+
+import org.json.simple.JSONArray;
+
+public class JSONUtils
+{
+
+  /**
+   * Converts a JSONArray of values to a string as a comma-separated list.
+   * Answers null if the array is null or empty.
+   * 
+   * @param jsonArray
+   * @return
+   */
+  public static String arrayToList(JSONArray jsonArray)
+  {
+    if (jsonArray == null)
+    {
+      return null;
+    }
+
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < jsonArray.size(); i++)
+    {
+      if (i > 0)
+      {
+        sb.append(",");
+      }
+      sb.append(jsonArray.get(i).toString());
+    }
+    return sb.length() == 0 ? null : sb.toString();
+  }
+
+}
diff --git a/test/jalview/util/JSONUtilsTest.java b/test/jalview/util/JSONUtilsTest.java
new file mode 100644
index 0000000..45f1c48
--- /dev/null
+++ b/test/jalview/util/JSONUtilsTest.java
@@ -0,0 +1,26 @@
+package jalview.util;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
+
+import org.json.JSONException;
+import org.json.simple.JSONArray;
+import org.testng.annotations.Test;
+
+public class JSONUtilsTest
+{
+  @Test(groups = "Functional")
+  public void testArrayToList() throws JSONException
+  {
+    assertNull(JSONUtils.arrayToList(null));
+
+    JSONArray ja = new JSONArray();
+    assertNull(JSONUtils.arrayToList(null));
+
+    ja.add("hello");
+    assertEquals(JSONUtils.arrayToList(ja), "hello");
+
+    ja.add("world");
+    assertEquals(JSONUtils.arrayToList(ja), "hello,world");
+  }
+}
-- 
1.7.10.2