fix(android): Requests cancel → MutationQueue offline fallback
Audit v2 silent breakage / standing rule violation (feedback_offline_first_for_server_writes): RequestsRepository.cancel was direct REST with no queue fallback. Tap cancel offline and the user's intent vanishes. Now mirrors the unflag / appendTrack / requestCreate pattern: REST on the happy path, MutationKind.REQUEST_CANCEL enqueued on IOException. Replayer's existing AuthStore.sessionCookie trigger drains it on the next signed-in transition. Repository signature changed from `suspend fun cancel(id): RequestRef` to `Pair<CancelOutcome, RequestRef?>` so callers can distinguish synced vs queued (RequestsViewModel ignores the distinction for now; optimistic removal already reflects the user's intent and the post-replay refresh surfaces the canonical row). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+20
@@ -20,6 +20,7 @@ object MutationKind {
|
|||||||
const val PLAYLIST_APPEND: String = "playlist_append"
|
const val PLAYLIST_APPEND: String = "playlist_append"
|
||||||
const val QUARANTINE_FLAG: String = "quarantine_flag"
|
const val QUARANTINE_FLAG: String = "quarantine_flag"
|
||||||
const val PLAY_OFFLINE: String = "play_offline"
|
const val PLAY_OFFLINE: String = "play_offline"
|
||||||
|
const val REQUEST_CANCEL: String = "request_cancel"
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -133,6 +134,16 @@ class MutationQueue @Inject constructor(
|
|||||||
payload = json.encodeToString(PlayOfflinePayload.serializer(), payload),
|
payload = json.encodeToString(PlayOfflinePayload.serializer(), payload),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
suspend fun enqueueRequestCancel(requestId: String): Long = dao.insert(
|
||||||
|
CachedMutationEntity(
|
||||||
|
kind = MutationKind.REQUEST_CANCEL,
|
||||||
|
payload = json.encodeToString(
|
||||||
|
RequestCancelPayload.serializer(),
|
||||||
|
RequestCancelPayload(requestId),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -183,3 +194,12 @@ data class PlayOfflinePayload(
|
|||||||
val durationPlayedMs: Long,
|
val durationPlayedMs: Long,
|
||||||
val source: String? = null,
|
val source: String? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Persisted payload for `MutationKind.REQUEST_CANCEL` — the
|
||||||
|
* `POST /api/requests/{id}/cancel` call lost during a connectivity
|
||||||
|
* hiccup. Server is idempotent on re-cancel (already-cancelled
|
||||||
|
* request stays cancelled).
|
||||||
|
*/
|
||||||
|
@Serializable
|
||||||
|
data class RequestCancelPayload(val requestId: String)
|
||||||
|
|||||||
+16
@@ -7,6 +7,7 @@ import com.fabledsword.minstrel.api.endpoints.FlagRequest
|
|||||||
import com.fabledsword.minstrel.api.endpoints.LikesApi
|
import com.fabledsword.minstrel.api.endpoints.LikesApi
|
||||||
import com.fabledsword.minstrel.api.endpoints.PlaylistsApi
|
import com.fabledsword.minstrel.api.endpoints.PlaylistsApi
|
||||||
import com.fabledsword.minstrel.api.endpoints.QuarantineApi
|
import com.fabledsword.minstrel.api.endpoints.QuarantineApi
|
||||||
|
import com.fabledsword.minstrel.api.endpoints.RequestsApi
|
||||||
import com.fabledsword.minstrel.models.wire.PlayOfflineRequest
|
import com.fabledsword.minstrel.models.wire.PlayOfflineRequest
|
||||||
import com.fabledsword.minstrel.auth.AuthStore
|
import com.fabledsword.minstrel.auth.AuthStore
|
||||||
import com.fabledsword.minstrel.cache.db.dao.CachedMutationDao
|
import com.fabledsword.minstrel.cache.db.dao.CachedMutationDao
|
||||||
@@ -64,6 +65,7 @@ class MutationReplayer @Inject constructor(
|
|||||||
private val quarantineApi: QuarantineApi = retrofit.create()
|
private val quarantineApi: QuarantineApi = retrofit.create()
|
||||||
private val playlistsApi: PlaylistsApi = retrofit.create()
|
private val playlistsApi: PlaylistsApi = retrofit.create()
|
||||||
private val eventsApi: EventsApi = retrofit.create()
|
private val eventsApi: EventsApi = retrofit.create()
|
||||||
|
private val requestsApi: RequestsApi = retrofit.create()
|
||||||
|
|
||||||
private val mutex = Mutex()
|
private val mutex = Mutex()
|
||||||
|
|
||||||
@@ -113,6 +115,7 @@ class MutationReplayer @Inject constructor(
|
|||||||
MutationKind.PLAYLIST_APPEND -> dispatchPlaylistAppend(row.payload)
|
MutationKind.PLAYLIST_APPEND -> dispatchPlaylistAppend(row.payload)
|
||||||
MutationKind.QUARANTINE_FLAG -> dispatchQuarantineFlag(row.payload)
|
MutationKind.QUARANTINE_FLAG -> dispatchQuarantineFlag(row.payload)
|
||||||
MutationKind.PLAY_OFFLINE -> dispatchPlayOffline(row.payload)
|
MutationKind.PLAY_OFFLINE -> dispatchPlayOffline(row.payload)
|
||||||
|
MutationKind.REQUEST_CANCEL -> dispatchRequestCancel(row.payload)
|
||||||
else -> {
|
else -> {
|
||||||
// Unknown kind — drop the row by claiming success so a
|
// Unknown kind — drop the row by claiming success so a
|
||||||
// stale schema entry can't wedge the queue forever.
|
// stale schema entry can't wedge the queue forever.
|
||||||
@@ -242,4 +245,17 @@ class MutationReplayer @Inject constructor(
|
|||||||
false
|
false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private suspend fun dispatchRequestCancel(payload: String): Boolean {
|
||||||
|
val decoded = json.decodeFromString(RequestCancelPayload.serializer(), payload)
|
||||||
|
return try {
|
||||||
|
requestsApi.cancel(decoded.requestId)
|
||||||
|
true
|
||||||
|
} catch (
|
||||||
|
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
|
||||||
|
) {
|
||||||
|
// Intentional swallow: row stays queued; next drain pass retries.
|
||||||
|
false
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+27
-9
@@ -1,25 +1,33 @@
|
|||||||
package com.fabledsword.minstrel.requests.data
|
package com.fabledsword.minstrel.requests.data
|
||||||
|
|
||||||
import com.fabledsword.minstrel.api.endpoints.RequestsApi
|
import com.fabledsword.minstrel.api.endpoints.RequestsApi
|
||||||
|
import com.fabledsword.minstrel.cache.mutations.MutationQueue
|
||||||
import com.fabledsword.minstrel.models.RequestRef
|
import com.fabledsword.minstrel.models.RequestRef
|
||||||
import com.fabledsword.minstrel.models.RequestStatus
|
import com.fabledsword.minstrel.models.RequestStatus
|
||||||
import com.fabledsword.minstrel.models.wire.RequestWire
|
import com.fabledsword.minstrel.models.wire.RequestWire
|
||||||
import retrofit2.Retrofit
|
import retrofit2.Retrofit
|
||||||
import retrofit2.create
|
import retrofit2.create
|
||||||
|
import java.io.IOException
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
import javax.inject.Singleton
|
import javax.inject.Singleton
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Outcome of a [RequestsRepository.cancel] call. Mirrors the
|
||||||
|
* [com.fabledsword.minstrel.likes.data.UnflagOutcome] / etc. shape.
|
||||||
|
*/
|
||||||
|
enum class CancelOutcome { Synced, Queued }
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Read-through accessor for the caller's own Lidarr requests +
|
* Read-through accessor for the caller's own Lidarr requests +
|
||||||
* cancel-while-pending write. No Room caching for v1 — same rationale
|
* cancel-while-pending write. No Room caching for v1 — same rationale
|
||||||
* as HistoryRepository; the offline-snapshot mirror lands with
|
* as HistoryRepository; the offline-snapshot mirror lands with a
|
||||||
* Phase 13. Cancel + the create path (DiscoverRepository) both fire
|
* later slice. Cancel is offline-first: direct REST on the happy
|
||||||
* direct REST today; the create path enqueues on failure via the
|
* path, MutationQueue fallback on transport failure so the user's
|
||||||
* MutationQueue, cancel does not (it's a no-op if the request is
|
* intent persists across connectivity loss.
|
||||||
* already gone).
|
|
||||||
*/
|
*/
|
||||||
@Singleton
|
@Singleton
|
||||||
class RequestsRepository @Inject constructor(
|
class RequestsRepository @Inject constructor(
|
||||||
|
private val mutationQueue: MutationQueue,
|
||||||
retrofit: Retrofit,
|
retrofit: Retrofit,
|
||||||
) {
|
) {
|
||||||
private val api: RequestsApi = retrofit.create()
|
private val api: RequestsApi = retrofit.create()
|
||||||
@@ -28,11 +36,21 @@ class RequestsRepository @Inject constructor(
|
|||||||
api.listMine().map { it.toDomain() }
|
api.listMine().map { it.toDomain() }
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Cancels a pending request. Server returns the cancelled row
|
* Cancels a pending request. On 2xx, returns the canonical
|
||||||
* (now status=rejected with cancel notes) so callers can refresh
|
* cancelled row from the server (status=rejected). On transport
|
||||||
* local state from the return value without a list refetch.
|
* failure, enqueues a REQUEST_CANCEL mutation for later replay
|
||||||
|
* and returns null — the caller's optimistic UI removal already
|
||||||
|
* reflects the user's intent, and the next listMine() refetch
|
||||||
|
* (post-replay) will surface the canonical row.
|
||||||
*/
|
*/
|
||||||
suspend fun cancel(id: String): RequestRef = api.cancel(id).toDomain()
|
suspend fun cancel(id: String): Pair<CancelOutcome, RequestRef?> = try {
|
||||||
|
CancelOutcome.Synced to api.cancel(id).toDomain()
|
||||||
|
} catch (
|
||||||
|
@Suppress("SwallowedException") _: IOException,
|
||||||
|
) {
|
||||||
|
mutationQueue.enqueueRequestCancel(id)
|
||||||
|
CancelOutcome.Queued to null
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Mappers (internal — wire stays out of UI) ──
|
// ── Mappers (internal — wire stays out of UI) ──
|
||||||
|
|||||||
+19
-12
@@ -76,20 +76,27 @@ class RequestsViewModel @Inject constructor(
|
|||||||
}
|
}
|
||||||
viewModelScope.launch {
|
viewModelScope.launch {
|
||||||
try {
|
try {
|
||||||
val updated = repository.cancel(id)
|
val (_, updated) = repository.cancel(id)
|
||||||
internal.update { state ->
|
if (updated != null) {
|
||||||
if (state !is RequestsUiState.Success) {
|
internal.update { state ->
|
||||||
// Mid-refresh — let the refresh result win, but
|
if (state !is RequestsUiState.Success) {
|
||||||
// splice the cancelled row in so it shows the new
|
// Mid-refresh — let the refresh result win, but
|
||||||
// status if the user re-pulls.
|
// splice the cancelled row in so it shows the new
|
||||||
RequestsUiState.Success(listOf(updated))
|
// status if the user re-pulls.
|
||||||
} else {
|
RequestsUiState.Success(listOf(updated))
|
||||||
// Re-insert the cancelled row in its old position
|
} else {
|
||||||
// would require remembering the prior ordering;
|
// Re-insert the cancelled row in its old position
|
||||||
// simpler to refetch in the background.
|
// would require remembering the prior ordering;
|
||||||
state
|
// simpler to refetch in the background.
|
||||||
|
state
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Refresh fetches the canonical list whether the cancel
|
||||||
|
// went through live (Synced) or got enqueued (Queued).
|
||||||
|
// The Queued case shows the optimistic removal until the
|
||||||
|
// replayer drains; the next list fetch surfaces the
|
||||||
|
// server's canonical view.
|
||||||
refresh()
|
refresh()
|
||||||
} catch (
|
} catch (
|
||||||
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
|
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
|
||||||
|
|||||||
Reference in New Issue
Block a user