diff --git a/app/src/main/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSync.kt b/app/src/main/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSync.kt index 798f2e8a..87b9d1c3 100644 --- a/app/src/main/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSync.kt +++ b/app/src/main/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSync.kt @@ -59,28 +59,20 @@ class LocalNetworkGuardedSync( } /** - * Settling a conflict sends the answer to the server, so it is - * guarded too. + * Keeping this device's position sends it to the server, so it is + * guarded; taking the server's is not. * - * Both of these are reached from the reader's own tap, and both - * already answer `Failed(Offline)` when there is no network at all. - * A blocked one is the same thing said differently: the write is - * not going to arrive, and a timeout the reader watches is the - * worst way to tell them so. The status line is left alone, as it - * is for a preview. + * The asymmetry is the two operations', not this class's. Taking + * the server's position applies an answer an earlier run already + * brought back and wrote down, and touches nothing but this + * phone's database — which is why it works on a plane, and must go + * on working behind a blocked network for the same reason. Keeping + * this device's dials, and already answers `Failed(Offline)` when + * there is no network at all; a blocked one is that said + * differently, and a timeout the reader sits and watches is the + * worst way to say it. The status line is left alone, as it is for + * a preview: one book being settled is not a run. */ - override suspend fun takeRemotePosition( - bookUrl: String, - atRevision: Long, - peerId: String?, - expectedAccountKey: String?, - ): ResolveOutcome = - if (blocked()) { - ResolveOutcome.Failed(SyncFailure.LocalNetworkBlocked) - } else { - delegate.takeRemotePosition(bookUrl, atRevision, peerId, expectedAccountKey) - } - override suspend fun keepLocalPosition(bookUrl: String, peerId: String?): ResolveOutcome = if (blocked()) { ResolveOutcome.Failed(SyncFailure.LocalNetworkBlocked) diff --git a/app/src/test/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSyncTest.kt b/app/src/test/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSyncTest.kt index 7075eeaf..3f28c59d 100644 --- a/app/src/test/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSyncTest.kt +++ b/app/src/test/kotlin/com/chmouel/liseur/data/remote/LocalNetworkGuardedSyncTest.kt @@ -79,13 +79,15 @@ class LocalNetworkGuardedSyncTest { } /** - * Settling a conflict writes the answer to the server, so it is - * refused too rather than left to sit on a timeout the reader is - * watching. The status line is not touched: this is one book being - * settled, not a run. + * Keeping this device's position writes it to the server, so it is + * refused rather than left to sit on a timeout the reader is + * watching. Taking the server's applies an answer an earlier run + * already wrote down and touches nothing but this phone, so it goes + * through — the same reason it works on a plane. Neither touches + * the status line: this is one book being settled, not a run. */ @Test - fun `settling a conflict is refused rather than dialled`() = runTest { + fun `sending an answer is refused but accepting one still works`() = runTest { val peer = FakePeer(PeerPositionSync.CATALOG) val reporting = SyncReporting() val guarded = guard(peer, reporting, "http://192.168.1.20:8083") @@ -94,8 +96,8 @@ class LocalNetworkGuardedSyncTest { val taken = guarded.takeRemotePosition("book", 1L, null, null) assertEquals(ResolveOutcome.Failed(SyncFailure.LocalNetworkBlocked), kept) - assertEquals(ResolveOutcome.Failed(SyncFailure.LocalNetworkBlocked), taken) - assertEquals(0, peer.resolutions) + assertEquals(ResolveOutcome.Done, taken) + assertEquals(1, peer.resolutions) assertEquals(PositionSyncStatus.Idle, reporting.statusOf(PeerPositionSync.CATALOG)) } diff --git a/docs/adr/0028-local-network-permission.md b/docs/adr/0028-local-network-permission.md index 7df24ce5..5c7c80f0 100644 --- a/docs/adr/0028-local-network-permission.md +++ b/docs/adr/0028-local-network-permission.md @@ -214,12 +214,16 @@ the connected account does not use. Answering it there would also have meant a `when (kind)` over which credential each kind of catalog needs, at exactly the kind of call site the router exists to keep them out of. -Settling a conflict is guarded as well, because `keepLocalPosition` and -`takeRemotePosition` both write to the server. They already answer -"failed, offline" when there is no network at all; a blocked one is the -same thing said differently, and a timeout the reader sits and watches -is the worst possible way to say it. Like a preview, they leave the -account's status line alone: one book being settled is not a run. +Half of settling a conflict is guarded as well. `keepLocalPosition` +sends this device's answer to the server, and already says "failed, +offline" when there is no network at all; a blocked one is that said +differently, and a timeout the reader sits and watches is the worst +possible way to say it. `takeRemotePosition` is not guarded, because it +sends nothing: it applies a position an earlier run already brought back +and wrote down, and touches nothing but this phone's database. That is +why it works on a plane, and it must go on working behind a blocked +network for exactly the same reason. Neither touches the account's +status line: one book being settled is not a run. Downloads and uploads are left alone. They fail in front of a reader who can be told, and the notice on the connected card is what explains them.