Skip to content

Commit afdd3ef

Browse files
committed
Made handling of merge_request event more resilient (#332).
1 parent 74d9394 commit afdd3ef

File tree

1 file changed

+29
-20
lines changed

1 file changed

+29
-20
lines changed

src/main/java/org/gitlab4j/api/systemhooks/SystemHookManager.java

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
public class SystemHookManager extends HookManager {
2525

2626
public static final String SYSTEM_HOOK_EVENT = "System Hook";
27-
2827
private final static Logger LOGGER = GitLabApi.getLogger();
2928
private final JacksonJson jacksonJson = new JacksonJson();
3029

@@ -77,10 +76,11 @@ public void handleEvent(HttpServletRequest request) throws GitLabApiException {
7776
throw new GitLabApiException(message);
7877
}
7978

79+
// Get the JSON as a JsonNode tree. We do not directly unmarshal the input as special handling must
80+
// be done for "merge_request" events.
81+
JsonNode tree;
8082
try {
8183

82-
SystemHookEvent event;
83-
JsonNode tree;
8484
if (LOGGER.isLoggable(Level.FINE)) {
8585
LOGGER.fine(HttpRequestUtils.getShortRequestDump("System Hook", true, request));
8686
String postData = HttpRequestUtils.getPostDataAsString(request);
@@ -91,30 +91,39 @@ public void handleEvent(HttpServletRequest request) throws GitLabApiException {
9191
tree = jacksonJson.readTree(reader);
9292
}
9393

94-
// NOTE: This is a hack based on the GitLab documentation showing that the "event_name" property
95-
// is missing from the merge_request system hook event
96-
if (!tree.has("event_name") && tree.has("object_kind")) {
97-
String objectKind = tree.asText("object_kind");
98-
switch (objectKind) {
99-
case MergeRequestSystemHookEvent.MERGE_REQUEST_EVENT:
100-
ObjectNode node = (ObjectNode)tree;
101-
node.put("event_name", MergeRequestSystemHookEvent.MERGE_REQUEST_EVENT);
102-
break;
103-
104-
default:
105-
String message = "Unsupported object_kind, object_kind=" + objectKind;
106-
LOGGER.warning(message);
107-
throw new GitLabApiException(message);
108-
}
94+
} catch (Exception e) {
95+
LOGGER.warning("Error reading JSON data, exception=" +
96+
e.getClass().getSimpleName() + ", error=" + e.getMessage());
97+
throw new GitLabApiException(e);
98+
}
99+
100+
// NOTE: This is a hack based on the GitLab documentation and actual content of the "merge_request" event
101+
// showing that the "event_name" property is missing from the merge_request system hook event. The hack is
102+
// to inject the "event_name" node so that the polymorphic deserialization of a SystemHookEvent works correctly
103+
// when the system hook event is a "merge_request" event.
104+
if (!tree.has("event_name") && tree.has("object_kind")) {
105+
106+
String objectKind = tree.get("object_kind").asText();
107+
if (MergeRequestSystemHookEvent.MERGE_REQUEST_EVENT.equals(objectKind)) {
108+
ObjectNode node = (ObjectNode)tree;
109+
node.put("event_name", MergeRequestSystemHookEvent.MERGE_REQUEST_EVENT);
110+
} else {
111+
String message = "Unsupported object_kind for system hook event, object_kind=" + objectKind;
112+
LOGGER.warning(message);
113+
throw new GitLabApiException(message);
109114
}
115+
}
110116

111-
event = jacksonJson.unmarshal(SystemHookEvent.class, tree);
117+
// Unmarshal the tree to a concrete instance of a SystemHookEvent and fire the event to any listeners
118+
try {
112119

120+
SystemHookEvent event = jacksonJson.unmarshal(SystemHookEvent.class, tree);
113121
if (LOGGER.isLoggable(Level.FINE)) {
114122
LOGGER.fine(event.getEventName() + "\n" + jacksonJson.marshal(event) + "\n");
115123
}
116124

117-
event.setRequestUrl(request.getRequestURL().toString());
125+
StringBuffer requestUrl = request.getRequestURL();
126+
event.setRequestUrl(requestUrl != null ? requestUrl.toString() : null);
118127
event.setRequestQueryString(request.getQueryString());
119128
fireEvent(event);
120129

0 commit comments

Comments
 (0)