MCR-3302 fix edge cases for MCRCategoryID.fromString() (#2327)

MyCoRe-Org · Nov 20, 2024 · 44d60af · 44d60af
1 parent 8fe70b9
commit 44d60af
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 26 deletions.
diff --git a/mycore-base/pom.xml b/mycore-base/pom.xml
@@ -456,6 +456,11 @@
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.junit.vintage</groupId>
       <artifactId>junit-vintage-engine</artifactId>

diff --git a/mycore-base/src/main/java/org/mycore/datamodel/classifications2/MCRCategoryID.java b/mycore-base/src/main/java/org/mycore/datamodel/classifications2/MCRCategoryID.java
@@ -21,7 +21,6 @@
 import java.io.Serializable;
 import java.text.MessageFormat;
 import java.util.Locale;
-import java.util.StringTokenizer;
 import java.util.regex.Pattern;
 
 import org.mycore.common.MCRException;
@@ -40,7 +39,7 @@
 /**
  * The composite identifier of a MCRCategory. If <code>rootID == ID</code> the
  * associated MCRCategory instance is a root category (a classification).
- * 
+ *
  * @author Thomas Scheffler (yagee)
  * @since 2.0
  */
@@ -88,19 +87,20 @@ public static MCRCategoryID rootID(String rootID) {
     /**
      * @param categoryId must be in format classificationId:categoryId
      * @return the {@link MCRCategoryID} if any
+     * @throws IllegalArgumentException if the given categoryId is invalid
      */
     @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
     public static MCRCategoryID fromString(String categoryId) {
-        StringTokenizer tok = new StringTokenizer(categoryId, ":");
-        String rootId = tok.nextToken();
-        if (!tok.hasMoreTokens()) {
-            return rootID(rootId);
-        }
-        String categId = tok.nextToken();
-        if (tok.hasMoreTokens()) {
-            throw new IllegalArgumentException("CategoryId is ambiguous: " + categoryId);
+        String[] parts = categoryId.split(":");
+        try {
+            return switch (parts.length) {
+                case 1 -> rootID(parts[0]);
+                case 2 -> new MCRCategoryID(parts[0], parts[1]);
+                default -> throw new IllegalArgumentException("CategoryId is ambiguous: " + categoryId);
+            };
+        } catch (MCRException e) {
+            throw new IllegalArgumentException("Invalid category ID: " + categoryId, e);
         }
-        return new MCRCategoryID(rootId, categId);
     }
 
     @Transient
@@ -110,7 +110,7 @@ public boolean isRootID() {
 
     /*
      * (non-Javadoc)
-     * 
+     *
      * @see java.lang.Object#hashCode()
      */
     @Override
@@ -124,7 +124,7 @@ public int hashCode() {
 
     /*
      * (non-Javadoc)
-     * 
+     *
      * @see java.lang.Object#equals(java.lang.Object)
      */
     @Override
@@ -220,15 +220,15 @@ private void setRootID(String rootID) {
         }
         if (rootID.length() > ROOT_ID_LENGTH) {
             throw new MCRException(String.format(Locale.ENGLISH,
-                "classification ID ''%s'' is more than %d chracters long: %d", rootID, ROOT_ID_LENGTH,
+                "classification ID ''%s'' is more than %d characters long: %d", rootID, ROOT_ID_LENGTH,
                 rootID.length()));
         }
         this.rootID = rootID.intern();
     }
 
     /*
      * (non-Javadoc)
-     * 
+     *
      * @see java.lang.Object#toString()
      */
     @Override

diff --git a/mycore-base/src/test/java/org/mycore/datamodel/classifications2/MCRCategoryIDTest.java b/mycore-base/src/test/java/org/mycore/datamodel/classifications2/MCRCategoryIDTest.java
@@ -19,16 +19,19 @@
 package org.mycore.datamodel.classifications2;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mycore.common.MCRException;
-import org.mycore.common.MCRTestCase;
 
 /**
  * @author Thomas Scheffler (yagee)
  *
  */
-public class MCRCategoryIDTest extends MCRTestCase {
+public class MCRCategoryIDTest {
     private static final String invalidID = "identifier:.sub";
 
     private static final String validRootID = "rootID";
@@ -56,23 +59,36 @@ public void testRootID() {
         assertEquals("RootIds do not match", validRootID, MCRCategoryID.rootID(validRootID).getRootID());
     }
 
-    @Test(expected = MCRException.class)
+    @Test
     public void testInvalidRootID() {
-        new MCRCategoryID(invalidID, validCategID);
+        assertThrows(MCRException.class, () -> new MCRCategoryID(invalidID, validCategID));
     }
 
-    @Test(expected = MCRException.class)
+    @Test
     public void testInvalidCategID() {
-        new MCRCategoryID(validRootID, invalidID);
+        assertThrows(MCRException.class, () -> new MCRCategoryID(validRootID, invalidID));
     }
 
-    @Test(expected = MCRException.class)
+    @Test
     public void testLongCategID() {
-        new MCRCategoryID(validRootID, toLongCategID);
+        assertThrows(MCRException.class, () -> new MCRCategoryID(validRootID, toLongCategID));
     }
 
-    @Test(expected = MCRException.class)
+    @Test
     public void testLongRootID() {
-        new MCRCategoryID(toLongRootID, validCategID);
+        assertThrows(MCRException.class, () -> new MCRCategoryID(toLongRootID, validCategID));
     }
+
+    /**
+     * @see <a href="https://mycore.atlassian.net/browse/MCR-3302">MCR-3302</a>
+     */
+    @ParameterizedTest
+    @Tag("MCR-3302")
+    @ValueSource(strings = {
+        "", "foo:bar:baz", ":bar", ":bar:", ":bar:baz", "foo::bar", "foo::bar::", "foo::bar::baz", "::bar", "::bar::baz"
+    })
+    public void testInvalidEdgeCases(String invalidCategoryId) {
+        assertThrows(IllegalArgumentException.class, () -> MCRCategoryID.fromString(invalidCategoryId));
+    }
+
 }