diff --git a/CHANGELOG b/CHANGELOG
index fc6e768e723b27707a15e1c32a84ac272b3052e6..64fd46a81741edf0f676cea7c34fc6ea5d89d176 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,9 @@
+minerva (14.0.3) stable; urgency=medium
+ * Bug fix: some SBGN files uploaded to minerva could not be exported to SBML
+ due to problems with identifiers used by SBGN (#1006)
+
+ -- Piotr Gawron <piotr.gawron@uni.lu> Wed, 06 Nov 2019 12:00:00 +0200
+
minerva (14.0.3) stable; urgency=medium
* Bug fix: default zoom level on main map works even when x or y are
undefined (#993)
diff --git a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlBioEntityExporter.java b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlBioEntityExporter.java
index fc092330c465da21d2fbccb554867c0cbb6d2ed0..262fcf0afd949791200a1ba54aadf8ed74b2b0ef 100644
--- a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlBioEntityExporter.java
+++ b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlBioEntityExporter.java
@@ -1,7 +1,8 @@
package lcsb.mapviewer.converter.model.sbml;
-import java.awt.*;
+import java.awt.Color;
import java.util.*;
+import java.util.regex.Pattern;
import javax.xml.stream.XMLStreamException;
@@ -10,10 +11,8 @@ import org.apache.logging.log4j.Logger;
import org.sbml.jsbml.Model;
import org.sbml.jsbml.ext.SBasePlugin;
import org.sbml.jsbml.ext.layout.*;
-import org.sbml.jsbml.ext.layout.Point;
import org.sbml.jsbml.ext.multi.MultiModelPlugin;
import org.sbml.jsbml.ext.render.*;
-import org.sbml.jsbml.ext.render.Polygon;
import lcsb.mapviewer.common.XmlParser;
import lcsb.mapviewer.common.exception.InvalidStateException;
@@ -49,12 +48,26 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
private boolean provideDefaults;
+ private Pattern sbmlValidIdMatcher;
+
+ /**
+ * Map with elementId mapping for identifiers that cannot be used in sbml.
+ */
+ private Map<String, String> correctedElementId = new HashMap<>();
+
public SbmlBioEntityExporter(Model sbmlModel, lcsb.mapviewer.model.map.model.Model minervaModel,
Collection<SbmlExtension> sbmlExtensions) {
this.sbmlModel = sbmlModel;
this.layout = getLayout(sbmlModel);
this.minervaModel = minervaModel;
this.sbmlExtensions.addAll(sbmlExtensions);
+
+ String underscore = "_";
+ String letter = "a-zA-Z";
+ String digit = "0-9";
+ String idChar = '[' + letter + digit + underscore + ']';
+ sbmlValidIdMatcher = Pattern.compile("^[" + letter + underscore + "]" + idChar + '*');
+
}
public void exportElements() throws InconsistentModelException {
@@ -63,10 +76,10 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
try {
S sbmlElement = getSbmlElement(bioEntity);
- if (sbmlElementByElementId.get(bioEntity.getElementId()) != null) {
- throw new InconsistentModelException("More than one species with id: " + bioEntity.getElementId());
+ if (sbmlElementByElementId.get(getElementId(bioEntity)) != null) {
+ throw new InconsistentModelException("More than one species with id: " + getElementId(bioEntity));
}
- sbmlElementByElementId.put(bioEntity.getElementId(), sbmlElement);
+ sbmlElementByElementId.put(getElementId(bioEntity), sbmlElement);
} catch (Exception e) {
throw new InconsistentModelException(new ElementUtils().getElementTag(bioEntity) +
"Problem with exporting bioEntity", e);
@@ -76,7 +89,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
for (T bioEntity : speciesList) {
try {
AbstractReferenceGlyph elementGlyph = createGlyph(bioEntity);
- sbmlGlyphByElementId.put(bioEntity.getElementId(), elementGlyph);
+ sbmlGlyphByElementId.put(getElementId(bioEntity), elementGlyph);
} catch (Exception e) {
throw new InconsistentModelException(new ElementUtils().getElementTag(bioEntity) +
"Problem with exporting bioEntity", e);
@@ -127,8 +140,8 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
protected abstract void assignLayoutToGlyph(T element, AbstractReferenceGlyph compartmentGlyph);
protected AbstractReferenceGlyph createGlyph(T element) {
- String sbmlElementId = sbmlElementByElementId.get(element.getElementId()).getId();
- String glyphId = element.getElementId();
+ String sbmlElementId = sbmlElementByElementId.get(getElementId(element)).getId();
+ String glyphId = getElementId(element);
AbstractReferenceGlyph elementGlyph = createElementGlyph(sbmlElementId, glyphId);
assignLayoutToGlyph(element, elementGlyph);
return elementGlyph;
@@ -403,4 +416,19 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
this.provideDefaults = provideDefaults;
}
+ public String getElementId(BioEntity bioEntity) {
+ String result = bioEntity.getElementId();
+ if (!sbmlValidIdMatcher.matcher(result).matches()) {
+ if (correctedElementId.get(result) == null) {
+ String newElementId = this.getClass().getSimpleName() + "_" + getNextId();
+ logger.warn(new ElementUtils().getElementTag(bioEntity) + " Element id cannot be used in sbml. Changing to: "
+ + newElementId);
+ correctedElementId.put(result, newElementId);
+ }
+ result = correctedElementId.get(result);
+ }
+
+ return result;
+ }
+
}
diff --git a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlExporter.java b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlExporter.java
index c86d28e26e932207c290bbd8d111af44ffdd79a5..784fefebd28dce33bfc08452315acb0f5e7317d7 100644
--- a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlExporter.java
+++ b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/SbmlExporter.java
@@ -73,7 +73,12 @@ public class SbmlExporter {
*/
protected SBMLDocument toSbmlDocument(lcsb.mapviewer.model.map.model.Model model) throws InconsistentModelException {
SBMLDocument doc = new SBMLDocument(3, 2);
- Model result = doc.createModel(model.getIdModel());
+ Model result = doc.createModel();
+ try {
+ result.setId(model.getIdModel());
+ } catch (IllegalArgumentException e) {
+ logger.warn("Invalid model identifier: \"" + model.getIdModel() + "\". Ignoring.");
+ }
result.setName(model.getName());
try {
result.setNotes(NotesUtility.prepareEscapedXmlNotes(model.getNotes()));
diff --git a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/compartment/SbmlCompartmentExporter.java b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/compartment/SbmlCompartmentExporter.java
index d4d47b95c9de55ae5253051d4b1a90dfc657bcfb..f3a7ac1cd850507da72d95a28ec41e34adc049ea 100644
--- a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/compartment/SbmlCompartmentExporter.java
+++ b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/compartment/SbmlCompartmentExporter.java
@@ -34,7 +34,7 @@ public class SbmlCompartmentExporter extends SbmlElementExporter<Compartment, or
result.addAll(getMinervaModel().getCompartments());
boolean defaultFound = false;
for (Compartment compartment : result) {
- if (compartment.getElementId().equals("default")) {
+ if (getElementId(compartment).equals("default")) {
defaultFound = true;
}
}
@@ -48,7 +48,7 @@ public class SbmlCompartmentExporter extends SbmlElementExporter<Compartment, or
@Override
public org.sbml.jsbml.Compartment createSbmlElement(Compartment element) throws InconsistentModelException {
org.sbml.jsbml.Compartment result;
- if (element == null || element.getElementId().equals("default")) {
+ if (element == null || getElementId(element).equals("default")) {
result = getSbmlModel().createCompartment("default");
} else {
result = getSbmlModel().createCompartment("comp_" + (getNextId()));
@@ -77,7 +77,7 @@ public class SbmlCompartmentExporter extends SbmlElementExporter<Compartment, or
@Override
protected String getSbmlIdKey(Compartment compartment) {
- if (compartment == null || compartment.getElementId().equals("default")) {
+ if (compartment == null || getElementId(compartment).equals("default")) {
return "default";
}
return compartment.getName();
@@ -131,7 +131,7 @@ public class SbmlCompartmentExporter extends SbmlElementExporter<Compartment, or
@Override
protected void assignLayoutToGlyph(Compartment element, AbstractReferenceGlyph speciesGlyph) {
- if (element.getElementId().equals("default")) {
+ if (getElementId(element).equals("default")) {
BoundingBox boundingBox = new BoundingBox();
boundingBox.setPosition(new Point(element.getX(), element.getY(), 0));
@@ -145,7 +145,7 @@ public class SbmlCompartmentExporter extends SbmlElementExporter<Compartment, or
speciesGlyph.setBoundingBox(boundingBox);
} else {
super.assignLayoutToGlyph(element, speciesGlyph);
- TextGlyph textGlyph = getLayout().createTextGlyph("text_" + element.getElementId(), element.getName());
+ TextGlyph textGlyph = getLayout().createTextGlyph("text_" + getElementId(element), element.getName());
Point2D point = element.getNamePoint();
double width = element.getWidth() - (point.getX() - element.getX());
double height = element.getHeight() - (point.getY() - element.getY());
diff --git a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/reaction/SbmlReactionExporter.java b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/reaction/SbmlReactionExporter.java
index d4184cb7b4e211f4b6370c32635b14e4976af7a3..ff142e888749871471836450813bd26f3f49a198 100644
--- a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/reaction/SbmlReactionExporter.java
+++ b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/reaction/SbmlReactionExporter.java
@@ -98,11 +98,12 @@ public class SbmlReactionExporter extends SbmlBioEntityExporter<Reaction, org.sb
}
private String getReactionId(Reaction reaction) {
- int separatorIndex = reaction.getElementId().indexOf("__");
+ String elementId = getElementId(reaction);
+ int separatorIndex = elementId.indexOf("__");
if (separatorIndex > 0) {
- return reaction.getElementId().substring(0, separatorIndex);
+ return elementId.substring(0, separatorIndex);
}
- return reaction.getElementId();
+ return elementId;
}
@Override
@@ -124,19 +125,22 @@ public class SbmlReactionExporter extends SbmlBioEntityExporter<Reaction, org.sb
result.setSBOTerm(sboTerm);
}
for (Product product : reaction.getProducts()) {
- Species sbmlSymbol = speciesExporter.getSbmlElementByElementId(product.getElement().getElementId());
+ Species sbmlSymbol = speciesExporter
+ .getSbmlElementByElementId(speciesExporter.getElementId(product.getElement()));
SpeciesReference speciesReference = result.createProduct(sbmlSymbol);
speciesReference.setConstant(false);
speciesReferenceByReactionNode.put(product, speciesReference);
}
for (Reactant reactant : reaction.getReactants()) {
- Species sbmlSymbol = speciesExporter.getSbmlElementByElementId(reactant.getElement().getElementId());
+ Species sbmlSymbol = speciesExporter
+ .getSbmlElementByElementId(speciesExporter.getElementId(reactant.getElement()));
SpeciesReference speciesReference = result.createReactant(sbmlSymbol);
speciesReference.setConstant(false);
speciesReferenceByReactionNode.put(reactant, speciesReference);
}
for (Modifier modifier : reaction.getModifiers()) {
- Species sbmlSymbol = speciesExporter.getSbmlElementByElementId(modifier.getElement().getElementId());
+ Species sbmlSymbol = speciesExporter
+ .getSbmlElementByElementId(speciesExporter.getElementId(modifier.getElement()));
SimpleSpeciesReference speciesReference = result.createModifier(sbmlSymbol);
speciesReferenceByReactionNode.put(modifier, speciesReference);
String term = SBOTermModifierType.getTermByType(modifier.getClass());
@@ -151,8 +155,8 @@ public class SbmlReactionExporter extends SbmlBioEntityExporter<Reaction, org.sb
}
@Override
- protected String getSbmlIdKey(Reaction element) {
- return element.getClass().getSimpleName() + "\n" + element.getElementId();
+ protected String getSbmlIdKey(Reaction reaction) {
+ return reaction.getClass().getSimpleName() + "\n" + getElementId(reaction);
}
@Override
@@ -367,7 +371,8 @@ public class SbmlReactionExporter extends SbmlBioEntityExporter<Reaction, org.sb
private SpeciesReferenceGlyph createNodeGlyph(ReactionGlyph reactionGlyph, ReactionNode node) {
SpeciesReferenceGlyph reactantGlyph = reactionGlyph.createSpeciesReferenceGlyph("node_" + getNextId());
- reactantGlyph.setSpeciesGlyph(speciesExporter.getSbmlGlyphByElementId(node.getElement().getElementId()).getId());
+ reactantGlyph.setSpeciesGlyph(
+ speciesExporter.getSbmlGlyphByElementId(speciesExporter.getElementId(node.getElement())).getId());
Curve curve = createCurve(node.getLine(), node instanceof Reactant);
if (curve.getCurveSegmentCount() == 0) {
logger.warn(new ElementUtils().getElementTag(node) + " Problematic line");
diff --git a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/species/SbmlSpeciesExporter.java b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/species/SbmlSpeciesExporter.java
index a7cf1fdfd142f2eb56c6266bbb900b91d4b2b71d..deb5139c335c66672d9d3f2b927692e7ea96cfec 100644
--- a/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/species/SbmlSpeciesExporter.java
+++ b/converter-sbml/src/main/java/lcsb/mapviewer/converter/model/sbml/species/SbmlSpeciesExporter.java
@@ -466,7 +466,7 @@ public class SbmlSpeciesExporter extends SbmlElementExporter<Species, org.sbml.j
}
}
}
- TextGlyph textGlyph = getLayout().createTextGlyph("text_" + element.getElementId(), element.getName());
+ TextGlyph textGlyph = getLayout().createTextGlyph("text_" + getElementId(element), element.getName());
textGlyph
.setBoundingBox(createBoundingBox(element.getX(), element.getY(), element.getZ() + 1, element.getWidth(),
element.getHeight()));
diff --git a/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlExporterTest.java b/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlExporterTest.java
index 145e36e21ba025019f5d521ee7215783aa88f87d..bd052950b39ce7015886d0a9e4b5b243ea133c2b 100644
--- a/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlExporterTest.java
+++ b/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlExporterTest.java
@@ -3,7 +3,6 @@ package lcsb.mapviewer.converter.model.sbml;
import static org.junit.Assert.*;
import java.awt.Color;
-import java.awt.geom.Line2D;
import java.awt.geom.Point2D;
import java.io.File;
import java.lang.reflect.Modifier;
@@ -17,7 +16,6 @@ import org.sbml.jsbml.SBMLDocument;
import org.sbml.jsbml.ext.multi.*;
import lcsb.mapviewer.common.Configuration;
-import lcsb.mapviewer.common.comparator.LineComparator;
import lcsb.mapviewer.common.comparator.ListComparator;
import lcsb.mapviewer.converter.ConverterParams;
import lcsb.mapviewer.converter.model.celldesigner.CellDesignerXmlParser;
@@ -605,4 +603,80 @@ public class SbmlExporterTest extends SbmlTestFunctions {
assertEquals(0, comparator.compare(model, originalModel));
}
+ @Test
+ public void testInvalidSpeciesId() throws Exception {
+ Model model = createEmptyModel();
+
+ GenericProtein p = createProtein();
+ p.setElementId("a-b");
+ model.addElement(p);
+
+ String xml = exporter.toXml(model);
+ assertNotNull(xml);
+
+ assertEquals(1, getWarnings().size());
+ }
+
+ @Test
+ public void testInvalidReactionId() throws Exception {
+ Model model = createEmptyModel();
+
+ Protein p1 = createProtein();
+ Protein p2 = createProtein();
+ Reaction p = createReaction(p1, p2);
+ p.setIdReaction("a-b");
+ model.addElement(p1);
+ model.addElement(p2);
+ model.addReaction(p);
+
+ String xml = exporter.toXml(model);
+ assertNotNull(xml);
+ assertEquals(1, getWarnings().size());
+
+ }
+
+ @Test
+ public void testInvalidSpeciesIdInReaction() throws Exception {
+ Model model = createEmptyModel();
+
+ Protein p1 = createProtein();
+ Protein p2 = createProtein();
+ Reaction p = createReaction(p1, p2);
+ p1.setElementId("a-b");
+ model.addElement(p1);
+ model.addElement(p2);
+ model.addReaction(p);
+
+ String xml = exporter.toXml(model);
+ assertNotNull(xml);
+ assertEquals(1, getWarnings().size());
+
+ }
+
+ @Test
+ public void testInvalidCompartmentId() throws Exception {
+ Model model = createEmptyModel();
+
+ Compartment p = createCompartment();
+ p.setElementId("a-b");
+ model.addElement(p);
+
+ String xml = exporter.toXml(model);
+ assertNotNull(xml);
+
+ assertEquals(1, getWarnings().size());
+ }
+
+ @Test
+ public void testInvalidModelId() throws Exception {
+ Model model = createEmptyModel();
+ model.setIdModel("a-b");
+
+ String xml = exporter.toXml(model);
+ assertNotNull(xml);
+ assertEquals(1, getWarnings().size());
+
+ }
+
+
}
diff --git a/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlTestFunctions.java b/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlTestFunctions.java
index 1be181333c72c3a208210ed372dedfd6e3843a24..c62b98cf280ea802be7e868d8273b1c0f0a9cb03 100644
--- a/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlTestFunctions.java
+++ b/converter-sbml/src/test/java/lcsb/mapviewer/converter/model/sbml/SbmlTestFunctions.java
@@ -18,7 +18,8 @@ import org.apache.http.entity.mime.MultipartEntityBuilder;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.util.EntityUtils;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.junit.*;
import org.w3c.dom.Document;
@@ -39,14 +40,14 @@ import lcsb.mapviewer.model.map.species.Element;
import lcsb.mapviewer.model.map.species.GenericProtein;
public class SbmlTestFunctions {
+ private Logger logger = LogManager.getLogger();
private static int identifierCounter = 0;
+
@Rule
public UnitTestFailedWatcher unitTestFailedWatcher = new UnitTestFailedWatcher();
SbmlParser parser = new SbmlParser();
SbmlExporter exporter;
- @SuppressWarnings("unused")
- private Logger logger = Logger.getLogger(SbmlTestFunctions.class);
private MinervaLoggerAppender appender;
public SbmlTestFunctions() {
diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/convert/ConvertRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/convert/ConvertRestImplTest.java
index 0c69b7ff2f481c275206d3603374c22d233d1f55..5154d715356ebd0f9cb9e50f4615e7c63c8b78a9 100644
--- a/rest-api/src/test/java/lcsb/mapviewer/api/convert/ConvertRestImplTest.java
+++ b/rest-api/src/test/java/lcsb/mapviewer/api/convert/ConvertRestImplTest.java
@@ -211,4 +211,10 @@ public class ConvertRestImplTest extends RestTestFunctions {
test2All(content, "SBGN-ML");
}
+ @Test
+ public void testNewtSbgn2All() throws Exception {
+ String content = readFile("testFiles/convert/newt.sbgn");
+ test2All(content, "SBGN-ML");
+ }
+
}
diff --git a/rest-api/testFiles/convert/newt.sbgn b/rest-api/testFiles/convert/newt.sbgn
new file mode 100644
index 0000000000000000000000000000000000000000..459ff42ac25c4ff91fd884d547f56c0629dff216
--- /dev/null
+++ b/rest-api/testFiles/convert/newt.sbgn
@@ -0,0 +1,9 @@
+<?xml version='1.0' encoding='UTF-8' standalone='yes'?>
+<sbgn xmlns="http://sbgn.org/libsbgn/0.2">
+ <map language="process description">
+ <glyph id="nwtN_f034af08-f2b3-4912-aaa8-58b4f55f43b9" class="macromolecule">
+ <label text="dffgdf"/>
+ <bbox x="707" y="292" w="60" h="30"/>
+ </glyph>
+ </map>
+</sbgn>
\ No newline at end of file