Refactor the message pack support within the MessageExtras to support…

… bidirectional coding of 'raw' arbitrary JSON.
ably · Jun 10, 2020 · 0752b58 · 0752b58
1 parent f3dfaf9
commit 0752b58
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 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
@@ -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<Value, Value> 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
@@ -4,32 +4,31 @@
 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 {
 	private static final String TAG = MessageExtras.class.getName();
 
 	/**
 	 * 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<Value, Value> 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<Value, Value> 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
@@ -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;
 	}
 }