diff --git a/core/src/org/sbml/jsbml/validator/OverdeterminationValidator.java b/core/src/org/sbml/jsbml/validator/OverdeterminationValidator.java index 833842af2..751add089 100644 --- a/core/src/org/sbml/jsbml/validator/OverdeterminationValidator.java +++ b/core/src/org/sbml/jsbml/validator/OverdeterminationValidator.java @@ -38,6 +38,7 @@ import org.sbml.jsbml.ListOf; import org.sbml.jsbml.LocalParameter; import org.sbml.jsbml.Model; +import org.sbml.jsbml.NamedSBase; import org.sbml.jsbml.Parameter; import org.sbml.jsbml.RateRule; import org.sbml.jsbml.Reaction; @@ -45,6 +46,9 @@ import org.sbml.jsbml.SBase; import org.sbml.jsbml.Species; import org.sbml.jsbml.SpeciesReference; +import org.w3c.dom.Node; + +import com.thoughtworks.xstream.io.binary.Token.StartNode; /** * This class creates a bipartite graph and a matching for the given model using @@ -813,6 +817,42 @@ public Map getMatching() { return matching; } + /** + * Resolve an identifier from a MathML {@link ASTNode} to the + * corresponding {@link SBase} object in the model. + * + * This is in particular needed for SBML Level 3 Version 2 + * {@code rateOf} csymbols, where the {@link ASTNode#getVariable()} + * may not yet be initialized. + * + * Only model-level {@link Species}, {@link Compartment}, + * {@link Parameter}, and {@link Reaction} objects are considered. + * + * @param id the SBML id of the object + * @return the corresponding {@link SBase}, or {@code null} if none + * can be found or if the id refers to another type of + * {@link NamedSBase}. + */ + private SBase resolveSBaseFromId(String id) { + if (model == null || id == null) { + return null; + } + + NamedSBase named = model.findNamedSBase(id); + if (named == null) { + return null; + } + + if (named instanceof Species + || named instanceof Compartment + || named instanceof Parameter + || named instanceof Reaction) { + return named; + } + + return null; + } + /** * Returns the variables in a MathML object without local parameter * @@ -824,46 +864,59 @@ public Map getMatching() { private void getVariables(ListOf param, ASTNode node, List variables, int level) { - if (node == null) - { + if (node == null) { return; } - // found node with species - if ((node.getChildCount() == 0) && (node.isString()) && - (node.getType() != Type.NAME_TIME) && - (node.getType() != Type.NAME_AVOGADRO)) { // TODO - deal with csymbol rateOf as well ? + // found node with species / compartment / parameter / reaction id + if ((node.getChildCount() == 0) && node.isString() + && (node.getType() != Type.NAME_TIME) + && (node.getType() != Type.NAME_AVOGADRO)) { + if (!node.isConstant()) { - if (param == null) { - SBase variable=node.getVariable(); - if (level==1) { - int insertingPosition = 0; - for (SBase element:variables) { - if (!(element instanceof Parameter) || (!((Parameter)element).isSetValue())) { - insertingPosition++; - } - } - variables.add(insertingPosition, variable); - } - else { - variables.add(variable); + + // Try to get the referenced SBase. For identifiers used in + // csymbol rateOf (L3V2), getVariable() may not yet be set, + // so we fall back to resolving the id in the model. + SBase variable = node.getVariable(); + if (variable == null) { + String id = node.getName(); + if (id != null) { + variable = resolveSBaseFromId(id); } - } else { - if (!param.contains(node.getVariable())) { - SBase variable=node.getVariable(); - if (level==1) { - int insertingPosition=0; - for (SBase element:variables) { - if (!(element instanceof Parameter) || - (!((Parameter) element).isSetValue())) { + } + + if (variable != null) { + if (param == null) { + // Global identifier + if (level == 1) { + int insertingPosition = 0; + for (SBase element : variables) { + if (!(element instanceof Parameter) + || (!((Parameter) element).isSetValue())) { insertingPosition++; } } variables.add(insertingPosition, variable); - } - else { + } else { variables.add(variable); } + } else { + // Exclude local parameters + if (!param.contains(variable)) { + if (level == 1) { + int insertingPosition = 0; + for (SBase element : variables) { + if (!(element instanceof Parameter) + || (!((Parameter) element).isSetValue())) { + insertingPosition++; + } + } + variables.add(insertingPosition, variable); + } else { + variables.add(variable); + } + } } } } @@ -933,4 +986,4 @@ private void updateMatching(List> path) { } } -} +} \ No newline at end of file diff --git a/core/test/org/sbml/jsbml/validator/OverdeterminationValidatorTest.java b/core/test/org/sbml/jsbml/validator/OverdeterminationValidatorTest.java new file mode 100644 index 000000000..bacd0d24b --- /dev/null +++ b/core/test/org/sbml/jsbml/validator/OverdeterminationValidatorTest.java @@ -0,0 +1,62 @@ +package org.sbml.jsbml.validator; + +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.sbml.jsbml.ASTNode; +import org.sbml.jsbml.Model; +import org.sbml.jsbml.Reaction; +import org.sbml.jsbml.SBase; + +/** + * Tests for {@link OverdeterminationValidator}. + */ +public class OverdeterminationValidatorTest { + + /** + * In SBML Level 3 Version 2, the csymbol {@code rateOf} can target + * a reaction identifier. The {@link OverdeterminationValidator} + * must treat that identifier as a referenced variable in the MathML + * expression. + * + * This test checks that when an {@link ASTNode} refers to a reaction + * by its id, and {@code getVariable()} is not set, the validator still + * resolves the identifier via the model and collects the reaction as + * a variable. + */ + @Test + public void testGetVariablesResolvesReactionIdWhenVariableNotSet() throws Exception { + // Create minimal L3V2 model with one reaction + Model model = new Model(3, 2); + model.setId("m"); + Reaction r1 = model.createReaction(); + r1.setId("R1"); + + // Create an AST node that refers to the reaction by its id. + // In some L3V2 rateOf constructs, getVariable() may not be set + // on such nodes; the validator must then resolve the id via the model. + ASTNode nameNode = new ASTNode("R1"); + + OverdeterminationValidator validator = new OverdeterminationValidator(model); + + // Prepare list to receive variables + List vars = new ArrayList(); + + // Call the private getVariables(..) method via reflection + Method m = OverdeterminationValidator.class.getDeclaredMethod( + "getVariables", + org.sbml.jsbml.ListOf.class, + ASTNode.class, + List.class, + int.class); + m.setAccessible(true); + m.invoke(validator, null, nameNode, vars, model.getLevel()); + + assertTrue("The reaction referenced by id must be collected as a variable", + vars.contains(r1)); + } +} \ No newline at end of file diff --git a/extensions/fbc/src/org/sbml/jsbml/ext/fbc/GeneProduct.java b/extensions/fbc/src/org/sbml/jsbml/ext/fbc/GeneProduct.java index 534d2de4b..47871c706 100644 --- a/extensions/fbc/src/org/sbml/jsbml/ext/fbc/GeneProduct.java +++ b/extensions/fbc/src/org/sbml/jsbml/ext/fbc/GeneProduct.java @@ -112,12 +112,22 @@ public GeneProduct() { } /** - * Creates a new {@link GeneProduct} instance. + * Creates a new {@link GeneProduct} instance by copying all fields. * * @param nsb the instance to clone */ public GeneProduct(GeneProduct nsb) { super(nsb); + + // copy GeneProduct-specific fields + if (nsb.isSetLabel()) { + // use the setter to keep property-change semantics consistent + setLabel(nsb.getLabel()); + } + + if (nsb.isSetAssociatedSpecies()) { + setAssociatedSpecies(nsb.getAssociatedSpecies()); + } } /**