Avoid clashes in ModelBuilder by reusing existing model elements

core/src/org/sbml/jsbml/util/ModelBuilder.java

@@ -36,6 +36,7 @@
 import org.sbml.jsbml.Unit.Kind;
 import org.sbml.jsbml.UnitDefinition;
 import org.sbml.jsbml.text.parser.ParseException;
+import org.sbml.jsbml.SBase;
 
 /**
  * This class provides a collection of convenient methods to create SBML models
@@ -250,16 +251,58 @@ public void buildCBMunits() {
    * @param sizeUnits
    * @return
    */
-  public Compartment buildCompartment(String id, boolean constant, String name, double spatialDimensions, double size, String sizeUnits) {
+  /**
+   * Creates or returns a {@link Compartment} with the given identifier.
+   * <p>
+   * If a compartment with the given {@code id} already exists in the underlying
+   * {@link Model}, this method returns that compartment unchanged and does not
+   * apply the other arguments. If no such compartment exists, a new one is created
+   * and the provided arguments are used to initialise its attributes.
+   * <p>
+   * If an element with the same {@code id} exists in the model but is not a
+   * {@link Compartment}, an {@link IllegalArgumentException} is thrown.
+   *
+   * @param id
+   *        the id of the compartment to create or return
+   * @param constant
+   *        value for {@code constant} if a new compartment is created
+   * @param name
+   *        value for {@code name} if a new compartment is created
+   * @param spatialDimensions
+   *        value for {@code spatialDimensions} if a new compartment is created
+   * @param size
+   *        value for {@code size} if a new compartment is created
+   * @param sizeUnits
+   *        units for {@code size} if a new compartment is created
+   * @return the existing or newly created {@link Compartment}
+   */
+  public Compartment buildCompartment(String id, boolean constant, String name,
+      double spatialDimensions, double size, String sizeUnits) {
     Model model = getModel();
-    Compartment c = model.createCompartment(id);
+    Compartment c = model.getCompartment(id);
+
+    if (c != null) {
+      return c;
+    }
+
+    if (id != null) {
+      SBase existing = model.findNamedSBase(id);
+      if (existing != null && !(existing instanceof Compartment)) {
+        throw new IllegalArgumentException(
+            "Element with id '" + id + "' already exists and is of type "
+                + existing.getElementName() + ", not a compartment.");
+      }
+    }
+
+    c = model.createCompartment(id);
     c.setConstant(constant);
     c.setName(name);
     c.setSpatialDimensions(spatialDimensions);
     c.setSize(size);
     if (sizeUnits != null) {
       c.setUnits(sizeUnits);
     }
+
     return c;
   }
 
@@ -292,14 +335,32 @@ public Compartment buildCompartment(String id, boolean constant, String name, do
   }
 
   /**
+   * Creates or returns the {@link Model} associated with this {@link SBMLDocument}.
+   * <p>
+   * If the document already contains a model, this method returns that model unchanged
+   * and does not alter its identifier. Otherwise, a new model is created with the
+   * given {@code id}. If {@code name} is non-null, it is applied as the model's
+   * name (only when the model is created or does not yet have a name).
    *
    * @param id
+   *        the id to use if a new model is created
    * @param name
-   * @return
+   *        an optional name to assign
+   * @return the existing or newly created {@link Model}
    */
   public Model buildModel(String id, String name) {
-    Model model = doc.createModel(id);
-    model.setName(name);
+    Model model;
+
+    if (doc.isSetModel()) {
+      model = doc.getModel();
+    } else {
+      model = doc.createModel(id);
+    }
+
+    if (name != null && !model.isSetName()) {
+      model.setName(name);
+    }
+
     return model;
   }
 
@@ -312,12 +373,41 @@ public Model buildModel(String id, String name) {
    * @param units
    * @return
    */
-  public Parameter buildParameter(String id, String name, double value, boolean constant, String units) {
-    Parameter p = getModel().createParameter(id);
+  /**
+   * Creates or returns a {@link Parameter} with the given identifier.
+   * <p>
+   * If a parameter with the given {@code id} already exists in the underlying
+   * {@link Model}, this method returns that parameter unchanged and does not
+   * apply the other arguments. If no such parameter exists, a new one is
+   * created and the provided arguments are used to initialise its attributes.
+   * <p>
+   * If an element with the same {@code id} exists in the model but is not a
+   * {@link Parameter}, an {@link IllegalArgumentException} is thrown.
+   */
+  public Parameter buildParameter(String id, String name, double value,
+      boolean constant, String units) {
+    Model model = getModel();
+    Parameter p = model.getParameter(id);
+
+    if (p != null) {
+      return p;
+    }
+
+    if (id != null) {
+      SBase existing = model.findNamedSBase(id);
+      if (existing != null && !(existing instanceof Parameter)) {
+        throw new IllegalArgumentException(
+            "Element with id '" + id + "' already exists and is of type "
+                + existing.getElementName() + ", not a parameter.");
+      }
+    }
+
+    p = model.createParameter(id);
     p.setName(name);
     p.setValue(value);
     p.setConstant(constant);
     p.setUnits(units);
+
     return p;
   }
 
@@ -369,15 +459,43 @@ public Reaction buildReaction(String id, String name, Compartment compartment, b
    * @param reversible
    * @return
    */
-  public Reaction buildReaction(String id, String name, String compartment, boolean fast, boolean reversible) {
+  /**
+   * Creates or returns a {@link Reaction} with the given identifier.
+   * <p>
+   * If a reaction with the given {@code id} already exists in the underlying
+   * {@link Model}, this method returns that reaction unchanged and does not
+   * apply the other arguments. If no such reaction exists, a new one is created
+   * and the provided arguments are used to initialise its attributes.
+   * <p>
+   * If an element with the same {@code id} exists in the model but is not a
+   * {@link Reaction}, an {@link IllegalArgumentException} is thrown.
+   */
+  public Reaction buildReaction(String id, String name, String compartment,
+      boolean fast, boolean reversible) {
     Model model = getModel();
-    Reaction r = model.createReaction(id);
+    Reaction r = model.getReaction(id);
+
+    if (r != null) {
+      return r;
+    }
+
+    if (id != null) {
+      SBase existing = model.findNamedSBase(id);
+      if (existing != null && !(existing instanceof Reaction)) {
+        throw new IllegalArgumentException(
+            "Element with id '" + id + "' already exists and is of type "
+                + existing.getElementName() + ", not a reaction.");
+      }
+    }
+
+    r = model.createReaction(id);
     r.setName(name);
     if (compartment != null) {
       r.setCompartment(compartment);
     }
     r.setFast(fast);
     r.setReversible(reversible);
+
     return r;
   }
 
@@ -450,19 +568,46 @@ public Species buildSpecies(String id, String name,
    * @param substanceUnits
    * @return
    */
+  /**
+   * Creates or returns a {@link Species} with the given identifier.
+   * <p>
+   * If a species with the given {@code id} already exists in the underlying
+   * {@link Model}, this method returns that species unchanged and does not
+   * apply the other arguments. If no such species exists, a new one is created
+   * and the provided arguments are used to initialise its attributes.
+   * <p>
+   * If an element with the same {@code id} exists in the model but is not a
+   * {@link Species}, an {@link IllegalArgumentException} is thrown.
+   */
   public Species buildSpecies(String id, String name,
-    String compartmentId, boolean hasOnlySubstanceUnits,
-    boolean boundaryCondition, boolean constant, double initialConcentration,
-    String substanceUnits) {
+      String compartmentId, boolean hasOnlySubstanceUnits,
+      boolean boundaryCondition, boolean constant, double initialConcentration,
+      String substanceUnits) {
     Model model = getModel();
-    Species s = model.createSpecies(id);
+    Species s = model.getSpecies(id);
+
+    if (s != null) {
+      return s;
+    }
+
+    if (id != null) {
+      SBase existing = model.findNamedSBase(id);
+      if (existing != null && !(existing instanceof Species)) {
+        throw new IllegalArgumentException(
+            "Element with id '" + id + "' already exists and is of type "
+                + existing.getElementName() + ", not a species.");
+      }
+    }
+
+    s = model.createSpecies(id);
     s.setName(name);
     s.setCompartment(compartmentId);
     s.setHasOnlySubstanceUnits(hasOnlySubstanceUnits);
     s.setBoundaryCondition(boundaryCondition);
     s.setConstant(constant);
     s.setInitialConcentration(initialConcentration);
     s.setSubstanceUnits(substanceUnits);
+
     return s;
   }
 
@@ -481,15 +626,40 @@ public Unit buildUnit(double multiplier, int scale, Kind kind, double exponent)
    * @param name
    * @return
    */
+  /**
+   * Creates or returns a {@link UnitDefinition} with the given identifier.
+   * <p>
+   * If a unit definition with the given {@code id} already exists in the
+   * underlying {@link Model}, this method returns that unit definition unchanged
+   * and does not apply the other arguments. If no such unit definition exists,
+   * a new one is created and the provided arguments are used to initialise its
+   * attributes.
+   * <p>
+   * Note that {@link UnitDefinition} objects live in their own namespace and
+   * may have the same id as SId-based elements without causing clashes.
+   *
+   * @param id
+   *        the id of the unit definition
+   * @param name
+   *        the optional name to assign
+   * @param units
+   *        optional units to add if a new definition is created
+   * @return the existing or newly created {@link UnitDefinition}
+   */
   public UnitDefinition buildUnitDefinition(String id, String name, Unit... units) {
     Model model = getModel();
-    UnitDefinition ud = model.createUnitDefinition(id);
-    ud.setName(name);
-    if (units != null) {
-      for (Unit unit : units) {
-        ud.addUnit(unit);
+    UnitDefinition ud = model.getUnitDefinition(id);
+
+    if (ud == null) {
+      ud = model.createUnitDefinition(id);
+      ud.setName(name);
+      if (units != null) {
+        for (Unit unit : units) {
+          ud.addUnit(unit);
+        }
       }
     }
+
     return ud;
   }
 

core/test/org/sbml/jsbml/util/ModelBuilderReuseTest.java

@@ -0,0 +1,91 @@
+/*
+ * ----------------------------------------------------------------------------
+ * This file is part of JSBML. Please visit <http://sbml.org/Software/JSBML>
+ * for the latest version of JSBML and more information about SBML.
+ *
+ * Copyright (C) 2009-2022 jointly by the following organizations:
+ * 1. The University of Tuebingen, Germany
+ * 2. EMBL European Bioinformatics Institute (EBML-EBI), Hinxton, UK
+ * 3. The California Institute of Technology, Pasadena, CA, USA
+ * 4. The University of California, San Diego, La Jolla, CA, USA
+ * 5. The Babraham Institute, Cambridge, UK
+ * 
+ * This library is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation. A copy of the license agreement is provided
+ * in the file named "LICENSE.txt" included with this software distribution
+ * and also available online as <http://sbml.org/Software/JSBML/License>.
+ * ----------------------------------------------------------------------------
+ */
+
+package org.sbml.jsbml.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+import org.junit.Test;
+import org.sbml.jsbml.Compartment;
+import org.sbml.jsbml.Model;
+import org.sbml.jsbml.SBMLDocument;
+import org.sbml.jsbml.Species;
+
+/**
+ * Tests for the reuse and clash-detection behaviour in {@link ModelBuilder}.
+ */
+public class ModelBuilderReuseTest {
+
+  /**
+   * If a compartment with the given id already exists, buildCompartment should
+   * return it unchanged and not overwrite its attributes.
+   */
+  @Test
+  public void reusesExistingCompartmentWithoutOverwritingAttributes() {
+    SBMLDocument doc = new SBMLDocument(3, 1);
+    Model model = doc.createModel("m");
+
+    Compartment original = model.createCompartment("c1");
+    original.setName("original");
+    original.setSpatialDimensions(3d);
+    original.setSize(1.23);
+    original.setUnits("litre");
+    original.setConstant(false);
+
+    ModelBuilder builder = new ModelBuilder(doc);
+
+    // Call with different arguments – these should NOT be applied,
+    // because the compartment already exists.
+    Compartment result = builder.buildCompartment("c1", true, "newName",
+        1d, 9.99, "otherUnits");
+
+    assertSame(original, result);
+    assertEquals("original", result.getName());
+    assertEquals(3d, result.getSpatialDimensions(), 0.0);
+    assertEquals(1.23, result.getSize(), 0.0);
+    assertEquals("litre", result.getUnits());
+    // remains false, not overwritten by the 'true' passed in
+    assertEquals(false, result.getConstant());
+  }
+
+  /**
+   * If an element with the same id exists but is of a different type,
+   * ModelBuilder should throw an IllegalArgumentException instead of
+   * silently creating a new element.
+   */
+  @Test(expected = IllegalArgumentException.class)
+  public void throwsIfIdIsUsedByDifferentType() {
+    SBMLDocument doc = new SBMLDocument(3, 1);
+    Model model = doc.createModel("m");
+
+    // Create a species with id 'shared'
+    Species species = model.createSpecies("shared");
+    species.setCompartment("c");
+    species.setInitialConcentration(1.0);
+
+    ModelBuilder builder = new ModelBuilder(doc);
+
+    // Attempt to create a compartment with the same id.
+    // Our new implementation should detect the clash via findNamedSBase
+    // and throw an IllegalArgumentException.
+    builder.buildCompartment("shared", true, "compartment", 3d, 1.0, "litre");
+  }
+}