localized names: more tests and fix removing names did not actually r…

…emove them (and other issues)

app/src/main/java/de/westnordost/streetcomplete/osm/LocalizedNames.kt

@@ -27,7 +27,6 @@ data class LocalizedName(var languageTag: String, var name: String)
  *  */
 fun createLocalizedNames(tags: Map<String, String>): List<LocalizedName>? {
     val result = ArrayList<LocalizedName>()
-    val namePattern = Regex("name(?::([a-z]{2,3}(?:-[a-zA-Z]{4})?))?")
     for ((key, value) in tags) {
         val m = namePattern.matchEntire(key)
         if (m != null) {
@@ -48,26 +47,39 @@ fun createLocalizedNames(tags: Map<String, String>): List<LocalizedName>? {
 }
 
 fun List<LocalizedName>.applyTo(tags: Tags) {
+    // applying a list of localized names always replaces the entire list of localized names, i.e.
+    // anything not specified gets deleted.
+    for (key in tags.keys) {
+        val isLocalizedName = namePattern.matches(key)
+        if (isLocalizedName) tags.remove(key)
+    }
+    tags.remove("int_name")
+
     if (isEmpty()) return
 
+    // if it has names, it is not noname...
+    tags.remove("noname")
+    tags.remove("name:signed")
+
     // language is only specified explicitly in OSM (usually) if there is more than one name specified
     if (size == 1) {
         tags["name"] = first().name
-        return
-    }
-
-    for ((language, name) in this) {
-        val key = when (language) {
-            "" -> "name"
-            "international" -> "int_name"
-            else -> "name:$language"
+    } else {
+        for ((language, name) in this) {
+            val key = when (language) {
+                "" -> "name"
+                "international" -> "int_name"
+                else -> "name:$language"
+            }
+            tags[key] = name
         }
-        tags[key] = name
-    }
 
-    // but if there is more than one language, ensure that a "main" name is also specified
-    if (find { it.languageTag == "" } == null) {
-        // use the name specified in the topmost row for that
-        tags["name"] = first().name
+        // but if there is more than one language, ensure that a "main" name is also specified
+        if (find { it.languageTag == "" } == null) {
+            // use the name specified in the topmost row for that
+            tags["name"] = first().name
+        }
     }
 }
+
+private val namePattern = Regex("name(?::([a-z]{2,3}(?:-[a-zA-Z]{4})?))?")

app/src/test/java/de/westnordost/streetcomplete/osm/LocalizedNamesKtTest.kt

@@ -1,6 +1,10 @@
 package de.westnordost.streetcomplete.osm
 
 import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChangesBuilder
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryChange
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete
+import org.assertj.core.api.Assertions
 import org.junit.Assert.assertEquals
 import org.junit.Test
 
@@ -28,27 +32,108 @@ internal class LocalizedNamesKtTest {
         )
     }
 
-    @Test fun localizedNamesToTags() {
-        val builder = StringMapChangesBuilder(mapOf())
-        listOf(
-            LocalizedName("", "näm"),
-            LocalizedName("international", "ünt_näm"),
-            LocalizedName("de", "zörmän näm"),
-            LocalizedName("th-Latn", "ziämäs näm in Lätin"),
-            LocalizedName("guj", "güjäräti näm")
-        ).applyTo(builder)
-        val map = mutableMapOf<String, String>()
-        builder.create().applyTo(map)
+    @Test fun `apply empty localized names`() {
+        verifyAnswer(
+            mapOf(),
+            listOf(),
+            arrayOf()
+        )
+    }
 
-        assertEquals(
+    @Test fun `apply empty localized names removes previously set names`() {
+        verifyAnswer(
             mapOf(
-                "name" to "näm",
-                "int_name" to "ünt_näm",
-                "name:de" to "zörmän näm",
-                "name:th-Latn" to "ziämäs näm in Lätin",
-                "name:guj" to "güjäräti näm"
+                "name" to "1",
+                "int_name" to "2",
+                "name:de" to "3",
+                "name:th-Latn" to "4",
+                "name:guj" to "5"
+            ),
+            listOf(),
+            arrayOf(
+                StringMapEntryDelete("name", "1"),
+                StringMapEntryDelete("int_name", "2"),
+                StringMapEntryDelete("name:de", "3"),
+                StringMapEntryDelete("name:th-Latn", "4"),
+                StringMapEntryDelete("name:guj", "5")
+            )
+        )
+    }
+
+    @Test fun `apply empty localized names does not remove noname and variants`() {
+        verifyAnswer(
+            mapOf(
+                "noname" to "yes",
+                "name:signed" to "no"
             ),
-            map
+            listOf(),
+            arrayOf()
         )
     }
+
+    @Test fun `apply localized names`() {
+        verifyAnswer(
+            mapOf(),
+            listOf(
+                LocalizedName("", "1"),
+                LocalizedName("international", "2"),
+                LocalizedName("de", "3"),
+                LocalizedName("th-Latn", "4"),
+                LocalizedName("guj", "5")
+            ),
+            arrayOf(
+                StringMapEntryAdd("name", "1"),
+                StringMapEntryAdd("int_name", "2"),
+                StringMapEntryAdd("name:de", "3"),
+                StringMapEntryAdd("name:th-Latn", "4"),
+                StringMapEntryAdd("name:guj", "5")
+            )
+        )
+    }
+
+    @Test fun `apply localized names removes noname and variants`() {
+        verifyAnswer(
+            mapOf("noname" to "yes", "name:signed" to "no"),
+            listOf(LocalizedName("", "1")),
+            arrayOf(
+                StringMapEntryAdd("name", "1"),
+                StringMapEntryDelete("noname", "yes"),
+                StringMapEntryDelete("name:signed", "no"),
+            )
+        )
+    }
+
+    @Test fun `apply one name only drops language tag`() {
+        verifyAnswer(
+            mapOf("name:de" to "hoho"),
+            listOf(LocalizedName("de", "1")),
+            arrayOf(
+                StringMapEntryAdd("name", "1"),
+                StringMapEntryDelete("name:de", "hoho"),
+            )
+        )
+    }
+
+
+    @Test fun `apply two names adds name tag`() {
+        verifyAnswer(
+            mapOf(),
+            listOf(
+                LocalizedName("de", "haha"),
+                LocalizedName("es", "jaja")
+            ),
+            arrayOf(
+                StringMapEntryAdd("name", "haha"),
+                StringMapEntryAdd("name:de", "haha"),
+                StringMapEntryAdd("name:es", "jaja"),
+            )
+        )
+    }
+}
+
+private fun verifyAnswer(tags: Map<String, String>, answer: List<LocalizedName>, expectedChanges: Array<StringMapEntryChange>) {
+    val cb = StringMapChangesBuilder(tags)
+    answer.applyTo(cb)
+    val changes = cb.create().changes
+    Assertions.assertThat(changes).containsExactlyInAnyOrder(*expectedChanges)
 }