Http logs out of db

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", "<osm>...</osm>", null, 1))
+
+        DatabaseInitializer.onUpgrade(database, 28, 30)
+
+        assertEquals(emptyList(), dao.getAll())
+    }
+}

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)
         }

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<MainActivity>(intent).use { scenario ->
+            assertEquals(Lifecycle.State.DESTROYED, scenario.state)
+        }
+        intended(hasComponent(WorkSpaceActivity::class.java.name))
+    }
+
+    private val context get() = InstrumentationRegistry.getInstrumentation().targetContext
+}

app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt

@@ -18,20 +18,20 @@ 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
 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
 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 +143,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 +172,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 +191,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 +204,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 }
             }
@@ -225,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
+            }
         }
     }
 
@@ -292,6 +289,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

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<T> : 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) {

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()
@@ -1789,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))
@@ -1803,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() {

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)
+    }
+}