From 91ca637e2c377a8a4db4c34ca165fd6ea1b8cf30 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 4 Mar 2026 16:12:37 -0500 Subject: [PATCH] Revert "Add @Required annotation with optional validator (#1558)" This reverts commit 372fff8c7d28bfb491b886b60bb783aca2f61c6d. --- .../main/java/com/slack/api/SlackConfig.java | 8 - .../com/slack/api/util/json/GsonFactory.java | 18 +-- .../slack/api/util/annotation/Required.java | 29 ---- ...quiredPropertyDetectionAdapterFactory.java | 111 ------------- .../api/util/predicate/FieldPredicate.java | 11 -- .../predicate/IsNotNullFieldPredicate.java | 10 -- .../java/test_locally/unit/GsonFactory.java | 21 +-- .../test_locally/util/JSONUtilityTest.java | 152 ------------------ 8 files changed, 7 insertions(+), 353 deletions(-) delete mode 100644 slack-api-model/src/main/java/com/slack/api/util/annotation/Required.java delete mode 100644 slack-api-model/src/main/java/com/slack/api/util/json/RequiredPropertyDetectionAdapterFactory.java delete mode 100644 slack-api-model/src/main/java/com/slack/api/util/predicate/FieldPredicate.java delete mode 100644 slack-api-model/src/main/java/com/slack/api/util/predicate/IsNotNullFieldPredicate.java diff --git a/slack-api-client/src/main/java/com/slack/api/SlackConfig.java b/slack-api-client/src/main/java/com/slack/api/SlackConfig.java index ac2a6329e..a3b0707b2 100644 --- a/slack-api-client/src/main/java/com/slack/api/SlackConfig.java +++ b/slack-api-client/src/main/java/com/slack/api/SlackConfig.java @@ -62,9 +62,6 @@ public void setFailOnUnknownProperties(boolean failOnUnknownProperties) { throwException(); } - @Override - public void setFailOnRequiredProperties(boolean failOnRequiredProperties) { throwException(); } - @Override public void setPrettyResponseLoggingEnabled(boolean prettyResponseLoggingEnabled) { throwException(); @@ -251,11 +248,6 @@ public void setLibraryMaintainerMode(boolean libraryMaintainerMode) { */ private boolean failOnUnknownProperties = false; - /** - * If you would like to detect required properties by throwing exceptions, set this flag as true. - */ - private boolean failOnRequiredProperties = false; - /** * Slack Web API client verifies the existence of tokens before sending HTTP requests to Slack servers. */ diff --git a/slack-api-client/src/main/java/com/slack/api/util/json/GsonFactory.java b/slack-api-client/src/main/java/com/slack/api/util/json/GsonFactory.java index 3bd6c9c7a..4468f2df4 100644 --- a/slack-api-client/src/main/java/com/slack/api/util/json/GsonFactory.java +++ b/slack-api-client/src/main/java/com/slack/api/util/json/GsonFactory.java @@ -44,17 +44,12 @@ public static Gson createSnakeCase(SlackConfig config) { GsonBuilder gsonBuilder = new GsonBuilder() .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES); registerTypeAdapters(gsonBuilder, failOnUnknownProps); - if (failOnUnknownProps || config.isLibraryMaintainerMode()) { - gsonBuilder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()); - } - if (config.isFailOnRequiredProperties()) { - gsonBuilder.registerTypeAdapterFactory(new RequiredPropertyDetectionAdapterFactory()); + gsonBuilder = gsonBuilder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()); } if (config.isPrettyResponseLoggingEnabled()) { - gsonBuilder.setPrettyPrinting(); + gsonBuilder = gsonBuilder.setPrettyPrinting(); } - return gsonBuilder.create(); } @@ -65,17 +60,12 @@ public static Gson createCamelCase(SlackConfig config) { boolean failOnUnknownProps = config.isFailOnUnknownProperties(); GsonBuilder gsonBuilder = new GsonBuilder(); registerTypeAdapters(gsonBuilder, failOnUnknownProps); - if (failOnUnknownProps || config.isLibraryMaintainerMode()) { - gsonBuilder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()); - } - if (config.isFailOnRequiredProperties()) { - gsonBuilder.registerTypeAdapterFactory(new RequiredPropertyDetectionAdapterFactory()); + gsonBuilder = gsonBuilder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()); } if (config.isPrettyResponseLoggingEnabled()) { - gsonBuilder.setPrettyPrinting(); + gsonBuilder = gsonBuilder.setPrettyPrinting(); } - return gsonBuilder.create(); } diff --git a/slack-api-model/src/main/java/com/slack/api/util/annotation/Required.java b/slack-api-model/src/main/java/com/slack/api/util/annotation/Required.java deleted file mode 100644 index 76c85c075..000000000 --- a/slack-api-model/src/main/java/com/slack/api/util/annotation/Required.java +++ /dev/null @@ -1,29 +0,0 @@ -package com.slack.api.util.annotation; - -import com.slack.api.util.predicate.FieldPredicate; -import com.slack.api.util.predicate.IsNotNullFieldPredicate; -import com.slack.api.util.json.RequiredPropertyDetectionAdapterFactory; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Field-level annotation indicating whether the field is a "required" field or not on the model object. - *

