Use custom deserializer for Event class

Using newer streaming versions of Gson causes deserialization
problems if the payload is before the type field in the returned
JSON using the current custom Event formatting.

This changes the behavior to use an internal custom Gson instance
just to handle parsing events that allows a chance to check the
type and create the custom payload before the Event object is
returned to any service callers.

This approach simplifies the code but does require an additional
Gson instance to be kept around to handle the default serialization
of the top-level Event fields.

Change-Id: Ic10a755bbf798ec2d1d4a4c53ccd9154e0cb91e2

3 files changed, 98 insertions, 103 deletions

diff --git a/org.eclipse.egit.github.core.tests/src/org/eclipse/egit/github/core/tests/EventFormatterTest.java b/org.eclipse.egit.github.core.tests/src/org/eclipse/egit/github/core/tests/EventFormatterTest.java
index d507f30d..0777f58d 100644
--- a/org.eclipse.egit.github.core.tests/src/org/eclipse/egit/github/core/tests/EventFormatterTest.java
+++ b/org.eclipse.egit.github.core.tests/src/org/eclipse/egit/github/core/tests/EventFormatterTest.java
@@ -10,45 +10,65 @@
  *******************************************************************************/
 package org.eclipse.egit.github.core.tests;
 
+import static org.eclipse.egit.github.core.event.Event.TYPE_FOLLOW;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNull;
 
 import org.eclipse.egit.github.core.client.EventFormatter;
+import org.eclipse.egit.github.core.client.GsonUtils;
+import org.eclipse.egit.github.core.event.Event;
 import org.eclipse.egit.github.core.event.EventPayload;
