Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better deal with bad OSM data for OH #3719

Merged
merged 12 commits into from
Feb 11, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class AddOpeningHours(

/* See also AddWheelchairAccessBusiness and AddPlaceName, which has a similar list and is/should
be ordered in the same way for better overview */
private val filter by lazy { ("""
matkoniecz marked this conversation as resolved.
Show resolved Hide resolved
val filter by lazy { ("""
nodes, ways, relations with
(
(
Expand Down Expand Up @@ -93,12 +93,25 @@ class AddOpeningHours(
),
).map { it.key + " ~ " + it.value.joinToString("|") }.joinToString("\n or ") + "\n" + """
)
and !opening_hours
and (!opening_hours or opening_hours older today -1 years)
)
or
(
opening_hours older today -1 years
and
(
leisure=park
matkoniecz marked this conversation as resolved.
Show resolved Hide resolved
or
barrier
or
amenity=toilets
or
amenity=bicycle_rental
)
westnordost marked this conversation as resolved.
Show resolved Hide resolved
)
or opening_hours older today -1 years
)
and access !~ private|no
and (name or brand or noname = yes or name:signed = no or amenity=recycling)
and (name or brand or noname = yes or name:signed = no or amenity=recycling or amenity=toilets)
westnordost marked this conversation as resolved.
Show resolved Hide resolved
and opening_hours:signed != no
""").toElementFilterExpression() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ import de.westnordost.streetcomplete.data.meta.toCheckDate
import de.westnordost.streetcomplete.data.meta.toCheckDateString
import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd
import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryModify
import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
import de.westnordost.streetcomplete.ktx.toEpochMilli
import de.westnordost.streetcomplete.osm.opening_hours.parser.OpeningHoursRuleList
import de.westnordost.streetcomplete.quests.verifyAnswer
import de.westnordost.streetcomplete.testutils.mock
import de.westnordost.streetcomplete.testutils.node
import de.westnordost.streetcomplete.data.osm.mapdata.Node
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import java.lang.System.currentTimeMillis
import java.time.LocalDate

class AddOpeningHoursTest {
Expand Down Expand Up @@ -160,6 +163,60 @@ class AddOpeningHoursTest {
)))
}

@Test fun `isApplicableTo returns false for known places with recently edited opening hours`() {
assertFalse(questType.isApplicableTo(
node(tags = mapOf("shop" to "sports", "name" to "Atze's Angelladen", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis())
))
}

@Test fun `isApplicableTo returns true for known places with old opening hours`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
matkoniecz marked this conversation as resolved.
Show resolved Hide resolved
assertTrue(questType.isApplicableTo(
node(tags = mapOf("shop" to "sports", "name" to "Atze's Angelladen", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns false for closed shops with old opening hours`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
assertFalse(questType.isApplicableTo(
node(tags = mapOf("nonexisting:shop" to "sports", "name" to "Atze's Angelladen", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns true for parks with old opening hours`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
assertTrue(questType.isApplicableTo(
node(tags = mapOf("leisure" to "park", "name" to "Trolololo", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns true for unnamed battery recycling with old opening hours`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
assertTrue(questType.filter.matches(
node(tags = mapOf("amenity" to "recycling", "recycling:batteries" to "yes", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns true for unnamed toilets with old opening hours`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
assertTrue(questType.filter.matches(
node(tags = mapOf("amenity" to "toilets", "opening_hours" to "Mo-Fr 10:00-20:00"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns true for unnamed toilets with old opening hours WAT`() {
val milisecondsFor400Days : Long = 34560000000 // 1000 * 60 * 60 * 24 * 400
assertTrue(questType.filter.matches(
node(tags = mapOf("amenity" to "toilets", "opening_hours" to "Mo-Fr 10:00-20:00", "noname" to "yes"), timestamp = currentTimeMillis() - milisecondsFor400Days)
))
}

@Test fun `isApplicableTo returns false for toilets without opening hours`() {
assertFalse(questType.isApplicableTo(
node(tags = mapOf("amenity" to "toilets"), timestamp = currentTimeMillis())
))
}

@Test fun `isApplicableTo returns true if the opening hours cannot be parsed`() {
assertTrue(questType.isApplicableTo(node(
tags = mapOf(
Expand Down