Commit 4e8fff4e authored by Piotr Gawron's avatar Piotr Gawron
Browse files

check if new identifier is not used in the model before using it

parent bdc03780
Pipeline #50791 failed with stage
in 60 minutes and 4 seconds
minerva (16.0.6) stable; urgency=medium
* Bug fix: multiple conversion between formats SBGN->SBML->SBGN->SBML could
crash if in the meantime there were additional elements added (#1617)
-- Piotr Gawron <piotr.gawron@uni.lu> Thu, 16 Dec 2021 11:00:00 +0200
minerva (16.0.5) stable; urgency=medium
* Bug fix (performance): uploading of data overlays was time consuming
(#1591)
......
package lcsb.mapviewer.converter.model.sbml;
import java.awt.Color;
import java.util.*;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import javax.xml.stream.XMLStreamException;
......@@ -9,12 +13,23 @@ import javax.xml.stream.XMLStreamException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.sbml.jsbml.Model;
import org.sbml.jsbml.ext.layout.*;
import org.sbml.jsbml.ext.layout.AbstractReferenceGlyph;
import org.sbml.jsbml.ext.layout.BoundingBox;
import org.sbml.jsbml.ext.layout.Layout;
import org.sbml.jsbml.ext.multi.MultiModelPlugin;
import org.sbml.jsbml.ext.render.*;
import org.sbml.jsbml.ext.render.ColorDefinition;
import org.sbml.jsbml.ext.render.LineEnding;
import org.sbml.jsbml.ext.render.LocalStyle;
import org.sbml.jsbml.ext.render.Polygon;
import org.sbml.jsbml.ext.render.RelAbsVector;
import org.sbml.jsbml.ext.render.RenderConstants;
import org.sbml.jsbml.ext.render.RenderGraphicalObjectPlugin;
import org.sbml.jsbml.ext.render.RenderPoint;
import org.sbml.jsbml.xml.XMLNode;
import lcsb.mapviewer.common.exception.InvalidStateException;
import lcsb.mapviewer.model.LogMarker;
import lcsb.mapviewer.model.ProjectLogEntryType;
import lcsb.mapviewer.model.map.BioEntity;
import lcsb.mapviewer.model.map.InconsistentModelException;
import lcsb.mapviewer.modelutils.map.ElementUtils;
......@@ -55,8 +70,8 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
private SbmlModelUtils sbmlModelUtils;
public SbmlBioEntityExporter(Model sbmlModel, lcsb.mapviewer.model.map.model.Model minervaModel,
Collection<SbmlExtension> sbmlExtensions) {
public SbmlBioEntityExporter(final Model sbmlModel, final lcsb.mapviewer.model.map.model.Model minervaModel,
final Collection<SbmlExtension> sbmlExtensions) {
this.sbmlModel = sbmlModel;
this.sbmlModelUtils = new SbmlModelUtils(sbmlModel);
this.layout = sbmlModelUtils.getLayout();
......@@ -108,7 +123,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
* {@link SbmlExtension} to be checked
* @return <code>true</code> if extension should be supported by exported file
*/
protected boolean isExtensionEnabled(SbmlExtension sbmlExtension) {
protected boolean isExtensionEnabled(final SbmlExtension sbmlExtension) {
return sbmlExtensions.contains(sbmlExtension);
}
......@@ -116,7 +131,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
public abstract S createSbmlElement(T element) throws InconsistentModelException;
public S getSbmlElement(T element) throws InconsistentModelException {
public S getSbmlElement(final T element) throws InconsistentModelException {
String mapKey = getSbmlIdKey(element);
if (!sbmlElementByElementNameAndCompartmentName.containsKey(mapKey)) {
S sbmlElement = createSbmlElement(element);
......@@ -143,7 +158,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
protected abstract void assignLayoutToGlyph(T element, AbstractReferenceGlyph compartmentGlyph);
protected AbstractReferenceGlyph createGlyph(T element) {
protected AbstractReferenceGlyph createGlyph(final T element) {
String sbmlElementId = sbmlElementByElementId.get(getElementId(element)).getId();
String glyphId = getElementId(element);
AbstractReferenceGlyph elementGlyph = createElementGlyph(sbmlElementId, glyphId);
......@@ -173,15 +188,15 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
return sbmlModel;
}
public S getSbmlElementByElementId(String id) {
public S getSbmlElementByElementId(final String id) {
return sbmlElementByElementId.get(id);
}
public AbstractReferenceGlyph getSbmlGlyphByElementId(String elementId) {
public AbstractReferenceGlyph getSbmlGlyphByElementId(final String elementId) {
return sbmlGlyphByElementId.get(elementId);
}
protected void assignStyleToGlyph(AbstractReferenceGlyph speciesGlyph, LocalStyle style) {
protected void assignStyleToGlyph(final AbstractReferenceGlyph speciesGlyph, final LocalStyle style) {
RenderGraphicalObjectPlugin rgop = new RenderGraphicalObjectPlugin(speciesGlyph);
if (style.getGroup().isSetEndHead()) {
LineEnding lineEnding = sbmlModelUtils.createLineEndingStyle(style.getGroup().getEndHead());
......@@ -191,7 +206,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
speciesGlyph.addExtension(RenderConstants.shortLabel, rgop);
}
protected BoundingBox createBoundingBox(double x, double y, int z, double w, double h) {
protected BoundingBox createBoundingBox(final double x, final double y, final int z, final double w, final double h) {
return sbmlModelUtils.createBoundingBox(x, y, z, w, h);
}
......@@ -201,17 +216,20 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
return provideDefaults;
}
public void setProvideDefaults(boolean provideDefaults) {
public void setProvideDefaults(final boolean provideDefaults) {
this.provideDefaults = provideDefaults;
}
public String getElementId(BioEntity bioEntity) {
public String getElementId(final 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);
String newElementId = null;
do {
newElementId = this.getClass().getSimpleName() + "_" + getNextId();
} while (minervaModel.getElementByElementId(newElementId) != null || minervaModel.getReactionByReactionId(newElementId) != null);
logger.warn(new LogMarker(ProjectLogEntryType.EXPORT_ISSUE, bioEntity),
"Element id " + bioEntity.getElementId() + " cannot be used in sbml. Changing to: " + newElementId);
correctedElementId.put(result, newElementId);
}
result = correctedElementId.get(result);
......@@ -220,7 +238,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
return result;
}
protected ColorDefinition getColorDefinition(Color color) {
protected ColorDefinition getColorDefinition(final Color color) {
return sbmlModelUtils.getColorDefinition(color);
}
......@@ -228,7 +246,7 @@ public abstract class SbmlBioEntityExporter<T extends BioEntity, S extends org.s
return sbmlModelUtils.createStyle();
}
protected void createAbsolutePoint(Polygon polygon, double x, double y) {
protected void createAbsolutePoint(final Polygon polygon, final double x, final double y) {
RenderPoint p1 = polygon.createRenderPoint();
p1.setX(new RelAbsVector(Math.round(x)));
p1.setY(new RelAbsVector(Math.round(y)));
......
package lcsb.mapviewer.converter.model.sbml;
import static org.junit.Assert.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.awt.Color;
import java.awt.geom.Point2D;
import java.io.*;
import java.io.File;
import java.lang.reflect.Modifier;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.apache.commons.lang3.mutable.MutableBoolean;
import org.apache.commons.text.StringEscapeUtils;
......@@ -15,18 +22,36 @@ import org.apache.logging.log4j.Logger;
import org.junit.Test;
import org.reflections.Reflections;
import org.sbml.jsbml.SBMLDocument;
import org.sbml.jsbml.ext.multi.*;
import org.sbml.jsbml.ext.multi.MultiModelPlugin;
import org.sbml.jsbml.ext.multi.MultiSpeciesPlugin;
import org.sbml.jsbml.ext.multi.MultiSpeciesType;
import org.sbml.jsbml.ext.multi.PossibleSpeciesFeatureValue;
import org.sbml.jsbml.ext.multi.SpeciesFeatureType;
import lcsb.mapviewer.common.Configuration;
import lcsb.mapviewer.common.comparator.ListComparator;
import lcsb.mapviewer.converter.ConverterParams;
import lcsb.mapviewer.converter.model.celldesigner.CellDesignerXmlParser;
import lcsb.mapviewer.model.graphics.*;
import lcsb.mapviewer.model.graphics.PolylineData;
import lcsb.mapviewer.model.graphics.PolylineDataComparator;
import lcsb.mapviewer.model.graphics.VerticalAlign;
import lcsb.mapviewer.model.map.compartment.Compartment;
import lcsb.mapviewer.model.map.model.*;
import lcsb.mapviewer.model.map.reaction.*;
import lcsb.mapviewer.model.map.species.*;
import lcsb.mapviewer.model.map.species.field.*;
import lcsb.mapviewer.model.map.model.Model;
import lcsb.mapviewer.model.map.model.ModelComparator;
import lcsb.mapviewer.model.map.model.ModelFullIndexed;
import lcsb.mapviewer.model.map.reaction.AbstractNode;
import lcsb.mapviewer.model.map.reaction.NodeOperator;
import lcsb.mapviewer.model.map.reaction.Product;
import lcsb.mapviewer.model.map.reaction.Reaction;
import lcsb.mapviewer.model.map.reaction.ReactionNode;
import lcsb.mapviewer.model.map.species.Element;
import lcsb.mapviewer.model.map.species.GenericProtein;
import lcsb.mapviewer.model.map.species.Protein;
import lcsb.mapviewer.model.map.species.Species;
import lcsb.mapviewer.model.map.species.field.ModificationState;
import lcsb.mapviewer.model.map.species.field.PositionToCompartment;
import lcsb.mapviewer.model.map.species.field.Residue;
import lcsb.mapviewer.model.map.species.field.StructuralState;
public class SbmlExporterTest extends SbmlTestFunctions {
Logger logger = LogManager.getLogger();
......@@ -55,7 +80,7 @@ public class SbmlExporterTest extends SbmlTestFunctions {
assertFalse(compartment.getClass().equals(Compartment.class));
}
private Model getModelAfterSerializing(String filename) throws Exception {
private Model getModelAfterSerializing(final String filename) throws Exception {
Model originalModel = parser.createModel(new ConverterParams().filename(filename));
return getModelAfterSerializing(originalModel);
}
......@@ -507,7 +532,7 @@ public class SbmlExporterTest extends SbmlTestFunctions {
speciesExtension.getListOfSpeciesFeatures().size() > 0);
}
private StructuralState createStructuralState(Element element) {
private StructuralState createStructuralState(final Element element) {
StructuralState structuralState = new StructuralState();
structuralState.setValue("xxx");
structuralState.setPosition(new Point2D.Double(element.getX(), element.getY() - 10));
......@@ -731,4 +756,18 @@ public class SbmlExporterTest extends SbmlTestFunctions {
assertFalse(exceptionHappened.booleanValue());
}
@Test
public void testExportWithProblematicIds() throws Exception {
Model model = createEmptyModel();
GenericProtein p1 = createProtein();
p1.setElementId("x-1");
model.addElement(p1);
GenericProtein p2 = createProtein();
p2.setElementId("SbmlSpeciesExporter_1");
model.addElement(p2);
exporter.toXml(model);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment