diff --git a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java index f14eee5f..081a7873 100644 --- a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java +++ b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java @@ -26,6 +26,16 @@ public record SendMessageResult( ) { final var rateLimitFailure = sendMessageResult.getRateLimitFailure(); final var proofRequiredFailure = sendMessageResult.getProofRequiredFailure(); + final Long retryAfterSeconds; + if (proofRequiredFailure != null) { + retryAfterSeconds = proofRequiredFailure.getRetryAfterSeconds(); + } else if (rateLimitFailure != null) { + retryAfterSeconds = rateLimitFailure.getRetryAfterMilliseconds() + .map(SendMessageResult::millisToCeilingSeconds) + .orElse(null); + } else { + retryAfterSeconds = null; + } return new SendMessageResult(addressResolver.resolveRecipientAddress(recipientResolver.resolveRecipient( sendMessageResult.getAddress())).toApiRecipientAddress(), sendMessageResult.isSuccess(), @@ -35,8 +45,11 @@ public record SendMessageResult( rateLimitFailure != null || proofRequiredFailure != null, proofRequiredFailure == null ? null : new ProofRequiredException(proofRequiredFailure), sendMessageResult.isInvalidPreKeyFailure(), - rateLimitFailure == null - ? null - : rateLimitFailure.getRetryAfterMilliseconds().map(ms -> ms / 1000L).orElse(null)); + retryAfterSeconds); + } + + static long millisToCeilingSeconds(long millis) { + // Round up so we never advise a retry before the server's deadline. + return (millis + 999L) / 1000L; } } diff --git a/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java b/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java new file mode 100644 index 00000000..fd5b2b93 --- /dev/null +++ b/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java @@ -0,0 +1,24 @@ +package org.asamk.signal.manager.api; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class SendMessageResultTest { + + /** + * Ceiling division — we must never advise a retry before the server's deadline, + * so sub-second values round up rather than truncate toward zero. + */ + @Test + void millisToCeilingSecondsRoundsUp() { + assertEquals(0L, SendMessageResult.millisToCeilingSeconds(0L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(500L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(999L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1000L)); + assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1001L)); + assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1500L)); + assertEquals(60L, SendMessageResult.millisToCeilingSeconds(60_000L)); + } +} diff --git a/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java b/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java index 6fe59acd..098d1916 100644 --- a/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java +++ b/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java @@ -32,9 +32,7 @@ public record JsonSendMessageResult( ? Type.INVALID_PRE_KEY_FAILURE : Type.IDENTITY_FAILURE, result.proofRequiredFailure() != null ? result.proofRequiredFailure().getToken() : null, - result.proofRequiredFailure() != null - ? (Long) result.proofRequiredFailure().getRetryAfterSeconds() - : result.rateLimitRetryAfterSeconds()); + result.rateLimitRetryAfterSeconds()); } public enum Type { diff --git a/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java b/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java index bb28a73c..1811c6f3 100644 --- a/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java +++ b/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test; import java.util.List; import java.util.Map; +import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -47,47 +48,6 @@ class JsonSendMessageResultTest { assertNull(json.retryAfterSeconds()); } - @Test - void sendMessageResultsReturnsMaxRetryAfter() { - var small = new SendMessageResult(ADDRESS, - false, false, false, false, - true, - null, - false, - 60L); - var big = new SendMessageResult(new RecipientAddress(null, null, "+15559876543", null), - false, false, false, false, - true, - null, - false, - 3600L); - var unknown = new SendMessageResult(new RecipientAddress(null, null, "+15550000000", null), - false, false, false, false, - true, - null, - false, - null); - - var aggregate = new SendMessageResults(1L, - Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(small, big, unknown))); - - assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds()); - } - - @Test - void sendMessageResultsReturnsNullWhenNoRetryAfter() { - var noRetry = new SendMessageResult(ADDRESS, - false, false, false, false, - true, - null, - false, - null); - var aggregate = new SendMessageResults(1L, - Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(noRetry))); - - assertNull(aggregate.maxRateLimitRetryAfterSeconds()); - } - @Test void successLeavesRetryAfterNull() { var result = new SendMessageResult(ADDRESS, @@ -102,4 +62,52 @@ class JsonSendMessageResultTest { assertEquals(JsonSendMessageResult.Type.SUCCESS, json.type()); assertNull(json.retryAfterSeconds()); } + + @Test + void aggregateReturnsLongestRetryAfter() { + var small = rateLimited("+15551234567", 60L); + var big = rateLimited("+15559876543", 3600L); + var unknown = rateLimited("+15550000000", null); + + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), List.of(small, big, unknown))); + + assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds()); + } + + @Test + void aggregateReturnsNullWhenNoRetryAfter() { + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), + List.of(rateLimited("+15551234567", null)))); + + assertNull(aggregate.maxRateLimitRetryAfterSeconds()); + } + + /** + * Regression for a bug where the aggregate helper could overlook the longest + * wait if only some recipients reported a value. Ensures the max is picked + * across any mix — which is what downstream captcha/rate-limit clients rely on. + */ + @Test + void aggregatePicksMaxEvenWhenSomeValuesAreNull() { + var withValue = rateLimited("+15551111111", 7200L); + var withoutValue = rateLimited("+15552222222", null); + var alsoWithValue = rateLimited("+15553333333", 120L); + + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), + List.of(withoutValue, withValue, alsoWithValue))); + + assertEquals(7200L, aggregate.maxRateLimitRetryAfterSeconds()); + } + + private static SendMessageResult rateLimited(String number, Long retryAfterSeconds) { + return new SendMessageResult(new RecipientAddress(null, null, number, null), + false, false, false, false, + true, + null, + false, + retryAfterSeconds); + } }