Avoid annotation-driven parsing of device capabilities in `DeviceCont…

…roller`
signalapp · Oct 31, 2024 · 63021e0 · 63021e0
1 parent bf741df
commit 63021e0
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 24 deletions.
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/DeviceController.java
@@ -4,8 +4,6 @@
  */
 package org.whispersystems.textsecuregcm.controllers;
 
-import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
-import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.net.HttpHeaders;
 import io.dropwizard.auth.Auth;
@@ -414,13 +412,12 @@ public void setUnauthenticatedDelivery(@Mutable @Auth AuthenticatedDevice auth)
   public void setCapabilities(@Mutable @Auth final AuthenticatedDevice auth,
 
       @NotNull
-      @JsonSerialize(using = DeviceCapabilityAdapter.Serializer.class)
-      @JsonDeserialize(using = DeviceCapabilityAdapter.Deserializer.class)
-      final Set<DeviceCapability> capabilities) {
+      final Map<String, Boolean> capabilities) {
 
     assert (auth.getAuthenticatedDevice() != null);
     final byte deviceId = auth.getAuthenticatedDevice().getId();
-    accounts.updateDevice(auth.getAccount(), deviceId, d -> d.setCapabilities(capabilities));
+    accounts.updateDevice(auth.getAccount(), deviceId,
+        d -> d.setCapabilities(DeviceCapabilityAdapter.mapToSet(capabilities)));
   }
 
   @PUT

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeviceCapability.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeviceCapability.java
@@ -5,6 +5,8 @@
 
 package org.whispersystems.textsecuregcm.storage;
 
+import java.util.Optional;
+
 public enum DeviceCapability {
   STORAGE("storage", AccountCapabilityMode.ANY_DEVICE, false, false),
   TRANSFER("transfer", AccountCapabilityMode.PRIMARY_DEVICE, false, false),
@@ -50,13 +52,12 @@ public boolean includeInProfile() {
     return includeInProfile;
   }
 
-  public static DeviceCapability forName(final String name) {
+  public static Optional<DeviceCapability> forName(final String name) {
     for (final DeviceCapability capability : DeviceCapability.values()) {
       if (capability.getName().equals(name)) {
-        return capability;
+        return Optional.of(capability);
       }
     }
-
-    throw new IllegalArgumentException("Unknown capability: " + name);
+    return Optional.empty();
   }
 }
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceCapabilityAdapter.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/DeviceCapabilityAdapter.java
@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.util.EnumSet;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -44,20 +45,19 @@ public static class Deserializer extends JsonDeserializer<Set<DeviceCapability>>
     public Set<DeviceCapability> deserialize(final JsonParser jsonParser,
         final DeserializationContext deserializationContext) throws IOException {
 
-      final Map<String, Boolean> capabilitiesMap = jsonParser.readValueAs(STRING_TO_BOOLEAN_MAP_TYPE);
-      final EnumSet<DeviceCapability> capabilities = EnumSet.noneOf(DeviceCapability.class);
+      return mapToSet(jsonParser.readValueAs(STRING_TO_BOOLEAN_MAP_TYPE));
+    }
 
-      capabilitiesMap.forEach((capability, active) -> {
-        if (active) {
-          try {
-            capabilities.add(DeviceCapability.forName(capability));
-          } catch (final IllegalArgumentException ignored) {
-            // This most likely means we've retired a capability
-          }
-        }
-      });
+  }
 
-      return capabilities;
-    }
+  public static Set<DeviceCapability> mapToSet(Map<String, Boolean> capabilitiesMap) {
+    return capabilitiesMap.entrySet()
+        .stream()
+        .filter(Map.Entry::getValue)
+        .map(entry -> DeviceCapability.forName(entry.getKey()))
+        .filter(Optional::isPresent)
+        .map(Optional::get)
+        .collect(Collectors.toCollection(() -> EnumSet.noneOf(DeviceCapability.class)));
   }
+
 }
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/DeviceControllerTest.java
@@ -785,10 +785,11 @@ void putCapabilitiesSuccessTest() {
         .request()
         .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD))
         .header(HttpHeaders.USER_AGENT, "Signal-Android/5.42.8675309 Android/30")
-        .put(Entity.entity(Set.of(), MediaType.APPLICATION_JSON_TYPE))) {
+        .put(Entity.json("{\"deleteSync\": true, \"notARealDeviceCapability\": true}"))) {
 
       assertThat(response.getStatus()).isEqualTo(204);
       assertThat(response.hasEntity()).isFalse();
+      verify(AuthHelper.VALID_DEVICE).setCapabilities(Set.of(DeviceCapability.DELETE_SYNC));
     }
   }