+import org.eclipse.egit.github.core.event.FollowPayload;
 import org.junit.Test;
 
 /**
- * Unit tests of {@link EventFormatter} and subclasses
+ * Unit tests of {@link EventFormatter}
  */
 public class EventFormatterTest {
 
 	/**
-	 * Create instance of EventFormatter
+	 * Follow event payload returned as {@link FollowPayload}
 	 */
 	@Test
-	public void createEventFormatterInstance() {
-		EventFormatter formatter = new EventFormatter();
-		assertNotNull(formatter.getEventCreator());
-		assertNotNull(formatter.getPayloadDeserializer());
+	public void followPayload() {
+		Event event = GsonUtils.fromJson("{\"type\":\"" + TYPE_FOLLOW
+				+ "\",\"payload\":{}}", Event.class);
+		assertNotNull(event);
+		assertNotNull(event.getPayload());
+		assertEquals(FollowPayload.class, event.getPayload().getClass());
 	}
 
 	/**
-	 * Create instance of Event
+	 * Unknown event payload returned as {@link EventPayload}
 	 */
 	@Test
-	public void createEventInstance() {
-		EventFormatter formatter = new EventFormatter();
-		assertNotNull(formatter.getEventCreator().createInstance(null));
+	public void unknownPayload() {
+		Event event = GsonUtils.fromJson(
+				"{\"type\":\"NotAnEventType\",\"payload\":{}}", Event.class);
+		assertNotNull(event);
+		assertNotNull(event.getPayload());
+		assertEquals(EventPayload.class, event.getPayload().getClass());
 	}
 
 	/**
-	 * Unknown event payload returned as EventPayload
+	 * Event with missing type has payload returned as {@link EventPayload}
 	 */
 	@Test
-	public void unknownPayload() {
-		EventFormatter formatter = new EventFormatter();
-		formatter.getEventCreator().createInstance(null);
-		EventPayload payload = formatter.getPayloadDeserializer().deserialize(null, null, null);
-		assertTrue(payload instanceof EventPayload);
+	public void missingType() {
+		Event event = GsonUtils.fromJson("{\"payload\":{}}", Event.class);
+		assertNotNull(event);
+		assertNotNull(event.getPayload());
+		assertEquals(EventPayload.class, event.getPayload().getClass());
+	}
+
+	/**
+	 * Missing payload
+	 */
+	@Test
+	public void missingPayload() {
+		Event event = GsonUtils.fromJson("{}", Event.class);
+		assertNotNull(event);
+		assertNull(event.getPayload());
 	}
 }
diff --git a/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/EventFormatter.java b/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/EventFormatter.java
index daad25d5..8cbfb9ab 100644
--- a/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/EventFormatter.java
+++ b/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/EventFormatter.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.egit.github.core.client;
 
+import static com.google.gson.FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES;
 import static org.eclipse.egit.github.core.event.Event.TYPE_COMMIT_COMMENT;
 import static org.eclipse.egit.github.core.event.Event.TYPE_CREATE;
 import static org.eclipse.egit.github.core.event.Event.TYPE_DELETE;
@@ -29,13 +30,15 @@ import static org.eclipse.egit.github.core.event.Event.TYPE_PUSH;
 import static org.eclipse.egit.github.core.event.Event.TYPE_TEAM_ADD;
 import static org.eclipse.egit.github.core.event.Event.TYPE_WATCH;
 
-import com.google.gson.InstanceCreator;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import com.google.gson.JsonDeserializationContext;
 import com.google.gson.JsonDeserializer;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonParseException;
 
 import java.lang.reflect.Type;
+import java.util.Date;
 
 import org.eclipse.egit.github.core.event.CommitCommentPayload;
 import org.eclipse.egit.github.core.event.CreatePayload;
@@ -62,89 +65,66 @@ import org.eclipse.egit.github.core.event.WatchPayload;
  * Formats an event's payload with the appropriate class given a certain event
  * type
  */
-public class EventFormatter {
+public class EventFormatter implements JsonDeserializer<Event> {
 
-	private class PayloadDeserializer implements JsonDeserializer<EventPayload> {
+	private final Gson gson = new GsonBuilder()
+			.registerTypeAdapter(Date.class, new DateFormatter())
+			.setFieldNamingPolicy(LOWER_CASE_WITH_UNDERSCORES).create();
 
-		public EventPayload deserialize(JsonElement json, Type typeOfT,
-				JsonDeserializationContext context) throws JsonParseException {
-			String type = event.get().getType();
-			Class<? extends EventPayload> clazz = EventPayload.class;
-			if (TYPE_COMMIT_COMMENT.equals(type))
-				clazz = CommitCommentPayload.class;
-			else if (TYPE_CREATE.equals(type))
-				clazz = CreatePayload.class;
-			else if (TYPE_DELETE.equals(type))
-				clazz = DeletePayload.class;
-			else if (TYPE_DOWNLOAD.equals(type))
-				clazz = DownloadPayload.class;
-			else if (TYPE_FOLLOW.equals(type))
-				clazz = FollowPayload.class;
-			else if (TYPE_FORK.equals(type))
-				clazz = ForkPayload.class;
-			else if (TYPE_FORK_APPLY.equals(type))
-				clazz = ForkApplyPayload.class;
-			else if (TYPE_GIST.equals(type))
-				clazz = GistPayload.class;
-			else if (TYPE_GOLLUM.equals(type))
-				clazz = GollumPayload.class;
-			else if (TYPE_ISSUE_COMMENT.equals(type))
-				clazz = IssueCommentPayload.class;
-			else if (TYPE_ISSUES.equals(type))
-				clazz = IssuesPayload.class;
-			else if (TYPE_MEMBER.equals(type))
-				clazz = MemberPayload.class;
-			else if (TYPE_PUBLIC.equals(type))
-				clazz = PublicPayload.class;
-			else if (TYPE_PULL_REQUEST.equals(type))
-				clazz = PullRequestPayload.class;
-			else if (TYPE_PULL_REQUEST_REVIEW_COMMENT.equals(type))
-				clazz = PullRequestReviewCommentPayload.class;
-			else if (TYPE_PUSH.equals(type))
-				clazz = PushPayload.class;
-			else if (TYPE_TEAM_ADD.equals(type))
-				clazz = TeamAddPayload.class;
-			else if (TYPE_WATCH.equals(type))
-				clazz = WatchPayload.class;
-
-			// payload not recognized
-			if (clazz == EventPayload.class)
-				return new EventPayload();
-
-			return context.deserialize(json, clazz);
-		}
-	}
-
-	private class EventCreator implements InstanceCreator<Event> {
-
-		public Event createInstance(Type type) {
-			final Event event = new Event();
-			EventFormatter.this.event.set(event);
+	public Event deserialize(JsonElement json, Type typeOfT,
+			JsonDeserializationContext context) throws JsonParseException {
+		final Event event = gson.fromJson(json, Event.class);
+		if (event == null || !json.isJsonObject())
+			return event;
+		final JsonElement rawPayload = json.getAsJsonObject().get("payload");
+		if (rawPayload == null || !rawPayload.isJsonObject())
+			return event;
+		final String type = event.getType();
+		if (type == null || type.length() == 0)
 			return event;
-		}
-	}
-
-	private final ThreadLocal<Event> event = new ThreadLocal<Event>();
-
-	private final PayloadDeserializer payloadDeserializer = new PayloadDeserializer();
-
-	private final EventCreator eventCreator = new EventCreator();
 
-	/**
-	 * Get deserializer for {@link EventPayload} objects
-	 *
-	 * @return deserializer
-	 */
-	public JsonDeserializer<EventPayload> getPayloadDeserializer() {
-		return payloadDeserializer;
-	}
+		Class<? extends EventPayload> payloadClass;
+		if (TYPE_COMMIT_COMMENT.equals(type))
+			payloadClass = CommitCommentPayload.class;
+		else if (TYPE_CREATE.equals(type))
+			payloadClass = CreatePayload.class;
+		else if (TYPE_DELETE.equals(type))
+			payloadClass = DeletePayload.class;
+		else if (TYPE_DOWNLOAD.equals(type))
+			payloadClass = DownloadPayload.class;
+		else if (TYPE_FOLLOW.equals(type))
+			payloadClass = FollowPayload.class;
+		else if (TYPE_FORK.equals(type))
+			payloadClass = ForkPayload.class;
+		else if (TYPE_FORK_APPLY.equals(type))
+			payloadClass = ForkApplyPayload.class;
+		else if (TYPE_GIST.equals(type))
+			payloadClass = GistPayload.class;
+		else if (TYPE_GOLLUM.equals(type))
+			payloadClass = GollumPayload.class;
+		else if (TYPE_ISSUE_COMMENT.equals(type))
+			payloadClass = IssueCommentPayload.class;
+		else if (TYPE_ISSUES.equals(type))
+			payloadClass = IssuesPayload.class;
+		else if (TYPE_MEMBER.equals(type))
+			payloadClass = MemberPayload.class;
+		else if (TYPE_PUBLIC.equals(type))
+			payloadClass = PublicPayload.class;
+		else if (TYPE_PULL_REQUEST.equals(type))
+			payloadClass = PullRequestPayload.class;
+		else if (TYPE_PULL_REQUEST_REVIEW_COMMENT.equals(type))
+			payloadClass = PullRequestReviewCommentPayload.class;
+		else if (TYPE_PUSH.equals(type))
+			payloadClass = PushPayload.class;
+		else if (TYPE_TEAM_ADD.equals(type))
+			payloadClass = TeamAddPayload.class;
+		else if (TYPE_WATCH.equals(type))
+			payloadClass = WatchPayload.class;
+		else
+			return event;
 
-	/**
-	 * Get instance creator for {@link Event} objects
-	 *
-	 * @return instance creator
-	 */
-	public InstanceCreator<Event> getEventCreator() {
-		return eventCreator;
+		EventPayload typedPayload = context.deserialize(rawPayload,
+				payloadClass);
+		return event.setPayload(typedPayload);
 	}
 }
diff --git a/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/GsonUtils.java b/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/GsonUtils.java
index 6adb331d..29bcc9d4 100644
--- a/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/GsonUtils.java
+++ b/org.eclipse.egit.github.core/src/org/eclipse/egit/github/core/client/GsonUtils.java
@@ -20,7 +20,6 @@ import java.lang.reflect.Type;
 import java.util.Date;
 
 import org.eclipse.egit.github.core.event.Event;
-import org.eclipse.egit.github.core.event.EventPayload;
 
 /**
  * Gson utilities.
@@ -51,11 +50,7 @@ public abstract class GsonUtils {
 	public static final Gson createGson(final boolean serializeNulls) {
 		final GsonBuilder builder = new GsonBuilder();
 		builder.registerTypeAdapter(Date.class, new DateFormatter());
-		EventFormatter eventFormatter = new EventFormatter();
-		builder.registerTypeAdapter(Event.class,
-				eventFormatter.getEventCreator());
-		builder.registerTypeAdapter(EventPayload.class,
-				eventFormatter.getPayloadDeserializer());
+		builder.registerTypeAdapter(Event.class, new EventFormatter());
 		builder.setFieldNamingPolicy(LOWER_CASE_WITH_UNDERSCORES);
 		if (serializeNulls)
 			builder.serializeNulls();