SP-482, BIS-318: Throw exception MaterialIdentifier.tryCreate() has inconsistent arguments.

SVN: 28578

openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MaterialIdentifier.java

@@ -93,29 +93,35 @@ public final class MaterialIdentifier implements Serializable
      * full identifier is specified.
      * 
      * @return <code>null</code> if no full identifier specified and material type is unknown.
+     * @throw {@link IllegalArgumentException} if material type of full identifier doesn't match
+     *        specified material type.
      */
     public static MaterialIdentifier tryCreate(String codeOrIdentifierOrNull,
             ICodeHolder materialTypeCodeHolderOrNull)
     {
         MaterialIdentifier materialIdentifier =
                 MaterialIdentifier.tryParseIdentifier(codeOrIdentifierOrNull);
-        if (materialIdentifier == null)
+        if (materialIdentifier != null)
         {
-            // if the material type of the property is fixed, then we accept when only material
-            // code is specified and its type is skipped (we know what the type should be)
             if (materialTypeCodeHolderOrNull != null
-                    && StringUtils.isBlank(codeOrIdentifierOrNull) == false)
+                    && materialIdentifier.getTypeCode().equals(
+                            materialTypeCodeHolderOrNull.getCode()) == false)
             {
-                materialIdentifier =
-                        new MaterialIdentifier(codeOrIdentifierOrNull,
-                                materialTypeCodeHolderOrNull.getCode());
-            } else
-            {
-                // identifier is invalid or null
-                return null;
+                throw new IllegalArgumentException("Material identified by '" + materialIdentifier
+                        + "' has to be of type " + materialTypeCodeHolderOrNull.getCode() + ".");
             }
+            return materialIdentifier;
+        }
+        // if the material type of the property is fixed, then we accept when only material
+        // code is specified and its type is skipped (we know what the type should be)
+        if (materialTypeCodeHolderOrNull != null
+                && StringUtils.isBlank(codeOrIdentifierOrNull) == false)
+        {
+            return new MaterialIdentifier(codeOrIdentifierOrNull,
+                    materialTypeCodeHolderOrNull.getCode());
         }
-        return materialIdentifier;
+        // identifier is invalid or null
+        return null;
     }
 
     /**

openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/EntityExistenceChecker.java

@@ -357,15 +357,21 @@ class EntityExistenceChecker
             {
                 String value = property.getValue();
                 MaterialTypePE materialType = propertyTypePE.getMaterialType();
-                MaterialIdentifier materialIdentifier =
-                        MaterialIdentifier.tryCreate(value, materialType);
-                if (materialIdentifier == null)
+                try
                 {
-                    errors.add("Material identifier not in the form '<material code> (<material type code>)': "
-                            + value);
-                } else
+                    MaterialIdentifier materialIdentifier =
+                            MaterialIdentifier.tryCreate(value, materialType);
+                    if (materialIdentifier == null)
+                    {
+                        errors.add("Material identifier not in the form '<material code> (<material type code>)': "
+                                + value);
+                    } else
+                    {
+                        materialExistenceManager.exists(materialIdentifier);
+                    }
+                } catch (Exception ex)
                 {
-                    materialExistenceManager.exists(materialIdentifier);
+                    errors.add(ex.getMessage());
                 }
             }
         }

openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MaterialIdentifierTest.java

@@ -53,11 +53,26 @@ public class MaterialIdentifierTest extends AssertJUnit
     @Test
     public void testTryCreateWithFullIdentifierAndType()
     {
-        MaterialType materialType = new MaterialTypeBuilder().code("MY_M").getMaterialType();
+        MaterialType materialType = new MaterialTypeBuilder().code("MY_MATERIAL").getMaterialType();
         assertEquals("ABC (MY_MATERIAL)",
                 MaterialIdentifier.tryCreate("ABC (MY_MATERIAL)", materialType).toString());
     }
 
+    @Test
+    public void testTryCreateWithFullIdentifierAndInconsistentType()
+    {
+        MaterialType materialType = new MaterialTypeBuilder().code("MY_M").getMaterialType();
+        try
+        {
+            MaterialIdentifier.tryCreate("ABC (MY_MATERIAL)", materialType);
+            fail("IllegalArgumentException expected");
+        } catch (IllegalArgumentException ex)
+        {
+            assertEquals("Material identified by 'ABC (MY_MATERIAL)' has to be of type MY_M.",
+                    ex.getMessage());
+        }
+    }
+
     @Test
     public void testTryCreateWithCodeAndType()
     {