diff --git a/CHANGELOG.md b/CHANGELOG.md index cebf849..557ed4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # CHANGELOG -## [2.6.53] - 05.09.2026 +## [2.6.54] - 05.09.2026 **New** diff --git a/app/build.gradle b/app/build.gradle index f12a310..c69a447 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -21,7 +21,7 @@ apply from: "$projectDir/gradle/libs-task.gradle" apply from: "$projectDir/gradle/changelog-task.gradle" -def appVersionCode = 53 +def appVersionCode = 54 def appVersionName = "2.6.${appVersionCode}" def gitCommitHashProvider = providers.exec { diff --git a/app/src/androidTest/java/com/pasich/mynotes/db/RoomSyncStoreTest.java b/app/src/androidTest/java/com/pasich/mynotes/db/RoomSyncStoreTest.java index fe8ee88..2132dcf 100644 --- a/app/src/androidTest/java/com/pasich/mynotes/db/RoomSyncStoreTest.java +++ b/app/src/androidTest/java/com/pasich/mynotes/db/RoomSyncStoreTest.java @@ -796,10 +796,21 @@ public void resolveConflict_dropsARowWhoseWinnerTheRecordHasAlreadyLeftBehind() // pre-selected winner is a version the record has moved past. Offered anyway, it was // applied on a tap; here the winner is a deletion. int noteId = seedNote("loser", "body", null); + // The alternative is the note exactly as this store serialises it, so the record's + // content genuinely equals the loser: a rule that only drops rows equal to neither + // offer keeps this one and applies the deletion. + SyncRecord local = onlyNote(store.buildSnapshot().requireSnapshot()); db.syncMetadataDao().touch(SyncMetadata.RECORD_TYPE_NOTE, noteId, 5_000L); com.pasich.mynotes.data.database.entities.SyncConflictEntity row = noteConflictRow( "11111111-1111-4111-8111-111111111111", "deleted-winner", 3_000L, 2_000L); + row.loserVersionId = local.getCanonicalPayloadHash(); + row.loserJson = + "{\"type\":\"note\",\"id\":\"11111111-1111-4111-8111-111111111111\"," + + "\"updatedAt\":\"1970-01-01T00:00:02Z\",\"deletedAt\":null," + + "\"payload\":" + + new com.google.gson.Gson().toJson(local.getPayload()) + + "}"; row.winnerJson = "{\"type\":\"note\",\"id\":\"11111111-1111-4111-8111-111111111111\"," + "\"updatedAt\":\"1970-01-01T00:00:03Z\",\"deletedAt\":\"1970-01-01T00:00:03Z\"," @@ -866,6 +877,52 @@ public void applySnapshot_doesNotRecordThePreferencesBaseUntilTheCommitSucceeded .isEqualTo(remote.getCanonicalPayloadHash()); } + @Test + public void recoveryRecordsTheBaseOfAnApplyThatDiedBeforeItsBookkeeping() throws Exception { + PreferencesAdapter adapter = new PreferencesAdapter(); + new RoomSyncStore(context, db, adapter.helper).readState(); + com.google.gson.Gson gson = new com.google.gson.Gson(); + SyncRecord remote = + SyncRecord.live( + SyncRecord.Type.PREFERENCES, + "00000000-0000-4000-8000-000000000000", + java.time.Instant.ofEpochMilli(2_000L), + gson.toJsonTree(preferencesWithTheme(3)).getAsJsonObject()); + // What the apply transaction leaves when the process dies right after it: the record + // timestamped at the remote version, its payload journaled against the live digest, the + // base not yet recorded. + String stagedJson = gson.toJson(preferencesWithTheme(3)); + String liveDigest = + sha256(gson.toJson(preferencesWithTheme(1)).getBytes(StandardCharsets.UTF_8)); + db.syncMetadataDao().setVersion(SyncMetadata.RECORD_TYPE_PREFERENCES, 0, 2_000L, null); + db.syncPendingPreferencesDao() + .upsert( + new com.pasich.mynotes.data.database.entities.SyncPendingPreferencesEntity( + 1, + stagedJson, + sha256(stagedJson.getBytes(StandardCharsets.UTF_8)), + liveDigest, + 2_000L, + false, + 0L, + "")); + + // A fresh store seeds, which is where recovery replays the journal. + new RoomSyncStore(context, db, adapter.helper).readState(); + + assertThat(adapter.committed.get().getThemeValue()).isEqualTo(3); + assertThat(db.syncPendingPreferencesDao().get()).isNull(); + // Replayed but left with the base naming the version before it, the next merge saw a + // local edit that nobody made. + assertThat( + db.syncMetadataDao() + .getByStableId( + SyncMetadata.RECORD_TYPE_PREFERENCES, + "00000000-0000-4000-8000-000000000000") + .syncedVersionId) + .isEqualTo(remote.getCanonicalPayloadHash()); + } + /** A stored note conflict whose winner is titled after its version id. */ private com.pasich.mynotes.data.database.entities.SyncConflictEntity noteConflictRow( String stableId, String winnerVersionId, long winnerUpdatedAt, long loserUpdatedAt) { diff --git a/app/src/main/java/com/pasich/mynotes/data/sync/RoomSyncStore.java b/app/src/main/java/com/pasich/mynotes/data/sync/RoomSyncStore.java index 64f4458..9954bdc 100644 --- a/app/src/main/java/com/pasich/mynotes/data/sync/RoomSyncStore.java +++ b/app/src/main/java/com/pasich/mynotes/data/sync/RoomSyncStore.java @@ -643,6 +643,7 @@ private void recoverPendingPreferences() throws IOException { // stale baseline, calls this a local edit and touches the record to now, which // lets an unchanged copy outrank a genuine edit made on another device. preferences.edit().putString(PREFERENCES_HASH, target).commit(); + recordRecoveredPreferencesBase(pending, backup); finishJournal(pending); return; case DISCARD_STALE: @@ -655,10 +656,43 @@ private void recoverPendingPreferences() throws IOException { default: // Replay was decided because the live values still match the baseline. commitPendingPreferences(backup, target, pending.baselineHash); + recordRecoveredPreferencesBase(pending, backup); finishJournal(pending); } } + /** + * Records, on recovery, the base the interrupted apply would have recorded after its commit. + * + *

