Skip to content

Commit 6184bb6

Browse files
WilliamBergaminseratch
WilliamBergamin
and
seratch
authored
Arraylist<LayoutBlock> GSON anonymous inner class initialization bug (#1004)
* Added fix for GSON issue with anonymous inner class initialization * Added handling for Map GSON has a known issue where null is returned if it tries to serialize a list of object that was initialized using an anonymous inner class. This commit tried to patch this. Co-authored-by: Kazuhiro Sera <[email protected]>
1 parent 716a14d commit 6184bb6

File tree

2 files changed

+220
-21
lines changed

2 files changed

+220
-21
lines changed

‎slack-api-client/src/main/java/com/slack/api/methods/RequestFormBuilder.java‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,15 @@
108108
import com.slack.api.methods.request.workflows.WorkflowsUpdateStepRequest;
109109
import com.slack.api.model.Attachment;
110110
import com.slack.api.model.ConversationType;
111+
import com.slack.api.model.block.LayoutBlock;
111112
import com.slack.api.util.json.GsonFactory;
112113
import lombok.extern.slf4j.Slf4j;
113114
import okhttp3.FormBody;
114115
import okhttp3.MediaType;
115116
import okhttp3.MultipartBody;
116117
import okhttp3.RequestBody;
117118

118-
import java.util.ArrayList;
119-
import java.util.Arrays;
120-
import java.util.List;
119+
import java.util.*;
121120

122121
import static java.util.stream.Collectors.joining;
123122

@@ -922,7 +921,7 @@ public static FormBody.Builder toForm(CallsAddRequest req) {
922921
setIfNotNull("external_display_id", req.getExternalDisplayId(), form);
923922
setIfNotNull("title", req.getTitle(), form);
924923
if (req.getUsers() != null && req.getUsers().size() > 0) {
925-
String usersJson = GSON.toJson(req.getUsers());
924+
String usersJson = getJsonWithGsonAnonymInnerClassHandling(req.getUsers());
926925
setIfNotNull("users", usersJson, form);
927926
}
928927
return form;
@@ -954,7 +953,7 @@ public static FormBody.Builder toForm(CallsParticipantsAddRequest req) {
954953
FormBody.Builder form = new FormBody.Builder();
955954
setIfNotNull("id", req.getId(), form);
956955
if (req.getUsers() != null && req.getUsers().size() > 0) {
957-
String usersJson = GSON.toJson(req.getUsers());
956+
String usersJson = getJsonWithGsonAnonymInnerClassHandling(req.getUsers());
958957
setIfNotNull("users", usersJson, form);
959958
}
960959
return form;
@@ -964,7 +963,7 @@ public static FormBody.Builder toForm(CallsParticipantsRemoveRequest req) {
964963
FormBody.Builder form = new FormBody.Builder();
965964
setIfNotNull("id", req.getId(), form);
966965
if (req.getUsers() != null && req.getUsers().size() > 0) {
967-
String usersJson = GSON.toJson(req.getUsers());
966+
String usersJson = getJsonWithGsonAnonymInnerClassHandling(req.getUsers());
968967
setIfNotNull("users", usersJson, form);
969968
}
970969
return form;
@@ -1134,7 +1133,7 @@ public static FormBody.Builder toForm(ChatScheduleMessageRequest req) {
11341133
if (req.getBlocksAsString() != null) {
11351134
form.add("blocks", req.getBlocksAsString());
11361135
} else if (req.getBlocks() != null) {
1137-
String json = GSON.toJson(req.getBlocks());
1136+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getBlocks());
11381137
form.add("blocks", json);
11391138
}
11401139
if (req.getBlocksAsString() != null && req.getBlocks() != null) {
@@ -1144,7 +1143,7 @@ public static FormBody.Builder toForm(ChatScheduleMessageRequest req) {
11441143
if (req.getAttachmentsAsString() != null) {
11451144
form.add("attachments", req.getAttachmentsAsString());
11461145
} else if (req.getAttachments() != null) {
1147-
String json = GSON.toJson(req.getAttachments());
1146+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getAttachments());
11481147
form.add("attachments", json);
11491148
}
11501149
setIfNotNull("link_names", req.isLinkNames(), form);
@@ -1183,7 +1182,7 @@ public static FormBody.Builder toForm(ChatPostEphemeralRequest req) {
11831182
if (req.getBlocksAsString() != null) {
11841183
form.add("blocks", req.getBlocksAsString());
11851184
} else if (req.getBlocks() != null) {
1186-
String json = GSON.toJson(req.getBlocks());
1185+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getBlocks());
11871186
form.add("blocks", json);
11881187
}
11891188
if (req.getBlocksAsString() != null && req.getBlocks() != null) {
@@ -1193,7 +1192,7 @@ public static FormBody.Builder toForm(ChatPostEphemeralRequest req) {
11931192
if (req.getAttachmentsAsString() != null) {
11941193
form.add("attachments", req.getAttachmentsAsString());
11951194
} else if (req.getAttachments() != null) {
1196-
String json = GSON.toJson(req.getAttachments());
1195+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getAttachments());
11971196
form.add("attachments", json);
11981197
}
11991198
setIfNotNull("thread_ts", req.getThreadTs(), form);
@@ -1229,7 +1228,7 @@ public static FormBody.Builder toForm(ChatPostMessageRequest req) {
12291228
if (req.getBlocksAsString() != null) {
12301229
form.add("blocks", req.getBlocksAsString());
12311230
} else if (req.getBlocks() != null) {
1232-
String json = GSON.toJson(req.getBlocks());
1231+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getBlocks());
12331232
form.add("blocks", json);
12341233
}
12351234
if (req.getBlocksAsString() != null && req.getBlocks() != null) {
@@ -1239,7 +1238,7 @@ public static FormBody.Builder toForm(ChatPostMessageRequest req) {
12391238
if (req.getAttachmentsAsString() != null) {
12401239
form.add("attachments", req.getAttachmentsAsString());
12411240
} else if (req.getAttachments() != null) {
1242-
String json = GSON.toJson(req.getAttachments());
1241+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getAttachments());
12431242
form.add("attachments", json);
12441243
}
12451244
setIfNotNull("unfurl_links", req.isUnfurlLinks(), form);
@@ -1279,7 +1278,7 @@ public static FormBody.Builder toForm(ChatUpdateRequest req) {
12791278
if (req.getBlocksAsString() != null) {
12801279
form.add("blocks", req.getBlocksAsString());
12811280
} else if (req.getBlocks() != null) {
1282-
String json = GSON.toJson(req.getBlocks());
1281+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getBlocks());
12831282
form.add("blocks", json);
12841283
}
12851284
if (req.getBlocksAsString() != null && req.getBlocks() != null) {
@@ -1289,7 +1288,7 @@ public static FormBody.Builder toForm(ChatUpdateRequest req) {
12891288
if (req.getAttachmentsAsString() != null) {
12901289
form.add("attachments", req.getAttachmentsAsString());
12911290
} else if (req.getAttachments() != null) {
1292-
String json = GSON.toJson(req.getAttachments());
1291+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getAttachments());
12931292
form.add("attachments", json);
12941293
}
12951294
setIfNotNull("as_user", req.isAsUser(), form);
@@ -1303,16 +1302,16 @@ public static FormBody.Builder toForm(ChatUnfurlRequest req) {
13031302
setIfNotNull("channel", req.getChannel(), form);
13041303
if (req.getRawUnfurls() != null) {
13051304
setIfNotNull("unfurls", req.getRawUnfurls(), form);
1306-
} else {
1307-
String json = GSON.toJson(req.getUnfurls());
1305+
} else if (req.getUnfurls() != null) {
1306+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getUnfurls());
13081307
setIfNotNull("unfurls", json, form);
13091308
}
13101309
setIfNotNull("user_auth_required", req.isUserAuthRequired(), form);
13111310
setIfNotNull("user_auth_message", req.getUserAuthMessage(), form);
13121311
if (req.getRawUserAuthBlocks() != null) {
13131312
setIfNotNull("user_auth_blocks", req.getRawUserAuthBlocks(), form);
13141313
} else if (req.getUserAuthBlocks() != null) {
1315-
String json = GSON.toJson(req.getUserAuthBlocks());
1314+
String json = getJsonWithGsonAnonymInnerClassHandling(req.getUserAuthBlocks());
13161315
setIfNotNull("user_auth_blocks", json, form);
13171316
}
13181317
setIfNotNull("user_auth_url", req.getUserAuthUrl(), form);
@@ -2455,15 +2454,19 @@ public static FormBody.Builder toForm(WorkflowsStepCompletedRequest req) {
24552454
if (req.getOutputsAsString() != null) {
24562455
setIfNotNull("outputs", req.getOutputsAsString(), form);
24572456
} else if (req.getOutputs() != null) {
2458-
setIfNotNull("outputs", GSON.toJson(req.getOutputs()), form);
2457+
setIfNotNull("outputs",
2458+
getJsonWithGsonAnonymInnerClassHandling(req.getOutputs()),
2459+
form);
24592460
}
24602461
return form;
24612462
}
24622463

24632464
public static FormBody.Builder toForm(WorkflowsStepFailedRequest req) {
24642465
FormBody.Builder form = new FormBody.Builder();
24652466
setIfNotNull("workflow_step_execute_id", req.getWorkflowStepExecuteId(), form);
2466-
setIfNotNull("error", GSON.toJson(req.getError()), form);
2467+
setIfNotNull("error",
2468+
getJsonWithGsonAnonymInnerClassHandling(req.getError()),
2469+
form);
24672470
return form;
24682471
}
24692472

@@ -2475,12 +2478,16 @@ public static FormBody.Builder toForm(WorkflowsUpdateStepRequest req) {
24752478
if (req.getInputsAsString() != null) {
24762479
setIfNotNull("inputs", req.getInputsAsString(), form);
24772480
} else if (req.getInputs() != null) {
2478-
setIfNotNull("inputs", GSON.toJson(req.getInputs()), form);
2481+
setIfNotNull("inputs",
2482+
getJsonWithGsonAnonymInnerClassHandling(req.getInputs()),
2483+
form);
24792484
}
24802485
if (req.getOutputsAsString() != null) {
24812486
setIfNotNull("outputs", req.getOutputsAsString(), form);
24822487
} else if (req.getOutputs() != null) {
2483-
setIfNotNull("outputs", GSON.toJson(req.getOutputs()), form);
2488+
setIfNotNull("outputs",
2489+
getJsonWithGsonAnonymInnerClassHandling(req.getOutputs()),
2490+
form);
24842491
}
24852492
return form;
24862493
}
@@ -2495,6 +2502,8 @@ public static FormBody.Builder toForm(WorkflowsUpdateStepRequest req) {
24952502
private static final String FALLBACK_WARN_MESSAGE_TEMPLATE =
24962503
"Additionally, the attachment-level `fallback` argument is missing in the request payload for a {} call - To avoid this warning, it is recommended to always provide a top-level `text` argument when posting a message. Alternatively, you can provide an attachment-level `fallback` argument, though this is now considered a legacy field (see https://api.slack.com/reference/messaging/attachments#legacy_fields for more details).";
24972504

2505+
private static final String GSON_ANONYM_INNER_CLASS_INIT_OUTPUT = "null";
2506+
24982507
private static void warnIfAttachmentWithoutFallbackDetected(String endpointName, List<Attachment> attachments) {
24992508
boolean fallbackMissing = false;
25002509
for (Attachment a : attachments) {
@@ -2553,4 +2562,16 @@ private static void setIfNotNull(String name, Object value, MultipartBody.Builde
25532562
}
25542563
}
25552564

2565+
// Workarounds to solve GSON not handling anonymous inner class object initialization
2566+
// https://github.com/google/gson/issues/2023
2567+
private static <T> String getJsonWithGsonAnonymInnerClassHandling(Map<String, T> stringTMap){
2568+
String json = GSON.toJson(stringTMap);
2569+
return GSON_ANONYM_INNER_CLASS_INIT_OUTPUT.equals(json) ? GSON.toJson(new HashMap<>(stringTMap)) : json;
2570+
}
2571+
2572+
private static <T> String getJsonWithGsonAnonymInnerClassHandling(List<T> tList){
2573+
String json = GSON.toJson(tList);
2574+
return GSON_ANONYM_INNER_CLASS_INIT_OUTPUT.equals(json) ? GSON.toJson(new ArrayList<>(tList)) : json;
2575+
}
2576+
25562577
}

‎slack-api-client/src/test/java/test_locally/api/methods/RequestFormBuilderTest.java‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
package test_locally.api.methods;
2+
3+
import com.slack.api.methods.RequestFormBuilder;
4+
import com.slack.api.methods.request.calls.CallsAddRequest;
5+
import com.slack.api.methods.request.chat.ChatPostMessageRequest;
6+
import com.slack.api.methods.request.chat.ChatUnfurlRequest;
7+
import com.slack.api.model.Attachment;
8+
import com.slack.api.model.CallParticipant;
9+
import com.slack.api.model.block.LayoutBlock;
10+
import lombok.AllArgsConstructor;
11+
import lombok.Builder;
12+
import lombok.Data;
13+
import lombok.NoArgsConstructor;
14+
import okhttp3.FormBody;
15+
import org.junit.Test;
16+
17+
import java.util.ArrayList;
18+
import java.util.HashMap;
19+
import java.util.List;
20+
import java.util.Map;
21+
22+
import static org.hamcrest.CoreMatchers.is;
23+
import static org.hamcrest.MatcherAssert.assertThat;
24+
25+
public class RequestFormBuilderTest {
26+
27+
28+
@Test
29+
public void testBlocksJsonSerialization() {
30+
// GIVEN
31+
List<LayoutBlock> blocks = new ArrayList<>();
32+
blocks.add(MyTestBlock.builder().build());
33+
34+
ChatPostMessageRequest chatPostMessageRequest = ChatPostMessageRequest.builder().build();
35+
chatPostMessageRequest.setBlocks(blocks);
36+
37+
// WHEN
38+
FormBody form = RequestFormBuilder.toForm(chatPostMessageRequest).build();
39+
40+
// THEN
41+
final int blocksIndexInForm = 2;
42+
assertThat(form.name(blocksIndexInForm), is("blocks"));
43+
assertThat(form.value(blocksIndexInForm), is("[{\"type\":\"myTestBlock\"}]"));
44+
}
45+
46+
@Test
47+
public void testEmptyListJsonSerialization() {
48+
// GIVEN
49+
List<LayoutBlock> blocks = new ArrayList<>();
50+
51+
ChatPostMessageRequest chatPostMessageRequest = ChatPostMessageRequest.builder().build();
52+
chatPostMessageRequest.setBlocks(blocks);
53+
54+
// WHEN
55+
FormBody form = RequestFormBuilder.toForm(chatPostMessageRequest).build();
56+
57+
// THEN
58+
final int blocksIndexInForm = 2;
59+
assertThat(form.name(blocksIndexInForm), is("blocks"));
60+
assertThat(form.value(blocksIndexInForm), is("[]"));
61+
}
62+
63+
@Test
64+
public void testBlocksJsonSerializationWithInnerClassInit() {
65+
// GIVEN
66+
ChatPostMessageRequest chatPostMessageRequest = ChatPostMessageRequest.builder().build();
67+
chatPostMessageRequest.setBlocks(new ArrayList<LayoutBlock>() {{
68+
add(MyTestBlock.builder().build());
69+
}});
70+
71+
// WHEN
72+
FormBody form = RequestFormBuilder.toForm(chatPostMessageRequest).build();
73+
74+
// THEN
75+
final int blocksIndexInForm = 2;
76+
assertThat(form.name(blocksIndexInForm), is("blocks"));
77+
assertThat(form.value(blocksIndexInForm), is("[{\"type\":\"myTestBlock\"}]"));
78+
}
79+
80+
@Test
81+
public void testAttachmentsJsonSerializationWithInnerClassInit() {
82+
// GIVEN
83+
ChatPostMessageRequest chatPostMessageRequest = ChatPostMessageRequest.builder().build();
84+
chatPostMessageRequest.setAttachments(new ArrayList<Attachment>() {{
85+
add(Attachment.builder().build());
86+
}});
87+
88+
// WHEN
89+
FormBody form = RequestFormBuilder.toForm(chatPostMessageRequest).build();
90+
91+
// THEN
92+
final int attachmentIndexInForm = 2;
93+
assertThat(form.name(attachmentIndexInForm), is("attachments"));
94+
assertThat(form.value(attachmentIndexInForm), is("[{}]"));
95+
}
96+
97+
@Test
98+
public void testUserAuthBlocksJsonSerializationWithInnerClassInit() {
99+
// GIVEN
100+
ChatUnfurlRequest chatUnfurlRequest = ChatUnfurlRequest.builder().build();
101+
chatUnfurlRequest.setUserAuthBlocks(new ArrayList<LayoutBlock>() {{
102+
add(MyTestBlock.builder().build());
103+
}});
104+
105+
// WHEN
106+
FormBody form = RequestFormBuilder.toForm(chatUnfurlRequest).build();
107+
108+
// THEN
109+
final int userAuthBlocksIndexInFrom = 1;
110+
assertThat(form.name(userAuthBlocksIndexInFrom), is("user_auth_blocks"));
111+
assertThat(form.value(userAuthBlocksIndexInFrom), is("[{\"type\":\"myTestBlock\"}]"));
112+
}
113+
114+
@Test
115+
public void testUsersJsonSerializationWithInnerClassInit() {
116+
// GIVEN
117+
CallParticipant participant = CallParticipant.builder().build();
118+
participant.setDisplayName("Bill");
119+
120+
CallsAddRequest callsAddRequest = CallsAddRequest.builder().build();
121+
callsAddRequest.setUsers(new ArrayList<CallParticipant>() {{
122+
add(participant);
123+
}});
124+
125+
// WHEN
126+
FormBody form = RequestFormBuilder.toForm(callsAddRequest).build();
127+
128+
// THEN
129+
final int usersIndexInFrom = 0;
130+
assertThat(form.name(usersIndexInFrom), is("users"));
131+
assertThat(form.value(usersIndexInFrom), is("[{\"display_name\":\"Bill\"}]"));
132+
}
133+
134+
@Test
135+
public void testUnfurlsJsonSerializationWithInnerClassInit() {
136+
// GIVEN
137+
ChatUnfurlRequest chatUnfurlRequest = ChatUnfurlRequest.builder().build();
138+
chatUnfurlRequest.setUnfurls(new HashMap<String, ChatUnfurlRequest.UnfurlDetail>() {{
139+
put("key", ChatUnfurlRequest.UnfurlDetail.builder().build());
140+
}});
141+
142+
// WHEN
143+
FormBody form = RequestFormBuilder.toForm(chatUnfurlRequest).build();
144+
145+
// THEN
146+
final int userAuthBlocksIndexInFrom = 0;
147+
assertThat(form.name(userAuthBlocksIndexInFrom), is("unfurls"));
148+
assertThat(form.value(userAuthBlocksIndexInFrom), is("{\"key\":{}}"));
149+
}
150+
151+
@Test
152+
public void testUnfurlsJsonSerialization() {
153+
// GIVEN
154+
Map<String, ChatUnfurlRequest.UnfurlDetail> unfurls = new HashMap<>();
155+
unfurls.put("key", ChatUnfurlRequest.UnfurlDetail.builder().build());
156+
157+
ChatUnfurlRequest chatUnfurlRequest = ChatUnfurlRequest.builder().build();
158+
chatUnfurlRequest.setUnfurls(unfurls);
159+
160+
// WHEN
161+
FormBody form = RequestFormBuilder.toForm(chatUnfurlRequest).build();
162+
163+
// THEN
164+
final int userAuthBlocksIndexInFrom = 0;
165+
assertThat(form.name(userAuthBlocksIndexInFrom), is("unfurls"));
166+
assertThat(form.value(userAuthBlocksIndexInFrom), is("{\"key\":{}}"));
167+
}
168+
169+
@Data
170+
@Builder
171+
@NoArgsConstructor
172+
@AllArgsConstructor
173+
private static class MyTestBlock implements LayoutBlock {
174+
public static final String TYPE = "myTestBlock";
175+
private final String type = TYPE;
176+
private String blockId;
177+
}
178+
}

0 commit comments

Comments
 (0)