From 75e7ffc073e4679f2027ab44f2016980d671bd58 Mon Sep 17 00:00:00 2001 From: Yurii Yakhnytsia Date: Wed, 14 Oct 2026 18:01:01 +0300 Subject: [PATCH 1/4] Clear chat.update attachments/blocks via type-level @JsonSerialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the BeanSerializerModifier approach (registered on ObjectMapperUtils.mapper()) with a @JsonSerialize serializer on ChatUpdateMessageParams, so the shouldClearAttachments/ shouldClearBlocks flags emit `[]` under ANY ObjectMapper — including a consumer's own mapper that never touches ObjectMapperUtils.mapper(). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ChatUpdateMessageParamsSerializer.java | 121 ++++++++++++++++++ ...UpdateMessageParamsSerializerModifier.java | 62 --------- .../jackson/ForceEmitPropertyWriter.java | 59 --------- .../client/jackson/ObjectMapperUtils.java | 7 - .../chat/ChatUpdateMessageParamsIF.java | 3 + ...tUpdateMessageParamsSerializationTest.java | 40 +++++- 6 files changed, 157 insertions(+), 135 deletions(-) create mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java delete mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java delete mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java new file mode 100644 index 00000000..e6cbd0f5 --- /dev/null +++ b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java @@ -0,0 +1,121 @@ +package com.hubspot.slack.client.jackson; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; +import com.hubspot.slack.client.methods.params.chat.ChatUpdateMessageParams; +import java.io.IOException; +import java.util.List; + +/** + * Custom serializer for {@link ChatUpdateMessageParams} that reproduces the type's default + * {@code @JsonInclude(NON_EMPTY)} behavior for every field, with one deliberate addition: the + * {@code attachments} / {@code blocks} arrays can be force-emitted as an empty array {@code []} + * when {@link ChatUpdateMessageParams#shouldClearAttachments()} / + * {@link ChatUpdateMessageParams#shouldClearBlocks()} are set. + * + *

Slack's {@code chat.update} only clears a field when it is sent as {@code []}; an omitted + * field is left unchanged. Because the params type is {@code NON_EMPTY}, an empty list is normally + * omitted — which is what every partial-update caller relies on (so the message keeps its existing + * attachments/blocks). The clear flags are the opt-in way to instead wipe one of those fields. + * + *

This is attached to the type via {@code @JsonSerialize(using = …)} rather than registered on a + * specific {@code ObjectMapper}, so the clear behavior is honored by any mapper that + * serializes these params (consumers often serialize request bodies with their own mapper, not + * {@link ObjectMapperUtils#mapper()}). With both flags left at their {@code false} default this + * serializer emits byte-for-byte the same JSON as the previous default bean serialization. + */ +public class ChatUpdateMessageParamsSerializer + extends StdSerializer { + + private static final String CHANNEL_FIELD = "channel"; + private static final String TEXT_FIELD = "text"; + private static final String TS_FIELD = "ts"; + private static final String FILE_IDS_FIELD = "file_ids"; + private static final String AS_USER_FIELD = "as_user"; + private static final String LINK_NAMES_FIELD = "link_names"; + private static final String PARSE_FIELD = "parse"; + private static final String ATTACHMENTS_FIELD = "attachments"; + private static final String BLOCKS_FIELD = "blocks"; + + public ChatUpdateMessageParamsSerializer() { + super(ChatUpdateMessageParams.class); + } + + @Override + public void serialize( + ChatUpdateMessageParams params, + JsonGenerator gen, + SerializerProvider provider + ) throws IOException { + gen.writeStartObject(); + + // NON_EMPTY string fields: present on the wire only when non-empty. + writeNonEmptyString(gen, CHANNEL_FIELD, params.getChannelId()); + if (params.getText().isPresent() && !params.getText().get().isEmpty()) { + gen.writeStringField(TEXT_FIELD, params.getText().get()); + } + writeNonEmptyString(gen, TS_FIELD, params.getTs()); + + if (!params.getFileIds().isEmpty()) { + gen.writeArrayFieldStart(FILE_IDS_FIELD); + for (String fileId : params.getFileIds()) { + gen.writeString(fileId); + } + gen.writeEndArray(); + } + + // as_user carries a field-level @JsonInclude(NON_ABSENT): present only when set. + if (params.getAsUser().isPresent()) { + gen.writeBooleanField(AS_USER_FIELD, params.getAsUser().get()); + } + + // @Default fields that are never "empty" under NON_EMPTY, so always present today. + gen.writeBooleanField(LINK_NAMES_FIELD, params.shouldLinkNames()); + writeNonEmptyString(gen, PARSE_FIELD, params.getParse()); + + writeListOrClear( + gen, + ATTACHMENTS_FIELD, + params.getAttachments(), + params.shouldClearAttachments() + ); + writeListOrClear(gen, BLOCKS_FIELD, params.getBlocks(), params.shouldClearBlocks()); + + gen.writeEndObject(); + } + + private static void writeNonEmptyString( + JsonGenerator gen, + String fieldName, + String value + ) throws IOException { + if (value != null && !value.isEmpty()) { + gen.writeStringField(fieldName, value); + } + } + + /** + * Writes {@code fieldName} as a JSON array of the list's elements when the list is non-empty, or + * as an empty array {@code []} when {@code forceClear} is set (the Slack "clear this field" + * signal). When the list is empty and {@code forceClear} is {@code false} the field is omitted, + * matching {@code NON_EMPTY}. Each element is serialized through the active generator's codec so + * element-level custom serializers (e.g. {@link AttachmentSerializer}) still apply under whatever + * mapper is in use. + */ + private static void writeListOrClear( + JsonGenerator gen, + String fieldName, + List items, + boolean forceClear + ) throws IOException { + if (items.isEmpty() && !forceClear) { + return; + } + gen.writeArrayFieldStart(fieldName); + for (Object item : items) { + gen.writeObject(item); + } + gen.writeEndArray(); + } +} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java deleted file mode 100644 index 464a40c4..00000000 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java +++ /dev/null @@ -1,62 +0,0 @@ -package com.hubspot.slack.client.jackson; - -import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.SerializationConfig; -import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; -import com.fasterxml.jackson.databind.ser.BeanSerializerModifier; -import com.hubspot.slack.client.methods.params.chat.ChatUpdateMessageParams; -import java.util.List; - -/** - * Swaps the default {@code attachments} / {@code blocks} property writers on - * {@link ChatUpdateMessageParams} for {@link ForceEmitPropertyWriter}s driven by that type's - * {@code shouldClearAttachments()} / {@code shouldClearBlocks()} flags. Every other property keeps - * its default writer, so the only behavioral change is that an empty {@code attachments}/ - * {@code blocks} list is emitted as {@code []} (instead of being omitted) when the matching flag is - * set — the signal Slack's {@code chat.update} needs to clear that field. With the flags at their - * {@code false} default, serialization is unchanged. - */ -public class ChatUpdateMessageParamsSerializerModifier extends BeanSerializerModifier { - - private static final String ATTACHMENTS_PROPERTY = "attachments"; - private static final String BLOCKS_PROPERTY = "blocks"; - - @Override - public List changeProperties( - SerializationConfig config, - BeanDescription beanDesc, - List beanProperties - ) { - if (!ChatUpdateMessageParams.class.isAssignableFrom(beanDesc.getBeanClass())) { - return beanProperties; - } - - for (int i = 0; i < beanProperties.size(); i++) { - BeanPropertyWriter writer = beanProperties.get(i); - switch (writer.getName()) { - case ATTACHMENTS_PROPERTY: - beanProperties.set( - i, - new ForceEmitPropertyWriter( - writer, - bean -> ChatUpdateMessageParams.class.cast(bean).shouldClearAttachments() - ) - ); - break; - case BLOCKS_PROPERTY: - beanProperties.set( - i, - new ForceEmitPropertyWriter( - writer, - bean -> ChatUpdateMessageParams.class.cast(bean).shouldClearBlocks() - ) - ); - break; - default: - break; - } - } - - return beanProperties; - } -} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java deleted file mode 100644 index 853de217..00000000 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java +++ /dev/null @@ -1,59 +0,0 @@ -package com.hubspot.slack.client.jackson; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; -import java.io.Serializable; - -/** - * Wraps a single {@link BeanPropertyWriter}. When {@code forceEmit} tests {@code true} for the bean - * being serialized, the property is written even if its value is empty — bypassing the type's - * {@code @JsonInclude(NON_EMPTY)} suppression so the field appears as {@code []}. Otherwise it - * defers to the wrapped writer, which omits an empty collection exactly as before. - * - *

Used by {@link ChatUpdateMessageParamsSerializerModifier} to let {@code chat.update} actively - * clear {@code attachments}/{@code blocks}; every other property keeps its default writer. - */ -public class ForceEmitPropertyWriter extends BeanPropertyWriter { - - /** - * Decides, per bean instance, whether the wrapped property should be force-emitted. Declared - * {@link Serializable} because {@link BeanPropertyWriter} is itself serializable, so this field - * must not be left as unrestorable (transient) state. - */ - @FunctionalInterface - public interface ForceEmitPredicate extends Serializable { - boolean test(Object bean); - } - - private static final long SERIAL_VERSION_UID = 1L; - - private final ForceEmitPredicate forceEmit; - - public ForceEmitPropertyWriter(BeanPropertyWriter base, ForceEmitPredicate forceEmit) { - super(base); - this.forceEmit = forceEmit; - } - - @Override - public void serializeAsField(Object bean, JsonGenerator gen, SerializerProvider prov) - throws Exception { - if (!forceEmit.test(bean)) { - // Unchanged behavior: the wrapped writer applies NON_EMPTY and omits an empty collection. - super.serializeAsField(bean, gen, prov); - return; - } - - Object value = get(bean); - gen.writeFieldName(getName()); - if (value == null) { - prov.defaultSerializeNull(gen); - return; - } - JsonSerializer serializer = _serializer != null - ? _serializer - : prov.findValueSerializer(value.getClass(), this); - serializer.serialize(value, gen, prov); - } -} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java index 89e95b43..546d47c9 100644 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java +++ b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.datatype.joda.JodaModule; @@ -26,12 +25,6 @@ private static ObjectMapper create() { mapper.registerModule(new Jdk8Module()); mapper.registerModule(new JavaTimeModule()); - SimpleModule chatUpdateClearFlagsModule = new SimpleModule(); - chatUpdateClearFlagsModule.setSerializerModifier( - new ChatUpdateMessageParamsSerializerModifier() - ); - mapper.registerModule(chatUpdateClearFlagsModule); - mapper.configure(JsonParser.Feature.ALLOW_COMMENTS, false); mapper.configure(JsonParser.Feature.AUTO_CLOSE_SOURCE, true); mapper.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, true); diff --git a/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java b/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java index 984d7afa..88665b63 100644 --- a/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java +++ b/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java @@ -6,10 +6,12 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.PropertyNamingStrategies.SnakeCaseStrategy; import com.fasterxml.jackson.databind.annotation.JsonNaming; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.hubspot.immutables.style.HubSpotStyle; +import com.hubspot.slack.client.jackson.ChatUpdateMessageParamsSerializer; import java.util.Optional; import org.immutables.value.Value.Check; import org.immutables.value.Value.Default; @@ -19,6 +21,7 @@ @HubSpotStyle @JsonNaming(SnakeCaseStrategy.class) @JsonInclude(Include.NON_EMPTY) +@JsonSerialize(using = ChatUpdateMessageParamsSerializer.class) public interface ChatUpdateMessageParamsIF extends MessageParams { @JsonProperty("channel") String getChannelId(); diff --git a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java index 7339a88a..78875306 100644 --- a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java +++ b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.hubspot.slack.client.jackson.ObjectMapperUtils; import com.hubspot.slack.client.models.Attachment; import com.hubspot.slack.client.models.blocks.Divider; @@ -240,11 +242,9 @@ public void itDefaultsBothClearFlagsToFalse() { } /** - * The {@code BeanSerializerModifier} registered on the shared mapper must only rewrite - * {@link ChatUpdateMessageParams} writers. Serializing a different {@link MessageParams} type - * exercises that guard: if the modifier matched too broadly, {@code ForceEmitPropertyWriter}'s - * predicate would try to cast this bean to {@code ChatUpdateMessageParams} and throw, so a clean - * serialization proves the modifier left other types alone. + * The custom serializer is attached only to {@link ChatUpdateMessageParams} via + * {@code @JsonSerialize}, so a different {@link MessageParams} type is unaffected and keeps its + * own serialization rules. */ @Test public void itDoesNotAffectOtherMessageParamTypes() { @@ -256,10 +256,36 @@ public void itDoesNotAffectOtherMessageParamTypes() { JsonNode json = OBJECT_MAPPER.valueToTree(postParams); - // ChatPostMessageParams keeps empty collections by its own (unchanged) rules — the point is - // simply that serialization succeeds and our clear-flag behavior did not leak onto it. assertThat(json.get("channel").asText()).isEqualTo("C123"); assertThat(json.has("should_clear_attachments")).isFalse(); assertThat(json.has("should_clear_blocks")).isFalse(); } + + /** + * The whole reason for {@code @JsonSerialize} on the type (rather than a mapper-level modifier): + * consumers serialize request bodies with their own {@link ObjectMapper}, not + * {@link ObjectMapperUtils#mapper()}. A foreign mapper with no slack-specific configuration must + * still honor the clear flags and emit {@code []}, otherwise the field never gets cleared and the + * message double-renders. This mirrors how slack-integration serializes (its own mapper). + */ + @Test + public void itHonorsClearFlagsUnderAForeignObjectMapper() { + ObjectMapper foreignMapper = new ObjectMapper() + .registerModule(new Jdk8Module()) + .registerModule(new GuavaModule()); + + JsonNode json = foreignMapper.valueToTree( + ChatUpdateMessageParams + .builder() + .setChannelId("C123") + .setTs("1790587670.603089") + .setText("testText") + .setShouldClearBlocks(true) + .build() + ); + + assertThat(json.get("blocks").isArray()).isTrue(); + assertThat(json.get("blocks")).isEmpty(); + assertThat(json.has("attachments")).isFalse(); + } } From cd36820b7a42ce513d17c438baf9079a10137868 Mon Sep 17 00:00:00 2001 From: Yurii Yakhnytsia Date: Wed, 14 Oct 2026 18:01:01 +0300 Subject: [PATCH 2/4] Clear chat.update attachments/blocks via type-level @JsonSerialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the BeanSerializerModifier approach (registered on ObjectMapperUtils.mapper()) with a @JsonSerialize serializer on ChatUpdateMessageParams, so the shouldClearAttachments/ shouldClearBlocks flags emit `[]` under ANY ObjectMapper — including a consumer's own mapper that never touches ObjectMapperUtils.mapper(). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ChatUpdateMessageParamsSerializer.java | 121 ++++++++++++++++++ ...UpdateMessageParamsSerializerModifier.java | 62 --------- .../jackson/ForceEmitPropertyWriter.java | 59 --------- .../client/jackson/ObjectMapperUtils.java | 7 - .../chat/ChatUpdateMessageParamsIF.java | 3 + ...tUpdateMessageParamsSerializationTest.java | 82 +++++++++++- 6 files changed, 199 insertions(+), 135 deletions(-) create mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java delete mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java delete mode 100644 slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java new file mode 100644 index 00000000..e6cbd0f5 --- /dev/null +++ b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java @@ -0,0 +1,121 @@ +package com.hubspot.slack.client.jackson; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; +import com.hubspot.slack.client.methods.params.chat.ChatUpdateMessageParams; +import java.io.IOException; +import java.util.List; + +/** + * Custom serializer for {@link ChatUpdateMessageParams} that reproduces the type's default + * {@code @JsonInclude(NON_EMPTY)} behavior for every field, with one deliberate addition: the + * {@code attachments} / {@code blocks} arrays can be force-emitted as an empty array {@code []} + * when {@link ChatUpdateMessageParams#shouldClearAttachments()} / + * {@link ChatUpdateMessageParams#shouldClearBlocks()} are set. + * + *

Slack's {@code chat.update} only clears a field when it is sent as {@code []}; an omitted + * field is left unchanged. Because the params type is {@code NON_EMPTY}, an empty list is normally + * omitted — which is what every partial-update caller relies on (so the message keeps its existing + * attachments/blocks). The clear flags are the opt-in way to instead wipe one of those fields. + * + *

This is attached to the type via {@code @JsonSerialize(using = …)} rather than registered on a + * specific {@code ObjectMapper}, so the clear behavior is honored by any mapper that + * serializes these params (consumers often serialize request bodies with their own mapper, not + * {@link ObjectMapperUtils#mapper()}). With both flags left at their {@code false} default this + * serializer emits byte-for-byte the same JSON as the previous default bean serialization. + */ +public class ChatUpdateMessageParamsSerializer + extends StdSerializer { + + private static final String CHANNEL_FIELD = "channel"; + private static final String TEXT_FIELD = "text"; + private static final String TS_FIELD = "ts"; + private static final String FILE_IDS_FIELD = "file_ids"; + private static final String AS_USER_FIELD = "as_user"; + private static final String LINK_NAMES_FIELD = "link_names"; + private static final String PARSE_FIELD = "parse"; + private static final String ATTACHMENTS_FIELD = "attachments"; + private static final String BLOCKS_FIELD = "blocks"; + + public ChatUpdateMessageParamsSerializer() { + super(ChatUpdateMessageParams.class); + } + + @Override + public void serialize( + ChatUpdateMessageParams params, + JsonGenerator gen, + SerializerProvider provider + ) throws IOException { + gen.writeStartObject(); + + // NON_EMPTY string fields: present on the wire only when non-empty. + writeNonEmptyString(gen, CHANNEL_FIELD, params.getChannelId()); + if (params.getText().isPresent() && !params.getText().get().isEmpty()) { + gen.writeStringField(TEXT_FIELD, params.getText().get()); + } + writeNonEmptyString(gen, TS_FIELD, params.getTs()); + + if (!params.getFileIds().isEmpty()) { + gen.writeArrayFieldStart(FILE_IDS_FIELD); + for (String fileId : params.getFileIds()) { + gen.writeString(fileId); + } + gen.writeEndArray(); + } + + // as_user carries a field-level @JsonInclude(NON_ABSENT): present only when set. + if (params.getAsUser().isPresent()) { + gen.writeBooleanField(AS_USER_FIELD, params.getAsUser().get()); + } + + // @Default fields that are never "empty" under NON_EMPTY, so always present today. + gen.writeBooleanField(LINK_NAMES_FIELD, params.shouldLinkNames()); + writeNonEmptyString(gen, PARSE_FIELD, params.getParse()); + + writeListOrClear( + gen, + ATTACHMENTS_FIELD, + params.getAttachments(), + params.shouldClearAttachments() + ); + writeListOrClear(gen, BLOCKS_FIELD, params.getBlocks(), params.shouldClearBlocks()); + + gen.writeEndObject(); + } + + private static void writeNonEmptyString( + JsonGenerator gen, + String fieldName, + String value + ) throws IOException { + if (value != null && !value.isEmpty()) { + gen.writeStringField(fieldName, value); + } + } + + /** + * Writes {@code fieldName} as a JSON array of the list's elements when the list is non-empty, or + * as an empty array {@code []} when {@code forceClear} is set (the Slack "clear this field" + * signal). When the list is empty and {@code forceClear} is {@code false} the field is omitted, + * matching {@code NON_EMPTY}. Each element is serialized through the active generator's codec so + * element-level custom serializers (e.g. {@link AttachmentSerializer}) still apply under whatever + * mapper is in use. + */ + private static void writeListOrClear( + JsonGenerator gen, + String fieldName, + List items, + boolean forceClear + ) throws IOException { + if (items.isEmpty() && !forceClear) { + return; + } + gen.writeArrayFieldStart(fieldName); + for (Object item : items) { + gen.writeObject(item); + } + gen.writeEndArray(); + } +} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java deleted file mode 100644 index 464a40c4..00000000 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializerModifier.java +++ /dev/null @@ -1,62 +0,0 @@ -package com.hubspot.slack.client.jackson; - -import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.SerializationConfig; -import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; -import com.fasterxml.jackson.databind.ser.BeanSerializerModifier; -import com.hubspot.slack.client.methods.params.chat.ChatUpdateMessageParams; -import java.util.List; - -/** - * Swaps the default {@code attachments} / {@code blocks} property writers on - * {@link ChatUpdateMessageParams} for {@link ForceEmitPropertyWriter}s driven by that type's - * {@code shouldClearAttachments()} / {@code shouldClearBlocks()} flags. Every other property keeps - * its default writer, so the only behavioral change is that an empty {@code attachments}/ - * {@code blocks} list is emitted as {@code []} (instead of being omitted) when the matching flag is - * set — the signal Slack's {@code chat.update} needs to clear that field. With the flags at their - * {@code false} default, serialization is unchanged. - */ -public class ChatUpdateMessageParamsSerializerModifier extends BeanSerializerModifier { - - private static final String ATTACHMENTS_PROPERTY = "attachments"; - private static final String BLOCKS_PROPERTY = "blocks"; - - @Override - public List changeProperties( - SerializationConfig config, - BeanDescription beanDesc, - List beanProperties - ) { - if (!ChatUpdateMessageParams.class.isAssignableFrom(beanDesc.getBeanClass())) { - return beanProperties; - } - - for (int i = 0; i < beanProperties.size(); i++) { - BeanPropertyWriter writer = beanProperties.get(i); - switch (writer.getName()) { - case ATTACHMENTS_PROPERTY: - beanProperties.set( - i, - new ForceEmitPropertyWriter( - writer, - bean -> ChatUpdateMessageParams.class.cast(bean).shouldClearAttachments() - ) - ); - break; - case BLOCKS_PROPERTY: - beanProperties.set( - i, - new ForceEmitPropertyWriter( - writer, - bean -> ChatUpdateMessageParams.class.cast(bean).shouldClearBlocks() - ) - ); - break; - default: - break; - } - } - - return beanProperties; - } -} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java deleted file mode 100644 index 853de217..00000000 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ForceEmitPropertyWriter.java +++ /dev/null @@ -1,59 +0,0 @@ -package com.hubspot.slack.client.jackson; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.SerializerProvider; -import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; -import java.io.Serializable; - -/** - * Wraps a single {@link BeanPropertyWriter}. When {@code forceEmit} tests {@code true} for the bean - * being serialized, the property is written even if its value is empty — bypassing the type's - * {@code @JsonInclude(NON_EMPTY)} suppression so the field appears as {@code []}. Otherwise it - * defers to the wrapped writer, which omits an empty collection exactly as before. - * - *

Used by {@link ChatUpdateMessageParamsSerializerModifier} to let {@code chat.update} actively - * clear {@code attachments}/{@code blocks}; every other property keeps its default writer. - */ -public class ForceEmitPropertyWriter extends BeanPropertyWriter { - - /** - * Decides, per bean instance, whether the wrapped property should be force-emitted. Declared - * {@link Serializable} because {@link BeanPropertyWriter} is itself serializable, so this field - * must not be left as unrestorable (transient) state. - */ - @FunctionalInterface - public interface ForceEmitPredicate extends Serializable { - boolean test(Object bean); - } - - private static final long SERIAL_VERSION_UID = 1L; - - private final ForceEmitPredicate forceEmit; - - public ForceEmitPropertyWriter(BeanPropertyWriter base, ForceEmitPredicate forceEmit) { - super(base); - this.forceEmit = forceEmit; - } - - @Override - public void serializeAsField(Object bean, JsonGenerator gen, SerializerProvider prov) - throws Exception { - if (!forceEmit.test(bean)) { - // Unchanged behavior: the wrapped writer applies NON_EMPTY and omits an empty collection. - super.serializeAsField(bean, gen, prov); - return; - } - - Object value = get(bean); - gen.writeFieldName(getName()); - if (value == null) { - prov.defaultSerializeNull(gen); - return; - } - JsonSerializer serializer = _serializer != null - ? _serializer - : prov.findValueSerializer(value.getClass(), this); - serializer.serialize(value, gen, prov); - } -} diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java index 89e95b43..546d47c9 100644 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java +++ b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ObjectMapperUtils.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.datatype.joda.JodaModule; @@ -26,12 +25,6 @@ private static ObjectMapper create() { mapper.registerModule(new Jdk8Module()); mapper.registerModule(new JavaTimeModule()); - SimpleModule chatUpdateClearFlagsModule = new SimpleModule(); - chatUpdateClearFlagsModule.setSerializerModifier( - new ChatUpdateMessageParamsSerializerModifier() - ); - mapper.registerModule(chatUpdateClearFlagsModule); - mapper.configure(JsonParser.Feature.ALLOW_COMMENTS, false); mapper.configure(JsonParser.Feature.AUTO_CLOSE_SOURCE, true); mapper.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, true); diff --git a/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java b/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java index 984d7afa..88665b63 100644 --- a/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java +++ b/slack-base/src/main/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsIF.java @@ -6,10 +6,12 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.PropertyNamingStrategies.SnakeCaseStrategy; import com.fasterxml.jackson.databind.annotation.JsonNaming; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.hubspot.immutables.style.HubSpotStyle; +import com.hubspot.slack.client.jackson.ChatUpdateMessageParamsSerializer; import java.util.Optional; import org.immutables.value.Value.Check; import org.immutables.value.Value.Default; @@ -19,6 +21,7 @@ @HubSpotStyle @JsonNaming(SnakeCaseStrategy.class) @JsonInclude(Include.NON_EMPTY) +@JsonSerialize(using = ChatUpdateMessageParamsSerializer.class) public interface ChatUpdateMessageParamsIF extends MessageParams { @JsonProperty("channel") String getChannelId(); diff --git a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java index 7339a88a..5b204f0c 100644 --- a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java +++ b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializationTest.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.guava.GuavaModule; +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.hubspot.slack.client.jackson.ObjectMapperUtils; import com.hubspot.slack.client.models.Attachment; import com.hubspot.slack.client.models.blocks.Divider; @@ -240,11 +242,9 @@ public void itDefaultsBothClearFlagsToFalse() { } /** - * The {@code BeanSerializerModifier} registered on the shared mapper must only rewrite - * {@link ChatUpdateMessageParams} writers. Serializing a different {@link MessageParams} type - * exercises that guard: if the modifier matched too broadly, {@code ForceEmitPropertyWriter}'s - * predicate would try to cast this bean to {@code ChatUpdateMessageParams} and throw, so a clean - * serialization proves the modifier left other types alone. + * The custom serializer is attached only to {@link ChatUpdateMessageParams} via + * {@code @JsonSerialize}, so a different {@link MessageParams} type is unaffected and keeps its + * own serialization rules. */ @Test public void itDoesNotAffectOtherMessageParamTypes() { @@ -256,10 +256,78 @@ public void itDoesNotAffectOtherMessageParamTypes() { JsonNode json = OBJECT_MAPPER.valueToTree(postParams); - // ChatPostMessageParams keeps empty collections by its own (unchanged) rules — the point is - // simply that serialization succeeds and our clear-flag behavior did not leak onto it. assertThat(json.get("channel").asText()).isEqualTo("C123"); assertThat(json.has("should_clear_attachments")).isFalse(); assertThat(json.has("should_clear_blocks")).isFalse(); } + + /** + * The whole reason for {@code @JsonSerialize} on the type (rather than a mapper-level modifier): + * consumers serialize request bodies with their own {@link ObjectMapper}, not + * {@link ObjectMapperUtils#mapper()}. A foreign mapper with no slack-specific configuration must + * still honor the clear flags and emit {@code []}, otherwise the field never gets cleared and the + * message double-renders. This mirrors how slack-integration serializes (its own mapper). + */ + @Test + public void itHonorsClearFlagsUnderAForeignObjectMapper() { + ObjectMapper foreignMapper = new ObjectMapper() + .registerModule(new Jdk8Module()) + .registerModule(new GuavaModule()); + + JsonNode json = foreignMapper.valueToTree( + ChatUpdateMessageParams + .builder() + .setChannelId("C123") + .setTs("1790587670.603089") + .setText("testText") + .setShouldClearBlocks(true) + .build() + ); + + assertThat(json.get("blocks").isArray()).isTrue(); + assertThat(json.get("blocks")).isEmpty(); + assertThat(json.has("attachments")).isFalse(); + } + + @Test + public void itHonorsClearAttachmentsUnderAForeignObjectMapper() { + ObjectMapper foreignMapper = new ObjectMapper() + .registerModule(new Jdk8Module()) + .registerModule(new GuavaModule()); + + JsonNode json = foreignMapper.valueToTree( + ChatUpdateMessageParams + .builder() + .setChannelId("C123") + .setTs("1790587670.603089") + .setText("testText") + .setShouldClearAttachments(true) + .build() + ); + + assertThat(json.get("attachments").isArray()).isTrue(); + assertThat(json.get("attachments")).isEmpty(); + assertThat(json.has("blocks")).isFalse(); + } + + /** + * Mirrors {@code NON_EMPTY} for the Optional {@code text} field: a present-but-empty string is + * omitted, not written as {@code ""}. A block is included so the {@code @Check} (text or + * attachments or blocks required) still passes. + */ + @Test + public void itOmitsEmptyTextString() { + ChatUpdateMessageParams params = ChatUpdateMessageParams + .builder() + .setChannelId("C123") + .setTs("1790587670.603089") + .setText("") + .addBlocks(Divider.builder().build()) + .build(); + + JsonNode json = OBJECT_MAPPER.valueToTree(params); + + assertThat(json.has("text")).isFalse(); + assertThat(json.get("blocks")).hasSize(1); + } } From 02f49eff7c16631be18463760807996be3934ab4 Mon Sep 17 00:00:00 2001 From: Yurii Yakhnytsia Date: Thu, 15 Oct 2026 10:09:18 +0300 Subject: [PATCH 3/4] Add unit tests to avoid the risk of missing props with custom deserializer for ChatUpdateMessageParams --- ...geParamsSerializerDriftReflectionTest.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java diff --git a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java new file mode 100644 index 00000000..c88ff2c6 --- /dev/null +++ b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java @@ -0,0 +1,54 @@ +package com.hubspot.slack.client.methods.params.chat; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.BeanDescription; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.hubspot.slack.client.jackson.ObjectMapperUtils; +import com.hubspot.slack.client.models.Attachment; +import com.hubspot.slack.client.models.blocks.Divider; +import org.junit.Test; + +/** + * Stronger drift guard than {@code itSerializesEveryPopulatedField}: instead of asserting a fixed + * list of fields, it derives the set of serializable properties from the type via Jackson + * introspection ({@code @JsonIgnore} properties — the shouldClear* flags — are excluded) and + * asserts the hand-written {@code ChatUpdateMessageParamsSerializer} emits exactly that many fields + * for a fully-populated instance. Add a getter to ChatUpdateMessageParamsIF (or MessageParams) + * without updating the serializer and this fails, where a by-hand test would not. + */ +public class ChatUpdateMessageParamsSerializerDriftReflectionTest { + + private static final ObjectMapper OBJECT_MAPPER = ObjectMapperUtils.mapper(); + + @Test + public void itSerializesEveryNonIgnoredProperty() { + JavaType type = OBJECT_MAPPER.constructType(ChatUpdateMessageParams.class); + BeanDescription description = OBJECT_MAPPER.getSerializationConfig().introspect(type); + int declaredSerializableProperties = description.findProperties().size(); + + ChatUpdateMessageParams fullyPopulated = ChatUpdateMessageParams + .builder() + .setChannelId("C123") + .setTs("1790587670.603089") + .setText("testText") + .addFileIds("F0C4SK2P1EZ") + .setAsUser(true) + .setShouldLinkNames(false) + .setParse("full") + .addAttachments(Attachment.builder().setText("attachment").build()) + .addBlocks(Divider.builder().build()) + .build(); + + JsonNode json = OBJECT_MAPPER.valueToTree(fullyPopulated); + + assertThat(json.size()) + .as( + "ChatUpdateMessageParamsSerializer must emit one field per non-@JsonIgnore property; " + + "a mismatch means a property was added to the type but not to the serializer" + ) + .isEqualTo(declaredSerializableProperties); + } +} From 0278731326ca7941be9e0972d8dc97aec5d04a5d Mon Sep 17 00:00:00 2001 From: Yurii Yakhnytsia Date: Fri, 16 Oct 2026 09:32:02 +0300 Subject: [PATCH 4/4] Refactor deserializer, extract test constants. --- .../ChatUpdateMessageParamsSerializer.java | 22 ++++++++++++------- .../hubspot/slack/client/TestConstants.java | 10 +++++++++ ...geParamsSerializerDriftReflectionTest.java | 15 ++++++++----- 3 files changed, 34 insertions(+), 13 deletions(-) create mode 100644 slack-base/src/test/java/com/hubspot/slack/client/TestConstants.java diff --git a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java index d47cd7f3..410bbc7b 100644 --- a/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java +++ b/slack-base/src/main/java/com/hubspot/slack/client/jackson/ChatUpdateMessageParamsSerializer.java @@ -59,14 +59,7 @@ public void serialize( writeNonEmptyString(gen, CHANNEL_FIELD, params.getChannelId()); writeNonEmptyString(gen, TEXT_FIELD, params.getText().orElse(null)); writeNonEmptyString(gen, TS_FIELD, params.getTs()); - - if (!params.getFileIds().isEmpty()) { - gen.writeArrayFieldStart(FILE_IDS_FIELD); - for (String fileId : params.getFileIds()) { - gen.writeString(fileId); - } - gen.writeEndArray(); - } + writeNonEmptyFileIds(params, gen); // as_user carries a field-level @JsonInclude(NON_ABSENT): present only when set. if (params.getAsUser().isPresent()) { @@ -95,6 +88,19 @@ public void serialize( gen.writeEndObject(); } + private static void writeNonEmptyFileIds( + ChatUpdateMessageParams params, + JsonGenerator gen + ) throws IOException { + if (!params.getFileIds().isEmpty()) { + gen.writeArrayFieldStart(FILE_IDS_FIELD); + for (String fileId : params.getFileIds()) { + gen.writeString(fileId); + } + gen.writeEndArray(); + } + } + private static void writeNonEmptyString( JsonGenerator gen, String fieldName, diff --git a/slack-base/src/test/java/com/hubspot/slack/client/TestConstants.java b/slack-base/src/test/java/com/hubspot/slack/client/TestConstants.java new file mode 100644 index 00000000..b4b742b1 --- /dev/null +++ b/slack-base/src/test/java/com/hubspot/slack/client/TestConstants.java @@ -0,0 +1,10 @@ +package com.hubspot.slack.client; + +public class TestConstants { + + public static final String TEST_CHANNEL_ID = "C123"; + public static final String TEST_MESSAGE_TEXT = "The test message text"; + public static final String TEST_MESSAGE_TS = "1790587670.603089"; + public static final String TEST_FILE_ID = "F0C4SK2P1EZ"; + public static final String TEST_ATTACHMENTS_TEXT = "attachments text"; +} diff --git a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java index c88ff2c6..d9c7f7eb 100644 --- a/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java +++ b/slack-base/src/test/java/com/hubspot/slack/client/methods/params/chat/ChatUpdateMessageParamsSerializerDriftReflectionTest.java @@ -1,5 +1,10 @@ package com.hubspot.slack.client.methods.params.chat; +import static com.hubspot.slack.client.TestConstants.TEST_ATTACHMENTS_TEXT; +import static com.hubspot.slack.client.TestConstants.TEST_CHANNEL_ID; +import static com.hubspot.slack.client.TestConstants.TEST_FILE_ID; +import static com.hubspot.slack.client.TestConstants.TEST_MESSAGE_TEXT; +import static com.hubspot.slack.client.TestConstants.TEST_MESSAGE_TS; import static org.assertj.core.api.Assertions.assertThat; import com.fasterxml.jackson.databind.BeanDescription; @@ -31,14 +36,14 @@ public void itSerializesEveryNonIgnoredProperty() { ChatUpdateMessageParams fullyPopulated = ChatUpdateMessageParams .builder() - .setChannelId("C123") - .setTs("1790587670.603089") - .setText("testText") - .addFileIds("F0C4SK2P1EZ") + .setChannelId(TEST_CHANNEL_ID) + .setTs(TEST_MESSAGE_TS) + .setText(TEST_MESSAGE_TEXT) + .addFileIds(TEST_FILE_ID) .setAsUser(true) .setShouldLinkNames(false) .setParse("full") - .addAttachments(Attachment.builder().setText("attachment").build()) + .addAttachments(Attachment.builder().setText(TEST_ATTACHMENTS_TEXT).build()) .addBlocks(Divider.builder().build()) .build();