Improve cleanup of old parking tags

app/src/main/java/de/westnordost/streetcomplete/osm/street_parking/StreetParking.kt

@@ -5,11 +5,13 @@ import de.westnordost.streetcomplete.osm.hasCheckDateForKey
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.DIAGONAL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PARALLEL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PERPENDICULAR
+import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.UNKNOWN_ORIENTATION
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.HALF_ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_STREET
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.PAINTED_AREA_ONLY
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.STREET_SIDE
+import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.UNKNOWN_POSITION
 import de.westnordost.streetcomplete.osm.updateCheckDateForKey
 import kotlinx.serialization.Serializable
 
@@ -23,35 +25,44 @@ data class LeftAndRightStreetParking(val left: StreetParking?, val right: Street
 @Serializable object NoStreetParking : StreetParking()
 /** When an unknown/unsupported value has been used */
 @Serializable object UnknownStreetParking : StreetParking()
-/** When not both parking orientation and position have been specified*/
+/** When not both parking orientation and position have been specified */
 @Serializable object IncompleteStreetParking : StreetParking()
 /** There is street parking, but it is mapped as separate geometry */
 @Serializable object StreetParkingSeparate : StreetParking()
 
 @Serializable data class StreetParkingPositionAndOrientation(
-    val orientation: ParkingOrientation,
-    val position: ParkingPosition
+    val orientation: ParkingOrientation?,
+    val position: ParkingPosition?
 ) : StreetParking()
 
 enum class ParkingOrientation {
-    PARALLEL, DIAGONAL, PERPENDICULAR
+    PARALLEL,
+    DIAGONAL,
+    PERPENDICULAR,
+    UNKNOWN_ORIENTATION
 }
 
 enum class ParkingPosition {
     ON_STREET,
     HALF_ON_KERB,
     ON_KERB,
     STREET_SIDE,
-    PAINTED_AREA_ONLY
+    PAINTED_AREA_ONLY,
+    UNKNOWN_POSITION
 }
 
 fun LeftAndRightStreetParking.validOrNullValues(): LeftAndRightStreetParking {
     if (left?.isValid != false && right?.isValid != false) return this
     return LeftAndRightStreetParking(left?.takeIf { it.isValid }, right?.takeIf { it.isValid })
 }
 
-private val StreetParking.isValid: Boolean get() = when(this) {
+val StreetParking.isValid: Boolean get() = when(this) {
     IncompleteStreetParking, UnknownStreetParking -> false
+    is StreetParkingPositionAndOrientation ->
+        position != null &&
+        position != UNKNOWN_POSITION &&
+        orientation != null &&
+        orientation != UNKNOWN_ORIENTATION
     else -> true
 }
 
@@ -65,49 +76,63 @@ val StreetParking.estimatedWidthOffRoad: Float get() = when (this) {
     else -> 0f // otherwise let's assume it's not on the street itself
 }
 
-private val ParkingOrientation.estimatedWidth: Float get() = when (this) {
+private val ParkingOrientation?.estimatedWidth: Float get() = when (this) {
     PARALLEL -> 2f
     DIAGONAL -> 3f
     PERPENDICULAR -> 4f
+    else -> 2f
 }
 
-private val ParkingPosition.estimatedWidthOnRoadFactor: Float get() = when (this) {
+private val ParkingPosition?.estimatedWidthOnRoadFactor: Float get() = when (this) {
     ON_STREET -> 1f
     HALF_ON_KERB -> 0.5f
     ON_KERB -> 0f
     else -> 0.5f // otherwise let's assume it is somehow on the street
 }
 
 fun LeftAndRightStreetParking.applyTo(tags: Tags) {
+    if (right?.isValid == false || left?.isValid == false) {
+        throw IllegalArgumentException("Attempting to tag parking with invalid values")
+    }
+
     val currentParking = createStreetParkingSides(tags)
 
-    // was set before and changed: may be incorrect now - remove subtags!
-    if (currentParking?.left != null && currentParking.left != left ||
-        currentParking?.right != null && currentParking.right != right) {
-        /* This includes removing any parking:condition:*, which is a bit peculiar because most
-         * values are not even set in this function. But on the other hand, when the physical layout
-         * of the parking changes (=redesign of the street layout and furniture), the condition may
-         * very well change too, so better delete it to be on the safe side. (It is better to have
-         * no data than to have wrong data.) */
-        val parkingLaneSubtagging = Regex("^parking:(lane|condition):.*")
+    // parking:condition:<left/right/both>
+    val conditionRight = right?.toOsmConditionValue()
+    val conditionLeft = left?.toOsmConditionValue()
+
+    /* Clean up previous parking tags when the associated side is changed. This can include removing
+     * parking:condition:*, which is a bit peculiar because most values are not even set in this
+     * function. But on the other hand, when the physical layout of the parking changes (=redesign
+     * of the street layout and furniture), the condition may very well change too, so better delete
+     * it to be on the safe side. (It is better to have no data than to have wrong data.) */
+    val rightSideChanged = !right.isSupplementingOrEqual(currentParking?.right)
+    val leftSideChanged = !left.isSupplementingOrEqual(currentParking?.left)
+
+    val keysToRemove = mutableListOf<Regex>()
+    if (right == left) {
+        keysToRemove.add(Regex("^parking:lane:.*"))
+        if (conditionLeft != null && conditionRight == conditionLeft) {
+            keysToRemove.add(Regex("^parking:condition:.*"))
+        }
+    }
+    if (rightSideChanged && leftSideChanged) {
+        keysToRemove.add(Regex("^parking:(lane|condition):.*"))
+    } else if (rightSideChanged) {
+        keysToRemove.add(Regex("^parking:(lane|condition):(both|right).*"))
+    } else if (leftSideChanged) {
+        keysToRemove.add(Regex("^parking:(lane|condition):(both|left).*"))
+    }
+
+    if (keysToRemove.isNotEmpty()) {
         for (key in tags.keys) {
-            if (key.matches(parkingLaneSubtagging)) {
-                tags.remove(key)
-            }
+            if (keysToRemove.any { it.matches(key) }) tags.remove(key)
         }
     }
 
     // parking:lane:<left/right/both>
-    val laneRight = if (right != null) {
-        right.toOsmLaneValue() ?: throw IllegalArgumentException("Attempting to tag incomplete parking lane")
-    } else {
-        null
-    }
-    val laneLeft = if (left != null) {
-        left.toOsmLaneValue() ?: throw IllegalArgumentException("Attempting to tag incomplete parking lane")
-    } else {
-        null
-    }
+    val laneRight = right?.toOsmLaneValue()
+    val laneLeft = left?.toOsmLaneValue()
 
     if (laneLeft == laneRight) {
         if (laneLeft != null) tags["parking:lane:both"] = laneLeft
@@ -116,10 +141,6 @@ fun LeftAndRightStreetParking.applyTo(tags: Tags) {
         if (laneRight != null) tags["parking:lane:right"] = laneRight
     }
 
-    // parking:condition:<left/right/both>
-    val conditionRight = right?.toOsmConditionValue()
-    val conditionLeft = left?.toOsmConditionValue()
-
     if (conditionLeft == conditionRight) {
         if (conditionLeft != null) tags["parking:condition:both"] = conditionLeft
     } else {
@@ -143,9 +164,18 @@ fun LeftAndRightStreetParking.applyTo(tags: Tags) {
     }
 }
 
+private fun StreetParking?.isSupplementingOrEqual(other: StreetParking?): Boolean =
+    this == other || (
+    this is StreetParkingPositionAndOrientation &&
+        other is StreetParkingPositionAndOrientation &&
+        (position == other.position || other.position == null) &&
+        (orientation == other.orientation || other.orientation == null)
+    )
+
 /** get the OSM value for the parking:lane key */
 private fun StreetParking.toOsmLaneValue(): String? = when (this) {
-    is StreetParkingPositionAndOrientation -> orientation.toOsmValue()
+    is StreetParkingPositionAndOrientation -> orientation?.toOsmValue()
+        ?: throw IllegalArgumentException("Attempting to tag parking without orientation")
     NoStreetParking, StreetParkingProhibited, StreetStandingProhibited, StreetStoppingProhibited -> "no"
     StreetParkingSeparate -> "separate"
     UnknownStreetParking, IncompleteStreetParking -> null
@@ -164,10 +194,12 @@ private fun ParkingPosition.toOsmValue() = when (this) {
     ON_KERB -> "on_kerb"
     STREET_SIDE -> "street_side"
     PAINTED_AREA_ONLY -> "painted_area_only"
+    UNKNOWN_POSITION -> throw IllegalArgumentException("Attempting to tag invalid parking position")
 }
 
 private fun ParkingOrientation.toOsmValue() = when (this) {
     PARALLEL -> "parallel"
     DIAGONAL -> "diagonal"
     PERPENDICULAR -> "perpendicular"
+    UNKNOWN_ORIENTATION -> throw IllegalArgumentException("Attempting to tag invalid parking orientation")
 }

app/src/main/java/de/westnordost/streetcomplete/osm/street_parking/StreetParkingDrawable.kt

@@ -11,11 +11,13 @@ import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.DIAGONAL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PARALLEL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PERPENDICULAR
+import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.UNKNOWN_ORIENTATION
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.HALF_ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_STREET
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.PAINTED_AREA_ONLY
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.STREET_SIDE
+import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.UNKNOWN_POSITION
 import de.westnordost.streetcomplete.util.ktx.isApril1st
 import kotlin.math.ceil
 import kotlin.math.round
@@ -105,20 +107,23 @@ private fun getStreetDrawableResId(orientation: ParkingOrientation, position: Pa
             PARALLEL -> R.drawable.ic_street_parking_bays_parallel
             DIAGONAL -> R.drawable.ic_street_parking_bays_diagonal
             PERPENDICULAR -> R.drawable.ic_street_parking_bays_perpendicular
+            UNKNOWN_ORIENTATION -> null
         }
         PAINTED_AREA_ONLY -> when (orientation) {
             PARALLEL -> R.drawable.ic_street_marked_parking_parallel
             DIAGONAL -> R.drawable.ic_street_marked_parking_diagonal
             PERPENDICULAR -> R.drawable.ic_street_marked_parking_perpendicular
+            UNKNOWN_ORIENTATION -> null
         }
-        null -> null
+        UNKNOWN_POSITION, null -> null
     }
 
 /** number of cars parked */
 private val ParkingOrientation.carCount: Int get() = when (this) {
     PARALLEL -> 4
     DIAGONAL -> 6
     PERPENDICULAR -> 8
+    UNKNOWN_ORIENTATION -> throw IllegalArgumentException()
 }
 
 /** which car indices to not draw */
@@ -128,11 +133,13 @@ private fun getOmittedCarIndices(orientation: ParkingOrientation, position: Park
             PARALLEL -> listOf(1, 2)
             DIAGONAL -> listOf(2, 3)
             PERPENDICULAR -> listOf(0, 3, 4, 7)
+            UNKNOWN_ORIENTATION -> throw IllegalArgumentException()
         }
         PAINTED_AREA_ONLY -> when (orientation) {
             PARALLEL -> listOf(0, 3)
             DIAGONAL -> listOf(0, 1, 4, 5)
             PERPENDICULAR -> listOf(0, 1, 5, 6, 7)
+            UNKNOWN_ORIENTATION -> throw IllegalArgumentException()
         }
         else -> emptyList()
     }
@@ -142,13 +149,15 @@ private val ParkingOrientation.carsX: Float get() = when (this) {
     PARALLEL -> 0.44f
     DIAGONAL -> 0.50f
     PERPENDICULAR -> 0.50f
+    UNKNOWN_ORIENTATION -> throw IllegalArgumentException()
 }
 
 /** rotation of the cars */
 private val ParkingOrientation.carsRotation: Float get() = when (this) {
     PARALLEL -> 0f
     DIAGONAL -> 55f
     PERPENDICULAR -> 90f
+    UNKNOWN_ORIENTATION -> throw IllegalArgumentException()
 }
 
 private val CAR_RES_IDS = listOf(

app/src/main/java/de/westnordost/streetcomplete/osm/street_parking/StreetParkingItem.kt

@@ -8,6 +8,7 @@ import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_STREET
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.PAINTED_AREA_ONLY
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.STREET_SIDE
+import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.UNKNOWN_POSITION
 import de.westnordost.streetcomplete.util.ktx.noParkingLineStyleResId
 import de.westnordost.streetcomplete.util.ktx.noParkingSignDrawableResId
 import de.westnordost.streetcomplete.util.ktx.noStandingLineStyleResId
@@ -49,7 +50,7 @@ fun StreetParking.asStreetSideItem(
 
 private val StreetParking.titleResId: Int? get() = when (this) {
     NoStreetParking -> R.string.street_parking_no
-    is StreetParkingPositionAndOrientation -> position.titleResId
+    is StreetParkingPositionAndOrientation -> position?.titleResId
     StreetParkingProhibited -> R.string.street_parking_prohibited
     StreetParkingSeparate -> R.string.street_parking_separate
     StreetStandingProhibited -> R.string.street_standing_prohibited
@@ -102,24 +103,25 @@ private fun StreetParking.getFloatingIcon(countryInfo: CountryInfo): Image? = wh
 }?.let { ResImage(it) }
 
 fun StreetParkingPositionAndOrientation.asItem(context: Context, isUpsideDown: Boolean) =
-    Item2(this, getDialogIcon(context, isUpsideDown), ResText(position.titleResId))
+    Item2(this, getDialogIcon(context, isUpsideDown), ResText(position!!.titleResId))
 
 /** An icon for a street parking is square and shows always the same car so it is easier to spot
  *  the variation that matters(on kerb, half on kerb etc) */
 private fun StreetParkingPositionAndOrientation.getDialogIcon(context: Context, isUpsideDown: Boolean): Image =
-    DrawableImage(StreetParkingDrawable(context, orientation, position, isUpsideDown, 128, 128, R.drawable.ic_car1))
+    DrawableImage(StreetParkingDrawable(context, orientation!!, position, isUpsideDown, 128, 128, R.drawable.ic_car1))
 
 /** An image for a street parking to be displayed shows a wide variety of different cars so that
  *  it looks nicer and/or closer to reality */
 private fun StreetParkingPositionAndOrientation.getIcon(context: Context, isUpsideDown: Boolean): Image =
-    DrawableImage(StreetParkingDrawable(context, orientation, position, isUpsideDown, 128, 512))
+    DrawableImage(StreetParkingDrawable(context, orientation!!, position, isUpsideDown, 128, 512))
 
 private val ParkingPosition.titleResId: Int get() = when (this) {
     ON_STREET -> R.string.street_parking_on_street
     HALF_ON_KERB -> R.string.street_parking_half_on_kerb
     ON_KERB -> R.string.street_parking_on_kerb
     STREET_SIDE -> R.string.street_parking_street_side
     PAINTED_AREA_ONLY -> R.string.street_parking_painted_area_only
+    UNKNOWN_POSITION -> throw IllegalArgumentException()
 }
 
 val DISPLAYED_PARKING_POSITIONS: List<ParkingPosition> = listOf(

app/src/main/java/de/westnordost/streetcomplete/osm/street_parking/StreetParkingParser.kt

@@ -3,6 +3,7 @@ package de.westnordost.streetcomplete.osm.street_parking
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.DIAGONAL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PARALLEL
 import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.PERPENDICULAR
+import de.westnordost.streetcomplete.osm.street_parking.ParkingOrientation.UNKNOWN_ORIENTATION
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.HALF_ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_KERB
 import de.westnordost.streetcomplete.osm.street_parking.ParkingPosition.ON_STREET
@@ -25,43 +26,42 @@ private fun createParkingForSide(tags: Map<String, String>, side: String?): Stre
 
     val parkingValue = tags["parking:lane$sideVal"]
 
+    // some kind of no parking
     when (parkingValue) {
         // old style tagging
         "no_parking" -> return StreetParkingProhibited
         "no_standing" -> return StreetStandingProhibited
         "no_stopping" -> return StreetStoppingProhibited
-        null, "no" -> return when (tags["parking:condition$sideVal"]) {
+        null, "no" -> when (tags["parking:condition$sideVal"]) {
             // new style tagging
-            "no_parking" -> StreetParkingProhibited
-            "no_standing" -> StreetStandingProhibited
-            "no_stopping" -> StreetStoppingProhibited
-            "no" -> NoStreetParking
-            null -> if (parkingValue == "no") NoStreetParking else null
-            else -> null
+            "no_parking" -> return StreetParkingProhibited
+            "no_standing" -> return StreetStandingProhibited
+            "no_stopping" -> return StreetStoppingProhibited
+            "no" -> return NoStreetParking
+            null -> if (parkingValue == "no") return NoStreetParking
         }
         "yes" -> return IncompleteStreetParking
         "separate" -> return StreetParkingSeparate
-        else -> {
-            val parkingOrientation = parkingValue.toParkingOrientation()
-                // regard parking:lanes:*=marked as incomplete (because position is missing implicitly)
-                ?: return if (parkingValue == "marked") IncompleteStreetParking else UnknownStreetParking
+    }
 
-            val parkingPositionValue = tags["parking:lane$sideVal:$parkingValue"]
-                // parking position is mandatory to be regarded as complete
-                ?: return IncompleteStreetParking
+    // position + orientation parking
+    val parkingOrientation = parkingValue?.toParkingOrientation()
 
-            val parkingPosition = parkingPositionValue.toParkingPosition() ?: return UnknownStreetParking
+    val parkingPositionValue = tags["parking:lane$sideVal:$parkingValue"]
 
-            return StreetParkingPositionAndOrientation(parkingOrientation, parkingPosition)
-        }
-    }
+    val parkingPosition = parkingPositionValue?.toParkingPosition()
+        ?: if (parkingValue == "marked") PAINTED_AREA_ONLY else null
+
+    if (parkingPosition == null && parkingOrientation == null) return null
+
+    return StreetParkingPositionAndOrientation(parkingOrientation, parkingPosition)
 }
 
 private fun String.toParkingOrientation() = when (this) {
     "parallel" -> PARALLEL
     "diagonal" -> DIAGONAL
     "perpendicular" -> PERPENDICULAR
-    else -> null
+    else -> UNKNOWN_ORIENTATION
 }
 
 private fun String.toParkingPosition() = when (this) {
@@ -70,7 +70,7 @@ private fun String.toParkingPosition() = when (this) {
     "on_kerb" -> ON_KERB
     "painted_area_only", "marked" -> PAINTED_AREA_ONLY
     "lay_by", "street_side", "bays" -> STREET_SIDE
-    else -> null
+    else -> ParkingPosition.UNKNOWN_POSITION
 }
 
 private fun expandRelevantSidesTags(tags: Map<String, String>): Map<String, String> {