The apply transaction stamps the record with the remote version's time and journals its + * payload; the base is recorded only once the commit has succeeded. Dying in between left the + * live settings at the remote version with the base still naming the one before it, so the next + * merge saw a local edit nobody made. The version id is rebuilt from the journal: the same + * timestamp and the same payload, serialised by the same class, hash to the same id. A journal + * written by a conflict resolution is left alone; a resolution re-times the record without + * touching its base. + */ + private void recordRecoveredPreferencesBase( + @NonNull SyncPendingPreferencesEntity pending, @NonNull PreferencesBackup backup) { + if (pending.conflictId > 0 || pending.recordUpdatedAt <= 0L) return; + SyncMetadataEntity metadata = + database.syncMetadataDao() + .getByStableId(SyncMetadata.RECORD_TYPE_PREFERENCES, PREFERENCES_STABLE_ID); + if (metadata == null || metadata.updatedAt != pending.recordUpdatedAt) return; + String versionId = + SyncRecord.live( + SyncRecord.Type.PREFERENCES, + PREFERENCES_STABLE_ID, + Instant.ofEpochMilli(pending.recordUpdatedAt), + gson.toJsonTree(backup).getAsJsonObject()) + .getCanonicalPayloadHash(); + database.runInTransaction( + () -> + database.syncMetadataDao() + .setSyncedVersion( + metadata.recordType, metadata.localId, versionId)); + } + /** Clears the journal, completing the conflict bookkeeping when it names one. */ private void finishJournal(@NonNull SyncPendingPreferencesEntity pending) { if (pending.conflictId > 0) { diff --git a/app/src/test/java/com/pasich/mynotes/data/sync/TwoPeerConvergenceTest.java b/app/src/test/java/com/pasich/mynotes/data/sync/TwoPeerConvergenceTest.java index 47ac7b3..ef998e0 100644 --- a/app/src/test/java/com/pasich/mynotes/data/sync/TwoPeerConvergenceTest.java +++ b/app/src/test/java/com/pasich/mynotes/data/sync/TwoPeerConvergenceTest.java @@ -97,6 +97,13 @@ public void aRecordRevivedElsewhereLeavesNoPhantomDeletionOnAPeerThatNeverHeldIt assertThat(a.store.pendingConflicts()).isEmpty(); assertThat(a.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B"); + // The tap that did the damage: accepting whatever the dialog pre-selects. With the row + // gone there is nothing to accept; with it present, this deleted the task on A. + clock.advance(); + a.store.resolveAllKeepingWinner(clock.instant()); + assertThat(a.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B"); + clock.advance(); + assertThat(a.sync().getConflictCount()).isEqualTo(0); clock.advance(); assertThat(c.sync().getConflictCount()).isEqualTo(0); assertThat(c.store.titleOf(type, RECORD_ID)).isEqualTo("TaskX-B"); @@ -220,6 +227,7 @@ static final class PeerStore implements SyncStore { private final Map records = new LinkedHashMap<>(); private final Map bases = new LinkedHashMap<>(); private final List conflicts = new ArrayList<>(); + private final Map built = new LinkedHashMap<>(); private SyncState state = SyncState.idle(); PeerStore(Clock clock) { @@ -276,74 +284,136 @@ List pendingConflicts() { /** The dialog loop: every open conflict, one after another, keeping the live version. */ void resolveAllKeepingLive(Instant resolvedAt) { for (ConflictRow row : new ArrayList<>(pendingConflicts())) { - resolveKeepingLive(row, resolvedAt); + resolve( + row, + row.conflict.getWinner().isTombstone() + ? SyncResolution.KEEP_ALTERNATIVE + : SyncResolution.KEEP_WINNER, + resolvedAt); + } + } + + /** The dialog loop with the pre-selected choice accepted every time, deletions included. */ + void resolveAllKeepingWinner(Instant resolvedAt) { + for (ConflictRow row : new ArrayList<>(pendingConflicts())) { + resolve(row, SyncResolution.KEEP_WINNER, resolvedAt); } } /** - * What resolveConflict does: drop a row whose record's content has moved on, otherwise - * re-time the chosen version and mark the row settled. + * What resolveConflict does: drop a row the record has moved past, otherwise apply the + * chosen version — a deletion included — re-timed and with the base left alone, and mark + * the row settled. */ - void resolveKeepingLive(ConflictRow row, Instant resolvedAt) { - SyncRecord chosen = - row.conflict.getWinner().isTombstone() - ? row.conflict.getLoser() - : row.conflict.getWinner(); - String key = key(chosen); - SyncRecord current = records.get(key); - if (current != null - && current.getUpdatedAt() - .isAfter( - row.conflict - .getWinner() - .getUpdatedAt() - .isAfter(row.conflict.getLoser().getUpdatedAt()) - ? row.conflict.getWinner().getUpdatedAt() - : row.conflict.getLoser().getUpdatedAt()) - && !contentDigest(current).equals(contentDigest(row.conflict.getWinner()))) { + void resolve(ConflictRow row, SyncResolution resolution, Instant resolvedAt) { + if (isSuperseded(row.conflict)) { conflicts.remove(row); return; } + SyncRecord chosen = + resolution == SyncResolution.KEEP_WINNER + ? row.conflict.getWinner() + : row.conflict.getLoser(); + String key = key(chosen); + SyncRecord current = records.get(key); Instant updatedAt = current != null && !resolvedAt.isAfter(current.getUpdatedAt()) ? current.getUpdatedAt().plusMillis(1) : resolvedAt; - records.put( - key, - SyncRecord.live( - chosen.getType(), chosen.getId(), updatedAt, chosen.getPayload())); + if (chosen.isTombstone()) { + // A record never held here has nothing to delete; the store leaves it alone. + if (current != null) { + records.put( + key, + SyncRecord.tombstone( + chosen.getType(), chosen.getId(), updatedAt, updatedAt)); + } + } else { + records.put( + key, + SyncRecord.live( + chosen.getType(), chosen.getId(), updatedAt, chosen.getPayload())); + } row.resolved = true; } + /** + * RoomSyncStore.isSuperseded: a record this peer does not hold is never superseded; one + * that is newer than both offers is, unless it still equals the row's winner. + */ + private boolean isSuperseded(SyncMergeResult.Conflict conflict) { + SyncRecord current = records.get(conflict.getType() + ":" + conflict.getId()); + if (current == null) return false; + Instant newest = + conflict.getWinner().getUpdatedAt().isAfter(conflict.getLoser().getUpdatedAt()) + ? conflict.getWinner().getUpdatedAt() + : conflict.getLoser().getUpdatedAt(); + if (!current.getUpdatedAt().isAfter(newest)) return false; + return !contentDigest(current).equals(contentDigest(conflict.getWinner())); + } + + /** + * RoomSyncStore.retireConflictsSupersededBy: open rows whose winner is not this version. + */ + private void retireConflictsSupersededBy(SyncRecord applied) { + String key = key(applied); + conflicts.removeIf( + row -> + !row.resolved + && key.equals( + row.conflict.getType() + ":" + row.conflict.getId()) + && !row.conflict + .getWinnerVersionId() + .equals(applied.getCanonicalPayloadHash())); + } + @Override public SyncSnapshot readSnapshot() { List withBases = new ArrayList<>(); + built.clear(); for (Map.Entry entry : records.entrySet()) { + built.put(entry.getKey(), entry.getValue().getCanonicalPayloadHash()); withBases.add(entry.getValue().withBaseVersion(bases.get(entry.getKey()))); } return new SyncSnapshot(withBases); } + /** + * The four paths of RoomSyncStore.applySnapshot, kept distinct on purpose: which of them + * retire open rows is exactly what the phantom-deletion scenario depends on. + */ @Override public void applySnapshot(SyncSnapshot snapshot, List incoming) { Set skipped = new LinkedHashSet<>(); for (SyncRecord record : snapshot.getRecords()) { String key = key(record); SyncRecord current = records.get(key); - if (current != null && current.getUpdatedAt().isAfter(record.getUpdatedAt())) { + if (current == null && !record.isTombstone()) { + // Insert: a record this peer never held. + records.put(key, record.withBaseVersion(null)); + bases.put(key, record.getCanonicalPayloadHash()); + retireConflictsSupersededBy(record); + continue; + } + if (current == null) { + // A deletion of a record never held: the store creates no metadata for it, + // so nothing is stored and nothing is published later; only open rows follow. + retireConflictsSupersededBy(record); + continue; + } + if (current.getUpdatedAt().isAfter(record.getUpdatedAt())) { + // Edited during the sync: left alone, except that this peer's own published + // version becomes the base the edit is measured against. skipped.add(key); + if (record.getCanonicalPayloadHash().equals(built.get(key))) { + bases.put(key, record.getCanonicalPayloadHash()); + } continue; } + // Tombstone or live update of a held record. records.put(key, record.withBaseVersion(null)); bases.put(key, record.getCanonicalPayloadHash()); - conflicts.removeIf( - row -> - !row.resolved - && key.equals( - row.conflict.getType() + ":" + row.conflict.getId()) - && !row.conflict - .getWinnerVersionId() - .equals(record.getCanonicalPayloadHash())); + retireConflictsSupersededBy(record); } for (SyncMergeResult.Conflict conflict : incoming) { String key = conflict.getType() + ":" + conflict.getId();