From 2e1efadd74601b9677da0299a5331e18318d49f4 Mon Sep 17 00:00:00 2001
From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com>
Date: Thu, 22 Oct 2026 19:12:38 +0530
Subject: [PATCH 1/3] Refactor HTTP logging to use LogcatHttpLogger; update
database version to 29 and handle log deletion on upgrade
---
.../data/logs/HttpLogsMigrationTest.kt | 33 ++++++++++
.../main/MainActivityWithoutWorkspaceTest.kt | 62 +++++++++++++++++++
.../streetcomplete/ApplicationModule.kt | 28 +++------
.../screens/main/MainActivity.kt | 8 +++
.../util/logs/LogcatHttpLogger.kt | 16 +++++
.../util/logs/LogcatHttpLoggerTest.kt | 33 ++++++++++
.../data/DatabaseInitializer.kt | 12 +++-
7 files changed, 171 insertions(+), 21 deletions(-)
create mode 100644 app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/logs/HttpLogsMigrationTest.kt
create mode 100644 app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MainActivityWithoutWorkspaceTest.kt
create mode 100644 app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLogger.kt
create mode 100644 app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLoggerTest.kt
diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/logs/HttpLogsMigrationTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/logs/HttpLogsMigrationTest.kt
new file mode 100644
index 00000000000..d34c6fc6050
--- /dev/null
+++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/logs/HttpLogsMigrationTest.kt
@@ -0,0 +1,33 @@
+package de.westnordost.streetcomplete.data.logs
+
+import de.westnordost.streetcomplete.data.ApplicationDbTestCase
+import de.westnordost.streetcomplete.data.DatabaseInitializer
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class HttpLogsMigrationTest : ApplicationDbTestCase() {
+ private lateinit var dao: LogsDao
+
+ @BeforeTest fun createDao() {
+ dao = LogsDao(database)
+ }
+
+ @Test fun upgrading_to_29_deletes_stored_http_logs_only() {
+ listOf("OsmClient", "KtorClient", "Download", "QuestAutoSyncer").forEach { tag ->
+ dao.add(LogMessage(LogLevel.DEBUG, tag, "message of $tag", null, 1))
+ }
+
+ DatabaseInitializer.onUpgrade(database, 28, 29)
+
+ assertEquals(listOf("Download", "QuestAutoSyncer"), dao.getAll().map { it.tag }.sorted())
+ }
+
+ @Test fun upgrading_from_before_29_to_later_deletes_stored_http_logs() {
+ dao.add(LogMessage(LogLevel.DEBUG, "OsmClient", "...", null, 1))
+
+ DatabaseInitializer.onUpgrade(database, 28, 30)
+
+ assertEquals(emptyList(), dao.getAll())
+ }
+}
diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MainActivityWithoutWorkspaceTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MainActivityWithoutWorkspaceTest.kt
new file mode 100644
index 00000000000..03af6516337
--- /dev/null
+++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MainActivityWithoutWorkspaceTest.kt
@@ -0,0 +1,62 @@
+package de.westnordost.streetcomplete.screens.main
+
+import android.app.Activity
+import android.app.Instrumentation
+import android.content.Intent
+import android.net.Uri
+import androidx.lifecycle.Lifecycle
+import androidx.test.core.app.ActivityScenario
+import androidx.test.espresso.intent.Intents
+import androidx.test.espresso.intent.Intents.intended
+import androidx.test.espresso.intent.Intents.intending
+import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import de.westnordost.streetcomplete.screens.workspaces.WorkSpaceActivity
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.test.assertEquals
+
+/**
+ * MainActivity started without a workspace's long form - by the sync notification after the
+ * activity was closed, or by a geo:/streetcomplete: link - used to crash with an NPE in
+ * doLongForm. It goes to the workspace list instead.
+ */
+@RunWith(AndroidJUnit4::class)
+class MainActivityWithoutWorkspaceTest {
+
+ @Before fun setUp() {
+ Intents.init()
+ // only record it, don't actually open the workspace list
+ intending(hasComponent(WorkSpaceActivity::class.java.name))
+ .respondWith(Instrumentation.ActivityResult(Activity.RESULT_OK, null))
+ }
+
+ @After fun tearDown() {
+ Intents.release()
+ }
+
+ @Test fun startedLikeTheSyncNotificationGoesToWorkspaceList() {
+ assertGoesToWorkspaceList(
+ Intent(context, MainActivity::class.java)
+ .addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP)
+ )
+ }
+
+ @Test fun startedByGeoLinkGoesToWorkspaceList() {
+ assertGoesToWorkspaceList(
+ Intent(Intent.ACTION_VIEW, Uri.parse("geo:47.6,-122.3"), context, MainActivity::class.java)
+ )
+ }
+
+ private fun assertGoesToWorkspaceList(intent: Intent) {
+ ActivityScenario.launch(intent).use { scenario ->
+ assertEquals(Lifecycle.State.DESTROYED, scenario.state)
+ }
+ intended(hasComponent(WorkSpaceActivity::class.java.name))
+ }
+
+ private val context get() = InstrumentationRegistry.getInstrumentation().targetContext
+}
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
index ac14b4c8e17..9cf9f7c39b7 100644
--- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
@@ -18,6 +18,7 @@ import de.westnordost.streetcomplete.util.ResourceProvider
import de.westnordost.streetcomplete.util.SoundFx
import de.westnordost.streetcomplete.util.logs.DatabaseLogger
import de.westnordost.streetcomplete.util.logs.Log
+import de.westnordost.streetcomplete.util.logs.LogcatHttpLogger
import de.westnordost.streetcomplete.util.network.retryOnTransientHttpFailure
import de.westnordost.streetcomplete.util.satellite_layers.ImageryRepository
import io.ktor.client.HttpClient
@@ -29,9 +30,7 @@ import io.ktor.client.plugins.auth.providers.bearer
import io.ktor.client.plugins.compression.ContentEncoding
import io.ktor.client.plugins.contentnegotiation.ContentNegotiation
import io.ktor.client.plugins.defaultRequest
-import io.ktor.client.plugins.logging.DEFAULT
import io.ktor.client.plugins.logging.LogLevel
-import io.ktor.client.plugins.logging.Logger
import io.ktor.client.plugins.logging.Logging
import io.ktor.client.request.header
import io.ktor.client.request.post
@@ -143,11 +142,7 @@ val appModule = module {
get(named("refreshClient"))
)
install(Logging) {
- logger = object : Logger {
- override fun log(message: String) {
- Log.d("OsmClient", message) // Avoid System.err
- }
- }
+ logger = LogcatHttpLogger("OsmClient")
level = HTTP_LOG_LEVEL
sanitizeHeader { it == HttpHeaders.Authorization }
}
@@ -176,11 +171,7 @@ val appModule = module {
get(named("refreshClient"))
)
install(Logging) {
- logger = object : Logger {
- override fun log(message: String) {
- Log.d("KtorClient", message) // Avoid System.err
- }
- }
+ logger = LogcatHttpLogger("KtorClient")
level = HTTP_LOG_LEVEL
sanitizeHeader { it == HttpHeaders.Authorization }
}
@@ -199,7 +190,7 @@ val appModule = module {
json(Json { ignoreUnknownKeys = true })
}
install(Logging) {
- logger = Logger.DEFAULT
+ logger = LogcatHttpLogger("RefreshClient")
level = HTTP_LOG_LEVEL
sanitizeHeader { it == HttpHeaders.Authorization }
}
@@ -212,11 +203,7 @@ val appModule = module {
json(Json { ignoreUnknownKeys = true })
}
install(Logging) {
- logger = object : Logger {
- override fun log(message: String) {
- Log.d("KtorClient", message) // Avoid System.err
- }
- }
+ logger = LogcatHttpLogger("KtorClient")
level = HTTP_LOG_LEVEL
sanitizeHeader { it == HttpHeaders.Authorization }
}
@@ -292,6 +279,7 @@ suspend fun refreshJwtToken(
}
}
-// HTTP logs end up in the log database and from there in emailed crash reports - full bodies
-// (map data, long forms) make those multi-MB, so only log them in debug builds
+// full bodies (map data, long forms) are read into one string each just for the log - several MB
+// per map data download - so only log them in debug builds. Either way they only go to logcat,
+// not to the log database (see LogcatHttpLogger)
private val HTTP_LOG_LEVEL = if (BuildConfig.DEBUG) LogLevel.ALL else LogLevel.INFO
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
index 1aa152d816f..20252a7a4b0 100644
--- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
@@ -301,6 +301,14 @@ class MainActivity :
override fun onCreate(savedInstanceState: Bundle?) {
enableEdgeToEdge()
+ // started without a workspace's long form - e.g. from the sync notification after this
+ // activity was closed, or by a geo: link - there are no quest types to show: pick one first
+ if (!intent.hasExtra("LONG_FORM")) {
+ super.onCreate(null)
+ startActivity(Intent(this, WorkSpaceActivity::class.java))
+ finish()
+ return
+ }
// before super.onCreate: that restores the fragments, and a quest form restored after
// process death looks up its quest type in the registry, which is still empty until then
doLongForm()
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLogger.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLogger.kt
new file mode 100644
index 00000000000..4f899c196f9
--- /dev/null
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLogger.kt
@@ -0,0 +1,16 @@
+package de.westnordost.streetcomplete.util.logs
+
+import android.util.Log
+import io.ktor.client.plugins.logging.Logger as KtorLogger
+
+/** Logger for the Ktor Logging plugin of the app's HTTP clients: writes to logcat only, not
+ * through the app's [de.westnordost.streetcomplete.util.logs.Log], which also stores every
+ * message in the log database (DatabaseLogger). A logged response can be a whole map data
+ * download of several MB - stored in the database, those made it grow by MBs per download (kept
+ * for 14 days, see Cleaner) and every one of them was written into the same database the map
+ * and quests are read from. */
+class LogcatHttpLogger(private val tag: String) : KtorLogger {
+ override fun log(message: String) {
+ Log.d(tag, message)
+ }
+}
diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLoggerTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLoggerTest.kt
new file mode 100644
index 00000000000..099b7cc5335
--- /dev/null
+++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/util/logs/LogcatHttpLoggerTest.kt
@@ -0,0 +1,33 @@
+package de.westnordost.streetcomplete.util.logs
+
+import kotlin.test.AfterTest
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+class LogcatHttpLoggerTest {
+ private val received = mutableListOf()
+
+ /** stands in for DatabaseLogger, which [Log] forwards every message to */
+ private val appLogger = object : Logger {
+ override fun v(tag: String, message: String) { received += message }
+ override fun d(tag: String, message: String) { received += message }
+ override fun i(tag: String, message: String) { received += message }
+ override fun w(tag: String, message: String, exception: Throwable?) { received += message }
+ override fun e(tag: String, message: String, exception: Throwable?) { received += message }
+ }
+
+ @BeforeTest fun setUp() {
+ Log.instances.add(appLogger)
+ }
+
+ @AfterTest fun tearDown() {
+ Log.instances.remove(appLogger)
+ }
+
+ @Test fun `http logs do not go through the app's Log`() {
+ LogcatHttpLogger("OsmClient").log("RESPONSE: 200 OK\nBODY START\n...\nBODY END")
+
+ assertEquals(emptyList(), received)
+ }
+}
diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt
index adc6cc44d76..01f72483903 100644
--- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt
+++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt
@@ -34,7 +34,7 @@ import de.westnordost.streetcomplete.util.logs.Log
/** Creates the database and upgrades it */
object DatabaseInitializer {
- const val DB_VERSION = 28
+ const val DB_VERSION = 29
fun onCreate(db: Database) {
// OSM notes
@@ -352,9 +352,19 @@ object DatabaseInitializer {
db.tryExec("ALTER TABLE ${FeaturePhotosTable.NAME} ADD COLUMN ${FeaturePhotosTable.Columns.UPLOAD_ATTEMPTS} int NOT NULL DEFAULT 0")
db.exec(StuckPhotoUploadNoticesTable.CREATE)
}
+
+ if (oldVersion <= 28 && newVersion >= 29) {
+ // the HTTP clients' logs (whole requests and responses, e.g. several MB of map data
+ // per download) used to be stored here, kept for 14 days - they go to logcat only now
+ // (see LogcatHttpLogger), drop the ones already stored
+ db.delete(LogsTable.NAME, where = "${LogsTable.Columns.TAG} IN ($HTTP_LOG_TAGS)")
+ }
}
}
+/** Tags the HTTP clients logged under while their logs were still stored in the logs table */
+private const val HTTP_LOG_TAGS = "'OsmClient', 'KtorClient'"
+
/* E.g. when upgrading directly from a version below 10, it will create the ElementEditsTable
directly in the latest schema because we use the constant from EditElementsTable.CREATE to create
it.
From 6992cb4c9b5bf26990810db52dafb4889ebd56da Mon Sep 17 00:00:00 2001
From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com>
Date: Fri, 23 Oct 2026 11:35:29 +0530
Subject: [PATCH 2/3] Improve photo upload handling: process photos in a
coroutine to prevent UI freezing, adjust photo scaling for long form uploads,
and enhance KartaView error handling to skip subsequent uploads on connection
issues.
---
.../sidewalk_long_form/LongFormFormTest.kt | 39 ++++++++++++--
.../streetcomplete/ApplicationModule.kt | 10 ++++
.../quests/AbstractOsmQuestForm.kt | 40 ++++++++++-----
.../data/karta_view/KartaViewApiClientTest.kt | 18 +++++++
.../edits/upload/ElementEditsUploaderTest.kt | 51 +++++++++++++++++++
.../data/karta_view/KartaViewApiClient.kt | 12 ++++-
.../osm/edits/upload/ElementEditsUploader.kt | 16 +++++-
7 files changed, 167 insertions(+), 19 deletions(-)
diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormFormTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormFormTest.kt
index 5f8dd834744..ca7a483e172 100644
--- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormFormTest.kt
+++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormFormTest.kt
@@ -16,6 +16,7 @@ import org.hamcrest.Description
import org.hamcrest.Matcher
import android.app.Instrumentation
import android.graphics.Bitmap
+import android.graphics.BitmapFactory
import android.net.Uri
import android.provider.MediaStore
import androidx.core.content.IntentCompat
@@ -450,6 +451,18 @@ class LongFormFormTest {
assertEquals(capturedUris[1].lastPathSegment, File(photo.path).name)
}
+ // long form photos go to KartaView, often over mobile data - a camera's full size photo is cut down
+ @Test
+ fun capturedPhotoIsScaledDownForUpload() {
+ open(mapOf("ext:obstruction" to "yes", "ext:obstruction:type" to "bollard;other"))
+ capturePhoto(width = 3200, height = 2400)
+ submit()
+
+ val size = BitmapFactory.Options().apply { inJustDecodeBounds = true }
+ BitmapFactory.decodeFile(waitForPhotos(firstEditId).single().path, size)
+ assertEquals(1600 to 1200, size.outWidth to size.outHeight)
+ }
+
@Test
fun tappingANewPhotoShowsItFullScreen() {
open(mapOf("ext:obstruction" to "yes"))
@@ -574,14 +587,34 @@ class LongFormFormTest {
}
/** Taps the photo prompt (or retake) with the camera stubbed to "take" a small real JPEG -
- * onTookPhoto reads its EXIF and rescales it, so an empty file wouldn't do. */
- private fun capturePhoto(button: Int = R.id.choice_follow_up) {
+ * onTookPhoto reads its EXIF and rescales it, so an empty file wouldn't do. That happens in a
+ * coroutine Espresso can't see: waits until the photo is attached (and a retake has deleted
+ * the photo it replaces). */
+ private fun capturePhoto(button: Int = R.id.choice_follow_up, width: Int = 64, height: Int = 48) {
+ val earlierCaptures = capturedUris.toList()
+ takePhoto(button, width, height)
+ val deadline = SystemClock.uptimeMillis() + EDIT_TIMEOUT_MS
+ while (true) {
+ try {
+ onView(inRowOf(OBSTRUCTION_TYPE_Q, R.id.photo_title)).perform(scrollIntoView()).check(matches(withText("Photo attached")))
+ if (capturedUris.size > earlierCaptures.size && earlierCaptures.none { exists(it) }) return
+ } catch (e: Throwable) {
+ if (SystemClock.uptimeMillis() > deadline) throw e
+ }
+ if (SystemClock.uptimeMillis() > deadline) throw AssertionError("photo not attached")
+ Thread.sleep(100)
+ }
+ }
+
+ private fun takePhoto(button: Int, width: Int, height: Int) {
intending(hasAction(MediaStore.ACTION_IMAGE_CAPTURE)).respondWithFunction { intent ->
val uri = IntentCompat.getParcelableExtra(intent, MediaStore.EXTRA_OUTPUT, Uri::class.java)!!
capturedUris.add(uri)
val context = InstrumentationRegistry.getInstrumentation().targetContext
context.contentResolver.openOutputStream(uri)!!.use {
- Bitmap.createBitmap(64, 48, Bitmap.Config.ARGB_8888).compress(Bitmap.CompressFormat.JPEG, 90, it)
+ val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888)
+ bitmap.compress(Bitmap.CompressFormat.JPEG, 90, it)
+ bitmap.recycle()
}
Instrumentation.ActivityResult(Activity.RESULT_OK, null)
}
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
index 9cf9f7c39b7..9e2fb166859 100644
--- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt
@@ -24,6 +24,7 @@ import de.westnordost.streetcomplete.util.satellite_layers.ImageryRepository
import io.ktor.client.HttpClient
import io.ktor.client.HttpClientConfig
import io.ktor.client.plugins.HttpRequestRetry
+import io.ktor.client.plugins.HttpTimeout
import io.ktor.client.plugins.auth.Auth
import io.ktor.client.plugins.auth.providers.BearerTokens
import io.ktor.client.plugins.auth.providers.bearer
@@ -212,6 +213,15 @@ val appModule = module {
retryOnException(maxRetries = 3, retryOnTimeout = true)
exponentialDelay()
}
+ // without this, the CIO engine gives every request 15s in total: a photo upload over a
+ // weak mobile connection (~360 KB, ~20s at 150 kbps) timed out, was sent again 3 times
+ // by the retry above, and all of it again on the next sync. Instead, give it time as
+ // long as data is moving, and give up on a connection that stalls.
+ install(HttpTimeout) {
+ requestTimeoutMillis = 120_000
+ connectTimeoutMillis = 15_000
+ socketTimeoutMillis = 30_000
+ }
}
}
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt
index 88bb2aff877..97367e24e4d 100644
--- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt
@@ -27,7 +27,6 @@ import androidx.exifinterface.media.ExifInterface
import androidx.exifinterface.media.ExifInterface.TAG_GPS_IMG_DIRECTION
import androidx.exifinterface.media.ExifInterface.TAG_GPS_IMG_DIRECTION_REF
import de.westnordost.osmfeatures.FeatureDictionary
-import de.westnordost.streetcomplete.ApplicationConstants
import de.westnordost.streetcomplete.R
import de.westnordost.streetcomplete.data.location.SurveyChecker
import de.westnordost.streetcomplete.data.osm.edits.AddElementEditsController
@@ -522,14 +521,23 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta
file?.delete()
return
}
- try {
- val exif = ExifInterface(file)
- rescalePhotoFile(file)
- copyDirectionExifData(file, exif)
- onPhotoCaptured(file.path, capturedPhotoBearing())
- } catch (e: IOException) {
- Log.e(TAG, "Unable to process photo", e)
- file.delete()
+ val bearing = capturedPhotoBearing()
+ // decoding the full camera photo, scaling and re-encoding it takes long enough to
+ // visibly freeze the form when done on the main thread
+ viewLifecycleScope.launch {
+ val processed = withContext(Dispatchers.IO) {
+ try {
+ val exif = ExifInterface(file)
+ rescalePhotoFile(file)
+ copyDirectionExifData(file, exif)
+ true
+ } catch (e: IOException) {
+ Log.e(TAG, "Unable to process photo", e)
+ file.delete()
+ false
+ }
+ }
+ if (processed) onPhotoCaptured(file.path, bearing)
}
}
@@ -554,10 +562,18 @@ private fun createLongFormPhotoFile(context: Context): File {
return file
}
+/* Long form photos go to KartaView, often over a weak mobile connection while surveying. A street
+ photo at 1600px / quality 60 is ~360 KB, vs ~560 KB at the 1920px / 65 used for note photos
+ (ApplicationConstants.ATTACH_PHOTO_*) - still plenty of detail to judge e.g. a kerb or surface */
+private const val LONG_FORM_PHOTO_MAX_SIZE = 1600
+private const val LONG_FORM_PHOTO_QUALITY = 60
+
private fun rescalePhotoFile(file: File) {
- val bitmap = decodeScaledBitmapAndNormalize(file.path, ApplicationConstants.ATTACH_PHOTO_MAX_SIZE, ApplicationConstants.ATTACH_PHOTO_MAX_SIZE) ?: throw IOException()
- val out = FileOutputStream(file.path)
- bitmap.compress(Bitmap.CompressFormat.JPEG, ApplicationConstants.ATTACH_PHOTO_QUALITY, out)
+ val bitmap = decodeScaledBitmapAndNormalize(file.path, LONG_FORM_PHOTO_MAX_SIZE, LONG_FORM_PHOTO_MAX_SIZE) ?: throw IOException()
+ FileOutputStream(file.path).use { out ->
+ bitmap.compress(Bitmap.CompressFormat.JPEG, LONG_FORM_PHOTO_QUALITY, out)
+ }
+ bitmap.recycle()
}
private fun copyDirectionExifData(file: File, exif: ExifInterface) {
diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClientTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClientTest.kt
index ee52a412cd1..120f4548cb4 100644
--- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClientTest.kt
+++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClientTest.kt
@@ -17,6 +17,8 @@ import java.nio.channels.UnresolvedAddressException
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
/** Every way a photo upload can fail must surface as [KartaViewException] - that's what the edit
* and note uploaders treat as "photo failed, keep the edit for the next sync" (and count towards
@@ -54,6 +56,22 @@ class KartaViewApiClientTest {
}
}
+ // the edit uploader stops trying the other queued photos for the run on an unreachable
+ // KartaView, but not on an error status for one upload
+ @Test fun `unreachable KartaView is marked as unreachable`() = runTest {
+ val e = assertFailsWith {
+ client { throw IOException("timeout") }.uploadImages(image, position)
+ }
+ assertTrue(e.isUnreachable)
+ }
+
+ @Test fun `error status is not marked as unreachable`() = runTest {
+ val e = assertFailsWith {
+ client { respond("bad request", HttpStatusCode.BadRequest) }.uploadImages(image, position)
+ }
+ assertFalse(e.isUnreachable)
+ }
+
@Test fun `nothing to upload makes no request`() = runTest {
assertEquals(emptyList(), client { error("no request expected") }.uploadImages(emptyList(), position))
assertEquals(0, requests)
diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploaderTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploaderTest.kt
index c840a4b9978..c61ed116291 100644
--- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploaderTest.kt
+++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploaderTest.kt
@@ -3,6 +3,7 @@ package de.westnordost.streetcomplete.data.osm.edits.upload
import de.westnordost.streetcomplete.data.AuthorizationException
import de.westnordost.streetcomplete.data.ConflictException
import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient
+import de.westnordost.streetcomplete.data.karta_view.KartaViewException
import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesController
import de.westnordost.streetcomplete.data.osm.edits.ElementEditAction
import de.westnordost.streetcomplete.data.osm.edits.ElementEditsController
@@ -278,6 +279,56 @@ class ElementEditsUploaderTest {
//endregion
+ //region KartaView failing with photos queued
+
+ private fun answerWithPhoto(id: Long): ElementEdit {
+ val edit = edit(id = id, action = UpdateElementTagsAction(
+ node(id = id),
+ StringMapChanges(listOf(StringMapEntryAdd("surface", "concrete")))
+ ))
+ on(featurePhotosController.get(id)).thenReturn(listOf(FeaturePhoto("/photos/$id.jpg", 0f)))
+ return edit
+ }
+
+ @Test fun `once KartaView is unreachable, the other queued photos are not tried in the same run`() = runBlocking {
+ val first = answerWithPhoto(1)
+ val second = answerWithPhoto(2)
+ val withoutPhoto = edit(id = 3)
+ on(elementEditsController.getOldestUnsynced(any()))
+ .thenReturn(first).thenReturn(second).thenReturn(withoutPhoto).thenReturn(null)
+ on(imageUploader.upload(any(), any())).thenThrow(KartaViewException("timeout", isUnreachable = true))
+ on(singleUploader.upload(any(), any())).thenReturn(MapDataUpdates())
+
+ uploader.upload()
+
+ verify(imageUploader).upload(listOf("/photos/1.jpg" to 0f), first.position)
+ verify(imageUploader, never()).upload(eq(listOf("/photos/2.jpg" to 0f)), any())
+ // only an edit that was actually tried counts towards the stuck-photo notice
+ verify(featurePhotosController).incrementUploadAttempts(1)
+ verify(featurePhotosController, never()).incrementUploadAttempts(2)
+ // an answer without a photo doesn't need KartaView
+ verify(singleUploader).upload(eq(withoutPhoto), any())
+ verify(elementEditsController, never()).markSynced(eq(second), any())
+ }
+
+ @Test fun `after an error status for one photo, the next photo is still tried`() = runBlocking {
+ val first = answerWithPhoto(1)
+ val second = answerWithPhoto(2)
+ on(elementEditsController.getOldestUnsynced(any()))
+ .thenReturn(first).thenReturn(second).thenReturn(null)
+ on(imageUploader.upload(eq(listOf("/photos/1.jpg" to 0f)), any()))
+ .thenThrow(KartaViewException("400 Bad Request"))
+ on(imageUploader.upload(eq(listOf("/photos/2.jpg" to 0f)), any())).thenReturn(listOf(PHOTO_URL))
+ on(singleUploader.upload(any(), any())).thenReturn(MapDataUpdates())
+
+ uploader.upload()
+
+ verify(imageUploader).upload(listOf("/photos/2.jpg" to 0f), second.position)
+ verify(elementEditsController).markSynced(any(), any())
+ }
+
+ //endregion
+
private companion object {
const val PHOTO_URL = "https://storage.kartaview.org/files/photo/lth/1.jpg"
}
diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt
index bdfb82e5802..7a97b272958 100644
--- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt
+++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt
@@ -78,7 +78,10 @@ class KartaViewApiClient(
// edit for the next sync" and count it towards the stuck-photo notice. Anything else
// escaped them and aborted the whole upload run with a generic "Upload error" dialog.
catch (e: Exception) {
- throw KartaViewException("Could not reach KartaView. Image upload failed. Please try again later: $e")
+ throw KartaViewException(
+ "Could not reach KartaView. Image upload failed. Please try again later: $e",
+ isUnreachable = true
+ )
}
}
@@ -188,4 +191,9 @@ class KartaViewApiClient(
}
}
-class KartaViewException(message: String) : RuntimeException(message)
+/** @property isUnreachable KartaView didn't answer at all (no connection, timeout, ...) - unlike an
+ * error status for this one upload, the next upload right after would most likely fail the same */
+class KartaViewException(
+ message: String,
+ val isUnreachable: Boolean = false,
+) : RuntimeException(message)
diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt
index 4047e692da0..939e8cead48 100644
--- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt
+++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt
@@ -64,9 +64,17 @@ class ElementEditsUploader(
// failed edit, so it's a completely normal unsynced edit again afterwards, and the very
// next upload run (auto-triggered often - see QuestAutoSyncer) retries it fresh.
val failedEditIds = mutableSetOf()
+ // once KartaView couldn't be reached at all, don't try the photos of every other queued
+ // edit in this run too: each would wait for its timeouts and retries (with its photo
+ // sent again each time) while holding the sync lock, which the map download waits for
+ var isKartaViewUnreachable = false
try {
while (true) {
val edit = elementEditsController.getOldestUnsynced(failedEditIds) ?: break
+ if (isKartaViewUnreachable && hasPendingPhotos(edit)) {
+ failedEditIds += edit.id
+ continue
+ }
val getIdProvider: () -> ElementIdProvider = { elementEditsController.getIdProvider(edit.id) }
try {
/* the sync of local change -> API and its response should not be cancellable
@@ -83,6 +91,7 @@ class ElementEditsUploader(
Log.w(TAG, "Failed to upload photos for edit ${edit.id}, will retry on next sync: ${e.message}")
failedEditIds += edit.id
onPhotoUploadFailed(edit)
+ if (e.isUnreachable) isKartaViewUnreachable = true
}
}
} finally {
@@ -196,9 +205,8 @@ class ElementEditsUploader(
* Returns the edit whose action carries the URL tag(s). */
private suspend fun uploadPendingPhotos(edit: ElementEdit): ElementEdit {
val action = edit.action
- if (action !is CreateNodeAction && action !is UpdateElementTagsAction) return edit
+ if (!hasPendingPhotos(edit)) return edit
val photos = featurePhotosController.get(edit.id)
- if (photos.isEmpty()) return edit
val urls = imageUploader.upload(photos.map { it.path to it.bearing }, edit.position)
var uploadedEdit = edit
@@ -222,6 +230,10 @@ class ElementEditsUploader(
return uploadedEdit
}
+ private fun hasPendingPhotos(edit: ElementEdit): Boolean =
+ (edit.action is CreateNodeAction || edit.action is UpdateElementTagsAction)
+ && featurePhotosController.get(edit.id).isNotEmpty()
+
private suspend fun fetchElementComplete(elementType: ElementType, elementId: Long): MapData? =
when (elementType) {
ElementType.NODE -> mapDataApi.getNode(elementId)?.let { MutableMapData(listOf(it)) }
From b3b08205a68b4195eb7c8ac00dccb12e19a6430d Mon Sep 17 00:00:00 2001
From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com>
Date: Wed, 4 Nov 2026 15:49:20 +0530
Subject: [PATCH 3/3] Clear quest caches after setting up edit types to prevent
crashes from stale pins on activity recreation
---
.../streetcomplete/screens/main/MainActivity.kt | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
index 20252a7a4b0..d51f1bb9d42 100644
--- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
+++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt
@@ -1797,9 +1797,6 @@ class MainActivity :
questTypes.add(index to AddGenericLong(item, recencyPeriodInDays))
}
questTypeRegistry.addItem(questTypes)
- // the quest cache outlives this activity - quests cached under the previous long form
- // would show stale pins whose quest type is no longer registered (form crashes on open)
- visibleQuestsSource.clearAllCaches()
allEditTypes.registries.clear()
allEditTypes.registries.addAll(listOf(questTypeRegistry))
@@ -1811,6 +1808,12 @@ class MainActivity :
allEditTypes.unknownTypeFallback = { name ->
AddGenericLong(Elements(elementType = name), recencyPeriodInDays)
}
+
+ // the quest cache outlives this activity - quests cached under the previous long form
+ // would show stale pins whose quest type is no longer registered (form crashes on open).
+ // Only after the edit types are set up: on a fresh process this first access creates
+ // MapDataWithEditsSource, which loads the unsynced edits and needs to resolve their types
+ visibleQuestsSource.clearAllCaches()
}
private fun onClickImageryLayerButton() {