diff --git a/.editorconfig b/.editorconfig index 7057494e882..c0cb766f2e0 100644 --- a/.editorconfig +++ b/.editorconfig @@ -24,7 +24,6 @@ ktlint_standard_backing-property-naming = disabled ktlint_standard_blank-line-before-declaration = disabled ktlint_standard_blank-line-between-when-conditions = disabled ktlint_standard_chain-wrapping = disabled -ktlint_standard_comment-wrapping = disabled ktlint_standard_function-naming = disabled ktlint_standard_function-signature = disabled ktlint_standard_indent = disabled @@ -47,8 +46,6 @@ ktlint_standard_statement-wrapping = disabled ktlint_standard_string-template-indent = disabled ktlint_standard_trailing-comma-on-call-site = disabled ktlint_standard_trailing-comma-on-declaration-site = disabled -ktlint_standard_value-argument-comment = disabled -ktlint_standard_value-parameter-comment = disabled ktlint_standard_wrapping = disabled # enable some experimental Ktlint rules diff --git a/CHANGELOG.md b/CHANGELOG.md index efa32644a81..05c02728223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ you map places that are not shops - like hotels, hospitals, schools and so forth ### Other -- Fix a (shokingly common) crash (#5498) +- Fix a (shockingly common) crash (#5498) - Fix issues with doing edits while data is being downloaded (#4258) - Add some interesting links as achievement rewards (#5466) - Translation to Amharic has been disabled, it was not maintained for over a year diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 35a7326ba0d..54634c657e7 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -154,6 +154,11 @@ dependencies { // scheduling background jobs implementation("androidx.work:work-runtime-ktx:2.9.0") + // HTTP Client + implementation("io.ktor:ktor-client-core:2.3.8") + implementation("io.ktor:ktor-client-cio:2.3.8") + testImplementation("io.ktor:ktor-client-mock:2.3.8") + // finding in which country we are for country-specific logic implementation("de.westnordost:countryboundaries:2.1") // finding a name for a feature without a name tag diff --git a/app/proguard-rules.pro b/app/proguard-rules.pro index 8371cb97fd0..084bd0320fd 100644 --- a/app/proguard-rules.pro +++ b/app/proguard-rules.pro @@ -5,7 +5,6 @@ # let's just keep everything -keep class com.mapzen.tangram.** { *; } -keep class com.mapzen.tangram.* { *; } - # tangram end -------------------------------------------------------------------------------------- # Lifecycle @@ -47,6 +46,9 @@ kotlinx.serialization.KSerializer serializer(...); } +# ktor client, see https://youtrack.jetbrains.com/issue/KTOR-5528 +-dontwarn org.slf4j.impl.StaticLoggerBinder + # Change here com.yourcompany.yourpackage -keep,includedescriptorclasses class de.westnordost.streetcomplete.**$$serializer { *; } -keepclassmembers class de.westnordost.streetcomplete.** { diff --git a/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/edits/EditElementsDaoTest.kt b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/edits/EditElementsDaoTest.kt index 35670e1b656..6322c47e495 100644 --- a/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/edits/EditElementsDaoTest.kt +++ b/app/src/androidTest/java/de/westnordost/streetcomplete/data/osm/edits/EditElementsDaoTest.kt @@ -31,8 +31,8 @@ class EditElementsDaoTest : ApplicationDbTestCase() { )) dao.put(7, listOf( - ElementKey(ElementType.NODE, 0), // referring to same element - ElementKey(ElementType.WAY, 1), // but also another + ElementKey(ElementType.NODE, 0), // referring to same element + ElementKey(ElementType.WAY, 1), // but also another )) dao.put(3, listOf( diff --git a/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.kt index 4f2132de563..03904a97144 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/ApplicationModule.kt @@ -5,6 +5,9 @@ import android.content.res.Resources import de.westnordost.streetcomplete.util.CrashReportExceptionHandler import de.westnordost.streetcomplete.util.SoundFx import de.westnordost.streetcomplete.util.logs.DatabaseLogger +import io.ktor.client.HttpClient +import io.ktor.client.plugins.defaultRequest +import io.ktor.http.userAgent import org.koin.android.ext.koin.androidContext import org.koin.dsl.module @@ -15,4 +18,9 @@ val appModule = module { single { CrashReportExceptionHandler(androidContext(), get(), get(), "helium@vivaldi.net", "crashreport.txt") } single { DatabaseLogger(get()) } single { SoundFx(androidContext()) } + single { HttpClient { + defaultRequest { + userAgent(ApplicationConstants.USER_AGENT) + } + } } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/StreetCompleteApplication.kt b/app/src/main/java/de/westnordost/streetcomplete/StreetCompleteApplication.kt index 79a142c97a4..e35ef583112 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/StreetCompleteApplication.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/StreetCompleteApplication.kt @@ -150,7 +150,7 @@ class StreetCompleteApplication : Application() { preloader.preload() } - /* Force logout users who are logged in with OAuth 1.0a, they need to re-authenticate with OAuth 2 */ + // Force logout users who are logged in with OAuth 1.0a, they need to re-authenticate with OAuth 2 if (prefs.getStringOrNull(Prefs.OAUTH1_ACCESS_TOKEN) != null) { userLoginStatusController.logOut() } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/Cleaner.kt b/app/src/main/java/de/westnordost/streetcomplete/data/Cleaner.kt index 74004d6529c..ec6012bd8bb 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/Cleaner.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/Cleaner.kt @@ -28,7 +28,7 @@ class Cleaner( noteController.deleteOlderThan(oldDataTimestamp, MAX_DELETE_ELEMENTS) mapDataController.deleteOlderThan(oldDataTimestamp, MAX_DELETE_ELEMENTS) downloadedTilesController.deleteOlderThan(oldDataTimestamp) - /* do this after cleaning map data and notes, because some metadata rely on map data */ + // do this after cleaning map data and notes, because some metadata rely on map data questTypeRegistry.forEach { it.deleteMetadataOlderThan(oldDataTimestamp) } val oldLogTimestamp = nowAsEpochMilliseconds() - ApplicationConstants.DELETE_OLD_LOG_AFTER diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/DatabaseInitializer.kt b/app/src/main/java/de/westnordost/streetcomplete/data/DatabaseInitializer.kt index 497a9bda571..dd8f20c8dd7 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/DatabaseInitializer.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/DatabaseInitializer.kt @@ -29,7 +29,7 @@ import de.westnordost.streetcomplete.quests.oneway_suspects.data.WayTrafficFlowT /** Creates the database and upgrades it */ object DatabaseInitializer { - const val DB_VERSION = 15 + const val DB_VERSION = 16 fun onCreate(db: Database) { // OSM notes @@ -243,9 +243,22 @@ object DatabaseInitializer { if (oldVersion <= 14 && newVersion > 14) { db.renameOverlay("ShopsOverlay", "PlacesOverlay") } + if (oldVersion <= 15 && newVersion > 15) { + db.deleteQuest("AddCrossingType") + } } } +private fun Database.deleteQuest(name: String) { + deleteValue(ElementEditsTable.NAME, ElementEditsTable.Columns.QUEST_TYPE, name) + deleteValue(OsmQuestTable.NAME, OsmQuestTable.Columns.QUEST_TYPE, name) + deleteValue(OsmQuestsHiddenTable.NAME, OsmQuestsHiddenTable.Columns.QUEST_TYPE, name) + deleteValue(VisibleQuestTypeTable.NAME, VisibleQuestTypeTable.Columns.QUEST_TYPE, name) + deleteValue(OpenChangesetsTable.NAME, OpenChangesetsTable.Columns.QUEST_TYPE, name) + deleteValue(QuestTypeOrderTable.NAME, QuestTypeOrderTable.Columns.BEFORE, name) + deleteValue(QuestTypeOrderTable.NAME, QuestTypeOrderTable.Columns.AFTER, name) +} + private fun Database.renameQuest(old: String, new: String) { renameValue(ElementEditsTable.NAME, ElementEditsTable.Columns.QUEST_TYPE, old, new) renameValue(OsmQuestTable.NAME, OsmQuestTable.Columns.QUEST_TYPE, old, new) @@ -264,3 +277,7 @@ private fun Database.renameOverlay(old: String, new: String) { private fun Database.renameValue(table: String, column: String, oldValue: String, newValue: String) { update(table, listOf(column to newValue), "$column = ?", arrayOf(oldValue)) } + +private fun Database.deleteValue(table: String, column: String, value: String) { + delete(table, "$column = ?", arrayOf(value)) +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFilterExpression.kt b/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFilterExpression.kt index 82c51440201..2f95122afac 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFilterExpression.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFilterExpression.kt @@ -70,7 +70,7 @@ class ElementFilterExpression( internal val elementsTypes: Set, internal val elementExprRoot: BooleanExpression? ) { - /* Performance improvement: Allows to skip early on elements that have no tags at all */ + // Performance improvement: Allows to skip early on elements that have no tags at all val mayEvaluateToTrueWithNoTags = elementExprRoot?.mayEvaluateToTrueWithNoTags ?: true /** returns whether the given element is found through (=matches) this expression */ diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFiltersParser.kt b/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFiltersParser.kt index 44a5da73c87..4feed50eef5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFiltersParser.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/elementfilter/ElementFiltersParser.kt @@ -316,7 +316,7 @@ private fun StringWithCursor.parseQuotedWord(quot: Char): String? { val word = advanceBy(length) return word .substring(1, word.length - 1) // remove quotes - .replace("\\$quot", "$quot") // unescape quotes within string + .replace("\\$quot", "$quot") // unescape quotes within string } private fun StringWithCursor.parseWord(): String { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/location/RecentLocationStore.kt b/app/src/main/java/de/westnordost/streetcomplete/data/location/RecentLocationStore.kt index 51184f7568d..30c64b405a1 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/location/RecentLocationStore.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/location/RecentLocationStore.kt @@ -1,9 +1,11 @@ package de.westnordost.streetcomplete.data.location import android.location.Location +import de.westnordost.streetcomplete.util.ktx.elapsedDuration import de.westnordost.streetcomplete.util.ktx.toLatLon import de.westnordost.streetcomplete.util.math.flatDistanceTo -import kotlin.math.abs +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds class RecentLocationStore { private val recentLocations = ArrayDeque() @@ -17,7 +19,7 @@ class RecentLocationStore { previousLocation = it return@filter true } - if (abs(it.elapsedRealtimeNanos - loc.elapsedRealtimeNanos) > LOCATION_MIN_TIME_DIFFERENCE_NANOS + if ((it.elapsedDuration - loc.elapsedDuration).absoluteValue > LOCATION_MIN_TIME_DIFFERENCE && loc.toLatLon().flatDistanceTo(it.toLatLon()) >= MAX_DISTANCE_TO_ELEMENT_FOR_SURVEY / 2 ) { previousLocation = it @@ -30,13 +32,15 @@ class RecentLocationStore { fun add(location: Location) = synchronized(recentLocations) { while (recentLocations.isNotEmpty() - && recentLocations.first().elapsedRealtimeNanos <= location.elapsedRealtimeNanos - LOCATION_STORE_TIME_NANOS + && recentLocations.first().elapsedDuration <= location.elapsedDuration - LOCATION_STORE_TIME ) { recentLocations.removeFirst() } recentLocations.add(location) } -} -private const val LOCATION_STORE_TIME_NANOS = 600 * 1000 * 1000 * 1000L // 10 min -private const val LOCATION_MIN_TIME_DIFFERENCE_NANOS = 5 * 1000 * 1000 * 1000L // 5 sec + companion object { + private val LOCATION_STORE_TIME = 10.minutes + private val LOCATION_MIN_TIME_DIFFERENCE = 5.seconds + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt index 11992a92d5b..354ecbeaaa3 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt @@ -284,7 +284,7 @@ class MapDataWithEditsSource internal constructor( val ids = way.nodeIds.toHashSet() val nodes = getNodes(ids) - /* If the way is (now) not complete, this is not acceptable */ + // If the way is (now) not complete, this is not acceptable if (nodes.size < ids.size) { Log.w(TAG, "could not find nodes ${ids - nodes.map { it.id }} for way $way") return null @@ -324,7 +324,7 @@ class MapDataWithEditsSource internal constructor( private fun getRelationElements(relation: Relation): MutableMapData = synchronized(this) { val elements = ArrayList() for (member in relation.members) { - /* for way members, also get their nodes */ + // for way members, also get their nodes if (member.type == WAY) { val wayComplete = getWayComplete(member.ref) if (wayComplete != null) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloader.kt index 2f5aa0ba454..2eb939723d1 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloader.kt @@ -3,19 +3,23 @@ package de.westnordost.streetcomplete.data.osmnotes import de.westnordost.osmapi.user.UserApi import de.westnordost.streetcomplete.util.ktx.format import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds -import de.westnordost.streetcomplete.util.ktx.saveToFile import de.westnordost.streetcomplete.util.logs.Log +import io.ktor.client.HttpClient +import io.ktor.client.plugins.expectSuccess +import io.ktor.client.request.get +import io.ktor.client.statement.bodyAsChannel +import io.ktor.util.cio.writeChannel +import io.ktor.utils.io.copyAndClose import java.io.File -import java.io.IOException -import java.net.URL /** Downloads and stores the OSM avatars of users */ class AvatarsDownloader( + private val httpClient: HttpClient, private val userApi: UserApi, private val cacheDir: File ) { - fun download(userIds: Collection) { + suspend fun download(userIds: Collection) { if (!ensureCacheDirExists()) { Log.w(TAG, "Unable to create directories for avatars") return @@ -41,13 +45,16 @@ class AvatarsDownloader( } /** download avatar for the given user and a known avatar url */ - fun download(userId: Long, avatarUrl: String) { + suspend fun download(userId: Long, avatarUrl: String) { if (!ensureCacheDirExists()) return + val avatarFile = File(cacheDir, "$userId") try { - val avatarFile = File(cacheDir, "$userId") - URL(avatarUrl).saveToFile(avatarFile) + val response = httpClient.get(avatarUrl) { + expectSuccess = true + } + response.bodyAsChannel().copyAndClose(avatarFile.writeChannel()) Log.d(TAG, "Downloaded file: ${avatarFile.path}") - } catch (e: IOException) { + } catch (e: Exception) { Log.w(TAG, "Unable to download avatar for user id $userId") } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsInNotesUpdater.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsInNotesUpdater.kt index aecde11a3ce..98fc8b984b7 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsInNotesUpdater.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsInNotesUpdater.kt @@ -1,4 +1,5 @@ package de.westnordost.streetcomplete.data.osmnotes +import kotlinx.coroutines.runBlocking class AvatarsInNotesUpdater(private val downloader: AvatarsDownloader) : NoteController.Listener { @@ -7,7 +8,7 @@ class AvatarsInNotesUpdater(private val downloader: AvatarsDownloader) : if (added.isEmpty() && updated.isEmpty()) return val noteCommentUserIds = (added + updated).flatMap { it.userIds }.toSet() - downloader.download(noteCommentUserIds) + runBlocking { downloader.download(noteCommentUserIds) } } override fun onCleared() {} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt index 85f1c4fc2f4..183a8082929 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt @@ -8,11 +8,11 @@ import java.io.File val notesModule = module { factory(named("AvatarsCacheDirectory")) { File(get().cacheDir, ApplicationConstants.AVATARS_CACHE_DIRECTORY) } - factory { AvatarsDownloader(get(), get(named("AvatarsCacheDirectory"))) } + factory { AvatarsDownloader(get(), get(), get(named("AvatarsCacheDirectory"))) } factory { AvatarsInNotesUpdater(get()) } factory { NoteDao(get()) } factory { NotesDownloader(get(), get()) } - factory { StreetCompleteImageUploader(ApplicationConstants.SC_PHOTO_SERVICE_URL) } + factory { StreetCompleteImageUploader(get(), ApplicationConstants.SC_PHOTO_SERVICE_URL) } single { NoteController(get()).apply { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt index a7c83657da1..5aa5ddb1849 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploader.kt @@ -1,16 +1,22 @@ package de.westnordost.streetcomplete.data.osmnotes -import de.westnordost.streetcomplete.ApplicationConstants import de.westnordost.streetcomplete.data.download.ConnectionException +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.request.header +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.http.ContentType +import io.ktor.http.HttpStatusCode +import io.ktor.http.contentType +import io.ktor.http.defaultForFile +import io.ktor.util.cio.readChannel +import io.ktor.utils.io.errors.IOException import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.SerializationException import kotlinx.serialization.json.Json import java.io.File -import java.io.IOException -import java.net.HttpURLConnection -import java.net.URL -import java.net.URLConnection @Serializable private data class PhotoUploadResponse( @@ -19,9 +25,12 @@ private data class PhotoUploadResponse( ) /** Upload and activate a list of image paths to an instance of the - * StreetComplete image hosting service + * StreetComplete image hosting service */ -class StreetCompleteImageUploader(private val baseUrl: String) { +class StreetCompleteImageUploader( + private val httpClient: HttpClient, + private val baseUrl: String +) { private val json = Json { ignoreUnknownKeys = true } @@ -31,7 +40,7 @@ class StreetCompleteImageUploader(private val baseUrl: String) { * @throws ImageUploadServerException when there was a server error on upload (server error) * @throws ImageUploadClientException when the server rejected the upload request (client error) * @throws ConnectionException if it is currently not reachable (no internet etc) */ - fun upload(imagePaths: List): List { + suspend fun upload(imagePaths: List): List { val imageLinks = ArrayList() for (path in imagePaths) { @@ -39,35 +48,28 @@ class StreetCompleteImageUploader(private val baseUrl: String) { if (!file.exists()) continue try { - val connection = createConnection("upload.php") - connection.requestMethod = "POST" - connection.setRequestProperty("Content-Type", URLConnection.guessContentTypeFromName(file.path)) - connection.setRequestProperty("Content-Transfer-Encoding", "binary") - connection.setRequestProperty("Content-Length", file.length().toString()) - connection.outputStream.use { output -> - file.inputStream().use { input -> - input.copyTo(output) - } + val response = httpClient.post(baseUrl + "upload.php") { + contentType(ContentType.defaultForFile(file)) + header("Content-Transfer-Encoding", "binary") + setBody(file.readChannel()) } - val status = connection.responseCode - if (status == HttpURLConnection.HTTP_OK) { - val response = connection.inputStream.bufferedReader().use { it.readText() } + val status = response.status + val body = response.body() + if (status == HttpStatusCode.OK) { try { - val parsedResponse = json.decodeFromString(response) + val parsedResponse = json.decodeFromString(body) imageLinks.add(parsedResponse.futureUrl) } catch (e: SerializationException) { - throw ImageUploadServerException("Upload Failed: Unexpected response \"$response\"") + throw ImageUploadServerException("Upload Failed: Unexpected response \"$body\"") } } else { - val error = connection.errorStream.bufferedReader().use { it.readText() } - if (status / 100 == 5) { - throw ImageUploadServerException("Upload failed: Error code $status, Message: \"$error\"") + if (status.value / 100 == 5) { + throw ImageUploadServerException("Upload failed: Error code $status, Message: \"$body\"") } else { - throw ImageUploadClientException("Upload failed: Error code $status, Message: \"$error\"") + throw ImageUploadClientException("Upload failed: Error code $status, Message: \"$body\"") } } - connection.disconnect() } catch (e: IOException) { throw ConnectionException("Upload failed", e) } @@ -80,39 +82,29 @@ class StreetCompleteImageUploader(private val baseUrl: String) { * @throws ImageUploadServerException when there was a server error on upload (server error) * @throws ImageUploadClientException when the server rejected the upload request (client error) * @throws ConnectionException if it is currently not reachable (no internet etc) */ - fun activate(noteId: Long) { + suspend fun activate(noteId: Long) { try { - val connection = createConnection("activate.php") - connection.requestMethod = "POST" - connection.setRequestProperty("Content-Type", "Content-Type: application/json") - connection.outputStream.bufferedWriter().use { it.write("{\"osm_note_id\": $noteId}") } + val response = httpClient.post(baseUrl + "activate.php") { + contentType(ContentType.Application.Json) + setBody("{\"osm_note_id\": $noteId}") + } - val status = connection.responseCode - if (status == HttpURLConnection.HTTP_GONE) { + val status = response.status + if (status == HttpStatusCode.Gone) { // it's gone if the note does not exist anymore. That's okay, it should only fail // if we might want to try again later. - } else if (status != HttpURLConnection.HTTP_OK) { - val error = connection.errorStream.bufferedReader().use { it.readText() } - if (status / 100 == 5) { + } else if (status != HttpStatusCode.OK) { + val error = response.body() + if (status.value / 100 == 5) { throw ImageUploadServerException("Error code $status, Message: \"$error\"") } else { throw ImageUploadClientException("Error code $status, Message: \"$error\"") } } - connection.disconnect() } catch (e: IOException) { throw ConnectionException("", e) } } - - private fun createConnection(url: String): HttpURLConnection { - val connection = URL(baseUrl + url).openConnection() as HttpURLConnection - connection.useCaches = false - connection.doOutput = true - connection.doInput = true - connection.setRequestProperty("User-Agent", ApplicationConstants.USER_AGENT) - return connection - } } class ImageUploadServerException(message: String? = null, cause: Throwable? = null) : diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt index 144022498f9..e21210bc3ca 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt @@ -14,6 +14,7 @@ import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener import de.westnordost.streetcomplete.data.user.UserDataSource import de.westnordost.streetcomplete.util.ktx.truncate import de.westnordost.streetcomplete.util.logs.Log +import io.ktor.http.encodeURLPathPart import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -21,7 +22,6 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext -import java.net.URLEncoder class NoteEditsUploader( private val noteEditsController: NoteEditsController, @@ -51,7 +51,7 @@ class NoteEditsUploader( private suspend fun uploadMissedImageActivations() { while (true) { val edit = noteEditsController.getOldestNeedingImagesActivation() ?: break - /* see uploadEdits */ + // see uploadEdits withContext(scope.coroutineContext) { imageUploader.activate(edit.noteId) noteEditsController.markImagesActivated(edit.id) @@ -69,7 +69,7 @@ class NoteEditsUploader( } } - private fun uploadEdit(edit: NoteEdit) { + private suspend fun uploadEdit(edit: NoteEdit) { // try to upload the image and track if we have them val imageText = uploadAndGetAttachedPhotosText(edit.imagePaths) val trackText = uploadAndGetAttachedTrackText(edit.track, edit.text) @@ -118,7 +118,7 @@ class NoteEditsUploader( } } - private fun uploadAndGetAttachedPhotosText(imagePaths: List): String { + private suspend fun uploadAndGetAttachedPhotosText(imagePaths: List): String { if (imagePaths.isNotEmpty()) { val urls = imageUploader.upload(imagePaths) if (urls.isNotEmpty()) { @@ -134,7 +134,7 @@ class NoteEditsUploader( ): String { if (trackpoints.isEmpty()) return "" val trackId = tracksApi.create(trackpoints, noteText?.truncate(255)) - val encodedUsername = URLEncoder.encode(userDataSource.userName, "utf-8").replace("+", "%20") + val encodedUsername = userDataSource.userName!!.encodeURLPathPart() return "\n\nGPS Trace: https://www.openstreetmap.org/user/$encodedUsername/traces/$trackId\n" } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/UploadModule.kt b/app/src/main/java/de/westnordost/streetcomplete/data/upload/UploadModule.kt index 6a417c92d66..137eb6d3c33 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/upload/UploadModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/upload/UploadModule.kt @@ -8,7 +8,7 @@ import org.koin.core.qualifier.named import org.koin.dsl.module val uploadModule = module { - factory { VersionIsBannedChecker(BANNED_VERSION_URL, ApplicationConstants.USER_AGENT) } + factory { VersionIsBannedChecker(get(), BANNED_VERSION_URL, ApplicationConstants.USER_AGENT) } single { Uploader(get(), get(), get(), get(), get(), get(named("SerializeSync")), get(), get()) } /* uploading and downloading should be serialized, i.e. may not run in parallel, to avoid diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/Uploader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/upload/Uploader.kt index 90c370d5b7f..34ed534b4a4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/upload/Uploader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/upload/Uploader.kt @@ -33,7 +33,7 @@ class Uploader( private val listeners = Listeners() - private val bannedInfo by lazy { versionIsBannedChecker.get() } + private lateinit var bannedInfo: BannedInfo private val uploadedChangeRelay = object : OnUploadedChangeListener { override fun onUploaded(questType: String, at: LatLon) { @@ -58,7 +58,11 @@ class Uploader( try { isUploadInProgress = true listeners.forEach { it.onStarted() } - val banned = withContext(Dispatchers.IO) { bannedInfo } + + if (!::bannedInfo.isInitialized) { + bannedInfo = withContext(Dispatchers.IO) { versionIsBannedChecker.get() } + } + val banned = bannedInfo if (banned is IsBanned) { throw VersionBannedException(banned.reason) } else if (banned is UnknownIfBanned) { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedChecker.kt b/app/src/main/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedChecker.kt index 588d5fb45f9..dab553559e6 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedChecker.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedChecker.kt @@ -1,30 +1,33 @@ package de.westnordost.streetcomplete.data.upload -import java.io.IOException -import java.net.HttpURLConnection -import java.net.URL +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.plugins.expectSuccess +import io.ktor.client.request.get /** Asks remote server if this version of the app is banned */ -class VersionIsBannedChecker(private val url: String, private val userAgent: String) { +class VersionIsBannedChecker( + private val httpClient: HttpClient, + private val url: String, + private val userAgent: String +) { - fun get(): BannedInfo { - var connection: HttpURLConnection? = null + suspend fun get(): BannedInfo { try { - connection = (URL(url).openConnection() as HttpURLConnection) - connection.inputStream.bufferedReader().use { reader -> - for (line in reader.lineSequence()) { - val text = line.split("\t".toRegex()) - if (text[0] == userAgent) { - return IsBanned(if (text.size > 1) text[1] else null) - } + val response = httpClient.get(url) { + expectSuccess = true + } + val bannedVersions = response.body() + for (bannedVersion in bannedVersions.lines()) { + val destructuredVersion = bannedVersion.split("\t") + if (destructuredVersion[0] == userAgent) { + return IsBanned(if (destructuredVersion.size > 1) destructuredVersion[1] else null) } } - } catch (e: IOException) { + } catch (e: Exception) { // if there is an io exception, never mind then...! (The unreachability of the above // internet address should not lead to this app being unusable!) return UnknownIfBanned - } finally { - connection?.disconnect() } return IsNotBanned } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/urlconfig/UrlConfig.kt b/app/src/main/java/de/westnordost/streetcomplete/data/urlconfig/UrlConfig.kt index 3f797c32809..b90f7ada333 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/urlconfig/UrlConfig.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/urlconfig/UrlConfig.kt @@ -4,8 +4,8 @@ import de.westnordost.streetcomplete.data.overlays.OverlayRegistry import de.westnordost.streetcomplete.data.quest.QuestType import de.westnordost.streetcomplete.data.quest.QuestTypeRegistry import de.westnordost.streetcomplete.overlays.Overlay -import java.net.URLDecoder -import java.net.URLEncoder +import io.ktor.http.decodeURLQueryComponent +import io.ktor.http.encodeURLQueryComponent data class UrlConfig( val presetName: String?, @@ -45,7 +45,7 @@ fun parseConfigUrl( keyValue[0].lowercase() to keyValue[1] } - val name = parameters[PARAM_NAME]?.let { URLDecoder.decode(it, "UTF-8") } + val name = parameters[PARAM_NAME]?.let { it.decodeURLQueryComponent(plusIsSpace = true) } val questTypesString = parameters[PARAM_QUESTS] ?: return null val questTypes = stringToQuestTypes(questTypesString, questTypeRegistry) ?: return null @@ -79,7 +79,7 @@ fun createConfigUrl( val name = urlConfig.presetName if (name != null) { val shortenedName = if (name.length > 60) name.substring(0, 57) + "..." else name - parameters.add(PARAM_NAME to URLEncoder.encode(shortenedName, "UTF-8")) + parameters.add(PARAM_NAME to shortenedName.encodeURLQueryComponent(spaceToPlus = true)) } parameters.add(PARAM_QUESTS to questTypesToString(urlConfig.questTypes, questTypeRegistry)) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/UserModule.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/UserModule.kt index e05e6f340ef..6827f32ff98 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/user/UserModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/user/UserModule.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.data.user +import de.westnordost.streetcomplete.data.user.oauth.OAuthService import org.koin.dsl.module const val OAUTH2_TOKEN_URL = "https://www.openstreetmap.org/oauth2/token" @@ -42,4 +43,6 @@ val userModule = module { single { UserLoginStatusController(get(), get()) } single { UserUpdater(get(), get(), get(), get(), get()) } + + single { OAuthService(get()) } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorization.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorization.kt deleted file mode 100644 index b3af4291485..00000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorization.kt +++ /dev/null @@ -1,198 +0,0 @@ -package de.westnordost.streetcomplete.data.user.oauth - -import de.westnordost.streetcomplete.ApplicationConstants -import kotlinx.serialization.Serializable -import kotlinx.serialization.SerializationException -import kotlinx.serialization.Transient -import kotlinx.serialization.json.Json -import java.io.IOException -import java.net.HttpURLConnection -import java.net.URI -import java.net.URL -import java.net.URLDecoder -import java.net.URLEncoder -import java.nio.charset.StandardCharsets -import java.security.MessageDigest -import kotlin.io.encoding.Base64 -import kotlin.io.encoding.ExperimentalEncodingApi - -/** - * One authorization with OAuth. For each authorization request, a new instance must be created - * - * Authorization flow: - * - * 1. createAuthorizationUrl() and open it in a browser (or web view) - * 2. let user accept or deny the permission request in the browser (or web view). Authorization - * endpoint will call the redirect URI (aka callback URI) with parameters in either case. - * 3. check if the URI received is for this instance with itsForMe(uri) and then - * extractAuthorizationCode(uri) from that URI - * 4. retrieveAccessToken(authorizationCode) with the retrieved authorizationCode - * - * - * @param authorizationUrl the OAuth2 server authorization endpoint - * @param accessTokenUrl the OAuth2 server token endpoint - * @param clientId the OAuth2 client id - * @param scopes the scopes (aka permissions) to request - * @param redirectUri the redirect URI (aka callback URI) that the authorization endpoint should - * call when the user allowed the authorization. - * @param state optional string to identify this oauth authorization flow, in case there are - * several at once (using the same redirect URI) - */ -@Serializable class OAuthAuthorization( - private val authorizationUrl: String, - private val accessTokenUrl: String, - private val clientId: String, - private val scopes: List, - private val redirectUri: String, - private val state: String? = null -) { - /** - * For the code challenge as specified in RFC 7636 - * https://www.rfc-editor.org/rfc/rfc7636 - * - * and required in the OAuth 2.1 draft - * https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-09 - */ - private val codeVerifier: String = createRandomAlphanumericString(128) - - @Transient - private val json = Json { ignoreUnknownKeys = true } - - /** - * Creates the URL to be opened in the browser or a web view in which the user agrees to - * authorize the requested permissions. - */ - fun createAuthorizationUrl(): String = - authorizationUrl + "?" + listOfNotNull( - "response_type" to "code", - "client_id" to clientId, - "scope" to scopes.joinToString(" "), - "redirect_uri" to redirectUri, - "code_challenge_method" to "S256", - "code_challenge" to createPKCE_S256CodeChallenge(codeVerifier), - state?.let { "state" to it }, - ).toUrlParameters() - - /** - * Checks whether the given callback uri is meant for this instance - */ - fun itsForMe(uri: URI): Boolean { - val uri2 = URI(redirectUri) - return uri.scheme == uri2.scheme && uri.authority == uri2.authority && uri.path == uri2.path - && uri.getQueryParameters()["state"] == state - } - - /** - * Extracts the authorization code from a parameter of the callback uri - * - * @throws OAuthException if the URI does not contain the authorization code, e.g. - * the user did not accept the requested permissions - * @throws OAuthConnectionException if the server reply is malformed - */ - fun extractAuthorizationCode(uri: URI): String { - val parameters = uri.getQueryParameters() - val authorizationCode = parameters["code"] - if (authorizationCode != null) return authorizationCode - - val error = parameters["error"] - ?: throw OAuthConnectionException("OAuth 2 authorization endpoint did not return a valid error response: $uri") - - throw OAuthException(error, parameters["error_description"], parameters["error_uri"]) - } - - /** - * Retrieves the access token, using the previously retrieved [authorizationCode] - * - * @throws OAuthException if there has been an OAuth authorization error - * @throws OAuthConnectionException if the server reply is malformed or there is an issue with - * the connection - */ - fun retrieveAccessToken(authorizationCode: String): AccessTokenResponse { - val url = accessTokenUrl + "?" + listOfNotNull( - "grant_type" to "authorization_code", - "client_id" to clientId, - "code" to authorizationCode, - "redirect_uri" to redirectUri, - "code_verifier" to codeVerifier, - ).toUrlParameters() - - try { - val connection = URL(url).openConnection() as HttpURLConnection - connection.setRequestProperty("User-Agent", ApplicationConstants.USER_AGENT) - connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded") - connection.requestMethod = "POST" - - if (connection.responseCode != HttpURLConnection.HTTP_OK) { - val body = connection.errorStream.bufferedReader().use { it.readText() } - val response = json.decodeFromString(body) - throw OAuthException(response.error, response.error_description, response.error_uri) - } else { - val body = connection.inputStream.bufferedReader().use { it.readText() } - val response = json.decodeFromString(body) - if (response.token_type.lowercase() != "bearer") { - // hey! that's not what we asked for! (according to the RFC, the client MUST check this) - throw OAuthConnectionException("OAuth 2 token endpoint returned an unknown token type (${response.token_type})") - } - return AccessTokenResponse(response.access_token, response.scope?.split(" ")) - } - // if OSM server does not return valid JSON, it is the server's fault, hence - } catch (e: SerializationException) { - throw OAuthConnectionException("OAuth 2 token endpoint did not return a valid response", e) - } catch (e: IllegalArgumentException) { - throw OAuthConnectionException("OAuth 2 token endpoint did not return a valid response", e) - } catch (e: IOException) { - throw OAuthConnectionException(cause = e) - } - } -} - -data class AccessTokenResponse( - val accessToken: String, - /** Granted scopes may be null if all requested scopes were granted */ - val grantedScopes: List? = null -) - -@Serializable -private data class AccessTokenResponseJson( - val access_token: String, - val token_type: String, - val scope: String? = null, - // OSM does currently not issue refresh tokens and the access token has no expiry date, so - // we can ignore the below - val expires_in: Long? = null, - val refresh_token: String? = null, -) - -@Serializable -private data class ErrorResponseJson( - val error: String, - val error_description: String? = null, - val error_uri: String? = null, -) - -/** - * Create the RFC 7636 Proof Key for Code Exchange from a random string. - * - * See https://www.rfc-editor.org/rfc/rfc7636 - */ -@OptIn(ExperimentalEncodingApi::class) -private fun createPKCE_S256CodeChallenge(codeVerifier: String): String { - // S256: code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) - val encodedBytes = codeVerifier.toByteArray(StandardCharsets.US_ASCII) - val sha256 = MessageDigest.getInstance("SHA-256").digest(encodedBytes) - return Base64.UrlSafe.encode(sha256).split("=")[0] -} - -private fun Iterable>.toUrlParameters(): String = - joinToString("&") { (k, v) -> k + "=" + URLEncoder.encode(v, "US-ASCII") } - -private fun createRandomAlphanumericString(length: Int): String { - val allowedChars = ('A'..'Z') + ('a'..'z') + ('0'..'9') - return (0 until length).map { allowedChars.random() }.joinToString("") -} - -private fun URI.getQueryParameters(): Map = - query?.split('&')?.associate { - val parts = it.split('=') - parts[0] to URLDecoder.decode(parts[1], "US-ASCII") - }.orEmpty() diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthService.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthService.kt new file mode 100644 index 00000000000..66c80f6d675 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/user/oauth/OAuthService.kt @@ -0,0 +1,202 @@ +package de.westnordost.streetcomplete.data.user.oauth + +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.request.post +import io.ktor.http.ContentType +import io.ktor.http.HttpStatusCode +import io.ktor.http.URLBuilder +import io.ktor.http.URLParserException +import io.ktor.http.Url +import io.ktor.http.contentType +import io.ktor.http.decodeURLQueryComponent +import io.ktor.http.takeFrom +import io.ktor.utils.io.errors.IOException +import kotlinx.serialization.Serializable +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import java.nio.charset.StandardCharsets +import java.security.MessageDigest +import kotlin.io.encoding.Base64 +import kotlin.io.encoding.ExperimentalEncodingApi + +/** + * Authorization flow: + * + * 1. Generate and store a OAuthAuthorizationParams instance and open the authorizationRequestUrl + * in a browser (or web view) + * 2. Let user accept or deny the permission request in the browser (or web view). Authorization + * endpoint will call the redirect URI (aka callback URI) with parameters in either case. + * 3. Check if the URI received is matches the OAuthAuthorizationParams instance with itsForMe(uri) + * and then extractAuthorizationCode(uri) from that URI + * 4. OAuthService.retrieveAccessToken(authorizationCode) with the retrieved authorizationCode + */ +class OAuthService(private val httpClient: HttpClient) { + private val json = Json { ignoreUnknownKeys = true } + + /** + * Retrieves the access token, using the previously retrieved [authorizationCode] + * + * @throws OAuthException if there has been an OAuth authorization error + * @throws OAuthConnectionException if the server reply is malformed or there is an issue with + * the connection + */ + suspend fun retrieveAccessToken(request: OAuthAuthorizationParams, authorizationCode: String): AccessTokenResponse { + try { + val response = httpClient.post(request.accessTokenUrl) { + url { + parameters.append("grant_type", "authorization_code") + parameters.append("client_id", request.clientId) + parameters.append("code", authorizationCode) + parameters.append("redirect_uri", request.redirectUri) + parameters.append("code_verifier", request.codeVerifier) + } + contentType(ContentType.Application.FormUrlEncoded) + } + + if (response.status != HttpStatusCode.OK) { + val errorResponse = json.decodeFromString(response.body()) + throw OAuthException(errorResponse.error, errorResponse.error_description, errorResponse.error_uri) + } else { + val accessTokenResponse = json.decodeFromString(response.body()) + if (accessTokenResponse.token_type.lowercase() != "bearer") { + throw OAuthConnectionException("OAuth 2 token endpoint returned an unknown token type (${accessTokenResponse.token_type})") + } + return AccessTokenResponse(accessTokenResponse.access_token, accessTokenResponse.scope?.split(" ")) + } + // if OSM server does not return valid JSON, it is the server's fault, hence + } catch (e: SerializationException) { + throw OAuthConnectionException("OAuth 2 token endpoint did not return a valid response", e) + } catch (e: IllegalArgumentException) { + throw OAuthConnectionException("OAuth 2 token endpoint did not return a valid response", e) + } catch (e: IOException) { + throw OAuthConnectionException(cause = e) + } + } +} + +/** + * One authorization with OAuth. For each authorization request, a new instance must be created + * + * @param authorizationUrl the OAuth2 server authorization endpoint + * @param accessTokenUrl the OAuth2 server token endpoint + * @param clientId the OAuth2 client id + * @param scopes the scopes (aka permissions) to request + * @param redirectUri the redirect URI (aka callback URI) that the authorization endpoint should + * call when the user allowed the authorization. + * @param state optional string to identify this oauth authorization flow, in case there are + * several at once (using the same redirect URI) + */ +@Serializable data class OAuthAuthorizationParams( + val authorizationUrl: String, + val accessTokenUrl: String, + val clientId: String, + val scopes: List, + val redirectUri: String, + val state: String? = null +) { + /** + * For the code challenge as specified in RFC 7636 + * https://www.rfc-editor.org/rfc/rfc7636 + * + * and required in the OAuth 2.1 draft + * https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-09 + */ + public val codeVerifier: String = createRandomAlphanumericString(128) + + /** + * Creates the URL to be opened in the browser or a web view in which the user agrees to + * authorize the requested permissions. + */ + val authorizationRequestUrl get() = + URLBuilder().takeFrom(authorizationUrl).apply { + parameters.append("response_type", "code") + parameters.append("client_id", clientId) + parameters.append("scope", scopes.joinToString(" ")) + parameters.append("redirect_uri", redirectUri) + parameters.append("code_challenge_method", "S256") + parameters.append("code_challenge", createPKCE_S256CodeChallenge(codeVerifier)) + state?.let { parameters.append("state", it) } + }.build().toString() + + /** + * Checks whether the given callback uri is meant for this instance + */ + fun itsForMe(callBackUri: String): Boolean = try { + itsForMe(Url(callBackUri)) + } catch (e: URLParserException) { + false + } + + private fun itsForMe(callbackUri: Url): Boolean { + val uri2 = Url(redirectUri) + return callbackUri.protocol == uri2.protocol + && callbackUri.host == uri2.host + && callbackUri.pathSegments == uri2.pathSegments + && callbackUri.parameters["state"] == state + } +} + +/** + * Extracts the authorization code from a parameter of the callback uri + * + * @throws OAuthException if the URI does not contain the authorization code, e.g. + * the user did not accept the requested permissions + * @throws OAuthConnectionException if the server reply is malformed + */ +public fun extractAuthorizationCode(uri: String): String { + val parameters = Url(uri).parameters + val authorizationCode = parameters["code"] + if (authorizationCode != null) return authorizationCode + + val error = parameters["error"] + ?: throw OAuthConnectionException("OAuth 2 authorization endpoint did not return a valid error response: $uri") + + throw OAuthException( + error.decodeURLQueryComponent(plusIsSpace = true), + parameters["error_description"], + parameters["error_uri"] + ) +} + +data class AccessTokenResponse( + val accessToken: String, + /** Granted scopes may be null if all requested scopes were granted */ + val grantedScopes: List? = null +) + +@Serializable +private data class AccessTokenResponseJson( + val access_token: String, + val token_type: String, + val scope: String? = null, + // OSM does currently not issue refresh tokens and the access token has no expiry date, so + // we can ignore the below + val expires_in: Long? = null, + val refresh_token: String? = null, +) + +@Serializable +private data class ErrorResponseJson( + val error: String, + val error_description: String? = null, + val error_uri: String? = null, +) + +/** + * Create the RFC 7636 Proof Key for Code Exchange from a random string. + * + * See https://www.rfc-editor.org/rfc/rfc7636 + */ +@OptIn(ExperimentalEncodingApi::class) +private fun createPKCE_S256CodeChallenge(codeVerifier: String): String { + // S256: code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) + val encodedBytes = codeVerifier.toByteArray(StandardCharsets.US_ASCII) + val sha256 = MessageDigest.getInstance("SHA-256").digest(encodedBytes) + return Base64.UrlSafe.encode(sha256).split("=")[0] +} + +private fun createRandomAlphanumericString(length: Int): String { + val allowedChars = ('A'..'Z') + ('a'..'z') + ('0'..'9') + return (0 until length).map { allowedChars.random() }.joinToString("") +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloader.kt index 5378fe9991a..8bf39e54035 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloader.kt @@ -1,32 +1,20 @@ package de.westnordost.streetcomplete.data.user.statistics -import de.westnordost.streetcomplete.ApplicationConstants -import java.io.IOException -import java.net.HttpURLConnection -import java.net.URL +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.plugins.expectSuccess +import io.ktor.client.request.get /** Downloads statistics from the backend */ class StatisticsDownloader( + private val httpClient: HttpClient, private val baseUrl: String, private val statisticsParser: StatisticsParser ) { - fun download(osmUserId: Long): Statistics { - (URL("$baseUrl?user_id=$osmUserId").openConnection() as HttpURLConnection).run { - useCaches = false - doOutput = true - doInput = true - setRequestProperty("User-Agent", ApplicationConstants.USER_AGENT) - requestMethod = "GET" - when (responseCode) { - HttpURLConnection.HTTP_OK -> { - return statisticsParser.parse(inputStream.bufferedReader().use { it.readText() }) - } - else -> { - val errorMessage = responseMessage - val errorDescription = errorStream?.bufferedReader()?.use { it.readText() } - throw IOException("$responseCode $errorMessage: $errorDescription") - } - } + suspend fun download(osmUserId: Long): Statistics { + val response = httpClient.get("$baseUrl?user_id=$osmUserId") { + expectSuccess = true } + return statisticsParser.parse(response.body()) } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsModule.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsModule.kt index a48815219ed..daee508eb4f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsModule.kt @@ -14,7 +14,7 @@ val statisticsModule = module { factory { ActiveDatesDao(get()) } - factory { StatisticsDownloader(STATISTICS_BACKEND_URL, get()) } + factory { StatisticsDownloader(get(), STATISTICS_BACKEND_URL, get()) } factory { StatisticsParser(get(named("TypeAliases"))) } single { get() } diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/Area.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/Area.kt index e4d70c8a876..3c88daf3a36 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/Area.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/Area.kt @@ -14,7 +14,7 @@ val IS_AREA_EXPRESSION = """ private fun isAreaExpressionFragment(prefix: String? = null): String { val p = if (prefix != null) "$prefix:" else "" - /* roughly sorted by occurrence count */ + // roughly sorted by occurrence count return """ ${p}building or ${p}landuse diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt index f9c4e83337a..c0d4fe2e0ff 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt @@ -34,9 +34,9 @@ fun Element.isPlace(): Boolean = private val IS_PLACE_EXPRESSION by lazy { val tags = mapOf( "amenity" to listOf( - /* grouped by subcategory, sorted by alphabet */ + // grouped by subcategory, sorted by alphabet - /* sustenance */ + // sustenance "bar", "biergarten", "cafe", @@ -46,7 +46,7 @@ private val IS_PLACE_EXPRESSION by lazy { "pub", "restaurant", - /* education */ + // education "childcare", "college", "dancing_school", @@ -64,7 +64,7 @@ private val IS_PLACE_EXPRESSION by lazy { "training", "university", - /* transportation */ + // transportation "boat_rental", // usually there is some kind of office, even if it is just a small stall "car_rental", // usually there is some kind of office "car_wash", @@ -72,14 +72,14 @@ private val IS_PLACE_EXPRESSION by lazy { "motorcycle_rental", "vehicle_inspection", - /* financial */ + // financial "bank", "bureau_de_change", "mobile_money_agent", "money_transfer", "payment_centre", - /* healthcare */ + // healthcare "clinic", "dentist", "doctors", @@ -90,7 +90,7 @@ private val IS_PLACE_EXPRESSION by lazy { "social_facility", "veterinary", - /* entertainment, arts & culture */ + // entertainment, arts & culture "archive", "arts_centre", "brothel", @@ -113,7 +113,7 @@ private val IS_PLACE_EXPRESSION by lazy { "swingerclub", "theatre", - /* public service */ + // public service "courthouse", "embassy", // deprecated now "fire_station", @@ -125,11 +125,11 @@ private val IS_PLACE_EXPRESSION by lazy { "ranger_station", "townhall", - /* facilities */ + // facilities "lavoir", "left_luggage", - /* others */ + // others "animal_boarding", "animal_shelter", "animal_training", diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/Things.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/Things.kt index c8195b04ab9..333c9ab2468 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/Things.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/Things.kt @@ -38,9 +38,9 @@ private val IS_THING_EXPRESSION by lazy { "windsock", ), "amenity" to listOf( - /* grouped by subcategory, sorted by alphabet */ + // grouped by subcategory, sorted by alphabet - /* transportation */ + // transportation "bicycle_parking", "bicycle_rental", "bicycle_repair_station", @@ -56,23 +56,23 @@ private val IS_THING_EXPRESSION by lazy { "ticket_validator", "vacuum_cleaner", - /* financial */ + // financial "atm", "payment_terminal", - /* healthcare */ + // healthcare "baby_hatch", "kneipp_water_cure", - /* entertainment, arts & culture */ + // entertainment, arts & culture "fountain", "public_bookcase", "whirlpool", - /* public service */ + // public service // "post_box", - blocked by https://github.com/streetcomplete/StreetComplete/issues/4916 - /* facilities & others */ + // facilities & others "baking_oven", "bbq", "bench", @@ -106,13 +106,13 @@ private val IS_THING_EXPRESSION by lazy { "water_point", "watering_place", - /* waste management */ + // waste management // "recycling" only for containers, see bottom "sanitary_dump_station", "waste_basket", "waste_disposal", - /* animals */ + // animals "feeding_place", "game_feeding", "hunting_stand", diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/WaysCrossing.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/WaysCrossing.kt index 26b2e6624f5..b2e4fac3101 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/WaysCrossing.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/WaysCrossing.kt @@ -20,7 +20,7 @@ data class WaysCrossing(val node: Node, var barrierWays: List, var movingWa */ fun findNodesAtCrossingsOf(barrierWays: Sequence, movingWays: Sequence, mapData: MapData): Iterable { val barriersByNodeId = barrierWays.groupByNodeIds() - /* filter out nodes of roads that are the end of a barrier not continuing further */ + // filter out nodes of roads that are the end of a barrier not continuing further barriersByNodeId.removeEndNodes() val waysByNodeId = movingWays.groupByNodeIds() @@ -32,7 +32,7 @@ fun findNodesAtCrossingsOf(barrierWays: Sequence, movingWays: Sequence * https://www.openstreetmap.org/node/56606744 (with roads) */ waysByNodeId.removeEndNodes() - /* filter out all nodes that are not shared nodes of both a road and a footway */ + // filter out all nodes that are not shared nodes of both a road and a footway barriersByNodeId.keys.retainAll(waysByNodeId.keys) waysByNodeId.keys.retainAll(barriersByNodeId.keys) diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/address/StreetOrPlaceNameViewController.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/address/StreetOrPlaceNameViewController.kt index 051b23cb10a..eb59439fcb5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/address/StreetOrPlaceNameViewController.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/address/StreetOrPlaceNameViewController.kt @@ -57,7 +57,7 @@ class StreetOrPlaceNameViewController( } get() = when (spinnerSelection) { STREET -> streetNameInputCtrl.streetName?.let { StreetName(it) } - PLACE -> placeNameInput.nonBlankTextOrNull?.let { PlaceName(it) } + PLACE -> placeNameInput.nonBlankTextOrNull?.let { PlaceName(it) } } private var spinnerSelection: StreetOrPlace diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/building/BuildingType.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/building/BuildingType.kt index 4b72e35b352..04164507f24 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/building/BuildingType.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/building/BuildingType.kt @@ -109,7 +109,7 @@ enum class BuildingType(val osmKey: String?, val osmValue: String?) { ("building" to "cottage") to BUNGALOW, // not documented ("building" to "ger") to TENT, // a Mongolian tent ("building" to "stilt_house") to HOUSE, - ("building" to "terrace_house") to HOUSE, // not documented, auto-changing to house would be a loss of information + ("building" to "terrace_house") to HOUSE, // not documented, auto-changing to house would be a loss of information ("building" to "trullo") to HUT, ("building" to "pajaru") to HUT, // not documented, but similar to trullo https://it.wikipedia.org/wiki/Pajaru diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/Cycleway.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/Cycleway.kt index f8e03549b49..9d5b7b8c2dc 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/Cycleway.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/Cycleway.kt @@ -34,7 +34,7 @@ fun LeftAndRightCycleway.wasNoOnewayForCyclistsButNowItIs(tags: Map): Boolean? { - val onewayDir = when { + val onewayDir = when { isForwardOneway(tags) -> FORWARD isReversedOneway(tags) -> BACKWARD else -> return true diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt index 45b640ad917..a6415b3a889 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/cycleway/CyclewayCreator.kt @@ -53,7 +53,7 @@ fun LeftAndRightCycleway.applyTo(tags: Tags, isLeftHandTraffic: Boolean) { ).applyTo(tags) } -/* bare cycleway tags are interpreted differently for oneways */ +// bare cycleway tags are interpreted differently for oneways private fun expandBareTags(tags: Tags, isLeftHandTraffic: Boolean) { val cycleway = tags["cycleway"] ?: return // i.e. they are only expanded into one side. Which side depends on country, direction of oneway diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/lane_narrowing_traffic_calming/LaneNarrowingTrafficCalmingParser.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/lane_narrowing_traffic_calming/LaneNarrowingTrafficCalmingParser.kt index 74a4a5d9347..4e104b888bc 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/lane_narrowing_traffic_calming/LaneNarrowingTrafficCalmingParser.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/lane_narrowing_traffic_calming/LaneNarrowingTrafficCalmingParser.kt @@ -33,7 +33,7 @@ internal fun expandTrafficCalmingValue(values: String): List = values .split(';') // split e.g. choker;table;island .flatMap { - when { + when { // e.g. choked_table, choked_island... anything chocked_* is also a choker it.startsWith("choked_") -> listOf("choker", it.substringAfter('_')) diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/sidewalk/SidewalkCreator.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/sidewalk/SidewalkCreator.kt index af291eee431..50e9b2d6f30 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/sidewalk/SidewalkCreator.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/sidewalk/SidewalkCreator.kt @@ -27,7 +27,7 @@ fun LeftAndRightSidewalk.applyTo(tags: Tags) { tags.expandSides("sidewalk", includeBareTag = false) tags.separateConflatedSidewalk() - if (left != null) tags["sidewalk:left"] = left.osmValue + if (left != null) tags["sidewalk:left"] = left.osmValue if (right != null) tags["sidewalk:right"] = right.osmValue // use shortcut syntax if possible, preferred by community according to usage numbers on taginfo diff --git a/app/src/main/java/de/westnordost/streetcomplete/overlays/buildings/BuildingsOverlay.kt b/app/src/main/java/de/westnordost/streetcomplete/overlays/buildings/BuildingsOverlay.kt index eca1665f201..29d1265d901 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/overlays/buildings/BuildingsOverlay.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/overlays/buildings/BuildingsOverlay.kt @@ -23,7 +23,7 @@ class BuildingsOverlay : Overlay { override val achievements = listOf(BUILDING) override val hidesQuestTypes = setOf(AddBuildingType::class.simpleName!!) - /* building:use not supported, so don't offer to change it -> exclude from the overlay */ + // building:use not supported, so don't offer to change it -> exclude from the overlay override fun getStyledElements(mapData: MapDataWithGeometry) = mapData.filter( """ ways, relations with diff --git a/app/src/main/java/de/westnordost/streetcomplete/overlays/cycleway/StreetCyclewayOverlayForm.kt b/app/src/main/java/de/westnordost/streetcomplete/overlays/cycleway/StreetCyclewayOverlayForm.kt index f9b8c555937..b15cd0a22f0 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/overlays/cycleway/StreetCyclewayOverlayForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/overlays/cycleway/StreetCyclewayOverlayForm.kt @@ -233,7 +233,7 @@ class StreetCyclewayOverlayForm : AStreetSideSelectOverlayForm { return addressesWithoutStreet } - /* cannot be determined because of the associated street relations */ + // cannot be determined because of the associated street relations override fun isApplicableTo(element: Element): Boolean? = if (!filter.matches(element)) false else null diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_indoor/AddIsAmenityIndoor.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_indoor/AddIsAmenityIndoor.kt index a6e72e1c00e..93ac7eafdf4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_indoor/AddIsAmenityIndoor.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/amenity_indoor/AddIsAmenityIndoor.kt @@ -77,7 +77,7 @@ class AddIsAmenityIndoor(private val getFeature: (Element) -> Feature?) : buildings.any { building -> val buildingGeometry = buildingGeometriesById[building.id] - if (buildingGeometry != null && buildingGeometry.getBounds().contains(it.position)) { + if (buildingGeometry != null && buildingGeometry.getBounds().contains(it.position)) { it.position.isInMultipolygon(buildingGeometry.polygons) } else { false diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/building_entrance_reference/AddEntranceReference.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/building_entrance_reference/AddEntranceReference.kt index 4b17b94dc42..2762738a6ca 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/building_entrance_reference/AddEntranceReference.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/building_entrance_reference/AddEntranceReference.kt @@ -91,7 +91,7 @@ class AddEntranceReference : OsmElementQuestType { is Relation -> building.getMultipolygonNodeIds(mapData).toHashSet() else -> emptyList() } - val buildingEntrances = buildingsWayNodeIds.mapNotNull { mapData.getNode(it) } + val buildingEntrances = buildingsWayNodeIds.mapNotNull { mapData.getNode(it) } .filter { entrancesFilter.matches(it) } if (buildingEntrances.count() < 2) continue result.addAll(buildingEntrances.filter { noEntranceRefFilter.matches(it) && it.id !in excludedWayNodeIds }) 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 index 4592ab817a1..d22198e9ae9 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/existence/CheckExistence.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/existence/CheckExistence.kt @@ -61,6 +61,7 @@ class CheckExistence( or amenity = recycling and recycling_type = container or amenity = toilets or amenity = drinking_water + or man_made = planter ) and (${lastChecked(6.0)}) ) or ( diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafType.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafType.kt new file mode 100644 index 00000000000..22eb10df030 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafType.kt @@ -0,0 +1,37 @@ +package de.westnordost.streetcomplete.quests.leaf_detail + +import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression +import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry +import de.westnordost.streetcomplete.data.osm.mapdata.Element +import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry +import de.westnordost.streetcomplete.data.osm.mapdata.filter +import de.westnordost.streetcomplete.data.osm.osmquests.OsmFilterQuestType +import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement.OUTDOORS +import de.westnordost.streetcomplete.osm.Tags + +class AddTreeLeafType : OsmFilterQuestType() { + override val elementFilter = """ + nodes with + natural = tree + and !leaf_type + and !~"(taxon|genus|species).*" + """ + override val changesetComment = "Specify leaf types" + override val wikiLink = "Key:leaf_type" + override val icon = R.drawable.ic_quest_leaf + override val isDeleteElementEnabled = true + override val achievements = listOf(OUTDOORS) + override val defaultDisabledMessage = R.string.default_disabled_msg_difficult_and_time_consuming + + override fun getTitle(tags: Map) = R.string.quest_leafType_tree_title + + override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry) = + getMapData().filter("nodes with natural = tree".toElementFilterExpression()) + + override fun createForm() = AddTreeLeafTypeForm() + + override fun applyAnswerTo(answer: TreeLeafType, tags: Tags, geometry: ElementGeometry, timestampEdited: Long) { + tags["leaf_type"] = answer.osmValue + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafTypeForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafTypeForm.kt new file mode 100644 index 00000000000..454f26c6d81 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/AddTreeLeafTypeForm.kt @@ -0,0 +1,13 @@ +package de.westnordost.streetcomplete.quests.leaf_detail + +import de.westnordost.streetcomplete.quests.AImageListQuestForm + +class AddTreeLeafTypeForm : AImageListQuestForm() { + + override val items = TreeLeafType.entries.map { it.asItem() } + override val itemsPerRow = 2 + + override fun onClickOk(selectedItems: List) { + applyAnswer(selectedItems.single()) + } +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/ForestLeafTimeItem.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/ForestLeafTypeItem.kt similarity index 100% rename from app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/ForestLeafTimeItem.kt rename to app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/ForestLeafTypeItem.kt diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafType.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafType.kt new file mode 100644 index 00000000000..48843c475dc --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafType.kt @@ -0,0 +1,6 @@ +package de.westnordost.streetcomplete.quests.leaf_detail + +enum class TreeLeafType(val osmValue: String) { + NEEDLELEAVED("needleleaved"), + BROADLEAVED("broadleaved"), +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafTypeItem.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafTypeItem.kt new file mode 100644 index 00000000000..a3ad87c0d6b --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/leaf_detail/TreeLeafTypeItem.kt @@ -0,0 +1,18 @@ +package de.westnordost.streetcomplete.quests.leaf_detail + +import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.quests.leaf_detail.TreeLeafType.BROADLEAVED +import de.westnordost.streetcomplete.quests.leaf_detail.TreeLeafType.NEEDLELEAVED +import de.westnordost.streetcomplete.view.image_select.Item + +fun TreeLeafType.asItem() = Item(this, iconResId, titleResId) + +private val TreeLeafType.titleResId: Int get() = when (this) { + NEEDLELEAVED -> R.string.quest_leaf_type_needles + BROADLEAVED -> R.string.quest_leaf_type_broadleaved +} + +private val TreeLeafType.iconResId: Int get() = when (this) { + NEEDLELEAVED -> R.drawable.leaf_type_needleleaved + BROADLEAVED -> R.drawable.leaf_type_broadleaved +} diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeedForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeedForm.kt index c30c4fc6f39..ed219c82091 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeedForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/max_speed/AddMaxSpeedForm.kt @@ -383,7 +383,11 @@ class AddMaxSpeedForm : AbstractOsmQuestForm> try { - trafficDirectionMap = trafficFlowSegmentsApi.get(bbox) + runBlocking { trafficDirectionMap = trafficFlowSegmentsApi.get(bbox) } } catch (e: Exception) { Log.e("AddSuspectedOneway", "Unable to download traffic metadata", e) return emptyList() diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt index ac2c95d0f3d..0af7d5ccd07 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsApi.kt @@ -2,14 +2,20 @@ package de.westnordost.streetcomplete.quests.oneway_suspects.data import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox import de.westnordost.streetcomplete.util.ktx.format +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.plugins.expectSuccess +import io.ktor.client.request.get import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json -import java.net.URL -/** Dao for using this API: https://github.com/ENT8R/oneway-data-api */ -class TrafficFlowSegmentsApi(private val apiUrl: String) { +/** Dao for using this API: https://github.com/streetcomplete/oneway-data-api */ +class TrafficFlowSegmentsApi( + private val httpClient: HttpClient, + private val apiUrl: String +) { - fun get(bbox: BoundingBox): Map> { + suspend fun get(bbox: BoundingBox): Map> { val leftBottomRightTopString = listOf( bbox.min.longitude, bbox.min.latitude, @@ -17,9 +23,10 @@ class TrafficFlowSegmentsApi(private val apiUrl: String) { bbox.max.latitude ).joinToString(",") { it.format(7) } - val url = URL("$apiUrl?bbox=$leftBottomRightTopString") - val json = url.openConnection().getInputStream().bufferedReader().use { it.readText() } - return parse(json) + val response = httpClient.get("$apiUrl?bbox=$leftBottomRightTopString") { + expectSuccess = true + } + return parse(response.body()) } companion object { diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsModule.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsModule.kt index 9c95113d3ad..c94536acdb5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsModule.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/oneway_suspects/data/TrafficFlowSegmentsModule.kt @@ -4,5 +4,5 @@ import org.koin.dsl.module const val ONEWAY_API_URL = "https://www.westnordost.de/streetcomplete/oneway-data-api/" val trafficFlowSegmentsModule = module { - factory { TrafficFlowSegmentsApi(ONEWAY_API_URL) } + factory { TrafficFlowSegmentsApi(get(), ONEWAY_API_URL) } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt index d7f7ec1a99f..6c6a17ec040 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt @@ -79,7 +79,7 @@ class CheckOpeningHoursSigned( } } else { tags["opening_hours:signed"] = "no" - /* still unsigned: just set the check date to now, user was on-site */ + // still unsigned: just set the check date to now, user was on-site tags.updateCheckDateForKey("opening_hours") } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/religion/ReligionItem.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/religion/ReligionItem.kt index ddd3ca367e4..d8ab3163c08 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/religion/ReligionItem.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/religion/ReligionItem.kt @@ -20,7 +20,7 @@ import de.westnordost.streetcomplete.view.image_select.Item fun Religion.asItem(): DisplayItem? { val iconResId = iconResId ?: return null - val titleResId = titleResId ?: return null + val titleResId = titleResId ?: return null return Item(this, iconResId, titleResId) } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/smoking/SmokingAllowedForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/smoking/SmokingAllowedForm.kt index 0d1bf436f9d..1157f96f7dc 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/smoking/SmokingAllowedForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/smoking/SmokingAllowedForm.kt @@ -18,7 +18,7 @@ class SmokingAllowedForm : AListQuestForm() { val noOutdoorSmoking = tags["outdoor_seating"] == "no" && tags["amenity"] != "nightclub" && tags["amenity"] != "stripclub" && tags["amenity"] != "pub" - /* nightclubs etc. might have outside smoking areas even when there is no seating outside */ + // nightclubs etc. might have outside smoking areas even when there is no seating outside return listOfNotNull( TextItem(NO, R.string.quest_smoking_no), diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/sport/AddSport.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/sport/AddSport.kt index 832c4f85450..661a58b9e98 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/sport/AddSport.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/sport/AddSport.kt @@ -14,7 +14,7 @@ class AddSport : OsmFilterQuestType>() { and (!sport or sport ~ football|skating|hockey|team_handball) and access !~ private|no """ - /* treat ambiguous values as if it is not set */ + // treat ambiguous values as if it is not set override val changesetComment = "Specify sport played on pitches" override val wikiLink = "Key:sport" override val icon = R.drawable.ic_quest_sport diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt index 139c38967d9..e8058283aa8 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt @@ -36,7 +36,7 @@ class AddPathSurface : OsmFilterQuestType() { ) and ~path|footway|cycleway|bridleway !~ link """ - /* ~paved ways are less likely to change the surface type */ + // ~paved ways are less likely to change the surface type override val changesetComment = "Specify path surfaces" override val wikiLink = "Key:surface" diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt index 8db098d74d8..71f20db0507 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/tracktype/AddTracktype.kt @@ -21,7 +21,7 @@ class AddTracktype : OsmFilterQuestType() { ) and (access !~ private|no or (foot and foot !~ private|no)) """ - /* ~paved tracks are less likely to change the surface type */ + // ~paved tracks are less likely to change the surface type override val changesetComment = "Specify tracktypes" override val wikiLink = "Key:tracktype" override val icon = R.drawable.ic_quest_tractor diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/about/LogsFiltersDialog.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/about/LogsFiltersDialog.kt index 81522a43c36..17519115767 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/about/LogsFiltersDialog.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/about/LogsFiltersDialog.kt @@ -113,7 +113,7 @@ class LogsFiltersDialog( private fun updateNewerThanInput() { binding.newerThanEditTextLayout.isEndIconVisible = timestampNewerThan != null - binding.newerThanEditText.setText(timestampNewerThan?.let { dateTimeToString(locale, it) } ?: "") + binding.newerThanEditText.setText(timestampNewerThan?.let { dateTimeToString(locale, it) } ?: "") } private fun updateOlderThanInput() { diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/main/MainFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/main/MainFragment.kt index ce22871037d..8bd9d6bf41d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/main/MainFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/main/MainFragment.kt @@ -998,7 +998,7 @@ class MainFragment : } private fun onClickCompassButton() { - /* Clicking the compass button will always rotate the map back to north and remove tilt */ + // Clicking the compass button will always rotate the map back to north and remove tilt val mapFragment = mapFragment ?: return val camera = mapFragment.cameraPosition ?: return diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt index 744c035c2be..771d2347a94 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt @@ -30,6 +30,7 @@ import de.westnordost.streetcomplete.quests.note_discussion.AttachPhotoFragment import de.westnordost.streetcomplete.util.ktx.childFragmentManagerOrNull import de.westnordost.streetcomplete.util.ktx.getLocationInWindow import de.westnordost.streetcomplete.util.ktx.hideKeyboard +import de.westnordost.streetcomplete.util.ktx.isKeyboardOpen import de.westnordost.streetcomplete.util.ktx.viewLifecycleScope import de.westnordost.streetcomplete.util.dialogs.showOutsideDownloadedAreaDialog import de.westnordost.streetcomplete.util.viewBinding @@ -159,7 +160,10 @@ class CreateNoteFragment : AbstractCreateNoteFragment() { override fun onComposedNote(text: String, imagePaths: List, isGpxNote: Boolean) { /* pressing once on "OK" should first only close the keyboard, so that the user can review the position of the note he placed (this is now optional) */ - if (prefs.getBoolean(Prefs.HIDE_KEYBOARD_FOR_NOTE, true) && contentBinding.noteInput.hideKeyboard() == true) return + if (prefs.getBoolean(Prefs.HIDE_KEYBOARD_FOR_NOTE, true) && contentBinding.noteInput.isKeyboardOpen) { + contentBinding.noteInput.hideKeyboard() + return + } val createNoteMarker = binding.markerCreateLayout.createNoteMarker val screenPos = createNoteMarker.getLocationInWindow() diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/main/controls/UndoButtonFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/main/controls/UndoButtonFragment.kt index ca28b30c657..ea28ecaa1d2 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/main/controls/UndoButtonFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/main/controls/UndoButtonFragment.kt @@ -31,7 +31,7 @@ class UndoButtonFragment : Fragment(R.layout.fragment_undo_button) { } private val listener: Listener? get() = parentFragment as? Listener ?: activity as? Listener - /* undo button is not shown when there is nothing to undo */ + // undo button is not shown when there is nothing to undo private val editHistoryListener = object : EditHistorySource.Listener { override fun onAdded(edit: Edit) { viewLifecycleScope.launch { animateInIfAnythingToUndo() } } override fun onSynced(edit: Edit) { viewLifecycleScope.launch { animateOutIfNothingLeftToUndo() } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/settings/DataManagementSettingsFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/settings/DataManagementSettingsFragment.kt index 51d906b5c7d..24f4b3b2d31 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/settings/DataManagementSettingsFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/settings/DataManagementSettingsFragment.kt @@ -813,7 +813,7 @@ private const val REQUEST_CODE_TREES_EXPORT = 5332 private const val REQUEST_CODE_CUSTOM_QUEST_IMPORT = 5333 private const val REQUEST_CODE_CUSTOM_QUEST_EXPORT = 5334 -const val LAST_KNOWN_DB_VERSION = 15L +const val LAST_KNOWN_DB_VERSION = 16L private const val BACKUP_HIDDEN_OSM_QUESTS = "quests" private const val BACKUP_HIDDEN_NOTES = "notes" diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/settings/SettingsFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/settings/SettingsFragment.kt index 0264feeec54..f52d571fb1f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/settings/SettingsFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/settings/SettingsFragment.kt @@ -108,7 +108,7 @@ class SettingsFragment : TwoPaneListFragment(), HasTitle { override fun onUnhidAll() { setHiddenQuestsSummary() } } - private val onAutosyncChanged = { + private val onAutosyncChanged = { if (Prefs.Autosync.valueOf(prefs.getStringOrNull(Prefs.AUTOSYNC) ?: "ON") != Prefs.Autosync.ON) { AlertDialog.Builder(requireContext()) .setView(layoutInflater.inflate(R.layout.dialog_tutorial_upload, null)) diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/user/login/OAuthFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/user/login/OAuthFragment.kt index 8a85c6626ea..94524984c3a 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/user/login/OAuthFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/user/login/OAuthFragment.kt @@ -18,8 +18,10 @@ import de.westnordost.streetcomplete.data.user.OAUTH2_REDIRECT_URI import de.westnordost.streetcomplete.data.user.OAUTH2_REQUESTED_SCOPES import de.westnordost.streetcomplete.data.user.OAUTH2_REQUIRED_SCOPES import de.westnordost.streetcomplete.data.user.OAUTH2_TOKEN_URL -import de.westnordost.streetcomplete.data.user.oauth.OAuthAuthorization +import de.westnordost.streetcomplete.data.user.oauth.OAuthAuthorizationParams import de.westnordost.streetcomplete.data.user.oauth.OAuthException +import de.westnordost.streetcomplete.data.user.oauth.OAuthService +import de.westnordost.streetcomplete.data.user.oauth.extractAuthorizationCode import de.westnordost.streetcomplete.databinding.FragmentOauthBinding import de.westnordost.streetcomplete.screens.HasTitle import de.westnordost.streetcomplete.util.ktx.toast @@ -31,8 +33,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json -import java.net.URI -import java.net.URISyntaxException +import org.koin.android.ext.android.inject import java.util.Locale import kotlin.coroutines.Continuation import kotlin.coroutines.resumeWithException @@ -49,8 +50,8 @@ class OAuthFragment : Fragment(R.layout.fragment_oauth), HasTitle { } private val listener: Listener? get() = parentFragment as? Listener ?: activity as? Listener private val webViewClient: OAuthWebViewClient = OAuthWebViewClient() - - private lateinit var oAuth: OAuthAuthorization + private val oAuthService: OAuthService by inject() + private lateinit var oAuth: OAuthAuthorizationParams override val title: String get() = getString(R.string.user_login) @@ -78,7 +79,7 @@ class OAuthFragment : Fragment(R.layout.fragment_oauth), HasTitle { override fun onCreate(inState: Bundle?) { super.onCreate(inState) oAuth = inState?.getString(OAUTH)?.let { Json.decodeFromString(it) } - ?: OAuthAuthorization( + ?: OAuthAuthorizationParams( OAUTH2_AUTHORIZATION_URL, OAUTH2_TOKEN_URL, OAUTH2_CLIENT_ID, @@ -113,7 +114,7 @@ class OAuthFragment : Fragment(R.layout.fragment_oauth), HasTitle { try { binding.webView.visibility = View.VISIBLE binding.webView.loadUrl( - oAuth.createAuthorizationUrl(), + oAuth.authorizationRequestUrl, mutableMapOf("Accept-Language" to Locale.getDefault().toLanguageTag()) ) val authorizationCode = webViewClient.awaitOAuthCallback() @@ -121,7 +122,7 @@ class OAuthFragment : Fragment(R.layout.fragment_oauth), HasTitle { binding.progressView.visibility = View.VISIBLE val accessTokenResponse = withContext(Dispatchers.IO) { - oAuth.retrieveAccessToken(authorizationCode) + oAuthService.retrieveAccessToken(oAuth, authorizationCode) } // not all requires scopes granted if (accessTokenResponse.grantedScopes?.containsAll(OAUTH2_REQUIRED_SCOPES) == false) { @@ -159,13 +160,8 @@ class OAuthFragment : Fragment(R.layout.fragment_oauth), HasTitle { @Deprecated("Deprecated in Java") override fun shouldOverrideUrlLoading(view: WebView?, url: String): Boolean { - val uri = try { - URI(url) - } catch (e: URISyntaxException) { - return false - } - if (!oAuth.itsForMe(uri)) return false - continuation?.resumeWith(runCatching { oAuth.extractAuthorizationCode(uri) }) + if (!oAuth.itsForMe(url)) return false + continuation?.resumeWith(runCatching { extractAuthorizationCode(url) }) return true } diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/user/profile/ProfileFragment.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/user/profile/ProfileFragment.kt index 299ffe0ead3..deecea3e433 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/user/profile/ProfileFragment.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/user/profile/ProfileFragment.kt @@ -71,7 +71,7 @@ class ProfileFragment : Fragment(R.layout.fragment_profile) { viewLifecycleScope.launch { updateEditCountTexts() } } override fun onSubtractedOne(type: String) { - viewLifecycleScope.launch { updateEditCountTexts() } + viewLifecycleScope.launch { updateEditCountTexts() } } override fun onUpdatedAll() { viewLifecycleScope.launch { updateStatisticsTexts() } diff --git a/app/src/main/java/de/westnordost/streetcomplete/screens/user/statistics/CircularFlagView.kt b/app/src/main/java/de/westnordost/streetcomplete/screens/user/statistics/CircularFlagView.kt index 844e526fd57..3a2242f2422 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/screens/user/statistics/CircularFlagView.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/screens/user/statistics/CircularFlagView.kt @@ -63,7 +63,7 @@ class CircularFlagView @JvmOverloads constructor( } override fun setRotation(rotation: Float) { - /* this view can't handle rotation properly */ + // this view can't handle rotation properly } override fun onDraw(canvas: Canvas) { @@ -137,7 +137,7 @@ class CircularFlagView @JvmOverloads constructor( } companion object { - /* make sure the YAML is only read once and kept once for all instances of SquareFlagView*/ + // make sure the YAML is only read once and kept once for all instances of SquareFlagView private var map: Map? = null private fun get(resources: Resources, countryCode: String): FlagAlignment? { diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Context.kt b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Context.kt index de592c67086..a65b44e6539 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Context.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Context.kt @@ -8,9 +8,6 @@ import android.content.Intent import android.content.IntentFilter import android.content.pm.PackageManager import android.location.LocationManager -import android.view.View -import android.view.inputmethod.InputMethodManager -import android.view.inputmethod.InputMethodManager.SHOW_IMPLICIT import android.widget.Toast import androidx.annotation.StringRes import androidx.core.content.ContextCompat @@ -51,16 +48,9 @@ fun Context.toast(@StringRes resId: Int, duration: Int = Toast.LENGTH_SHORT) { fun Context.hasPermission(permission: String): Boolean = ContextCompat.checkSelfPermission(this, permission) == PackageManager.PERMISSION_GRANTED -fun View.showKeyboard(): Boolean? = - context?.inputMethodManager?.showSoftInput(this, SHOW_IMPLICIT) - -fun View.hideKeyboard(): Boolean? = - context?.inputMethodManager?.hideSoftInputFromWindow(windowToken, 0) - val Context.isLocationEnabled: Boolean get() = LocationManagerCompat.isLocationEnabled(locationManager) val Context.hasLocationPermission: Boolean get() = hasPermission(ACCESS_FINE_LOCATION) -private val Context.inputMethodManager get() = getSystemService()!! private val Context.locationManager get() = getSystemService()!! /** Await a call from a broadcast once and return it */ diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Location.kt b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Location.kt index 573d9882026..ea91c4ed771 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Location.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/Location.kt @@ -2,5 +2,9 @@ package de.westnordost.streetcomplete.util.ktx import android.location.Location import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import kotlin.time.Duration +import kotlin.time.Duration.Companion.nanoseconds fun Location.toLatLon() = LatLon(latitude, longitude) + +val Location.elapsedDuration: Duration get() = elapsedRealtimeNanos.nanoseconds diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/URL.kt b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/URL.kt deleted file mode 100644 index 40750ce39ab..00000000000 --- a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/URL.kt +++ /dev/null @@ -1,12 +0,0 @@ -package de.westnordost.streetcomplete.util.ktx - -import java.io.File -import java.net.URL - -fun URL.saveToFile(file: File) { - openStream().use { input -> - file.outputStream().use { output -> - input.copyTo(output) - } - } -} diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/View.kt b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/View.kt index 2c3cb5c9417..1aa60ca45e9 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/ktx/View.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/util/ktx/View.kt @@ -9,6 +9,9 @@ import android.view.animation.AccelerateInterpolator import android.view.animation.DecelerateInterpolator import androidx.core.graphics.Insets import androidx.core.os.postDelayed +import androidx.core.view.SoftwareKeyboardControllerCompat +import androidx.core.view.ViewCompat +import androidx.core.view.WindowInsetsCompat import androidx.core.view.doOnPreDraw import androidx.core.view.updateLayoutParams import kotlinx.coroutines.suspendCancellableCoroutine @@ -95,3 +98,10 @@ fun View.updateMargins(left: Int? = null, top: Int? = null, right: Int? = null, ) } } + +fun View.showKeyboard(): Unit = SoftwareKeyboardControllerCompat(this).show() + +fun View.hideKeyboard(): Unit = SoftwareKeyboardControllerCompat(this).hide() + +val View.isKeyboardOpen: Boolean + get() = ViewCompat.getRootWindowInsets(this)?.isVisible(WindowInsetsCompat.Type.ime()) == true diff --git a/app/src/main/java/de/westnordost/streetcomplete/util/location/FineLocationManager.kt b/app/src/main/java/de/westnordost/streetcomplete/util/location/FineLocationManager.kt index e1314ccdd3b..c6d2a208f8e 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/util/location/FineLocationManager.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/util/location/FineLocationManager.kt @@ -15,6 +15,8 @@ import androidx.core.location.LocationListenerCompat import androidx.core.location.LocationManagerCompat import androidx.core.os.CancellationSignal import androidx.core.util.Consumer +import de.westnordost.streetcomplete.util.ktx.elapsedDuration +import kotlin.time.Duration.Companion.nanoseconds /** Convenience wrapper around the location manager with easier API, making use of both the GPS * and Network provider */ @@ -69,13 +71,13 @@ class FineLocationManager(context: Context, locationUpdateCallback: (Location) - fun getLastLocation() : Location? { if (deviceHasGPS) { locationManager.getLastKnownLocation(GPS_PROVIDER)?.let { - if (it.elapsedRealtimeNanos > SystemClock.elapsedRealtimeNanos() - TWO_MINUTES_IN_NANOSECONDS) + if ((SystemClock.elapsedRealtimeNanos().nanoseconds - it.elapsedDuration).inWholeMinutes < 2) return it } } if (deviceHasNetworkLocationProvider) { locationManager.getLastKnownLocation(NETWORK_PROVIDER)?.let { - if (it.elapsedRealtimeNanos > SystemClock.elapsedRealtimeNanos() - TWO_MINUTES_IN_NANOSECONDS) + if ((SystemClock.elapsedRealtimeNanos().nanoseconds - it.elapsedDuration).inWholeMinutes < 2) return it } } @@ -106,13 +108,11 @@ class FineLocationManager(context: Context, locationUpdateCallback: (Location) - // Based on https://web.archive.org/web/20180424190538/https://developer.android.com/guide/topics/location/strategies.html#BestEstimate -private const val TWO_MINUTES_IN_NANOSECONDS = 1000L * 1000 * 1000 * 60 * 2 - /** Determines whether this Location reading is better than the previous Location fix */ private fun Location.isBetterThan(previous: Location?): Boolean { // Check whether this is a valid location at all. // Happened once that lat/lon is NaN, maybe issue of that particular device - if (this.longitude.isNaN() || this.latitude.isNaN()) return false + if (longitude.isNaN() || latitude.isNaN()) return false // A new location is always better than no location if (previous == null) return true @@ -120,20 +120,18 @@ private fun Location.isBetterThan(previous: Location?): Boolean { // Check whether the new location fix is newer or older // we use elapsedRealtimeNanos instead of epoch time because some devices have issues // that may lead to incorrect GPS location.time (e.g. GPS week rollover, but also others) - // the use of nanoseconds is necessary because it is the only way to get - // elapsedRealtime of a location before API level 33 - val timeDelta = this.elapsedRealtimeNanos - previous.elapsedRealtimeNanos - val isMuchNewer = timeDelta > TWO_MINUTES_IN_NANOSECONDS - val isMuchOlder = timeDelta < -TWO_MINUTES_IN_NANOSECONDS - val isNewer = timeDelta > 0L + val diffMinutes = (elapsedDuration - previous.elapsedDuration).inWholeMinutes + val isMuchNewer = diffMinutes > 2 + val isMuchOlder = diffMinutes < -2 + val isNewer = diffMinutes > 0 // Check whether the new location fix is more or less accurate - val accuracyDelta = this.accuracy - previous.accuracy + val accuracyDelta = accuracy - previous.accuracy val isLessAccurate = accuracyDelta > 0f val isMoreAccurate = accuracyDelta < 0f val isMuchLessAccurate = accuracyDelta > 200f - val isFromSameProvider = this.provider == previous.provider + val isFromSameProvider = provider == previous.provider // Determine location quality using a combination of timeliness and accuracy return when { diff --git a/app/src/main/res/layout/fragment_overlay.xml b/app/src/main/res/layout/fragment_overlay.xml index 3508afe7086..497a65bc360 100644 --- a/app/src/main/res/layout/fragment_overlay.xml +++ b/app/src/main/res/layout/fragment_overlay.xml @@ -149,6 +149,7 @@ android:id="@+id/glassPane" android:layout_width="match_parent" android:layout_height="match_parent" + android:elevation="@dimen/speech_bubble_elevation" android:clickable="true" android:focusable="true" android:visibility="gone" diff --git a/app/src/main/res/layout/fragment_quest_answer.xml b/app/src/main/res/layout/fragment_quest_answer.xml index 19253393b45..12da540dd53 100644 --- a/app/src/main/res/layout/fragment_quest_answer.xml +++ b/app/src/main/res/layout/fragment_quest_answer.xml @@ -200,6 +200,7 @@ android:id="@+id/glassPane" android:layout_width="match_parent" android:layout_height="match_parent" + android:elevation="@dimen/speech_bubble_elevation" android:clickable="true" android:focusable="true" android:visibility="gone" diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ce5da194c43..6e09741e606 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1102,6 +1102,7 @@ If any lanes are reserved for buses, please leave a note instead." Needles Leaves Both are present + Does this tree have needles or leaves? On which level number is this place located? diff --git a/app/src/test/java/de/westnordost/streetcomplete/OpeningHoursParsingTest.kt b/app/src/test/java/de/westnordost/streetcomplete/OpeningHoursParsingTest.kt index 7b4829e8bbb..0cb715224eb 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/OpeningHoursParsingTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/OpeningHoursParsingTest.kt @@ -186,7 +186,7 @@ private fun TimesSelector.hasVariableTime(): Boolean = when (this) { private fun MonthsOrDateSelector.hasYear(): Boolean = when (this) { is Date -> hasYear() - is DateRange -> start.hasYear() || end.hasYear() + is DateRange -> start.hasYear() || end.hasYear() is DatesInMonth -> year != null is MonthRange -> year != null is SingleMonth -> year != null diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloaderTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloaderTest.kt new file mode 100644 index 00000000000..6e142befafe --- /dev/null +++ b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/AvatarsDownloaderTest.kt @@ -0,0 +1,78 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import de.westnordost.osmapi.user.UserApi +import de.westnordost.osmapi.user.UserInfo +import de.westnordost.streetcomplete.testutils.mock +import de.westnordost.streetcomplete.testutils.on +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respondError +import io.ktor.client.engine.mock.respondOk +import io.ktor.http.HttpMethod +import io.ktor.http.HttpStatusCode +import io.ktor.utils.io.errors.IOException +import kotlinx.coroutines.runBlocking +import java.nio.file.Files +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals + +class AvatarsDownloaderTest { + private val mockEngine = MockEngine { request -> when (request.url.encodedPath) { + "/NotFound" -> respondError(HttpStatusCode.NotFound) + "/ConnectionError" -> throw IOException("Cannot connect") + else -> respondOk("Image Content") + } } + private val tempFolder = Files.createTempDirectory("images").toFile() + private val userApi: UserApi = mock() + private val downloader = AvatarsDownloader(HttpClient(mockEngine), userApi, tempFolder) + private val userInfo = UserInfo(100, "Map Enthusiast 530") + + @BeforeTest fun setUp() { + userInfo.profileImageUrl = "http://example.com/BigImage.png" + on(userApi.get(userInfo.id)).thenReturn(userInfo) + } + + @Test + fun `download makes GET request to profileImageUrl`() = runBlocking { + downloader.download(listOf(userInfo.id)) + + assertEquals(1, mockEngine.requestHistory.size) + assertEquals(userInfo.profileImageUrl, mockEngine.requestHistory[0].url.toString()) + assertEquals(HttpMethod.Get, mockEngine.requestHistory[0].method) + } + + @Test + fun `download copies HTTP response from profileImageUrl into tempFolder`() = runBlocking { + downloader.download(listOf(userInfo.id)) + + assertEquals("Image Content", tempFolder.resolve("100").readText()) + } + + @Test + fun `download does not throw exception on HTTP NotFound`() = runBlocking { + userInfo.profileImageUrl = "http://example.com/NotFound" + + downloader.download(listOf(userInfo.id)) + + assertEquals(404, mockEngine.responseHistory[0].statusCode.value) + } + + @Test + fun `download does not throw exception on networking error`() = runBlocking { + userInfo.profileImageUrl = "http://example.com/ConnectionError" + + downloader.download(listOf(userInfo.id)) + + assertEquals(0, mockEngine.responseHistory.size) + } + + @Test + fun `download does not make HTTP request if profileImageUrl is NULL`() = runBlocking { + userInfo.profileImageUrl = null + + downloader.download(listOf(userInfo.id)) + + assertEquals(0, mockEngine.requestHistory.size) + } +} diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploaderTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploaderTest.kt new file mode 100644 index 00000000000..630ea8814de --- /dev/null +++ b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/StreetCompleteImageUploaderTest.kt @@ -0,0 +1,121 @@ +package de.westnordost.streetcomplete.data.osmnotes + +import de.westnordost.streetcomplete.data.download.ConnectionException +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respondBadRequest +import io.ktor.client.engine.mock.respondError +import io.ktor.client.engine.mock.respondOk +import io.ktor.client.engine.mock.toByteArray +import io.ktor.http.ContentType +import io.ktor.http.HttpMethod +import io.ktor.http.HttpStatusCode +import io.ktor.utils.io.errors.IOException +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class StreetCompleteImageUploaderTest { + private val mockEngine = MockEngine { request -> when (String(request.body.toByteArray())) { + // Upload requests + "valid\n" -> respondOk("{\"future_url\": \"market.jpg\"}") + "invalid\n" -> respondError(HttpStatusCode.InternalServerError) + "" -> respondBadRequest() + "ioexception\n" -> throw IOException("Unable to connect") + + // Activate requests + "{\"osm_note_id\": 180}" -> respondOk() + "{\"osm_note_id\": 190}" -> respondError(HttpStatusCode.InternalServerError) + "{\"osm_note_id\": 200}" -> respondBadRequest() + "{\"osm_note_id\": 210}" -> throw IOException("Unable to connect") + + else -> throw Exception("Invalid request body") + } } + private val uploader = StreetCompleteImageUploader(HttpClient(mockEngine), "http://example.com/" ) + + @Test + fun `upload makes POST request with file contents`() = runBlocking { + uploader.upload(listOf("src/test/resources/image_uploader/valid.jpg")) + + assertEquals(1, mockEngine.requestHistory.size) + assertEquals(HttpMethod.Post, mockEngine.requestHistory[0].method) + assertEquals("http://example.com/upload.php", mockEngine.requestHistory[0].url.toString()) + assertEquals(ContentType.Image.JPEG, mockEngine.requestHistory[0].body.contentType) + assertEquals("binary", mockEngine.requestHistory[0].headers["Content-Transfer-Encoding"]) + } + + @Test + fun `upload returns future_url value from response`() = runBlocking { + val uploads = uploader.upload(listOf("src/test/resources/image_uploader/valid.jpg")) + + assertContentEquals(listOf("market.jpg"), uploads) + } + + @Test + fun `upload throws ImageUploadServerException on 500 error`() = runBlocking { + val exception = assertFailsWith(ImageUploadServerException::class) { + uploader.upload(listOf("src/test/resources/image_uploader/invalid.jpg")) + } + + assertEquals("Upload failed: Error code 500 Internal Server Error, Message: \"Internal Server Error\"", exception.message) + } + + @Test + fun `upload throws ImageUploadClientException on 400 error`() = runBlocking { + val exception = assertFailsWith(ImageUploadClientException::class) { + uploader.upload(listOf("src/test/resources/image_uploader/empty.jpg")) + } + + assertEquals("Upload failed: Error code 400 Bad Request, Message: \"Bad Request\"", exception.message) + } + + @Test + fun `upload performs no requests with missing file`() = runBlocking { + assertContentEquals(listOf(), uploader.upload(listOf("no-such-file-at-this-path.jpg"))) + assertEquals(0, mockEngine.requestHistory.size) + } + + @Test + fun `upload throws ConnectionException on IOException`(): Unit = runBlocking { + assertFailsWith(ConnectionException::class) { + uploader.upload(listOf("src/test/resources/image_uploader/ioexception.jpg")) + } + } + + @Test + fun `activate makes POST request with note ID`() = runBlocking { + uploader.activate(180) + + assertEquals(1, mockEngine.requestHistory.size) + assertEquals("http://example.com/activate.php", mockEngine.requestHistory[0].url.toString()) + assertEquals(ContentType.Application.Json, mockEngine.requestHistory[0].body.contentType) + assertEquals("{\"osm_note_id\": 180}", String(mockEngine.requestHistory[0].body.toByteArray())) + } + + @Test + fun `activate throws ImageUploadServerException on 500 error`() = runBlocking { + val exception = assertFailsWith(ImageUploadServerException::class) { + uploader.activate(190) + } + + assertEquals("Error code 500 Internal Server Error, Message: \"Internal Server Error\"", exception.message) + } + + @Test + fun `activate throws ImageUploadClientException on 400 error`() = runBlocking { + val exception = assertFailsWith(ImageUploadClientException::class) { + uploader.activate(200) + } + + assertEquals("Error code 400 Bad Request, Message: \"Bad Request\"", exception.message) + } + + @Test + fun `activate throws ConnectionException on IOException`(): Unit = runBlocking { + assertFailsWith(ConnectionException::class) { + uploader.activate(210) + } + } +} diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt index a9bff060c72..6f0bb85ddcd 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt @@ -1,8 +1,10 @@ package de.westnordost.streetcomplete.data.osmnotes.edits +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.NoteController import de.westnordost.streetcomplete.data.osmnotes.NotesApi import de.westnordost.streetcomplete.data.osmnotes.StreetCompleteImageUploader +import de.westnordost.streetcomplete.data.osmtracks.Trackpoint import de.westnordost.streetcomplete.data.osmtracks.TracksApi import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener @@ -143,7 +145,7 @@ class NoteEditsUploaderTest { verify(listener, times(2))!!.onUploaded(any(), any()) } - @Test fun `upload note comment with attached images`() { + @Test fun `upload note comment with attached images`() = runBlocking { val pos = p(1.0, 13.0) val edit = noteEdit( noteId = 1L, @@ -169,7 +171,7 @@ class NoteEditsUploaderTest { verify(listener)!!.onUploaded("NOTE", pos) } - @Test fun `upload create note with attached images`() { + @Test fun `upload create note with attached images`() = runBlocking { val pos = p(1.0, 13.0) val edit = noteEdit( noteId = 1L, @@ -195,7 +197,31 @@ class NoteEditsUploaderTest { verify(listener)!!.onUploaded("NOTE", pos) } - @Test fun `upload missed image activations`() { + @Test fun `upload create note with attached GPS trace`() = runBlocking { + val pos = p(1.0, 13.0) + val edit = noteEdit( + noteId = 1L, + action = NoteEditAction.CREATE, + text = "test", + pos = pos, + track = listOf(Trackpoint(LatLon(0.0, 0.0), 180, 0.0f, 100.0f)) + ) + val note = note(1) + + on(noteEditsController.getOldestUnsynced()).thenReturn(edit).thenReturn(null) + on(userDataSource.userName).thenReturn("blah mc/Blah") + on(notesApi.create(any(), any())).thenReturn(note) + on(tracksApi.create(edit.track, edit.text)).thenReturn(988) + + upload() + + verify(notesApi).create(pos, "test\n\nGPS Trace: https://www.openstreetmap.org/user/blah%20mc%2FBlah/traces/988\n") + verify(noteController).put(note) + verify(noteEditsController).markSynced(edit, note) + verify(listener)!!.onUploaded("NOTE", pos) + } + + @Test fun `upload missed image activations`(): Unit = runBlocking { val edit = noteEdit(noteId = 3) on(noteEditsController.getOldestNeedingImagesActivation()).thenReturn(edit).thenReturn(null) diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedCheckerTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedCheckerTest.kt index e5199732042..ab9ac07172c 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedCheckerTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/data/upload/VersionIsBannedCheckerTest.kt @@ -1,21 +1,25 @@ package de.westnordost.streetcomplete.data.upload +import io.ktor.client.HttpClient +import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals class VersionIsBannedCheckerTest { - @Test fun `banned version `() { - assertEquals(IsBanned(null), VersionIsBannedChecker(URL, "StreetComplete 0.1").get()) + private val httpClient = HttpClient() + + @Test fun `banned version `() = runBlocking { + assertEquals(IsBanned(null), VersionIsBannedChecker(httpClient, URL, "StreetComplete 0.1").get()) } - @Test fun `not banned version `() { - assertEquals(IsNotBanned, VersionIsBannedChecker(URL, "StreetComplete 3.0").get()) + @Test fun `not banned version `() = runBlocking { + assertEquals(IsNotBanned, VersionIsBannedChecker(httpClient, URL, "StreetComplete 3.0").get()) } - @Test fun `banned version with reason`() { + @Test fun `banned version with reason`() = runBlocking { assertEquals( IsBanned("This version does not correctly determine in which country you are, necessary to tag certain answers correctly."), - VersionIsBannedChecker(URL, "StreetComplete 8.0").get() + VersionIsBannedChecker(httpClient, URL, "StreetComplete 8.0").get() ) } } diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorizationTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorizationTest.kt index b92ca703643..243ea1c5674 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorizationTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/data/user/oauth/OAuthAuthorizationTest.kt @@ -1,50 +1,55 @@ package de.westnordost.streetcomplete.data.user.oauth +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respondError +import io.ktor.client.engine.mock.respondOk +import io.ktor.http.HeadersBuilder +import io.ktor.http.HttpStatusCode +import io.ktor.http.ParametersBuilder +import io.ktor.http.Url +import kotlinx.coroutines.runBlocking import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json -import java.net.URI -import java.net.URL -import java.net.URLDecoder -import java.net.URLEncoder import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFails import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue class OAuthAuthorizationTest { @Test fun createAuthorizationUrl() { - val url = URL(createOAuth().createAuthorizationUrl()) + val url = Url(createOAuth().authorizationRequestUrl) - assertEquals("https", url.protocol) + assertEquals("https", url.protocol.name) assertEquals("test.me", url.host) - assertEquals("/auth", url.path) - - val parameters = url.queryParameters - assertEquals("code", parameters["response_type"]) - assertEquals("ClientId %#+!", parameters["client_id"]) - assertEquals("localhost://oauth", parameters["redirect_uri"]) - assertEquals("one! 2 THREE+(1/2)", parameters["scope"]) - assertEquals("S256", parameters["code_challenge_method"]) - assertTrue(parameters["code_challenge"]!!.length <= 128) - assertTrue(parameters["code_challenge"]!!.length >= 43) + assertEquals("/auth", url.encodedPath) + + assertEquals("code", url.parameters["response_type"]) + assertEquals("ClientId %#+!", url.parameters["client_id"]) + assertEquals("localhost://oauth", url.parameters["redirect_uri"]) + assertEquals("one! 2 THREE+(1/2)", url.parameters["scope"]) + assertEquals("S256", url.parameters["code_challenge_method"]) + assertTrue(url.parameters["code_challenge"]!!.length <= 128) + assertTrue(url.parameters["code_challenge"]!!.length >= 43) } @Test fun `createAuthorizationUrl with state`() { - val parameters = URL(createOAuth("123").createAuthorizationUrl()).queryParameters + val parameters = Url(createOAuth("123").authorizationRequestUrl).parameters assertEquals("123", parameters["state"]) } @Test fun `generates different code challenge for each instance`() { - val url1 = URL(createOAuth().createAuthorizationUrl()) - val url2 = URL(createOAuth().createAuthorizationUrl()) - assertTrue(url1.queryParameters["code_challenge"] != url2.queryParameters["code_challenge"]) + val url1 = Url(createOAuth().authorizationRequestUrl) + val url2 = Url(createOAuth().authorizationRequestUrl) + assertTrue(url1.parameters["code_challenge"] != url2.parameters["code_challenge"]) } @Test fun `serializes correctly`() { val oauth1 = createOAuth() val oauth1String = Json.encodeToString(oauth1) - val oauth2 = Json.decodeFromString(oauth1String) + val oauth2 = Json.decodeFromString(oauth1String) val oauth2String = Json.encodeToString(oauth2) assertEquals(oauth1String, oauth2String) @@ -54,54 +59,53 @@ class OAuthAuthorizationTest { val state = "123" val oauth = createOAuth(state) - assertFalse(oauth.itsForMe(URI("localhost://oauth"))) // no state - assertFalse(oauth.itsForMe(URI("localhost://oauth?state=abc"))) // different state - assertTrue(oauth.itsForMe(URI("localhost://oauth?state=$state"))) // same state + assertFalse(oauth.itsForMe("This isn't::::a valid URL")) + assertFalse(oauth.itsForMe("localhost://oauth")) // no state + assertFalse(oauth.itsForMe("localhost://oauth?state=abc")) // different state + assertTrue(oauth.itsForMe("localhost://oauth?state=$state")) // same state // different uri - assertFalse(oauth.itsForMe(URI("localhost://oauth3?state=$state"))) - assertFalse(oauth.itsForMe(URI("localhost://oauth/path?state=$state"))) - assertFalse(oauth.itsForMe(URI("localboost://oauth?state=$state"))) + assertFalse(oauth.itsForMe("localhost://oauth3?state=$state")) + assertFalse(oauth.itsForMe("localhost://oauth/path?state=$state")) + assertFalse(oauth.itsForMe("localboost://oauth?state=$state")) } @Test fun `itsForMe without state`() { val oauth = createOAuth() - assertTrue(oauth.itsForMe(URI("localhost://oauth"))) // no state - assertFalse(oauth.itsForMe(URI("localhost://oauth?state=abc"))) // different state + assertTrue(oauth.itsForMe("localhost://oauth")) // no state + assertFalse(oauth.itsForMe("localhost://oauth?state=abc")) // different state // different uri - assertFalse(oauth.itsForMe(URI("localhost://oauth3"))) - assertFalse(oauth.itsForMe(URI("localhost://oauth/path"))) - assertFalse(oauth.itsForMe(URI("localboost://oauth"))) + assertFalse(oauth.itsForMe("localhost://oauth3")) + assertFalse(oauth.itsForMe("localhost://oauth/path")) + assertFalse(oauth.itsForMe("localboost://oauth")) } @Test fun `extractAuthorizationCode fails with useful error messages`() { - val oauth = createOAuth() - // server did not respond correctly with "error" assertFailsWith { - oauth.extractAuthorizationCode(URI("localhost://oauth?e=something")) + extractAuthorizationCode("localhost://oauth?e=something") } try { - oauth.extractAuthorizationCode(URI("localhost://oauth?error=hey%2Bwhat%27s%2Bup")) + extractAuthorizationCode("localhost://oauth?error=hey%2Bwhat%27s%2Bup") } catch (e: OAuthException) { assertEquals("hey what's up", e.message) } try { - oauth.extractAuthorizationCode(URI("localhost://oauth?error=A%21&error_description=B%21")) + extractAuthorizationCode("localhost://oauth?error=A%21&error_description=B%21") } catch (e: OAuthException) { assertEquals("A!: B!", e.message) } try { - oauth.extractAuthorizationCode(URI("localhost://oauth?error=A%21&error_uri=http%3A%2F%2Fabc.de")) + extractAuthorizationCode("localhost://oauth?error=A%21&error_uri=http%3A%2F%2Fabc.de") } catch (e: OAuthException) { assertEquals("A! (see http://abc.de)", e.message) } try { - oauth.extractAuthorizationCode(URI("localhost://oauth?error=A%21&error_description=B%21&error_uri=http%3A%2F%2Fabc.de")) + extractAuthorizationCode("localhost://oauth?error=A%21&error_description=B%21&error_uri=http%3A%2F%2Fabc.de") } catch (e: OAuthException) { assertEquals("A!: B! (see http://abc.de)", e.message) } @@ -110,15 +114,99 @@ class OAuthAuthorizationTest { @Test fun extractAuthorizationCode() { assertEquals( "my code", - createOAuth().extractAuthorizationCode(URI("localhost://oauth?code=my%20code")) + extractAuthorizationCode("localhost://oauth?code=my%20code") + ) + } + + @Test fun `retrieveAccessToken generates valid access token`() = runBlocking { + val service = OAuthService(HttpClient(MockEngine { _ -> respondOk("""{ + "access_token": "TOKEN", + "token_type": "bearer", + "scope": "A B C" + }""") + })) + + val tokenResponse = service.retrieveAccessToken(dummyOAuthAuthorization(), "") + + assertEquals(AccessTokenResponse("TOKEN", listOf("A", "B", "C")), tokenResponse) + } + + @Test fun `retrieveAccessToken throws OAuthConnectionException with invalid response token_type`(): Unit = runBlocking { + val service = OAuthService(HttpClient(MockEngine { _ -> respondOk("""{ + "access_token": "TOKEN", + "token_type": "an_unusual_token_type", + "scope": "A B C" + }""") + })) + + val exception = assertFailsWith { service.retrieveAccessToken(dummyOAuthAuthorization(), "") } + + assertEquals("OAuth 2 token endpoint returned an unknown token type (an_unusual_token_type)", exception.message) + } + + @Test fun `retrieveAccessToken throws OAuthException when error response`(): Unit = runBlocking { + val service = OAuthService(HttpClient(MockEngine { _ -> + respondError( + HttpStatusCode.BadRequest, """{ + "error": "Missing auth code", + "error_description": "Please specify a code", + "error_uri": "code" + }""" + ) + })) + + val exception = assertFailsWith { + service.retrieveAccessToken(dummyOAuthAuthorization(), "") + } + + assertEquals("Missing auth code", exception.error) + assertEquals("Please specify a code", exception.description) + assertEquals("code", exception.uri) + assertEquals("Missing auth code: Please specify a code (see code)", exception.message) + } + + @Test fun `retrieveAccessToken generates correct request URL`(): Unit = runBlocking { + val mockEngine = MockEngine { _ -> respondOk() } + val auth = OAuthAuthorizationParams( + "", + "https://www.openstreetmap.org", + "OAuthClientId", + listOf(), + "scheme://there" ) + + assertFails { OAuthService(HttpClient(mockEngine)).retrieveAccessToken(auth, "C0D3") } + + val expectedParams = ParametersBuilder() + expectedParams.append("grant_type", "authorization_code") + expectedParams.append("client_id", "OAuthClientId") + expectedParams.append("code", "C0D3") + expectedParams.append("redirect_uri", "scheme://there") + expectedParams.append("code_verifier", auth.codeVerifier) + + assertEquals(1, mockEngine.requestHistory.size) + assertEquals(expectedParams.build(), mockEngine.requestHistory[0].url.parameters) + assertEquals("www.openstreetmap.org", mockEngine.requestHistory[0].url.host) } - // it's not properly possible to test retrieveAccessToken in isolation because the http client - // is not injected (passed in the constructor) + @Test fun `retrieveAccessToken generates request headers`(): Unit = runBlocking { + val mockEngine = MockEngine { _ -> respondOk() } + + assertFails { OAuthService(HttpClient(mockEngine)).retrieveAccessToken(dummyOAuthAuthorization(), "") } + + val expectedHeaders = HeadersBuilder() + expectedHeaders.append("Content-Type", "application/x-www-form-urlencoded") + expectedHeaders.append("Accept-Charset", "UTF-8") + expectedHeaders.append("Accept", "*/*") + + assertEquals(1, mockEngine.requestHistory.size) + assertEquals(expectedHeaders.build(), mockEngine.requestHistory[0].headers) + } } -private fun createOAuth(state: String? = null) = OAuthAuthorization( +private fun dummyOAuthAuthorization() = OAuthAuthorizationParams("", "", "", listOf(), "") + +private fun createOAuth(state: String? = null) = OAuthAuthorizationParams( "https://test.me/auth", "https://test.me/token", "ClientId %#+!", @@ -126,11 +214,3 @@ private fun createOAuth(state: String? = null) = OAuthAuthorization( "localhost://oauth", state ) - -private fun urlEncode(s: String) = URLEncoder.encode(s, "US-ASCII") - -private val URL.queryParameters get(): Map = - query.split('&').associate { - val parts = it.split('=') - parts[0] to URLDecoder.decode(parts[1], "US-ASCII") - } diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloaderTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloaderTest.kt new file mode 100644 index 00000000000..03ec69519c8 --- /dev/null +++ b/app/src/test/java/de/westnordost/streetcomplete/data/user/statistics/StatisticsDownloaderTest.kt @@ -0,0 +1,44 @@ +package de.westnordost.streetcomplete.data.user.statistics + +import de.westnordost.streetcomplete.testutils.mock +import de.westnordost.streetcomplete.testutils.on +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respondBadRequest +import io.ktor.client.engine.mock.respondOk +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFails + +class StatisticsDownloaderTest { + private val statisticsParser: StatisticsParser = mock() + + private val validResponseMockEngine = MockEngine { _ -> respondOk("simple response") } + + @Test fun `download parses all statistics`() = runBlocking { + val stats = Statistics(types = listOf(), countries = listOf(), rank = 2, daysActive = 100, currentWeekRank = 50, currentWeekTypes = listOf(), currentWeekCountries = listOf(), activeDates = listOf(), activeDatesRange = 100, isAnalyzing = false, lastUpdate = 10) + on(statisticsParser.parse("simple response")).thenReturn(stats) + assertEquals(stats, StatisticsDownloader(HttpClient(validResponseMockEngine), "", statisticsParser).download(100)) + } + + @Test fun `download throws Exception for a 400 response`() = runBlocking { + val mockEngine = MockEngine { _ -> respondBadRequest() } + val exception = assertFails { StatisticsDownloader(HttpClient(mockEngine), "", statisticsParser).download(100) } + + assertEquals( + "Client request(GET http://localhost/?user_id=100) invalid: 400 Bad Request. Text: \"Bad Request\"", + exception.message + ) + } + + @Test fun `download constructs request URL`() = runBlocking { + StatisticsDownloader( + HttpClient(validResponseMockEngine), + "https://example.com/stats/", + statisticsParser + ).download(100) + + assertEquals("https://example.com/stats/?user_id=100", validResponseMockEngine.requestHistory[0].url.toString()) + } +} diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/accepts_cards/AddAcceptsCardsTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/accepts_cards/AddAcceptsCardsTest.kt index ef9d69bb35c..ee2f09f59c0 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/accepts_cards/AddAcceptsCardsTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/accepts_cards/AddAcceptsCardsTest.kt @@ -44,4 +44,22 @@ class AddAcceptsCardsTest { ) assertEquals(1, questType.getApplicableElements(mapData).toList().size) } + + @Test fun `applicable to weird shop values`() { + val mapData = TestMapDataWithGeometry( + listOf( + node(1, tags = mapOf("shop" to "dgfygyug", "name" to "Foobar")), + ), + ) + assertEquals(1, questType.getApplicableElements(mapData).toList().size) + } + + @Test fun `not applicable to vacant shop`() { + val mapData = TestMapDataWithGeometry( + listOf( + node(1, tags = mapOf("shop" to "vacant", "name" to "Foobar")), + ), + ) + assertEquals(0, questType.getApplicableElements(mapData).toList().size) + } } diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt index 10416556d6d..980d647bc93 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/oneway_suspects/TrafficFlowSegmentsApiTest.kt @@ -5,51 +5,85 @@ import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.quests.oneway_suspects.data.ONEWAY_API_URL import de.westnordost.streetcomplete.quests.oneway_suspects.data.TrafficFlowSegment import de.westnordost.streetcomplete.quests.oneway_suspects.data.TrafficFlowSegmentsApi +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.respondBadRequest +import io.ktor.client.engine.mock.respondError +import io.ktor.client.engine.mock.respondOk +import io.ktor.client.plugins.ClientRequestException +import io.ktor.client.plugins.ServerResponseException +import io.ktor.http.HttpStatusCode +import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertTrue +import kotlin.test.assertFailsWith class TrafficFlowSegmentsApiTest { + private val boundingBox = BoundingBox( + -34.0, + 18.0, + -33.0, + 19.0 + ) - @Test fun parseEmptyDoesNotResultInError() { - val result = TrafficFlowSegmentsApi.parse("{\"segments\":[]}") - assertTrue(result.isEmpty()) + @Test fun `get with empty response does not result in error`(): Unit = runBlocking { + val mockEngine = MockEngine { request -> respondOk("""{"segments": []}""") } + + assertEquals(mapOf(), TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox)) } - @Test fun parseTwoOfDifferentWay() { + @Test fun `get with two different ways`() = runBlocking { + val mockEngine = MockEngine { request -> respondOk(""" + {"segments":[ + {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}}, + {"wayId":2,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}} + ]} + """) } + assertEquals( mapOf( 1L to listOf(TrafficFlowSegment(1L, LatLon(2.0, 1.0), LatLon(6.0, 5.0))), 2L to listOf(TrafficFlowSegment(2L, LatLon(4.0, 3.0), LatLon(8.0, 7.0))) ), - TrafficFlowSegmentsApi.parse(""" - {"segments":[ - {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}}, - {"wayId":2,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}} - ]} - """.trimIndent() - ) + TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox) ) } - @Test fun parseTwoOfSameWay() { + @Test fun `get with two of same way`() = runBlocking { + val mockEngine = MockEngine { request -> respondOk(""" + {"segments":[ + {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}}, + {"wayId":1,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}} + ]} + """) } + assertEquals( mapOf(1L to listOf( TrafficFlowSegment(1L, LatLon(2.0, 1.0), LatLon(6.0, 5.0)), TrafficFlowSegment(1L, LatLon(4.0, 3.0), LatLon(8.0, 7.0)) )), - TrafficFlowSegmentsApi.parse(""" - {"segments":[ - {"wayId":1,"fromPosition":{"lon":1, "lat":2},"toPosition":{"lon":5, "lat":6}}, - {"wayId":1,"fromPosition":{"lon":3, "lat":4},"toPosition":{"lon":7, "lat":8}} - ]} - """.trimIndent() - ) + TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox) ) } - @Test fun withSomeRealData() { - // should just not crash... - TrafficFlowSegmentsApi(ONEWAY_API_URL).get(BoundingBox(-34.0, 18.0, -33.0, 19.0)) + @Test fun `get throws an ClientRequestException on a 400 response`(): Unit = runBlocking { + val mockEngine = MockEngine { request -> respondBadRequest() } + assertFailsWith { TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox) } + } + + @Test fun `get throws an ServerResponseException on a 500 response`(): Unit = runBlocking { + val mockEngine = MockEngine { request -> respondError(HttpStatusCode.InternalServerError) } + assertFailsWith { TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox) } + } + + @Test fun `get makes request to correct URL`(): Unit = runBlocking { + val mockEngine = MockEngine { request -> respondOk("""{"segments": []}""") } + + TrafficFlowSegmentsApi(HttpClient(mockEngine), ONEWAY_API_URL).get(boundingBox) + + assertEquals( + ONEWAY_API_URL + "?bbox=18.0000000,-34.0000000,19.0000000,-33.0000000", + mockEngine.requestHistory[0].url.toString() + ) } } diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/smoking/AddSmokingTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/smoking/AddSmokingTest.kt index aaf7ae06b7a..aa211aaf609 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/smoking/AddSmokingTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/smoking/AddSmokingTest.kt @@ -50,7 +50,7 @@ class AddSmokingTest { )))) } - /* we assume that seating is not present if not indicated for bakery and similar */ + // we assume that seating is not present if not indicated for bakery and similar @Test fun `not applicable to bakery without indicated seating`() { assertFalse(questType.isApplicableTo(node(tags = mapOf( "shop" to "bakery", @@ -105,7 +105,7 @@ class AddSmokingTest { )))) } - /* nighclubs etc. may have outdoor smoking areas even if no seating is present */ + // nighclubs etc. may have outdoor smoking areas even if no seating is present @Test fun `applicable to nightclub without any seating`() { assertTrue(questType.isApplicableTo(node(tags = mapOf( "amenity" to "nightclub", @@ -120,7 +120,7 @@ class AddSmokingTest { )))) } - /* we assume that seating is present if not indicated for cafe and similar */ + // we assume that seating is present if not indicated for cafe and similar @Test fun `applicable to cafe without indicated seating`() { assertTrue(questType.isApplicableTo(node(tags = mapOf( "amenity" to "cafe", diff --git a/app/src/test/resources/image_uploader/empty.jpg b/app/src/test/resources/image_uploader/empty.jpg new file mode 100644 index 00000000000..e69de29bb2d diff --git a/app/src/test/resources/image_uploader/invalid.jpg b/app/src/test/resources/image_uploader/invalid.jpg new file mode 100644 index 00000000000..9977a2836c1 --- /dev/null +++ b/app/src/test/resources/image_uploader/invalid.jpg @@ -0,0 +1 @@ +invalid diff --git a/app/src/test/resources/image_uploader/ioexception.jpg b/app/src/test/resources/image_uploader/ioexception.jpg new file mode 100644 index 00000000000..abfff0458b5 --- /dev/null +++ b/app/src/test/resources/image_uploader/ioexception.jpg @@ -0,0 +1 @@ +ioexception diff --git a/app/src/test/resources/image_uploader/valid.jpg b/app/src/test/resources/image_uploader/valid.jpg new file mode 100644 index 00000000000..1e2466dfadd --- /dev/null +++ b/app/src/test/resources/image_uploader/valid.jpg @@ -0,0 +1 @@ +valid diff --git a/buildSrc/src/main/java/DownloadAndConvertPresetIconsTask.kt b/buildSrc/src/main/java/DownloadAndConvertPresetIconsTask.kt index 170e2688d83..3f81275073b 100644 --- a/buildSrc/src/main/java/DownloadAndConvertPresetIconsTask.kt +++ b/buildSrc/src/main/java/DownloadAndConvertPresetIconsTask.kt @@ -109,7 +109,7 @@ open class DownloadAndConvertPresetIconsTask : DefaultTask() { } require(root != null) { "No root node found" } - require(root.tagName == "svg") { "Root must be " } + require(root.tagName == "svg") { "Root must be " } var width = root.getAttribute("width") var height = root.getAttribute("height")