From ff689b577bcab4d3249df96dec4fa941b009c0eb Mon Sep 17 00:00:00 2001 From: AsamK Date: Thu, 10 Sep 2026 19:25:55 +0200 Subject: [PATCH] Fix storage sync edge cases --- .../signal/manager/helper/StorageHelper.java | 2 +- .../syncStorage/StorageSyncLoopDetector.java | 16 +++++++++++-- .../manager/helper/StorageHelperTest.java | 3 +++ .../StorageSyncLoopDetectorTest.java | 23 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/org/asamk/signal/manager/helper/StorageHelper.java b/lib/src/main/java/org/asamk/signal/manager/helper/StorageHelper.java index 2828a7b3..8438cdba 100644 --- a/lib/src/main/java/org/asamk/signal/manager/helper/StorageHelper.java +++ b/lib/src/main/java/org/asamk/signal/manager/helper/StorageHelper.java @@ -414,7 +414,7 @@ public class StorageHelper { final WriteOperationResult writeOperation, final Set identityConflictsPendingRepair ) { - return !writeOperation.inserts().isEmpty() && writeOperation.inserts() + return writeOperation.deletes().isEmpty() && !writeOperation.inserts().isEmpty() && writeOperation.inserts() .stream() .allMatch(record -> identityConflictsPendingRepair.contains(record.getId())); } diff --git a/lib/src/main/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetector.java b/lib/src/main/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetector.java index 6f3a1e3e..b9b86fcd 100644 --- a/lib/src/main/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetector.java +++ b/lib/src/main/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetector.java @@ -20,6 +20,8 @@ public final class StorageSyncLoopDetector { private final LeakyBucket rateBucket = new LeakyBucket(100, Duration.ofMinutes(10).toMillis(), new InMemoryBucketState()); + private boolean lastAttemptChargedContent; + private boolean lastAttemptChargedRate; public StorageSyncLoopDetector(final BooleanSupplier isMultiDevice) { this.isMultiDevice = isMultiDevice; @@ -39,6 +41,8 @@ public final class StorageSyncLoopDetector { final boolean isRetry, final long now ) { + lastAttemptChargedContent = false; + lastAttemptChargedRate = false; if (!isMultiDevice.getAsBoolean() || isRetry) { return Decision.Allowed.INSTANCE; } @@ -56,9 +60,11 @@ public final class StorageSyncLoopDetector { if (chargeContent) { contentBucket.use(now); + lastAttemptChargedContent = true; } if (fetchedRemoteManifest) { rateBucket.use(now); + lastAttemptChargedRate = true; } if (fingerprint != null) { remember(fingerprint); @@ -72,8 +78,14 @@ public final class StorageSyncLoopDetector { } synchronized void onWriteFailed(final long now) { - contentBucket.refund(now); - rateBucket.refund(now); + if (lastAttemptChargedContent) { + contentBucket.refund(now); + lastAttemptChargedContent = false; + } + if (lastAttemptChargedRate) { + rateBucket.refund(now); + lastAttemptChargedRate = false; + } } public synchronized void onConverged() { diff --git a/lib/src/test/java/org/asamk/signal/manager/helper/StorageHelperTest.java b/lib/src/test/java/org/asamk/signal/manager/helper/StorageHelperTest.java index f9a93c8c..01a79ef8 100644 --- a/lib/src/test/java/org/asamk/signal/manager/helper/StorageHelperTest.java +++ b/lib/src/test/java/org/asamk/signal/manager/helper/StorageHelperTest.java @@ -38,6 +38,9 @@ class StorageHelperTest { assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of(pendingRecord, otherRecord)), Set.of(pendingId))); assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(write(List.of()), Set.of(pendingId))); + assertFalse(StorageHelper.containsOnlyIdentityConflictsPendingRepair(new WriteOperationResult(null, + List.of(pendingRecord), + List.of(new byte[]{3})), Set.of(pendingId))); } private static SignalStorageRecord record(final StorageId id) { diff --git a/lib/src/test/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetectorTest.java b/lib/src/test/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetectorTest.java index f9d4f98c..4e436ce7 100644 --- a/lib/src/test/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetectorTest.java +++ b/lib/src/test/java/org/asamk/signal/manager/syncStorage/StorageSyncLoopDetectorTest.java @@ -65,6 +65,29 @@ class StorageSyncLoopDetectorTest { } } + @Test + void failedWriteRefundsOnlyBucketsChargedByLatestAttempt() { + final var contentDetector = new StorageSyncLoopDetector(() -> true); + final var repeatedWrite = writeWithInsert(1); + for (var index = 0; index < 4; index++) { + assertAllowed(contentDetector.onWriteAttempt(repeatedWrite, true, false, NOW)); + } + final var deleteOnlyWrite = new WriteOperationResult(null, List.of(), List.of(new byte[]{1})); + assertAllowed(contentDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW)); + contentDetector.onWriteFailed(NOW); + assertEquals(new StorageSyncLoopDetector.Decision.Denied(StorageSyncLoopDetector.Cause.REPEATED_PAYLOAD, 3), + contentDetector.onWriteAttempt(repeatedWrite, true, false, NOW)); + + final var rateDetector = new StorageSyncLoopDetector(() -> true); + for (var index = 0; index < 100; index++) { + assertAllowed(rateDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW)); + } + assertAllowed(rateDetector.onWriteAttempt(deleteOnlyWrite, false, false, NOW)); + rateDetector.onWriteFailed(NOW); + assertEquals(new StorageSyncLoopDetector.Decision.Denied(StorageSyncLoopDetector.Cause.WRITE_RATE, 100), + rateDetector.onWriteAttempt(deleteOnlyWrite, true, false, NOW)); + } + @Test void rateBucketLimitsDeleteOnlyWrites() { final var detector = new StorageSyncLoopDetector(() -> true);