diff --git a/core/network/build.gradle.kts b/core/network/build.gradle.kts index 51a148a..50edc21 100644 --- a/core/network/build.gradle.kts +++ b/core/network/build.gradle.kts @@ -23,4 +23,5 @@ dependencies { implementation(libs.kotlinx.coroutines.android) testImplementation(libs.junit) + testImplementation(libs.okhttp.mockwebserver) } diff --git a/core/network/src/main/java/com/whtis/memosly/core/network/api/MemoApi.kt b/core/network/src/main/java/com/whtis/memosly/core/network/api/MemoApi.kt index 20d1929..bd5e0d6 100644 --- a/core/network/src/main/java/com/whtis/memosly/core/network/api/MemoApi.kt +++ b/core/network/src/main/java/com/whtis/memosly/core/network/api/MemoApi.kt @@ -79,8 +79,9 @@ interface MemoApi { @Body request: SetMemoRelationsRequest, ) - // v0.24: link resources to memo - @POST("api/v1/memos/{id}/resources") + // v0.24: link resources to memo. PATCH, not POST — the proto binds this to + // `patch: "/api/v1/{name=memos/*}/resources"`, and POST answers 501. + @PATCH("api/v1/memos/{id}/resources") suspend fun setMemoResources( @Path("id") id: String, @Body request: SetMemoResourcesRequest, diff --git a/core/network/src/test/java/com/whtis/memosly/core/network/api/MemoApiAttachmentEndpointTest.kt b/core/network/src/test/java/com/whtis/memosly/core/network/api/MemoApiAttachmentEndpointTest.kt new file mode 100644 index 0000000..d8b4b82 --- /dev/null +++ b/core/network/src/test/java/com/whtis/memosly/core/network/api/MemoApiAttachmentEndpointTest.kt @@ -0,0 +1,90 @@ +package com.whtis.memosly.core.network.api + +import com.squareup.moshi.Moshi +import com.squareup.moshi.kotlin.reflect.KotlinJsonAdapterFactory +import com.whtis.memosly.core.network.dto.ResourceRef +import com.whtis.memosly.core.network.dto.SetMemoAttachmentsRequest +import com.whtis.memosly.core.network.dto.SetMemoResourcesRequest +import kotlinx.coroutines.runBlocking +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import retrofit2.Retrofit +import retrofit2.converter.moshi.MoshiConverterFactory + +/** + * Pins the HTTP method and path used to link uploaded files to a memo. + * + * Both calls are bound to `patch:` in the Memos proto. Sending POST to the v0.24 + * endpoint answers 501 and the upload stays orphaned — server-side it shows up as an + * unused resource, and the memo renders without its images on every client. That + * shipped, because a wrong verb fails quietly and looks exactly like a save that worked. + */ +class MemoApiAttachmentEndpointTest { + + private lateinit var server: MockWebServer + private lateinit var api: MemoApi + + @Before + fun setUp() { + server = MockWebServer() + server.start() + api = Retrofit.Builder() + .baseUrl(server.url("/")) + .addConverterFactory( + MoshiConverterFactory.create( + Moshi.Builder().add(KotlinJsonAdapterFactory()).build() + ) + ) + .build() + .create(MemoApi::class.java) + } + + @After + fun tearDown() { + server.shutdown() + } + + @Test + fun `v024 setMemoResources patches the resources endpoint`() { + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + + runBlocking { + api.setMemoResources( + "4fLRfWCwya4YA2CvnBmWZY", + SetMemoResourcesRequest(listOf(ResourceRef("resources/R6dURNySEeeTUbaCz43Ft2"))), + ) + } + + val request = server.takeRequest() + assertEquals("PATCH", request.method) + assertEquals("/api/v1/memos/4fLRfWCwya4YA2CvnBmWZY/resources", request.path) + assertEquals( + """{"resources":[{"name":"resources/R6dURNySEeeTUbaCz43Ft2"}]}""", + request.body.readUtf8(), + ) + } + + @Test + fun `v025 and up setMemoAttachments patches the attachments endpoint`() { + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + + runBlocking { + api.setMemoAttachments( + "bsveqyE5j3H6YXvWBXuzBP", + SetMemoAttachmentsRequest(listOf(ResourceRef("attachments/Bc8eUUhk7jCGLgSkKrArYW"))), + ) + } + + val request = server.takeRequest() + assertEquals("PATCH", request.method) + assertEquals("/api/v1/memos/bsveqyE5j3H6YXvWBXuzBP/attachments", request.path) + assertEquals( + """{"attachments":[{"name":"attachments/Bc8eUUhk7jCGLgSkKrArYW"}]}""", + request.body.readUtf8(), + ) + } +} diff --git a/core/ui/src/main/res/values-zh/strings.xml b/core/ui/src/main/res/values-zh/strings.xml index 33ff921..fcf1838 100644 --- a/core/ui/src/main/res/values-zh/strings.xml +++ b/core/ui/src/main/res/values-zh/strings.xml @@ -147,6 +147,7 @@ 已跳过 %1$s — 超过 %2$d MB 无法读取 %1$s 仅添加了前 %1$d 个文件 + 备忘已保存，但附件关联失败。点击保存可重试。 下载 diff --git a/core/ui/src/main/res/values/strings.xml b/core/ui/src/main/res/values/strings.xml index d4275d8..6565d99 100644 --- a/core/ui/src/main/res/values/strings.xml +++ b/core/ui/src/main/res/values/strings.xml @@ -148,6 +148,7 @@ Skipped %1$s — larger than %2$d MB Couldn\'t read %1$s Only the first %1$d files were added + The memo was saved, but its attachments could not be linked. Tap save to retry. Download diff --git a/feature/memo/src/main/java/com/whtis/memosly/feature/memo/MemoEditorViewModel.kt b/feature/memo/src/main/java/com/whtis/memosly/feature/memo/MemoEditorViewModel.kt index d1486e4..1db0822 100644 --- a/feature/memo/src/main/java/com/whtis/memosly/feature/memo/MemoEditorViewModel.kt +++ b/feature/memo/src/main/java/com/whtis/memosly/feature/memo/MemoEditorViewModel.kt @@ -71,7 +71,11 @@ class MemoEditorViewModel @Inject constructor( val serverUrl: String get() = tokenManager.serverUrl.value ?: "" - private val memoId: String = savedStateHandle["memoId"] ?: "" + /** + * Blank for a new memo. Once a create succeeds this holds the new id, so a retry after a + * failed attachment link updates that memo instead of creating a second one. + */ + private var memoId: String = savedStateHandle["memoId"] ?: "" private val sharedText: String = savedStateHandle["sharedText"] ?: "" private val hasSharedMedia: Boolean = savedStateHandle["sharedMedia"] ?: false @@ -236,10 +240,12 @@ class MemoEditorViewModel @Inject constructor( _uiState.value = _uiState.value.copy(isSaving = true, error = null) try { val visibilityStr = _uiState.value.visibility.name - val memo = if (memoId.isNotBlank()) { + val isUpdate = memoId.isNotBlank() + val memo = if (isUpdate) { memoRepository.updateMemo(memoId, content, visibilityStr) } else { memoRepository.createMemo(content, visibilityStr).also { + memoId = it.name.substringAfterLast("/") analyticsHelper.logEvent("memo_create", mapOf( "has_tags" to if (content.extractTags().isNotEmpty()) "true" else "false", )) @@ -254,7 +260,7 @@ class MemoEditorViewModel @Inject constructor( // them now just orphans them. val pendingResources = _uiState.value.pendingResources val existingResources = _uiState.value.existingResources - val existingChanged = memoId.isNotBlank() && existingResources.size != memo.resources.size + val existingChanged = isUpdate && existingResources.size != memo.resources.size if (pendingResources.isNotEmpty() || existingChanged) { try { val existingNames = existingResources.map { it.name } @@ -262,7 +268,19 @@ class MemoEditorViewModel @Inject constructor( val allNames = (existingNames + newNames).distinct() memoRepository.setMemoResources(memo.name, allNames) } catch (e: Exception) { + // Swallowing this is what let the v0.24 verb bug ship: the memo saved, + // the upload vanished, and nothing said so. The memo exists either way, + // so stay in the editor and let the user retry — memoId now points at + // it, so retrying updates rather than posting a duplicate. android.util.Log.e("MemoEditor", "setMemoResources failed: ${e.message}", e) + _uiState.update { + it.copy( + isSaving = false, + isEditMode = true, + error = context.getString(UiR.string.attachment_link_failed), + ) + } + return@launch } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 46cca08..ce36083 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -122,6 +122,7 @@ kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx- # Testing junit = { group = "junit", name = "junit", version.ref = "junit" } +okhttp-mockwebserver = { group = "com.squareup.okhttp3", name = "mockwebserver", version.ref = "okhttp" } [plugins] android-application = { id = "com.android.application", version.ref = "agp" }