Avoid clashes in ModelBuilder by reusing existing model elements

[dyrpsf]

Summary

This PR updates ModelBuilder so that it checks whether a model element with a given id
already exists before creating a new one. This avoids clashes and duplicate elements when
using ModelBuilder to extend or modify existing models.

The following methods now reuse existing elements if present:

• buildModel(String id, String name) → reuses the existing model in the SBMLDocument if set.
• buildCompartment(...) → uses model.getCompartment(id) if available.
• buildParameter(...) → uses model.getParameter(id) if available.
• buildReaction(...) → uses model.getReaction(id) if available.
• buildSpecies(...) → uses model.getSpecies(id) if available.
• buildUnitDefinition(...) → uses model.getUnitDefinition(id) if available.

In each case, a new element is only created if no element with the given id already exists.

Rationale

Previously, ModelBuilder always called createXXX(id) on the model, assuming that elements
with the given IDs did not exist yet. When extending an existing model, this could lead to
clashes or exceptions if an element with that id was already present.

With these changes, ModelBuilder becomes safer to use for both new and existing models.

Related issue

• Addresses sbmlteam/jsbml#253

[draeger]

It would be great to add to the JavaDoc at least one line for each buildXYZ method stating that it either creates or returns the corresponding model element with the given identifier (ID). If such an element already exists, it returns that element as is without applying the other given arguments. If no such element with that ID can be found, it will be created, and the given arguments will be set as its attributes.

Please note that in the current imnplementation it is still possible that clashes can happen: In the case that another element with the given ID exists, e.g., a species with that ID when buildCompartment is called, the method won't find anything, but it also can't create it. Maybe it is safer to call findNamedSBase and check whether the returned object is an instance of the requested type.

Please note that UnitDefinition objects have their separate namespace.

[dyrpsf]

It would be great to add to the JavaDoc at least one line for each buildXYZ method stating that it either creates or returns the corresponding model element with the given identifier (ID). If such an element already exists, it returns that element as is without applying the other given arguments. If no such element with that ID can be found, it will be created, and the given arguments will be set as its attributes.

Please note that in the current imnplementation it is still possible that clashes can happen: In the case that another element with the given ID exists, e.g., a species with that ID when buildCompartment is called, the method won't find anything, but it also can't create it. Maybe it is safer to call findNamedSBase and check whether the returned object is an instance of the requested type.

Please note that UnitDefinition objects have their separate namespace.

Thanks for the helpful suggestions!

I’ve updated the build methods as follows:

• Each of buildModel, buildCompartment, buildParameter, buildReaction,
  buildSpecies, and buildUnitDefinition now has Javadoc explaining that the
  method either returns an existing element with the given id (without applying
  the other arguments), or creates a new one and initialises it with the
  provided attributes.

• For SId-based elements (Compartment, Parameter, Reaction, Species),
  the implementation now uses model.findNamedSBase(id) to detect clashes. If
  an element with the same id exists but is of a different type, an
  IllegalArgumentException is thrown instead of silently creating a new
  element.

• UnitDefinition reuses existing definitions via model.getUnitDefinition(id)
  and is documented as having a separate namespace, so ids can overlap with
  SId-based elements without causing clashes.

mvn -pl core test passes on this branch.

[dyrpsf]

I’ve also added a small regression test class
ModelBuilderReuseTest that covers:

• Reusing an existing compartment without overwriting its attributes when
  buildCompartment is called again with the same id but different arguments.
• Throwing an IllegalArgumentException when attempting to build a compartment
  with an id that is already used by a Species (i.e. clash between types).

mvn -pl core test passes on this branch.