Changeset 15952 in josm

Timestamp:

2020-02-28T21:42:52+01:00 (5 years ago)

Author:

Don-vip

Message:

see #18772 - better error message, add non-regression tests

Location:

trunk

Files:

• src/org/openstreetmap/josm/tools/Territories.java (modified) (3 diffs)
• test/data/taginfo (added)
• test/data/taginfo/geofabrik-index-v1-nogeom-broken.json (added)
• test/data/taginfo/geofabrik-index-v1-nogeom.json (added)
• test/unit/org/openstreetmap/josm/tools/TerritoriesTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/tools/Territories.java

@@ -2 +2 @@
 package org.openstreetmap.josm.tools;
 
+import static java.util.Optional.ofNullable;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -22 +23 @@
 import javax.json.Json;
 import javax.json.JsonArray;
-import javax.json.JsonObject;
 import javax.json.JsonString;
 import javax.json.JsonValue;
@@ -163 +163 @@
     private static void initializeExternalData() {
         taginfoGeofabrikCache = new TreeMap<>();
-        try (CachedFile cf = new CachedFile(Config.getUrls().getJOSMWebsite() + "/remote/geofabrik-index-v1-nogeom.json");
-                InputStream is = cf.getInputStream();
-                JsonParser json = Json.createParser(is)) {
+        initializeExternalData(taginfoGeofabrikCache, "Geofabrik",
+                Config.getUrls().getJOSMWebsite() + "/remote/geofabrik-index-v1-nogeom.json");
+    }
+
+    static void initializeExternalData(Map<String, TaginfoRegionalInstance> cache, String source, String path) {
+        try (CachedFile cf = new CachedFile(path); InputStream is = cf.getInputStream(); JsonParser json = Json.createParser(is)) {
             while (json.hasNext()) {
                 Event event = json.next();
                 if (event == Event.START_OBJECT) {
                     for (JsonValue feature : json.getObject().getJsonArray("features")) {
-                        JsonObject props = feature.asJsonObject().getJsonObject("properties");
-                        if (props != null) {
-                            JsonObject urls = props.getJsonObject("urls");
-                            if (urls != null) {
-                                String taginfo = urls.getString(TAGINFO);
-                                if (taginfo != null) {
-                                    JsonArray iso1 = props.getJsonArray(ISO3166_1_LC);
-                                    JsonArray iso2 = props.getJsonArray(ISO3166_2_LC);
-                                    if (iso1 != null) {
-                                        readExternalTaginfo(taginfo, iso1);
-                                    } else if (iso2 != null) {
-                                        readExternalTaginfo(taginfo, iso2);
-                                    }
-                                }
+                        ofNullable(feature.asJsonObject().getJsonObject("properties")).ifPresent(props ->
+                        ofNullable(props.getJsonObject("urls")).ifPresent(urls ->
+                        ofNullable(urls.getString(TAGINFO)).ifPresent(taginfo -> {
+                            JsonArray iso1 = props.getJsonArray(ISO3166_1_LC);
+                            JsonArray iso2 = props.getJsonArray(ISO3166_2_LC);
+                            if (iso1 != null) {
+                                readExternalTaginfo(cache, taginfo, iso1, source);
+                            } else if (iso2 != null) {
+                                readExternalTaginfo(cache, taginfo, iso2, source);
                             }
-                        }
+                        })));
                     }
                 }
             }
         } catch (IOException | JsonParsingException e) {
-            Logging.trace(e);
-            Logging.warn(tr("Failed to parse taginfo data geofabrik-index-v1-nogeom.json"));
-        }
-    }
-
-    private static void readExternalTaginfo(String taginfo, JsonArray jsonCodes) {
+            Logging.debug(e);
+            Logging.warn(tr("Failed to parse external taginfo data at {0}: {1}", path, e.getMessage()));
+        }
+    }
+
+    private static void readExternalTaginfo(Map<String, TaginfoRegionalInstance> cache, String taginfo, JsonArray jsonCodes, String source) {
         Set<String> isoCodes = jsonCodes.getValuesAs(JsonString.class).stream().map(JsonString::getString).collect(Collectors.toSet());
-        isoCodes.forEach(s -> taginfoGeofabrikCache.put(s, new TaginfoRegionalInstance(taginfo, isoCodes, "Geofabrik")));
+        isoCodes.forEach(s -> cache.put(s, new TaginfoRegionalInstance(taginfo, isoCodes, source)));
     }
 

trunk/test/unit/org/openstreetmap/josm/tools/TerritoriesTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.tools;
 
+import static java.util.Collections.singleton;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
 
 import org.junit.Rule;
 import org.junit.Test;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
@@ -45 +54 @@
         }
     }
+
+    /**
+     * Test of {@link Territories#initializeExternalData} - nominal case
+     */
+    @Test
+    public void testTaginfoGeofabrik_nominal() {
+        Map<String, TaginfoRegionalInstance> cache = new TreeMap<>();
+        Territories.initializeExternalData(cache, "foo", TestUtils.getTestDataRoot() + "/taginfo/geofabrik-index-v1-nogeom.json");
+        assertEquals(5, cache.size());
+        checkTaginfoInstance(cache.get("AF"), singleton("AF"), "https://taginfo.geofabrik.de/asia/afghanistan/");
+        checkTaginfoInstance(cache.get("AL"), singleton("AL"), "https://taginfo.geofabrik.de/europe/albania/");
+        checkTaginfoInstance(cache.get("CA-AB"), singleton("CA-AB"), "https://taginfo.geofabrik.de/north-america/canada/alberta/");
+        Set<String> israelAndPalestine = new HashSet<>(Arrays.asList("PS", "IL"));
+        checkTaginfoInstance(cache.get("PS"), israelAndPalestine, "https://taginfo.geofabrik.de/asia/israel-and-palestine/");
+        checkTaginfoInstance(cache.get("IL"), israelAndPalestine, "https://taginfo.geofabrik.de/asia/israel-and-palestine/");
+    }
+
+    private static void checkTaginfoInstance(TaginfoRegionalInstance instance, Set<String> expectedIsoCodes, String expectedUrl) {
+        assertEquals(expectedIsoCodes, instance.getIsoCodes());
+        assertEquals("foo", instance.getSuffix());
+        assertEquals(expectedUrl, instance.getUrl());
+    }
+
+    /**
+     * Test of {@link Territories#initializeExternalData} - broken contents
+     */
+    @Test
+    public void testTaginfoGeofabrik_broken() {
+        Map<String, TaginfoRegionalInstance> cache = new TreeMap<>();
+        Logging.clearLastErrorAndWarnings();
+        Territories.initializeExternalData(cache, "foo", TestUtils.getTestDataRoot() + "taginfo/geofabrik-index-v1-nogeom-broken.json");
+        assertTrue(cache.isEmpty());
+        assertEquals("W: Failed to parse external taginfo data at test/data/taginfo/geofabrik-index-v1-nogeom-broken.json: " +
+                "Invalid token=EOF at (line no=3, column no=49, offset=97). Expected tokens are: " +
+                "[CURLYOPEN, SQUAREOPEN, STRING, NUMBER, TRUE, FALSE, NULL, SQUARECLOSE]",
+                Logging.getLastErrorAndWarnings().get(0));
+    }
 }