Avoid JsonWriter multi-write error

JsonWriter requires that its clients pass it only a single top-level object.
The existing implementation tries to make code cleaner by having mJsonWriter
never be null, and instead use a global static "NULL_JSON_WRITER" that just
discards data.  But because JsonWriter complains if more than one top-level
object is passed, making this a global object does not work.  This change
instead copes with mJsonWriter being null.

Change-Id: Ia37ccfc8646e91f11a64713dd92d2846eb86ac54

4 files changed, 55 insertions, 49 deletions

diff --git a/java/src/com/android/inputmethod/research/LogUnit.java b/java/src/com/android/inputmethod/research/LogUnit.java
index 4d60bda53..cf1388f46 100644
--- a/java/src/com/android/inputmethod/research/LogUnit.java
+++ b/java/src/com/android/inputmethod/research/LogUnit.java
@@ -25,6 +25,7 @@ import com.android.inputmethod.latin.SuggestedWords;
 import com.android.inputmethod.latin.SuggestedWords.SuggestedWordInfo;
 import com.android.inputmethod.latin.define.ProductionFlag;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -135,9 +136,11 @@ public class LogUnit {
      * @param researchLog where to publish the contents of this {@code LogUnit}
      * @param canIncludePrivateData whether the private data in this {@code LogUnit} should be
      * included
+     *
+     * @throws IOException if publication to the log file is not possible
      */
     public synchronized void publishTo(final ResearchLog researchLog,
-            final boolean canIncludePrivateData) {
+            final boolean canIncludePrivateData) throws IOException {
         // Write out any logStatement that passes the privacy filter.
         final int size = mLogStatementList.size();
         if (size != 0) {
diff --git a/java/src/com/android/inputmethod/research/MainLogBuffer.java b/java/src/com/android/inputmethod/research/MainLogBuffer.java
index 9bdedbf6d..9aa349906 100644
--- a/java/src/com/android/inputmethod/research/MainLogBuffer.java
+++ b/java/src/com/android/inputmethod/research/MainLogBuffer.java
@@ -23,6 +23,7 @@ import com.android.inputmethod.latin.Dictionary;
 import com.android.inputmethod.latin.Suggest;
 import com.android.inputmethod.latin.define.ProductionFlag;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.LinkedList;
 
@@ -177,7 +178,7 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
         return numWordsInLogUnitList == minNGramSize;
     }
 
-    public void shiftAndPublishAll() {
+    public void shiftAndPublishAll() throws IOException {
         final LinkedList<LogUnit> logUnits = getLogUnits();
         while (!logUnits.isEmpty()) {
             publishLogUnitsAtFrontOfBuffer();
@@ -186,10 +187,16 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
 
     @Override
     protected final void onBufferFull() {
-        publishLogUnitsAtFrontOfBuffer();
+        try {
+            publishLogUnitsAtFrontOfBuffer();
+        } catch (final IOException e) {
+            if (DEBUG) {
+                Log.w(TAG, "IOException when publishing front of LogBuffer", e);
+            }
+        }
     }
 
-    protected final void publishLogUnitsAtFrontOfBuffer() {
+    protected final void publishLogUnitsAtFrontOfBuffer() throws IOException {
         // TODO: Refactor this method to require fewer passes through the LogUnits.  Should really
         // require only one pass.
         ArrayList<LogUnit> logUnits = peekAtFirstNWords(N_GRAM_SIZE);
@@ -224,9 +231,11 @@ public abstract class MainLogBuffer extends FixedLogBuffer {
      * @param logUnits The list of logUnits to be published.
      * @param canIncludePrivateData Whether the private data in the logUnits can be included in
      * publication.
+     *
+     * @throws IOException if publication to the log file is not possible
      */
     protected abstract void publish(final ArrayList<LogUnit> logUnits,
-            final boolean canIncludePrivateData);
+            final boolean canIncludePrivateData) throws IOException;
 
     @Override
     protected int shiftOutWords(final int numWords) {
diff --git a/java/src/com/android/inputmethod/research/ResearchLog.java b/java/src/com/android/inputmethod/research/ResearchLog.java
index 18bf7ba54..3e82139a6 100644
--- a/java/src/com/android/inputmethod/research/ResearchLog.java
+++ b/java/src/com/android/inputmethod/research/ResearchLog.java
@@ -25,6 +25,7 @@ import com.android.inputmethod.latin.define.ProductionFlag;
 
 import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
@@ -61,7 +62,11 @@ public class ResearchLog {
     /* package */ final File mFile;
     private final Context mContext;
 
-    private JsonWriter mJsonWriter = NULL_JSON_WRITER;
+    // Earlier implementations used a dummy JsonWriter that just swallowed what it was given, but
+    // this was tricky to do well, because JsonWriter throws an exception if it is passed more than
+    // one top-level object.
+    private JsonWriter mJsonWriter = null;
+
     // true if at least one byte of data has been written out to the log file.  This must be
     // remembered because JsonWriter requires that calls matching calls to beginObject and
     // endObject, as well as beginArray and endArray, and the file is opened lazily, only when
@@ -69,26 +74,6 @@ public class ResearchLog {
     // could be caught, but this might suppress other errors.
     private boolean mHasWrittenData = false;
 
-    private static final JsonWriter NULL_JSON_WRITER = new JsonWriter(
-            new OutputStreamWriter(new NullOutputStream()));
-    private static class NullOutputStream extends OutputStream {
-        /** {@inheritDoc} */
-        @Override
-        public void write(byte[] buffer, int offset, int count) {
-            // nop
-        }
-
-        /** {@inheritDoc} */
-        @Override
-        public void write(byte[] buffer) {
-            // nop
-        }
-
-        @Override
-        public void write(int oneByte) {
-        }
-    }
-
     public ResearchLog(final File outputFile, final Context context) {
         mExecutor = Executors.newSingleThreadScheduledExecutor();
         mFile = outputFile;
@@ -108,6 +93,7 @@ public class ResearchLog {
             @Override
             public Object call() throws Exception {
                 try {
+                    if (mJsonWriter == null) return null;
                     // TODO: This is necessary to avoid an exception.  Better would be to not even
                     // open the JsonWriter if the file is not even opened unless there is valid data
                     // to write.
@@ -119,9 +105,9 @@ public class ResearchLog {
                     mJsonWriter.flush();
                     mJsonWriter.close();
                     if (DEBUG) {
-                        Log.d(TAG, "wrote log to " + mFile);
+                        Log.d(TAG, "closed " + mFile);
                     }
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     Log.d(TAG, "error when closing ResearchLog:", e);
                 } finally {
                     // Marking the file as read-only signals that this log file is ready to be
@@ -162,6 +148,7 @@ public class ResearchLog {
             @Override
             public Object call() throws Exception {
                 try {
+                    if (mJsonWriter == null) return null;
                     if (mHasWrittenData) {
                         // TODO: This is necessary to avoid an exception.  Better would be to not
                         // even open the JsonWriter if the file is not even opened unless there is
@@ -217,7 +204,7 @@ public class ResearchLog {
     private final Callable<Object> mFlushCallable = new Callable<Object>() {
         @Override
         public Object call() throws Exception {
-            mJsonWriter.flush();
+            if (mJsonWriter != null) mJsonWriter.flush();
             return null;
         }
     };
@@ -263,30 +250,29 @@ public class ResearchLog {
     /**
      * Return a JsonWriter for this ResearchLog.  It is initialized the first time this method is
      * called.  The cached value is returned in future calls.
+     *
+     * @throws IOException if opening the JsonWriter is not possible
      */
-    public JsonWriter getInitializedJsonWriterLocked() {
-        if (mJsonWriter != NULL_JSON_WRITER || mFile == null) return mJsonWriter;
+    public JsonWriter getInitializedJsonWriterLocked() throws IOException {
+        if (mJsonWriter != null) return mJsonWriter;
+        if (mFile == null) throw new FileNotFoundException();
         try {
             final JsonWriter jsonWriter = createJsonWriter(mContext, mFile);
-            if (jsonWriter != null) {
-                jsonWriter.beginArray();
-                mJsonWriter = jsonWriter;
-                mHasWrittenData = true;
-            }
+            if (jsonWriter == null) throw new IOException("Could not create JsonWriter");
+
+            jsonWriter.beginArray();
+            mJsonWriter = jsonWriter;
+            mHasWrittenData = true;
+            return mJsonWriter;
         } catch (final IOException e) {
-            Log.w(TAG, "Error in JsonWriter; disabling logging", e);
-            try {
-                mJsonWriter.close();
-            } catch (final IllegalStateException e1) {
-                // Assume that this is just the json not being terminated properly.
-                // Ignore
-            } catch (final IOException e1) {
-                Log.w(TAG, "Error in closing JsonWriter; disabling logging", e1);
-            } finally {
-                mJsonWriter = NULL_JSON_WRITER;
+            if (DEBUG) {
+                Log.w(TAG, "Exception when creating JsonWriter", e);
+                Log.w(TAG, "Closing JsonWriter");
             }
+            if (mJsonWriter != null) mJsonWriter.close();
+            mJsonWriter = null;
+            throw e;
         }
-        return mJsonWriter;
     }
 
     /**
diff --git a/java/src/com/android/inputmethod/research/ResearchLogger.java b/java/src/com/android/inputmethod/research/ResearchLogger.java
index 4e839f972..b3c0faf29 100644
--- a/java/src/com/android/inputmethod/research/ResearchLogger.java
+++ b/java/src/com/android/inputmethod/research/ResearchLogger.java
@@ -422,11 +422,19 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
         // Commit mCurrentLogUnit before closing.
         commitCurrentLogUnit();
 
-        mMainLogBuffer.shiftAndPublishAll();
+        try {
+            mMainLogBuffer.shiftAndPublishAll();
+        } catch (final IOException e) {
+            Log.w(TAG, "IOException when publishing LogBuffer", e);
+        }
         logStatistics();
         commitCurrentLogUnit();
         mMainLogBuffer.setIsStopping();
-        mMainLogBuffer.shiftAndPublishAll();
+        try {
+            mMainLogBuffer.shiftAndPublishAll();
+        } catch (final IOException e) {
+            Log.w(TAG, "IOException when publishing LogBuffer", e);
+        }
         mMainResearchLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);
         mFeedbackLog.blockingClose(RESEARCHLOG_CLOSE_TIMEOUT_IN_MS);