diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b57e439c803..736ba7aefc4 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -62,6 +62,29 @@ val kartaViewAccessToken: String = System.getenv("KARTAVIEW_ACCESS_TOKEN") } ?: "" +/** Bearer token for photo uploads to the dev Panoramax instance (DEV environment): from env var, else root secrets.properties + * (git-ignored), else empty (build succeeds but Panoramax photo uploads fail auth at runtime). + * Never commit the token. */ +val panoramaxDevAccessToken: String = System.getenv("PANORAMAX_DEV_ACCESS_TOKEN") + ?: rootProject.file("secrets.properties").takeIf { it.exists() }?.let { file -> + Properties().apply { FileInputStream(file).use { load(it) } }.getProperty("panoramaxDevAccessToken") + } + ?: "" + +/** Same, for the stage Panoramax instance (used in the STAGE environment) */ +val panoramaxStageAccessToken: String = System.getenv("PANORAMAX_STAGE_ACCESS_TOKEN") + ?: rootProject.file("secrets.properties").takeIf { it.exists() }?.let { file -> + Properties().apply { FileInputStream(file).use { load(it) } }.getProperty("panoramaxStageAccessToken") + } + ?: "" + +/** Same, for the prod Panoramax instance (used in the PROD environment) */ +val panoramaxProdAccessToken: String = System.getenv("PANORAMAX_PROD_ACCESS_TOKEN") + ?: rootProject.file("secrets.properties").takeIf { it.exists() }?.let { file -> + Properties().apply { FileInputStream(file).use { load(it) } }.getProperty("panoramaxProdAccessToken") + } + ?: "" + buildkonfig { packageName = "de.westnordost.streetcomplete" objectName = "BuildConfig" @@ -71,6 +94,9 @@ buildkonfig { buildConfigField(STRING, "VERSION_NAME", appVersionName) buildConfigField(BOOLEAN, "DEBUG", "true") buildConfigField(STRING, "KARTAVIEW_ACCESS_TOKEN", kartaViewAccessToken) + buildConfigField(STRING, "PANORAMAX_DEV_ACCESS_TOKEN", panoramaxDevAccessToken) + buildConfigField(STRING, "PANORAMAX_STAGE_ACCESS_TOKEN", panoramaxStageAccessToken) + buildConfigField(STRING, "PANORAMAX_PROD_ACCESS_TOKEN", panoramaxProdAccessToken) } targetConfigs { diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDaoTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDaoTest.kt index 663f4eac83c..57d75d1ff92 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDaoTest.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDaoTest.kt @@ -41,7 +41,9 @@ class NoteEditsDaoTest : ApplicationDbTestCase() { listOf( Trackpoint(LatLon(3.0, 4.0), 1234L, 1f, 2f), Trackpoint(LatLon(1.0, 5.0), 12345L, 2f, 3f), - ) + ), + imageBearings = listOf(90f, null, 359f), + imageCaptureTimes = listOf(100L, 200L, null), ) dao.add(edit) assertEquals(edit, dao.get(1L)) 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 ca7a483e172..712bf33ff10 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 @@ -26,6 +26,7 @@ import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhoto import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosController import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.KARTAVIEW_URL_TAG +import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.STREET_IMAGES_TAG import org.hamcrest.Matchers.allOf import java.io.File import android.os.ParcelFileDescriptor @@ -381,11 +382,14 @@ class LongFormFormTest { val photos = waitForPhotos(firstEditId) assertEquals(1, photos.size) assertTrue(File(photos.single().path).exists()) + // stored with the question that asked for it - sent along to Panoramax on upload + assertEquals(OBSTRUCTION_TYPE_Q, photos.single().question) + assertEquals(105, photos.single().questId) } @Test fun existingPhotoCanBeMarkedForRemovalUndoneAndRemoved() { - open(mapOf("ext:obstruction" to "yes", "ext:obstruction:type" to "other", KARTAVIEW_URL_TAG to PHOTO_URL)) + open(mapOf("ext:obstruction" to "yes", "ext:obstruction:type" to "other", STREET_IMAGES_TAG to PHOTO_URL)) onView(inRowOf(OBSTRUCTION_TYPE_Q, R.id.photo_title)).perform(scrollIntoView()).check(matches(withText("Photo from last visit"))) onView(inRowOf(OBSTRUCTION_TYPE_Q, R.id.photo_delete)).perform(scrollIntoView(), click()) @@ -395,7 +399,7 @@ class LongFormFormTest { onView(inRowOf(OBSTRUCTION_TYPE_Q, R.id.photo_delete)).perform(scrollIntoView(), click()) submit() - assertFalse(KARTAVIEW_URL_TAG in edits.resultingTags(originalTags)) + assertFalse(STREET_IMAGES_TAG in edits.resultingTags(originalTags)) } @Test @@ -451,7 +455,7 @@ 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 + // long form photos go to Panoramax, 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")) @@ -477,7 +481,7 @@ class LongFormFormTest { @Test fun tappingThePhotoFromLastVisitShowsItFullScreen() { - open(mapOf("ext:obstruction" to "yes", "ext:obstruction:type" to "other", KARTAVIEW_URL_TAG to PHOTO_URL)) + open(mapOf("ext:obstruction" to "yes", "ext:obstruction:type" to "other", STREET_IMAGES_TAG to PHOTO_URL)) intending(hasAction(Intent.ACTION_VIEW)).respondWith(Instrumentation.ActivityResult(Activity.RESULT_OK, null)) onView(inRowOf(OBSTRUCTION_TYPE_Q, R.id.photo_thumb)).perform(scrollIntoView(), click()) @@ -686,7 +690,7 @@ class LongFormFormTest { private companion object { val ANIMATION_SCALES = listOf("animator_duration_scale", "transition_animation_scale", "window_animation_scale") const val QUEST_TYPE_NAME = "LongFormUiTestSidewalks" - const val PHOTO_URL = "https://kartaview.org/details/1/2/track-info" + const val PHOTO_URL = "https://panoramax.example/api/pictures/1/hd.jpg" const val EDIT_TIMEOUT_MS = 5_000L const val NO_EDIT_GRACE_MS = 1_000L } diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/AccessibilityScanTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/AccessibilityScanTest.kt index 404fb71470f..9509b70ca54 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/AccessibilityScanTest.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/AccessibilityScanTest.kt @@ -34,7 +34,7 @@ import de.westnordost.streetcomplete.screens.user.UserActivity import de.westnordost.streetcomplete.screens.workspaces.FakeWorkspaceRepository import de.westnordost.streetcomplete.screens.workspaces.WorkSpaceActivity import de.westnordost.streetcomplete.testutils.A11yScanner -import de.westnordost.streetcomplete.testutils.MockKartaView +import de.westnordost.streetcomplete.testutils.MockPanoramax import de.westnordost.streetcomplete.testutils.MockOsmServer import de.westnordost.streetcomplete.testutils.MockOsmServer.Failure import io.ktor.http.HttpStatusCode @@ -247,7 +247,7 @@ class AccessibilityScanTest : MapSyncTestBase() { @Test fun longFormPhotoFromLastVisit() { - MockOsmServer.put(SIDEWALK.copy(tags = SIDEWALK.tags + mapOf("obstacle" to "other", "ext:kartaview_url" to "https://storage.kartaview.org/files/photo/lth/x.jpg"))) + MockOsmServer.put(SIDEWALK.copy(tags = SIDEWALK.tags + mapOf("obstacle" to "other", "ext:street_images" to MockPanoramax.PHOTO_URL))) openWorkspaceAndSidewalkQuest() expandSheet() onView(inRowOf(OBSTRUCTION_Q, R.id.photo_title)).perform(scrollIntoView()) @@ -304,7 +304,7 @@ class AccessibilityScanTest : MapSyncTestBase() { fun stuckPhotoNotice() { Intents.init() try { - MockKartaView.failure = MockKartaView.Failure.Status(HttpStatusCode.ServiceUnavailable) + MockPanoramax.failure = MockPanoramax.Failure.Status(HttpStatusCode.ServiceUnavailable) openWorkspaceAndSidewalkQuest() tapTile(OBSTRUCTION_Q, "Other obstruction") capturePhoto() @@ -312,7 +312,7 @@ class AccessibilityScanTest : MapSyncTestBase() { val uploads: UploadProgressSource = koin.get() for (run in 1..3) { if (run > 1) tapUploadButton() - waitUntil("photo upload attempt $run") { MockKartaView.requests.size >= run && !uploads.isUploadInProgress } + waitUntil("photo upload attempt $run") { MockPanoramax.requests.size >= run && !uploads.isUploadInProgress } } waitForText("Photo upload failed") scan("Photo upload failed notice") diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncAnswersTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncAnswersTest.kt index da7313fe5e9..cc3ea53409e 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncAnswersTest.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncAnswersTest.kt @@ -11,6 +11,7 @@ import de.westnordost.streetcomplete.data.quest.VisibleQuestsSource import de.westnordost.streetcomplete.quests.sidewalk_long_form.inRowOf import de.westnordost.streetcomplete.quests.sidewalk_long_form.scrollIntoView import de.westnordost.streetcomplete.testutils.MockOsmServer +import de.westnordost.streetcomplete.testutils.MockPanoramax import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -40,7 +41,7 @@ class MapSyncAnswersTest : MapSyncTestBase() { @Test fun removingThePhotoFromTheLastVisit_removesItsTagOnOsm() { - val tags = SIDEWALK.tags + mapOf("obstacle" to "other", "ext:kartaview_url" to OLD_PHOTO_URL) + val tags = SIDEWALK.tags + mapOf("obstacle" to "other", "ext:street_images" to OLD_PHOTO_URL) MockOsmServer.put(SIDEWALK.copy(tags = tags)) openWorkspaceAndSidewalkQuest() @@ -51,7 +52,7 @@ class MapSyncAnswersTest : MapSyncTestBase() { submit() val way = awaitUploads(1).single().modifiedWays.single() - assertEquals(tags - "ext:kartaview_url", way.tags) + assertEquals(tags - "ext:street_images", way.tags) } // last edited long ago (the mock's default: 2025-01-01) - due for a recheck @@ -79,7 +80,7 @@ class MapSyncAnswersTest : MapSyncTestBase() { } private companion object { - const val OLD_PHOTO_URL = "https://storage.kartaview.org/files/photo/lth/from-last-visit.jpg" + val OLD_PHOTO_URL get() = "${MockPanoramax.PHOTO_URL}?from-last-visit" val FULLY_ANSWERED = SIDEWALK.tags + mapOf( "surface" to "asphalt", "width" to "60", "obstacle" to "bollard", "note" to "all good", ) diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncMultiSelectTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncMultiSelectTest.kt index 759341923f7..33101003506 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncMultiSelectTest.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncMultiSelectTest.kt @@ -13,7 +13,7 @@ import de.westnordost.streetcomplete.data.osm.edits.ElementEditsController import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosController import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.upload.UploadProgressSource -import de.westnordost.streetcomplete.testutils.MockKartaView +import de.westnordost.streetcomplete.testutils.MockPanoramax import de.westnordost.streetcomplete.testutils.MockOsmServer import de.westnordost.streetcomplete.testutils.MockOsmServer.MockNode import io.ktor.http.HttpStatusCode @@ -69,23 +69,23 @@ class MapSyncMultiSelectTest : MapSyncTestBase() { onView(withId(R.id.yesButton)).perform(click()) waitUntil("long form opened") { isDisplayed(SURFACE_Q) } - // KartaView down at first, so the photos stay around to be looked at - MockKartaView.failure = MockKartaView.Failure.Status(HttpStatusCode.ServiceUnavailable) + // Panoramax down at first, so the photos stay around to be looked at + MockPanoramax.failure = MockPanoramax.Failure.Status(HttpStatusCode.ServiceUnavailable) tapTile(SURFACE_Q, "Concrete") tapTile(OBSTRUCTION_Q, "Other obstruction") capturePhoto() submit() val uploads: UploadProgressSource = koin.get() - waitUntil("photo upload attempted") { MockKartaView.requests.isNotEmpty() && !uploads.isUploadInProgress } + waitUntil("photo upload attempted") { MockPanoramax.requests.isNotEmpty() && !uploads.isUploadInProgress } val edits = koin.get().getAllUnsynced() assertEquals("one edit per selected sidewalk", 2, edits.size) val photos = edits.map { koin.get().get(it.id).single() } assertEquals("each edit owns its own photo file", 2, photos.map { it.path }.toSet().size) assertTrue(photos.all { File(it.path).exists() }) - // KartaView back - both go up, each carrying its photo's URL - MockKartaView.failure = null + // Panoramax back - both go up, each carrying its photo's URL + MockPanoramax.failure = null tapUploadButton() waitUntil("both sidewalks uploaded") { MockOsmServer.snapshot().uploads.flatMap { it.modifiedWays }.map { it.id }.toSet() == setOf(SIDEWALK.id, SECOND_SIDEWALK_ID) @@ -93,7 +93,7 @@ class MapSyncMultiSelectTest : MapSyncTestBase() { for (way in MockOsmServer.snapshot().uploads.flatMap { it.modifiedWays }) { assertEquals( "way ${way.id}", - SIDEWALK.tags + mapOf("surface" to "concrete", "obstacle" to "other", "ext:kartaview_url" to MockKartaView.PHOTO_URL), + SIDEWALK.tags + mapOf("surface" to "concrete", "obstacle" to "other", "ext:street_images" to MockPanoramax.PHOTO_URL), way.tags ) } diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncPhotoTest.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncPhotoTest.kt index 065efcb2191..49204a151ea 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncPhotoTest.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncPhotoTest.kt @@ -23,7 +23,7 @@ import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotos import de.westnordost.streetcomplete.data.upload.UploadProgressSource import de.westnordost.streetcomplete.quests.sidewalk_long_form.inRowOf import de.westnordost.streetcomplete.quests.sidewalk_long_form.scrollIntoView -import de.westnordost.streetcomplete.testutils.MockKartaView +import de.westnordost.streetcomplete.testutils.MockPanoramax import de.westnordost.streetcomplete.testutils.MockOsmServer import io.ktor.http.HttpStatusCode import org.junit.After @@ -35,9 +35,9 @@ import org.junit.Test import org.junit.runner.RunWith /** - * A long-form answer with a photo, when the photo can't be uploaded to KartaView: after three + * A long-form answer with a photo, when the photo can't be uploaded to Panoramax: after three * failed upload runs the user is told and can remove the photo (the answer then uploads without - * it) or keep trying. KartaView is [MockKartaView], hooked into the app's own client - no photo + * it) or keep trying. Panoramax is [MockPanoramax], hooked into the app's own client - no photo * ever reaches the real service. See [MapSyncTestBase] for the rest of the setup. */ @RunWith(AndroidJUnit4::class) @@ -53,23 +53,34 @@ class MapSyncPhotoTest : MapSyncTestBase() { Intents.release() } - // the normal case: KartaView takes the photo, and the answer goes up carrying its public URL + // the normal case: Panoramax takes the photo, and the answer goes up carrying its public URL @Test fun photoUploads_andTheAnswerCarriesItsUrl() { - MockKartaView.failure = null + MockPanoramax.failure = null openWorkspaceAndSidewalkQuest() answerWithPhoto() val way = awaitUploads(1).single().modifiedWays.single() assertEquals( - SIDEWALK.tags + mapOf("surface" to "concrete", "obstacle" to "other", "ext:kartaview_url" to MockKartaView.PHOTO_URL), + SIDEWALK.tags + mapOf("surface" to "concrete", "obstacle" to "other", "ext:street_images" to MockPanoramax.PHOTO_URL), way.tags ) // uploaded the way the real API expects it, before the answer assertEquals( - listOf("POST /1.0/sequence/", "POST /1.0/photo/", "POST /1.0/sequence/finished-uploading/", "GET /2.0/photo/mock-photo-1"), - MockKartaView.requests.toList() + listOf( + "POST /api/upload_sets", + "POST /api/upload_sets/mock-upload-set-1/files", + "POST /api/upload_sets/mock-upload-set-1/complete", + ), + MockPanoramax.requests.toList() ) + // with what the photo is of: the sidewalk, and the question that asked for it + val photoBody = MockPanoramax.fileBodies.single() + assertTrue(photoBody, photoBody.contains("""{"key":"element_id","value":"way/${SIDEWALK.id}"}""")) + assertTrue(photoBody, photoBody.contains("""{"key":"question","value":"$OBSTRUCTION_Q"}""")) + assertTrue(photoBody, photoBody.contains("""{"key":"quest_id","value":"103"}""")) + // nothing identifying the user, the pictures are public (MapSyncTestBase logs in with an email) + assertFalse(photoBody, photoBody.contains("user_email")) // the photo is done with: its record (and file) are gone, nothing is left queued waitUntil("answer synced") { unsyncedEditsCount() == 0 } assertEquals(0, koin.get().query(FeaturePhotosTable.NAME) { 1 }.size) @@ -79,7 +90,7 @@ class MapSyncPhotoTest : MapSyncTestBase() { @Test fun photoKeepsFailingToUpload_userIsTold_andCanRemoveIt_thenTheAnswerUploads() { - MockKartaView.failure = MockKartaView.Failure.Status(HttpStatusCode.ServiceUnavailable) + MockPanoramax.failure = MockPanoramax.Failure.Status(HttpStatusCode.ServiceUnavailable) openWorkspaceAndSidewalkQuest() answerWithPhoto() @@ -90,12 +101,12 @@ class MapSyncPhotoTest : MapSyncTestBase() { val way = awaitUploads(1).single().modifiedWays.single() assertEquals(SIDEWALK.tags + mapOf("surface" to "concrete", "obstacle" to "other"), way.tags) - assertFalse(way.tags.keys.any { it.startsWith("ext:kartaview") }) + assertFalse(way.tags.keys.any { it == "ext:street_images" || it.startsWith("ext:kartaview") }) } @Test fun keepTrying_closesTheNotice_andTheAnswerStaysQueued() { - MockKartaView.failure = MockKartaView.Failure.Status(HttpStatusCode.ServiceUnavailable) + MockPanoramax.failure = MockPanoramax.Failure.Status(HttpStatusCode.ServiceUnavailable) openWorkspaceAndSidewalkQuest() answerWithPhoto() @@ -109,12 +120,12 @@ class MapSyncPhotoTest : MapSyncTestBase() { assertEquals(1, unsyncedEditsCount()) } - // KartaView not answering at all (no connection, DNS, timeout) must be handled like it + // Panoramax not answering at all (no connection, DNS, timeout) must be handled like it // answering with an error: the photo's answer is held, the others aren't, and after three // runs the user is told @Test - fun kartaViewUnreachable_isHandledLikeAKartaViewError() { - MockKartaView.failure = MockKartaView.Failure.Unreachable + fun panoramaxUnreachable_isHandledLikeAPanoramaxError() { + MockPanoramax.failure = MockPanoramax.Failure.Unreachable openWorkspaceAndSidewalkQuest() answerWithPhoto() @@ -139,7 +150,7 @@ class MapSyncPhotoTest : MapSyncTestBase() { for (run in 1..3) { if (run > 1) tapUploadButton() waitUntil("photo upload attempt $run") { - MockKartaView.requests.size >= run && !uploads.isUploadInProgress + MockPanoramax.requests.size >= run && !uploads.isUploadInProgress } } assertTrue(MockOsmServer.snapshot().uploads.isEmpty()) diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt index f768a86f262..9fb4607a503 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt @@ -73,7 +73,7 @@ import de.westnordost.streetcomplete.screens.main.map.MainMapFragment import de.westnordost.streetcomplete.screens.workspaces.FakeWorkspaceRepository import de.westnordost.streetcomplete.screens.workspaces.WorkSpaceActivity import de.westnordost.streetcomplete.testutils.MockGps -import de.westnordost.streetcomplete.testutils.MockKartaView +import de.westnordost.streetcomplete.testutils.MockPanoramax import de.westnordost.streetcomplete.testutils.MockOsmServer import de.westnordost.streetcomplete.testutils.MockOsmServer.MockNode import de.westnordost.streetcomplete.testutils.MockOsmServer.MockWay @@ -109,7 +109,7 @@ import org.koin.dsl.module * is downloaded from [MockOsmServer], and the sidewalk's quest pin can be tapped on the map. * * Everything past the fake login is the app's own code. The network is [MockOsmServer] and - * [MockKartaView], both hooked into the app's own HttpClients (see [MockOsmServer.attachTo] for + * [MockPanoramax], both hooked into the app's own HttpClients (see [MockOsmServer.attachTo] for * why), and a mock workspace API behind the real [WorkspaceApiService] / [WorkspaceRepositoryImpl] * / [WorkspaceDao]. */ @@ -150,7 +150,7 @@ abstract class MapSyncTestBase { MockOsmServer.reset() MockOsmServer.put(*sidewalkNodes.toTypedArray(), sidewalk, *otherElements.toTypedArray()) - MockKartaView.reset() + MockPanoramax.reset() val realRepository = WorkspaceRepositoryImpl(workspaceApiService(), workspaceDao) fakeRepository.workspaces = { realRepository.getWorkspaces(it) } @@ -160,9 +160,9 @@ abstract class MapSyncTestBase { }) MockOsmServer.attachTo(koin.get(named("osmClient"))) MockOsmServer.isActive = true - // photo uploads must never reach the real KartaView either - MockKartaView.attachTo(koin.get(named("kartaViewClient"))) - MockKartaView.isActive = true + // photo uploads must never reach the real Panoramax either + MockPanoramax.attachTo(koin.get(named("panoramaxClient"))) + MockPanoramax.isActive = true // photo thumbnails of existing answers are real https URLs (loaded by Coil, which the // network guard doesn't cover) - never fetch them in tests @@ -190,7 +190,7 @@ abstract class MapSyncTestBase { finishAllActivities() clearAppState() MockOsmServer.isActive = false - MockKartaView.isActive = false + MockPanoramax.isActive = false preferences.isLowBandwidthModeEnabled = wasLowBandwidth } diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockKartaView.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockPanoramax.kt similarity index 52% rename from app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockKartaView.kt rename to app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockPanoramax.kt index 8de39cec85c..356771bcebe 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockKartaView.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/MockPanoramax.kt @@ -1,9 +1,11 @@ package de.westnordost.streetcomplete.testutils +import de.westnordost.streetcomplete.data.preferences.EnvironmentManager import io.ktor.client.HttpClient import io.ktor.client.engine.mock.MockEngine import io.ktor.client.engine.mock.MockRequestHandleScope import io.ktor.client.engine.mock.respond +import io.ktor.client.engine.mock.toByteArray import io.ktor.client.plugins.HttpSend import io.ktor.client.plugins.contentnegotiation.ContentNegotiation import io.ktor.client.plugins.plugin @@ -18,21 +20,26 @@ import io.ktor.http.HttpStatusCode import io.ktor.http.headersOf import io.ktor.serialization.kotlinx.json.json import kotlinx.serialization.json.Json +import org.koin.core.context.GlobalContext import java.io.IOException import java.util.Collections import java.util.WeakHashMap /** - * Keeps photo uploads away from the real KartaView: hooked into the app's own "kartaViewClient" + * Keeps photo uploads away from the real Panoramax: hooked into the app's own "panoramaxClient" * (same reason as [MockOsmServer.attachTo]). While [isActive] every request is answered here: - * - [failure] null: like the real API when all goes well - sequence created, photo uploaded, - * sequence closed, photo looked up with [PHOTO_URL] as its public URL + * - [failure] null: like the real API when all goes well - upload set created, photo added as + * picture [PICTURE_ID] (public URL [PHOTO_URL]), upload set completed, semantics patched * - [Failure.Unreachable]: like an unreachable server (IOException) * - [Failure.Status]: with that HTTP error status */ -object MockKartaView { +object MockPanoramax { - const val PHOTO_URL = "https://storage.kartaview.org/files/photo/lth/mock-photo-1.jpg" + const val PICTURE_ID = "mock-picture-1" + /** on the instance of the environment the app is on, see Environment.panoramaxBaseUrl */ + val PHOTO_URL: String + get() = "${GlobalContext.get().get().currentEnvironment.panoramaxBaseUrl}" + + "api/pictures/$PICTURE_ID/hd.jpg" sealed interface Failure { data object Unreachable : Failure @@ -41,49 +48,55 @@ object MockKartaView { @Volatile var isActive = false @Volatile var failure: Failure? = null + /** "METHOD path" of each request, in order */ val requests: MutableList = Collections.synchronizedList(mutableListOf()) + /** the (multipart) bodies of the photo file uploads, in order */ + val fileBodies: MutableList = Collections.synchronizedList(mutableListOf()) private val attachedTo = Collections.newSetFromMap(WeakHashMap()) // the app reads the body through this client's call, so it needs the same JSON support as - // the app's own kartaViewClient + // the app's own panoramaxClient private val server = HttpClient(MockEngine { request -> answer(request) }) { install(ContentNegotiation) { json(Json { ignoreUnknownKeys = true }) } } - fun attachTo(kartaViewClient: HttpClient) = synchronized(attachedTo) { - if (!attachedTo.add(kartaViewClient)) return - kartaViewClient.plugin(HttpSend).intercept { request -> + fun attachTo(panoramaxClient: HttpClient) = synchronized(attachedTo) { + if (!attachedTo.add(panoramaxClient)) return + panoramaxClient.plugin(HttpSend).intercept { request -> if (!isActive) return@intercept execute(request) requests.add("${request.method.value} ${java.net.URI(request.url.buildString()).path}") - if (failure == Failure.Unreachable) throw IOException("mock KartaView: unreachable") + if (failure == Failure.Unreachable) throw IOException("mock Panoramax: unreachable") server.request(HttpRequestBuilder().takeFrom(request)).call } } fun reset() { requests.clear() + fileBodies.clear() failure = null } - private fun MockRequestHandleScope.answer(request: HttpRequestData): HttpResponseData { - (failure as? Failure.Status)?.let { return respond("mock KartaView failure", it.code) } + private suspend fun MockRequestHandleScope.answer(request: HttpRequestData): HttpResponseData { + (failure as? Failure.Status)?.let { return respond("mock Panoramax failure", it.code) } val path = request.url.encodedPath return when { - request.method == HttpMethod.Post && path.endsWith("/1.0/sequence/") -> - json("""{"osv":{"sequence":{"id":"mock-sequence-1"}},"status":$STATUS}""") - request.method == HttpMethod.Post && path.endsWith("/1.0/photo/") -> - json("""{"osv":{"photo":{"id":"mock-photo-1","path":"mock","photoName":"mock.jpg"}},"status":$STATUS}""") - request.method == HttpMethod.Post && path.endsWith("/1.0/sequence/finished-uploading/") -> - json("""{"osv":{},"status":$STATUS}""") - request.method == HttpMethod.Get && path.endsWith("/2.0/photo/mock-photo-1") -> - json("""{"result":{"data":{"imageLthUrl":"$PHOTO_URL"}}}""") - else -> respond("unexpected mock KartaView request", HttpStatusCode.NotFound) + request.method == HttpMethod.Post && path.endsWith("/api/upload_sets") -> + json("""{"id":"$UPLOAD_SET_ID"}""") + request.method == HttpMethod.Post && path.endsWith("/api/upload_sets/$UPLOAD_SET_ID/files") -> { + fileBodies.add(request.body.toByteArray().decodeToString()) + json("""{"picture_id":"$PICTURE_ID","rejected":null}""", HttpStatusCode.Accepted) + } + request.method == HttpMethod.Post && path.endsWith("/api/upload_sets/$UPLOAD_SET_ID/complete") -> + json("""{"id":"$UPLOAD_SET_ID"}""") + request.method == HttpMethod.Patch && path.endsWith("/api/pictures/$PICTURE_ID") -> + json("""{"id":"$PICTURE_ID"}""") + else -> respond("unexpected mock Panoramax request", HttpStatusCode.NotFound) } } - private fun MockRequestHandleScope.json(body: String) = - respond(body, HttpStatusCode.OK, headersOf(HttpHeaders.ContentType, "application/json")) + private fun MockRequestHandleScope.json(body: String, status: HttpStatusCode = HttpStatusCode.OK) = + respond(body, status, headersOf(HttpHeaders.ContentType, "application/json")) - private const val STATUS = """{"apiCode":600,"apiMessage":"ok","httpCode":200,"httpMessage":"Success"}""" + private const val UPLOAD_SET_ID = "mock-upload-set-1" } diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/RealNetworkGuard.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/RealNetworkGuard.kt index 46937597e90..1ffd6a95539 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/RealNetworkGuard.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/testutils/RealNetworkGuard.kt @@ -11,7 +11,7 @@ import java.util.Collections /** * Keeps every instrumented test off the real network on the app's own Ktor clients - the main - * workspace/TDEI client, "osmClient", "kartaViewClient" and "refreshClient" (the one the 401 + * workspace/TDEI client, "osmClient", "kartaViewClient", "panoramaxClient" and "refreshClient" (the one the 401 * token refresh uses). A request that no mock answered is blocked: it fails like an unreachable * server (IOException, so the app treats it as offline), is logged under [TAG] and recorded in * [blocked]. Installed once per test process by [SandboxedTestRunner]. @@ -30,8 +30,9 @@ object RealNetworkGuard { fun install(koin: Koin) { guard(koin.get(), "default") { false } guard(koin.get(named("osmClient")), "osmClient") { MockOsmServer.isActive } - guard(koin.get(named("kartaViewClient")), "kartaViewClient") { MockKartaView.isActive } + guard(koin.get(named("kartaViewClient")), "kartaViewClient") { false } guard(koin.get(named("refreshClient")), "refreshClient") { false } + guard(koin.get(named("panoramaxClient")), "panoramaxClient") { MockPanoramax.isActive } } private fun guard(client: HttpClient, name: String, answeredByMock: () -> Boolean) { diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt index 9e2fb166859..021ea846b47 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt @@ -5,6 +5,7 @@ import android.content.Intent import android.content.res.AssetManager import android.content.res.Resources import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxApiClient import de.westnordost.streetcomplete.data.preferences.EnvironmentManager import de.westnordost.streetcomplete.data.preferences.Preferences import de.westnordost.streetcomplete.data.user.UserLoginController @@ -227,6 +228,33 @@ val appModule = module { single { KartaViewApiClient(get(), get(named("kartaViewClient")), get()) } + // photo uploads (replaced KartaView) - same setup as kartaViewClient above + single(named("panoramaxClient")) { + HttpClient { + install(ContentNegotiation) { + json(Json { ignoreUnknownKeys = true }) + } + install(Logging) { + logger = LogcatHttpLogger("KtorClient") + level = HTTP_LOG_LEVEL + sanitizeHeader { it == HttpHeaders.Authorization } + } + install(HttpRequestRetry) { + retryOnServerErrors(maxRetries = 3) + retryOnException(maxRetries = 3, retryOnTimeout = true) + exponentialDelay() + } + // see kartaViewClient + install(HttpTimeout) { + requestTimeoutMillis = 120_000 + connectTimeoutMillis = 15_000 + socketTimeoutMillis = 30_000 + } + } + } + + single { PanoramaxApiClient(get(), get(named("panoramaxClient")), get()) } + single { Res } single { SystemFileSystem } single { DefaultResourceProvider(androidContext(), get()) } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt index aca5fb29c3d..b9d65d19906 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.quests +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import android.annotation.SuppressLint import android.content.Context import android.os.Bundle @@ -17,7 +18,7 @@ import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.databinding.QuestLongFormListBinding import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.LongFormAdapter import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.LongFormQuest -import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.KARTAVIEW_URL_TAG +import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.getAttachedPhotoUrl import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.PhotoAttachment import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.PhotoState import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.PhotoTransition @@ -40,7 +41,7 @@ abstract class ALongForm : AbstractOsmQuestForm() { /** The (at most one) photo attached to this quest - either just captured locally this visit * (not yet uploaded - upload happens in the background sync, see AbstractOsmQuestForm) or - * read back from the element's own ext:kartaview_url tag on open (already synced from a + * read back from the element's own ext:street_images (or older ext:kartaview_url) tag on open (already synced from a * previous visit) - plus whether an already-synced one was removed. The attachment is * mirrored onto the adapter so the row showing the relevant choiceFollowUp can render it - * see LongFormAdapter.updateChoiceFollowUp. Transitions live in LongFormSubmission.kt. */ @@ -128,7 +129,7 @@ abstract class ALongForm : AbstractOsmQuestForm() { // LongFormQuest.seedFrom for individual questions - not shown in multi-select, since // that's about to be applied to several elements that may not share the same photo if (!isMultiSelectActive) { - element.tags[KARTAVIEW_URL_TAG]?.let { photoState = PhotoState(PhotoAttachment.Uploaded(it)) } + getAttachedPhotoUrl(element.tags)?.let { photoState = PhotoState(PhotoAttachment.Uploaded(it)) } } binding.recyclerView.apply { layoutManager = LinearLayoutManager(activity) @@ -163,8 +164,8 @@ abstract class ALongForm : AbstractOsmQuestForm() { } } - override fun onPhotoCaptured(path: String, bearing: Float) { - applyPhotoTransition(photoState.afterPhotoCaptured(path, bearing)) + override fun onPhotoCaptured(photo: CapturedPhoto) { + applyPhotoTransition(photoState.afterPhotoCaptured(photo.path, photo.bearing, photo.capturedAt)) } private fun onPhotoDeleted() { 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 97367e24e4d..7161e9f4c64 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt @@ -1,31 +1,18 @@ package de.westnordost.streetcomplete.quests import android.content.ActivityNotFoundException -import android.content.Context import android.content.res.Configuration import android.content.res.Resources -import android.graphics.Bitmap -import android.hardware.SensorManager import android.location.Location -import android.net.Uri import android.os.Bundle -import android.os.Environment import android.util.Log import android.view.Menu import android.view.View import android.view.ViewGroup -import android.view.WindowManager import android.widget.PopupMenu -import androidx.activity.result.ActivityResultLauncher -import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.app.AlertDialog -import androidx.core.content.FileProvider -import androidx.core.content.getSystemService import androidx.core.os.bundleOf import androidx.core.view.children -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.R import de.westnordost.streetcomplete.data.location.SurveyChecker @@ -55,11 +42,8 @@ import de.westnordost.streetcomplete.data.quest.QuestKey import de.westnordost.streetcomplete.data.visiblequests.HideQuestController import de.westnordost.streetcomplete.data.visiblequests.QuestsHiddenController import de.westnordost.streetcomplete.quests.sidewalk_long_form.AddGenericLong -import de.westnordost.streetcomplete.screens.main.map.Compass -import de.westnordost.streetcomplete.util.decodeScaledBitmapAndNormalize import de.westnordost.streetcomplete.util.getNameAndLocationSpanned import de.westnordost.streetcomplete.util.ktx.isSplittable -import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds import de.westnordost.streetcomplete.util.ktx.viewLifecycleScope import de.westnordost.streetcomplete.view.add import de.westnordost.streetcomplete.view.confirmIsSurvey @@ -70,10 +54,7 @@ import kotlinx.serialization.json.Json import org.koin.android.ext.android.inject import org.koin.core.qualifier.named import java.io.File -import java.io.FileOutputStream -import java.io.IOException import java.util.Locale -import kotlin.math.PI /** Abstract base class for any bottom sheet with which the user answers a specific quest(ion) */ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDetails { @@ -88,8 +69,13 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta protected val featureDictionary: FeatureDictionary get() = featureDictionaryLazy.value private val featurePhotosController: FeaturePhotosController by inject() - private lateinit var takePhotoLauncher: ActivityResultLauncher - private var pendingPhotoFile: File? = null + private val photoCapture = PhotoCapture( + fragment = this, + fileNamePrefix = "long_form_photo", + size = LONG_FORM_PHOTO_SIZE, + getLocation = { listener?.displayedMapLocation }, + onCaptured = { onPhotoCaptured(it) }, + ) // only used for testing / only used for ShowQuestFormsScreen! Found no better way to do this var addElementEditsController: AddElementEditsController = elementEditsController @@ -110,7 +96,6 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta // overridable by child classes open val otherAnswers = listOf() open val buttonPanelAnswers = listOf() - private var compassBearing: Double = 0.0 interface Listener { /** The GPS position at which the user is displayed at */ @@ -141,18 +126,10 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } private val listener: Listener? get() = parentFragment as? Listener ?: activity as? Listener - private lateinit var compass: Compass override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - compass = Compass( - context?.getSystemService()!!, - context?.getSystemService()!!.defaultDisplay, - this::onCompassRotationChanged - ) - lifecycle.addObserver(compass) - val args = requireArguments() val getElement: Element? = args.getString(ARG_ELEMENT)?.let { @@ -161,30 +138,12 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta if (getElement != null) { element = getElement } - pendingPhotoFile = savedInstanceState?.getString(ARG_PENDING_PHOTO_FILE)?.let { File(it) } - takePhotoLauncher = - registerForActivityResult(ActivityResultContracts.TakePicture(), ::onTookPhoto) - } - - private fun onCompassRotationChanged(rot: Float, tilt: Float) { - compassBearing = rot * 180 / PI - } - - /** The compass bearing (0-359, clockwise from north) to record for a photo captured right - * now - the live GPS bearing if the map is currently pointed somewhere meaningful, otherwise - * the device's own compass sensor reading. */ - private fun capturedPhotoBearing(): Float { - val displayedLocation = listener?.displayedMapLocation - return if (displayedLocation?.hasBearing() == true && displayedLocation.bearing != 0f) { - displayedLocation.bearing - } else { - compassBearing.toFloat() - } + photoCapture.restoreState(savedInstanceState) } override fun onSaveInstanceState(outState: Bundle) { super.onSaveInstanceState(outState) - pendingPhotoFile?.let { outState.putString(ARG_PENDING_PHOTO_FILE, it.path) } + photoCapture.saveState(outState) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -498,15 +457,9 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } override fun setCameraIntent() { - val activity = activity ?: return try { - val file = createLongFormPhotoFile(activity) - pendingPhotoFile = file - val uri = FileProvider.getUriForFile(activity, activity.getString(R.string.fileprovider_authority), file) - takePhotoLauncher.launch(uri) + photoCapture.launch() } catch (e: Exception) { - pendingPhotoFile?.delete() - pendingPhotoFile = null if (e !is ActivityNotFoundException) { Log.e(TAG, "Unable to create photo", e) } @@ -514,38 +467,10 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } } - private fun onTookPhoto(hasSavedPhoto: Boolean) { - val file = pendingPhotoFile - pendingPhotoFile = null - if (!hasSavedPhoto || file == null) { - file?.delete() - return - } - 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) - } - } - companion object { private const val TAG = "AbstractOsmQuestForm" private const val ARG_ELEMENT = "element" private const val ARG_DISPLAYED_LOCATION = "displayedLocation" - private const val ARG_PENDING_PHOTO_FILE = "pendingPhotoFile" fun createArguments(element: Element, displayedLocation: Location? = null) = bundleOf( ARG_ELEMENT to Json.encodeToString(element), @@ -554,31 +479,7 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } } -private fun createLongFormPhotoFile(context: Context): File { - val directory = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES) - val imageFileName = "long_form_photo_" + nowAsEpochMilliseconds() + ".jpg" - val file = File(directory, imageFileName) - if (!file.createNewFile()) throw IOException("Photo file with exactly the same name already exists") - return file -} - -/* Long form photos go to KartaView, often over a weak mobile connection while surveying. A street +/* Long form photos go to Panoramax, 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, 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) { - val newExif = ExifInterface(file.path) - newExif.setAttribute(TAG_GPS_IMG_DIRECTION, exif.getAttribute(TAG_GPS_IMG_DIRECTION)) - newExif.setAttribute(TAG_GPS_IMG_DIRECTION_REF, exif.getAttribute(TAG_GPS_IMG_DIRECTION_REF)) - newExif.saveAttributes() -} +private val LONG_FORM_PHOTO_SIZE = PhotoStorageSize(maxSize = 1600, quality = 60) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractQuestForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractQuestForm.kt index e3f57017bc8..fe4e3b34bdf 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractQuestForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractQuestForm.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.quests +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import android.os.Bundle import android.view.LayoutInflater import android.view.View @@ -151,10 +152,9 @@ abstract class AbstractQuestForm : open fun setCameraIntent() {} - /** Called when a photo was captured to local disk at [path], facing compass [bearing] - * (0-359, clockwise from north) - upload happens later, in the background sync, not at - * capture time (see AbstractOsmQuestForm). */ - open fun onPhotoCaptured(path: String, bearing: Float) {} + /** Called when a [photo] was captured to local disk - upload happens later, in the + * background sync, not at capture time (see AbstractOsmQuestForm). */ + open fun onPhotoCaptured(photo: CapturedPhoto) {} protected fun showProgressbar() { binding.progressBar.visibility = View.VISIBLE diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/CameraBearingSensor.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/CameraBearingSensor.kt new file mode 100644 index 00000000000..0072d4cdb87 --- /dev/null +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/CameraBearingSensor.kt @@ -0,0 +1,102 @@ +package de.westnordost.streetcomplete.quests + +import android.hardware.GeomagneticField +import android.hardware.Sensor +import android.hardware.SensorEvent +import android.hardware.SensorEventListener +import android.hardware.SensorManager +import android.location.Location +import android.os.SystemClock +import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withTimeoutOrNull +import kotlin.coroutines.resume +import kotlin.math.atan2 +import kotlin.math.hypot + +/** Measures the compass direction the back camera of the device is facing - not the direction of + * the device's top edge, like the map compass (which is meant for a device lying flat): when a + * photo is taken, the device is held upright and the camera looks out of its back (-Z axis). */ +class CameraBearingSensor(private val sensorManager: SensorManager) { + + /** Waits up to [timeoutMillis] for a fresh reading and returns the direction the back camera + * faces, in degrees clockwise from true north (from magnetic north if [location] is null), or + * null if there is no usable reading: no sensor, no reading in time, or the camera pointing + * (almost) straight up or down. */ + suspend fun awaitBearing(location: Location?, timeoutMillis: Long = 1500): Float? { + val bearing = withTimeoutOrNull(timeoutMillis) { awaitMagneticBearing() } ?: return null + val declination = location?.let { + GeomagneticField( + it.latitude.toFloat(), it.longitude.toFloat(), it.altitude.toFloat(), + nowAsEpochMilliseconds() + ).declination + } ?: 0f + return (bearing + declination).mod(360f) + } + + private suspend fun awaitMagneticBearing(): Float? { + val rotationVector = sensorManager.getDefaultSensor(Sensor.TYPE_ROTATION_VECTOR) + val accelerometer = sensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER) + val magnetometer = sensorManager.getDefaultSensor(Sensor.TYPE_MAGNETIC_FIELD) + if (rotationVector == null && (accelerometer == null || magnetometer == null)) return null + + return suspendCancellableCoroutine { cont -> + val start = SystemClock.elapsedRealtimeNanos() + val rotationMatrix = FloatArray(9) + var gravity: FloatArray? = null + var geomagnetic: FloatArray? = null + + val listener = object : SensorEventListener { + override fun onAccuracyChanged(sensor: Sensor, accuracy: Int) {} + + override fun onSensorChanged(event: SensorEvent) { + // the first events after (re-)registering can still carry an old state + // (event.timestamp's time base differs between devices, so measured on receipt) + if (SystemClock.elapsedRealtimeNanos() - start < SETTLE_NANOS) return + val hasMatrix = when (event.sensor.type) { + Sensor.TYPE_ROTATION_VECTOR -> { + SensorManager.getRotationMatrixFromVector(rotationMatrix, event.values) + true + } + else -> { + if (event.sensor.type == Sensor.TYPE_ACCELEROMETER) { + gravity = event.values.copyOf() + } else { + geomagnetic = event.values.copyOf() + } + val g = gravity ?: return + val m = geomagnetic ?: return + SensorManager.getRotationMatrix(rotationMatrix, null, g, m) + } + } + if (!hasMatrix || !cont.isActive) return + sensorManager.unregisterListener(this) + cont.resume(getBackCameraBearing(rotationMatrix)) + } + } + if (rotationVector != null) { + sensorManager.registerListener(listener, rotationVector, SensorManager.SENSOR_DELAY_GAME) + } else { + sensorManager.registerListener(listener, accelerometer, SensorManager.SENSOR_DELAY_GAME) + sensorManager.registerListener(listener, magnetometer, SensorManager.SENSOR_DELAY_GAME) + } + cont.invokeOnCancellation { sensorManager.unregisterListener(listener) } + } + } + + /** [r] maps device coordinates to world coordinates (x = east, y = magnetic north, z = up). + * The back camera looks along the device's -Z axis, i.e. the world vector -(r[2], r[5], r[8]). + * Returns its horizontal direction, or null if it points (almost) straight up or down. */ + private fun getBackCameraBearing(r: FloatArray): Float? { + val east = -r[2] + val north = -r[5] + if (hypot(east, north) < MIN_HORIZONTAL) return null + return Math.toDegrees(atan2(east, north).toDouble()).toFloat() + } + + companion object { + private const val SETTLE_NANOS = 200_000_000L + /** sin(~10°): pointing more steeply up or down than that, the direction is meaningless */ + private const val MIN_HORIZONTAL = 0.17f + } +} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt index 4ab5a5fc61e..6fd3080d9d3 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/LeaveNoteInsteadFragment.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.quests +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import android.os.Bundle import android.view.LayoutInflater import android.view.View @@ -83,7 +84,7 @@ class LeaveNoteInsteadFragment : AbstractCreateNoteFragment() { _binding = null } - override fun onComposedNote(text: String, imagePaths: List) { + override fun onComposedNote(text: String, photos: List) { val fullText = mutableListOf() leaveNoteContext?.let { fullText += it } fullText += "– https://osm.org/${elementType.name.lowercase()}/$elementId" @@ -91,7 +92,7 @@ class LeaveNoteInsteadFragment : AbstractCreateNoteFragment() { viewLifecycleScope.launch { withContext(Dispatchers.IO) { - noteEditsController.add(0, NoteEditAction.CREATE, position, fullText.joinToString(" "), imagePaths) + noteEditsController.add(0, NoteEditAction.CREATE, position, fullText.joinToString(" "), photos) } listener?.onCreatedNote(position) } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/PhotoCapture.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/PhotoCapture.kt new file mode 100644 index 00000000000..8daff301df8 --- /dev/null +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/PhotoCapture.kt @@ -0,0 +1,175 @@ +package de.westnordost.streetcomplete.quests + +import android.content.Context +import android.graphics.Bitmap +import android.hardware.GeomagneticField +import android.hardware.SensorManager +import android.location.Location +import android.os.Bundle +import android.os.Environment +import androidx.activity.result.contract.ActivityResultContracts +import androidx.core.content.FileProvider +import androidx.core.content.getSystemService +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 androidx.fragment.app.Fragment +import androidx.lifecycle.lifecycleScope +import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.photos.CapturedPhoto +import de.westnordost.streetcomplete.util.decodeScaledBitmapAndNormalize +import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds +import de.westnordost.streetcomplete.util.logs.Log +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import java.io.File +import java.io.FileOutputStream +import java.io.IOException + +/** How a captured photo is stored: scaled down to at most [maxSize] px on each side, saved as JPEG + * at [quality]. */ +data class PhotoStorageSize(val maxSize: Int, val quality: Int) + +/** + * The shared logic for every place that attaches photos (long form, notes, note comments, new + * features). It takes a photo with the camera app, scales it down off the main thread and records + * when it was taken and the direction the camera faced. That way each photo, even one of several, + * gets its own [CapturedPhoto.bearing] and [CapturedPhoto.capturedAt]. + * + * The direction is the one the camera app wrote into the photo, if any, because it knows the + * moment of the shot. Otherwise it is where the back camera points right after the shot (see + * [CameraBearingSensor]): the user is usually still aiming then. The GPS direction of travel is not + * used, because it is not where the camera points. + * + * Create it while [fragment] is initialized or in its onCreate, because it registers for an + * activity result. Forward onCreate's savedInstanceState to [restoreState] and onSaveInstanceState + * to [saveState], so a photo being taken survives the app process being killed meanwhile. + * + * @param getLocation where the user is, used to correct magnetic north to true north. It is called + * once the photo is taken. + * @param onCaptured called on the main thread with each photo once it is processed + */ +class PhotoCapture( + private val fragment: Fragment, + private val fileNamePrefix: String, + private val size: PhotoStorageSize, + private val getLocation: () -> Location?, + private val onCaptured: (CapturedPhoto) -> Unit, +) { + private var pendingFile: File? = null + private val launcher = fragment.registerForActivityResult(ActivityResultContracts.TakePicture(), ::onResult) + private val bearingSensor by lazy { + CameraBearingSensor(fragment.requireContext().getSystemService()!!) + } + private val stateKey get() = "photoCapture_$fileNamePrefix" + + fun restoreState(savedInstanceState: Bundle?) { + pendingFile = savedInstanceState?.getString(stateKey)?.let { File(it) } + } + + fun saveState(outState: Bundle) { + pendingFile?.let { outState.putString(stateKey, it.path) } + } + + /** Opens the camera app. + * @throws android.content.ActivityNotFoundException if there is no camera app + * @throws IOException if the photo file could not be created */ + fun launch() { + val context = fragment.requireContext() + val file = createPhotoFile(context, fileNamePrefix) + pendingFile = file + try { + val uri = FileProvider.getUriForFile(context, context.getString(R.string.fileprovider_authority), file) + launcher.launch(uri) + } catch (e: Exception) { + file.delete() + pendingFile = null + throw e + } + } + + private fun onResult(hasSavedPhoto: Boolean) { + val file = pendingFile + pendingFile = null + if (!hasSavedPhoto || file == null) { + file?.delete() + return + } + // control only comes back once the photo is taken, so this is when it was taken + val capturedAt = nowAsEpochMilliseconds() + val location = getLocation() + fragment.viewLifecycleOwner.lifecycleScope.launch { + val sensorBearing = async { bearingSensor.awaitBearing(location) } + // decoding, scaling and re-encoding the full camera photo takes long enough to freeze + // the UI visibly when done on the main thread + val exifBearing = try { + withContext(Dispatchers.IO) { + val exif = ExifInterface(file) + rescalePhotoFile(file, size) + copyDirectionExifData(file, exif) + Result.success(exif.getTrueNorthImageDirection(location)) + } + } catch (e: IOException) { + Log.e(TAG, "Unable to process photo", e) + null + } catch (e: CancellationException) { + // the screen was closed meanwhile: nobody takes ownership of the file + file.delete() + throw e + } + if (exifBearing == null) { + sensorBearing.cancel() + file.delete() + return@launch + } + val bearing = exifBearing.getOrNull() + ?.also { sensorBearing.cancel() } + ?: sensorBearing.await() + onCaptured(CapturedPhoto(file.path, bearing, capturedAt)) + } + } + + companion object { + private const val TAG = "PhotoCapture" + } +} + +private fun createPhotoFile(context: Context, prefix: String): File { + val directory = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES) + val file = File(directory, prefix + "_" + nowAsEpochMilliseconds() + ".jpg") + if (!file.createNewFile()) throw IOException("Photo file with exactly the same name already exists") + return file +} + +private fun rescalePhotoFile(file: File, size: PhotoStorageSize) { + val bitmap = decodeScaledBitmapAndNormalize(file.path, size.maxSize, size.maxSize) ?: throw IOException() + FileOutputStream(file.path).use { out -> + bitmap.compress(Bitmap.CompressFormat.JPEG, size.quality, out) + } + bitmap.recycle() +} + +/** The GPSImgDirection the camera app wrote into the photo (0-359, clockwise from true north), + * or null if it didn't. If the camera recorded it relative to magnetic north, it is converted + * using the declination at [location]. */ +private fun ExifInterface.getTrueNorthImageDirection(location: Location?): Float? { + val direction = getAttributeDouble(TAG_GPS_IMG_DIRECTION, Double.NaN) + if (direction.isNaN()) return null + val declination = if (getAttribute(TAG_GPS_IMG_DIRECTION_REF) == "M" && location != null) { + GeomagneticField( + location.latitude.toFloat(), location.longitude.toFloat(), location.altitude.toFloat(), + nowAsEpochMilliseconds() + ).declination + } else 0f + return (direction.toFloat() + declination).mod(360f) +} + +private fun copyDirectionExifData(file: File, exif: ExifInterface) { + val newExif = ExifInterface(file.path) + newExif.setAttribute(TAG_GPS_IMG_DIRECTION, exif.getAttribute(TAG_GPS_IMG_DIRECTION)) + newExif.setAttribute(TAG_GPS_IMG_DIRECTION_REF, exif.getAttribute(TAG_GPS_IMG_DIRECTION_REF)) + newExif.saveAttributes() +} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/AttachPhotoFragment.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/AttachPhotoFragment.kt index 1e2d661e6bc..e3cd3a98a68 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/AttachPhotoFragment.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/AttachPhotoFragment.kt @@ -1,50 +1,64 @@ package de.westnordost.streetcomplete.quests.note_discussion import android.content.ActivityNotFoundException -import android.content.Context import android.content.pm.PackageManager.FEATURE_CAMERA_ANY -import android.graphics.Bitmap +import android.location.Location import android.os.Bundle -import android.os.Environment import android.view.View -import androidx.activity.result.contract.ActivityResultContracts -import androidx.core.content.FileProvider import androidx.core.view.isGone -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 androidx.fragment.app.Fragment import de.westnordost.streetcomplete.ApplicationConstants import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import de.westnordost.streetcomplete.databinding.FragmentAttachPhotoBinding -import de.westnordost.streetcomplete.util.decodeScaledBitmapAndNormalize -import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds +import de.westnordost.streetcomplete.quests.PhotoCapture +import de.westnordost.streetcomplete.quests.PhotoStorageSize import de.westnordost.streetcomplete.util.ktx.toast import de.westnordost.streetcomplete.util.logs.Log import de.westnordost.streetcomplete.util.viewBinding import de.westnordost.streetcomplete.view.AdapterDataChangedWatcher +import kotlinx.serialization.json.Json import java.io.File -import java.io.FileOutputStream -import java.io.IOException class AttachPhotoFragment : Fragment(R.layout.fragment_attach_photo) { + /** Where the user is, so each photo's bearing is relative to true north (see PhotoCapture) */ + interface Listener { + val displayedMapLocation: Location? + } + private val listener: Listener? + get() = generateSequence(parentFragment) { it.parentFragment }.firstNotNullOfOrNull { it as? Listener } + ?: activity as? Listener + private val binding by viewBinding(FragmentAttachPhotoBinding::bind) - private val takePhoto = registerForActivityResult(ActivityResultContracts.TakePicture(), ::onTookPhoto) + private val photoCapture = PhotoCapture( + fragment = this, + fileNamePrefix = "photo", + size = PhotoStorageSize(ApplicationConstants.ATTACH_PHOTO_MAX_SIZE, ApplicationConstants.ATTACH_PHOTO_QUALITY), + getLocation = { listener?.displayedMapLocation }, + onCaptured = ::onCapturedPhoto, + ) private lateinit var noteImageAdapter: NoteImageAdapter - private var newPhotoFile: File? = null + /** the bearing and capture time of each photo, by path. The adapter holds the paths in their + * order, and the user can delete photos there. */ + private val capturedPhotos = mutableMapOf() val imagePaths: List get() = noteImageAdapter.list + /** The attached photos in the order they were taken, each with its own bearing and capture time */ + val photos: List get() = imagePaths.map { capturedPhotos[it] ?: CapturedPhoto(it) } var hasGpxAttached = false override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - val paths = savedInstanceState?.getStringArrayList(PHOTO_PATHS) ?: ArrayList() - noteImageAdapter = NoteImageAdapter(paths, requireContext()) - newPhotoFile = savedInstanceState?.getString(NEW_PHOTO_FILE)?.let { File(it) } + val photos = savedInstanceState?.getString(PHOTOS) + ?.let { Json.decodeFromString>(it) } + .orEmpty() + photos.associateByTo(capturedPhotos) { it.path } + noteImageAdapter = NoteImageAdapter(photos.map { it.path }, requireContext()) + photoCapture.restoreState(savedInstanceState) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -68,19 +82,14 @@ class AttachPhotoFragment : Fragment(R.layout.fragment_attach_photo) { override fun onSaveInstanceState(outState: Bundle) { super.onSaveInstanceState(outState) - outState.putStringArrayList(PHOTO_PATHS, ArrayList(imagePaths)) - outState.putString(NEW_PHOTO_FILE, newPhotoFile?.path) + outState.putString(PHOTOS, Json.encodeToString(photos)) + photoCapture.saveState(outState) } private fun takePhoto() { - val activity = activity ?: return try { - val file = createImageFile(activity) - newPhotoFile = file - val photoUri = FileProvider.getUriForFile(activity, activity.getString(R.string.fileprovider_authority), file) - takePhoto.launch(photoUri) + photoCapture.launch() } catch (e: Exception) { - newPhotoFile?.let { deleteImageFile(it) } when (e) { is ActivityNotFoundException -> context?.toast(R.string.no_camera_app) else -> { @@ -91,20 +100,10 @@ class AttachPhotoFragment : Fragment(R.layout.fragment_attach_photo) { } } - private fun onTookPhoto(hasSavedPhoto: Boolean) { - val file = newPhotoFile - if (!hasSavedPhoto) { - newPhotoFile?.let { deleteImageFile(it) } - return - } - if (file != null) { - val exif = ExifInterface(file) - rescaleImageFile(file) - copyDirectionExifData(file, exif) - - noteImageAdapter.list.add(file.path) - noteImageAdapter.notifyItemInserted(imagePaths.size - 1) - } + private fun onCapturedPhoto(photo: CapturedPhoto) { + capturedPhotos[photo.path] = photo + noteImageAdapter.list.add(photo.path) + noteImageAdapter.notifyItemInserted(imagePaths.size - 1) } fun deleteImages() { @@ -118,32 +117,6 @@ class AttachPhotoFragment : Fragment(R.layout.fragment_attach_photo) { companion object { private const val TAG = "AttachPhotoFragment" - private const val PHOTO_PATHS = "photo_paths" - private const val NEW_PHOTO_FILE = "photo_file" + private const val PHOTOS = "photos" } } - -private fun createImageFile(context: Context): File { - val directory = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES) - val imageFileName = "photo_" + nowAsEpochMilliseconds() + ".jpg" - val file = File(directory, imageFileName) - if (!file.createNewFile()) throw IOException("Photo file with exactly the same name already exists") - return file -} - -private fun rescaleImageFile(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) -} - -private fun copyDirectionExifData(file: File, exif: ExifInterface) { - val newExif = ExifInterface(file.path) - newExif.setAttribute(TAG_GPS_IMG_DIRECTION, exif.getAttribute(TAG_GPS_IMG_DIRECTION)) - newExif.setAttribute(TAG_GPS_IMG_DIRECTION_REF, exif.getAttribute(TAG_GPS_IMG_DIRECTION_REF)) - newExif.saveAttributes() -} - -private fun deleteImageFile(file: File) { - if (file.exists()) file.delete() -} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/NoteDiscussionForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/NoteDiscussionForm.kt index b96de2b78ee..22f6dacfee7 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/NoteDiscussionForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/note_discussion/NoteDiscussionForm.kt @@ -109,10 +109,10 @@ class NoteDiscussionForm : AbstractQuestForm() { override fun onClickOk() { require(noteText != null) { "NoteQuest has been answered with an empty comment!" } - val imagePaths = attachPhotoFragment?.imagePaths.orEmpty() + val photos = attachPhotoFragment?.photos.orEmpty() viewLifecycleScope.launch { withContext(Dispatchers.IO) { - noteEditsController.add(noteId, NoteEditAction.COMMENT, geometry.center, noteText, imagePaths) + noteEditsController.add(noteId, NoteEditAction.COMMENT, geometry.center, noteText, photos) } listener?.onNoteQuestSolved(questType, noteId, geometry.center) } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt index 2c6527fe599..1899b612d6b 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt @@ -28,6 +28,7 @@ import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.GridLayoutManager import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView.ViewHolder +import coil.dispose import com.google.android.material.textfield.TextInputLayout import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.data.preferences.Preferences @@ -57,8 +58,13 @@ sealed class PhotoAttachment { * replace, if any (never another [Pending] or [None] - see ALongForm.onPhotoCaptured) - kept * so cancelling this capture (the remove button) can revert to it instead of always dropping to * [None] and silently losing track of an already-synced photo that was never actually asked - * to be removed. */ - data class Pending(val path: String, val bearing: Float = 0f, val replaces: PhotoAttachment? = null) : PhotoAttachment() + * to be removed. [capturedAt] is when it was taken (epoch milliseconds). */ + data class Pending( + val path: String, + val bearing: Float? = null, + val replaces: PhotoAttachment? = null, + val capturedAt: Long? = null, + ) : PhotoAttachment() /** Attached and synced on a previous visit, read back from the element's own tags. */ data class Uploaded(val url: String) : PhotoAttachment() /** Was [Uploaded] as [url], but the user tapped delete this visit - shown as a distinct, @@ -514,8 +520,14 @@ class LongFormAdapter( // survive rebinds and holders recycled from other questions updateChoiceFollowUp(item) + // from the live givenItems entry, not [item]: that may be a snapshot taken before the + // selection was made (items is only resynced for questions in needRefreshIds), e.g. + // when this row is rebound after a photo was attached to its choiceFollowUp - the + // selection highlight vanished while the photo card (resolved live, see + // updateChoiceFollowUp) still showed + val live = item.questId?.let { id -> givenItems.firstOrNull { it.questId == id } } ?: item imageSelectAdapter.selectedIndices = - item.selectedIndex ?: emptyList() + live.selectedIndex ?: emptyList() selectionListener?.let { imageSelectAdapter.listeners.remove(it) } val listener = object : ImageSelectAdapter.OnItemSelectionListener { @@ -644,6 +656,9 @@ class LongFormAdapter( showUndo = false, onThumbClick = { openLocalPhotoFullScreen(binding.root.context, attachment.path) }, ) { + // a still running load of a previously shown uploaded photo mustn't replace it + binding.photoThumb.dispose() + binding.photoThumb.tag = null binding.photoThumb.doOnLayout { val bitmap = decodeScaledBitmapAndNormalize(attachment.path, it.width, it.height) binding.photoThumb.setImageBitmap(bitmap) @@ -651,21 +666,43 @@ class LongFormAdapter( } is PhotoAttachment.Uploaded -> bindPhotoCard( title = "Photo from last visit", - subtitle = "Retake or remove it", + subtitle = "You can retake or remove it", showDelete = true, showUndo = false, onThumbClick = { openRemotePhotoFullScreen(binding.root.context, attachment.url) }, - ) { binding.photoThumb.setImage(ImageUrl(attachment.url), progressBar = binding.photoThumbProgress) } + ) { loadUploadedPhotoThumb(attachment.url) } is PhotoAttachment.PendingRemoval -> bindPhotoCard( title = "Photo will be removed", subtitle = "Removed when you submit", showDelete = false, showUndo = true, onThumbClick = { openRemotePhotoFullScreen(binding.root.context, attachment.url) }, - ) { binding.photoThumb.setImage(ImageUrl(attachment.url), progressBar = binding.photoThumbProgress) } + ) { loadUploadedPhotoThumb(attachment.url) } } } + /** Shows the already uploaded photo at [url] in the small preview - by its thumbnail (see + * getPhotoPreviewUrl) or, if that can't be loaded (e.g. 404), by [url] itself. Only one + * retry: the fallback's own failure leaves the preview blank, as before. (setImage reports + * a failed load - and a load skipped in low-bandwidth mode - via imageIsEmptyUpdateTextSize; + * rebinding the view cancels a pending load, so a late failure can't load into a recycled + * view.) */ + private fun loadUploadedPhotoThumb(url: String) { + val previewUrl = getPhotoPreviewUrl(url) + binding.photoThumb.tag = url + binding.photoThumb.setImage( + ImageUrl(previewUrl), + imageIsEmptyUpdateTextSize = { + // still meant to show this photo (not rebound in the meantime) + val stillShown = binding.photoThumb.tag == url + if (stillShown && previewUrl != url && !preferences.isLowBandwidthModeEnabled) { + binding.photoThumb.setImage(ImageUrl(url), progressBar = binding.photoThumbProgress) + } + }, + progressBar = binding.photoThumbProgress, + ) + } + /** The retake camera icon is always shown once any photo exists - capturing a new one * always supersedes whatever's there (see ALongForm.onPhotoCaptured), so there's never a * state where replacing needs an extra step first. [showDelete] (the remove button, @@ -886,7 +923,7 @@ private fun openLocalPhotoFullScreen(context: Context, path: String) { } } -/** Opens the KartaView photo at [url] full-screen via a plain ACTION_VIEW intent - the browser, +/** Opens the uploaded photo at [url] full-screen via a plain ACTION_VIEW intent - the browser, * typically, since it's a plain https URL. */ private fun openRemotePhotoFullScreen(context: Context, url: String) { try { diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormSubmission.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormSubmission.kt index 42e4f32a1d6..81ca34b1836 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormSubmission.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormSubmission.kt @@ -1,9 +1,30 @@ package de.westnordost.streetcomplete.quests.sidewalk_long_form.data import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhoto +import de.westnordost.streetcomplete.data.preferences.Environment +/** URL of the (Panoramax) photo attached to the element - see ElementEditsUploader */ +const val STREET_IMAGES_TAG = "ext:street_images" +/** where the photo URL went before the switch from KartaView to Panoramax - still read, and + * removed along with the photo */ const val KARTAVIEW_URL_TAG = "ext:kartaview_url" +/** The URL of the photo already attached to an element with [tags], if any */ +fun getAttachedPhotoUrl(tags: Map): String? = + tags[STREET_IMAGES_TAG] ?: tags[KARTAVIEW_URL_TAG] + +/** The URL to show the attached photo at [url] with in the form's small preview: for a picture of + * our Panoramax instance its thumbnail (~500px wide) instead of the HD image - the tag may also + * come from other clients that upload full-size photos (several MB). Opening the photo + * full-screen still uses [url]. Any other URL (e.g. KartaView) is returned as is. */ +fun getPhotoPreviewUrl(url: String): String { + val isPanoramaxHd = url.endsWith(PANORAMAX_HD_SUFFIX) && + Environment.entries.any { url.startsWith(it.panoramaxBaseUrl + "api/pictures/") } + return if (isPanoramaxHd) url.removeSuffix(PANORAMAX_HD_SUFFIX) + "/thumb.jpg" else url +} + +private const val PANORAMAX_HD_SUFFIX = "/hd.jpg" + /** The photo-related state of a long-form quest while it is open - see ALongForm. Kept as plain * values (with the pure transitions below) rather than private Fragment state so the photo * lifecycle and the submit decision can be unit-tested without a Fragment. */ @@ -24,7 +45,7 @@ data class PhotoTransition(val state: PhotoState, val fileToDelete: String? = nu * it ever existed. Retaking again before submitting (Pending -> camera -> new Pending) * carries the ORIGINAL replaces forward, not the intermediate Pending, so no matter how * many times the user retakes, cancelling always lands back on the true starting point. */ -fun PhotoState.afterPhotoCaptured(path: String, bearing: Float): PhotoTransition { +fun PhotoState.afterPhotoCaptured(path: String, bearing: Float?, capturedAt: Long? = null): PhotoTransition { val previous = attachment val replaces = when (previous) { is PhotoAttachment.Pending -> previous.replaces @@ -32,7 +53,7 @@ fun PhotoState.afterPhotoCaptured(path: String, bearing: Float): PhotoTransition PhotoAttachment.None -> null } return PhotoTransition( - copy(attachment = PhotoAttachment.Pending(path, bearing, replaces)), + copy(attachment = PhotoAttachment.Pending(path, bearing, replaces, capturedAt)), fileToDelete = (previous as? PhotoAttachment.Pending)?.path ) } @@ -146,9 +167,14 @@ fun computeLongFormSubmission( // later, in the background sync - see AbstractOsmQuestForm.applyAnswer) - only an // explicit removal of an already-synced photo (without a replacement) is a tag change we // can express right away - val photo = (finalPhotoState.attachment as? PhotoAttachment.Pending)?.let { FeaturePhoto(it.path, it.bearing) } + // the question whose choiceFollowUp asked for the photo - the same one whose row shows it + // (see LongFormAdapter.photoAttachment) + val photoQuestion = givenItems.firstOrNull { it.visible && it.activeChoiceFollowUp() != null } + val photo = (finalPhotoState.attachment as? PhotoAttachment.Pending)?.let { + FeaturePhoto(it.path, it.bearing, photoQuestion?.questTitle, photoQuestion?.questId, it.capturedAt) + } val removeTagKeys = if (photo == null && finalPhotoState.existingPhotoRemoved) { - listOf(KARTAVIEW_URL_TAG) + listOf(STREET_IMAGES_TAG, KARTAVIEW_URL_TAG) } else { emptyList() } 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 d51f1bb9d42..2d1b0e864df 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 @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.screens.main +import de.westnordost.streetcomplete.quests.note_discussion.AttachPhotoFragment import android.annotation.SuppressLint import android.app.AlertDialog import android.content.BroadcastReceiver @@ -221,6 +222,7 @@ class MainActivity : LeaveNoteInsteadFragment.Listener, CreateNoteFragment.Listener, CreateFeatureFragment.Listener, + AttachPhotoFragment.Listener, MoveNodeFragment.Listener, // listeners to changes to data: VisibleQuestsSource.Listener, diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/AbstractCreateNoteFragment.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/AbstractCreateNoteFragment.kt index 126d238e2d1..8b8d3149da4 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/AbstractCreateNoteFragment.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/AbstractCreateNoteFragment.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.screens.main.bottom_sheet +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import android.os.Bundle import android.view.View import android.widget.EditText @@ -32,7 +33,7 @@ abstract class AbstractCreateNoteFragment : AbstractBottomSheetFragment() { } protected fun onClickOk() { - onComposedNote(noteText!!, attachPhotoFragment?.imagePaths.orEmpty()) + onComposedNote(noteText!!, attachPhotoFragment?.photos.orEmpty()) } override fun onDiscard() { @@ -53,5 +54,5 @@ abstract class AbstractCreateNoteFragment : AbstractBottomSheetFragment() { } } - protected abstract fun onComposedNote(text: String, imagePaths: List) + protected abstract fun onComposedNote(text: String, photos: List) } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateFeatureFragment.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateFeatureFragment.kt index e6f29853699..8e5b2eb9376 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateFeatureFragment.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateFeatureFragment.kt @@ -266,7 +266,7 @@ class CreateFeatureFragment : AbstractBottomSheetFragment() { screenPos.offset(0, -statusBarHeight) val position = listener?.getMapPositionAt(screenPos.toPointF()) ?: return - val imagePaths = attachPhotoFragment?.imagePaths.orEmpty() + val photos = attachPhotoFragment?.photos.orEmpty() val notes = contentBinding.notesInput.text.toString().trim() val tags = if (notes.isNotEmpty()) preset.tags + ("ext:notes" to notes) else preset.tags @@ -284,10 +284,9 @@ class CreateFeatureFragment : AbstractBottomSheetFragment() { ) // photos are NOT deleted here - they live on disk until the edit syncs, at which // point they are uploaded and cleaned up (or deleted with the edit if it is undone) - if (imagePaths.isNotEmpty()) { - // no capture bearing tracked here yet - see FeaturePhoto/ALongForm for the - // long-form equivalent this could follow if Add Feature gains it later - featurePhotosController.add(editId, imagePaths.map { FeaturePhoto(it) }) + if (photos.isNotEmpty()) { + // each with its own bearing and capture time + featurePhotosController.add(editId, photos.map { FeaturePhoto(it, featureType = preset.name) }) } } listener?.onCreatedFeature(position) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt index 5354e95f058..dc9ea6edca3 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/bottom_sheet/CreateNoteFragment.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.screens.main.bottom_sheet +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import android.content.res.Configuration import android.graphics.PointF import android.os.Bundle @@ -160,7 +161,7 @@ class CreateNoteFragment : AbstractCreateNoteFragment() { binding.markerCreateLayout.markerLayoutContainer.visibility = View.INVISIBLE } - override fun onComposedNote(text: String, imagePaths: List) { + override fun onComposedNote(text: String, photos: List) { /* pressing once on "OK" should first only close the keyboard, so that the user can review the position of the note he placed */ if (contentBinding.noteInput.isKeyboardOpen) { @@ -188,7 +189,7 @@ class CreateNoteFragment : AbstractCreateNoteFragment() { if (hasGpxAttached) listener?.getRecordedTrack().orEmpty() else emptyList() noteEditsController.add( 0, NoteEditAction.CREATE, position, - text, imagePaths, recordedTrack + text, photos, recordedTrack ) } } diff --git a/app/src/androidMain/res/xml/file_paths.xml b/app/src/androidMain/res/xml/file_paths.xml index bc55a97853d..b0ccd3f3159 100644 --- a/app/src/androidMain/res/xml/file_paths.xml +++ b/app/src/androidMain/res/xml/file_paths.xml @@ -1,6 +1,6 @@ + no hardcoded applicationId needed (see PhotoCapture's createPhotoFile) --> 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 c61ed116291..fa4cf2058ae 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 @@ -2,26 +2,31 @@ 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.panoramax.PanoramaxApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxException +import de.westnordost.streetcomplete.data.panoramax.PanoramaxPhoto import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesController import de.westnordost.streetcomplete.data.osm.edits.ElementEditAction import de.westnordost.streetcomplete.data.osm.edits.ElementEditsController import de.westnordost.streetcomplete.data.osm.edits.ElementEdit +import de.westnordost.streetcomplete.data.osm.edits.create.CreateNodeAction import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhoto import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosController import de.westnordost.streetcomplete.data.osm.edits.create_feature.StuckPhotoUploadNoticesController import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChanges import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryChange +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryModify import de.westnordost.streetcomplete.data.osm.edits.update_tags.UpdateElementTagsAction import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChangesetsManager import de.westnordost.streetcomplete.data.osm.mapdata.ElementKey import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.ElementIdUpdate import de.westnordost.streetcomplete.data.osm.mapdata.MapDataApiClient import de.westnordost.streetcomplete.data.osm.mapdata.MapDataController import de.westnordost.streetcomplete.data.osm.mapdata.MapDataUpdates +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsController import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener import de.westnordost.streetcomplete.data.user.statistics.StatisticsController @@ -55,7 +60,7 @@ class ElementEditsUploaderTest { private lateinit var mapDataApi: MapDataApiClient private lateinit var statisticsController: StatisticsController private lateinit var discardedEditNoticesController: DiscardedEditNoticesController - private lateinit var imageUploader: KartaViewApiClient + private lateinit var imageUploader: PanoramaxApiClient private lateinit var featurePhotosController: FeaturePhotosController private lateinit var stuckPhotoUploadNoticesController: StuckPhotoUploadNoticesController private lateinit var changesetManager: OpenChangesetsManager @@ -204,9 +209,9 @@ class ElementEditsUploaderTest { //endregion - //region photo uploaded successfully (long form: ext:kartaview_url) + //region photo uploaded successfully (long form: ext:street_images) - /** A long-form answer (surface) with a photo waiting to be uploaded; KartaView answers [url]. */ + /** A long-form answer (surface) with a photo waiting to be uploaded; Panoramax answers [url]. */ private suspend fun givenAnswerWithPhoto(existingTags: Map = emptyMap(), url: String? = PHOTO_URL): ElementEdit { val edit = edit(action = UpdateElementTagsAction( node(id = 1, tags = existingTags), @@ -230,20 +235,23 @@ class ElementEditsUploaderTest { uploader.upload() - verify(imageUploader).upload(listOf("/photos/p.jpg" to 90f), edit.position) + verify(imageUploader).upload( + listOf(PanoramaxPhoto("/photos/p.jpg", 90f, mapOf("element_id" to "node/1"), edit.createdTimestamp)), + edit.position + ) assertEquals( - setOf(StringMapEntryAdd("surface", "concrete"), StringMapEntryAdd("ext:kartaview_url", PHOTO_URL)), + setOf(StringMapEntryAdd("surface", "concrete"), StringMapEntryAdd("ext:street_images", PHOTO_URL)), uploadedChanges() ) verify(elementEditsController).markSynced(any(), any()) } @Test fun `a new photo replaces the element's existing photo URL`() = runBlocking { - givenAnswerWithPhoto(existingTags = mapOf("ext:kartaview_url" to "https://old/photo.jpg")) + givenAnswerWithPhoto(existingTags = mapOf("ext:street_images" to "https://old/photo.jpg")) uploader.upload() - assertTrue(StringMapEntryModify("ext:kartaview_url", "https://old/photo.jpg", PHOTO_URL) in uploadedChanges()) + assertTrue(StringMapEntryModify("ext:street_images", "https://old/photo.jpg", PHOTO_URL) in uploadedChanges()) } // a crash between the two must neither lose the URL nor upload the photo twice @@ -258,7 +266,7 @@ class ElementEditsUploaderTest { order.verify(singleUploader).upload(any(), any()) } - @Test fun `no URL from KartaView uploads the answer as it is`() = runBlocking { + @Test fun `no URL from Panoramax uploads the answer as it is`() = runBlocking { val edit = givenAnswerWithPhoto(url = null) uploader.upload() @@ -268,7 +276,7 @@ class ElementEditsUploaderTest { verify(featurePhotosController).markUploaded(edit.id) } - @Test fun `an answer without a photo doesn't touch KartaView`() = runBlocking { + @Test fun `an answer without a photo doesn't touch Panoramax`() = runBlocking { on(elementEditsController.getOldestUnsynced()).thenReturn(edit()).thenReturn(null) on(singleUploader.upload(any(), any())).thenReturn(MapDataUpdates()) @@ -277,9 +285,138 @@ class ElementEditsUploaderTest { verifyNoInteractions(imageUploader) } + @Test fun `the question the photo was taken for is sent along`() = runBlocking { + val edit = givenAnswerWithPhoto() + on(featurePhotosController.get(edit.id)).thenReturn(listOf(FeaturePhoto("/photos/p.jpg", null, "Is it steep?", 12))) + + uploader.upload() + + verify(imageUploader).upload( + listOf(PanoramaxPhoto("/photos/p.jpg", null, mapOf( + "element_id" to "node/1", "question" to "Is it steep?", "quest_id" to "12" + ), edit.createdTimestamp)), + edit.position + ) + } + + // not the edit's creation time (or the upload time): the form may have stayed open a while + @Test fun `the time the photo was taken is sent as its capture time`() = runBlocking { + val edit = givenAnswerWithPhoto() + on(featurePhotosController.get(edit.id)).thenReturn(listOf(FeaturePhoto("/photos/p.jpg", 90f, capturedAt = 100L))) + + uploader.upload() + + verify(imageUploader).upload( + listOf(PanoramaxPhoto("/photos/p.jpg", 90f, mapOf("element_id" to "node/1"), captureTime = 100L)), + edit.position + ) + } + + // photos uploaded to KartaView before the switch to Panoramax + @Test fun `a new photo replaces the element's KartaView photo URL`() = runBlocking { + givenAnswerWithPhoto(existingTags = mapOf("ext:kartaview_url" to "https://old/photo.jpg")) + + uploader.upload() + + assertEquals( + setOf( + StringMapEntryAdd("surface", "concrete"), + StringMapEntryAdd("ext:street_images", PHOTO_URL), + StringMapEntryDelete("ext:kartaview_url", "https://old/photo.jpg"), + ), + uploadedChanges() + ) + } + //endregion - //region KartaView failing with photos queued + //region photos of a new node + + /** A new node with two photos waiting to be uploaded, which is created as node 42 */ + private suspend fun givenNewNodeWithPhotos(): ElementEdit { + val edit = edit(action = CreateNodeAction(LatLon(1.0, 2.0), mapOf("amenity" to "bench"))) + on(elementEditsController.getOldestUnsynced()).thenReturn(edit).thenReturn(null) + on(featurePhotosController.get(edit.id)).thenReturn(listOf(FeaturePhoto("/photos/a.jpg"), FeaturePhoto("/photos/b.jpg"))) + on(imageUploader.upload(any(), any())).thenReturn(listOf(PHOTO_URL, PHOTO_URL_2)) + on(singleUploader.upload(any(), any())).thenReturn( + MapDataUpdates(idUpdates = listOf(ElementIdUpdate(ElementType.NODE, -1, 42))) + ) + return edit + } + + @Test fun `photos of a new node are uploaded with its feature type`() = runBlocking { + val edit = givenNewNodeWithPhotos() + on(featurePhotosController.get(edit.id)).thenReturn(listOf( + FeaturePhoto("/photos/a.jpg", 45f, capturedAt = 100L, featureType = "Intersection"), + )) + + uploader.upload() + + verify(imageUploader).upload( + listOf(PanoramaxPhoto( + "/photos/a.jpg", 45f, + mapOf("source" to "new_feature", "feature_type" to "Intersection"), + captureTime = 100L + )), + edit.position + ) + } + + @Test fun `photos of a new node are uploaded without element id and go up with the node`() = runBlocking { + val edit = givenNewNodeWithPhotos() + + uploader.upload() + + // its id is not known yet + // nor when they were taken: right before the node was made + val semantics = mapOf("source" to "new_feature") + verify(imageUploader).upload( + listOf( + PanoramaxPhoto("/photos/a.jpg", null, semantics, edit.createdTimestamp), + PanoramaxPhoto("/photos/b.jpg", null, semantics, edit.createdTimestamp), + ), + edit.position + ) + val uploaded = argumentCaptor() + verify(singleUploader).upload(capture(uploaded), any()) + assertEquals( + mapOf("amenity" to "bench", "ext:image_url1" to PHOTO_URL, "ext:image_url2" to PHOTO_URL_2), + (uploaded.value.action as CreateNodeAction).tags + ) + } + + @Test fun `once the new node is uploaded, its id is added to its photos`() = runBlocking { + givenNewNodeWithPhotos() + + uploader.upload() + + verify(imageUploader).addSemantics(PHOTO_URL, mapOf("element_id" to "node/42")) + verify(imageUploader).addSemantics(PHOTO_URL_2, mapOf("element_id" to "node/42")) + } + + // the node is already uploaded at that point - that must not be undone or repeated + @Test fun `failing to add the id to the photos doesn't fail the upload`() = runBlocking { + givenNewNodeWithPhotos() + on(imageUploader.addSemantics(any(), any())).thenThrow(PanoramaxException("down", isUnreachable = true)) + + uploader.upload() + + verify(elementEditsController).markSynced(any(), any()) + // still tried for the other photo + verify(imageUploader).addSemantics(PHOTO_URL_2, mapOf("element_id" to "node/42")) + } + + @Test fun `a tag update's photos are not patched afterwards`() = runBlocking { + givenAnswerWithPhoto() + + uploader.upload() + + verify(imageUploader, never()).addSemantics(any(), any()) + } + + //endregion + + //region Panoramax failing with photos queued private fun answerWithPhoto(id: Long): ElementEdit { val edit = edit(id = id, action = UpdateElementTagsAction( @@ -290,23 +427,28 @@ class ElementEditsUploaderTest { return edit } - @Test fun `once KartaView is unreachable, the other queued photos are not tried in the same run`() = runBlocking { + /** what answerWithPhoto([id])'s photo is uploaded as - taken when the edit was made */ + private fun photo(id: Long) = PanoramaxPhoto( + "/photos/$id.jpg", 0f, mapOf("element_id" to "node/$id"), captureTime = edit(id = id).createdTimestamp + ) + + @Test fun `once Panoramax 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(imageUploader.upload(any(), any())).thenThrow(PanoramaxException("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()) + verify(imageUploader).upload(listOf(photo(1)), first.position) + verify(imageUploader, never()).upload(eq(listOf(photo(2))), 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 + // an answer without a photo doesn't need Panoramax verify(singleUploader).upload(eq(withoutPhoto), any()) verify(elementEditsController, never()).markSynced(eq(second), any()) } @@ -316,21 +458,22 @@ class ElementEditsUploaderTest { 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(imageUploader.upload(eq(listOf(photo(1))), any())) + .thenThrow(PanoramaxException("400 Bad Request")) + on(imageUploader.upload(eq(listOf(photo(2))), 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(imageUploader).upload(listOf(photo(2)), second.position) verify(elementEditsController).markSynced(any(), any()) } //endregion private companion object { - const val PHOTO_URL = "https://storage.kartaview.org/files/photo/lth/1.jpg" + const val PHOTO_URL = "https://panoramax.example/api/pictures/1/hd.jpg" + const val PHOTO_URL_2 = "https://panoramax.example/api/pictures/2/hd.jpg" } } diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsControllerTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsControllerTest.kt index 0cd4ff8b1c8..93f55ddf338 100644 --- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsControllerTest.kt +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsControllerTest.kt @@ -1,5 +1,10 @@ package de.westnordost.streetcomplete.data.osmnotes.edits +import kotlin.test.assertTrue +import kotlin.test.assertEquals +import de.westnordost.streetcomplete.testutils.capture +import de.westnordost.streetcomplete.testutils.argumentCaptor +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import de.westnordost.streetcomplete.data.osm.mapdata.ElementIdUpdate import de.westnordost.streetcomplete.data.osm.mapdata.ElementType import de.westnordost.streetcomplete.testutils.any @@ -38,6 +43,19 @@ class NoteEditsControllerTest { verify(listener).onAddedEdit(any()) } + @Test fun `add keeps the bearing and capture time of each photo`() { + ctrl.add(1L, NoteEditAction.COMMENT, p(1.0, 1.0), "hi", listOf( + CapturedPhoto("a.jpg", 90f, 100L), + CapturedPhoto("b.jpg", null, 200L), + )) + + val added = argumentCaptor() + verify(db).add(capture(added)) + assertEquals(listOf("a.jpg", "b.jpg"), added.value.imagePaths) + assertEquals(listOf(CapturedPhoto("a.jpg", 90f, 100L), CapturedPhoto("b.jpg", null, 200L)), added.value.photos) + assertTrue(added.value.imagesNeedActivation) + } + @Test fun syncFailed() { val edit = noteEdit(noteId = 1) ctrl.markSyncFailed(edit) diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt index 447db1353fe..3082f266c32 100644 --- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploaderTest.kt @@ -2,7 +2,8 @@ package de.westnordost.streetcomplete.data.osmnotes.edits import de.westnordost.streetcomplete.ApplicationConstants import de.westnordost.streetcomplete.data.ConflictException -import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxPhoto import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.NoteController import de.westnordost.streetcomplete.data.osmnotes.NotesApiClient @@ -33,7 +34,7 @@ class NoteEditsUploaderTest { private lateinit var noteEditsController: NoteEditsController private lateinit var notesApi: NotesApiClient private lateinit var tracksApi: TracksApiClient - private lateinit var imageUploader: KartaViewApiClient + private lateinit var imageUploader: PanoramaxApiClient private lateinit var userDataSource: UserDataSource private lateinit var uploader: NoteEditsUploader @@ -170,10 +171,33 @@ class NoteEditsUploaderTest { verify(noteController).put(note) verify(noteEditsController).markSynced(edit, note) verify(noteEditsController).markImagesActivated(1L) - verify(imageUploader).upload(listOf("a" to 0f, "b" to 0f, "c" to 0f), pos) + // taken right before the note was made + val photos = listOf("a", "b", "c").map { PanoramaxPhoto(it, captureTime = edit.createdTimestamp) } + verify(imageUploader).upload(photos, pos) verify(listener)!!.onUploaded("NOTE", pos) } + @Test fun `each attached image is uploaded with its own bearing and capture time`() = runBlocking { + val pos = p(1.0, 13.0) + val edit = noteEdit(noteId = 1L, action = NoteEditAction.COMMENT, text = "test", pos = pos, imagePaths = listOf("a", "b")) + .copy(imageBearings = listOf(90f, null), imageCaptureTimes = listOf(100L, null)) + + on(noteEditsController.getOldestUnsynced()).thenReturn(edit).thenReturn(null) + on(notesApi.comment(anyLong(), any())).thenReturn(note(1)) + on(imageUploader.upload(any(), any())).thenReturn(listOf("x", "y")) + + upload() + + verify(imageUploader).upload( + listOf( + PanoramaxPhoto("a", 90f, captureTime = 100L), + // capture time unknown: the note was made right after it was taken + PanoramaxPhoto("b", null, captureTime = edit.createdTimestamp), + ), + pos + ) + } + @Test fun `upload create note with attached images`() = runBlocking { val pos = p(1.0, 13.0) val edit = noteEdit( @@ -195,7 +219,9 @@ class NoteEditsUploaderTest { verify(noteController).put(note) verify(noteEditsController).markSynced(edit, note) verify(noteEditsController).markImagesActivated(1L) - verify(imageUploader).upload(listOf("a" to 0f, "b" to 0f, "c" to 0f), pos) + // taken right before the note was made + val photos = listOf("a", "b", "c").map { PanoramaxPhoto(it, captureTime = edit.createdTimestamp) } + verify(imageUploader).upload(photos, pos) verify(listener)!!.onUploaded("NOTE", pos) } @@ -229,7 +255,7 @@ class NoteEditsUploaderTest { } @Test fun `upload missed image activations`(): Unit = runBlocking { - // KartaView photos need no activation step (unlike the old PhotoServiceApiClient) - this + // Panoramax photos need no activation step (unlike the old PhotoServiceApiClient) - this // just clears any leftover flags from before the switch, with no imageUploader call at all val edit = noteEdit(noteId = 3) diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClientTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClientTest.kt new file mode 100644 index 00000000000..71ac83e62e7 --- /dev/null +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClientTest.kt @@ -0,0 +1,361 @@ +package de.westnordost.streetcomplete.data.panoramax + +import de.westnordost.streetcomplete.data.photos.CapturedPhoto +import com.russhwolf.settings.MapSettings +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.preferences.Environment +import de.westnordost.streetcomplete.data.preferences.EnvironmentManager +import de.westnordost.streetcomplete.data.preferences.Preferences +import io.ktor.client.HttpClient +import io.ktor.client.engine.mock.MockEngine +import io.ktor.client.engine.mock.MockRequestHandleScope +import io.ktor.client.engine.mock.respond +import io.ktor.client.engine.mock.toByteArray +import io.ktor.client.plugins.contentnegotiation.ContentNegotiation +import io.ktor.client.request.HttpRequestData +import io.ktor.client.request.HttpResponseData +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpMethod +import io.ktor.http.HttpStatusCode +import io.ktor.http.headersOf +import io.ktor.serialization.kotlinx.json.json +import kotlinx.coroutines.test.runTest +import kotlinx.io.files.SystemFileSystem +import kotlinx.serialization.json.Json +import java.io.File +import java.io.IOException +import java.nio.channels.UnresolvedAddressException +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** The upload flow against the Panoramax API (upload set: create, add files, complete), what is + * sent along with each picture, and that every way an upload can fail surfaces as + * [PanoramaxException] - that's what the edit and note uploaders treat as "photo failed, keep the + * edit for the next sync" (and count towards the stuck-photo notice); anything else aborts the + * whole upload run. */ +class PanoramaxApiClientTest { + + private val base = Environment.PANORAMAX_DEV_URL + private val position = LatLon(47.6553, -122.3035) + + /** "METHOD path" of each request, in order */ + private val requests = mutableListOf() + /** request bodies of the file uploads, in order */ + private val fileBodies = mutableListOf() + private var createBody: String? = null + private var patchBody: String? = null + + private val preferences = Preferences(MapSettings()) + private lateinit var photo1: File + private lateinit var photo2: File + + @BeforeTest fun setUp() { + preferences.environment = Environment.DEV.name + photo1 = File.createTempFile("photo1", ".jpg").apply { writeBytes(ByteArray(8) { 1 }) } + photo2 = File.createTempFile("photo2", ".jpg").apply { writeBytes(ByteArray(8) { 2 }) } + } + + @AfterTest fun tearDown() { + photo1.delete() + photo2.delete() + } + + private fun client(answer: suspend MockRequestHandleScope.(HttpRequestData) -> HttpResponseData = { ok(it) }) = + PanoramaxApiClient( + SystemFileSystem, + HttpClient(MockEngine { request -> + requests.add("${request.method.value} ${request.url.encodedPath}") + answer(request) + }) { + install(ContentNegotiation) { json(Json { ignoreUnknownKeys = true }) } + }, + EnvironmentManager(preferences), + accessTokenOverride = "token", + ) + + /** like the real API when all goes well, picture ids pic-1, pic-2, ... */ + private suspend fun MockRequestHandleScope.ok(request: HttpRequestData): HttpResponseData { + val path = request.url.encodedPath + return when { + request.method == HttpMethod.Post && path == "/api/upload_sets" -> { + createBody = request.body.toByteArray().decodeToString() + json("""{"id":"set-1"}""") + } + request.method == HttpMethod.Post && path == "/api/upload_sets/set-1/files" -> { + fileBodies.add(request.body.toByteArray().decodeToString()) + json("""{"picture_id":"pic-${fileBodies.size}","rejected":null}""", HttpStatusCode.Accepted) + } + request.method == HttpMethod.Post && path == "/api/upload_sets/set-1/complete" -> + json("""{"id":"set-1"}""") + request.method == HttpMethod.Patch && path.startsWith("/api/pictures/") -> { + patchBody = request.body.toByteArray().decodeToString() + json("""{}""") + } + else -> respond("unexpected", HttpStatusCode.NotFound) + } + } + + private fun MockRequestHandleScope.json(body: String, status: HttpStatusCode = HttpStatusCode.OK) = + respond(body, status, headersOf(HttpHeaders.ContentType, "application/json")) + + //region uploading + + @Test fun `uploads all photos into one upload set and returns their picture URLs`() = runTest { + val urls = client().upload( + listOf(PanoramaxPhoto(photo1.path, 90f), PanoramaxPhoto(photo2.path, 180f)), + position + ) + + assertEquals( + listOf("${base}api/pictures/pic-1/hd.jpg", "${base}api/pictures/pic-2/hd.jpg"), + urls + ) + assertEquals( + listOf( + "POST /api/upload_sets", + "POST /api/upload_sets/set-1/files", + "POST /api/upload_sets/set-1/files", + "POST /api/upload_sets/set-1/complete", + ), + requests + ) + } + + @Test fun `uploads to the Panoramax instance of the current environment`() = runTest { + preferences.environment = Environment.STAGE.name + val hosts = mutableListOf() + + val urls = client { request -> + hosts.add(request.url.host) + ok(request) + }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + + assertEquals(listOf("${Environment.PANORAMAX_STAGE_URL}api/pictures/pic-1/hd.jpg"), urls) + val stageHost = Environment.PANORAMAX_STAGE_URL.removePrefix("https://").trimEnd('/') + assertEquals(listOf(stageHost), hosts.distinct()) + } + + @Test fun `PROD uploads to the prod Panoramax instance`() = runTest { + preferences.environment = Environment.PROD.name + val hosts = mutableListOf() + + val urls = client { request -> + hosts.add(request.url.host) + ok(request) + }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + + assertEquals(listOf("${Environment.PANORAMAX_PROD_URL}api/pictures/pic-1/hd.jpg"), urls) + val prodHost = Environment.PANORAMAX_PROD_URL.removePrefix("https://").trimEnd('/') + assertEquals(listOf(prodHost), hosts.distinct()) + assertNotEquals(Environment.PANORAMAX_DEV_URL, Environment.PANORAMAX_PROD_URL) + } + + // all photos of an edit are at the same position - with the server's deduplication, it'd drop + // all but the first when completing the upload set, after having given each a picture id + @Test fun `upload set is created without deduplication and splitting`() = runTest { + client().upload(listOf(PanoramaxPhoto(photo1.path), PanoramaxPhoto(photo2.path)), position) + + val body = createBody!! + assertTrue(body.contains("""no_deduplication":true"""), body) + assertTrue(body.contains("""no_split":true"""), body) + assertTrue(body.contains("""estimated_nb_files":2"""), body) + } + + @Test fun `position and bearing are sent along`() = runTest { + client().upload(listOf(PanoramaxPhoto(photo1.path, 90.4f)), position) + + val body = fileBodies.single() + assertTrue(body.hasField("override_latitude", "47.6553")) + assertTrue(body.hasField("override_longitude", "-122.3035")) + assertTrue(body.hasField("override_Exif.GPSInfo.GPSImgDirection", "90/1")) + assertTrue(body.hasField("override_Exif.GPSInfo.GPSImgDirectionRef", "T")) + assertTrue(Regex("name=\"?override_capture_time\"?\r\n").containsMatchIn(body)) + } + + @Test fun `capture time is the time the photo was taken, not the upload time`() = runTest { + // 2026-11-10T08:30:00Z + client().upload(listOf(PanoramaxPhoto(photo1.path, captureTime = 1_794_299_400_000L)), position) + + assertTrue(fileBodies.single().hasField("override_capture_time", "2026-11-10T08:30:00Z")) + } + + @Test fun `each photo is sent with its own capture time`() = runTest { + client().upload( + listOf( + PanoramaxPhoto(photo1.path, captureTime = 1_794_299_400_000L), + PanoramaxPhoto(photo2.path, captureTime = 1_794_299_460_000L), + ), + position + ) + + assertTrue(fileBodies[0].hasField("override_capture_time", "2026-11-10T08:30:00Z")) + assertTrue(fileBodies[1].hasField("override_capture_time", "2026-11-10T08:31:00Z")) + } + + @Test fun `a captured photo keeps its bearing and capture time`() { + assertEquals( + PanoramaxPhoto("a.jpg", 90f, mapOf("question" to "q"), 100L), + CapturedPhoto("a.jpg", 90f, 100L).toPanoramaxPhoto(fallbackCaptureTime = 5L, mapOf("question" to "q")) + ) + } + + @Test fun `a captured photo without capture time is taken at the fallback time`() { + assertEquals( + PanoramaxPhoto("a.jpg", null, captureTime = 5L), + CapturedPhoto("a.jpg").toPanoramaxPhoto(fallbackCaptureTime = 5L) + ) + } + + @Test fun `negative bearing is sent as 0-359`() = runTest { + client().upload(listOf(PanoramaxPhoto(photo1.path, -90f)), position) + + assertTrue(fileBodies.single().hasField("override_Exif.GPSInfo.GPSImgDirection", "270/1")) + } + + // 0 would claim the photo faces north + @Test fun `no bearing is sent if unknown`() = runTest { + client().upload(listOf(PanoramaxPhoto(photo1.path, null)), position) + + assertFalse(fileBodies.single().contains("GPSImgDirection")) + } + + @Test fun `workspace and the photo's own semantics are sent along`() = runTest { + preferences.workspaceId = 7 + + client().upload( + listOf(PanoramaxPhoto(photo1.path, 0f, mapOf( + "element_id" to "way/123", "question" to "Is it steep?", "quest_id" to "12" + ))), + position + ) + + val body = fileBodies.single() + assertTrue(body.contains( + """[{"key":"workspace_id","value":"7"},{"key":"element_id","value":"way/123"},""" + + """{"key":"question","value":"Is it steep?"},{"key":"quest_id","value":"12"}]""" + ), body) + } + + // the pictures are public + @Test fun `the user's email is never sent`() = runTest { + preferences.workspaceId = 7 + preferences.workspaceUserEmail = "surveyor@example.org" + + client().upload(listOf(PanoramaxPhoto(photo1.path)), position) + + val body = fileBodies.single() + assertTrue(body.contains("""[{"key":"workspace_id","value":"7"}]"""), body) + assertFalse(body.contains("surveyor@example.org")) + assertFalse(body.contains("user_email")) + } + + @Test fun `photos whose file doesn't exist are skipped`() = runTest { + val urls = client().upload( + listOf(PanoramaxPhoto("/does/not/exist.jpg"), PanoramaxPhoto(photo1.path)), + position + ) + + assertEquals(listOf("${base}api/pictures/pic-1/hd.jpg"), urls) + assertEquals(1, fileBodies.size) + } + + @Test fun `nothing to upload makes no request`() = runTest { + assertEquals(emptyList(), client { error("no request expected") }.upload(emptyList(), position)) + assertEquals( + emptyList(), + client { error("no request expected") }.upload(listOf(PanoramaxPhoto("/does/not/exist.jpg")), position) + ) + assertEquals(emptyList(), requests) + } + + //endregion + + //region failing + + @Test fun `unreachable Panoramax is a PanoramaxException marked as unreachable`() = runTest { + val e = assertFailsWith { + client { throw IOException("connection refused") }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + } + assertTrue(e.isUnreachable) + } + + @Test fun `DNS failure is a PanoramaxException`() = runTest { + // not an IOException + assertFailsWith { + client { throw UnresolvedAddressException() }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + } + } + + // the edit uploader stops trying the other queued photos for the run on an unreachable + // Panoramax, but not on an error status for one upload + @Test fun `error status is a PanoramaxException not marked as unreachable`() = runTest { + val e = assertFailsWith { + client { respond("down", HttpStatusCode.ServiceUnavailable) }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + } + assertFalse(e.isUnreachable) + } + + @Test fun `rejected picture is a PanoramaxException`() = runTest { + val e = assertFailsWith { + client { request -> + if (request.url.encodedPath.endsWith("/files")) { + json( + """{"picture_id":null,"rejected":{"reason":"invalid_metadata","severity":"error","message":"no position"}}""", + HttpStatusCode.Accepted + ) + } else ok(request) + }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + } + assertFalse(e.isUnreachable) + assertTrue(e.message!!.contains("invalid_metadata")) + } + + @Test fun `failing to complete the upload set is a PanoramaxException`() = runTest { + assertFailsWith { + client { request -> + if (request.url.encodedPath.endsWith("/complete")) { + respond("nope", HttpStatusCode.InternalServerError) + } else ok(request) + }.upload(listOf(PanoramaxPhoto(photo1.path)), position) + } + } + + //endregion + + //region adding semantics later + + @Test fun `adds semantics to an uploaded picture`() = runTest { + assertTrue(client().addSemantics("${base}api/pictures/pic-9/hd.jpg", mapOf("element_id" to "node/5"))) + + assertEquals(listOf("PATCH /api/pictures/pic-9"), requests) + assertEquals("""{"semantics":[{"key":"element_id","value":"node/5"}]}""", patchBody) + } + + // e.g. a photo uploaded to KartaView before the switch + @Test fun `does nothing for a picture of somewhere else`() = runTest { + assertFalse(client { error("no request expected") }.addSemantics( + "https://storage.kartaview.org/files/photo/lth/1.jpg", mapOf("element_id" to "node/5") + )) + assertEquals(emptyList(), requests) + } + + @Test fun `failing to add semantics is a PanoramaxException`() = runTest { + assertFailsWith { + client { respond("nope", HttpStatusCode.Forbidden) } + .addSemantics("${base}api/pictures/pic-9/hd.jpg", mapOf("element_id" to "node/5")) + } + } + + //endregion + + /** whether this multipart body has a form field [name] with [value] */ + private fun String.hasField(name: String, value: String): Boolean = + Regex("name=\"?${Regex.escape(name)}\"?\r\n(?:[^\r\n]+\r\n)*\r\n${Regex.escape(value)}\r\n") + .containsMatchIn(this) +} diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormSubmissionTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormSubmissionTest.kt index b907584baf4..e722af45454 100644 --- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormSubmissionTest.kt +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/LongFormSubmissionTest.kt @@ -7,6 +7,7 @@ import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.osm.Tags import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.Elements import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.KARTAVIEW_URL_TAG +import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.STREET_IMAGES_TAG import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.LongFormAdapter import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.LongFormSubmission import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.PhotoAttachment @@ -267,16 +268,23 @@ class LongFormSubmissionTest { val adapter = openForm(obstructionWithPhotoTags) val submission = adapter.submit(PhotoState(PhotoAttachment.Pending(pendingPath, 90f))) assertEquals(listOf(OBSTRUCTION_TYPE), submission.submittedIds()) - assertEquals(FeaturePhoto(pendingPath, 90f), submission.photo) + // with the question whose follow-up asked for it + assertEquals(FeaturePhoto(pendingPath, 90f, questId = OBSTRUCTION_TYPE), submission.photo) assertNull(submission.photoTransition) } + @Test fun `a new photo is submitted with the time it was taken`() { + val adapter = openForm(obstructionWithPhotoTags) + val submission = adapter.submit(PhotoState(PhotoAttachment.Pending(pendingPath, 90f, capturedAt = 100L))) + assertEquals(FeaturePhoto(pendingPath, 90f, questId = OBSTRUCTION_TYPE, capturedAt = 100L), submission.photo) + } + @Test fun `a new photo rides along with other changes`() { val adapter = openForm(obstructionWithPhotoTags) adapter.typeNumber(WIDTH, "60") val submission = adapter.submit(PhotoState(PhotoAttachment.Pending(pendingPath))) assertEquals(listOf(WIDTH), submission.submittedIds()) - assertEquals(FeaturePhoto(pendingPath, 0f), submission.photo) + assertEquals(FeaturePhoto(pendingPath, questId = OBSTRUCTION_TYPE), submission.photo) } @Test fun `a new photo replacing an existing one does not remove the tag up front`() { @@ -295,7 +303,7 @@ class LongFormSubmissionTest { val submission = adapter.submit(PhotoState(PhotoAttachment.PendingRemoval(photoUrl), existingPhotoRemoved = true)) assertFalse(submission.isEmpty) assertEquals(emptyList(), submission.items) - assertEquals(listOf(KARTAVIEW_URL_TAG), submission.removeTagKeys) + assertEquals(listOf(STREET_IMAGES_TAG, KARTAVIEW_URL_TAG), submission.removeTagKeys) assertNull(submission.photo) } @@ -322,7 +330,7 @@ class LongFormSubmissionTest { val adapter = openForm(obstructionWithPhotoTags) adapter.tapChoice(OBSTRUCTION_TYPE, "other") val submission = adapter.submit(PhotoState(PhotoAttachment.Uploaded(photoUrl))) - assertEquals(listOf(KARTAVIEW_URL_TAG), submission.removeTagKeys) + assertEquals(listOf(STREET_IMAGES_TAG, KARTAVIEW_URL_TAG), submission.removeTagKeys) assertNull(submission.photoTransition!!.fileToDelete) } @@ -333,7 +341,7 @@ class LongFormSubmissionTest { val submission = adapter.submit(photo) assertEquals(pendingPath, submission.photoTransition!!.fileToDelete) - assertEquals(listOf(KARTAVIEW_URL_TAG), submission.removeTagKeys) + assertEquals(listOf(STREET_IMAGES_TAG, KARTAVIEW_URL_TAG), submission.removeTagKeys) val tags = submission.applyTo(obstructionWithPhotoTags + (KARTAVIEW_URL_TAG to photoUrl)) assertEquals(mapOf("ext:obstruction" to "no"), tags) } diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoPreviewUrlTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoPreviewUrlTest.kt new file mode 100644 index 00000000000..f91e54fb982 --- /dev/null +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoPreviewUrlTest.kt @@ -0,0 +1,31 @@ +package de.westnordost.streetcomplete.quests.sidewalk_long_form + +import de.westnordost.streetcomplete.data.preferences.Environment +import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.getPhotoPreviewUrl +import kotlin.test.Test +import kotlin.test.assertEquals + +/** The form's small photo preview loads the Panoramax thumbnail, not the (possibly several MB) + * HD image - and leaves any other photo URL alone. */ +class PhotoPreviewUrlTest { + + private val pictures = "${Environment.PANORAMAX_DEV_URL}api/pictures/abc-123" + + @Test fun `Panoramax HD picture is previewed by its thumbnail`() { + assertEquals("$pictures/thumb.jpg", getPhotoPreviewUrl("$pictures/hd.jpg")) + } + + @Test fun `KartaView photo is previewed as is`() { + val url = "https://storage.kartaview.org/files/photo/lth/1.jpg" + assertEquals(url, getPhotoPreviewUrl(url)) + } + + @Test fun `HD picture of another server is previewed as is`() { + val url = "https://panoramax.example/api/pictures/abc-123/hd.jpg" + assertEquals(url, getPhotoPreviewUrl(url)) + } + + @Test fun `other image of our Panoramax is previewed as is`() { + assertEquals("$pictures/sd.jpg", getPhotoPreviewUrl("$pictures/sd.jpg")) + } +} diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoStateTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoStateTest.kt index a7f85930a7e..9c597cd6987 100644 --- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoStateTest.kt +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/PhotoStateTest.kt @@ -28,6 +28,13 @@ class PhotoStateTest { ) } + @Test fun `capture keeps when the photo was taken`() { + assertEquals( + Pending("a.jpg", 45f, capturedAt = 100L), + PhotoState().afterPhotoCaptured("a.jpg", 45f, 100L).state.attachment + ) + } + @Test fun `capture over an existing photo remembers what it replaces`() { assertEquals( PhotoState(Pending("a.jpg", 0f, replaces = Uploaded(url))), 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 01f72483903..68a591ad777 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 = 29 + const val DB_VERSION = 30 fun onCreate(db: Database) { // OSM notes @@ -359,6 +359,21 @@ object DatabaseInitializer { // (see LogcatHttpLogger), drop the ones already stored db.delete(LogsTable.NAME, where = "${LogsTable.Columns.TAG} IN ($HTTP_LOG_TAGS)") } + + if (oldVersion <= 29 && newVersion >= 30) { + // per-photo long-form question (title and quest_id), sent along to Panoramax as + // semantic tags + db.tryExec("ALTER TABLE ${FeaturePhotosTable.NAME} ADD COLUMN ${FeaturePhotosTable.Columns.PHOTO_QUESTIONS} text") + db.tryExec("ALTER TABLE ${FeaturePhotosTable.NAME} ADD COLUMN ${FeaturePhotosTable.Columns.PHOTO_QUEST_IDS} text") + // per-photo capture time, sent to Panoramax as the picture's capture time instead of + // the (possibly much later) upload time + db.tryExec("ALTER TABLE ${FeaturePhotosTable.NAME} ADD COLUMN ${FeaturePhotosTable.Columns.PHOTO_CAPTURED_AT} text") + // feature preset name of a new feature's photos, sent along as a semantic tag + db.tryExec("ALTER TABLE ${FeaturePhotosTable.NAME} ADD COLUMN ${FeaturePhotosTable.Columns.PHOTO_FEATURE_TYPES} text") + // same for the photos of notes - bearing and capture time of each one + db.tryExec("ALTER TABLE ${NoteEditsTable.NAME} ADD COLUMN ${NoteEditsTable.Columns.IMAGE_BEARINGS} text") + db.tryExec("ALTER TABLE ${NoteEditsTable.NAME} ADD COLUMN ${NoteEditsTable.Columns.IMAGE_CAPTURE_TIMES} text") + } } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosController.kt index fd665edd77d..cbe6966bc83 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosController.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosController.kt @@ -2,16 +2,36 @@ package de.westnordost.streetcomplete.data.osm.edits.create_feature import de.westnordost.streetcomplete.data.osm.edits.ElementEdit import de.westnordost.streetcomplete.data.osm.edits.ElementEditsSource +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import kotlinx.io.files.FileSystem import kotlinx.io.files.Path import kotlinx.serialization.Serializable /** One photo attached to a not-yet-synced edit, with the compass bearing (0-359, clockwise from - * north) the device was facing at capture time - or 0f when a caller doesn't track it (not every - * photo-attaching flow captures a bearing yet). Uploaded with that bearing at sync time - see + * true north) the camera was facing at capture time - or null if unknown (not every + * photo-attaching flow captures a bearing yet) - and the long-form [question] it was taken for, + * if any, and when it was taken ([capturedAt], epoch milliseconds - null if not tracked, the + * edit's creation time is used then), and the [featureType] (feature preset name, e.g. + * "Intersection") of the new feature it was taken of, if any. Uploaded with those at sync time - see * [de.westnordost.streetcomplete.data.osm.edits.upload.ElementEditsUploader.uploadPendingPhotos]. */ @Serializable -data class FeaturePhoto(val path: String, val bearing: Float = 0f) +data class FeaturePhoto( + val path: String, + val bearing: Float? = null, + val question: String? = null, + val questId: Int? = null, + val capturedAt: Long? = null, + val featureType: String? = null, +) { + constructor( + photo: CapturedPhoto, + question: String? = null, + questId: Int? = null, + featureType: String? = null, + ) : this(photo.path, photo.bearing, question, questId, photo.capturedAt, featureType) + + val captured: CapturedPhoto get() = CapturedPhoto(path, bearing, capturedAt) +} /** Manages the photos attached to not-yet-synced create-feature/tag-update edits: their file * paths (and capture bearings) are held here until the edit is uploaded, at which point the diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosDao.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosDao.kt index 2faa588f4b8..0dfbfabf598 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosDao.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosDao.kt @@ -3,7 +3,11 @@ package de.westnordost.streetcomplete.data.osm.edits.create_feature import de.westnordost.streetcomplete.data.Database import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.EDIT_ID import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_BEARINGS +import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_CAPTURED_AT +import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_FEATURE_TYPES import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_PATHS +import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_QUESTIONS +import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.PHOTO_QUEST_IDS import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.UPLOAD_ATTEMPTS import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.Columns.WORKSPACE_ID import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosTable.NAME @@ -26,6 +30,10 @@ class FeaturePhotosDao( EDIT_ID to editId, PHOTO_PATHS to json.encodeToString(photos.map { it.path }), PHOTO_BEARINGS to json.encodeToString(photos.map { it.bearing }), + PHOTO_QUESTIONS to json.encodeToString(photos.map { it.question }), + PHOTO_QUEST_IDS to json.encodeToString(photos.map { it.questId }), + PHOTO_CAPTURED_AT to json.encodeToString(photos.map { it.capturedAt }), + PHOTO_FEATURE_TYPES to json.encodeToString(photos.map { it.featureType }), WORKSPACE_ID to workspaceId )) } @@ -36,9 +44,20 @@ class FeaturePhotosDao( where = "$WORKSPACE_ID = $workspaceId AND $EDIT_ID = $editId" ) { row -> val paths = json.decodeFromString>(row.getString(PHOTO_PATHS)) - // absent on rows written before PHOTO_BEARINGS existed - treated as all-zero bearings - val bearings = row.getStringOrNull(PHOTO_BEARINGS)?.let { json.decodeFromString>(it) } - paths.mapIndexed { i, path -> FeaturePhoto(path, bearings?.getOrNull(i) ?: 0f) } + // absent on rows written before PHOTO_BEARINGS existed - unknown bearings then + val bearings = row.getStringOrNull(PHOTO_BEARINGS)?.let { json.decodeFromString>(it) } + // absent on rows written before PHOTO_QUESTIONS / PHOTO_QUEST_IDS existed + val questions = row.getStringOrNull(PHOTO_QUESTIONS)?.let { json.decodeFromString>(it) } + val questIds = row.getStringOrNull(PHOTO_QUEST_IDS)?.let { json.decodeFromString>(it) } + // absent on rows written before PHOTO_CAPTURED_AT existed + val capturedAts = row.getStringOrNull(PHOTO_CAPTURED_AT)?.let { json.decodeFromString>(it) } + val featureTypes = row.getStringOrNull(PHOTO_FEATURE_TYPES)?.let { json.decodeFromString>(it) } + paths.mapIndexed { i, path -> + FeaturePhoto( + path, bearings?.getOrNull(i), questions?.getOrNull(i), questIds?.getOrNull(i), + capturedAts?.getOrNull(i), featureTypes?.getOrNull(i) + ) + } } ?: emptyList() fun delete(editId: Long): Boolean = diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosTable.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosTable.kt index 32dcd175e2a..c4ae47f4a4b 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosTable.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/create_feature/FeaturePhotosTable.kt @@ -7,10 +7,23 @@ object FeaturePhotosTable { const val ID = "id" const val EDIT_ID = "edit_id" const val PHOTO_PATHS = "photo_paths" - /** JSON list of Floats, index-aligned with PHOTO_PATHS - the compass bearing the device - * was facing at capture time for each photo, or absent/null on rows written before this - * column existed (treated as all-zero bearings for those - see FeaturePhotosDao.get). */ + /** JSON list of nullable Floats, index-aligned with PHOTO_PATHS - the compass bearing the + * camera was facing at capture time for each photo (null: unknown), or absent/null on + * rows written before this column existed (unknown bearings - see FeaturePhotosDao.get). */ const val PHOTO_BEARINGS = "photo_bearings" + /** JSON list of nullable Strings, index-aligned with PHOTO_PATHS - the long-form question + * each photo was taken for, or absent/null on rows written before this column existed */ + const val PHOTO_QUESTIONS = "photo_questions" + /** JSON list of nullable Ints, index-aligned with PHOTO_PATHS - the quest_id of that + * long-form question, or absent/null on rows written before this column existed */ + const val PHOTO_QUEST_IDS = "photo_quest_ids" + /** JSON list of nullable Longs, index-aligned with PHOTO_PATHS - when each photo was + * taken (epoch milliseconds), or absent/null on rows written before this column existed */ + const val PHOTO_CAPTURED_AT = "photo_captured_at" + /** JSON list of nullable Strings, index-aligned with PHOTO_PATHS - the feature preset name + * of the new feature each photo was taken of, or absent/null on rows written before this + * column existed */ + const val PHOTO_FEATURE_TYPES = "photo_feature_types" /** How many times uploading this edit's photo(s) has failed in a row - see * ElementEditsUploader.uploadPendingPhotos and StuckPhotoUploadNotice. Reset to 0 once * the user picks "keep trying" on the resulting notice. */ @@ -24,6 +37,10 @@ object FeaturePhotosTable { ${Columns.EDIT_ID} int NOT NULL, ${Columns.PHOTO_PATHS} text NOT NULL, ${Columns.PHOTO_BEARINGS} text, + ${Columns.PHOTO_QUESTIONS} text, + ${Columns.PHOTO_QUEST_IDS} text, + ${Columns.PHOTO_CAPTURED_AT} text, + ${Columns.PHOTO_FEATURE_TYPES} text, ${Columns.UPLOAD_ATTEMPTS} int NOT NULL DEFAULT 0, ${Columns.WORKSPACE_ID} int NOT NULL ); 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 939e8cead48..59059e09dee 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 @@ -1,8 +1,9 @@ package de.westnordost.streetcomplete.data.osm.edits.upload 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.panoramax.PanoramaxApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxException +import de.westnordost.streetcomplete.data.panoramax.toPanoramaxPhoto import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesController import de.westnordost.streetcomplete.data.osm.edits.ElementEdit @@ -13,6 +14,8 @@ import de.westnordost.streetcomplete.data.osm.edits.create.CreateNodeAction import de.westnordost.streetcomplete.data.osm.edits.create_feature.FeaturePhotosController import de.westnordost.streetcomplete.data.osm.edits.create_feature.StuckPhotoUploadNotice import de.westnordost.streetcomplete.data.osm.edits.create_feature.StuckPhotoUploadNoticesController +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChanges +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete import de.westnordost.streetcomplete.data.osm.edits.update_tags.UpdateElementTagsAction import de.westnordost.streetcomplete.data.osm.edits.update_tags.withTag import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChangesetsManager @@ -29,6 +32,7 @@ import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener import de.westnordost.streetcomplete.data.user.statistics.StatisticsController import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds import de.westnordost.streetcomplete.util.logs.Log +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -46,7 +50,7 @@ class ElementEditsUploader( private val mapDataApi: MapDataApiClient, private val statisticsController: StatisticsController, private val discardedEditNoticesController: DiscardedEditNoticesController, - private val imageUploader: KartaViewApiClient, + private val imageUploader: PanoramaxApiClient, private val featurePhotosController: FeaturePhotosController, private val stuckPhotoUploadNoticesController: StuckPhotoUploadNoticesController, private val changesetManager: OpenChangesetsManager, @@ -57,21 +61,21 @@ class ElementEditsUploader( private val scope = CoroutineScope(SupervisorJob() + CoroutineName("ElementEditsUploader")) suspend fun upload() = mutex.withLock { withContext(Dispatchers.IO) { - // KartaView is a third-party service outside our control, and can be down/flaky + // Panoramax is a separate service, and can be down/flaky // independently of everything else - a photo upload failure here must never block every // OTHER queued edit (e.g. plain tag-only quests with no photo at all) from uploading. // Tracked only in memory, for this one upload() run: nothing is written to the DB for a // 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 + // once Panoramax 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 + var isPhotoServiceUnreachable = false try { while (true) { val edit = elementEditsController.getOldestUnsynced(failedEditIds) ?: break - if (isKartaViewUnreachable && hasPendingPhotos(edit)) { + if (isPhotoServiceUnreachable && hasPendingPhotos(edit)) { failedEditIds += edit.id continue } @@ -81,7 +85,7 @@ class ElementEditsUploader( * because otherwise an inconsistency in the data would occur. E.g. no "star" for * an uploaded change, a change could be uploaded twice etc */ withContext(scope.coroutineContext) { uploadEdit(edit, getIdProvider) } - } catch (e: KartaViewException) { + } catch (e: PanoramaxException) { /* the edit's photos failed to upload (plain network failure) - leave the edit * unsynced for the next sync attempt instead of letting this bubble up and abort * uploading of any other edits still queued, e.g. note edits (see @@ -91,7 +95,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 + if (e.isUnreachable) isPhotoServiceUnreachable = true } } } finally { @@ -104,7 +108,7 @@ class ElementEditsUploader( } } /** Bumps the edit's photo-upload-failure streak and, once it reaches [STUCK_PHOTO_UPLOAD_ATTEMPTS] - * (KartaView failing this many times in a row - not just a one-off network blip), raises a + * (Panoramax failing this many times in a row - not just a one-off network blip), raises a * notice so the user can choose to keep waiting or drop the photo and submit without it. * Only fires once per streak: as long as the notice is still unresolved, further failures * keep incrementing past the threshold without adding duplicate notices for the same edit. */ @@ -143,6 +147,7 @@ class ElementEditsUploader( elementEditsController.markSynced(edit, updates) mapDataController.updateAll(updates) noteEditsController.updateElementIds(updates.idUpdates) + addElementIdToNewNodePhotos(edit, updates) if (edit.action is IsRevertAction) { statisticsController.subtractOne(edit.type.name, edit.position) @@ -197,9 +202,9 @@ class ElementEditsUploader( } /** If the edit is a node creation or a long-form tag update with photo(s) still awaiting - * upload, uploads them to KartaView and folds the resulting URL(s) into the action's tags - + * upload, uploads them to Panoramax and folds the resulting URL(s) into the action's tags - * ext:image_url1, ext:image_url2, ... (one per image, in attach order) for a node creation, - * or the single ext:kartaview_url for a tag update (long form only ever attaches one photo + * or the single ext:street_images for a tag update (long form only ever attaches one photo * per edit - see ALongForm). The rewritten action is persisted BEFORE the photo records/files * are deleted, so a crash in between cannot lose the URL(s) or upload the photos twice. * Returns the edit whose action carries the URL tag(s). */ @@ -208,7 +213,25 @@ class ElementEditsUploader( if (!hasPendingPhotos(edit)) return edit val photos = featurePhotosController.get(edit.id) - val urls = imageUploader.upload(photos.map { it.path to it.bearing }, edit.position) + // the element is only known for a tag update - a new node gets its id on upload, which is + // added to the pictures afterwards, see addElementIdToNewNodePhotos + val elementId = (action as? UpdateElementTagsAction)?.originalElement?.let { element -> + toElementIdSemantic(element.type, element.id) + } + val urls = imageUploader.upload( + photos.map { photo -> + photo.captured.toPanoramaxPhoto(edit.createdTimestamp, buildMap { + elementId?.let { put(PanoramaxApiClient.ELEMENT_ID, it) } + if (action is CreateNodeAction) { + put(PanoramaxApiClient.SOURCE, PanoramaxApiClient.SOURCE_NEW_FEATURE) + photo.featureType?.let { put(PanoramaxApiClient.FEATURE_TYPE, it) } + } + photo.question?.let { put(PanoramaxApiClient.QUESTION, it) } + photo.questId?.let { put(PanoramaxApiClient.QUEST_ID, it.toString()) } + }) + }, + edit.position + ) var uploadedEdit = edit if (urls.isNotEmpty()) { uploadedEdit = when (action) { @@ -217,9 +240,16 @@ class ElementEditsUploader( edit.copy(action = action.copy(tags = action.tags + urlTags)) } is UpdateElementTagsAction -> { - val changes = action.changes.withTag( - KARTAVIEW_URL_TAG, urls.first(), action.originalElement.tags - ) + val originalTags = action.originalElement.tags + var changes = action.changes.withTag(STREET_IMAGES_TAG, urls.first(), originalTags) + // a photo uploaded to KartaView before the switch to Panoramax is replaced + // by this one, don't leave its URL behind next to the new one + originalTags[KARTAVIEW_URL_TAG]?.let { oldUrl -> + changes = StringMapChanges( + changes.changes.filterNot { it.key == KARTAVIEW_URL_TAG }.toSet() + + StringMapEntryDelete(KARTAVIEW_URL_TAG, oldUrl) + ) + } edit.copy(action = action.copy(changes = changes)) } else -> edit @@ -230,6 +260,30 @@ class ElementEditsUploader( return uploadedEdit } + /** Once a node created with photos is uploaded, its id is known: add it to the photos' + * pictures on Panoramax (the URLs are in its ext:image_url* tags, see uploadPendingPhotos). + * Best effort - the edit is already uploaded at this point, a failure here must not undo or + * repeat that, so it is only logged. */ + private suspend fun addElementIdToNewNodePhotos(edit: ElementEdit, updates: MapDataUpdates) { + val action = edit.action as? CreateNodeAction ?: return + val urls = action.tags.filterKeys { it.startsWith("ext:image_url") }.values + if (urls.isEmpty()) return + val newNodeId = updates.idUpdates.firstOrNull { it.elementType == ElementType.NODE }?.newElementId ?: return + val semantics = mapOf(PanoramaxApiClient.ELEMENT_ID to toElementIdSemantic(ElementType.NODE, newNodeId)) + for (url in urls) { + try { + imageUploader.addSemantics(url, semantics) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Log.w(TAG, "Failed to add the element id to the photo $url of edit ${edit.id}: ${e.message}") + } + } + } + + /** e.g. "way/123" */ + private fun toElementIdSemantic(type: ElementType, id: Long): String = "${type.name.lowercase()}/$id" + private fun hasPendingPhotos(edit: ElementEdit): Boolean = (edit.action is CreateNodeAction || edit.action is UpdateElementTagsAction) && featurePhotosController.get(edit.id).isNotEmpty() @@ -243,8 +297,10 @@ class ElementEditsUploader( companion object { private const val TAG = "ElementEditsUploader" + private const val STREET_IMAGES_TAG = "ext:street_images" + /** where the photo URL went before the switch to Panoramax */ private const val KARTAVIEW_URL_TAG = "ext:kartaview_url" - /** consecutive KartaView upload failures for the same edit before nagging the user about it */ + /** consecutive Panoramax upload failures for the same edit before nagging the user about it */ private const val STUCK_PHOTO_UPLOAD_ATTEMPTS = 3 } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEdit.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEdit.kt index 15c7a817d65..23981b0817d 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEdit.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEdit.kt @@ -4,6 +4,7 @@ import de.westnordost.streetcomplete.data.edithistory.Edit import de.westnordost.streetcomplete.data.edithistory.NoteEditKey import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmtracks.Trackpoint +import de.westnordost.streetcomplete.data.photos.CapturedPhoto /** Contains all necessary information to create/comment on an OSM note. */ data class NoteEdit( @@ -37,8 +38,19 @@ data class NoteEdit( /** attached GPS location history */ val track: List, - override var workspaceId: Int = 0 + override var workspaceId: Int = 0, + + /** compass bearing the camera faced for each of [imagePaths], index-aligned (null: unknown) */ + val imageBearings: List = emptyList(), + + /** when each of [imagePaths] was taken (epoch ms), index-aligned (null: unknown) */ + val imageCaptureTimes: List = emptyList(), ) : Edit { + /** the attached photos with their bearing and capture time */ + val photos: List get() = imagePaths.mapIndexed { i, path -> + CapturedPhoto(path, imageBearings.getOrNull(i), imageCaptureTimes.getOrNull(i)) + } + override val isUndoable: Boolean get() = !isSynced override val key: NoteEditKey get() = NoteEditKey(id) } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsController.kt index ccf18e29761..5d857c316b0 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsController.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsController.kt @@ -1,5 +1,6 @@ package de.westnordost.streetcomplete.data.osmnotes.edits +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox import de.westnordost.streetcomplete.data.osm.mapdata.ElementIdUpdate import de.westnordost.streetcomplete.data.osm.mapdata.LatLon @@ -28,7 +29,7 @@ class NoteEditsController( action: NoteEditAction, position: LatLon, text: String? = null, - imagePaths: List = emptyList(), + photos: List = emptyList(), track: List = emptyList(), ) { val edit = NoteEdit( @@ -37,11 +38,13 @@ class NoteEditsController( position, action, text, - imagePaths, + photos.map { it.path }, nowAsEpochMilliseconds(), false, - imagePaths.isNotEmpty(), - track, workspaceId + photos.isNotEmpty(), + track, workspaceId, + imageBearings = photos.map { it.bearing }, + imageCaptureTimes = photos.map { it.capturedAt }, ) lock.withLock { editsDB.add(edit) } onAddedEdit(edit) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDao.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDao.kt index 874ec9af8ef..25f5c549916 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDao.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsDao.kt @@ -7,6 +7,8 @@ import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.CREATED_TIMESTAMP import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.ID import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.IMAGES_NEED_ACTIVATION +import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.IMAGE_BEARINGS +import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.IMAGE_CAPTURE_TIMES import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.IMAGE_PATHS import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.IS_SYNCED import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsTable.Columns.LATITUDE @@ -137,6 +139,8 @@ class NoteEditsDao(private val db: Database, private val preferences: Preference TEXT to text, IMAGE_PATHS to Json.encodeToString(imagePaths), IMAGES_NEED_ACTIVATION to if (imagesNeedActivation) 1 else 0, + IMAGE_BEARINGS to Json.encodeToString(imageBearings), + IMAGE_CAPTURE_TIMES to Json.encodeToString(imageCaptureTimes), TRACK to Json.encodeToString(track), TYPE to action.name, WORKSPACE_ID to workspaceId @@ -153,6 +157,9 @@ class NoteEditsDao(private val db: Database, private val preferences: Preference getInt(IS_SYNCED) == 1, getInt(IMAGES_NEED_ACTIVATION) == 1, Json.decodeFromString(getString(TRACK)), - getInt(WORKSPACE_ID) + getInt(WORKSPACE_ID), + // absent on rows written before these columns existed + getStringOrNull(IMAGE_BEARINGS)?.let { Json.decodeFromString>(it) }.orEmpty(), + getStringOrNull(IMAGE_CAPTURE_TIMES)?.let { Json.decodeFromString>(it) }.orEmpty(), ) } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsTable.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsTable.kt index 364f86572d9..fa957704bc9 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsTable.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsTable.kt @@ -14,6 +14,12 @@ object NoteEditsTable { const val TEXT = "text" const val IMAGE_PATHS = "image_paths" const val IMAGES_NEED_ACTIVATION = "images_need_activation" + /** JSON list of nullable Floats, index-aligned with IMAGE_PATHS - the bearing of each + * photo, or null on rows written before this column existed */ + const val IMAGE_BEARINGS = "image_bearings" + /** JSON list of nullable Longs, index-aligned with IMAGE_PATHS - when each photo was taken + * (epoch ms), or null on rows written before this column existed */ + const val IMAGE_CAPTURE_TIMES = "image_capture_times" const val TRACK = "track" const val WORKSPACE_ID = "workspace_id" } @@ -29,6 +35,8 @@ object NoteEditsTable { ${Columns.TEXT} text, ${Columns.IMAGE_PATHS} text NOT NULL, ${Columns.IMAGES_NEED_ACTIVATION} int NOT NULL, + ${Columns.IMAGE_BEARINGS} text, + ${Columns.IMAGE_CAPTURE_TIMES} text, ${Columns.TRACK} text NOT NULL, ${Columns.TYPE} varchar(255), ${Columns.WORKSPACE_ID} int NOT NULL diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt index c1af1985004..84916261617 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt @@ -2,8 +2,10 @@ package de.westnordost.streetcomplete.data.osmnotes.edits import de.westnordost.streetcomplete.ApplicationConstants 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.panoramax.PanoramaxApiClient +import de.westnordost.streetcomplete.data.panoramax.PanoramaxException +import de.westnordost.streetcomplete.data.panoramax.toPanoramaxPhoto +import de.westnordost.streetcomplete.data.photos.CapturedPhoto import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.NoteController import de.westnordost.streetcomplete.data.osmnotes.NotesApiClient @@ -32,7 +34,7 @@ class NoteEditsUploader( private val userDataSource: UserDataSource, private val notesApi: NotesApiClient, private val tracksApi: TracksApiClient, - private val imageUploader: KartaViewApiClient, + private val imageUploader: PanoramaxApiClient, private val fileSystem: FileSystem, ) { var uploadedChangeListener: OnUploadedChangeListener? = null @@ -53,8 +55,8 @@ class NoteEditsUploader( } } private suspend fun uploadMissedImageActivations() { - // KartaView photos need no activation step (their URLs are embedded in the note text at - // upload) - just clear any flags still set, e.g. from before the switch to KartaView + // Panoramax photos need no activation step (their URLs are embedded in the note text at + // upload) - just clear any flags still set, e.g. from before the switch to KartaView/Panoramax while (true) { val edit = noteEditsController.getOldestNeedingImagesActivation() ?: break noteEditsController.markImagesActivated(edit.id) @@ -69,7 +71,7 @@ class NoteEditsUploader( * because otherwise an inconsistency in the data would occur. E.g. a note could * be uploaded twice */ withContext(scope.coroutineContext) { uploadEdit(edit) } - } catch (e: KartaViewException) { + } catch (e: PanoramaxException) { // photo upload failed (plain network failure) - leave the note edit unsynced for // the next sync attempt instead of aborting the whole upload (see class KDoc) Log.w(TAG, "Failed to upload photos, will retry on next sync: ${e.message}") @@ -80,7 +82,7 @@ class NoteEditsUploader( private suspend fun uploadEdit(edit: NoteEdit) { // try to upload the image and track if we have them - val imageText = uploadAndGetAttachedPhotosText(edit.imagePaths, edit.position) + val imageText = uploadAndGetAttachedPhotosText(edit.photos, edit.position, edit.createdTimestamp) val trackText = uploadAndGetAttachedTrackText(edit.track, edit.text) val text = edit.text.orEmpty() + imageText + trackText @@ -130,11 +132,15 @@ class NoteEditsUploader( } } - private suspend fun uploadAndGetAttachedPhotosText(imagePaths: List, position: LatLon): String { - if (imagePaths.isNotEmpty()) { - // note photos don't carry a capture bearing yet - see FeaturePhoto/ElementEditsUploader - // for the long-form equivalent this could follow if notes gain the same later - val urls = imageUploader.upload(imagePaths.map { it to 0f }, position) + private suspend fun uploadAndGetAttachedPhotosText( + photos: List, + position: LatLon, + /** when the note was made, i.e. right after its photos were taken - used for photos whose + * capture time is unknown (queued before it was recorded) */ + createdTimestamp: Long, + ): String { + if (photos.isNotEmpty()) { + val urls = imageUploader.upload(photos.map { it.toPanoramaxPhoto(createdTimestamp) }, position) if (urls.isNotEmpty()) { return "\n\nAttached photo:\n" + urls.joinToString("\n") } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClient.kt new file mode 100644 index 00000000000..3f3e06c4108 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/PanoramaxApiClient.kt @@ -0,0 +1,287 @@ +package de.westnordost.streetcomplete.data.panoramax + +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.panoramax.domain.model.CreateUploadSetRequest +import de.westnordost.streetcomplete.data.panoramax.domain.model.PatchPictureRequest +import de.westnordost.streetcomplete.data.panoramax.domain.model.SemanticTag +import de.westnordost.streetcomplete.data.panoramax.domain.model.UploadSet +import de.westnordost.streetcomplete.data.panoramax.domain.model.UploadSetFile +import de.westnordost.streetcomplete.data.photos.CapturedPhoto +import de.westnordost.streetcomplete.data.preferences.EnvironmentManager +import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds +import de.westnordost.streetcomplete.util.ktx.systemTimeNow +import de.westnordost.streetcomplete.util.logs.Log +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.request.bearerAuth +import io.ktor.client.request.forms.MultiPartFormDataContent +import io.ktor.client.request.forms.formData +import io.ktor.client.request.patch +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.client.statement.HttpResponse +import io.ktor.client.statement.bodyAsText +import io.ktor.http.ContentType +import io.ktor.http.Headers +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import io.ktor.http.contentType +import kotlinx.coroutines.CancellationException +import kotlinx.io.buffered +import kotlinx.io.files.FileSystem +import kotlinx.io.files.Path +import kotlinx.io.readByteArray +import kotlinx.serialization.json.Json +import kotlin.math.roundToInt +import kotlin.time.ExperimentalTime +import kotlin.time.Instant + +/** A photo to upload: the image file at [path], the compass [bearing] (0-359 clockwise from + * true north) the camera was facing at capture time - null if unknown, then none is sent - and + * semantic tags to attach to the picture, on top of the ones every picture gets (see + * [PanoramaxApiClient.upload]). */ +data class PanoramaxPhoto( + val path: String, + val bearing: Float? = null, + val semantics: Map = emptyMap(), + /** when the photo was taken, epoch milliseconds - the upload time if null */ + val captureTime: Long? = null, +) + +/** This photo as uploaded to Panoramax, with [semantics]. If when it was taken is unknown (e.g. + * queued before that was recorded), [fallbackCaptureTime] is used: the time its edit was made, + * right after the photo was taken, which is much closer than the upload time. */ +fun CapturedPhoto.toPanoramaxPhoto( + fallbackCaptureTime: Long, + semantics: Map = emptyMap(), +) = PanoramaxPhoto(path, bearing, semantics, capturedAt ?: fallbackCaptureTime) + +/** Uploads photos to our Panoramax instance, see + * https://docs.panoramax.fr/backend/api/api/ - via the upload set API: create an upload set, add + * the files to it, complete it. The server then turns it into a sequence (collection) and + * processes (blurs) the pictures in the background. */ +class PanoramaxApiClient( + private val fileSystem: FileSystem, + private val httpClient: HttpClient, + private val environmentManager: EnvironmentManager, + /** overrides the current environment's token (tests) */ + private val accessTokenOverride: String? = null, +) { + private val baseUrl: String get() = environmentManager.currentEnvironment.panoramaxBaseUrl + private val accessToken: String + get() = accessTokenOverride ?: environmentManager.currentEnvironment.panoramaxAccessToken + private val preferences get() = environmentManager.preferences + + /** Uploads [photos] into one new Panoramax upload set at [position], in list order, completes + * the upload set and returns the HD picture URL for each uploaded image, in the same order. + * Photos whose file doesn't exist are skipped. + * + * Every picture gets the semantic tag [WORKSPACE_ID] of the logged-in user, plus the photo's + * own [PanoramaxPhoto.semantics]. Nothing identifying the user (e.g. their email) is sent - + * the pictures are public. + * + * The URLs are known right away (they only depend on the picture id), but only answer with + * the image once the server has processed the picture, usually within seconds. + * + * @throws PanoramaxException naming the step that failed */ + suspend fun upload( + photos: List, + position: LatLon, + ): List { + val images = photos.mapNotNull { photo -> + val file = Path(photo.path) + if (fileSystem.exists(file)) { + photo to fileSystem.source(file).buffered().use { it.readByteArray() } + } else null + } + if (images.isEmpty()) return emptyList() + try { + val commonSemantics = getCommonSemantics() + val uploadSetId = createUploadSet(images.size) + val pictureIds = images.mapIndexed { index, (photo, image) -> + uploadPhoto( + uploadSetId, index + 1, image, position, photo.bearing, + photo.captureTime ?: nowAsEpochMilliseconds(), + commonSemantics + photo.semantics + ) + } + completeUploadSet(uploadSetId) + return pictureIds.map { pictureId -> "${baseUrl}api/pictures/$pictureId/hd.jpg" } + } catch (e: CancellationException) { + throw e + } catch (e: PanoramaxException) { + throw e + } + // Panoramax not answering at all (no connection, DNS failure - an + // UnresolvedAddressException, which isn't an IOException - timeout, reset) or answering + // with something unparseable is a photo upload failure like any error status: callers + // treat PanoramaxException as "leave the edit for the next sync" and count it towards the + // stuck-photo notice. Anything else would abort the whole upload run. + catch (e: Exception) { + throw PanoramaxException( + "Could not reach Panoramax. Image upload failed. Please try again later: $e", + isUnreachable = true + ) + } + } + + /** Adds [semantics] to the already uploaded picture at [pictureUrl] (as returned by + * [upload]). Returns false if [pictureUrl] isn't a picture of this Panoramax instance (e.g. a + * photo uploaded to KartaView before the switch) - nothing to do then. + * + * @throws PanoramaxException if it failed */ + suspend fun addSemantics(pictureUrl: String, semantics: Map): Boolean { + val pictureId = getPictureId(pictureUrl) ?: return false + val response = try { + httpClient.patch(baseUrl + "api/pictures/$pictureId") { + bearerAuth(accessToken) + contentType(ContentType.Application.Json) + setBody(PatchPictureRequest(semantics.toSemanticTags())) + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + throw PanoramaxException("Could not reach Panoramax: $e", isUnreachable = true) + } + if (response.status != HttpStatusCode.OK) { + throw PanoramaxException( + "Failed to add semantics to Panoramax picture $pictureId " + response.errorText() + ) + } + Log.d(TAG, "Added semantics ${semantics.keys} to picture $pictureId") + return true + } + + private fun getPictureId(pictureUrl: String): String? { + val prefix = baseUrl + "api/pictures/" + if (!pictureUrl.startsWith(prefix)) return null + return pictureUrl.removePrefix(prefix).substringBefore('/').takeIf { it.isNotEmpty() } + } + + private fun getCommonSemantics(): Map = buildMap { + preferences.workspaceId?.let { put(WORKSPACE_ID, it.toString()) } + } + + private suspend fun createUploadSet(fileCount: Int): String { + val response = httpClient.post(baseUrl + "api/upload_sets") { + bearerAuth(accessToken) + contentType(ContentType.Application.Json) + setBody(CreateUploadSetRequest( + title = "AVIV ScoutRoute ${systemTimeNow()}", + estimatedNbFiles = fileCount, + )) + } + if (response.status != HttpStatusCode.OK) { + throw PanoramaxException( + "Failed to create Panoramax upload set. Image upload failed. Please try again later " + + response.errorText() + ) + } + val uploadSet = response.body() + Log.d(TAG, "Upload set created: ${uploadSet.id}") + return uploadSet.id + } + + @OptIn(ExperimentalTime::class) + private suspend fun uploadPhoto( + uploadSetId: String, + index: Int, + image: ByteArray, + position: LatLon, + bearing: Float?, + /** epoch milliseconds */ + captureTime: Long, + semantics: Map, + ): String { + // the (rescaled) photos carry no position and time in their EXIF any more, so these are + // given as overrides - Panoramax rejects pictures without. The time is when it was taken, + // not now: photos are often uploaded much later, see ElementEditsUploader. Same for the + // heading - but only if known: 0 would claim the photo faces north + val heading = bearing?.roundToInt()?.mod(360) + val response = httpClient.post(baseUrl + "api/upload_sets/$uploadSetId/files") { + bearerAuth(accessToken) + setBody(MultiPartFormDataContent(formData { + append("override_latitude", position.latitude.toString()) + append("override_longitude", position.longitude.toString()) + // ISO 8601, e.g. 2026-11-10T08:30:00Z + append("override_capture_time", Instant.fromEpochMilliseconds(captureTime).toString()) + if (heading != null) { + append("override_Exif.GPSInfo.GPSImgDirection", "$heading/1") + append("override_Exif.GPSInfo.GPSImgDirectionRef", "T") + } + if (semantics.isNotEmpty()) { + // must be a JSON part, see the API docs + append( + "semantics", + json.encodeToString(semantics.toSemanticTags()), + Headers.build { append(HttpHeaders.ContentType, "application/json") } + ) + } + append("file", image, Headers.build { + append(HttpHeaders.ContentType, "image/jpeg") + append(HttpHeaders.ContentDisposition, "filename=\"asr-$index.jpg\"") + }) + })) + } + if (response.status != HttpStatusCode.Accepted) { + throw PanoramaxException( + "Failed to upload image to Panoramax. Please try again later " + response.errorText() + ) + } + val file = response.body() + file.rejected?.let { rejection -> + throw PanoramaxException( + "Panoramax rejected the image (${rejection.reason}): ${rejection.message}" + ) + } + val pictureId = file.pictureId ?: throw PanoramaxException( + "Panoramax did not return a picture id for the uploaded image" + ) + Log.d(TAG, "Image $index uploaded to upload set $uploadSetId as picture $pictureId") + return pictureId + } + + private suspend fun completeUploadSet(uploadSetId: String) { + val response = httpClient.post(baseUrl + "api/upload_sets/$uploadSetId/complete") { + bearerAuth(accessToken) + } + if (response.status != HttpStatusCode.OK) { + throw PanoramaxException( + "Failed to complete Panoramax upload set. Please try again later " + response.errorText() + ) + } + Log.d(TAG, "Upload set $uploadSetId completed") + } + + private fun Map.toSemanticTags() = map { (key, value) -> SemanticTag(key, value) } + + private suspend fun HttpResponse.errorText(): String = + "$status: ${runCatching { bodyAsText().take(500) }.getOrDefault("")}" + + companion object { + private const val TAG = "PanoramaxApiClient" + private val json = Json + + /** semantic tag keys put on the uploaded pictures */ + const val WORKSPACE_ID = "workspace_id" + /** the OSM element the photo is of, e.g. "way/123" */ + const val ELEMENT_ID = "element_id" + /** the long-form question the photo was taken for */ + const val QUESTION = "question" + /** the quest_id of that long-form question */ + const val QUEST_ID = "quest_id" + /** the feature preset name of the new feature the photo is of, e.g. "Intersection" */ + const val FEATURE_TYPE = "feature_type" + /** what the photo was taken for, see [SOURCE_NEW_FEATURE] */ + const val SOURCE = "source" + /** [SOURCE] of the photos of a new feature (Add Feature) */ + const val SOURCE_NEW_FEATURE = "new_feature" + } +} + +/** @property isUnreachable Panoramax 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 PanoramaxException( + message: String, + val isUnreachable: Boolean = false, +) : RuntimeException(message) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/domain/model/UploadSet.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/domain/model/UploadSet.kt new file mode 100644 index 00000000000..65f3a57ec23 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/panoramax/domain/model/UploadSet.kt @@ -0,0 +1,64 @@ +package de.westnordost.streetcomplete.data.panoramax.domain.model + +import kotlinx.serialization.EncodeDefault +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** Body of POST /api/upload_sets. + * + * The flags below must always be sent: kotlinx.serialization leaves out properties that have + * their default value (encodeDefaults is off), and the server then applies its own defaults - + * which made it drop every photo after the first of an edit as a duplicate, see noDeduplication */ +@OptIn(ExperimentalSerializationApi::class) +@Serializable +data class CreateUploadSetRequest( + val title: String, + @SerialName("estimated_nb_files") + val estimatedNbFiles: Int, + /** all photos of one edit go into one sequence */ + @SerialName("no_split") + @EncodeDefault + val noSplit: Boolean = true, + /** the photos of one edit are all taken at the same position (edit.position), so with the + * instance's default (1 m / 60°) the server rejects all but one as duplicates ("capture_duplicate") + * when completing the upload set - after it had already answered the upload with a picture id, + * so the URL of such a photo leads nowhere (404) */ + @SerialName("no_deduplication") + @EncodeDefault + val noDeduplication: Boolean = true, +) + +/** Answer of POST /api/upload_sets and POST /api/upload_sets/{id}/complete - only what we use */ +@Serializable +data class UploadSet( + val id: String, +) + +/** Answer of POST /api/upload_sets/{id}/files - only what we use */ +@Serializable +data class UploadSetFile( + @SerialName("picture_id") + val pictureId: String? = null, + val rejected: FileRejection? = null, +) + +@Serializable +data class FileRejection( + val reason: String, + val message: String? = null, +) + +/** A Panoramax semantic tag (key/value) on a picture - see + * https://docs.panoramax.fr/backend/install/deep_dive/semantics/ */ +@Serializable +data class SemanticTag( + val key: String, + val value: String, +) + +/** Body of PATCH /api/pictures/{id} - only what we use */ +@Serializable +data class PatchPictureRequest( + val semantics: List, +) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/photos/CapturedPhoto.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/photos/CapturedPhoto.kt new file mode 100644 index 00000000000..dbadb58982c --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/photos/CapturedPhoto.kt @@ -0,0 +1,14 @@ +package de.westnordost.streetcomplete.data.photos + +import kotlinx.serialization.Serializable + +/** A photo taken with the camera and saved locally at [path], not uploaded yet. Each photo has its + * own [bearing], the compass direction the camera faced (0-359, clockwise from true north, null + * if unknown), and [capturedAt], when it was taken (epoch milliseconds, null if unknown). Both are + * sent to Panoramax when it is uploaded. */ +@Serializable +data class CapturedPhoto( + val path: String, + val bearing: Float? = null, + val capturedAt: Long? = null, +) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt index c44b20ee442..e380d43a82d 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt @@ -1,10 +1,14 @@ package de.westnordost.streetcomplete.data.preferences +import de.westnordost.streetcomplete.BuildConfig + enum class Environment( val workspaceBaseUrl: String, val tdeiBaseUrl: String, val osmUrl: String, val tdeiWebUrl: String, + /** Panoramax instance photos are uploaded to, with trailing slash */ + val panoramaxBaseUrl: String = PANORAMAX_DEV_BASE_URL, val appUpdateVersionCheckUrl: String = APP_UPDATE_VERSION_CHECKER_URL, val firebaseUpdateUrl: String = FIREBASE_UPDATE_URL, ) { @@ -12,22 +16,36 @@ enum class Environment( "https://api.workspaces-stage.sidewalks.washington.edu/api/v1/workspaces", "https://portal-api-stage.tdei.us/api/v1", "https://osm-workspaces-proxy.azurewebsites.net/stage/api/0.6/", - "https://portal-stage.tdei.us" + "https://portal-stage.tdei.us", + panoramaxBaseUrl = PANORAMAX_STAGE_BASE_URL, ), DEV( "https://api.workspaces-dev.sidewalks.washington.edu/api/v1/workspaces", "https://portal-api-dev.tdei.us/api/v1", "https://osm-workspaces-proxy.azurewebsites.net/dev/api/0.6/", - "https://portal-dev.tdei.us" + "https://portal-dev.tdei.us", + panoramaxBaseUrl = PANORAMAX_DEV_BASE_URL, ), PROD( "https://api.workspaces.sidewalks.washington.edu/api/v1/workspaces", "https://portal-api.tdei.us/api/v1", "https://osm-workspaces-proxy.azurewebsites.net/prod/api/0.6/", - "https://portal.tdei.us" + "https://portal.tdei.us", + panoramaxBaseUrl = PANORAMAX_PROD_BASE_URL, ); + /** Bearer token for [panoramaxBaseUrl] - each instance has its own (build secrets, see + * secrets.properties.example) */ + val panoramaxAccessToken: String get() = when (this) { + DEV -> BuildConfig.PANORAMAX_DEV_ACCESS_TOKEN + STAGE -> BuildConfig.PANORAMAX_STAGE_ACCESS_TOKEN + PROD -> BuildConfig.PANORAMAX_PROD_ACCESS_TOKEN + } + companion object { + const val PANORAMAX_DEV_URL = PANORAMAX_DEV_BASE_URL + const val PANORAMAX_STAGE_URL = PANORAMAX_STAGE_BASE_URL + const val PANORAMAX_PROD_URL = PANORAMAX_PROD_BASE_URL const val APP_UPDATE_VERSION_CHECKER_URL = "https://raw.githubusercontent.com/TaskarCenterAtUW/asr-config/refs/heads/main/force-update/app-force-update.json" const val FIREBASE_UPDATE_URL = @@ -35,6 +53,14 @@ enum class Environment( } } +// outside of Environment's companion: an enum entry's arguments can't use that yet +private const val PANORAMAX_DEV_BASE_URL = + "https://panoramax-dev-faaadghgcfgwcchk.westus2-01.azurewebsites.net/" +private const val PANORAMAX_STAGE_BASE_URL = + "https://panoramax-stage-eubgh6hxgmf8ase5.westus2-01.azurewebsites.net/" +private const val PANORAMAX_PROD_BASE_URL = + "https://panoramax-h8a0fha2ebcpgkdn.westus2-01.azurewebsites.net/" + class EnvironmentManager(val preferences: Preferences) { var currentEnvironment: Environment = Environment.valueOf(preferences.environment) get() { diff --git a/secrets.properties.example b/secrets.properties.example index 91b949ea791..a659dc28a0a 100644 --- a/secrets.properties.example +++ b/secrets.properties.example @@ -6,3 +6,14 @@ # Obtain one from the KartaView account that owns the app's photo sequences. # Env var alternative: KARTAVIEW_ACCESS_TOKEN kartaViewAccessToken= + +# Bearer token (JWT) for photo uploads to the dev Panoramax instance (DEV environment). +# Generate one for the upload account with POST /api/users/me/tokens on the instance. +# Env var alternative: PANORAMAX_DEV_ACCESS_TOKEN +panoramaxDevAccessToken= +# Same for the stage Panoramax instance (STAGE environment). +# Env var alternative: PANORAMAX_STAGE_ACCESS_TOKEN +panoramaxStageAccessToken= +# Same for the prod Panoramax instance (PROD environment). +# Env var alternative: PANORAMAX_PROD_ACCESS_TOKEN +panoramaxProdAccessToken=