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,41 @@ public void setUnits(String units) {
       unitsID = null;
     } else {
       units = units.trim();
+      unitsID = units;
 
-      boolean illegalArgument = false;
+      boolean isSyntaxValid = SyntaxChecker.isValidId(units, getLevel(), getVersion());
+      boolean isReferenceValid = isSyntaxValid && Unit.isValidUnit(getModel(), units);
 
-      if (!Unit.isValidUnit(getModel(), units)) {
-        illegalArgument = true; // TODO - make use of the offline validation once attributes validation is in place.
-      }
-      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 = !isSyntaxValid ? SBMLErrorCodes.CORE_10311 : SBMLErrorCodes.CORE_10313;
+
+            org.sbml.jsbml.SBMLError error = SBMLErrorFactory.createError(
+              errorCode, 
+              getLevel(), 
+              getVersion(),
+              false, // Let the factory fetch the official SBML specification message!
+              this
+            );
+
+            if (error != null) {
+              doc.getErrorLog().add(error);
+            }
+            if (setLevelAndVersion(isSetNotes())) {
+          } 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,42 @@ 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!");
+  }
+
 }