From 4a41bb1ec83981a7b942622026a2ca3e907f29b4 Mon Sep 17 00:00:00 2001 From: Flo Edelmann Date: Wed, 28 Apr 2021 21:18:35 +0200 Subject: [PATCH 1/3] Wrap some osmapi exceptions in Kotlin exceptions * AuthorizationException * ConflictException * QueryTooBigException --- .../streetcomplete/MainActivity.kt | 4 +- .../osm/edits/upload/ElementEditUploader.kt | 17 +------ .../changesets/ChangesetAutoCloserWorker.kt | 4 +- .../changesets/OpenQuestChangesetsManager.kt | 9 ++-- .../data/osm/mapdata/MapDataApi.kt | 37 +++++--------- .../data/osm/mapdata/MapDataApiImpl.kt | 51 ++++++++++++++----- .../data/osm/mapdata/MapDataDownloader.kt | 4 +- .../streetcomplete/data/osmnotes/NotesApi.kt | 18 +++---- .../data/osmnotes/NotesApiImpl.kt | 28 +++++++--- .../data/osmnotes/edits/NoteEditsUploader.kt | 20 +------- .../data/upload/QueryTooBigException.kt | 4 ++ .../streetcomplete/data/upload/Uploader.kt | 6 +-- .../data/user/AuthorizationException.kt | 4 ++ .../edits/upload/ElementEditUploaderTest.kt | 7 ++- .../osmnotes/edits/NoteEditsUploaderTest.kt | 7 ++- 15 files changed, 112 insertions(+), 108 deletions(-) create mode 100644 app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt create mode 100644 app/src/main/java/de/westnordost/streetcomplete/data/user/AuthorizationException.kt diff --git a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt index 80e62393e8..202534d325 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt @@ -23,7 +23,6 @@ import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.preference.PreferenceManager import de.westnordost.osmapi.common.errors.OsmApiException import de.westnordost.osmapi.common.errors.OsmApiReadResponseException -import de.westnordost.osmapi.common.errors.OsmAuthorizationException import de.westnordost.osmapi.common.errors.OsmConnectionException import de.westnordost.streetcomplete.Injector.applicationComponent import de.westnordost.streetcomplete.controls.NotificationButtonFragment @@ -37,6 +36,7 @@ import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.upload.UploadController import de.westnordost.streetcomplete.data.upload.UploadProgressListener import de.westnordost.streetcomplete.data.upload.VersionBannedException +import de.westnordost.streetcomplete.data.user.AuthorizationException import de.westnordost.streetcomplete.data.user.UserController import de.westnordost.streetcomplete.ktx.toast import de.westnordost.streetcomplete.location.LocationRequestFragment @@ -237,7 +237,7 @@ class MainActivity : AppCompatActivity(), // a 5xx error is not the fault of this app. Nothing we can do about it, so // just notify the user toast(R.string.upload_server_error, Toast.LENGTH_LONG) - } else if (e is OsmAuthorizationException) { + } else if (e is AuthorizationException) { // delete secret in case it failed while already having a token -> token is invalid userController.logOut() RequestLoginDialog(this@MainActivity).show() diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt index 9f480686e4..e41670f1ba 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt @@ -1,7 +1,5 @@ package de.westnordost.streetcomplete.data.osm.edits.upload -import de.westnordost.osmapi.common.errors.OsmApiException -import de.westnordost.osmapi.common.errors.OsmConflictException import de.westnordost.streetcomplete.data.osm.edits.ElementEdit import de.westnordost.streetcomplete.data.osm.edits.ElementIdProvider import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenQuestChangesetsManager @@ -25,21 +23,10 @@ class ElementEditUploader @Inject constructor( return try { val changesetId = changesetManager.getOrCreateChangeset(edit.questType, edit.source) - uploadInChangeset(changesetId, mapDataChanges) + mapDataApi.uploadChanges(changesetId, mapDataChanges) } catch (e: ConflictException) { val changesetId = changesetManager.createChangeset(edit.questType, edit.source) - uploadInChangeset(changesetId, mapDataChanges) - } - } - - /** Upload the changes for a single change. Returns the updated element(s). */ - private fun uploadInChangeset(changesetId: Long, mapDataChanges: MapDataChanges): MapDataUpdates { - try { - return mapDataApi.uploadChanges(changesetId, mapDataChanges) - } catch (e: OsmConflictException) { - throw ConflictException(e.message, e) - } catch (e: OsmApiException) { - throw ConflictException(e.message, e) + mapDataApi.uploadChanges(changesetId, mapDataChanges) } } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt index 767b1731aa..5598aca267 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt @@ -6,9 +6,9 @@ import javax.inject.Inject import androidx.work.Worker import androidx.work.WorkerParameters -import de.westnordost.osmapi.common.errors.OsmAuthorizationException import de.westnordost.osmapi.common.errors.OsmConnectionException import de.westnordost.streetcomplete.Injector +import de.westnordost.streetcomplete.data.user.AuthorizationException class ChangesetAutoCloserWorker(context: Context, workerParams: WorkerParameters) : Worker(context, workerParams) { @@ -26,7 +26,7 @@ class ChangesetAutoCloserWorker(context: Context, workerParams: WorkerParameters // wasn't able to connect to the server (i.e. connection timeout). Oh well, then, // never mind. Could also retry later with Result.retry() but the OSM API closes open // changesets after 1 hour anyway. - } catch (e: OsmAuthorizationException) { + } catch (e: AuthorizationException) { // the user may not be authorized yet (or not be authorized anymore) #283 // nothing we can do about here. He will have to reauthenticate when he next opens the app return Result.failure() diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/OpenQuestChangesetsManager.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/OpenQuestChangesetsManager.kt index 74a9bd3aa4..678a725206 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/OpenQuestChangesetsManager.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/OpenQuestChangesetsManager.kt @@ -1,14 +1,13 @@ package de.westnordost.streetcomplete.data.osm.edits.upload.changesets import android.util.Log -import de.westnordost.streetcomplete.data.osm.mapdata.MapDataApi - -import de.westnordost.osmapi.common.errors.OsmConflictException import de.westnordost.streetcomplete.ApplicationConstants.QUESTTYPE_TAG_KEY import de.westnordost.streetcomplete.ApplicationConstants.USER_AGENT import de.westnordost.streetcomplete.data.osm.edits.upload.LastEditTimeStore -import de.westnordost.streetcomplete.data.quest.QuestType +import de.westnordost.streetcomplete.data.osm.mapdata.MapDataApi import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType +import de.westnordost.streetcomplete.data.quest.QuestType +import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.ktx.toBcp47LanguageTag import java.util.Locale import javax.inject.Inject @@ -45,7 +44,7 @@ class OpenQuestChangesetsManager @Inject constructor( try { mapDataApi.closeChangeset(info.changesetId) Log.i(TAG, "Closed changeset #${info.changesetId}") - } catch (e: OsmConflictException) { + } catch (e: ConflictException) { Log.w(TAG, "Couldn't close changeset #${info.changesetId} because it has already been closed") } finally { openChangesetsDB.delete(info.questType, info.source) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt index 50cf729846..d2663c98f9 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt @@ -1,8 +1,9 @@ package de.westnordost.streetcomplete.data.osm.mapdata -import de.westnordost.osmapi.common.errors.* +import de.westnordost.streetcomplete.data.upload.ConflictException +import de.westnordost.streetcomplete.data.upload.QueryTooBigException +import de.westnordost.streetcomplete.data.user.AuthorizationException -// TODO create own exception classes /** Get and upload changes to map data */ interface MapDataApi : MapDataRepository { @@ -12,15 +13,11 @@ interface MapDataApi : MapDataRepository { * @param changesetId id of the changeset to upload changes into * @param changes changes to upload. * - * @throws OsmNotFoundException if the changeset does not exist (yet) or an element in the - * does not exist - * @throws OsmConflictException if the changeset has already been closed, there is a conflict - * for the elements being uploaded or the user who created the - * changeset is not the same as the one uploading the change - * @throws OsmAuthorizationException if the application does not have permission to edit the - * map (Permission.MODIFY_MAP) - * @throws OsmPreconditionFailedException if the deletion of an element was uploaded but that - * element is still referred to by another element + * @throws ConflictException if the changeset has already been closed, there is a conflict for + * the elements being uploaded or the user who created the changeset + * is not the same as the one uploading the change + * @throws AuthorizationException if the application does not have permission to edit the map + * (Permission.MODIFY_MAP) * * @return the updated elements */ @@ -31,8 +28,8 @@ interface MapDataApi : MapDataRepository { * * @param tags tags of this changeset. Usually it is comment and source. * - * @throws OsmAuthorizationException if the application does not have permission to edit the - * map (Permission.MODIFY_MAP) + * @throws AuthorizationException if the application does not have permission to edit the map + * (Permission.MODIFY_MAP) * @return the id of the changeset */ fun openChangeset(tags: Map): Long @@ -42,10 +39,9 @@ interface MapDataApi : MapDataRepository { * * @param changesetId id of the changeset to close * - * @throws OsmConflictException if the changeset has already been closed - * @throws OsmNotFoundException if the changeset does not exist (yet) - * @throws OsmAuthorizationException if the application does not have permission to edit the - * map (Permission.MODIFY_MAP) + * @throws ConflictException if the changeset has already been closed + * @throws AuthorizationException if the application does not have permission to edit the map + * (Permission.MODIFY_MAP) */ fun closeChangeset(changesetId: Long) @@ -58,8 +54,7 @@ interface MapDataApi : MapDataRepository { * @param mutableMapData mutable map data to add the add the data to * @param ignoreRelationTypes don't put any relations of the given types in the given mutableMapData * - * @throws OsmQueryTooBigException if the bounds are is too large - * @throws IllegalArgumentException if the bounds cross the 180th meridian. + * @throws QueryTooBigException if the bounds area is too large * * @return the map data */ @@ -71,8 +66,6 @@ interface MapDataApi : MapDataRepository { * * @param id the way's id * - * @throws OsmNotFoundException if the way with the given id does not exist - * * @return the map data */ override fun getWayComplete(id: Long): MapData? @@ -84,8 +77,6 @@ interface MapDataApi : MapDataRepository { * * @param id the relation's id * - * @throws OsmNotFoundException if the relation with the given id does not exist - * * @return the map data */ override fun getRelationComplete(id: Long): MapData? diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt index ffd668b9ae..63d8ad9e8e 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt @@ -1,7 +1,7 @@ package de.westnordost.streetcomplete.data.osm.mapdata import de.westnordost.osmapi.OsmConnection -import de.westnordost.osmapi.common.errors.OsmNotFoundException +import de.westnordost.osmapi.common.errors.* import de.westnordost.osmapi.map.data.* import de.westnordost.osmapi.map.MapDataApi as OsmApiMapDataApi @@ -14,6 +14,9 @@ import de.westnordost.osmapi.map.data.BoundingBox as OsmApiBoundingBox import de.westnordost.osmapi.map.changes.DiffElement as OsmApiDiffElement import de.westnordost.osmapi.map.handler.MapDataHandler +import de.westnordost.streetcomplete.data.upload.ConflictException +import de.westnordost.streetcomplete.data.upload.QueryTooBigException +import de.westnordost.streetcomplete.data.user.AuthorizationException import java.time.Instant class MapDataApiImpl(osm: OsmConnection) : MapDataApi { @@ -21,23 +24,47 @@ class MapDataApiImpl(osm: OsmConnection) : MapDataApi { private val api: OsmApiMapDataApi = OsmApiMapDataApi(osm) override fun uploadChanges(changesetId: Long, changes: MapDataChanges): MapDataUpdates { - val handler = UpdatedElementsHandler() - api.uploadChanges(changesetId, changes.toOsmApiElements()) { - handler.handle(it.toDiffElement()) + try { + val handler = UpdatedElementsHandler() + api.uploadChanges(changesetId, changes.toOsmApiElements()) { + handler.handle(it.toDiffElement()) + } + val allChangedElements = changes.creations + changes.modifications + changes.deletions + return handler.getElementUpdates(allChangedElements) + } catch (e: OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } catch (e: OsmConflictException) { + throw ConflictException(e.message, e) + } catch (e: OsmApiException) { + throw ConflictException(e.message, e) } - val allChangedElements = changes.creations + changes.modifications + changes.deletions - return handler.getElementUpdates(allChangedElements) } - override fun openChangeset(tags: Map): Long = api.openChangeset(tags) + override fun openChangeset(tags: Map): Long = + try { + api.openChangeset(tags) + } catch (e: OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } catch (e: OsmConflictException) { + throw ConflictException(e.message, e) + } - override fun closeChangeset(changesetId: Long) = api.closeChangeset(changesetId) + override fun closeChangeset(changesetId: Long) = + try { + api.closeChangeset(changesetId) + } catch (e: OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } override fun getMap(bounds: BoundingBox, mutableMapData: MutableMapData, ignoreRelationTypes: Set) = - api.getMap( - bounds.toOsmApiBoundingBox(), - MapDataApiHandler(mutableMapData, ignoreRelationTypes) - ) + try { + api.getMap( + bounds.toOsmApiBoundingBox(), + MapDataApiHandler(mutableMapData, ignoreRelationTypes) + ) + } catch (e: OsmQueryTooBigException) { + throw QueryTooBigException(e.message, e) + } override fun getWayComplete(id: Long): MapData? = try { diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt index 1ed1217e31..3b27fec8e7 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt @@ -1,8 +1,8 @@ package de.westnordost.streetcomplete.data.osm.mapdata import android.util.Log -import de.westnordost.osmapi.common.errors.OsmQueryTooBigException import de.westnordost.streetcomplete.ApplicationConstants +import de.westnordost.streetcomplete.data.upload.QueryTooBigException import de.westnordost.streetcomplete.ktx.format import de.westnordost.streetcomplete.util.enlargedBy import kotlinx.coroutines.Dispatchers @@ -34,7 +34,7 @@ class MapDataDownloader @Inject constructor( private fun getMapAndHandleTooBigQuery(bounds: BoundingBox, mutableMapData: MutableMapData) { try { mapDataApi.getMap(bounds, mutableMapData, ApplicationConstants.IGNORED_RELATION_TYPES) - } catch (e : OsmQueryTooBigException) { + } catch (e : QueryTooBigException) { for (subBounds in bounds.splitIntoFour()) { getMapAndHandleTooBigQuery(subBounds, mutableMapData) } diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt index c4bdcf5b96..c3b92f9c19 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt @@ -1,10 +1,10 @@ package de.westnordost.streetcomplete.data.osmnotes -import de.westnordost.osmapi.common.errors.* import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.upload.ConflictException +import de.westnordost.streetcomplete.data.user.AuthorizationException -// TODO create own exception classes /** * Creates, comments, closes, reopens and search for notes. * All interactions with this class require an OsmConnection with a logged in user. @@ -16,8 +16,8 @@ interface NotesApi { * @param pos position of the note. * @param text text for the new note. Must not be empty. * - * @throws OsmAuthorizationException if this application is not authorized to write notes - * (Permission.WRITE_NOTES) + * @throws AuthorizationException if this application is not authorized to write notes + * (Permission.WRITE_NOTES) * * @return the new note */ @@ -27,10 +27,9 @@ interface NotesApi { * @param id id of the note * @param text comment to be added to the note. Must not be empty * - * @throws OsmConflictException if the note has already been closed. - * @throws OsmAuthorizationException if this application is not authorized to write notes - * (Permission.WRITE_NOTES) - * @throws OsmNotFoundException if the note with the given id does not exist (anymore) + * @throws ConflictException if the note has already been closed or doesn't exist (anymore). + * @throws AuthorizationException if this application is not authorized to write notes + * (Permission.WRITE_NOTES) * * @return the updated commented note */ @@ -53,9 +52,6 @@ interface NotesApi { * -1 means that all notes should be returned, 0 that only open notes * are returned. * - * @throws OsmQueryTooBigException if the bounds area is too large - * @throws IllegalArgumentException if the bounds cross the 180th meridian - * * @return the incoming notes */ suspend fun getAll(bounds: BoundingBox, limit: Int, hideClosedNoteAfter: Int): List diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt index 33515ba9c1..9a85caefd1 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt @@ -1,9 +1,14 @@ package de.westnordost.streetcomplete.data.osmnotes import de.westnordost.osmapi.OsmConnection +import de.westnordost.osmapi.common.errors.OsmAuthorizationException +import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.common.errors.OsmNotFoundException import de.westnordost.osmapi.map.data.OsmLatLon import de.westnordost.osmapi.notes.NotesApi as OsmApiNotesApi import de.westnordost.streetcomplete.data.osm.mapdata.* +import de.westnordost.streetcomplete.data.upload.ConflictException +import de.westnordost.streetcomplete.data.user.AuthorizationException import de.westnordost.streetcomplete.data.user.User import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -16,9 +21,22 @@ open class NotesApiImpl(osm: OsmConnection) : NotesApi { private val api: OsmApiNotesApi = OsmApiNotesApi(osm) override fun create(pos: LatLon, text: String): Note = - api.create(OsmLatLon(pos.latitude, pos.longitude), text).toNote() + try { + api.create(OsmLatLon(pos.latitude, pos.longitude), text).toNote() + } catch (e: OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } - override fun comment(id: Long, text: String): Note = api.comment(id, text).toNote() + override fun comment(id: Long, text: String): Note = + try { + api.comment(id, text).toNote() + } catch (e: OsmNotFoundException) { + // someone else already closed the note -> our contribution is probably worthless + throw ConflictException(e.message, e) + } catch (e: OsmConflictException) { + // was closed by admin + throw ConflictException(e.message, e) + } override fun get(id: Long): Note? = api.get(id)?.toNote() @@ -27,10 +45,8 @@ open class NotesApiImpl(osm: OsmConnection) : NotesApi { val notes = ArrayList() api.getAll( OsmApiBoundingBox( - bounds.min.latitude, - bounds.min.longitude, - bounds.max.latitude, - bounds.max.longitude + bounds.min.latitude, bounds.min.longitude, + bounds.max.latitude, bounds.max.longitude ), null, { notes.add(it.toNote()) }, 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 3ab86ff4a8..6dbb6ea3a9 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 @@ -1,9 +1,6 @@ package de.westnordost.streetcomplete.data.osmnotes.edits import android.util.Log -import de.westnordost.osmapi.common.errors.OsmConflictException -import de.westnordost.osmapi.common.errors.OsmNotFoundException -import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.* import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditAction.* @@ -62,8 +59,8 @@ class NoteEditsUploader @Inject constructor( try { val note = when(edit.action) { - CREATE -> uploadCreateNote(edit.position, text) - COMMENT -> uploadCommentNote(edit.noteId, text) + CREATE -> notesApi.create(edit.position, text) + COMMENT -> notesApi.comment(edit.noteId, text) } Log.d(TAG, @@ -99,19 +96,6 @@ class NoteEditsUploader @Inject constructor( } } - private fun uploadCreateNote(pos: LatLon, text: String): Note = - notesApi.create(pos, text) - - private fun uploadCommentNote(noteId: Long, text: String): Note = try { - notesApi.comment(noteId, text) - } catch (e: OsmNotFoundException) { - // someone else already closed the note -> our contribution is probably worthless - throw ConflictException(e.message, e) - } catch (e: OsmConflictException) { - // was closed by admin - throw ConflictException(e.message, e) - } - private fun uploadAndGetAttachedPhotosText(imagePaths: List): String { if (imagePaths.isNotEmpty()) { val urls = imageUploader.upload(imagePaths) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt b/app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt new file mode 100644 index 0000000000..fb2a332044 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt @@ -0,0 +1,4 @@ +package de.westnordost.streetcomplete.data.upload + +class QueryTooBigException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : RuntimeException(message, cause) 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 8ae3f9132c..b5060348e9 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 @@ -1,19 +1,17 @@ package de.westnordost.streetcomplete.data.upload import android.util.Log -import de.westnordost.osmapi.common.errors.OsmAuthorizationException import de.westnordost.streetcomplete.ApplicationConstants import de.westnordost.streetcomplete.data.download.tiles.DownloadedTilesDao import de.westnordost.streetcomplete.data.osm.edits.upload.ElementEditsUploader import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsUploader +import de.westnordost.streetcomplete.data.user.AuthorizationException import de.westnordost.streetcomplete.data.user.UserController import de.westnordost.streetcomplete.util.enclosingTilePos import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import javax.inject.Inject @@ -52,7 +50,7 @@ class Uploader @Inject constructor( // let's fail early in case of no authorization if (!userController.isLoggedIn) { - throw OsmAuthorizationException(401, "Unauthorized", "User is not authorized") + throw AuthorizationException("User is not authorized") } Log.i(TAG, "Starting upload") diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/user/AuthorizationException.kt b/app/src/main/java/de/westnordost/streetcomplete/data/user/AuthorizationException.kt new file mode 100644 index 0000000000..a06aa621ae --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/user/AuthorizationException.kt @@ -0,0 +1,4 @@ +package de.westnordost.streetcomplete.data.user + +class AuthorizationException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : RuntimeException(message, cause) diff --git a/app/src/test/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploaderTest.kt b/app/src/test/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploaderTest.kt index 6a61122756..3e662a86a3 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploaderTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploaderTest.kt @@ -1,6 +1,5 @@ package de.westnordost.streetcomplete.data.osm.edits.upload -import de.westnordost.osmapi.common.errors.OsmConflictException import de.westnordost.streetcomplete.data.osm.mapdata.MapDataApi import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenQuestChangesetsManager import de.westnordost.streetcomplete.data.osm.mapdata.MapDataUpdates @@ -49,8 +48,8 @@ class ElementEditUploaderTest { on(changesetManager.getOrCreateChangeset(any(), any())).thenReturn(1) on(changesetManager.createChangeset(any(), any())).thenReturn(1) on(mapDataApi.uploadChanges(anyLong(), any())) - .thenThrow(OsmConflictException(1, "", "")) - .thenThrow(OsmConflictException(1, "", "")) + .thenThrow(ConflictException()) + .thenThrow(ConflictException()) uploader.upload(edit(element = node(1)), mock()) } @@ -60,7 +59,7 @@ class ElementEditUploaderTest { on(mapDataApi.getNode(anyLong())).thenReturn(node) on(changesetManager.getOrCreateChangeset(any(), any())).thenReturn(1) on(changesetManager.createChangeset(any(), any())).thenReturn(1) - doThrow(OsmConflictException(1, "", "")).doAnswer { MapDataUpdates() } + doThrow(ConflictException()).doAnswer { MapDataUpdates() } .on(mapDataApi).uploadChanges(anyLong(), any()) uploader.upload(edit(element = node(1)), mock()) 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 5dde232dc5..a4803b254f 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,10 +1,9 @@ package de.westnordost.streetcomplete.data.osmnotes.edits -import de.westnordost.osmapi.common.errors.OsmConflictException -import de.westnordost.osmapi.common.errors.OsmNotFoundException 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.upload.ConflictException import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener import de.westnordost.streetcomplete.testutils.* import de.westnordost.streetcomplete.testutils.any @@ -90,7 +89,7 @@ class NoteEditsUploaderTest { val note = note(1) on(noteEditsController.getOldestUnsynced()).thenReturn(edit).thenReturn(null) - on(notesApi.comment(anyLong(), any())).thenThrow(OsmConflictException(403,"","")) + on(notesApi.comment(anyLong(), any())).thenThrow(ConflictException()) on(notesApi.get(1L)).thenReturn(note) upload() @@ -108,7 +107,7 @@ class NoteEditsUploaderTest { val note = note(1) on(noteEditsController.getOldestUnsynced()).thenReturn(edit).thenReturn(null) - on(notesApi.comment(anyLong(), any())).thenThrow(OsmNotFoundException(410,"","")) + on(notesApi.comment(anyLong(), any())).thenThrow(ConflictException()) on(notesApi.get(1L)).thenReturn(null) upload() From 460691d9c8229f74a958b82f43f09d52167f78c2 Mon Sep 17 00:00:00 2001 From: Flo Edelmann Date: Sun, 2 May 2021 17:00:44 +0200 Subject: [PATCH 2/3] Change package of `QueryTooBigException` --- .../data/{upload => download}/QueryTooBigException.kt | 2 +- .../westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt | 2 +- .../streetcomplete/data/osm/mapdata/MapDataApiImpl.kt | 2 +- .../streetcomplete/data/osm/mapdata/MapDataDownloader.kt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename app/src/main/java/de/westnordost/streetcomplete/data/{upload => download}/QueryTooBigException.kt (73%) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt b/app/src/main/java/de/westnordost/streetcomplete/data/download/QueryTooBigException.kt similarity index 73% rename from app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt rename to app/src/main/java/de/westnordost/streetcomplete/data/download/QueryTooBigException.kt index fb2a332044..f97ac54293 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/upload/QueryTooBigException.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/QueryTooBigException.kt @@ -1,4 +1,4 @@ -package de.westnordost.streetcomplete.data.upload +package de.westnordost.streetcomplete.data.download class QueryTooBigException @JvmOverloads constructor( message: String? = null, cause: Throwable? = null) : RuntimeException(message, cause) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt index d2663c98f9..2b11b89275 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt @@ -1,7 +1,7 @@ package de.westnordost.streetcomplete.data.osm.mapdata +import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.data.upload.ConflictException -import de.westnordost.streetcomplete.data.upload.QueryTooBigException import de.westnordost.streetcomplete.data.user.AuthorizationException /** Get and upload changes to map data */ diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt index 63d8ad9e8e..a6df1553d9 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt @@ -14,8 +14,8 @@ import de.westnordost.osmapi.map.data.BoundingBox as OsmApiBoundingBox import de.westnordost.osmapi.map.changes.DiffElement as OsmApiDiffElement import de.westnordost.osmapi.map.handler.MapDataHandler +import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.data.upload.ConflictException -import de.westnordost.streetcomplete.data.upload.QueryTooBigException import de.westnordost.streetcomplete.data.user.AuthorizationException import java.time.Instant diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt index 3b27fec8e7..8dd6234295 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataDownloader.kt @@ -2,7 +2,7 @@ package de.westnordost.streetcomplete.data.osm.mapdata import android.util.Log import de.westnordost.streetcomplete.ApplicationConstants -import de.westnordost.streetcomplete.data.upload.QueryTooBigException +import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.ktx.format import de.westnordost.streetcomplete.util.enlargedBy import kotlinx.coroutines.Dispatchers From 0ec0ac93e766d66a11a15f1f10bce7fd02086d23 Mon Sep 17 00:00:00 2001 From: Flo Edelmann Date: Sun, 2 May 2021 18:05:56 +0200 Subject: [PATCH 3/3] Add `ConnectionException` and `wrapExceptions` helper function --- .../streetcomplete/MainActivity.kt | 22 ++---- .../data/download/ConnectionException.kt | 4 ++ .../changesets/ChangesetAutoCloserWorker.kt | 4 +- .../data/osm/mapdata/MapDataApi.kt | 25 +++++++ .../data/osm/mapdata/MapDataApiImpl.kt | 67 +++++++++++-------- .../streetcomplete/data/osmnotes/NotesApi.kt | 10 +++ .../data/osmnotes/NotesApiImpl.kt | 64 ++++++++++++------ 7 files changed, 129 insertions(+), 67 deletions(-) create mode 100644 app/src/main/java/de/westnordost/streetcomplete/data/download/ConnectionException.kt diff --git a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt index 202534d325..4bb7f5a4ce 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/MainActivity.kt @@ -21,9 +21,6 @@ import androidx.core.content.getSystemService import androidx.lifecycle.lifecycleScope import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.preference.PreferenceManager -import de.westnordost.osmapi.common.errors.OsmApiException -import de.westnordost.osmapi.common.errors.OsmApiReadResponseException -import de.westnordost.osmapi.common.errors.OsmConnectionException import de.westnordost.streetcomplete.Injector.applicationComponent import de.westnordost.streetcomplete.controls.NotificationButtonFragment import de.westnordost.streetcomplete.data.download.DownloadController @@ -32,6 +29,7 @@ import de.westnordost.streetcomplete.data.notifications.Notification import de.westnordost.streetcomplete.data.quest.Quest import de.westnordost.streetcomplete.data.quest.QuestAutoSyncer import de.westnordost.streetcomplete.data.UnsyncedChangesCountSource +import de.westnordost.streetcomplete.data.download.ConnectionException import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.upload.UploadController import de.westnordost.streetcomplete.data.upload.UploadProgressListener @@ -233,9 +231,9 @@ class MainActivity : AppCompatActivity(), messageView.movementMethod = LinkMovementMethod.getInstance() Linkify.addLinks(messageView, Linkify.WEB_URLS) } - } else if (e is OsmConnectionException) { - // a 5xx error is not the fault of this app. Nothing we can do about it, so - // just notify the user + } else if (e is ConnectionException) { + // A network connection error is not the fault of this app. Nothing we can do about + // it, so it does not make sense to send an error report. Just notify the user. toast(R.string.upload_server_error, Toast.LENGTH_LONG) } else if (e is AuthorizationException) { // delete secret in case it failed while already having a token -> token is invalid @@ -253,15 +251,9 @@ class MainActivity : AppCompatActivity(), private val downloadProgressListener: DownloadProgressListener = object : DownloadProgressListener { @AnyThread override fun onError(e: Exception) { runOnUiThread { - // a 5xx error is not the fault of this app. Nothing we can do about it, so it does - // not make sense to send an error report. Just notify the user. Further, we treat - // the following errors the same as a (temporary) connection error: - // - an invalid response (OsmApiReadResponseException) - // - request timeout (OsmApiException with error code 408) - val isEnvironmentError = e is OsmConnectionException || - e is OsmApiReadResponseException || - e is OsmApiException && e.errorCode == 408 - if (isEnvironmentError) { + // A network connection error is not the fault of this app. Nothing we can do about + // it, so it does not make sense to send an error report. Just notify the user. + if (e is ConnectionException) { toast(R.string.download_server_error, Toast.LENGTH_LONG) } else { crashReportExceptionHandler.askUserToSendErrorReport(this@MainActivity, R.string.download_error, e) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/download/ConnectionException.kt b/app/src/main/java/de/westnordost/streetcomplete/data/download/ConnectionException.kt new file mode 100644 index 0000000000..a95e1a01e7 --- /dev/null +++ b/app/src/main/java/de/westnordost/streetcomplete/data/download/ConnectionException.kt @@ -0,0 +1,4 @@ +package de.westnordost.streetcomplete.data.download + +class ConnectionException @JvmOverloads constructor( + message: String? = null, cause: Throwable? = null) : RuntimeException(message, cause) diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt index 5598aca267..f0749dcbeb 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/edits/upload/changesets/ChangesetAutoCloserWorker.kt @@ -6,8 +6,8 @@ import javax.inject.Inject import androidx.work.Worker import androidx.work.WorkerParameters -import de.westnordost.osmapi.common.errors.OsmConnectionException import de.westnordost.streetcomplete.Injector +import de.westnordost.streetcomplete.data.download.ConnectionException import de.westnordost.streetcomplete.data.user.AuthorizationException class ChangesetAutoCloserWorker(context: Context, workerParams: WorkerParameters) : @@ -22,7 +22,7 @@ class ChangesetAutoCloserWorker(context: Context, workerParams: WorkerParameters override fun doWork(): Result { try { openQuestChangesetsManager.closeOldChangesets() - } catch (e: OsmConnectionException) { + } catch (e: ConnectionException) { // wasn't able to connect to the server (i.e. connection timeout). Oh well, then, // never mind. Could also retry later with Result.retry() but the OSM API closes open // changesets after 1 hour anyway. diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt index 2b11b89275..91075cafcf 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApi.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.data.osm.mapdata +import de.westnordost.streetcomplete.data.download.ConnectionException import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.data.user.AuthorizationException @@ -18,6 +19,7 @@ interface MapDataApi : MapDataRepository { * is not the same as the one uploading the change * @throws AuthorizationException if the application does not have permission to edit the map * (Permission.MODIFY_MAP) + * @throws ConnectionException if a temporary network connection problem occurs * * @return the updated elements */ @@ -30,6 +32,8 @@ interface MapDataApi : MapDataRepository { * * @throws AuthorizationException if the application does not have permission to edit the map * (Permission.MODIFY_MAP) + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the id of the changeset */ fun openChangeset(tags: Map): Long @@ -42,6 +46,7 @@ interface MapDataApi : MapDataRepository { * @throws ConflictException if the changeset has already been closed * @throws AuthorizationException if the application does not have permission to edit the map * (Permission.MODIFY_MAP) + * @throws ConnectionException if a temporary network connection problem occurs */ fun closeChangeset(changesetId: Long) @@ -55,6 +60,8 @@ interface MapDataApi : MapDataRepository { * @param ignoreRelationTypes don't put any relations of the given types in the given mutableMapData * * @throws QueryTooBigException if the bounds area is too large + * @throws IllegalArgumentException if the bounds cross the 180th meridian. + * @throws ConnectionException if a temporary network connection problem occurs * * @return the map data */ @@ -66,6 +73,8 @@ interface MapDataApi : MapDataRepository { * * @param id the way's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the map data */ override fun getWayComplete(id: Long): MapData? @@ -77,6 +86,8 @@ interface MapDataApi : MapDataRepository { * * @param id the relation's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the map data */ override fun getRelationComplete(id: Long): MapData? @@ -86,6 +97,8 @@ interface MapDataApi : MapDataRepository { * * @param id the node's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the node with the given id or null if it does not exist */ override fun getNode(id: Long): Node? @@ -95,6 +108,8 @@ interface MapDataApi : MapDataRepository { * * @param id the way's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the way with the given id or null if it does not exist */ override fun getWay(id: Long): Way? @@ -104,6 +119,8 @@ interface MapDataApi : MapDataRepository { * * @param id the relation's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the relation with the given id or null if it does not exist */ override fun getRelation(id: Long): Relation? @@ -113,6 +130,8 @@ interface MapDataApi : MapDataRepository { * * @param id the node's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return all ways that reference the node with the given id. Empty if none. */ override fun getWaysForNode(id: Long): List @@ -122,6 +141,8 @@ interface MapDataApi : MapDataRepository { * * @param id the node's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return all relations that reference the node with the given id. Empty if none. */ override fun getRelationsForNode(id: Long): List @@ -131,6 +152,8 @@ interface MapDataApi : MapDataRepository { * * @param id the way's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return all relations that reference the way with the given id. Empty if none. */ override fun getRelationsForWay(id: Long): List @@ -140,6 +163,8 @@ interface MapDataApi : MapDataRepository { * * @param id the relation's id * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return all relations that reference the relation with the given id. Empty if none. */ override fun getRelationsForRelation(id: Long): List diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt index a6df1553d9..e411b187ec 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiImpl.kt @@ -14,6 +14,7 @@ import de.westnordost.osmapi.map.data.BoundingBox as OsmApiBoundingBox import de.westnordost.osmapi.map.changes.DiffElement as OsmApiDiffElement import de.westnordost.osmapi.map.handler.MapDataHandler +import de.westnordost.streetcomplete.data.download.ConnectionException import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.data.user.AuthorizationException @@ -23,53 +24,41 @@ class MapDataApiImpl(osm: OsmConnection) : MapDataApi { private val api: OsmApiMapDataApi = OsmApiMapDataApi(osm) - override fun uploadChanges(changesetId: Long, changes: MapDataChanges): MapDataUpdates { + override fun uploadChanges(changesetId: Long, changes: MapDataChanges) = wrapExceptions { try { val handler = UpdatedElementsHandler() api.uploadChanges(changesetId, changes.toOsmApiElements()) { handler.handle(it.toDiffElement()) } val allChangedElements = changes.creations + changes.modifications + changes.deletions - return handler.getElementUpdates(allChangedElements) - } catch (e: OsmAuthorizationException) { - throw AuthorizationException(e.message, e) - } catch (e: OsmConflictException) { - throw ConflictException(e.message, e) + handler.getElementUpdates(allChangedElements) } catch (e: OsmApiException) { throw ConflictException(e.message, e) } } override fun openChangeset(tags: Map): Long = - try { - api.openChangeset(tags) - } catch (e: OsmAuthorizationException) { - throw AuthorizationException(e.message, e) - } catch (e: OsmConflictException) { - throw ConflictException(e.message, e) - } + wrapExceptions { api.openChangeset(tags) } override fun closeChangeset(changesetId: Long) = try { - api.closeChangeset(changesetId) - } catch (e: OsmAuthorizationException) { - throw AuthorizationException(e.message, e) + wrapExceptions { api.closeChangeset(changesetId) } + } catch (e: OsmNotFoundException) { + throw ConflictException(e.message, e) } override fun getMap(bounds: BoundingBox, mutableMapData: MutableMapData, ignoreRelationTypes: Set) = - try { + wrapExceptions { api.getMap( bounds.toOsmApiBoundingBox(), MapDataApiHandler(mutableMapData, ignoreRelationTypes) ) - } catch (e: OsmQueryTooBigException) { - throw QueryTooBigException(e.message, e) } override fun getWayComplete(id: Long): MapData? = try { val result = MutableMapData() - api.getWayComplete(id, MapDataApiHandler(result)) + wrapExceptions { api.getWayComplete(id, MapDataApiHandler(result)) } result } catch (e: OsmNotFoundException) { null @@ -78,31 +67,53 @@ class MapDataApiImpl(osm: OsmConnection) : MapDataApi { override fun getRelationComplete(id: Long): MapData? = try { val result = MutableMapData() - api.getRelationComplete(id, MapDataApiHandler(result)) + wrapExceptions { api.getRelationComplete(id, MapDataApiHandler(result)) } result } catch (e: OsmNotFoundException) { null } - override fun getNode(id: Long): Node? = api.getNode(id)?.toNode() + override fun getNode(id: Long): Node? = + wrapExceptions { api.getNode(id)?.toNode() } - override fun getWay(id: Long): Way? = api.getWay(id)?.toWay() + override fun getWay(id: Long): Way? = + wrapExceptions { api.getWay(id)?.toWay() } - override fun getRelation(id: Long): Relation? = api.getRelation(id)?.toRelation() + override fun getRelation(id: Long): Relation? = + wrapExceptions { api.getRelation(id)?.toRelation() } override fun getWaysForNode(id: Long): List = - api.getWaysForNode(id).map { it.toWay() } + wrapExceptions { api.getWaysForNode(id).map { it.toWay() } } override fun getRelationsForNode(id: Long): List = - api.getRelationsForNode(id).map { it.toRelation() } + wrapExceptions { api.getRelationsForNode(id).map { it.toRelation() } } override fun getRelationsForWay(id: Long): List = - api.getRelationsForWay(id).map { it.toRelation() } + wrapExceptions { api.getRelationsForWay(id).map { it.toRelation() } } override fun getRelationsForRelation(id: Long): List = - api.getRelationsForRelation(id).map { it.toRelation() } + wrapExceptions { api.getRelationsForRelation(id).map { it.toRelation() } } } +private inline fun wrapExceptions(block: () -> T): T = + try { + block() + } catch (e : OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } catch (e : OsmConflictException) { + throw ConflictException(e.message, e) + } catch (e : OsmQueryTooBigException) { + throw QueryTooBigException(e.message, e) + } catch (e : OsmConnectionException) { + throw ConnectionException(e.message, e) + } catch (e : OsmApiReadResponseException) { + // probably a temporary connection error + throw ConnectionException(e.message, e) + } catch (e : OsmApiException) { + // request timeout is a temporary connection error + throw if (e.errorCode == 408) ConnectionException(e.message, e) else e + } + /* --------------------------------- Element -> OsmApiElement ----------------------------------- */ private fun MapDataChanges.toOsmApiElements(): List = diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt index c3b92f9c19..b679bfff8f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApi.kt @@ -2,6 +2,8 @@ package de.westnordost.streetcomplete.data.osmnotes import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.download.ConnectionException +import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.streetcomplete.data.upload.ConflictException import de.westnordost.streetcomplete.data.user.AuthorizationException @@ -18,6 +20,7 @@ interface NotesApi { * * @throws AuthorizationException if this application is not authorized to write notes * (Permission.WRITE_NOTES) + * @throws ConnectionException if a temporary network connection problem occurs * * @return the new note */ @@ -30,6 +33,7 @@ interface NotesApi { * @throws ConflictException if the note has already been closed or doesn't exist (anymore). * @throws AuthorizationException if this application is not authorized to write notes * (Permission.WRITE_NOTES) + * @throws ConnectionException if a temporary network connection problem occurs * * @return the updated commented note */ @@ -38,6 +42,8 @@ interface NotesApi { /** * @param id id of the note * + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the note with the given id. null if the note with that id does not exist (anymore). */ fun get(id: Long): Note? @@ -52,6 +58,10 @@ interface NotesApi { * -1 means that all notes should be returned, 0 that only open notes * are returned. * + * @throws QueryTooBigException if the bounds area is too large + * @throws IllegalArgumentException if the bounds cross the 180th meridian + * @throws ConnectionException if a temporary network connection problem occurs + * * @return the incoming notes */ suspend fun getAll(bounds: BoundingBox, limit: Int, hideClosedNoteAfter: Int): List diff --git a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt index 9a85caefd1..d55ca5381f 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/data/osmnotes/NotesApiImpl.kt @@ -1,10 +1,16 @@ package de.westnordost.streetcomplete.data.osmnotes import de.westnordost.osmapi.OsmConnection +import de.westnordost.osmapi.common.errors.OsmApiException +import de.westnordost.osmapi.common.errors.OsmApiReadResponseException import de.westnordost.osmapi.common.errors.OsmAuthorizationException import de.westnordost.osmapi.common.errors.OsmConflictException +import de.westnordost.osmapi.common.errors.OsmConnectionException import de.westnordost.osmapi.common.errors.OsmNotFoundException +import de.westnordost.osmapi.common.errors.OsmQueryTooBigException import de.westnordost.osmapi.map.data.OsmLatLon +import de.westnordost.streetcomplete.data.download.ConnectionException +import de.westnordost.streetcomplete.data.download.QueryTooBigException import de.westnordost.osmapi.notes.NotesApi as OsmApiNotesApi import de.westnordost.streetcomplete.data.osm.mapdata.* import de.westnordost.streetcomplete.data.upload.ConflictException @@ -21,42 +27,56 @@ open class NotesApiImpl(osm: OsmConnection) : NotesApi { private val api: OsmApiNotesApi = OsmApiNotesApi(osm) override fun create(pos: LatLon, text: String): Note = - try { - api.create(OsmLatLon(pos.latitude, pos.longitude), text).toNote() - } catch (e: OsmAuthorizationException) { - throw AuthorizationException(e.message, e) - } + wrapExceptions { api.create(OsmLatLon(pos.latitude, pos.longitude), text).toNote() } override fun comment(id: Long, text: String): Note = try { - api.comment(id, text).toNote() + wrapExceptions { api.comment(id, text).toNote() } } catch (e: OsmNotFoundException) { // someone else already closed the note -> our contribution is probably worthless throw ConflictException(e.message, e) - } catch (e: OsmConflictException) { - // was closed by admin - throw ConflictException(e.message, e) } - override fun get(id: Long): Note? = api.get(id)?.toNote() + override fun get(id: Long): Note? = wrapExceptions { api.get(id)?.toNote() } override suspend fun getAll(bounds: BoundingBox, limit: Int, hideClosedNoteAfter: Int) = withContext(Dispatchers.IO) { - val notes = ArrayList() - api.getAll( - OsmApiBoundingBox( - bounds.min.latitude, bounds.min.longitude, - bounds.max.latitude, bounds.max.longitude - ), - null, - { notes.add(it.toNote()) }, - limit, - hideClosedNoteAfter - ) - notes + wrapExceptions { + val notes = ArrayList() + api.getAll( + OsmApiBoundingBox( + bounds.min.latitude, bounds.min.longitude, + bounds.max.latitude, bounds.max.longitude + ), + null, + { notes.add(it.toNote()) }, + limit, + hideClosedNoteAfter + ) + notes + } } } +private inline fun wrapExceptions(block: () -> T): T = + try { + block() + } catch (e : OsmAuthorizationException) { + throw AuthorizationException(e.message, e) + } catch (e : OsmConflictException) { + throw ConflictException(e.message, e) + } catch (e : OsmQueryTooBigException) { + throw QueryTooBigException(e.message, e) + } catch (e : OsmConnectionException) { + throw ConnectionException(e.message, e) + } catch (e : OsmApiReadResponseException) { + // probably a temporary connection error + throw ConnectionException(e.message, e) + } catch (e : OsmApiException) { + // request timeout is a temporary connection error + throw if (e.errorCode == 408) ConnectionException(e.message, e) else e + } + private fun OsmApiNote.toNote() = Note( LatLon(position.latitude, position.longitude), id,