Merge pull request #5273 from FloEdelmann/get-feature

streetcomplete · Sep 26, 2023 · f007952 · f007952
2 parents 6b4ccbf + 4734274
commit f007952
Show file tree

Hide file tree

Showing 13 changed files with 97 additions and 133 deletions.
diff --git a/...ests/existence/CheckExistenceLabelTest.kt → ...complete/util/NameAndLocationLabelTest.kt b/...ests/existence/CheckExistenceLabelTest.kt → ...complete/util/NameAndLocationLabelTest.kt
@@ -1,4 +1,4 @@
-package de.westnordost.streetcomplete.quests.existence
+package de.westnordost.streetcomplete.util
 
 import android.content.res.Configuration
 import android.content.res.Resources
@@ -7,28 +7,22 @@ import de.westnordost.osmfeatures.AndroidFeatureDictionary
 import de.westnordost.osmfeatures.FeatureDictionary
 import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
 import de.westnordost.streetcomplete.data.osm.mapdata.Node
-import de.westnordost.streetcomplete.util.getNameAndLocationLabel
 import java.util.Locale
-import java.util.concurrent.FutureTask
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
-class CheckExistenceLabelTest {
-    private var featureDictionaryFuture: FutureTask<FeatureDictionary>
+class NameAndLocationLabelTest {
+    private var featureDictionary: FeatureDictionary
     private var englishResources: Resources
-    private var questType: CheckExistence
 
     init {
         val context = InstrumentationRegistry.getInstrumentation().targetContext
-        featureDictionaryFuture = FutureTask { AndroidFeatureDictionary.create(context.assets, "osmfeatures/default", "osmfeatures/brands") }
-        featureDictionaryFuture.run()
+        featureDictionary = AndroidFeatureDictionary.create(context.assets, "osmfeatures/default", "osmfeatures/brands")
 
         val conf = Configuration(context.resources.configuration)
         conf.setLocale(Locale.ENGLISH)
         val localizedContext = context.createConfigurationContext(conf)
         englishResources = localizedContext.resources
-
-        questType = CheckExistence(featureDictionaryFuture)
     }
 
     // https://github.com/streetcomplete/StreetComplete/issues/2512
@@ -90,6 +84,6 @@ class CheckExistenceLabelTest {
         getNameAndLocationLabel(
             Node(0, LatLon(0.0, 0.0), tags),
             englishResources,
-            featureDictionaryFuture.get()
+            featureDictionary
         )?.toString()
 }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/overlays/OverlaysModule.kt b/app/src/main/java/de/westnordost/streetcomplete/overlays/OverlaysModule.kt
@@ -1,5 +1,9 @@
 package de.westnordost.streetcomplete.overlays
 
+import de.westnordost.countryboundaries.CountryBoundaries
+import de.westnordost.osmfeatures.Feature
+import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.streetcomplete.data.meta.CountryInfos
 import de.westnordost.streetcomplete.data.overlays.OverlayRegistry
 import de.westnordost.streetcomplete.overlays.address.AddressOverlay
 import de.westnordost.streetcomplete.overlays.cycleway.CyclewayOverlay
@@ -8,19 +12,37 @@ import de.westnordost.streetcomplete.overlays.sidewalk.SidewalkOverlay
 import de.westnordost.streetcomplete.overlays.street_parking.StreetParkingOverlay
 import de.westnordost.streetcomplete.overlays.surface.SurfaceOverlay
 import de.westnordost.streetcomplete.overlays.way_lit.WayLitOverlay
+import de.westnordost.streetcomplete.util.ktx.getFeature
 import org.koin.core.qualifier.named
 import org.koin.dsl.module
+import java.util.concurrent.FutureTask
 
 /* Each overlay is assigned an ordinal. This is used for serialization and is thus never changed,
 *  even if the order of overlays is changed.  */
 val overlaysModule = module {
-    single { OverlayRegistry(listOf(
-        0 to WayLitOverlay(),
-        6 to SurfaceOverlay(),
-        1 to SidewalkOverlay(),
-        5 to CyclewayOverlay(get(), get(named("CountryBoundariesFuture"))),
-        2 to StreetParkingOverlay(),
-        3 to AddressOverlay(get(named("CountryBoundariesFuture"))),
-        4 to ShopsOverlay(get(named("FeatureDictionaryFuture"))),
-    )) }
+    single {
+        overlaysRegistry(
+            get(),
+            get(named("CountryBoundariesFuture")),
+            { tags ->
+                get<FutureTask<FeatureDictionary>>(named("FeatureDictionaryFuture"))
+                .get().getFeature(tags)
+            }
+        )
+    }
 }
+
+fun overlaysRegistry(
+    countryInfos: CountryInfos,
+    countryBoundariesFuture: FutureTask<CountryBoundaries>,
+    getFeature: (tags: Map<String, String>) -> Feature?,
+) = OverlayRegistry(listOf(
+
+    0 to WayLitOverlay(),
+    6 to SurfaceOverlay(),
+    1 to SidewalkOverlay(),
+    5 to CyclewayOverlay(countryInfos, countryBoundariesFuture),
+    2 to StreetParkingOverlay(),
+    3 to AddressOverlay(countryBoundariesFuture),
+    4 to ShopsOverlay(getFeature),
+))
diff --git a/app/src/main/java/de/westnordost/streetcomplete/overlays/shops/ShopsOverlay.kt b/app/src/main/java/de/westnordost/streetcomplete/overlays/shops/ShopsOverlay.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.overlays.shops
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.osm.mapdata.Element
 import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
@@ -16,9 +16,8 @@ import de.westnordost.streetcomplete.quests.place_name.AddPlaceName
 import de.westnordost.streetcomplete.quests.shop_type.CheckShopType
 import de.westnordost.streetcomplete.quests.shop_type.SpecifyShopType
 import de.westnordost.streetcomplete.util.getNameLabel
-import java.util.concurrent.FutureTask
 
-class ShopsOverlay(private val featureDictionaryFuture: FutureTask<FeatureDictionary>) : Overlay {
+class ShopsOverlay(private val getFeature: (tags: Map<String, String>) -> Feature?) : Overlay {
 
     override val title = R.string.overlay_shops
     override val icon = R.drawable.ic_quest_shop
@@ -41,10 +40,7 @@ class ShopsOverlay(private val featureDictionaryFuture: FutureTask<FeatureDictio
         mapData
             .filter(IS_SHOP_OR_DISUSED_SHOP_EXPRESSION)
             .map { element ->
-                val feature = featureDictionaryFuture.get()
-                    .byTags(element.tags)
-                    .isSuggestion(false) // no suggestions because we just want the icon
-                    .find().firstOrNull()
+                val feature = getFeature(element.tags)
 
                 val icon = "ic_preset_" + (feature?.icon ?: "maki-shop" ).replace('-', '_')
                 val label = getNameLabel(element.tags)

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/QuestsModule.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/QuestsModule.kt
@@ -1,6 +1,7 @@
 package de.westnordost.streetcomplete.quests
 
 import de.westnordost.countryboundaries.CountryBoundaries
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.osmfeatures.FeatureDictionary
 import de.westnordost.streetcomplete.data.meta.CountryInfos
 import de.westnordost.streetcomplete.data.osmnotes.notequests.OsmNoteQuestType
@@ -167,6 +168,7 @@ import de.westnordost.streetcomplete.quests.wheelchair_access.AddWheelchairAcces
 import de.westnordost.streetcomplete.quests.width.AddCyclewayWidth
 import de.westnordost.streetcomplete.quests.width.AddRoadWidth
 import de.westnordost.streetcomplete.screens.measure.ArSupportChecker
+import de.westnordost.streetcomplete.util.ktx.getFeature
 import org.koin.core.qualifier.named
 import org.koin.dsl.module
 import java.util.concurrent.FutureTask
@@ -175,23 +177,28 @@ val questsModule = module {
     factory { RoadNameSuggestionsSource(get()) }
     factory { WayTrafficFlowDao(get()) }
 
-    single { questTypeRegistry(
-        get(),
-        get(),
-        get(named("FeatureDictionaryFuture")),
-        get(),
-        get(named("CountryBoundariesFuture")),
-        get(),
-    ) }
+    single {
+        questTypeRegistry(
+            get(),
+            get(),
+            get(),
+            get(named("CountryBoundariesFuture")),
+            get(),
+            { tags ->
+                get<FutureTask<FeatureDictionary>>(named("FeatureDictionaryFuture"))
+                    .get().getFeature(tags)
+            }
+        )
+    }
 }
 
 fun questTypeRegistry(
     trafficFlowSegmentsApi: TrafficFlowSegmentsApi,
     trafficFlowDao: WayTrafficFlowDao,
-    featureDictionaryFuture: FutureTask<FeatureDictionary>,
     countryInfos: CountryInfos,
     countryBoundariesFuture: FutureTask<CountryBoundaries>,
-    arSupportChecker: ArSupportChecker
+    arSupportChecker: ArSupportChecker,
+    getFeature: (tags: Map<String, String>) -> Feature?,
 ) = QuestTypeRegistry(listOf(
 
     /* The quest types are primarily sorted by how easy they can be solved:
@@ -246,7 +253,7 @@ fun questTypeRegistry(
     9 to AddCarWashType(),
 
     10 to AddBenchBackrest(),
-    11 to AddAmenityCover(featureDictionaryFuture),
+    11 to AddAmenityCover(getFeature),
 
     12 to AddBridgeStructure(),
 
@@ -322,7 +329,7 @@ fun questTypeRegistry(
     /* pulled up in priority to be before CheckExistence because this is basically the check
        whether the postbox is still there in countries in which it is enabled */
     48 to AddPostboxCollectionTimes(),
-    49 to CheckExistence(featureDictionaryFuture),
+    49 to CheckExistence(getFeature),
     155 to AddGritBinSeasonal(),
 
     50 to AddBoardType(),
@@ -372,9 +379,9 @@ fun questTypeRegistry(
     157 to AddHairdresserCustomers(), // almost always marked on sign outside
     78 to SpecifyShopType(), // above add place name as some brand presets will set the name too
     79 to CheckShopType(),
-    80 to AddPlaceName(featureDictionaryFuture),
-    77 to CheckOpeningHoursSigned(featureDictionaryFuture),
-    81 to AddOpeningHours(featureDictionaryFuture),
+    80 to AddPlaceName(getFeature),
+    77 to CheckOpeningHoursSigned(getFeature),
+    81 to AddOpeningHours(getFeature),
     83 to AddBicyclePump(), // visible from the outside, but only during opening hours
 
     84 to AddAtmOperator(),
@@ -459,7 +466,7 @@ fun questTypeRegistry(
     132 to AddAcceptsCash(),
 
     133 to AddFuelSelfService(),
-    156 to CheckShopExistence(featureDictionaryFuture), // after opening hours and similar so they will be preferred if enabled
+    156 to CheckShopExistence(getFeature), // after opening hours and similar so they will be preferred if enabled
 
     /* ↓ 5.quests that are very numerous ---------------------------------------------------- */
 

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_cover/AddAmenityCover.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_cover/AddAmenityCover.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.quests.amenity_cover
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
 import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
@@ -11,10 +11,9 @@ import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement.
 import de.westnordost.streetcomplete.osm.Tags
 import de.westnordost.streetcomplete.quests.YesNoQuestForm
 import de.westnordost.streetcomplete.util.ktx.toYesNo
-import java.util.concurrent.FutureTask
 
 class AddAmenityCover (
-    private val featureDictionaryFuture: FutureTask<FeatureDictionary>
+    private val getFeature: (tags: Map<String, String>) -> Feature?
 ) : OsmElementQuestType<Boolean> {
 
     private val nodesFilter by lazy { """
@@ -38,17 +37,12 @@ class AddAmenityCover (
     override fun isApplicableTo(element: Element) =
         nodesFilter.matches(element) && hasAnyName(element.tags)
 
-    private fun hasAnyName(tags: Map<String, String>): Boolean =
-        featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()
+    private fun hasAnyName(tags: Map<String, String>) = getFeature(tags) != null
 
     override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry): Sequence<Element> {
         /* put markers for objects that are exactly the same as for which this quest is asking for
            e.g. it's a ticket validator? -> display other ticket validators. Etc. */
-        val feature = featureDictionaryFuture.get()
-            .byTags(element.tags)
-            .isSuggestion(false) // not brands
-            .find()
-            .firstOrNull() ?: return emptySequence()
+        val feature = getFeature(element.tags) ?: return emptySequence()
 
         return getMapData().filter { it.tags.containsAll(feature.tags) }.asSequence()
     }

diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/existence/CheckExistence.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/existence/CheckExistence.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.quests.existence
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
 import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
@@ -12,10 +12,9 @@ import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement.
 import de.westnordost.streetcomplete.osm.LAST_CHECK_DATE_KEYS
 import de.westnordost.streetcomplete.osm.Tags
 import de.westnordost.streetcomplete.osm.updateCheckDate
-import java.util.concurrent.FutureTask
 
 class CheckExistence(
-    private val featureDictionaryFuture: FutureTask<FeatureDictionary>
+    private val getFeature: (tags: Map<String, String>) -> Feature?
 ) : OsmElementQuestType<Unit> {
 
     private val nodesFilter by lazy { """
@@ -102,12 +101,7 @@ class CheckExistence(
     override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry): Sequence<Element> {
         /* put markers for objects that are exactly the same as for which this quest is asking for
            e.g. it's a ticket validator? -> display other ticket validators. Etc. */
-        val feature = featureDictionaryFuture.get()
-            .byTags(element.tags)
-            .isSuggestion(false) // not brands
-            .find()
-            .firstOrNull() ?: return emptySequence()
-
+        val feature = getFeature(element.tags) ?: return emptySequence()
         return getMapData().filter { it.tags.containsAll(feature.tags) }.asSequence()
     }
 
@@ -122,8 +116,7 @@ class CheckExistence(
         or ${LAST_CHECK_DATE_KEYS.joinToString(" or ") { "$it < today -$yearsAgo years" }}
     """.trimIndent()
 
-    private fun hasAnyName(tags: Map<String, String>): Boolean =
-        featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()
+    private fun hasAnyName(tags: Map<String, String>) = getFeature(tags) != null
 }
 
 private fun <X, Y> Map<X, Y>.containsAll(other: Map<X, Y>) = other.all { this[it.key] == it.value }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.quests.opening_hours
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.elementfilter.filters.RelativeDate
 import de.westnordost.streetcomplete.data.elementfilter.filters.TagOlderThan
@@ -17,10 +17,9 @@ import de.westnordost.streetcomplete.osm.opening_hours.parser.isSupportedOpening
 import de.westnordost.streetcomplete.osm.opening_hours.parser.toOpeningHoursRules
 import de.westnordost.streetcomplete.osm.updateCheckDateForKey
 import de.westnordost.streetcomplete.osm.updateWithCheckDate
-import java.util.concurrent.FutureTask
 
 class AddOpeningHours(
-    private val featureDictionaryFuture: FutureTask<FeatureDictionary>
+    private val getFeature: (tags: Map<String, String>) -> Feature?
 ) : OsmElementQuestType<OpeningHoursAnswer> {
 
     /* See also AddWheelchairAccessBusiness and AddPlaceName, which has a similar list and is/should
@@ -187,6 +186,5 @@ class AddOpeningHours(
     private fun hasProperName(tags: Map<String, String>): Boolean =
         tags.containsKey("name") || tags.containsKey("brand")
 
-    private fun hasFeatureName(tags: Map<String, String>): Boolean =
-        featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()
+    private fun hasFeatureName(tags: Map<String, String>) = getFeature(tags) != null
 }
diff --git a/...java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt b/...java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.quests.opening_hours_signed
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
 import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
@@ -19,10 +19,9 @@ import de.westnordost.streetcomplete.quests.YesNoQuestForm
 import kotlinx.datetime.Instant
 import kotlinx.datetime.TimeZone
 import kotlinx.datetime.toLocalDateTime
-import java.util.concurrent.FutureTask
 
 class CheckOpeningHoursSigned(
-    private val featureDictionaryFuture: FutureTask<FeatureDictionary>
+    private val getFeature: (tags: Map<String, String>) -> Feature?
 ) : OsmElementQuestType<Boolean> {
 
     private val filter by lazy { """
@@ -92,6 +91,5 @@ class CheckOpeningHoursSigned(
     private fun hasProperName(tags: Map<String, String>): Boolean =
         tags.containsKey("name") || tags.containsKey("brand")
 
-    private fun hasFeatureName(tags: Map<String, String>): Boolean =
-        featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()
+    private fun hasFeatureName(tags: Map<String, String>) = getFeature(tags) != null
 }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/place_name/AddPlaceName.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/place_name/AddPlaceName.kt
@@ -1,6 +1,6 @@
 package de.westnordost.streetcomplete.quests.place_name
 
-import de.westnordost.osmfeatures.FeatureDictionary
+import de.westnordost.osmfeatures.Feature
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
 import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
@@ -12,10 +12,9 @@ import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement.
 import de.westnordost.streetcomplete.osm.IS_SHOP_OR_DISUSED_SHOP_EXPRESSION
 import de.westnordost.streetcomplete.osm.Tags
 import de.westnordost.streetcomplete.osm.applyTo
-import java.util.concurrent.FutureTask
 
 class AddPlaceName(
-    private val featureDictionaryFuture: FutureTask<FeatureDictionary>
+    private val getFeature: (tags: Map<String, String>) -> Feature?
 ) : OsmElementQuestType<PlaceNameAnswer> {
 
     private val filter by lazy { ("""
@@ -142,6 +141,5 @@ class AddPlaceName(
         }
     }
 
-    private fun hasFeatureName(tags: Map<String, String>): Boolean =
-        featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()
+    private fun hasFeatureName(tags: Map<String, String>) = getFeature(tags) != null
 }