Merge pull request #1225 from square/jwilson.0914.nextSourceHacks

Small improvements to JsonReader.nextSource

moshi/src/main/java/com/squareup/moshi/JsonUtf8Reader.java

@@ -241,10 +241,6 @@ final class JsonUtf8Reader extends JsonReader {
   }
 
   private int doPeek() throws IOException {
-    if (valueSource != null) {
-      valueSource.discard();
-      valueSource = null;
-    }
     int peekStack = scopes[stackSize - 1];
     if (peekStack == JsonScope.EMPTY_ARRAY) {
       scopes[stackSize - 1] = JsonScope.NONEMPTY_ARRAY;
@@ -329,6 +325,13 @@ final class JsonUtf8Reader extends JsonReader {
       } else {
         checkLenient();
       }
+    } else if (peekStack == JsonScope.STREAMING_VALUE) {
+      valueSource.discard();
+      valueSource = null;
+      stackSize--;
+      pathIndices[stackSize - 1]++;
+      pathNames[stackSize - 1] = "null";
+      return doPeek();
     } else if (peekStack == JsonScope.CLOSED) {
       throw new IllegalStateException("JsonReader is closed");
     }
@@ -1067,9 +1070,8 @@ final class JsonUtf8Reader extends JsonReader {
     }
 
     valueSource = new JsonValueSource(source, prefix, state, valueSourceStackSize);
+    pushScope(JsonScope.STREAMING_VALUE);
     peeked = PEEKED_NONE;
-    pathIndices[stackSize - 1]++;
-    pathNames[stackSize - 1] = "null";
 
     return Okio.buffer(valueSource);
   }

moshi/src/main/java/com/squareup/moshi/JsonValueSource.java

@@ -76,7 +76,14 @@ final class JsonValueSource implements Source {
   }
 
   /**
-   * Advance {@link #limit} until it is at least {@code byteCount} or the JSON object is complete.
+   * Advance {@link #limit} until any of these conditions are met:
+   *
+   * <ul>
+   *   <li>Limit is at least {@code byteCount}. We can satisfy the caller's request!
+   *   <li>The JSON value is complete. This stream is exhausted.
+   *   <li>We have some data to return and returning more would require reloading the buffer. We
+   *       prefer to return some data immediately when more data requires blocking.
+   * </ul>
    *
    * @throws EOFException if the stream is exhausted before the JSON object completes.
    */
@@ -87,9 +94,10 @@ final class JsonValueSource implements Source {
         return;
       }
 
-      // If advancing requires more data in the buffer, grow it.
+      // If we can't return any bytes without more data in the buffer, grow the buffer.
       if (limit == buffer.size()) {
-        source.require(limit + 1L);
+        if (limit > 0L) return;
+        source.require(1L);
       }
 
       // Find the next interesting character for the current state. If the buffer doesn't have one,
@@ -196,6 +204,7 @@ final class JsonValueSource implements Source {
     if (!prefix.exhausted()) {
       long prefixResult = prefix.read(sink, byteCount);
       byteCount -= prefixResult;
+      if (buffer.exhausted()) return prefixResult; // Defer a blocking call.
       long suffixResult = read(sink, byteCount);
       return suffixResult != -1L ? suffixResult + prefixResult : prefixResult;
     }

moshi/src/test/java/com/squareup/moshi/JsonUtf8ReaderTest.java

@@ -38,6 +38,7 @@ import okio.Buffer;
 import okio.BufferedSource;
 import okio.ForwardingSource;
 import okio.Okio;
+import okio.Source;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -1408,4 +1409,81 @@ public final class JsonUtf8ReaderTest {
       assertThat(valueSource.readUtf8()).isEqualTo("-2");
     }
   }
+
+  /**
+   * Confirm that {@link JsonReader#nextSource} doesn't load data from the underlying stream until
+   * its required by the caller. If the source is backed by a slow network stream, we want users to
+   * get data as it arrives.
+   *
+   * <p>Because we don't have a slow stream in this test, we just add bytes to our underlying stream
+   * immediately before they're needed.
+   */
+  @Test
+  public void nextSourceStreams() throws IOException {
+    Buffer stream = new Buffer();
+    stream.writeUtf8("[\"");
+
+    JsonReader reader = JsonReader.of(Okio.buffer((Source) stream));
+    reader.beginArray();
+    BufferedSource source = reader.nextSource();
+    assertThat(source.readUtf8(1)).isEqualTo("\"");
+    stream.writeUtf8("hello");
+    assertThat(source.readUtf8(5)).isEqualTo("hello");
+    stream.writeUtf8("world");
+    assertThat(source.readUtf8(5)).isEqualTo("world");
+    stream.writeUtf8("\"");
+    assertThat(source.readUtf8(1)).isEqualTo("\"");
+    stream.writeUtf8("]");
+    assertThat(source.exhausted()).isTrue();
+    reader.endArray();
+  }
+
+  @Test
+  public void nextSourceObjectAfterSelect() throws IOException {
+    // language=JSON
+    JsonReader reader = newReader("[\"p\u0065psi\"]");
+    reader.beginArray();
+    assertThat(reader.selectName(JsonReader.Options.of("coke"))).isEqualTo(-1);
+    try (BufferedSource valueSource = reader.nextSource()) {
+      assertThat(valueSource.readUtf8()).isEqualTo("\"pepsi\""); // not the original characters!
+    }
+  }
+
+  @Test
+  public void nextSourceObjectAfterPromoteNameToValue() throws IOException {
+    // language=JSON
+    JsonReader reader = newReader("{\"a\":true}");
+    reader.beginObject();
+    reader.promoteNameToValue();
+    try (BufferedSource valueSource = reader.nextSource()) {
+      assertThat(valueSource.readUtf8()).isEqualTo("\"a\"");
+    }
+    assertThat(reader.nextBoolean()).isEqualTo(true);
+    reader.endObject();
+  }
+
+  @Test
+  public void nextSourcePath() throws IOException {
+    // language=JSON
+    JsonReader reader = newReader("{\"a\":true,\"b\":[],\"c\":false}");
+    reader.beginObject();
+
+    assertThat(reader.nextName()).isEqualTo("a");
+    assertThat(reader.getPath()).isEqualTo("$.a");
+    assertThat(reader.nextBoolean()).isTrue();
+    assertThat(reader.getPath()).isEqualTo("$.a");
+
+    assertThat(reader.nextName()).isEqualTo("b");
+    try (BufferedSource valueSource = reader.nextSource()) {
+      assertThat(reader.getPath()).isEqualTo("$.b");
+      assertThat(valueSource.readUtf8()).isEqualTo("[]");
+    }
+    assertThat(reader.getPath()).isEqualTo("$.b");
+
+    assertThat(reader.nextName()).isEqualTo("c");
+    assertThat(reader.getPath()).isEqualTo("$.c");
+    assertThat(reader.nextBoolean()).isFalse();
+    assertThat(reader.getPath()).isEqualTo("$.c");
+    reader.endObject();
+  }
 }