- * The enforcement of the field's presence in instantiated instances of the model object is accomplished using the - * {@link RequiredPropertyDetectionAdapterFactory} which ensures all fields marked with {@link Required} are - * present during the object deserialization (or serialization) process. Note that the enforcement of this annotation - * is opt-in and defaults to "off". - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.FIELD) -public @interface Required { - /** - * Optional predicate to evaluate against the field annotated with {@link Required}. By default, all fields - * marked with {@link Required} are checked for null. Primitive field types are initialized by the JVM, and thus - * are never null by default. - */ - Class validator() default IsNotNullFieldPredicate.class; -} diff --git a/slack-api-model/src/main/java/com/slack/api/util/json/RequiredPropertyDetectionAdapterFactory.java b/slack-api-model/src/main/java/com/slack/api/util/json/RequiredPropertyDetectionAdapterFactory.java deleted file mode 100644 index 41a17ed88..000000000 --- a/slack-api-model/src/main/java/com/slack/api/util/json/RequiredPropertyDetectionAdapterFactory.java +++ /dev/null @@ -1,111 +0,0 @@ -package com.slack.api.util.json; - -import com.google.gson.JsonParseException; -import com.google.gson.Gson; -import com.google.gson.TypeAdapterFactory; -import com.google.gson.TypeAdapter; -import com.google.gson.reflect.TypeToken; -import com.google.gson.stream.JsonReader; -import com.google.gson.stream.JsonWriter; -import com.slack.api.util.predicate.FieldPredicate; -import com.slack.api.util.annotation.Required; -import lombok.EqualsAndHashCode; -import lombok.RequiredArgsConstructor; - -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.io.IOException; - -/** - * Adapter factory for processing objects annotated with {@link Required}. This annotation signals what properties - * of a model object are required, and thus should be expected to be initialized on instantiated instances. For all - * fields on the model objected annotated with {@link Required} applies the {@link FieldPredicate#validate(Object)} via the - * specified {@link Required#validator()}. - *

