Skip to content

Commit 7755418

Browse files
sacOO7sacOO7
committed
[ECO-5386] Fixed code as per review comments
1. Made LiveObjectSerializer variable thread safe inside LiveObjectsHelper 2. Added null check for MessageUnpacker inside readObjectMessage
1 parent 8d23a1f commit 7755418

4 files changed

Lines changed: 34 additions & 22 deletions

File tree

‎lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
public class LiveObjectsHelper {
99

1010
private static final String TAG = LiveObjectsHelper.class.getName();
11-
private static LiveObjectSerializer liveObjectSerializer;
11+
private static volatile LiveObjectSerializer liveObjectSerializer;
1212

1313
public static LiveObjectsPlugin tryInitializeLiveObjectsPlugin(AblyRealtime ablyRealtime) {
1414
try {
@@ -25,14 +25,17 @@ public static LiveObjectsPlugin tryInitializeLiveObjectsPlugin(AblyRealtime ably
2525
}
2626

2727
public static LiveObjectSerializer getLiveObjectSerializer() {
28-
if (liveObjectSerializer == null) {
29-
try {
30-
Class<?> serializerClass = Class.forName("io.ably.lib.objects.serialization.DefaultLiveObjectSerializer");
31-
liveObjectSerializer = (LiveObjectSerializer) serializerClass.getDeclaredConstructor().newInstance();
32-
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException | NoSuchMethodException |
33-
InvocationTargetException e) {
34-
Log.e(TAG, "Failed to init LiveObjectSerializer, LiveObjects plugin not included in the classpath", e);
35-
return null;
28+
synchronized (LiveObjectsHelper.class) {
29+
if (liveObjectSerializer == null) {
30+
try {
31+
Class<?> serializerClass = Class.forName("io.ably.lib.objects.serialization.DefaultLiveObjectSerializer");
32+
liveObjectSerializer = (LiveObjectSerializer) serializerClass.getDeclaredConstructor().newInstance();
33+
} catch (ClassNotFoundException | InstantiationException | IllegalAccessException |
34+
NoSuchMethodException |
35+
InvocationTargetException e) {
36+
Log.e(TAG, "Failed to init LiveObjectSerializer, LiveObjects plugin not included in the classpath", e);
37+
return null;
38+
}
3639
}
3740
}
3841
return liveObjectSerializer;

‎lib/src/main/java/io/ably/lib/types/ProtocolMessage.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ void writeMsgpack(MessagePacker packer) throws IOException {
160160
if(params != null) ++fieldCount;
161161
if(channelSerial != null) ++fieldCount;
162162
if(annotations != null) ++fieldCount;
163-
if(state != null) ++fieldCount;
163+
if(state != null && LiveObjectsHelper.getLiveObjectSerializer() != null) ++fieldCount;
164164
packer.packMapHeader(fieldCount);
165165
packer.packString("action");
166166
packer.packInt(action.getValue());

‎live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ internal fun ObjectMessage.writeMsgpack(packer: MessagePacker) {
9191
* Read an ObjectMessage from MessageUnpacker
9292
*/
9393
internal fun readObjectMessage(unpacker: MessageUnpacker): ObjectMessage {
94+
if (unpacker.nextFormat == MessageFormat.NIL) {
95+
unpacker.unpackNil()
96+
return ObjectMessage() // default/empty message
97+
}
98+
9499
val fieldCount = unpacker.unpackMapHeader()
95100

96101
var id: String? = null
@@ -105,7 +110,7 @@ internal fun readObjectMessage(unpacker: MessageUnpacker): ObjectMessage {
105110

106111
for (i in 0 until fieldCount) {
107112
val fieldName = unpacker.unpackString().intern()
108-
val fieldFormat = unpacker.getNextFormat()
113+
val fieldFormat = unpacker.nextFormat
109114

110115
if (fieldFormat == MessageFormat.NIL) {
111116
unpacker.unpackNil()
@@ -207,7 +212,7 @@ private fun ObjectOperation.writeMsgpack(packer: MessagePacker) {
207212
private fun readObjectOperation(unpacker: MessageUnpacker): ObjectOperation {
208213
val fieldCount = unpacker.unpackMapHeader()
209214

210-
var action: ObjectOperationAction = ObjectOperationAction.MapCreate // Default value
215+
var action: ObjectOperationAction? = null
211216
var objectId: String = ""
212217
var mapOp: ObjectMapOp? = null
213218
var counterOp: ObjectCounterOp? = null
@@ -219,7 +224,7 @@ private fun readObjectOperation(unpacker: MessageUnpacker): ObjectOperation {
219224

220225
for (i in 0 until fieldCount) {
221226
val fieldName = unpacker.unpackString().intern()
222-
val fieldFormat = unpacker.getNextFormat()
227+
val fieldFormat = unpacker.nextFormat
223228

224229
if (fieldFormat == MessageFormat.NIL) {
225230
unpacker.unpackNil()
@@ -246,6 +251,10 @@ private fun readObjectOperation(unpacker: MessageUnpacker): ObjectOperation {
246251
}
247252
}
248253

254+
if (action == null) {
255+
throw IllegalArgumentException("Missing required 'action' field in ObjectOperation")
256+
}
257+
249258
return ObjectOperation(
250259
action = action,
251260
objectId = objectId,
@@ -315,7 +324,7 @@ private fun readObjectState(unpacker: MessageUnpacker): ObjectState {
315324

316325
for (i in 0 until fieldCount) {
317326
val fieldName = unpacker.unpackString().intern()
318-
val fieldFormat = unpacker.getNextFormat()
327+
val fieldFormat = unpacker.nextFormat
319328

320329
if (fieldFormat == MessageFormat.NIL) {
321330
unpacker.unpackNil()
@@ -382,7 +391,7 @@ private fun readObjectMapOp(unpacker: MessageUnpacker): ObjectMapOp {
382391

383392
for (i in 0 until fieldCount) {
384393
val fieldName = unpacker.unpackString().intern()
385-
val fieldFormat = unpacker.getNextFormat()
394+
val fieldFormat = unpacker.nextFormat
386395

387396
if (fieldFormat == MessageFormat.NIL) {
388397
unpacker.unpackNil()
@@ -425,7 +434,7 @@ private fun readObjectCounterOp(unpacker: MessageUnpacker): ObjectCounterOp {
425434

426435
for (i in 0 until fieldCount) {
427436
val fieldName = unpacker.unpackString().intern()
428-
val fieldFormat = unpacker.getNextFormat()
437+
val fieldFormat = unpacker.nextFormat
429438

430439
if (fieldFormat == MessageFormat.NIL) {
431440
unpacker.unpackNil()
@@ -478,7 +487,7 @@ private fun readObjectMap(unpacker: MessageUnpacker): ObjectMap {
478487

479488
for (i in 0 until fieldCount) {
480489
val fieldName = unpacker.unpackString().intern()
481-
val fieldFormat = unpacker.getNextFormat()
490+
val fieldFormat = unpacker.nextFormat
482491

483492
if (fieldFormat == MessageFormat.NIL) {
484493
unpacker.unpackNil()
@@ -531,7 +540,7 @@ private fun readObjectCounter(unpacker: MessageUnpacker): ObjectCounter {
531540

532541
for (i in 0 until fieldCount) {
533542
val fieldName = unpacker.unpackString().intern()
534-
val fieldFormat = unpacker.getNextFormat()
543+
val fieldFormat = unpacker.nextFormat
535544

536545
if (fieldFormat == MessageFormat.NIL) {
537546
unpacker.unpackNil()
@@ -587,7 +596,7 @@ private fun readObjectMapEntry(unpacker: MessageUnpacker): ObjectMapEntry {
587596

588597
for (i in 0 until fieldCount) {
589598
val fieldName = unpacker.unpackString().intern()
590-
val fieldFormat = unpacker.getNextFormat()
599+
val fieldFormat = unpacker.nextFormat
591600

592601
if (fieldFormat == MessageFormat.NIL) {
593602
unpacker.unpackNil()
@@ -667,7 +676,7 @@ private fun readObjectData(unpacker: MessageUnpacker): ObjectData {
667676

668677
for (i in 0 until fieldCount) {
669678
val fieldName = unpacker.unpackString().intern()
670-
val fieldFormat = unpacker.getNextFormat()
679+
val fieldFormat = unpacker.nextFormat
671680

672681
if (fieldFormat == MessageFormat.NIL) {
673682
unpacker.unpackNil()

‎live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal class DefaultLiveObjectSerializer : LiveObjectSerializer {
2222
return Array(objectMessagesCount) { readObjectMessage(unpacker) }
2323
}
2424

25-
override fun writeMsgpackArray(objects: Array<out Any>?, packer: MessagePacker) {
25+
override fun writeMsgpackArray(objects: Array<out Any>, packer: MessagePacker) {
2626
val objectMessages: Array<ObjectMessage> = objects as Array<ObjectMessage>
2727
packer.packArrayHeader(objectMessages.size)
2828
objectMessages.forEach { it.writeMsgpack(packer) }
@@ -35,7 +35,7 @@ internal class DefaultLiveObjectSerializer : LiveObjectSerializer {
3535
}.toTypedArray()
3636
}
3737

38-
override fun asJsonArray(objects: Array<out Any>?): JsonArray {
38+
override fun asJsonArray(objects: Array<out Any>): JsonArray {
3939
val objectMessages: Array<ObjectMessage> = objects as Array<ObjectMessage>
4040
val jsonArray = JsonArray()
4141
for (objectMessage in objectMessages) {

0 commit comments

Comments
 (0)