Issue #23: CHE-GR needs to throw UnknownTerritoryException, not return MX-GRO

Author: rijnb

src/main/java/com/mapcode/Territory.java

@@ -739,7 +739,7 @@ public String toAlphaCode(@Nonnull final AlphaCodeFormat format, @Nullable final
         checkNonnull("format", format);
         String result = name().replace('_', '-');
         if (format != AlphaCodeFormat.INTERNATIONAL) {
-            final int index = name().indexOf('_');
+            final int index = name().lastIndexOf('_');
             if (index != -1) {
                 assert name().length() > (index + 1);
                 final String shortName = name().substring(index + 1);
@@ -903,11 +903,15 @@ private Territory(
      * @return Territory.
      * @throws UnknownTerritoryException Thrown if the territory is not found.
      */
+    @SuppressWarnings("TailRecursion")
     @Nonnull
     private static Territory createFromString(
             @Nonnull final String alphaCode,
             @Nullable final Territory parentTerritory) throws UnknownTerritoryException {
-        final String trimmed = Mapcode.convertStringToPlainAscii(alphaCode.trim().replace('_', '-')).toUpperCase();
+
+        // Replace '_' with '-', but leave spaces alone (may be part of the name).
+        final String trimmed = Mapcode.convertStringToPlainAscii(
+                alphaCode.trim().replace('_', '-')).toUpperCase();
 
         // Try as alpha code.
         final List<Territory> territories = nameMap.get(trimmed);
@@ -920,14 +924,23 @@ private static Territory createFromString(
                     return territory;
                 }
             }
-            throw new UnknownTerritoryException(trimmed);
-        }
+        } else {
 
-        // Check for a case such as USA-NLD (=NLD)
-        final int dividerLocation = Math.max(trimmed.indexOf('-'), trimmed.indexOf(' '));
-        if (dividerLocation >= 0) {
-            //noinspection TailRecursion
-            return createFromString(trimmed.substring(dividerLocation + 1), parentTerritory);
+            // Check for a case such as "United States of America-IN".
+            final int lastSeparator = Math.max(     // Find last separator.
+                    trimmed.lastIndexOf('-'),       // Allow '-'.
+                    trimmed.lastIndexOf(' '));      // And ' '.
+            if (lastSeparator >= 0) {
+                final String prefix = trimmed.substring(0, lastSeparator);
+                final Territory parent = createFromString(prefix, parentTerritory);
+                if (PARENT_TERRITORIES.contains(parent)) {
+                    final String postfix = trimmed.substring(lastSeparator + 1);
+                    final Territory child = createFromString(postfix, parentTerritory);
+                    if (child.parentTerritory == parent) {
+                        return child;
+                    }
+                }
+            }
         }
         throw new UnknownTerritoryException(trimmed);
     }
@@ -946,7 +959,7 @@ private static void addNameWithParentVariants(
         addNameWithSeperatorVariants(name, territory);
 
         if (name.contains("-")) {
-            final String childTerritoryName = name.substring(name.indexOf('-') + 1);
+            final String childTerritoryName = name.substring(name.lastIndexOf('-') + 1);
 
             // Tolerate a child territory specified without the parent.
             // (e.g. "CA" rather than "US-CA")

src/site/apt/ReleaseNotes.apt.vm

@@ -11,7 +11,9 @@
 
 * 2.2.3
 
-    * To do.
+    * Issue #23: Fixed <<<Territory.fromString>>> to make sure the parent territory is valid for
+      input like "CHE-GR". This returned "MX-GRO" instead of throwing <<<UnknownTerritoryException>>>.
+      Added unit test for this type of case.
 
     []
 

src/test/java/com/mapcode/TerritoryTest.java

@@ -17,6 +17,7 @@
 package com.mapcode;
 
 import com.mapcode.Territory.AlphaCodeFormat;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -52,6 +53,10 @@ public void checkDash() throws Exception {
         assertEquals(Territory.IN_MN, Territory.fromString("IND_MN"));
         assertEquals(Territory.US_MN, Territory.fromString("USA-MN"));
         assertEquals(Territory.US_MN, Territory.fromString("USA_MN"));
+
+        assertEquals(Territory.COD, Territory.fromString("Congo-Kinshasa"));
+        assertEquals(Territory.US_IN, Territory.fromString("United States of America-IN"));
+        assertEquals(Territory.US_IN, Territory.fromString("United States of America IN"));
     }
 
     @Test
@@ -100,10 +105,26 @@ public void testTerritoryFromString() throws Exception {
         }
     }
 
+    @Test(expected = UnknownTerritoryException.class)
+    public void testTerritoryFromStringIncorrectDash1() throws Exception {
+        LOG.info("testTerritoryFromStringIncorrectDash1");
+
+        // Issue: https://github.com/mapcode-foundation/mapcode-java/issues/23
+        assertEquals(Territory.AAA, Territory.fromString("CHE-GR"));    // Exception must be thrown.
+    }
+
+    @Test(expected = UnknownTerritoryException.class)
+    public void testTerritoryFromStringIncorrectDash2() throws Exception {
+        LOG.info("testTerritoryFromStringIncorrectDash2");
+        assertEquals(Territory.AAA, Territory.fromString("USA-NLD"));    // Exception must be thrown.
+    }
+
     @Test(expected = UnknownTerritoryException.class)
     public void testTerritoryFromStringNumeric() throws Exception {
         LOG.info("testTerritoryFromStringNumeric");
-        assertEquals(Territory.VAT, Territory.fromString("0"));
+
+        // No longer support: numeric codes.
+        assertEquals(Territory.AAA, Territory.fromString("0"));         // Exception must be thrown.
     }
 
     @Test