- * Note that this adapter handles both deserialization (JSON --> POJO) and serialization (POJO --> JSON). - */ -public class RequiredPropertyDetectionAdapterFactory implements TypeAdapterFactory { - @Override - public TypeAdapter create(Gson gson, TypeToken type) { - TypeAdapter delegate = gson.getDelegateAdapter(this, type); - List entries = buildRequiredFieldEntries(type.getRawType()); - - if (entries.isEmpty()) { - return delegate; - } - - return new TypeAdapter() { - @Override - public void write(JsonWriter out, T value) throws IOException { - if (value != null) { - ensureFieldValidity(value, entries); - } - delegate.write(out, value); - } - - @Override - public T read(JsonReader in) throws IOException { - T result = delegate.read(in); - if (result == null) { - return null; - } - - ensureFieldValidity(result, entries); - return result; - } - }; - } - - /** - * Scans the given class for fields annotated with {@link Required}, pre-resolves each field's - * accessibility and {@link FieldPredicate} instance, and returns an immutable list of entries. - * This is called once per type during Gson adapter creation. - */ - private List buildRequiredFieldEntries(Class clazz) { - List entries = new ArrayList<>(); - for (Field field : clazz.getDeclaredFields()) { - Required annotation = field.getAnnotation(Required.class); - if (annotation != null) { - field.setAccessible(true); - try { - FieldPredicate predicate = annotation.validator().getDeclaredConstructor().newInstance(); - entries.add(new RequiredFieldEntry(field, predicate)); - } catch (NoSuchMethodException | InstantiationException | - IllegalAccessException | InvocationTargetException e) { - throw new JsonParseException( - "Cannot instantiate validator for field: " + field.getName(), e); - } - } - } - return Collections.unmodifiableList(entries); - } - - private void ensureFieldValidity(T obj, List entries) { - for (RequiredFieldEntry entry : entries) { - try { - Object value = entry.field.get(obj); - if (!entry.predicate.validate(value)) { - throw new JsonParseException("Required field '" + entry.field.getName() - + "' failed validation in " + obj.getClass().getSimpleName() - + " using predicate " + entry.predicate.getClass().getSimpleName()); - } - } catch (IllegalAccessException e) { - throw new JsonParseException( - "Cannot access field: " + entry.field.getName(), e); - } - } - } - - /** - * Class holding the accessible {@link Field} handle and the cached instance of {@link FieldPredicate}. - */ - @RequiredArgsConstructor - @EqualsAndHashCode - private static class RequiredFieldEntry { - final Field field; - final FieldPredicate predicate; - } -} diff --git a/slack-api-model/src/main/java/com/slack/api/util/predicate/FieldPredicate.java b/slack-api-model/src/main/java/com/slack/api/util/predicate/FieldPredicate.java deleted file mode 100644 index 968ebe6ff..000000000 --- a/slack-api-model/src/main/java/com/slack/api/util/predicate/FieldPredicate.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.slack.api.util.predicate; - -/** - * A functional interface for defining validation predicates against {@link java.lang.reflect.Field}. Used by - * {@link com.slack.api.util.annotation.Required} during object serialization and deserialization to ensure the - * field member is "valid" per the defined predicate. - */ -@FunctionalInterface -public interface FieldPredicate { - boolean validate(Object obj); -} diff --git a/slack-api-model/src/main/java/com/slack/api/util/predicate/IsNotNullFieldPredicate.java b/slack-api-model/src/main/java/com/slack/api/util/predicate/IsNotNullFieldPredicate.java deleted file mode 100644 index a80ce45f9..000000000 --- a/slack-api-model/src/main/java/com/slack/api/util/predicate/IsNotNullFieldPredicate.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.slack.api.util.predicate; - -import java.util.Objects; - -public class IsNotNullFieldPredicate implements FieldPredicate { - @Override - public boolean validate(Object obj) { - return Objects.nonNull(obj); - } -} diff --git a/slack-api-model/src/test/java/test_locally/unit/GsonFactory.java b/slack-api-model/src/test/java/test_locally/unit/GsonFactory.java index 9f6591b02..409056175 100644 --- a/slack-api-model/src/test/java/test_locally/unit/GsonFactory.java +++ b/slack-api-model/src/test/java/test_locally/unit/GsonFactory.java @@ -27,19 +27,7 @@ public static Gson createSnakeCaseWithoutUnknownPropertyDetection(boolean failOn return createSnakeCase(failOnUnknownProperties, false); } - public static Gson createSnakeCaseWithRequiredPropertyDetection() { - return createSnakeCase(false, true, true); - } - public static Gson createSnakeCase(boolean failOnUnknownProperties, boolean unknownPropertyDetection) { - return createSnakeCase(failOnUnknownProperties, unknownPropertyDetection, false); - } - - public static Gson createSnakeCase( - boolean failOnUnknownProperties, - boolean unknownPropertyDetection, - boolean failOnRequiredProperties - ) { GsonBuilder builder = new GsonBuilder() .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) .registerTypeAdapter(File.class, new GsonFileFactory(failOnUnknownProperties)) @@ -57,12 +45,9 @@ public static Gson createSnakeCase( new GsonMessageChangedEventPreviousMessageFactory(failOnUnknownProperties)); if (unknownPropertyDetection) { - builder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()); - } - if (failOnRequiredProperties) { - builder.registerTypeAdapterFactory(new RequiredPropertyDetectionAdapterFactory()); + return builder.registerTypeAdapterFactory(new UnknownPropertyDetectionAdapterFactory()).create(); + } else { + return builder.create(); } - - return builder.create(); } } diff --git a/slack-api-model/src/test/java/test_locally/util/JSONUtilityTest.java b/slack-api-model/src/test/java/test_locally/util/JSONUtilityTest.java index 718721586..c2139faee 100644 --- a/slack-api-model/src/test/java/test_locally/util/JSONUtilityTest.java +++ b/slack-api-model/src/test/java/test_locally/util/JSONUtilityTest.java @@ -11,31 +11,20 @@ import com.slack.api.model.block.element.ImageElement; import com.slack.api.model.block.element.OverflowMenuElement; import com.slack.api.model.event.FunctionExecutedEvent; -import com.slack.api.util.annotation.Required; import com.slack.api.util.json.*; -import com.slack.api.util.predicate.FieldPredicate; -import lombok.Builder; -import lombok.Data; import org.junit.Test; import test_locally.unit.GsonFactory; import java.lang.reflect.Type; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.function.Predicate; import static com.slack.api.model.block.composition.BlockCompositions.markdownText; import static com.slack.api.model.block.composition.BlockCompositions.plainText; import static com.slack.api.model.block.element.BlockElements.image; import static com.slack.api.model.block.element.BlockElements.overflowMenu; import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThrows; public class JSONUtilityTest { @@ -164,145 +153,4 @@ public void testGsonFunctionExecutedEventInputValueFactory() { parsed = f.deserialize(json, FunctionExecutedEvent.InputValue.class, context); assertThat(parsed.asStringArray(), is(Arrays.asList("C111", "C222"))); } - - @Test - public void testRequiredPropertyDetectionAdapterFactory_basicCase_failureCases() { - Gson gson = GsonFactory.createSnakeCaseWithRequiredPropertyDetection(); - - // Serialization - TestClassWithRequiredBasic instance = TestClassWithRequiredBasic.builder().build(); - assertThrows(JsonParseException.class, () -> gson.toJson(instance)); - - // Deserialization - String json = "{\"name\": \"Hello\"}"; - assertThrows(JsonParseException.class, () -> gson.fromJson(json, TestClassWithRequiredBasic.class)); - } - - @Test - public void testRequiredPropertyDetectionAdapterFactory_basicCase_happyPath() { - Gson gson = GsonFactory.createSnakeCaseWithRequiredPropertyDetection(); - TestClassWithRequiredBasic instanceNoName = TestClassWithRequiredBasic.builder().id(1).build(); - TestClassWithRequiredBasic instanceWithName = TestClassWithRequiredBasic.builder().id(1).name("Hello").build(); - - // Serialization - assertThat(gson.toJson(instanceNoName), is("{\"id\":1}")); - assertThat(gson.toJson(instanceWithName), is("{\"id\":1,\"name\":\"Hello\"}")); - - // Deserialization - String json = "{\"id\": 1}"; - TestClassWithRequiredBasic instance = gson.fromJson(json, TestClassWithRequiredBasic.class); - assertThat(instance.getId(), is(1)); - assertNull(instance.getName()); - - json = "{\"id\": 1, \"name\": \"Hello\"}"; - instance = gson.fromJson(json, TestClassWithRequiredBasic.class); - assertThat(instance.getId(), is(1)); - assertThat(instance.getName(), is("Hello")); - } - - @Test - public void testRequiredPropertyDetectionAdapterFactory_advancedCase_failureCases() { - Gson gson = GsonFactory.createSnakeCaseWithRequiredPropertyDetection(); - List users = new ArrayList<>(); - // Serialization - JsonParseException e = assertThrows(JsonParseException.class, () -> gson.toJson(TestClassWithRequiredAdvanced.builder().build())); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'id' failed validation in TestClassWithRequiredAdvanced using predicate IntegerGreaterThanZero")); - - e = assertThrows(JsonParseException.class, () -> gson.toJson(TestClassWithRequiredAdvanced.builder().id(1).build())); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'name' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyString")); - - e = assertThrows(JsonParseException.class, () -> gson.toJson(TestClassWithRequiredAdvanced.builder().id(1).name("").build())); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'name' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyString")); - - e = assertThrows(JsonParseException.class, () -> gson.toJson(TestClassWithRequiredAdvanced.builder().id(1).name("Hello").build())); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'users' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyCollection")); - - users.add("user1"); - e = assertThrows(JsonParseException.class, () -> gson.toJson(TestClassWithRequiredAdvanced.builder().id(1).name("Hello").users(users).build())); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'myBool' failed validation in TestClassWithRequiredAdvanced using predicate isNotNullFieldPredicate")); - - // Deserialization - e = assertThrows(JsonParseException.class, () -> gson.fromJson("{\"id\": 0}", TestClassWithRequiredAdvanced.class)); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'id' failed validation in TestClassWithRequiredAdvanced using predicate IntegerGreaterThanZero")); - - e = assertThrows(JsonParseException.class, () -> gson.fromJson("{\"id\": 1}", TestClassWithRequiredAdvanced.class)); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'name' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyString")); - - e = assertThrows(JsonParseException.class, () -> gson.fromJson("{\"id\": 1, \"name\": ''}", TestClassWithRequiredAdvanced.class)); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'name' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyString")); - - e = assertThrows(JsonParseException.class, () -> gson.fromJson("{\"id\":1,\"name\":\"test\",\"users\":[]}", TestClassWithRequiredAdvanced.class)); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'users' failed validation in TestClassWithRequiredAdvanced using predicate NonEmptyCollection")); - - e = assertThrows(JsonParseException.class, () -> gson.fromJson("{\"id\":1,\"name\":\"test\",\"users\":[\"hello\"]}", TestClassWithRequiredAdvanced.class)); - assertThat(e.getMessage(), equalToIgnoringCase("Required field 'myBool' failed validation in TestClassWithRequiredAdvanced using predicate isNotNullFieldPredicate")); - } - - @Test - public void testRequiredPropertyDetectionAdapterFactory_advancedCase_happyPath() { - Gson gson = GsonFactory.createSnakeCaseWithRequiredPropertyDetection(); - List users = new ArrayList<>(); - users.add("testUser"); - TestClassWithRequiredAdvanced instance = TestClassWithRequiredAdvanced.builder() - .id(1) - .name("test") - .users(users) - .myBool(true) - .build(); - - // Serialization - assertThat(gson.toJson(instance), is("{\"id\":1,\"name\":\"test\",\"users\":[\"testUser\"],\"my_bool\":true}")); - - // Deserialization - String json = "{\"id\":1,\"name\":\"test\",\"users\":[\"testUser\"],\"my_bool\":true}"; - instance = gson.fromJson(json, TestClassWithRequiredAdvanced.class); - assertThat(instance.getId(), is(1)); - assertThat(instance.getName(), is("test")); - assertThat(instance.getUsers().get(0), is("testUser")); - assertThat(instance.getMyBool(), is(true)); - assertNull(instance.getCanBeNull()); - } - - @Data - @Builder - private static class TestClassWithRequiredBasic { - @Required private Integer id; - private String name; - } - - @Data - @Builder - private static class TestClassWithRequiredAdvanced { - @Required(validator = IntegerGreaterThanZero.class) - private int id; - @Required(validator = NonEmptyString.class) - private String name; - @Required(validator = NonEmptyCollection.class) - private List users; - @Required - Boolean myBool; - private String canBeNull; - - public static class IntegerGreaterThanZero implements FieldPredicate { - @Override - public boolean validate(Object obj) { - return obj instanceof Integer && (int)obj > 0; - } - } - - public static class NonEmptyString implements FieldPredicate { - @Override - public boolean validate(Object obj) { - return obj instanceof String && !((String) obj).isEmpty(); - } - } - - public static class NonEmptyCollection implements FieldPredicate { - @Override - public boolean validate(Object obj) { - Predicate> isNotEmpty = collection -> !collection.isEmpty(); - return obj instanceof Collection && isNotEmpty.test((Collection) obj); - } - } - } }