From a050229a74c881296af701b503b71395ef25c8fa Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 15:58:45 +0100 Subject: [PATCH 01/24] Add constructor to MessageExtras allowing it to be instantiated from a JsonObject. Some refactoring here to improve our use of gson also. --- .../java/io/ably/lib/types/BaseMessage.java | 62 +++++++++---------- .../java/io/ably/lib/types/DeltaExtras.java | 35 ++++++++--- .../main/java/io/ably/lib/types/Message.java | 12 ++-- .../java/io/ably/lib/types/MessageExtras.java | 59 +++++++++++++++--- .../io/ably/lib/types/PresenceMessage.java | 6 +- .../java/io/ably/lib/util/Serialisation.java | 22 ++++--- 6 files changed, 127 insertions(+), 69 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/BaseMessage.java b/lib/src/main/java/io/ably/lib/types/BaseMessage.java index f0da4618a..2595cc13d 100644 --- a/lib/src/main/java/io/ably/lib/types/BaseMessage.java +++ b/lib/src/main/java/io/ably/lib/types/BaseMessage.java @@ -1,27 +1,23 @@ package io.ably.lib.types; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.lang.reflect.Type; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import org.msgpack.core.MessageFormat; -import org.msgpack.core.MessagePacker; -import org.msgpack.core.MessageUnpacker; - import com.davidehrmann.vcdiff.VCDiffDecoder; import com.davidehrmann.vcdiff.VCDiffDecoderBuilder; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParseException; -import com.google.gson.JsonSerializationContext; - import io.ably.lib.util.Base64Coder; import io.ably.lib.util.Crypto.ChannelCipher; 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 java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class BaseMessage implements Cloneable { /** @@ -75,7 +71,7 @@ public void decode(ChannelOptions opts) throws MessageDecodeException { this.decode(opts, new DecodingContext()); } - + private final static VCDiffDecoder vcdiffDecoder = VCDiffDecoderBuilder.builder().buildSimple(); private static byte[] vcdiffApply(byte[] delta, byte[] base) throws MessageDecodeException { @@ -190,27 +186,27 @@ private String join(String[] elements, char separator, int start, int end) { return result.toString(); } - /* Gson Serializer */ - public static class Serializer { - public JsonElement serialize(BaseMessage message, Type typeOfMessage, JsonSerializationContext ctx) { - JsonObject json = new JsonObject(); - Object data = message.data; - String encoding = message.encoding; - if(data != null) { - if(data instanceof byte[]) { - byte[] dataBytes = (byte[])data; - json.addProperty("data", new String(Base64Coder.encode(dataBytes))); - encoding = (encoding == null) ? "base64" : encoding + "/base64"; - } else { - json.addProperty("data", data.toString()); - } - if(encoding != null) json.addProperty("encoding", encoding); + /** + * Base for gson serialisers. + */ + public static JsonObject toJsonObject(final BaseMessage message) { + JsonObject json = new JsonObject(); + Object data = message.data; + String encoding = message.encoding; + if(data != null) { + if(data instanceof byte[]) { + byte[] dataBytes = (byte[])data; + json.addProperty("data", new String(Base64Coder.encode(dataBytes))); + encoding = (encoding == null) ? "base64" : encoding + "/base64"; + } else { + json.addProperty("data", data.toString()); } - if(message.id != null) json.addProperty("id", message.id); - if(message.clientId != null) json.addProperty("clientId", message.clientId); - if(message.connectionId != null) json.addProperty("connectionId", message.connectionId); - return json; + if(encoding != null) json.addProperty("encoding", encoding); } + if(message.id != null) json.addProperty("id", message.id); + if(message.clientId != null) json.addProperty("clientId", message.clientId); + if(message.connectionId != null) json.addProperty("connectionId", message.connectionId); + return json; } /* Msgpack processing */ 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 ac701e762..9ae4d7232 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -1,21 +1,27 @@ package io.ably.lib.types; -import java.io.IOException; - +import com.google.gson.Gson; +import com.google.gson.JsonElement; +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 io.ably.lib.util.Log; +import java.io.IOException; +import java.lang.reflect.Type; public final class DeltaExtras { private static final String TAG = DeltaExtras.class.getName(); - + public static final String FORMAT_VCDIFF = "vcdiff"; private final String format; private final String from; - + public DeltaExtras(final String format, final String from) { if (null == format) { throw new IllegalArgumentException("format cannot be null."); @@ -23,11 +29,11 @@ public DeltaExtras(final String format, final String from) { if (null == from) { throw new IllegalArgumentException("from cannot be null."); } - + this.format = format; this.from = from; } - + /** * The delta format. As at API version 1.2, only {@link DeltaExtras.FORMAT_VCDIFF} is supported. * Will never return null. @@ -35,7 +41,7 @@ public DeltaExtras(final String format, final String from) { public String getFormat() { return format; } - + /** * The id of the message the delta was generated from. * Will never return null. @@ -75,7 +81,18 @@ public String getFrom() { unpacker.skipValue(); } } - + return new DeltaExtras(format, from); } + + public static class Serializer implements JsonSerializer { + @Override + public JsonElement serialize(final DeltaExtras src, final Type typeOfSrc, final JsonSerializationContext context) { + final JsonObject json = new JsonObject(); + final Gson gson = Serialisation.gson; + json.addProperty("format", src.getFormat()); + json.addProperty("from", src.getFrom()); + return json; + } + } } diff --git a/lib/src/main/java/io/ably/lib/types/Message.java b/lib/src/main/java/io/ably/lib/types/Message.java index 4d667af81..dadb143ac 100644 --- a/lib/src/main/java/io/ably/lib/types/Message.java +++ b/lib/src/main/java/io/ably/lib/types/Message.java @@ -240,12 +240,16 @@ public static Message[] fromEncodedArray(String messagesArray, ChannelOptions ch } } - public static class Serializer extends BaseMessage.Serializer implements JsonSerializer { + public static class Serializer implements JsonSerializer { @Override public JsonElement serialize(Message message, Type typeOfMessage, JsonSerializationContext ctx) { - JsonObject json = (JsonObject) super.serialize(message, typeOfMessage, ctx); - if(message.name != null) json.addProperty("name", message.name); - if(message.extras != null) json.add("extras", message.extras.toJsonElement()); + final JsonObject json = BaseMessage.toJsonObject(message); + if (message.name != null) { + json.addProperty("name", message.name); + } + if (message.extras != null) { + json.add("extras", Serialisation.gson.toJsonTree(message.extras)); + } return json; } } 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 15c96c3f5..34f1bc2bc 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -1,6 +1,9 @@ package io.ably.lib.types; import com.google.gson.JsonElement; +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; @@ -8,26 +11,49 @@ import org.msgpack.core.MessageUnpacker; import java.io.IOException; +import java.lang.reflect.Type; public final class MessageExtras { private static final String TAG = MessageExtras.class.getName(); - - // TODO what about the 1.1 push extension? (TM2i) - private final DeltaExtras delta; // may be null - + /** + * 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 + + /** + * Creates a MessageExtras instance to be sent as extra with a Message to Ably's servers. + * + * @see Channel-based push notification example + * + * @since 1.2.1 + */ + public MessageExtras(final JsonObject raw) { + this.raw = raw; + delta = null; + } + + /** + * @since 1.2.0 + */ public MessageExtras(final DeltaExtras delta) { this.delta = delta; + raw = null; } - + public DeltaExtras getDelta() { return delta; } - /* package private */ JsonElement toJsonElement() { - return Serialisation.gson.toJsonTree(this); - } - /* package private */ void writeMsgpack(MessagePacker packer) throws IOException { int fieldCount = 0; if (this.delta != null) { @@ -58,7 +84,20 @@ public DeltaExtras getDelta() { unpacker.skipValue(); } } - + return new MessageExtras(delta); } + + public static class Serializer implements JsonSerializer { + @Override + public JsonElement serialize(final MessageExtras src, final Type typeOfSrc, final JsonSerializationContext context) { + return (null != src.raw) ? src.raw : wrapDelta(src.getDelta()); + } + + private JsonObject wrapDelta(final DeltaExtras delta) { + final JsonObject json = new JsonObject(); + json.add("delta", Serialisation.gson.toJsonTree(delta)); + return json; + } + } } diff --git a/lib/src/main/java/io/ably/lib/types/PresenceMessage.java b/lib/src/main/java/io/ably/lib/types/PresenceMessage.java index 97e6880b7..4c86bbe90 100644 --- a/lib/src/main/java/io/ably/lib/types/PresenceMessage.java +++ b/lib/src/main/java/io/ably/lib/types/PresenceMessage.java @@ -206,13 +206,13 @@ public Action deserialize(JsonElement json, Type t, JsonDeserializationContext c } } - public static class Serializer extends BaseMessage.Serializer implements JsonSerializer { + public static class Serializer implements JsonSerializer { @Override public JsonElement serialize(PresenceMessage message, Type typeOfMessage, JsonSerializationContext ctx) { - JsonObject json = (JsonObject)super.serialize(message, typeOfMessage, ctx); + final JsonObject json = BaseMessage.toJsonObject(message); if(message.action != null) json.addProperty("action", message.action.getValue()); return json; - } + } } /** diff --git a/lib/src/main/java/io/ably/lib/util/Serialisation.java b/lib/src/main/java/io/ably/lib/util/Serialisation.java index f16fe59a3..0ab1d58fc 100644 --- a/lib/src/main/java/io/ably/lib/util/Serialisation.java +++ b/lib/src/main/java/io/ably/lib/util/Serialisation.java @@ -8,7 +8,15 @@ import com.google.gson.JsonObject; import com.google.gson.JsonParser; import com.google.gson.JsonPrimitive; - +import io.ably.lib.http.HttpCore; +import io.ably.lib.platform.Platform; +import io.ably.lib.types.AblyException; +import io.ably.lib.types.DeltaExtras; +import io.ably.lib.types.ErrorInfo; +import io.ably.lib.types.Message; +import io.ably.lib.types.MessageExtras; +import io.ably.lib.types.PresenceMessage; +import io.ably.lib.types.ProtocolMessage; import org.msgpack.core.MessagePack; import org.msgpack.core.MessagePack.PackerConfig; import org.msgpack.core.MessagePack.UnpackerConfig; @@ -25,14 +33,6 @@ import java.util.Map; import java.util.Set; -import io.ably.lib.http.HttpCore; -import io.ably.lib.platform.Platform; -import io.ably.lib.types.AblyException; -import io.ably.lib.types.ErrorInfo; -import io.ably.lib.types.Message; -import io.ably.lib.types.PresenceMessage; -import io.ably.lib.types.ProtocolMessage; - public class Serialisation { public static final JsonParser gsonParser; public static final GsonBuilder gsonBuilder; @@ -45,6 +45,8 @@ public class Serialisation { gsonParser = new JsonParser(); gsonBuilder = new GsonBuilder(); gsonBuilder.registerTypeAdapter(Message.class, new Message.Serializer()); + gsonBuilder.registerTypeAdapter(MessageExtras.class, new MessageExtras.Serializer()); + gsonBuilder.registerTypeAdapter(DeltaExtras.class, new DeltaExtras.Serializer()); gsonBuilder.registerTypeAdapter(PresenceMessage.class, new PresenceMessage.Serializer()); gsonBuilder.registerTypeAdapter(PresenceMessage.Action.class, new PresenceMessage.ActionSerializer()); gsonBuilder.registerTypeAdapter(ProtocolMessage.Action.class, new ProtocolMessage.ActionSerializer()); @@ -286,4 +288,4 @@ public static JsonElement msgpackToGson(Value value) { return null; } } -} \ No newline at end of file +} From 0f8ee1d83a2ccdc6d0790563904e29c4971b8a8f Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 15:59:03 +0100 Subject: [PATCH 02/24] Add instructions for running pure unit tests to the read me. --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4d595c4d2..28fb8d98e 100644 --- a/README.md +++ b/README.md @@ -508,13 +508,18 @@ To run tests against a specific host, specify in the environment: env ABLY_ENV=staging ./gradlew testRealtimeSuite -Tests will run against sandbox by default. +Tests will run against the sandbox environment by default. Tests can be run on the Android-specific library. An Android device must be connected, either a real device or the Android emulator. ./gradlew android:connectedAndroidTest +We also have a small, fledgling set of unit tests which do not communicate with Ably's servers. +The plan is to expand this collection of tests in due course: + + ./gradlew java:runUnitTests + ### Interactive push tests End-to-end tests for push notifications (ie where the Android client is the target) can be tested interactively via a [separate app](https://github.com/ably/push-example-android). From e8d93359f9dc964dd3ac526bbb13da35d486a8ae Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 16:24:12 +0100 Subject: [PATCH 03/24] Add unit test as basic smoke test for the new constructor. --- .../io/ably/lib/types/MessageExtrasTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java diff --git a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java new file mode 100644 index 000000000..b05a1f3e2 --- /dev/null +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -0,0 +1,31 @@ +package io.ably.lib.types; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import org.junit.Test; + +public class MessageExtrasTest { + /** + * Construct an instance from a JSON source and validate that the + * serialised JSON is the same. + */ + @Test + public void raw() { + final JsonObject objectA = new JsonObject(); + objectA.addProperty("someKey", "someValue"); + + final JsonObject objectB = new JsonObject(); + objectB.addProperty("someOtherKey", "someValue"); + + final MessageExtras messageExtras = new MessageExtras(objectA); + final MessageExtras.Serializer serializer = new MessageExtras.Serializer(); + final JsonElement serialised = serializer.serialize(messageExtras, null, null); + + assertEquals(objectA, serialised); + assertNotEquals(objectB, serialised); + assertNotEquals(objectB, objectA); + } +} From 13dd4076b0c8f79b5b2904623c18edc322f4decb Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 19:03:33 +0100 Subject: [PATCH 04/24] Implement equals and hashCode on MessageExtras and DeltaExtras. --- .../main/java/io/ably/lib/types/DeltaExtras.java | 15 +++++++++++++++ .../java/io/ably/lib/types/MessageExtras.java | 15 +++++++++++++++ 2 files changed, 30 insertions(+) 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 9ae4d7232..643421603 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -13,6 +13,7 @@ import java.io.IOException; import java.lang.reflect.Type; +import java.util.Objects; public final class DeltaExtras { private static final String TAG = DeltaExtras.class.getName(); @@ -85,6 +86,20 @@ public String getFrom() { return new DeltaExtras(format, from); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + DeltaExtras that = (DeltaExtras) o; + return format.equals(that.format) && + from.equals(that.from); + } + + @Override + public int hashCode() { + return Objects.hash(format, from); + } + public static class Serializer implements JsonSerializer { @Override public JsonElement serialize(final DeltaExtras src, final Type typeOfSrc, final JsonSerializationContext context) { 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 34f1bc2bc..c01c3b263 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.lang.reflect.Type; +import java.util.Objects; public final class MessageExtras { private static final String TAG = MessageExtras.class.getName(); @@ -88,6 +89,20 @@ public DeltaExtras getDelta() { return new MessageExtras(delta); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + MessageExtras that = (MessageExtras) o; + return Objects.equals(delta, that.delta) && + Objects.equals(raw, that.raw); + } + + @Override + public int hashCode() { + return Objects.hash(delta, raw); + } + public static class Serializer implements JsonSerializer { @Override public JsonElement serialize(final MessageExtras src, final Type typeOfSrc, final JsonSerializationContext context) { From 00ad728b39d00ef22a021af48e6336ed50c5f110 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 20:38:47 +0100 Subject: [PATCH 05/24] Add unit test for the MessageExtras constructor that takes a DeltaExtras. --- .../io/ably/lib/types/MessageExtrasTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 b05a1f3e2..a2082eaa3 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -28,4 +28,31 @@ public void raw() { assertNotEquals(objectB, serialised); assertNotEquals(objectB, objectA); } + + /** + * Construct an instance with DeltaExtras and validate that the + * serialised JSON is as expected. Also validate that the DeltaExtras + * retrieved is the same. + */ + @Test + public void delta() { + final DeltaExtras deltaExtrasA = new DeltaExtras("someFormat", "someSource"); + final DeltaExtras deltaExtrasB = new DeltaExtras("someFormat", "someOtherSource"); + + final MessageExtras messageExtras = new MessageExtras(deltaExtrasA); + assertEquals(deltaExtrasA, messageExtras.getDelta()); + 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 MessageExtras.Serializer serializer = new MessageExtras.Serializer(); + final JsonElement serialised = serializer.serialize(messageExtras, null, null); + + assertEquals(expectedJsonElement, serialised); + } } From f2cc32102e50793c2bc50dc09369dd0349aa1f50 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 20:41:32 +0100 Subject: [PATCH 06/24] Refine existing unit test to validate that a MessageExtras instance created from JSON does not return a delta extras instance. --- lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java | 3 +++ 1 file changed, 3 insertions(+) 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 a2082eaa3..1cd3e04ba 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -2,6 +2,7 @@ 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; @@ -21,6 +22,8 @@ public void raw() { objectB.addProperty("someOtherKey", "someValue"); final MessageExtras messageExtras = new MessageExtras(objectA); + assertNull(messageExtras.getDelta()); + final MessageExtras.Serializer serializer = new MessageExtras.Serializer(); final JsonElement serialised = serializer.serialize(messageExtras, null, null); From ef99b5b139a2a263591a5d16d02da3bbf79d2b4d Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 20:58:54 +0100 Subject: [PATCH 07/24] Add unit test for MessagePack encode and decode of DeltaExtras within MessageExtras. --- .../io/ably/lib/types/MessageExtrasTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) 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 1cd3e04ba..4183f143a 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -6,7 +6,13 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import io.ably.lib.util.Serialisation; import org.junit.Test; +import org.msgpack.core.MessagePacker; +import org.msgpack.core.MessageUnpacker; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; public class MessageExtrasTest { /** @@ -58,4 +64,23 @@ public void delta() { assertEquals(expectedJsonElement, serialised); } + + @Test + public void deltaViaMessagePack() throws IOException { + final DeltaExtras deltaExtras = new DeltaExtras("tamrof", "morf"); + final MessageExtras messageExtras = new MessageExtras(deltaExtras); + + // Encode to MessagePack + final ByteArrayOutputStream out = new ByteArrayOutputStream(); + final MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); + messageExtras.writeMsgpack(packer); + packer.flush(); + + // Decode from MessagePack + System.out.println("len: " + out.toByteArray().length); + MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); + final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); + + assertEquals(messageExtras, unpacked); + } } From f3dfaf96f1371f926c9ccb4c17686b8fd5744e20 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 21:01:36 +0100 Subject: [PATCH 08/24] Add commentary. --- lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java | 4 ++++ 1 file changed, 4 insertions(+) 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 4183f143a..1d523a116 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -65,6 +65,10 @@ public void delta() { assertEquals(expectedJsonElement, serialised); } + /** + * Construct an instance with DeltaExtras and validate that it can be encoded + * to MessagePack and then decoded back again from MessagePack. + */ @Test public void deltaViaMessagePack() throws IOException { final DeltaExtras deltaExtras = new DeltaExtras("tamrof", "morf"); From 0752b5896305f8f5ed43f853f615ce96dc22a9c9 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Wed, 10 Jun 2020 22:05:23 +0100 Subject: [PATCH 09/24] 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; } } From c56c859fc133f73c559fa7525d9e4409c95700e2 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 11:33:36 +0100 Subject: [PATCH 10/24] Remove misleading commentary. --- lib/src/main/java/io/ably/lib/types/MessageExtras.java | 7 ------- 1 file changed, 7 deletions(-) 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 d4327682e..7eb7be884 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -20,14 +20,7 @@ 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). - */ private final DeltaExtras delta; - - /** - * Only intended for use with MessageExtras instances created to be sent to Ably's servers (outbound). - */ private final JsonObject raw; /** From 8a65f7e81aa81c0162a78f92f33d39148790e1c3 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 13:10:33 +0100 Subject: [PATCH 11/24] Improve message extras hash and equality implementations so that, if raw is available, they only use that value. Also adds a toString, which assists with test debugging apart from anything else. --- .../main/java/io/ably/lib/types/MessageExtras.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) 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 7eb7be884..c5d66f91f 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -100,13 +100,22 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; MessageExtras that = (MessageExtras) o; - return Objects.equals(delta, that.delta) && + return (null == raw) ? + Objects.equals(delta, that.delta) : Objects.equals(raw, that.raw); } @Override public int hashCode() { - return Objects.hash(delta, raw); + return (null == raw) ? Objects.hashCode(delta) : Objects.hashCode(raw); + } + + @Override + public String toString() { + return "MessageExtras{" + + "delta=" + delta + + ", raw=" + raw + + '}'; } public static class Serializer implements JsonSerializer { From cb6bc7cfc4f998b3aa93635a0648c4421e5725ed Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 13:11:13 +0100 Subject: [PATCH 12/24] Add integration test for RSL6a2. It fails for the text protocol at the moment due to an issue with inbound JSON decoding. --- .../lib/test/common/ParameterizedTest.java | 14 +++++ .../test/realtime/RealtimeMessageTest.java | 56 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/lib/src/test/java/io/ably/lib/test/common/ParameterizedTest.java b/lib/src/test/java/io/ably/lib/test/common/ParameterizedTest.java index 59f21c6b3..6ee8719dc 100644 --- a/lib/src/test/java/io/ably/lib/test/common/ParameterizedTest.java +++ b/lib/src/test/java/io/ably/lib/test/common/ParameterizedTest.java @@ -1,6 +1,8 @@ package io.ably.lib.test.common; +import java.text.SimpleDateFormat; import java.util.Arrays; +import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -75,4 +77,16 @@ protected static Param[] mergeParams(Param[] target, Param[] src) { return mergeParams(new Param[][]{target, src}); } + final SimpleDateFormat timestampDateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS"); + + /** + * Generate a channel name that conforms to Ably's restrictions but is, as far as is + * reasonably achievable, unique to the test that's running. + * + * @see What restrictions exist for the name field of a channel? + */ + protected String createChannelName(final String testTitle) { + return this.getClass().getCanonicalName() + "/" + testTitle + + "/" + timestampDateFormat.format(new Date()); + } } diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java index 40ea9dbb2..94b51020e 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java @@ -13,6 +13,7 @@ import java.util.Locale; import com.google.gson.*; +import io.ably.lib.types.MessageExtras; import io.ably.lib.util.Serialisation; import org.junit.Rule; import org.junit.Test; @@ -910,4 +911,59 @@ public void messages_from_encoded_json_array() throws AblyException { static class MessagesData { public Message[] messages; } + + /** + * Publish a message that contains extras of arbitrary creation. Validate that when we receive that message + * echoed back from the service that those extras remain intact. + * + * @see RSL6a2 + */ + @Test + public void opaque_message_extras() throws AblyException { + AblyRealtime ably = null; + try { + final JsonObject opaqueJson = new JsonObject(); + opaqueJson.addProperty("Some Property", "Lorem Ipsum"); + opaqueJson.addProperty("Some Truth", false); + opaqueJson.addProperty("Some Number", 321); + + final MessageExtras extras = new MessageExtras(opaqueJson); + final Message message = new Message(); + message.name = "The Test Message"; + message.data = "Some Value"; + message.extras = extras; + + final ClientOptions clientOptions = createOptions(testVars.keys[0].keyStr); + ably = new AblyRealtime(clientOptions); + + // create a channel and attach to it + final Channel channel = ably.channels.get(createChannelName("opaque_message_extras")); + channel.attach(); + (new ChannelWaiter(channel)).waitFor(ChannelState.attached); + assertEquals(ChannelState.attached, channel.state); + + // subscribe + final MessageWaiter messageWaiter = new MessageWaiter(channel); + + // publish and await success + final CompletionWaiter completionWaiter = new CompletionWaiter(); + channel.publish(message, completionWaiter); + completionWaiter.waitFor(); + assertTrue(completionWaiter.success); + + // wait for the subscriber to receive the message + messageWaiter.waitFor(1); + assertEquals(1, messageWaiter.receivedMessages.size()); + + // validate the contents of the received message + final Message received = messageWaiter.receivedMessages.get(0); + assertEquals("The Test Message", received.name); + assertEquals("Some Value", received.data); + assertEquals(extras, received.extras); + } finally { + if(ably != null) { + ably.close(); + } + } + } } From 41d88571bee2aef753550bade6badc11b14dfb27 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 13:31:19 +0100 Subject: [PATCH 13/24] Fix test now that MessageExtras quality check has been refactored to only inspect raw if it's available. --- lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 4cfa035a7..77deefd42 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -102,10 +102,7 @@ public void deltaViaMessagePack() throws IOException { final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); 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); + assertEquals(messageExtras, unpacked); assertNull(messageExtras.getRaw()); assertEquals(deltaExtrasJsonObject("tamrof", "morf"), unpacked.getRaw()); } From 65b2b67a57797c3750acfcf4d74831dce332d87f Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 13:32:26 +0100 Subject: [PATCH 14/24] Update gson dependency to latest release (October 2019), replacing the old version we had in place (November 2015). --- dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies.gradle b/dependencies.gradle index 2f47c374b..90a12b911 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -3,7 +3,7 @@ dependencies { implementation 'org.msgpack:msgpack-core:0.8.11' implementation 'org.java-websocket:Java-WebSocket:1.4.0' - implementation 'com.google.code.gson:gson:2.5' + implementation 'com.google.code.gson:gson:2.8.6' implementation 'com.davidehrmann.vcdiff:vcdiff-core:0.1.1' testImplementation 'org.hamcrest:hamcrest-all:1.3' testImplementation 'junit:junit:4.12' From d41635ba343aef58dcafe75e30d91f6e86e970d1 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 13:39:56 +0100 Subject: [PATCH 15/24] Simplify method names. --- lib/src/main/java/io/ably/lib/types/DeltaExtras.java | 4 ++-- lib/src/main/java/io/ably/lib/types/Message.java | 5 ++--- lib/src/main/java/io/ably/lib/types/MessageExtras.java | 8 ++++---- .../test/java/io/ably/lib/types/MessageExtrasTest.java | 8 ++++---- 4 files changed, 12 insertions(+), 13 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 becd9bf3e..6118883e8 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -51,7 +51,7 @@ public String getFrom() { return from; } - /* package private */ void writeMsgpack(MessagePacker packer) throws IOException { + /* package private */ void write(MessagePacker packer) throws IOException { packer.packMapHeader(2); packer.packString("format"); @@ -61,7 +61,7 @@ public String getFrom() { packer.packString(from); } - /* package private */ static DeltaExtras fromMessagePackMap(final Map map) throws IOException { + /* package private */ static DeltaExtras read(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()); diff --git a/lib/src/main/java/io/ably/lib/types/Message.java b/lib/src/main/java/io/ably/lib/types/Message.java index dadb143ac..ad7602017 100644 --- a/lib/src/main/java/io/ably/lib/types/Message.java +++ b/lib/src/main/java/io/ably/lib/types/Message.java @@ -11,7 +11,6 @@ import org.msgpack.core.MessageUnpacker; import io.ably.lib.util.Log; -import io.ably.lib.util.Serialisation; /** * A class representing an individual message to be sent or received @@ -92,7 +91,7 @@ void writeMsgpack(MessagePacker packer) throws IOException { } if(extras != null) { packer.packString("extras"); - extras.writeMsgpack(packer); + extras.write(packer); } } @@ -112,7 +111,7 @@ Message readMsgpack(MessageUnpacker unpacker) throws IOException { if(fieldName.equals("name")) { name = unpacker.unpackString(); } else if (fieldName == "extras") { - extras = MessageExtras.fromMsgpack(unpacker); + extras = MessageExtras.read(unpacker); } else { Log.v(TAG, "Unexpected field: " + fieldName); unpacker.skipValue(); 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 c5d66f91f..8ff4c1dfc 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -56,19 +56,19 @@ public DeltaExtras getDelta() { return raw; } - /* package private */ void writeMsgpack(MessagePacker packer) throws IOException { + /* package private */ void write(MessagePacker packer) throws IOException { if (null == raw) { // raw is null, so delta is not null packer.packMapHeader(1); packer.packString("delta"); - this.delta.writeMsgpack(packer); + delta.write(packer); } else { // raw is not null, so delta can be ignored Serialisation.gsonToMsgpack(raw, packer); } } - /* package private */ static MessageExtras fromMsgpack(MessageUnpacker unpacker) throws IOException { + /* package private */ static MessageExtras read(MessageUnpacker unpacker) throws IOException { DeltaExtras delta = null; final ImmutableValue value = unpacker.unpackValue(); @@ -81,7 +81,7 @@ public DeltaExtras getDelta() { 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); + delta = DeltaExtras.read(deltaMap); } } 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 77deefd42..380fd9ee4 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -48,12 +48,12 @@ public void rawViaMessagePack() throws IOException { // Encode to MessagePack final ByteArrayOutputStream out = new ByteArrayOutputStream(); final MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); - messageExtras.writeMsgpack(packer); + messageExtras.write(packer); packer.flush(); // Decode from MessagePack MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); - final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); + final MessageExtras unpacked = MessageExtras.read(unpacker); assertEquals(messageExtras, unpacked); } @@ -93,13 +93,13 @@ public void deltaViaMessagePack() throws IOException { // Encode to MessagePack final ByteArrayOutputStream out = new ByteArrayOutputStream(); final MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); - messageExtras.writeMsgpack(packer); + messageExtras.write(packer); packer.flush(); // Decode from MessagePack System.out.println("len: " + out.toByteArray().length); MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); - final MessageExtras unpacked = MessageExtras.fromMsgpack(unpacker); + final MessageExtras unpacked = MessageExtras.read(unpacker); assertEquals(messageExtras.getDelta(), unpacked.getDelta()); assertEquals(messageExtras, unpacked); From 3cbb7d980e766f357f52fd160d9bfa0a818a680b Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 16:37:24 +0100 Subject: [PATCH 16/24] Add SLF4J binding when running tests to help debugging. --- dependencies.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/dependencies.gradle b/dependencies.gradle index 90a12b911..f413b2081 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -12,4 +12,5 @@ dependencies { testImplementation 'org.nanohttpd:nanohttpd-websocket:2.3.0' testImplementation 'org.mockito:mockito-core:1.10.19' testImplementation 'net.jodah:concurrentunit:0.4.2' + testImplementation 'org.slf4j:slf4j-simple:1.7.30' } From deb0f59b9287b0f943aa0ace4d4620a0a0c2992e Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 16:41:32 +0100 Subject: [PATCH 17/24] Add integration test for RSL6a2, covering the opaque case (raw JSON in message extras), fixing the implementation to support this. --- .../java/io/ably/lib/types/BaseMessage.java | 76 ++++++++++++++++--- .../java/io/ably/lib/types/DeltaExtras.java | 19 +++-- .../main/java/io/ably/lib/types/Message.java | 47 ++++++++++-- .../java/io/ably/lib/types/MessageExtras.java | 25 +++++- .../io/ably/lib/types/MessageSerializer.java | 33 ++++++-- .../io/ably/lib/test/rest/RestAuthTest.java | 10 +-- 6 files changed, 166 insertions(+), 44 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/BaseMessage.java b/lib/src/main/java/io/ably/lib/types/BaseMessage.java index 2595cc13d..c80faaed4 100644 --- a/lib/src/main/java/io/ably/lib/types/BaseMessage.java +++ b/lib/src/main/java/io/ably/lib/types/BaseMessage.java @@ -5,6 +5,7 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParseException; +import com.google.gson.JsonPrimitive; import io.ably.lib.util.Base64Coder; import io.ably.lib.util.Crypto.ChannelCipher; import io.ably.lib.util.Log; @@ -50,6 +51,13 @@ public class BaseMessage implements Cloneable { */ public Object data; + private static final String TIMESTAMP = "timestamp"; + private static final String ID = "id"; + private static final String CLIENT_ID = "clientId"; + private static final String CONNECTION_ID = "connectionId"; + private static final String ENCODING = "encoding"; + private static final String DATA = "data"; + /** * Generate a String summary of this BaseMessage * @return string @@ -209,21 +217,65 @@ public static JsonObject toJsonObject(final BaseMessage message) { return json; } + /** + * Populate fields from JSON. + */ + protected void read(final JsonObject map) throws MessageDecodeException { + final Long optionalTimestamp = readLong(map, TIMESTAMP); + if (null != optionalTimestamp) { + timestamp = optionalTimestamp; // unbox + } + + id = readString(map, ID); + clientId = readString(map, CLIENT_ID); + connectionId = readString(map, CONNECTION_ID); + encoding = readString(map, ENCODING); + data = readString(map, DATA); + } + + /** + * Read an optional textual value. + * @return The value, or null if the key was not present in the map. + * @throws ClassCastException if an element exists for that key and that element is not a {@link JsonPrimitive} + * or is not a valid string value. + */ + protected String readString(final JsonObject map, final String key) { + final JsonElement element = map.get(key); + if (null == element) { + return null; + } + return element.getAsString(); + } + + /** + * Read an optional numerical value. + * @return The value, or null if the key was not present in the map. + * @throws ClassCastException if an element exists for that key and that element is not a {@link JsonPrimitive} + * or is not a valid long value. + */ + protected Long readLong(final JsonObject map, final String key) { + final JsonElement element = map.get(key); + if (null == element) { + return null; + } + return element.getAsLong(); + } + /* Msgpack processing */ boolean readField(MessageUnpacker unpacker, String fieldName, MessageFormat fieldType) throws IOException { boolean result = true; switch (fieldName) { - case "timestamp": + case TIMESTAMP: timestamp = unpacker.unpackLong(); break; - case "id": + case ID: id = unpacker.unpackString(); break; - case "clientId": + case CLIENT_ID: clientId = unpacker.unpackString(); break; - case "connectionId": + case CONNECTION_ID: connectionId = unpacker.unpackString(); break; - case "encoding": + case ENCODING: encoding = unpacker.unpackString(); break; - case "data": + case DATA: if(fieldType.getValueType().isBinaryType()) { byte[] byteData = new byte[unpacker.unpackBinaryHeader()]; unpacker.readPayload(byteData); @@ -252,27 +304,27 @@ protected int countFields() { void writeFields(MessagePacker packer) throws IOException { if(timestamp > 0) { - packer.packString("timestamp"); + packer.packString(TIMESTAMP); packer.packLong(timestamp); } if(id != null) { - packer.packString("id"); + packer.packString(ID); packer.packString(id); } if(clientId != null) { - packer.packString("clientId"); + packer.packString(CLIENT_ID); packer.packString(clientId); } if(connectionId != null) { - packer.packString("connectionId"); + packer.packString(CONNECTION_ID); packer.packString(connectionId); } if(encoding != null) { - packer.packString("encoding"); + packer.packString(ENCODING); packer.packString(encoding); } if(data != null) { - packer.packString("data"); + packer.packString(DATA); if(data instanceof byte[]) { byte[] byteData = (byte[])data; packer.packBinaryHeader(byteData.length); 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 6118883e8..54cd38d10 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -20,6 +20,9 @@ public final class DeltaExtras { public static final String FORMAT_VCDIFF = "vcdiff"; + private static final String FROM = "from"; + private static final String FORMAT = "format"; + private final String format; private final String from; @@ -54,19 +57,23 @@ public String getFrom() { /* package private */ void write(MessagePacker packer) throws IOException { packer.packMapHeader(2); - packer.packString("format"); + packer.packString(FORMAT); packer.packString(format); - packer.packString("from"); + packer.packString(FROM); packer.packString(from); } /* package private */ static DeltaExtras read(final Map map) throws IOException { - final Value format = map.get(ValueFactory.newString("format")); - final Value from = map.get(ValueFactory.newString("from")); + 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()); } + /* package private */ static DeltaExtras read(final JsonObject map) { + return new DeltaExtras(map.get(FORMAT).getAsString(), map.get(FROM).getAsString()); + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -86,8 +93,8 @@ public static class Serializer implements JsonSerializer { public JsonElement serialize(final DeltaExtras src, final Type typeOfSrc, final JsonSerializationContext context) { final JsonObject json = new JsonObject(); final Gson gson = Serialisation.gson; - json.addProperty("format", src.getFormat()); - json.addProperty("from", src.getFrom()); + json.addProperty(FORMAT, src.getFormat()); + json.addProperty(FROM, src.getFrom()); return json; } } diff --git a/lib/src/main/java/io/ably/lib/types/Message.java b/lib/src/main/java/io/ably/lib/types/Message.java index ad7602017..d3b3a8be2 100644 --- a/lib/src/main/java/io/ably/lib/types/Message.java +++ b/lib/src/main/java/io/ably/lib/types/Message.java @@ -28,6 +28,9 @@ public class Message extends BaseMessage { */ public MessageExtras extras; + private static final String NAME = "name"; + private static final String EXTRAS = "extras"; + /** * Default constructor */ @@ -108,9 +111,9 @@ Message readMsgpack(MessageUnpacker unpacker) throws IOException { if(super.readField(unpacker, fieldName, fieldFormat)) { continue; } - if(fieldName.equals("name")) { + if(fieldName.equals(NAME)) { name = unpacker.unpackString(); - } else if (fieldName == "extras") { + } else if (fieldName.equals(EXTRAS)) { extras = MessageExtras.read(unpacker); } else { Log.v(TAG, "Unexpected field: " + fieldName); @@ -170,7 +173,8 @@ static Message fromMsgpack(MessageUnpacker unpacker) throws IOException { */ public static Message fromEncoded(JsonObject messageJson, ChannelOptions channelOptions) throws MessageDecodeException { try { - Message message = Serialisation.gson.fromJson(messageJson, Message.class); + final Message message = new Message(); + message.read(messageJson); message.decode(channelOptions); return message; } catch(Exception e) { @@ -239,18 +243,49 @@ public static Message[] fromEncodedArray(String messagesArray, ChannelOptions ch } } - public static class Serializer implements JsonSerializer { + @Override + protected void read(final JsonObject map) throws MessageDecodeException { + super.read(map); + + name = readString(map, NAME); + + final JsonElement extrasElement = map.get(EXTRAS); + if (null != extrasElement) { + if (!(extrasElement instanceof JsonObject)) { + throw MessageDecodeException.fromDescription("Message extras is of type \"" + extrasElement.getClass() + "\" when expected a JSON object."); + } + extras = MessageExtras.read((JsonObject) extrasElement); + } + } + + public static class Serializer implements JsonSerializer, JsonDeserializer { @Override public JsonElement serialize(Message message, Type typeOfMessage, JsonSerializationContext ctx) { final JsonObject json = BaseMessage.toJsonObject(message); if (message.name != null) { - json.addProperty("name", message.name); + json.addProperty(NAME, message.name); } if (message.extras != null) { - json.add("extras", Serialisation.gson.toJsonTree(message.extras)); + json.add(EXTRAS, Serialisation.gson.toJsonTree(message.extras)); } return json; } + + @Override + public Message deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { + if (!(json instanceof JsonObject)) { + throw new JsonParseException("Expected an object but got \"" + json.getClass() + "\"."); + } + + final Message message = new Message(); + try { + message.read((JsonObject)json); + } catch (MessageDecodeException e) { + e.printStackTrace(); + throw new JsonParseException("Failed to deserialize Message from JSON.", e); + } + return message; + } } private static final String TAG = Message.class.getName(); 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 8ff4c1dfc..9854983e8 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -20,6 +20,8 @@ public final class MessageExtras { private static final String TAG = MessageExtras.class.getName(); + private static final String DELTA = "delta"; + private final DeltaExtras delta; private final JsonObject raw; @@ -60,7 +62,7 @@ public DeltaExtras getDelta() { if (null == raw) { // raw is null, so delta is not null packer.packMapHeader(1); - packer.packString("delta"); + packer.packString(DELTA); delta.write(packer); } else { // raw is not null, so delta can be ignored @@ -74,7 +76,7 @@ public DeltaExtras getDelta() { final ImmutableValue value = unpacker.unpackValue(); if (value instanceof ImmutableMapValue) { final Map map = ((ImmutableMapValue) value).map(); - final Value deltaValue = map.get(ValueFactory.newString("delta")); + 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. @@ -95,6 +97,21 @@ public DeltaExtras getDelta() { return new MessageExtras(raw, delta); } + /* package private */ static MessageExtras read(final JsonObject raw) throws MessageDecodeException { + DeltaExtras delta = null; + + final JsonElement deltaElement = raw.get(DELTA); + if (deltaElement instanceof JsonObject) { + delta = DeltaExtras.read((JsonObject)deltaElement); + } else { + if (null != deltaElement) { + throw MessageDecodeException.fromDescription("The value under the delta key is of the wrong type \"" + deltaElement.getClass() + "\" when expected a map."); + } + } + + return new MessageExtras(raw, delta); + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -113,7 +130,7 @@ public int hashCode() { @Override public String toString() { return "MessageExtras{" + - "delta=" + delta + + DELTA + "=" + delta + ", raw=" + raw + '}'; } @@ -126,7 +143,7 @@ public JsonElement serialize(final MessageExtras src, final Type typeOfSrc, fina private JsonObject wrapDelta(final DeltaExtras delta) { final JsonObject json = new JsonObject(); - json.add("delta", Serialisation.gson.toJsonTree(delta)); + json.add(DELTA, Serialisation.gson.toJsonTree(delta)); return json; } } diff --git a/lib/src/main/java/io/ably/lib/types/MessageSerializer.java b/lib/src/main/java/io/ably/lib/types/MessageSerializer.java index 740d167ac..7ccf84858 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageSerializer.java +++ b/lib/src/main/java/io/ably/lib/types/MessageSerializer.java @@ -1,10 +1,19 @@ package io.ably.lib.types; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import io.ably.lib.http.HttpCore; import io.ably.lib.http.HttpUtils; import io.ably.lib.util.Log; @@ -81,22 +90,22 @@ public static void write(final Map map, final MessagePacker pack packer.packString(entry.getValue()); } } - + public static Map readStringMap(final MessageUnpacker unpacker) throws IOException { final Map map = new HashMap<>(); final int fieldCount = unpacker.unpackMapHeader(); for(int i = 0; i < fieldCount; i++) { final String fieldName = unpacker.unpackString(); final MessageFormat fieldFormat = unpacker.getNextFormat(); - + // TODO is this required? It seems to be saying that if we have a null value // then nothing should be added to the map, despite the fact that the key // was potentially viable. if(fieldFormat.equals(MessageFormat.NIL)) { unpacker.unpackNil(); continue; } - + map.put(fieldName, unpacker.unpackString()); } - return map; + return map; } public static HttpCore.RequestBody asMsgpackRequest(Message.Batch[] pubSpecs) { @@ -126,8 +135,16 @@ static void writeMsgpackArray(Message.Batch[] pubSpecs, MessagePacker packer) th * JSON decode ****************************************/ - public static Message[] readJSON(byte[] packed) throws IOException { - return Serialisation.gson.fromJson(new String(packed), Message[].class); + public static Message[] readMessagesFromJson(byte[] packed) throws MessageDecodeException { + final InputStream stream = new ByteArrayInputStream(packed); + final Reader reader = new InputStreamReader(stream); + final JsonElement root = JsonParser.parseReader(reader); + + if (!(root instanceof JsonArray)) { + throw MessageDecodeException.fromDescription("Expected a JSON array but found type \"" + root.getClass() + "\"."); + } + + return Message.fromEncodedArray((JsonArray)root, null); } /**************************************** @@ -163,7 +180,7 @@ public Message[] handleResponseBody(String contentType, byte[] body) throws Ably try { Message[] messages = null; if("application/json".equals(contentType)) - messages = readJSON(body); + messages = readMessagesFromJson(body); else if("application/x-msgpack".equals(contentType)) messages = readMsgpack(body); if(messages != null) { @@ -176,7 +193,7 @@ else if("application/x-msgpack".equals(contentType)) } } return messages; - } catch(IOException e) { + } catch (MessageDecodeException e) { throw AblyException.fromThrowable(e); } } diff --git a/lib/src/test/java/io/ably/lib/test/rest/RestAuthTest.java b/lib/src/test/java/io/ably/lib/test/rest/RestAuthTest.java index c4bded04e..5e16c5978 100644 --- a/lib/src/test/java/io/ably/lib/test/rest/RestAuthTest.java +++ b/lib/src/test/java/io/ably/lib/test/rest/RestAuthTest.java @@ -1374,14 +1374,11 @@ public HttpCore.Response onRawHttpRequest(String id, HttpURLConnection conn, Str if(testParams.useBinaryProtocol) { messages[0] = MessageSerializer.readMsgpack(requestBody.getEncoded())[0]; } else { - messages[0] = MessageSerializer.readJSON(requestBody.getEncoded())[0]; + messages[0] = MessageSerializer.readMessagesFromJson(requestBody.getEncoded())[0]; } } catch (AblyException e) { e.printStackTrace(); fail("auth_clientid_publish_implicit: Unexpected exception"); - } catch (IOException e) { - e.printStackTrace(); - fail("auth_clientid_publish_implicit: Unexpected exception"); } return null; } @@ -1442,14 +1439,11 @@ public HttpCore.Response onRawHttpRequest(String id, HttpURLConnection conn, Str if(testParams.useBinaryProtocol) { messages[0] = MessageSerializer.readMsgpack(requestBody.getEncoded())[0]; } else { - messages[0] = MessageSerializer.readJSON(requestBody.getEncoded())[0]; + messages[0] = MessageSerializer.readMessagesFromJson(requestBody.getEncoded())[0]; } } catch (AblyException e) { e.printStackTrace(); fail("auth_clientid_publish_implicit: Unexpected exception"); - } catch (IOException e) { - e.printStackTrace(); - fail("auth_clientid_publish_implicit: Unexpected exception"); } return null; } From db2a17f83f28b32a7975c72f9ba7279c754a1680 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Thu, 11 Jun 2020 18:18:56 +0100 Subject: [PATCH 18/24] Revert some of the manual JSON deserialisation code I injected as it was causing REST crypto tests for text protocol to fail. --- lib/src/main/java/io/ably/lib/types/Message.java | 3 +-- .../main/java/io/ably/lib/types/MessageSerializer.java | 10 +--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/Message.java b/lib/src/main/java/io/ably/lib/types/Message.java index d3b3a8be2..f76304459 100644 --- a/lib/src/main/java/io/ably/lib/types/Message.java +++ b/lib/src/main/java/io/ably/lib/types/Message.java @@ -173,8 +173,7 @@ static Message fromMsgpack(MessageUnpacker unpacker) throws IOException { */ public static Message fromEncoded(JsonObject messageJson, ChannelOptions channelOptions) throws MessageDecodeException { try { - final Message message = new Message(); - message.read(messageJson); + Message message = Serialisation.gson.fromJson(messageJson, Message.class); message.decode(channelOptions); return message; } catch(Exception e) { diff --git a/lib/src/main/java/io/ably/lib/types/MessageSerializer.java b/lib/src/main/java/io/ably/lib/types/MessageSerializer.java index 7ccf84858..d04bf10a4 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageSerializer.java +++ b/lib/src/main/java/io/ably/lib/types/MessageSerializer.java @@ -136,15 +136,7 @@ static void writeMsgpackArray(Message.Batch[] pubSpecs, MessagePacker packer) th ****************************************/ public static Message[] readMessagesFromJson(byte[] packed) throws MessageDecodeException { - final InputStream stream = new ByteArrayInputStream(packed); - final Reader reader = new InputStreamReader(stream); - final JsonElement root = JsonParser.parseReader(reader); - - if (!(root instanceof JsonArray)) { - throw MessageDecodeException.fromDescription("Expected a JSON array but found type \"" + root.getClass() + "\"."); - } - - return Message.fromEncodedArray((JsonArray)root, null); + return Serialisation.gson.fromJson(new String(packed), Message[].class); } /**************************************** From e41c806d707f77e6755d19c1eb8555aa0688ef0a Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 11:52:51 +0100 Subject: [PATCH 19/24] Make the API to obtain a JsonObject from a MessageExtras instance public. Also: - some renaming for clarity - add code to the DeltaExtras constructor to ensure that a JsonObject is always available, making the asJsonObject method easier to reason about --- .../java/io/ably/lib/types/MessageExtras.java | 44 +++++++++++-------- .../io/ably/lib/types/MessageExtrasTest.java | 5 ++- 2 files changed, 28 insertions(+), 21 deletions(-) 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 9854983e8..375ed79ec 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -22,8 +22,8 @@ public final class MessageExtras { private static final String DELTA = "delta"; - private final DeltaExtras delta; - private final JsonObject raw; + private final DeltaExtras delta; // may be null + private final JsonObject jsonObject; // never null /** * Creates a MessageExtras instance to be sent as extra with a Message to Ably's servers. @@ -32,21 +32,23 @@ public final class MessageExtras { * * @since 1.2.1 */ - public MessageExtras(final JsonObject raw) { - this.raw = raw; - delta = null; + public MessageExtras(final JsonObject jsonObject) { + this(jsonObject, null); } /** * @since 1.2.0 */ public MessageExtras(final DeltaExtras delta) { - this.delta = delta; - raw = null; + this(Serializer.wrapDelta(delta), delta); } - private MessageExtras(final JsonObject raw, final DeltaExtras delta) { - this.raw = raw; + private MessageExtras(final JsonObject jsonObject, final DeltaExtras delta) { + if (null == jsonObject) { + throw new NullPointerException("jsonObject cannot be null."); + } + + this.jsonObject = jsonObject; this.delta = delta; } @@ -54,19 +56,19 @@ public DeltaExtras getDelta() { return delta; } - /* package private */ JsonObject getRaw() { - return raw; + public JsonObject asJsonObject() { + return jsonObject; } /* package private */ void write(MessagePacker packer) throws IOException { - if (null == raw) { + if (null == jsonObject) { // raw is null, so delta is not null packer.packMapHeader(1); packer.packString(DELTA); delta.write(packer); } else { // raw is not null, so delta can be ignored - Serialisation.gsonToMsgpack(raw, packer); + Serialisation.gsonToMsgpack(jsonObject, packer); } } @@ -117,31 +119,35 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; MessageExtras that = (MessageExtras) o; - return (null == raw) ? + return (null == jsonObject) ? Objects.equals(delta, that.delta) : - Objects.equals(raw, that.raw); + Objects.equals(jsonObject, that.jsonObject); } @Override public int hashCode() { - return (null == raw) ? Objects.hashCode(delta) : Objects.hashCode(raw); + return (null == jsonObject) ? Objects.hashCode(delta) : Objects.hashCode(jsonObject); } @Override public String toString() { return "MessageExtras{" + DELTA + "=" + delta + - ", raw=" + raw + + ", raw=" + jsonObject + '}'; } public static class Serializer implements JsonSerializer { @Override public JsonElement serialize(final MessageExtras src, final Type typeOfSrc, final JsonSerializationContext context) { - return (null != src.raw) ? src.raw : wrapDelta(src.getDelta()); + return (null != src.jsonObject) ? src.jsonObject : wrapDelta(src.getDelta()); } - private JsonObject wrapDelta(final DeltaExtras delta) { + public static JsonObject wrapDelta(final DeltaExtras delta) { + if (null == delta) { + throw new NullPointerException("delta cannot be null."); + } + final JsonObject json = new JsonObject(); json.add(DELTA, Serialisation.gson.toJsonTree(delta)); return json; 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 380fd9ee4..51ea110f3 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -89,6 +89,8 @@ public void delta() { public void deltaViaMessagePack() throws IOException { final DeltaExtras deltaExtras = new DeltaExtras("tamrof", "morf"); final MessageExtras messageExtras = new MessageExtras(deltaExtras); + final JsonObject expectedMessageExtrasJsonObject = deltaExtrasJsonObject("tamrof", "morf"); + assertEquals(expectedMessageExtrasJsonObject, messageExtras.asJsonObject()); // Encode to MessagePack final ByteArrayOutputStream out = new ByteArrayOutputStream(); @@ -103,8 +105,7 @@ public void deltaViaMessagePack() throws IOException { assertEquals(messageExtras.getDelta(), unpacked.getDelta()); assertEquals(messageExtras, unpacked); - assertNull(messageExtras.getRaw()); - assertEquals(deltaExtrasJsonObject("tamrof", "morf"), unpacked.getRaw()); + assertEquals(expectedMessageExtrasJsonObject, unpacked.asJsonObject()); } private static JsonObject deltaExtrasJsonObject(final String format, final String from) { From 99e3407a67f07052466a7900e08d2f5998095612 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 12:23:51 +0100 Subject: [PATCH 20/24] Add unit tests for constructor null arguments on MessageExtras. --- .../test/java/io/ably/lib/types/MessageExtrasTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 51ea110f3..5a989a875 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -58,6 +58,11 @@ public void rawViaMessagePack() throws IOException { assertEquals(messageExtras, unpacked); } + @Test(expected = NullPointerException.class) + public void rawNullArgument() { + new MessageExtras((JsonObject)null); + } + /** * Construct an instance with DeltaExtras and validate that the * serialised JSON is as expected. Also validate that the DeltaExtras @@ -108,6 +113,11 @@ public void deltaViaMessagePack() throws IOException { assertEquals(expectedMessageExtrasJsonObject, unpacked.asJsonObject()); } + @Test(expected = NullPointerException.class) + public void deltaNullArgument() { + new MessageExtras((DeltaExtras)null); + } + private static JsonObject deltaExtrasJsonObject(final String format, final String from) { final JsonObject deltaExtrasJsonElement = new JsonObject(); deltaExtrasJsonElement.addProperty("format", format); From d6ff228bfb8c4c939ed63d5cf1f699092c8ed143 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 15:35:19 +0100 Subject: [PATCH 21/24] Remove the unnecessary DeltaExtras constructor on MessageExtras. --- .../java/io/ably/lib/types/MessageExtras.java | 7 -- .../realtime/RealtimeDeltaDecoderTest.java | 23 +++++-- .../io/ably/lib/types/MessageExtrasTest.java | 64 ------------------- 3 files changed, 16 insertions(+), 78 deletions(-) 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 375ed79ec..8a921a8b6 100644 --- a/lib/src/main/java/io/ably/lib/types/MessageExtras.java +++ b/lib/src/main/java/io/ably/lib/types/MessageExtras.java @@ -36,13 +36,6 @@ public MessageExtras(final JsonObject jsonObject) { this(jsonObject, null); } - /** - * @since 1.2.0 - */ - public MessageExtras(final DeltaExtras delta) { - this(Serializer.wrapDelta(delta), delta); - } - private MessageExtras(final JsonObject jsonObject, final DeltaExtras delta) { if (null == jsonObject) { throw new NullPointerException("jsonObject cannot be null."); diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeDeltaDecoderTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeDeltaDecoderTest.java index ac1213106..c360aa89f 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeDeltaDecoderTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeDeltaDecoderTest.java @@ -5,6 +5,7 @@ import java.util.Objects; +import com.google.gson.JsonObject; import org.junit.Rule; import org.junit.Test; import org.junit.rules.Timeout; @@ -133,19 +134,27 @@ public ITransport getTransport(ITransport.TransportParams transportParams, Conne * Special transport class that corrupts the order bookkeeping of delta messages to allow testing delta recovery. */ private static class OutOfOrderDeltasWebsocketTransportMock extends WebSocketTransport { - + private static final String DELTA = "delta"; private OutOfOrderDeltasWebsocketTransportMock(TransportParams transportParams, ConnectionManager connectionManager) { super(transportParams, connectionManager); } @Override - protected void preProcessReceivedMessage(ProtocolMessage message) { - if(message.action == ProtocolMessage.Action.message && - message.messages[0].extras != null && - message.messages[0].extras.getDelta() != null) { - final String format = message.messages[0].extras.getDelta().getFormat(); - message.messages[0].extras = new MessageExtras(new DeltaExtras(format, "")); + protected void preProcessReceivedMessage(ProtocolMessage protocolMessage) { + if(protocolMessage.action == ProtocolMessage.Action.message) { + for (final Message message : protocolMessage.messages) { + final MessageExtras extras = message.extras; + if (extras != null) { + final JsonObject json = message.extras.asJsonObject(); + + if (json.has(DELTA)) { + // This MessageExtras (json) has DeltaExtras. + // Corrupt it by replacing the value at the from key with an empty string. + json.getAsJsonObject(DELTA).addProperty("from", ""); + } + } + } } } } 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 5a989a875..df5328e67 100644 --- a/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java +++ b/lib/src/test/java/io/ably/lib/types/MessageExtrasTest.java @@ -62,68 +62,4 @@ public void rawViaMessagePack() throws IOException { public void rawNullArgument() { new MessageExtras((JsonObject)null); } - - /** - * Construct an instance with DeltaExtras and validate that the - * serialised JSON is as expected. Also validate that the DeltaExtras - * retrieved is the same. - */ - @Test - public void delta() { - final DeltaExtras deltaExtrasA = new DeltaExtras("someFormat", "someSource"); - final DeltaExtras deltaExtrasB = new DeltaExtras("someFormat", "someOtherSource"); - - final MessageExtras messageExtras = new MessageExtras(deltaExtrasA); - assertEquals(deltaExtrasA, messageExtras.getDelta()); - assertNotEquals(deltaExtrasB, messageExtras.getDelta()); - assertNotEquals(deltaExtrasB, deltaExtrasA); - - final JsonObject expectedJsonElement = deltaExtrasJsonObject("someFormat", "someSource"); - - final MessageExtras.Serializer serializer = new MessageExtras.Serializer(); - final JsonElement serialised = serializer.serialize(messageExtras, null, null); - - assertEquals(expectedJsonElement, serialised); - } - - /** - * Construct an instance with DeltaExtras and validate that it can be encoded - * to MessagePack and then decoded back again from MessagePack. - */ - @Test - public void deltaViaMessagePack() throws IOException { - final DeltaExtras deltaExtras = new DeltaExtras("tamrof", "morf"); - final MessageExtras messageExtras = new MessageExtras(deltaExtras); - final JsonObject expectedMessageExtrasJsonObject = deltaExtrasJsonObject("tamrof", "morf"); - assertEquals(expectedMessageExtrasJsonObject, messageExtras.asJsonObject()); - - // Encode to MessagePack - final ByteArrayOutputStream out = new ByteArrayOutputStream(); - final MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); - messageExtras.write(packer); - packer.flush(); - - // Decode from MessagePack - System.out.println("len: " + out.toByteArray().length); - MessageUnpacker unpacker = Serialisation.msgpackUnpackerConfig.newUnpacker(out.toByteArray()); - final MessageExtras unpacked = MessageExtras.read(unpacker); - - assertEquals(messageExtras.getDelta(), unpacked.getDelta()); - assertEquals(messageExtras, unpacked); - assertEquals(expectedMessageExtrasJsonObject, unpacked.asJsonObject()); - } - - @Test(expected = NullPointerException.class) - public void deltaNullArgument() { - new MessageExtras((DeltaExtras)null); - } - - 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; - } } From 00f1167bd45b71e6d2c4852ccbd0bf7d9ba56504 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 15:38:42 +0100 Subject: [PATCH 22/24] Remove DeltaExtras JSON serialisation support as it's not needed outbound. --- lib/src/main/java/io/ably/lib/types/DeltaExtras.java | 11 ----------- lib/src/main/java/io/ably/lib/util/Serialisation.java | 1 - 2 files changed, 12 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 54cd38d10..64559b93c 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -87,15 +87,4 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(format, from); } - - public static class Serializer implements JsonSerializer { - @Override - public JsonElement serialize(final DeltaExtras src, final Type typeOfSrc, final JsonSerializationContext context) { - final JsonObject json = new JsonObject(); - final Gson gson = Serialisation.gson; - json.addProperty(FORMAT, src.getFormat()); - json.addProperty(FROM, src.getFrom()); - return json; - } - } } diff --git a/lib/src/main/java/io/ably/lib/util/Serialisation.java b/lib/src/main/java/io/ably/lib/util/Serialisation.java index 0ab1d58fc..5455f7a41 100644 --- a/lib/src/main/java/io/ably/lib/util/Serialisation.java +++ b/lib/src/main/java/io/ably/lib/util/Serialisation.java @@ -46,7 +46,6 @@ public class Serialisation { gsonBuilder = new GsonBuilder(); gsonBuilder.registerTypeAdapter(Message.class, new Message.Serializer()); gsonBuilder.registerTypeAdapter(MessageExtras.class, new MessageExtras.Serializer()); - gsonBuilder.registerTypeAdapter(DeltaExtras.class, new DeltaExtras.Serializer()); gsonBuilder.registerTypeAdapter(PresenceMessage.class, new PresenceMessage.Serializer()); gsonBuilder.registerTypeAdapter(PresenceMessage.Action.class, new PresenceMessage.ActionSerializer()); gsonBuilder.registerTypeAdapter(ProtocolMessage.Action.class, new ProtocolMessage.ActionSerializer()); From 7fbc7b1379ecc46f212efa6d038db037d5732962 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 15:42:40 +0100 Subject: [PATCH 23/24] Make the DeltaExtras constructor private as it's not needed in the public API. --- lib/src/main/java/io/ably/lib/types/DeltaExtras.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 64559b93c..f3615d0da 100644 --- a/lib/src/main/java/io/ably/lib/types/DeltaExtras.java +++ b/lib/src/main/java/io/ably/lib/types/DeltaExtras.java @@ -26,7 +26,7 @@ public final class DeltaExtras { private final String format; private final String from; - public DeltaExtras(final String format, final String from) { + private DeltaExtras(final String format, final String from) { if (null == format) { throw new IllegalArgumentException("format cannot be null."); } From 5101238c71c3a05e1ee879e6b3c494d1d54a08e7 Mon Sep 17 00:00:00 2001 From: Quintin Willison Date: Fri, 12 Jun 2020 15:51:20 +0100 Subject: [PATCH 24/24] Fix oversight in my base message deserialisation code, covering the case where a key is populated but with JSON null. --- lib/src/main/java/io/ably/lib/types/BaseMessage.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/io/ably/lib/types/BaseMessage.java b/lib/src/main/java/io/ably/lib/types/BaseMessage.java index c80faaed4..29c0f003f 100644 --- a/lib/src/main/java/io/ably/lib/types/BaseMessage.java +++ b/lib/src/main/java/io/ably/lib/types/BaseMessage.java @@ -3,6 +3,7 @@ import com.davidehrmann.vcdiff.VCDiffDecoder; import com.davidehrmann.vcdiff.VCDiffDecoderBuilder; import com.google.gson.JsonElement; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import com.google.gson.JsonParseException; import com.google.gson.JsonPrimitive; @@ -241,7 +242,7 @@ protected void read(final JsonObject map) throws MessageDecodeException { */ protected String readString(final JsonObject map, final String key) { final JsonElement element = map.get(key); - if (null == element) { + if (null == element || element instanceof JsonNull) { return null; } return element.getAsString(); @@ -255,7 +256,7 @@ protected String readString(final JsonObject map, final String key) { */ protected Long readLong(final JsonObject map, final String key) { final JsonElement element = map.get(key); - if (null == element) { + if (null == element || element instanceof JsonNull) { return null; } return element.getAsLong();