fix: don't reset webhook events when updating other fields (#188)

Co-authored-by: Faiz Shaikh <[email protected]>

Author: shaljam

src/main/java/io/getstream/chat/java/models/App.java

@@ -798,6 +798,7 @@ public static class AppUpdateRequestData {
 
     @NotNull
     @JsonProperty("reminders_interval")
+    @JsonInclude(Include.NON_DEFAULT)
     private int remindersInterval;
 
     @Nullable
@@ -887,17 +888,21 @@ public static class AppUpdateRequestData {
 
     @Nullable
     @JsonProperty("webhook_events")
+    @JsonInclude(Include.NON_NULL)
     private List<String> webhookEvents;
 
     @Nullable
     @JsonProperty("multi_tenant_enabled")
     @JsonInclude(Include.NON_NULL)
     private Boolean multiTenantEnabled;
 
+    static final Date defaultDate = new Date(Long.MIN_VALUE);
+
     @Nullable
     @JsonProperty("revoke_tokens_issued_before")
-    // This field can be sent as null
-    private Date revokeTokensIssuedBefore;
+    @JsonInclude(value = Include.CUSTOM, valueFilter = NonDefaultDateFilter.class)
+    @Builder.Default
+    private Date revokeTokensIssuedBefore = defaultDate;
 
     @Nullable
     @JsonProperty("channel_hide_members_only")
@@ -911,6 +916,7 @@ public static class AppUpdateRequestData {
 
     @Nullable
     @JsonProperty("grants")
+    @JsonInclude(Include.NON_NULL)
     private Map<String, List<String>> grants;
 
     @Nullable
@@ -924,6 +930,17 @@ protected Call<StreamResponseObject> generateCall(Client client) {
         return client.create(AppService.class).update(this.internalBuild());
       }
     }
+
+    // Filter that excludes default date and included null (used to reset)
+    static class NonDefaultDateFilter {
+      // Return true if filtering out (excluding), false to include
+      @Override
+      public boolean equals(Object o) {
+        // only ever called with String value
+        Date other = (Date) o;
+        return defaultDate.equals(other);
+      }
+    }
   }
 
   public static class AppGetRateLimitsRequest extends StreamRequest<AppGetRateLimitsResponse> {

src/test/java/io/getstream/chat/java/AppTest.java

@@ -1,5 +1,6 @@
 package io.getstream.chat.java;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import io.getstream.chat.java.exceptions.StreamException;
 import io.getstream.chat.java.models.App;
 import io.getstream.chat.java.models.App.AppCheckSnsResponse;
@@ -11,11 +12,13 @@
 import io.getstream.chat.java.models.Message;
 import io.getstream.chat.java.models.Message.MessageRequestObject;
 import io.getstream.chat.java.services.framework.DefaultClient;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.Date;
 import java.util.GregorianCalendar;
+import java.util.HashMap;
 import java.util.Properties;
 import java.util.Random;
 import org.junit.jupiter.api.Assertions;
@@ -250,4 +253,56 @@ void whenUpdatingAppSettingsWithSNSEventHook_thenNoException() throws StreamExce
       throw e;
     }
   }
+
+  @DisplayName("App Settings update webhook events")
+  @Test
+  void whenUpdatingAppSettings_thenDoesntAlwaysChangeWebhookEvents() {
+    var messageNewList = Arrays.asList("message.new");
+    Assertions.assertDoesNotThrow(() -> App.update().webhookEvents(messageNewList).request());
+
+    var appConfig = Assertions.assertDoesNotThrow(() -> App.get().request()).getApp();
+    Assertions.assertEquals(messageNewList, appConfig.getWebhookEvents());
+
+    // Updating another field should not change (reset) webhook events
+    Assertions.assertDoesNotThrow(() -> App.update().remindersInterval(60).request());
+
+    appConfig = Assertions.assertDoesNotThrow(() -> App.get().request()).getApp();
+    Assertions.assertEquals(messageNewList, appConfig.getWebhookEvents());
+
+    // Reset webhook events to defaults using an empty list
+    Assertions.assertDoesNotThrow(() -> App.update().webhookEvents(new ArrayList<>()).request());
+    appConfig = Assertions.assertDoesNotThrow(() -> App.get().request()).getApp();
+    Assertions.assertTrue(appConfig.getWebhookEvents().size() > 1);
+  }
+
+  @DisplayName("AppConfig encoding should not include null fields")
+  @Test
+  void whenEncodingAppConfig_thenNoNullFields() {
+    var appConfig = App.update().internalBuild();
+    final ObjectMapper mapper = new ObjectMapper();
+
+    String json = Assertions.assertDoesNotThrow(() -> mapper.writeValueAsString(appConfig));
+
+    // When we didn't set any fields, the JSON should be empty
+    Assertions.assertEquals("{}", json);
+
+    // We set some fields where we mean to reset them
+    var appConfigNull =
+        App.update()
+            .webhookEvents(new ArrayList<>())
+            .revokeTokensIssuedBefore(null)
+            .grants(new HashMap<>())
+            .internalBuild();
+
+    json = Assertions.assertDoesNotThrow(() -> mapper.writeValueAsString(appConfigNull));
+
+    Assertions.assertTrue(
+        json.contains("\"webhook_events\":[]"), "JSON should contain null webhook_events field");
+
+    Assertions.assertTrue(
+        json.contains("\"revoke_tokens_issued_before\":null"),
+        "JSON should contain null revoke_tokens_issued_before field");
+
+    Assertions.assertTrue(json.contains("\"grants\":{}"), "JSON should contain null grants field");
+  }
 }