Teammates: Instructor: search comments: fix 'Unparseable date' exception

[damithc]

v5.107
Received from the live server

Error Message: teammates.common.util.Logger severe: Unexpected exception caught by ControllerServlet :
com.google.gson.JsonParseException: java.text.ParseException: Unparseable date: "Mar 1, 2016 11:59:00 PM"
at teammates.common.util.JsonUtils$TeammatesDateAdapter.deserialize(JsonUtils.java:107)
at teammates.common.util.JsonUtils$TeammatesDateAdapter.deserialize(JsonUtils.java:88)
at com.google.gson.internal.bind.TreeTypeAdapter.read(TreeTypeAdapter.java:69)
at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:129)
at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:220)
at com.google.gson.Gson.fromJson(Gson.java:887)
at com.google.gson.Gson.fromJson(Gson.java:852)
at com.google.gson.Gson.fromJson(Gson.java:801)
at teammates.common.util.JsonUtils.fromJson(JsonUtils.java:66)
at teammates.storage.search.FeedbackResponseCommentSearchDocument.fromResults(FeedbackResponseCommentSearchDocument.java:245)
at teammates.storage.api.FeedbackResponseCommentsDb.search(FeedbackResponseCommentsDb.java:400)
at teammates.logic.core.FeedbackResponseCommentsLogic.searchFeedbackResponseComments(FeedbackResponseCommentsLogic.java:160)
at teammates.logic.api.Logic.searchFeedbackResponseComments(Logic.java:2008)
at teammates.ui.controller.InstructorSearchPageAction.execute(InstructorSearchPageAction.java:57)
at teammates.ui.controller.Action.executeAndPostProcess(Action.java:474)
at teammates.ui.controller.ControllerServlet.doPost(ControllerServlet.java:75)
at teammates.ui.controller.ControllerServlet.doGet(ControllerServlet.java:50)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:617)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:511)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1166)
at com.google.appengine.tools.appstats.AppstatsFilter.doFilter(AppstatsFilter.java:143)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1157)
at com.google.apphosting.utils.servlet.ParseBlobUploadFilter.doFilter(ParseBlobUploadFilter.java:125)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1157)
at com.google.apphosting.runtime.jetty.SaveSessionFilter.doFilter(SaveSessionFilter.java:37)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1157)
at com.google.apphosting.utils.servlet.JdbcMySqlConnectionCleanupFilter.doFilter(JdbcMySqlConnectionCleanupFilter.java:60)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1157)
at com.google.apphosting.utils.servlet.TransactionCleanupFilter.doFilter(TransactionCleanupFilter.java:48)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1157)
at org.mortbay.jetty.servlet.ServletHandler.handle(ServletHandler.java:388)
at org.mortbay.jetty.security.SecurityHandler.handle(SecurityHandler.java:216)
at org.mortbay.jetty.servlet.SessionHandler.handle(SessionHandler.java:182)
at org.mortbay.jetty.handler.ContextHandler.handle(ContextHandler.java:765)
at org.mortbay.jetty.webapp.WebAppContext.handle(WebAppContext.java:418)
at com.google.apphosting.runtime.jetty.AppVersionHandlerMap.handle(AppVersionHandlerMap.java:257)
at org.mortbay.jetty.handler.HandlerWrapper.handle(HandlerWrapper.java:152)
at org.mortbay.jetty.Server.handle(Server.java:326)
at org.mortbay.jetty.HttpConnection.handleRequest(HttpConnection.java:542)
at org.mortbay.jetty.HttpConnection$RequestHandler.headerComplete(HttpConnection.java:923)
at com.google.apphosting.runtime.jetty.RpcRequestParser.parseAvailable(RpcRequestParser.java:76)
at org.mortbay.jetty.HttpConnection.handle(HttpConnection.java:404)
at com.google.apphosting.runtime.jetty.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:145)
at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:661)
at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:621)
at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:591)
at com.google.tracing.TraceContext$TraceContextRunnable.runInContext(TraceContext.java:453)
at com.google.tracing.TraceContext$TraceContextRunnable$1.run(TraceContext.java:460)
at com.google.tracing.CurrentContext.runInContext(CurrentContext.java:293)
at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContextNoUnref(TraceContext.java:319)
at com.google.tracing.TraceContext$AbstractTraceContextCallback.runInInheritedContext(TraceContext.java:311)
at com.google.tracing.TraceContext$TraceContextRunnable.run(TraceContext.java:457)
at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:263)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.text.ParseException: Unparseable date: "Mar 1, 2016 11:59:00 PM"
at java.text.DateFormat.parse(DateFormat.java:357)
at teammates.common.util.JsonUtils$TeammatesDateAdapter.deserialize(JsonUtils.java:105)
... 54 more

To reproduce:
Search for something while this check box is ticked

[damithc]

Seems to be an error introduced in v5.107. I've rolled back to v5.106 for the time being. Hope we can fix it by v5.108 (hence the Urgent label)

[wkurniawan07]

@whipermr5 could this be related to #7521? In particular, this line.

[damithc]

Also, it points to a gap in our test coverage.

[whipermr5]

😅😅 I take responsibility for this.

GSON's default date type adapter tries some other formats (enUsFormat and ISO8601) if the provided format (Const.SystemParams.DEFAULT_DATE_TIME_FORMAT) fails. I didn't realise we were relying on this fallback mechanism to correctly parse dates for searching comments. My custom date adapter doesn't have such a mechanism and hence, fails.

Root issue: seems that some comments search documents have the date in the wrong format. This is only true for legacy data.

[whipermr5]

All comments before V5.93 was deployed have the date in the wrong format (en-US). This was fixed by #6196, where FeedbackResponseCommentSearchDocument switched to using JsonUtils (which uses Const.SystemParams.DEFAULT_DATE_TIME_FORMAT) instead of the default GSON builder (which uses the en-US format): https://github.com/TEAMMATES/teammates/pull/6196/files#diff-f1ad305721052e5fa35242d4cb376987R207.

[whipermr5]

@damithc Should I fix this by writing a data migration script for the affected search documents, or patch TeammatesDateAdapter to work with the legacy format (or both)?

EDIT: Looks like JsonUtils already has fallback code for legacy data; to use it I just need to catch the ParseException and rethrow it as JsonSyntaxException in TeammatesDateAdapter, like what the GSON one does if all the formats fail. I'll start by patching TeammatesDateAdapter to work with the legacy format.

Do I need to write new tests for legacy data? This would require very specialised code that writes documents in the wrong format at the Storage layer to prepare the test legacy data (as the current document creation code writes in the correct format).

[damithc]

Do I need to write new tests for legacy data? This would require very specialised code that writes documents in the wrong format at the Storage layer to prepare the test legacy data (as the current document creation code writes in the correct format).

If 'search feedback response comments' is already covered in tests, no need to write tests to cover legacy data. But we should do a migration (can be a separate PR).

[wkurniawan07]

@whipermr5 Good detective work, but it seems like you're linking the wrong commit. The fix that I did some time back and resulted in this was this one. In fact, we had a small discussion on why the fallback code is necessary here.

[whipermr5]

Oops! Edited.