Fix: Integrate offline validation for SBase unit assignments

core/src/org/sbml/jsbml/AbstractNamedSBaseWithUnit.java

@@ -24,6 +24,9 @@
 import org.apache.log4j.Logger;
 import org.sbml.jsbml.Unit.Kind;
 import org.sbml.jsbml.util.TreeNodeChangeEvent;
+import org.sbml.jsbml.validator.SyntaxChecker;
+import org.sbml.jsbml.validator.offline.factory.SBMLErrorCodes;
+import org.sbml.jsbml.validator.offline.factory.SBMLErrorFactory;
 
 /**
  * This simple implementation of the interfaces
@@ -283,7 +286,7 @@ public void setUnits(Kind unitKind) {
   @Override
   public void setUnits(String units) {
     if ((units != null) && (units.trim().length() == 0)) {
-      units = null; // If we pass the empty String or null, the value is reset.
+      units = null;
     }
 
     String oldUnits = unitsID;
@@ -292,24 +295,68 @@ public void setUnits(String units) {
       unitsID = null;
     } else {
       units = units.trim();
+      unitsID = units;
 
-      boolean illegalArgument = false;
-
-      if (!Unit.isValidUnit(getModel(), units)) {
-        illegalArgument = true; // TODO - make use of the offline validation once attributes validation is in place.
+      boolean isSyntaxValid = SyntaxChecker.isValidId(units, getLevel(), getVersion());
+
+      boolean isReferenceValid = true;
+      if (isSyntaxValid && getSBMLDocument() != null) {
+        Model m = getModel();
+        boolean definedInModel = (m != null) && (m.getUnitDefinition(units) != null);
+        isReferenceValid = definedInModel
+            || Unit.isUnitKind(units, getLevel(), getVersion())
+            || Unit.isPredefined(units, getLevel());
       }
-      if (illegalArgument) {
+
+      if (!isSyntaxValid || !isReferenceValid) {
         if (!isReadingInProgress()) {
-          throw new IllegalArgumentException(MessageFormat.format(
-            JSBML.ILLEGAL_UNIT_EXCEPTION_MSG, units));
+          SBMLDocument doc = getSBMLDocument();
+          if (doc != null && doc.getErrorLog() != null) {
+            int errorCode;
+            if (!isSyntaxValid) {
+              errorCode = SBMLErrorCodes.CORE_10311;
+            } else {
+              if (getLevel() > 2 || (getLevel() == 2 && getVersion() >= 5)) {
+                errorCode = SBMLErrorCodes.CORE_10313;
+              } else {
+                errorCode = SBMLErrorCodes.CORE_99303;
+              }
+            }
+
+            org.sbml.jsbml.SBMLError error = SBMLErrorFactory.createError(
+              errorCode, 
+              getLevel(), 
+              getVersion(),
+              false,
+              this
+            );
+
+            if (error == null) {
+              error = new org.sbml.jsbml.SBMLError();
+              error.setCode(errorCode);
+              error.setSource(this);
+            } else if (error.getMessageInstance() != null) {
+java.util.ResourceBundle postMessageBundle = SBMLErrorFactory.getSBMLErrorPostMessageBundle();
+String postMessagePattern = SBMLErrorFactory.getBundleString(postMessageBundle, Integer.toString(errorCode));
+if (postMessagePattern != null) {
+  String detailedMessage = MessageFormat.format(postMessagePattern, units, getElementName(), getId());
+  error.getMessageInstance().setMessage(error.getMessageInstance().getMessage() + '\n' + detailedMessage);
+              }
+            }
+
+            doc.getErrorLog().add(error);
+          } else {
+            unitsID = oldUnits;
+            throw new IllegalArgumentException(MessageFormat.format(
+              JSBML.ILLEGAL_UNIT_EXCEPTION_MSG, units));
+          }
         } else {
           logger.info(MessageFormat.format(JSBML.ILLEGAL_UNIT_EXCEPTION_MSG, units));
         }
       }
-      unitsID = units;
     }
 
-    if (oldUnits != unitsID) {
+    if (oldUnits != null ? !oldUnits.equals(unitsID) : unitsID != null) {
       firePropertyChange(TreeNodeChangeEvent.units, oldUnits, unitsID);
     }
   }

core/test/org/sbml/jsbml/xml/test/TestAbstractNamedSBaseWithUnits.java

@@ -26,9 +26,11 @@
 import org.junit.Test;
 import org.sbml.jsbml.AbstractNamedSBaseWithUnit;
 import org.sbml.jsbml.Model;
+import org.sbml.jsbml.Parameter;
 import org.sbml.jsbml.SBMLDocument;
 import org.sbml.jsbml.Unit;
 import org.sbml.jsbml.UnitDefinition;
+import org.sbml.jsbml.validator.offline.factory.SBMLErrorCodes;
 
 
 /**
@@ -127,4 +129,60 @@ public void testIsPredefinedUnitsID() {
     assertTrue(!sbase.isPredefinedUnitsID(kind.toString().toLowerCase()));
   }
 
+  /**
+   * Test method for {@link org.sbml.jsbml.AbstractNamedSBaseWithUnit#setUnits(java.lang.String)}
+   * when an invalid unit syntax is set on an attached node.
+   */
+  @Test
+  public void testSetInvalidUnitSyntaxWithDocumentLogsError() {
+    SBMLDocument doc = sbase.getSBMLDocument();
+    int initialErrorCount = doc.getErrorLog().getErrorCount();
+    sbase.setUnits("123 invalid syntax!"); // Malformed SId
+    org.junit.Assert.assertEquals("Error count should increment", initialErrorCount + 1, doc.getErrorLog().getErrorCount());
+    org.junit.Assert.assertEquals("Should log CORE_10311 for invalid UnitSId syntax",
+      (long) SBMLErrorCodes.CORE_10311, (long) doc.getErrorLog().getError(initialErrorCount).getCode());
+  }
+
+  /**
+   * Test method for {@link org.sbml.jsbml.AbstractNamedSBaseWithUnit#setUnits(java.lang.String)}
+   * when an undefined unit is set on an attached node.
+   */
+  @Test
+  public void testSetMissingUnitReferenceWithDocumentLogsError() {
+    SBMLDocument doc = sbase.getSBMLDocument();
+    int initialErrorCount = doc.getErrorLog().getErrorCount();
+    sbase.setUnits("valid_syntax_but_missing"); // Valid SId, but not defined in model or built-ins
+    org.junit.Assert.assertEquals("Error count should increment", initialErrorCount + 1, doc.getErrorLog().getErrorCount());
+    org.junit.Assert.assertEquals("Should log CORE_10313 for missing unit reference",
+      (long) SBMLErrorCodes.CORE_10313, (long) doc.getErrorLog().getError(initialErrorCount).getCode());
+  }
+
+  /**
+   * Test method for {@link org.sbml.jsbml.AbstractNamedSBaseWithUnit#setUnits(java.lang.String)}
+   * when an invalid unit is set on an isolated node.
+   */
+  @Test(expected = IllegalArgumentException.class)
+  public void testSetInvalidUnitWithoutDocumentThrowsException() {
+    Parameter isolatedParam = new Parameter(3, 1);
+    isolatedParam.setUnits("invalid syntax!");
+  }
+
+  /**
+   * Test method for {@link org.sbml.jsbml.AbstractNamedSBaseWithUnit#setUnits(java.lang.String)}
+   * when an undefined unit is set on an attached node in SBML Level 2 Version 4.
+   */
+  @Test
+  public void testSetMissingUnitReferenceLevel2Version4LogsError() {
+    Parameter paramL2V4 = new Parameter(2, 4);
+    SBMLDocument doc = new SBMLDocument(2, 4);
+    Model model = doc.createModel("test_model_l2v4");
+    model.addParameter(paramL2V4);
+
+    int initialErrorCount = doc.getErrorLog().getErrorCount();
+    paramL2V4.setUnits("valid_syntax_but_missing");
+
+    org.junit.Assert.assertEquals("Error count should increment", initialErrorCount + 1, doc.getErrorLog().getErrorCount());
+    org.junit.Assert.assertEquals("Should log CORE_99303 for missing unit reference in L2V4",
+      (long) SBMLErrorCodes.CORE_99303, (long) doc.getErrorLog().getError(initialErrorCount).getCode());
+  }
 }