return explicit Response rather than Void from async controllers with no expected body content

This commit is contained in:
Jonathan Klabunde Tomer 2023-11-14 21:57:25 -08:00 committed by GitHub
parent d4ef2adf0a
commit 7764185c57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 42 additions and 16 deletions

View File

@ -274,9 +274,9 @@ public class AccountController {
) )
@ApiResponse(responseCode = "204", description = "Username successfully deleted.", useReturnTypeSchema = true) @ApiResponse(responseCode = "204", description = "Username successfully deleted.", useReturnTypeSchema = true)
@ApiResponse(responseCode = "401", description = "Account authentication check failed.") @ApiResponse(responseCode = "401", description = "Account authentication check failed.")
public CompletableFuture<Void> deleteUsernameHash(@Auth final AuthenticatedAccount auth) { public CompletableFuture<Response> deleteUsernameHash(@Auth final AuthenticatedAccount auth) {
return accounts.clearUsernameHash(auth.getAccount()) return accounts.clearUsernameHash(auth.getAccount())
.thenRun(Util.NOOP); .thenApply(Util.ASYNC_EMPTY_RESPONSE);
} }
@PUT @PUT
@ -518,8 +518,8 @@ public class AccountController {
@DELETE @DELETE
@Path("/me") @Path("/me")
public CompletableFuture<Void> deleteAccount(@Auth DisabledPermittedAuthenticatedAccount auth) throws InterruptedException { public CompletableFuture<Response> deleteAccount(@Auth DisabledPermittedAuthenticatedAccount auth) throws InterruptedException {
return accounts.delete(auth.getAccount(), AccountsManager.DeletionReason.USER_REQUEST); return accounts.delete(auth.getAccount(), AccountsManager.DeletionReason.USER_REQUEST).thenApply(Util.ASYNC_EMPTY_RESPONSE);
} }
private void clearUsernameLink(final Account account) { private void clearUsernameLink(final Account account) {

View File

@ -121,7 +121,7 @@ public class KeysController {
@ApiResponse(responseCode = "401", description = "Account authentication check failed.") @ApiResponse(responseCode = "401", description = "Account authentication check failed.")
@ApiResponse(responseCode = "403", description = "Attempt to change identity key from a non-primary device.") @ApiResponse(responseCode = "403", description = "Attempt to change identity key from a non-primary device.")
@ApiResponse(responseCode = "422", description = "Invalid request format.") @ApiResponse(responseCode = "422", description = "Invalid request format.")
public CompletableFuture<Void> setKeys(@Auth final DisabledPermittedAuthenticatedAccount disabledPermittedAuth, public CompletableFuture<Response> setKeys(@Auth final DisabledPermittedAuthenticatedAccount disabledPermittedAuth,
@RequestBody @NotNull @Valid final PreKeyState preKeys, @RequestBody @NotNull @Valid final PreKeyState preKeys,
@Parameter(allowEmptyValue=true) @Parameter(allowEmptyValue=true)
@ -189,7 +189,8 @@ public class KeysController {
} }
return keys.store(getIdentifier(account, identityType), device.getId(), return keys.store(getIdentifier(account, identityType), device.getId(),
preKeys.getPreKeys(), preKeys.getPqPreKeys(), preKeys.getSignedPreKey(), preKeys.getPqLastResortPreKey()); preKeys.getPreKeys(), preKeys.getPqPreKeys(), preKeys.getSignedPreKey(), preKeys.getPqLastResortPreKey())
.thenApply(Util.ASYNC_EMPTY_RESPONSE);
} }
@GET @GET
@ -299,7 +300,7 @@ public class KeysController {
@ApiResponse(responseCode = "200", description = "Indicates that new prekey was successfully stored.") @ApiResponse(responseCode = "200", description = "Indicates that new prekey was successfully stored.")
@ApiResponse(responseCode = "401", description = "Account authentication check failed.") @ApiResponse(responseCode = "401", description = "Account authentication check failed.")
@ApiResponse(responseCode = "422", description = "Invalid request format.") @ApiResponse(responseCode = "422", description = "Invalid request format.")
public CompletableFuture<Void> setSignedKey(@Auth final AuthenticatedAccount auth, public CompletableFuture<Response> setSignedKey(@Auth final AuthenticatedAccount auth,
@Valid final ECSignedPreKey signedPreKey, @Valid final ECSignedPreKey signedPreKey,
@QueryParam("identity") final Optional<String> identityType) { @QueryParam("identity") final Optional<String> identityType) {
@ -314,7 +315,8 @@ public class KeysController {
}); });
return keys.storeEcSignedPreKeys(getIdentifier(auth.getAccount(), identityType), return keys.storeEcSignedPreKeys(getIdentifier(auth.getAccount(), identityType),
Map.of(device.getId(), signedPreKey)); Map.of(device.getId(), signedPreKey))
.thenApply(Util.ASYNC_EMPTY_RESPONSE);
} }
private static boolean usePhoneNumberIdentity(final Optional<String> identityType) { private static boolean usePhoneNumberIdentity(final Optional<String> identityType) {

View File

@ -581,7 +581,7 @@ public class MessageController {
@Timed @Timed
@DELETE @DELETE
@Path("/uuid/{uuid}") @Path("/uuid/{uuid}")
public CompletableFuture<Void> removePendingMessage(@Auth AuthenticatedAccount auth, @PathParam("uuid") UUID uuid) { public CompletableFuture<Response> removePendingMessage(@Auth AuthenticatedAccount auth, @PathParam("uuid") UUID uuid) {
return messagesManager.delete( return messagesManager.delete(
auth.getAccount().getUuid(), auth.getAccount().getUuid(),
auth.getAuthenticatedDevice().getId(), auth.getAuthenticatedDevice().getId(),
@ -603,7 +603,8 @@ public class MessageController {
} }
} }
}); });
}); })
.thenApply(Util.ASYNC_EMPTY_RESPONSE);
} }
@Timed @Timed

