Merge branch '1351-escape-xml' into 'devel_15.0.x'

Resolve "GPML: Conversion does not escape the & symbol"

See merge request !1234

CHANGELOG

@@ -16,6 +16,8 @@ minerva (15.0.3) stable; urgency=medium
     (#1349)
   * Bug fix: exporting to GPML sometimes resulted in invalid modification point
     end for catalysis (#1350)
+  * Bug fix: elements name were not escaped properly when exporting to GPML
+    (#1351)
 
  -- Piotr Gawron <piotr.gawron@uni.lu>  Wed, 8 Jul 2020 16:00:00 +0200
 

pathvisio/src/main/java/lcsb/mapviewer/wikipathway/XML/ModelContructor.java

@@ -43,7 +43,7 @@ public class ModelContructor {
   /**
    * Default color used by complexes.
    */
-  private static final Color DEFAULT_COMPLEX_ALIAS_COLOR = new Color(248, 247, 240);
+   static final Color DEFAULT_COMPLEX_ALIAS_COLOR = new Color(248, 247, 240);
 
   /**
    * Epsilon used for double comparison.

pathvisio/src/main/java/lcsb/mapviewer/wikipathway/XML/ModelToGPML.java

@@ -11,6 +11,7 @@ import org.apache.commons.text.StringEscapeUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import lcsb.mapviewer.common.XmlParser;
 import lcsb.mapviewer.common.exception.InvalidArgumentException;
 import lcsb.mapviewer.common.geometry.ColorParser;
 import lcsb.mapviewer.common.geometry.PointTransformation;
@@ -338,7 +339,7 @@ public class ModelToGPML {
     double x = ca.getCenterX(), y = ca.getCenterY(), h = ca.getHeight(), w = ca.getWidth();
     String shape = ShapeMapping.getShape(ca.getClass()).getStringRepresentation();
 
-    comparments.append("  <Shape TextLabel=\"" + ca.getName() + "\" GraphId=\"" + ca.getElementId() + "\">\n");
+    comparments.append("  <Shape TextLabel=\"" + XmlParser.escapeXml(ca.getName()) + "\" GraphId=\"" + ca.getElementId() + "\">\n");
     comparments.append("    <Comment>" + StringEscapeUtils.escapeXml11(getAllNames(ca)) + "</Comment>\n");
     comparments.append("    <Attribute Key=\"org.pathvisio.CellularComponentProperty\" Value=\"Cell\" />\n");
     comparments.append("    <Attribute Key=\"org.pathvisio.DoubleLineProperty\" Value=\"Double\" />\n");
@@ -499,7 +500,7 @@ public class ModelToGPML {
     for (Complex ca : model.getComplexList()) {
       if (ca.getElements().size() == 0) {
         dataNodes.append(
-            "  <DataNode TextLabel=\"" + ca.getName() + "\" GraphId=\"" + ca.getElementId() + "\" Type=\"Complex\"");
+            "  <DataNode TextLabel=\"" + XmlParser.escapeXml(ca.getName()) + "\" GraphId=\"" + ca.getElementId() + "\" Type=\"Complex\"");
         if (ca.getComplex() != null) {
           dataNodes.append(" GroupRef=\"" + ca.getComplex().getElementId() + "\"");
         }
@@ -530,38 +531,6 @@ public class ModelToGPML {
     return dataNodes.toString();
   }
 
-  String speciesToLabel(Species species) throws ConverterException {
-    StringBuilder result = new StringBuilder();
-    result.append(
-        "  <Label TextLabel=\"" + species.getName() + "\" GraphId=\"" + species.getElementId() + "\"");
-    if (species.getComplex() != null) {
-      result.append(" GroupRef=\"" + species.getComplex().getElementId() + "\"");
-    }
-    result.append(">\n");
-
-    result.append("    <Comment>" + StringEscapeUtils.escapeXml11(species.getNotes().trim()) + "</Comment>\n");
-    result.append(biopaxParser.toReferenceXml(species.getMiriamData()));
-
-    Rectangle2D rec = species.getBorder();
-
-    result.append(
-        "    <Graphics CenterX=\"" + rec.getCenterX() + "\" " +
-            "CenterY=\"" + rec.getCenterY() + "\" " +
-            "Width=\"" + rec.getWidth() + "\" " +
-            "Height=\"" + rec.getHeight() + "\" " +
-            "ZOrder=\"" + species.getZ() + "\" " +
-            "FontSize=\"" + species.getFontSize().intValue() + "\" " +
-            "Valign=\"Middle\" " +
-            "Color=\"" + colorToString(species.getFontColor()) + "\" " +
-            "FillColor=\"" + colorToString(species.getFillColor()) + "\"/>\n");
-
-    if (species.getMiriamData().size() > 0) {
-      logger.warn(new LogMarker(ProjectLogEntryType.EXPORT_ISSUE, species), "Annotations cannot be exported to gpml");
-    }
-    result.append("  </Label>\n");
-    return result.toString();
-  }
-
   String speciesToShape(Species species) throws ConverterException {
     String shape = ShapeMapping.getShape(species.getClass()).getStringRepresentation();
     StringBuilder result = new StringBuilder();
@@ -605,7 +574,7 @@ public class ModelToGPML {
     String nodeType = getType(species);
 
     result.append(
-        "  <DataNode TextLabel=\"" + species.getName() + "\" "
+        "  <DataNode TextLabel=\"" + XmlParser.escapeXml(species.getName()) + "\" "
             + "GraphId=\"" + species.getElementId() + "\"");
     if (nodeType != null) {
       result.append(" Type=\"" + nodeType + "\"");
@@ -659,7 +628,7 @@ public class ModelToGPML {
         stateString = mr.getName();
       }
     }
-    result.append("TextLabel=\"" + stateString + "\" ");
+    result.append("TextLabel=\"" + XmlParser.escapeXml(stateString) + "\" ");
     result.append(">");
     double relX = ((mr.getPosition().getX() - mr.getSpecies().getX()) / mr.getSpecies().getWidth() - 0.5) * 2;
     double relY = ((mr.getPosition().getY() - mr.getSpecies().getY()) / mr.getSpecies().getHeight() - 0.5) * 2;
@@ -693,7 +662,7 @@ public class ModelToGPML {
         groups.append("  <Group"
             + " GroupId=\"" + complex.getElementId() + "\""
             + " GraphId=\"" + complex.getElementId() + "\""
-            + " TextLabel=\"" + complex.getName() + "\"");
+            + " TextLabel=\"" + XmlParser.escapeXml(complex.getName()) + "\"");
         if (complex.getComplex() != null) {
           groups.append(" GroupRef=\"" + complex.getComplex().getElementId() + "\"");
         }

pathvisio/src/test/java/lcsb/mapviewer/wikipathway/XML/ModelToGPMLTest.java

@@ -13,19 +13,17 @@ import org.apache.logging.log4j.Logger;
 import org.junit.*;
 
 import lcsb.mapviewer.common.geometry.PointTransformation;
-import lcsb.mapviewer.converter.ConverterParams;
+import lcsb.mapviewer.converter.*;
 import lcsb.mapviewer.converter.model.celldesigner.CellDesignerXmlParser;
 import lcsb.mapviewer.model.graphics.*;
-import lcsb.mapviewer.model.map.MiriamData;
-import lcsb.mapviewer.model.map.MiriamType;
+import lcsb.mapviewer.model.map.*;
 import lcsb.mapviewer.model.map.compartment.Compartment;
 import lcsb.mapviewer.model.map.compartment.SquareCompartment;
 import lcsb.mapviewer.model.map.model.*;
 import lcsb.mapviewer.model.map.reaction.*;
 import lcsb.mapviewer.model.map.reaction.type.StateTransitionReaction;
 import lcsb.mapviewer.model.map.species.*;
-import lcsb.mapviewer.model.map.species.field.ModificationState;
-import lcsb.mapviewer.model.map.species.field.Residue;
+import lcsb.mapviewer.model.map.species.field.*;
 import lcsb.mapviewer.wikipathway.WikipathwaysTestFunctions;
 
 public class ModelToGPMLTest extends WikipathwaysTestFunctions {
@@ -58,9 +56,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testAnnotations() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     GenericProtein protein = createProtein();
     protein.addMiriamData(new MiriamData(MiriamType.HGNC_SYMBOL, "SNCA"));
@@ -87,9 +83,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testHypoteticalComplex() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Complex complex = createComplex();
     complex.addSpecies(createProtein());
@@ -121,9 +115,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testComplexName() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Complex complex = createComplex();
     complex.addSpecies(createProtein());
@@ -147,9 +139,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testReactionColor() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Protein p1 = createProtein();
     Protein p2 = createProtein();
@@ -174,9 +164,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testReversibleReaction() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Protein p1 = createProtein();
     Protein p2 = createProtein();
@@ -227,9 +215,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
   @Test
   public void testProteinModification() throws Exception {
 
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Protein p1 = createProtein();
     p1.addResidue(createResidue(p1));
@@ -270,16 +256,6 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
     assertNotNull(content);
   }
 
-  @Test
-  public void testSpeciesToLabel() throws Exception {
-    ModelToGPML parser = new ModelToGPML("xyz");
-    Unknown unknown = createUnknown();
-    unknown.addMiriamData(new MiriamData(MiriamType.HGNC_SYMBOL, "SNCA"));
-    String xml = parser.speciesToLabel(unknown);
-    assertFalse("Label tag cannot contain Xref child", xml.contains("<Xref"));
-    assertEquals(1, getWarnings().size());
-  }
-
   @Test
   public void testDegraded() throws Exception {
     Model model = new ModelFullIndexed(null);
@@ -327,7 +303,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
     protein.setHeight(10);
     protein.setX(100 * speciesCounter);
     protein.setY(10 + 100 * speciesCounter * Math.random());
-    protein.setZ(15);
+    protein.setZ(16);
 
     protein.setNamePoint(protein.getCenter());
     protein.setNameHorizontalAlign(HorizontalAlign.CENTER);
@@ -344,6 +320,10 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
     complex.setX(1);
     complex.setY(1);
     complex.setZ(15);
+    complex.setNamePoint(6, 9);
+    complex.setNameVerticalAlign(VerticalAlign.BOTTOM);
+    complex.setNameHorizontalAlign(HorizontalAlign.CENTER);
+    complex.setHypothetical(true);
     return complex;
   }
 
@@ -355,14 +335,18 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
     compartment.setX(1);
     compartment.setY(1);
     compartment.setZ(15);
+    compartment.setNamePoint(new Point2D.Double(1, 1));
+    compartment.setNameVerticalAlign(VerticalAlign.TOP);
+    compartment.setNameHorizontalAlign(HorizontalAlign.LEFT);
+    compartment.setThickness(0);
+    compartment.setOuterWidth(0);
+    compartment.setInnerWidth(0);
     return compartment;
   }
 
   @Test
   public void testExportHypothetical() throws Exception {
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Protein p1 = createProtein();
     p1.setHypothetical(true);
@@ -380,9 +364,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
 
   @Test
   public void testExportUnknown() throws Exception {
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
 
     Species p1 = createUnknown();
 
@@ -398,9 +380,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
 
   @Test
   public void testMapNotes() throws Exception {
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
     model.setNotes("xyz");
 
     lcsb.mapviewer.wikipathway.GpmlParser parser = new lcsb.mapviewer.wikipathway.GpmlParser();
@@ -413,9 +393,7 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
 
   @Test
   public void testHtmlTagInNotes() throws Exception {
-    Model model = new ModelFullIndexed(null);
-    model.setWidth(1000);
-    model.setHeight(1000);
+    Model model = createModel();
     model.setNotes("a < b");
 
     Species p1 = createProtein();
@@ -430,11 +408,92 @@ public class ModelToGPMLTest extends WikipathwaysTestFunctions {
     r.setNotes("<xml>tag</xml>");
     model.addReaction(r);
 
+    Model model2 = serializeOverGpml(model);
+
+    assertEquals(0, new ModelComparator().compare(model, model2));
+  }
+
+  private Model serializeOverGpml(Model model)
+      throws InconsistentModelException, ConverterException, InvalidInputDataExecption {
     lcsb.mapviewer.wikipathway.GpmlParser parser = new lcsb.mapviewer.wikipathway.GpmlParser();
 
     String xml = parser.model2String(model);
     Model model2 = parser.createModel(new ConverterParams().inputStream(new ByteArrayInputStream(xml.getBytes())));
+    return model2;
+  }
+
+  private Model createModel() {
+    Model model = new ModelFullIndexed(null);
+    model.setWidth(1000);
+    model.setHeight(1000);
+    return model;
+  }
+
+  @Test
+  public void specialCharacterInCompartmentName() throws Exception {
+    Model model = createModel();
+    Compartment compartment = createCompartment();
+    compartment.setName("hello&by");
+    model.addElement(compartment);
+
+    Model model2 = serializeOverGpml(model);
 
     assertEquals(0, new ModelComparator().compare(model, model2));
   }
+
+  @Test
+  public void specialCharacterInComplexName() throws Exception {
+    Model model = createModel();
+    Element element = createComplex();
+    element.setName("hello&by");
+    model.addElement(element);
+
+    Model model2 = serializeOverGpml(model);
+
+    assertEquals(0, new ModelComparator().compare(model, model2));
+  }
+
+  @Test
+  public void specialCharacterInProteinName() throws Exception {
+    Model model = createModel();
+    Element element = createProtein();
+    element.setName("hello&by");
+    model.addElement(element);
+
+    Model model2 = serializeOverGpml(model);
+
+    assertEquals(0, new ModelComparator().compare(model, model2));
+  }
+
+  @Test
+  public void specialCharacterInModificationName() throws Exception {
+    Model model = createModel();
+    Protein element = createProtein();
+    BindingRegion mr = createBindingRegion(element);
+    mr.setName("hello&by");
+    element.addBindingRegion(mr);
+    model.addElement(element);
+
+    serializeOverGpml(model);
+  }
+
+  @Test
+  public void specialCharacterInNonEmptyComplexName() throws Exception {
+    Model model = createModel();
+    Complex complex = createComplex();
+    Protein protein = createProtein();
+    complex.addSpecies(protein);
+    model.addElement(complex);
+    model.addElement(protein);
+
+    serializeOverGpml(model);
+  }
+
+  private BindingRegion createBindingRegion(Protein p1) {
+    BindingRegion residue = new BindingRegion();
+    residue.setIdModificationResidue("mr" + speciesCounter++);
+    residue.setPosition(new Point2D.Double(p1.getCenterX(), p1.getY()));
+    residue.setName("m");
+    return residue;
+  }
 }