From 0752b5896305f8f5ed43f853f615ce96dc22a9c9 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 22:05:23 +0100 Subject: [PATCH] Refactor the message pack support within the MessageExtras to support bidirectional coding of 'raw' arbitrary JSON. --- .../java/io/ably/lib/types/DeltaExtras.java | 33 ++------- .../java/io/ably/lib/types/MessageExtras.java | 67 +++++++++++-------- .../io/ably/lib/types/MessageExtrasTest.java | 51 +++++++++++--- 3 files changed, 88 insertions(+), 63 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java index 643421603..becd9bf3e 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -5,14 +5,14 @@ import com.google.gson.JsonObject; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; -import io.ably.lib.util.Log; import io.ably.lib.util.Serialisation; -import org.msgpack.core.MessageFormat; import org.msgpack.core.MessagePacker; -import org.msgpack.core.MessageUnpacker; +import org.msgpack.value.Value; +import org.msgpack.value.ValueFactory; import java.io.IOException; import java.lang.reflect.Type; +import java.util.Map; import java.util.Objects; public final class DeltaExtras { @@ -61,29 +61,10 @@ public String getFrom() { packer.packString(from); } - /* package private */ static DeltaExtras fromMsgpack(final MessageUnpacker unpacker) throws IOException { - final int fieldCount = unpacker.unpackMapHeader(); - String format = null; - String from = null; - for(int i = 0; i < fieldCount; i++) { - String fieldName = unpacker.unpackString(); - MessageFormat fieldFormat = unpacker.getNextFormat(); - if(fieldFormat.equals(MessageFormat.NIL)) { - unpacker.unpackNil(); - continue; - } - - if(fieldName.equals("format")) { - format = unpacker.unpackString(); - } else if (fieldName.equals("from")) { - from = unpacker.unpackString(); - } else { - Log.w(TAG, "Unexpected field: " + fieldName); - unpacker.skipValue(); - } - } - - return new DeltaExtras(format, from); + /* package private */ static DeltaExtras fromMessagePackMap(final Map map) throws IOException { + final Value format = map.get(ValueFactory.newString("format")); + final Value from = map.get(ValueFactory.newString("from")); + return new DeltaExtras(format.asStringValue().asString(), from.asStringValue().asString()); } @Override diff --git a/lib/src/main/java/io/ably/lib/types/MessageExtras.java b/lib/src/main/java/io/ably/lib/types/MessageExtras.java index c01c3b263..d4327682e 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -4,14 +4,17 @@ import com.google.gson.JsonObject; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; -import io.ably.lib.util.Log; import io.ably.lib.util.Serialisation; -import org.msgpack.core.MessageFormat; import org.msgpack.core.MessagePacker; import org.msgpack.core.MessageUnpacker; +import org.msgpack.value.ImmutableMapValue; +import org.msgpack.value.ImmutableValue; +import org.msgpack.value.Value; +import org.msgpack.value.ValueFactory; import java.io.IOException; import java.lang.reflect.Type; +import java.util.Map; import java.util.Objects; public final class MessageExtras { @@ -19,17 +22,13 @@ public final class MessageExtras { /** * Only intended for use with a MessageExtras instance received from Ably's servers (inbound). - * May be null if raw is null. - * Must be null if raw is not null. */ private final DeltaExtras delta; /** * Only intended for use with MessageExtras instances created to be sent to Ably's servers (outbound). - * May be null if delta is null. - * Must be null if delta is not null. */ - private final JsonObject raw; // may be null + private final JsonObject raw; /** * Creates a MessageExtras instance to be sent as extra with a Message to Ably's servers. @@ -51,42 +50,56 @@ public MessageExtras(final DeltaExtras delta) { raw = null; } + private MessageExtras(final JsonObject raw, final DeltaExtras delta) { + this.raw = raw; + this.delta = delta; + } + public DeltaExtras getDelta() { return delta; } + /* package private */ JsonObject getRaw() { + return raw; + } + /* package private */ void writeMsgpack(MessagePacker packer) throws IOException { - int fieldCount = 0; - if (this.delta != null) { - fieldCount++; - } - packer.packMapHeader(fieldCount); - if (this.delta != null) { + if (null == raw) { + // raw is null, so delta is not null + packer.packMapHeader(1); packer.packString("delta"); this.delta.writeMsgpack(packer); + } else { + // raw is not null, so delta can be ignored + Serialisation.gsonToMsgpack(raw, packer); } } /* package private */ static MessageExtras fromMsgpack(MessageUnpacker unpacker) throws IOException { - final int fieldCount = unpacker.unpackMapHeader(); DeltaExtras delta = null; - for(int i = 0; i < fieldCount; i++) { - String fieldName = unpacker.unpackString(); - MessageFormat fieldFormat = unpacker.getNextFormat(); - if(fieldFormat.equals(MessageFormat.NIL)) { - unpacker.unpackNil(); - continue; - } - if(fieldName.equals("delta")) { - delta = DeltaExtras.fromMsgpack(unpacker); - } else { - Log.w(TAG, "Unexpected field: " + fieldName); - unpacker.skipValue(); + final ImmutableValue value = unpacker.unpackValue(); + if (value instanceof ImmutableMapValue) { + final Map map = ((ImmutableMapValue) value).map(); + final Value deltaValue = map.get(ValueFactory.newString("delta")); + if (null != deltaValue) { + if (!(deltaValue instanceof ImmutableMapValue)) { + // There's a delta key but the value at that key is not a map. + throw new IOException("The delta extras unpacked to the wrong type \"" + deltaValue.getClass() + "\" when expected a map."); + } + final Map deltaMap = ((ImmutableMapValue)deltaValue).map(); + delta = DeltaExtras.fromMessagePackMap(deltaMap); } } - return new MessageExtras(delta); + final JsonElement element = Serialisation.msgpackToGson(value); + if (!(element instanceof JsonObject)) { + // The root thing that we unpacked was not a map. + throw new IOException("The extras unpacked to the wrong type \"" + element.getClass() + "\" when expected a JsonObject."); + } + final JsonObject raw = (JsonObject)element; + + return new MessageExtras(raw, delta); } @Override diff --git a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java index 1d523a116..4cfa035a7 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -1,9 +1,5 @@ package io.ably.lib.types; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNull; - import com.google.gson.JsonElement; import com.google.gson.JsonObject; import io.ably.lib.util.Serialisation; @@ -14,6 +10,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; + public class MessageExtrasTest { /** * Construct an instance from a JSON source and validate that the @@ -38,6 +38,26 @@ public void raw() { assertNotEquals(objectB, objectA); } + @Test + public void rawViaMessagePack() throws IOException { + final JsonObject object = new JsonObject(); + object.addProperty("foo", "bar"); + object.addProperty("cliché", "cache"); + final MessageExtras messageExtras = new MessageExtras(object); + + // Encode to MessagePack + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); + messageExtras.writeMsgpack(packer); + packer.flush(); + + // Decode from MessagePack + MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); + final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); + + assertEquals(messageExtras, unpacked); + } + /** * Construct an instance with DeltaExtras and validate that the * serialised JSON is as expected. Also validate that the DeltaExtras @@ -53,11 +73,7 @@ public void delta() { assertNotEquals(deltaExtrasB, messageExtras.getDelta()); assertNotEquals(deltaExtrasB, deltaExtrasA); - final JsonObject expectedDeltaExtrasJsonElement = new JsonObject(); - expectedDeltaExtrasJsonElement.addProperty("format", "someFormat"); - expectedDeltaExtrasJsonElement.addProperty("from", "someSource"); - final JsonObject expectedJsonElement = new JsonObject(); - expectedJsonElement.add("delta", expectedDeltaExtrasJsonElement); + final JsonObject expectedJsonElement = deltaExtrasJsonObject("someFormat", "someSource"); final MessageExtras.Serializer serializer = new MessageExtras.Serializer(); final JsonElement serialised = serializer.serialize(messageExtras, null, null); @@ -85,6 +101,21 @@ public void deltaViaMessagePack() throws IOException { MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); - assertEquals(messageExtras, unpacked); + assertEquals(messageExtras.getDelta(), unpacked.getDelta()); + + // On decode from MessagePack we now also get a raw value (JSON), hence the + // two original MessageExtras instance will not equal the unpacked instance. + assertNotEquals(messageExtras, unpacked); + assertNull(messageExtras.getRaw()); + assertEquals(deltaExtrasJsonObject("tamrof", "morf"), unpacked.getRaw()); + } + + private static JsonObject deltaExtrasJsonObject(final String format, final String from) { + final JsonObject deltaExtrasJsonElement = new JsonObject(); + deltaExtrasJsonElement.addProperty("format", format); + deltaExtrasJsonElement.addProperty("from", from); + final JsonObject jsonElement = new JsonObject(); + jsonElement.add("delta", deltaExtrasJsonElement); + return jsonElement; } }