Merge pull request #882 from lonvia/fix-language-dependet-context

This fixes a minor but long-standing bug where all extra context names ended up in the "default" language. Factors out the effected code into its own function to save a bit of code duplication.

There was also a small issue with names that are removed from the address list not ending up in the context because only "name" tags are taken into account.

Author: lonvia

app/es_embedded/src/main/java/de/komoot/photon/elasticsearch/PhotonDocConverter.java

@@ -76,7 +76,7 @@ public static XContentBuilder convert(PhotonDoc doc, String[] languages, String[
         String countryCode = doc.getCountryCode();
         if (countryCode != null)
             builder.field(Constants.COUNTRYCODE, countryCode);
-        writeContext(builder, doc.getContext(), languages);
+        writeContext(builder, doc.getContextByLanguage(languages));
         writeExtraTags(builder, doc.getExtratags(), extraTags);
         writeExtent(builder, doc.getBbox());
 
@@ -151,24 +151,10 @@ private static void write(XContentBuilder builder, Map<String, String> fNames, S
         builder.endObject();
     }
 
-    protected static void writeContext(XContentBuilder builder, Set<Map<String, String>> contexts, String[] languages) throws IOException {
-        final Map<String, Set<String>> multimap = new HashMap<>();
-
-        for (Map<String, String> context : contexts) {
-            if (context.get("name") != null) {
-                multimap.computeIfAbsent("default", k -> new HashSet<>()).add(context.get("name"));
-            }
-
-            for (String language : languages) {
-                if (context.get("name:" + language) != null) {
-                    multimap.computeIfAbsent("default", k -> new HashSet<>()).add(context.get("name:" + language));
-                }
-            }
-        }
-
-        if (!multimap.isEmpty()) {
+    protected static void writeContext(XContentBuilder builder, Map<String, Set<String>> contexts) throws IOException {
+        if (!contexts.isEmpty()) {
             builder.startObject("context");
-            for (Map.Entry<String, Set<String>> entry : multimap.entrySet()) {
+            for (Map.Entry<String, Set<String>> entry : contexts.entrySet()) {
                 builder.field(entry.getKey(), String.join(", ", entry.getValue()));
             }
             builder.endObject();

app/opensearch/src/main/java/de/komoot/photon/opensearch/PhotonDocSerializer.java

@@ -89,7 +89,7 @@ public void serialize(PhotonDoc value, JsonGenerator gen, SerializerProvider pro
             gen.writeStringField(Constants.COUNTRYCODE, countryCode);
         }
 
-        writeContext(gen, value.getContext());
+        writeContext(gen, value.getContextByLanguage(languages));
         writeExtraTags(gen, value.getExtratags());
         writeExtent(gen, value.getBbox());
 
@@ -115,24 +115,10 @@ private void writeName(JsonGenerator gen, PhotonDoc doc, String[] languages) thr
         gen.writeObjectField("name", fNames);
     }
 
-    private void writeContext(JsonGenerator gen, Set<Map<String, String>> contexts) throws IOException {
-        final Map<String, Set<String>> multimap = new HashMap<>();
-
-        for (Map<String, String> context : contexts) {
-            if (context.get("name") != null) {
-                multimap.computeIfAbsent("default", k -> new HashSet<>()).add(context.get("name"));
-            }
-
-            for (String language : languages) {
-                if (context.get("name:" + language) != null) {
-                    multimap.computeIfAbsent("default", k -> new HashSet<>()).add(context.get("name:" + language));
-                }
-            }
-        }
-
-        if (!multimap.isEmpty()) {
+    private void writeContext(JsonGenerator gen, Map<String, Set<String>> contexts) throws IOException {
+        if (!contexts.isEmpty()) {
             gen.writeObjectFieldStart("context");
-            for (Map.Entry<String, Set<String>> entry : multimap.entrySet()) {
+            for (Map.Entry<String, Set<String>> entry : contexts.entrySet()) {
                 gen.writeStringField(entry.getKey(), String.join(", ", entry.getValue()));
             }
             gen.writeEndObject();

src/main/java/de/komoot/photon/PhotonDoc.java

@@ -235,7 +235,7 @@ private void extractAddress(Map<String, String> address, AddressType addressType
                 LOGGER.debug("Replacing {} name '{}' with '{}' for osmId #{}", addressFieldName, existingName, field, osmId);
                 // we keep the former name in the context as it might be helpful when looking up typos
                 if (!Objects.isNull(existingName)) {
-                    context.add(Collections.singletonMap("formerName", existingName));
+                    context.add(Collections.singletonMap("name", existingName));
                 }
                 map.put("name", field);
                 addressParts.put(addressType, map);
@@ -273,6 +273,26 @@ public void completePlace(List<AddressRow> addresses) {
         }
     }
 
+    public Map<String, Set<String>> getContextByLanguage(String[] languages) {
+        final Map<String, Set<String>> multimap = new HashMap<>();
+
+        for (Map<String, String> cmap : context) {
+            String locName = cmap.get("name");
+            if (locName != null) {
+                multimap.computeIfAbsent("default", k -> new HashSet<>()).add(locName);
+            }
+
+            for (String language : languages) {
+                locName = cmap.get("name:" + language);
+                if (locName != null) {
+                    multimap.computeIfAbsent(language, k -> new HashSet<>()).add(locName);
+                }
+            }
+        }
+
+        return multimap;
+    }
+
     public void setCountry(Map<String, String> names) {
         addressParts.put(AddressType.COUNTRY, names);
     }