View File

@ -16,8 +16,12 @@ import java.util.Locale;
import java.util.Locale.LanguageRange; import java.util.Locale.LanguageRange;
import java.util.Optional; import java.util.Optional;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
public class Util { public class Util {
@ -28,6 +32,12 @@ public class Util {
public static final Runnable NOOP = () -> {}; public static final Runnable NOOP = () -> {};
// Use `CompletableFuture#thenApply(ASYNC_EMPTY_RESPONSE) to convert futures to
// CompletableFuture<Response> instead of using NOOP to convert them to CompletableFuture<Void>
// for jersey controllers; https://github.com/eclipse-ee4j/jersey/issues/3901 causes controllers
// returning Void futures to behave differently than synchronous controllers returning void
public static final Function<Object, Response> ASYNC_EMPTY_RESPONSE = ignored -> Response.noContent().build();
/** /**
* Checks that the given number is a valid, E164-normalized phone number. * Checks that the given number is a valid, E164-normalized phone number.
* *

View File

@ -38,6 +38,7 @@ import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream; import java.util.stream.Stream;
import javax.ws.rs.client.Entity; import javax.ws.rs.client.Entity;
import javax.ws.rs.client.Invocation; import javax.ws.rs.client.Invocation;
@ -90,6 +91,7 @@ import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableExceptio
import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
import org.whispersystems.textsecuregcm.util.MockUtils; import org.whispersystems.textsecuregcm.util.MockUtils;
import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.SystemMapper;
import org.whispersystems.textsecuregcm.util.UsernameHashZkProofVerifier; import org.whispersystems.textsecuregcm.util.UsernameHashZkProofVerifier;
@ -737,7 +739,7 @@ class AccountControllerTest {
@Test @Test
void testDeleteUsername() { void testDeleteUsername() {
when(accountsManager.clearUsernameHash(any())) when(accountsManager.clearUsernameHash(any()))
.thenAnswer(invocation -> CompletableFuture.completedFuture(invocation.getArgument(0))); .thenAnswer(invocation -> CompletableFutureTestUtil.almostCompletedFuture(invocation.getArgument(0)));
Response response = Response response =
resources.getJerseyTest() resources.getJerseyTest()
@ -746,6 +748,7 @@ class AccountControllerTest {
.header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
.delete(); .delete();
assertThat(response.readEntity(String.class)).isEqualTo("");
assertThat(response.getStatus()).isEqualTo(204); assertThat(response.getStatus()).isEqualTo(204);
verify(accountsManager).clearUsernameHash(AuthHelper.VALID_ACCOUNT); verify(accountsManager).clearUsernameHash(AuthHelper.VALID_ACCOUNT);
} }
@ -828,6 +831,8 @@ class AccountControllerTest {
@Test @Test
void testDeleteAccount() { void testDeleteAccount() {
when(accountsManager.delete(any(), any())).thenReturn(CompletableFutureTestUtil.almostCompletedFuture(null));
Response response = Response response =
resources.getJerseyTest() resources.getJerseyTest()
.target("/v1/accounts/me") .target("/v1/accounts/me")

View File

@ -74,6 +74,7 @@ import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import org.whispersystems.textsecuregcm.tests.util.KeysHelper; import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
import org.whispersystems.textsecuregcm.util.ByteArrayAdapter; import org.whispersystems.textsecuregcm.util.ByteArrayAdapter;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
@ExtendWith(DropwizardExtensionsSupport.class) @ExtendWith(DropwizardExtensionsSupport.class)
class KeysControllerTest { class KeysControllerTest {
@ -235,9 +236,9 @@ class KeysControllerTest {
when(rateLimiters.getPreKeysLimiter()).thenReturn(rateLimiter); when(rateLimiters.getPreKeysLimiter()).thenReturn(rateLimiter);
when(KEYS.store(any(), anyByte(), any(), any(), any(), any())).thenReturn(CompletableFuture.completedFuture(null)); when(KEYS.store(any(), anyByte(), any(), any(), any(), any())).thenReturn(CompletableFutureTestUtil.almostCompletedFuture(null));
when(KEYS.getEcSignedPreKey(any(), anyByte())).thenReturn(CompletableFuture.completedFuture(Optional.empty())); when(KEYS.getEcSignedPreKey(any(), anyByte())).thenReturn(CompletableFuture.completedFuture(Optional.empty()));
when(KEYS.storeEcSignedPreKeys(any(), any())).thenReturn(CompletableFuture.completedFuture(null)); when(KEYS.storeEcSignedPreKeys(any(), any())).thenReturn(CompletableFutureTestUtil.almostCompletedFuture(null));
when(KEYS.takeEC(EXISTS_UUID, sampleDeviceId)).thenReturn( when(KEYS.takeEC(EXISTS_UUID, sampleDeviceId)).thenReturn(
CompletableFuture.completedFuture(Optional.of(SAMPLE_KEY))); CompletableFuture.completedFuture(Optional.of(SAMPLE_KEY)));

View File

@ -121,6 +121,7 @@ import org.whispersystems.textsecuregcm.storage.ReportMessageManager;
import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper;
import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import org.whispersystems.textsecuregcm.tests.util.KeysHelper; import org.whispersystems.textsecuregcm.tests.util.KeysHelper;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.Pair;
import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.SystemMapper;
import org.whispersystems.textsecuregcm.util.UUIDUtil; import org.whispersystems.textsecuregcm.util.UUIDUtil;
@ -610,19 +611,19 @@ class MessageControllerTest {
UUID uuid1 = UUID.randomUUID(); UUID uuid1 = UUID.randomUUID();
when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid1, null)) when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid1, null))
.thenReturn( .thenReturn(
CompletableFuture.completedFuture(Optional.of(generateEnvelope(uuid1, Envelope.Type.CIPHERTEXT_VALUE, CompletableFutureTestUtil.almostCompletedFuture(Optional.of(generateEnvelope(uuid1, Envelope.Type.CIPHERTEXT_VALUE,
timestamp, sourceUuid, (byte) 1, AuthHelper.VALID_UUID, null, "hi".getBytes(), 0)))); timestamp, sourceUuid, (byte) 1, AuthHelper.VALID_UUID, null, "hi".getBytes(), 0))));
UUID uuid2 = UUID.randomUUID(); UUID uuid2 = UUID.randomUUID();
when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid2, null)) when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid2, null))
.thenReturn( .thenReturn(
CompletableFuture.completedFuture(Optional.of(generateEnvelope( CompletableFutureTestUtil.almostCompletedFuture(Optional.of(generateEnvelope(
uuid2, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, uuid2, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE,
System.currentTimeMillis(), sourceUuid, (byte) 1, AuthHelper.VALID_UUID, null, null, 0)))); System.currentTimeMillis(), sourceUuid, (byte) 1, AuthHelper.VALID_UUID, null, null, 0))));
UUID uuid3 = UUID.randomUUID(); UUID uuid3 = UUID.randomUUID();
when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid3, null)) when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid3, null))
.thenReturn(CompletableFuture.completedFuture(Optional.empty())); .thenReturn(CompletableFutureTestUtil.almostCompletedFuture(Optional.empty()));
UUID uuid4 = UUID.randomUUID(); UUID uuid4 = UUID.randomUUID();
when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid4, null)) when(messagesManager.delete(AuthHelper.VALID_UUID, (byte) 1, uuid4, null))

View File

@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionException;
import java.util.concurrent.TimeUnit;
public class CompletableFutureTestUtil { public class CompletableFutureTestUtil {
@ -24,4 +25,9 @@ public class CompletableFutureTestUtil {
final CompletionException completionException = assertThrows(CompletionException.class, completableFuture::join, message); final CompletionException completionException = assertThrows(CompletionException.class, completableFuture::join, message);
assertTrue(ExceptionUtils.unwrap(completionException).getClass().isAssignableFrom(expectedCause), message); assertTrue(ExceptionUtils.unwrap(completionException).getClass().isAssignableFrom(expectedCause), message);
} }
public static <T> CompletableFuture<T> almostCompletedFuture(T result) {
return new CompletableFuture<T>().completeOnTimeout(result, 5, TimeUnit.MILLISECONDS);
}
} }