From 75b549f1ff8d5d5b8138c7568b406af72b9883f6 Mon Sep 17 00:00:00 2001 From: jwilson Date: Sun, 29 Jan 2017 22:47:59 -0500 Subject: [PATCH] Implement promoteValueToName() for ObjectJsonWriter. This was renamed from promoteNameToValue(). Also run the test on both types of codecs and fix some implementation issues that uncovered. --- .../moshi/BufferedSinkJsonWriter.java | 22 +--- .../java/com/squareup/moshi/JsonWriter.java | 16 ++- .../com/squareup/moshi/MapJsonAdapter.java | 2 +- .../com/squareup/moshi/ObjectJsonReader.java | 8 +- .../com/squareup/moshi/ObjectJsonWriter.java | 17 ++- .../squareup/moshi/MapJsonAdapterTest.java | 16 +++ .../moshi/PromoteNameToValueTest.java | 111 +++++++++--------- 7 files changed, 109 insertions(+), 83 deletions(-) diff --git a/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java b/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java index cd5a834..9464dc5 100644 --- a/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java +++ b/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java @@ -62,8 +62,6 @@ final class BufferedSinkJsonWriter extends JsonWriter { private String deferredName; - private boolean promoteNameToValue; - BufferedSinkJsonWriter(BufferedSink sink) { if (sink == null) { throw new NullPointerException("sink == null"); @@ -92,7 +90,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { } @Override public JsonWriter endObject() throws IOException { - promoteNameToValue = false; + promoteValueToName = false; return close(EMPTY_OBJECT, NONEMPTY_OBJECT, "}"); } @@ -143,7 +141,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { } deferredName = name; pathNames[stackSize - 1] = name; - promoteNameToValue = false; + promoteValueToName = false; return this; } @@ -159,7 +157,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { if (value == null) { return nullValue(); } - if (promoteNameToValue) { + if (promoteValueToName) { return name(value); } writeDeferredName(); @@ -203,7 +201,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { if (!lenient && (Double.isNaN(value) || Double.isInfinite(value))) { throw new IllegalArgumentException("Numeric values must be finite, but was " + value); } - if (promoteNameToValue) { + if (promoteValueToName) { return name(Double.toString(value)); } writeDeferredName(); @@ -214,7 +212,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { } @Override public JsonWriter value(long value) throws IOException { - if (promoteNameToValue) { + if (promoteValueToName) { return name(Long.toString(value)); } writeDeferredName(); @@ -234,7 +232,7 @@ final class BufferedSinkJsonWriter extends JsonWriter { && (string.equals("-Infinity") || string.equals("Infinity") || string.equals("NaN"))) { throw new IllegalArgumentException("Numeric values must be finite, but was " + value); } - if (promoteNameToValue) { + if (promoteValueToName) { return name(string); } writeDeferredName(); @@ -369,12 +367,4 @@ final class BufferedSinkJsonWriter extends JsonWriter { throw new IllegalStateException("Nesting problem."); } } - - @Override void promoteNameToValue() throws IOException { - int context = peekScope(); - if (context != NONEMPTY_OBJECT && context != EMPTY_OBJECT) { - throw new IllegalStateException("Nesting problem."); - } - promoteNameToValue = true; - } } diff --git a/moshi/src/main/java/com/squareup/moshi/JsonWriter.java b/moshi/src/main/java/com/squareup/moshi/JsonWriter.java index e178760..c017995 100644 --- a/moshi/src/main/java/com/squareup/moshi/JsonWriter.java +++ b/moshi/src/main/java/com/squareup/moshi/JsonWriter.java @@ -20,6 +20,9 @@ import java.io.Flushable; import java.io.IOException; import okio.BufferedSink; +import static com.squareup.moshi.JsonScope.EMPTY_OBJECT; +import static com.squareup.moshi.JsonScope.NONEMPTY_OBJECT; + /** * Writes a JSON (RFC 7159) * encoded value to a stream, one token at a time. The stream includes both @@ -131,6 +134,7 @@ public abstract class JsonWriter implements Closeable, Flushable { String indent; boolean lenient; boolean serializeNulls; + boolean promoteValueToName; /** Returns a new instance that writes a JSON-encoded stream to {@code sink}. */ public static JsonWriter of(BufferedSink sink) { @@ -313,10 +317,16 @@ public abstract class JsonWriter implements Closeable, Flushable { public abstract JsonWriter value(Number value) throws IOException; /** - * Changes the reader to treat the next string value as a name. This is useful for map adapters so - * that arbitrary type adapters can use {@link #value(String)} to write a name value. + * Changes the writer to treat the next value as a string name. This is useful for map adapters so + * that arbitrary type adapters can use {@link #value} to write a name value. */ - abstract void promoteNameToValue() throws IOException; + final void promoteValueToName() throws IOException { + int context = peekScope(); + if (context != NONEMPTY_OBJECT && context != EMPTY_OBJECT) { + throw new IllegalStateException("Nesting problem."); + } + promoteValueToName = true; + } /** * Returns a JsonPath to diff --git a/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java index 3158108..8cf784d 100644 --- a/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java @@ -52,7 +52,7 @@ final class MapJsonAdapter extends JsonAdapter> { if (entry.getKey() == null) { throw new JsonDataException("Map key is null at " + writer.getPath()); } - writer.promoteNameToValue(); + writer.promoteValueToName(); keyAdapter.toJson(writer, entry.getKey()); valueAdapter.toJson(writer, entry.getValue()); } diff --git a/moshi/src/main/java/com/squareup/moshi/ObjectJsonReader.java b/moshi/src/main/java/com/squareup/moshi/ObjectJsonReader.java index 8f16fec..0f30d88 100644 --- a/moshi/src/main/java/com/squareup/moshi/ObjectJsonReader.java +++ b/moshi/src/main/java/com/squareup/moshi/ObjectJsonReader.java @@ -273,10 +273,10 @@ final class ObjectJsonReader extends JsonReader { } @Override void promoteNameToValue() throws IOException { - Map.Entry peeked = require(Map.Entry.class, Token.NAME); - - push(peeked.getKey()); - stack[stackSize - 2] = peeked.getValue(); + if (hasNext()) { + String name = nextName(); + push(name); + } } @Override public void close() throws IOException { diff --git a/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java b/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java index a4b7c53..237d462 100644 --- a/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java +++ b/moshi/src/main/java/com/squareup/moshi/ObjectJsonWriter.java @@ -79,6 +79,7 @@ final class ObjectJsonWriter extends JsonWriter { if (peekScope() != EMPTY_OBJECT || deferredName != null) { throw new IllegalStateException("Nesting problem."); } + promoteValueToName = false; stackSize--; stack[stackSize] = null; pathNames[stackSize] = null; // Free the last path name so that it can be garbage collected! @@ -96,12 +97,16 @@ final class ObjectJsonWriter extends JsonWriter { if (peekScope() != EMPTY_OBJECT || deferredName != null) { throw new IllegalStateException("Nesting problem."); } - pathNames[stackSize - 1] = name; deferredName = name; + pathNames[stackSize - 1] = name; + promoteValueToName = false; return this; } @Override public JsonWriter value(String value) throws IOException { + if (promoteValueToName) { + return name(value); + } add(value); pathIndices[stackSize - 1]++; return this; @@ -130,6 +135,9 @@ final class ObjectJsonWriter extends JsonWriter { } @Override public JsonWriter value(long value) throws IOException { + if (promoteValueToName) { + return name(Long.toString(value)); + } add(value); pathIndices[stackSize - 1]++; return this; @@ -142,15 +150,14 @@ final class ObjectJsonWriter extends JsonWriter { throw new IllegalArgumentException("Numeric values must be finite, but was " + value); } } + if (promoteValueToName) { + return name(value.toString()); + } add(value); pathIndices[stackSize - 1]++; return this; } - @Override void promoteNameToValue() throws IOException { - throw new UnsupportedOperationException(); - } - @Override public void close() throws IOException { int size = stackSize; if (size > 1 || size == 1 && scopes[size - 1] != NONEMPTY_DOCUMENT) { diff --git a/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java b/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java index 0fb1955..bb07498 100644 --- a/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java +++ b/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java @@ -125,6 +125,22 @@ public final class MapJsonAdapterTest { MapEntry.entry(5, true), MapEntry.entry(6, false), MapEntry.entry(7, null)); } + @Test public void mapWithNonStringKeysToJsonObject() throws Exception { + Map map = new LinkedHashMap<>(); + map.put(5, true); + map.put(6, false); + map.put(7, null); + + Map jsonObject = new LinkedHashMap<>(); + jsonObject.put("5", true); + jsonObject.put("6", false); + jsonObject.put("7", null); + + JsonAdapter> jsonAdapter = mapAdapter(Integer.class, Boolean.class); + assertThat(jsonAdapter.serializeNulls().toJsonObject(map)).isEqualTo(jsonObject); + assertThat(jsonAdapter.fromJsonObject(jsonObject)).isEqualTo(map); + } + private String toJson(Type keyType, Type valueType, Map value) throws IOException { JsonAdapter> jsonAdapter = mapAdapter(keyType, valueType); Buffer buffer = new Buffer(); diff --git a/moshi/src/test/java/com/squareup/moshi/PromoteNameToValueTest.java b/moshi/src/test/java/com/squareup/moshi/PromoteNameToValueTest.java index 284b095..c50005d 100644 --- a/moshi/src/test/java/com/squareup/moshi/PromoteNameToValueTest.java +++ b/moshi/src/test/java/com/squareup/moshi/PromoteNameToValueTest.java @@ -15,15 +15,27 @@ */ package com.squareup.moshi; -import okio.Buffer; +import java.util.List; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; +@RunWith(Parameterized.class) public final class PromoteNameToValueTest { + @Parameter public JsonCodecFactory factory; + + @Parameters(name = "{0}") + public static List parameters() { + return JsonCodecFactory.factories(); + } + @Test public void readerStringValue() throws Exception { - JsonReader reader = newReader("{\"a\":1}"); + JsonReader reader = factory.newReader("{\"a\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.a"); @@ -37,7 +49,7 @@ public final class PromoteNameToValueTest { } @Test public void readerIntegerValue() throws Exception { - JsonReader reader = newReader("{\"5\":1}"); + JsonReader reader = factory.newReader("{\"5\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.5"); @@ -51,7 +63,7 @@ public final class PromoteNameToValueTest { } @Test public void readerDoubleValue() throws Exception { - JsonReader reader = newReader("{\"5.5\":1}"); + JsonReader reader = factory.newReader("{\"5.5\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.5.5"); @@ -65,7 +77,7 @@ public final class PromoteNameToValueTest { } @Test public void readerBooleanValue() throws Exception { - JsonReader reader = newReader("{\"true\":1}"); + JsonReader reader = factory.newReader("{\"true\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.true"); @@ -74,7 +86,9 @@ public final class PromoteNameToValueTest { reader.nextBoolean(); fail(); } catch (JsonDataException e) { - assertThat(e).hasMessage("Expected a boolean but was STRING at path $.true"); + assertThat(e.getMessage()).isIn( + "Expected BOOLEAN but was true, a java.lang.String, at path $.true", + "Expected a boolean but was STRING at path $.true"); } assertThat(reader.getPath()).isEqualTo("$.true"); assertThat(reader.nextString()).isEqualTo("true"); @@ -85,7 +99,7 @@ public final class PromoteNameToValueTest { } @Test public void readerLongValue() throws Exception { - JsonReader reader = newReader("{\"5\":1}"); + JsonReader reader = factory.newReader("{\"5\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.5"); @@ -99,7 +113,7 @@ public final class PromoteNameToValueTest { } @Test public void readerNullValue() throws Exception { - JsonReader reader = newReader("{\"null\":1}"); + JsonReader reader = factory.newReader("{\"null\":1}"); reader.beginObject(); reader.promoteNameToValue(); assertThat(reader.getPath()).isEqualTo("$.null"); @@ -108,7 +122,9 @@ public final class PromoteNameToValueTest { reader.nextNull(); fail(); } catch (JsonDataException e) { - assertThat(e).hasMessage("Expected null but was STRING at path $.null"); + assertThat(e.getMessage()).isIn( + "Expected NULL but was null, a java.lang.String, at path $.null", + "Expected null but was STRING at path $.null"); } assertThat(reader.nextString()).isEqualTo("null"); assertThat(reader.getPath()).isEqualTo("$.null"); @@ -119,7 +135,7 @@ public final class PromoteNameToValueTest { } @Test public void readerMultipleValueObject() throws Exception { - JsonReader reader = newReader("{\"a\":1,\"b\":2}"); + JsonReader reader = factory.newReader("{\"a\":1,\"b\":2}"); reader.beginObject(); assertThat(reader.nextName()).isEqualTo("a"); assertThat(reader.nextInt()).isEqualTo(1); @@ -135,7 +151,7 @@ public final class PromoteNameToValueTest { } @Test public void readerEmptyValueObject() throws Exception { - JsonReader reader = newReader("{}"); + JsonReader reader = factory.newReader("{}"); reader.beginObject(); assertThat(reader.peek()).isEqualTo(JsonReader.Token.END_OBJECT); reader.promoteNameToValue(); @@ -145,7 +161,7 @@ public final class PromoteNameToValueTest { } @Test public void readerUnusedPromotionDoesntPersist() throws Exception { - JsonReader reader = newReader("[{},{\"a\":5}]"); + JsonReader reader = factory.newReader("[{},{\"a\":5}]"); reader.beginArray(); reader.beginObject(); reader.promoteNameToValue(); @@ -160,7 +176,7 @@ public final class PromoteNameToValueTest { } @Test public void readerUnquotedIntegerValue() throws Exception { - JsonReader reader = newReader("{5:1}"); + JsonReader reader = factory.newReader("{5:1}"); reader.setLenient(true); reader.beginObject(); reader.promoteNameToValue(); @@ -170,7 +186,7 @@ public final class PromoteNameToValueTest { } @Test public void readerUnquotedLongValue() throws Exception { - JsonReader reader = newReader("{5:1}"); + JsonReader reader = factory.newReader("{5:1}"); reader.setLenient(true); reader.beginObject(); reader.promoteNameToValue(); @@ -180,7 +196,7 @@ public final class PromoteNameToValueTest { } @Test public void readerUnquotedDoubleValue() throws Exception { - JsonReader reader = newReader("{5:1}"); + JsonReader reader = factory.newReader("{5:1}"); reader.setLenient(true); reader.beginObject(); reader.promoteNameToValue(); @@ -189,57 +205,49 @@ public final class PromoteNameToValueTest { reader.endObject(); } - private JsonReader newReader(String s) { - return JsonReader.of(new Buffer().writeUtf8(s)); - } - @Test public void writerStringValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.value("a"); assertThat(writer.getPath()).isEqualTo("$.a"); writer.value(1); assertThat(writer.getPath()).isEqualTo("$.a"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"a\":1}"); + assertThat(factory.json()).isEqualTo("{\"a\":1}"); } @Test public void writerIntegerValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.value(5); assertThat(writer.getPath()).isEqualTo("$.5"); writer.value(1); assertThat(writer.getPath()).isEqualTo("$.5"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"5\":1}"); + assertThat(factory.json()).isEqualTo("{\"5\":1}"); } @Test public void writerDoubleValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.value(5.5d); assertThat(writer.getPath()).isEqualTo("$.5.5"); writer.value(1); assertThat(writer.getPath()).isEqualTo("$.5.5"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"5.5\":1}"); + assertThat(factory.json()).isEqualTo("{\"5.5\":1}"); } @Test public void writerBooleanValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); try { writer.value(true); fail(); @@ -251,28 +259,26 @@ public final class PromoteNameToValueTest { writer.value(1); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"true\":1}"); + assertThat(factory.json()).isEqualTo("{\"true\":1}"); } @Test public void writerLongValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.value(5L); assertThat(writer.getPath()).isEqualTo("$.5"); writer.value(1); assertThat(writer.getPath()).isEqualTo("$.5"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"5\":1}"); + assertThat(factory.json()).isEqualTo("{\"5\":1}"); } @Test public void writerNullValue() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); try { writer.nullValue(); fail(); @@ -285,42 +291,39 @@ public final class PromoteNameToValueTest { assertThat(writer.getPath()).isEqualTo("$.null"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"null\":1}"); + assertThat(factory.json()).isEqualTo("{\"null\":1}"); } @Test public void writerMultipleValueObject() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); writer.name("a"); writer.value(1); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.value("b"); assertThat(writer.getPath()).isEqualTo("$.b"); writer.value(2); assertThat(writer.getPath()).isEqualTo("$.b"); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{\"a\":1,\"b\":2}"); + assertThat(factory.json()).isEqualTo("{\"a\":1,\"b\":2}"); } @Test public void writerEmptyValueObject() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); assertThat(writer.getPath()).isEqualTo("$."); writer.endObject(); assertThat(writer.getPath()).isEqualTo("$"); - assertThat(buffer.readUtf8()).isEqualTo("{}"); + assertThat(factory.json()).isEqualTo("{}"); } @Test public void writerUnusedPromotionDoesntPersist() throws Exception { - Buffer buffer = new Buffer(); - JsonWriter writer = JsonWriter.of(buffer); + JsonWriter writer = factory.newWriter(); writer.beginArray(); writer.beginObject(); - writer.promoteNameToValue(); + writer.promoteValueToName(); writer.endObject(); writer.beginObject(); try {