Skip to content

Commit 36f9844

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 36f9844

5 files changed

Lines changed: 45 additions & 25 deletions

File tree

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

Lines changed: 11 additions & 8 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 {
@@ -26,13 +26,16 @@ public static LiveObjectsPlugin tryInitializeLiveObjectsPlugin(AblyRealtime ably
2626

2727
public static LiveObjectSerializer getLiveObjectSerializer() {
2828
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;
29+
synchronized (LiveObjectsHelper.class) {
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: 29 additions & 14 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,16 +224,19 @@ 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()
226231
continue
227232
}
228233

229234
when (fieldName) {
230-
"action" -> action = ObjectOperationAction.entries.find { it.code == unpacker.unpackInt() }
231-
?: throw IllegalArgumentException("Unknown ObjectOperationAction code")
235+
"action" -> {
236+
val actionCode = unpacker.unpackInt()
237+
action = ObjectOperationAction.entries.find { it.code == actionCode }
238+
?: throw IllegalArgumentException("Unknown ObjectOperationAction code: $actionCode")
239+
}
232240
"objectId" -> objectId = unpacker.unpackString()
233241
"mapOp" -> mapOp = readObjectMapOp(unpacker)
234242
"counterOp" -> counterOp = readObjectCounterOp(unpacker)
@@ -246,6 +254,10 @@ private fun readObjectOperation(unpacker: MessageUnpacker): ObjectOperation {
246254
}
247255
}
248256

257+
if (action == null) {
258+
throw IllegalArgumentException("Missing required 'action' field in ObjectOperation")
259+
}
260+
249261
return ObjectOperation(
250262
action = action,
251263
objectId = objectId,
@@ -315,7 +327,7 @@ private fun readObjectState(unpacker: MessageUnpacker): ObjectState {
315327

316328
for (i in 0 until fieldCount) {
317329
val fieldName = unpacker.unpackString().intern()
318-
val fieldFormat = unpacker.getNextFormat()
330+
val fieldFormat = unpacker.nextFormat
319331

320332
if (fieldFormat == MessageFormat.NIL) {
321333
unpacker.unpackNil()
@@ -382,7 +394,7 @@ private fun readObjectMapOp(unpacker: MessageUnpacker): ObjectMapOp {
382394

383395
for (i in 0 until fieldCount) {
384396
val fieldName = unpacker.unpackString().intern()
385-
val fieldFormat = unpacker.getNextFormat()
397+
val fieldFormat = unpacker.nextFormat
386398

387399
if (fieldFormat == MessageFormat.NIL) {
388400
unpacker.unpackNil()
@@ -425,7 +437,7 @@ private fun readObjectCounterOp(unpacker: MessageUnpacker): ObjectCounterOp {
425437

426438
for (i in 0 until fieldCount) {
427439
val fieldName = unpacker.unpackString().intern()
428-
val fieldFormat = unpacker.getNextFormat()
440+
val fieldFormat = unpacker.nextFormat
429441

430442
if (fieldFormat == MessageFormat.NIL) {
431443
unpacker.unpackNil()
@@ -478,16 +490,19 @@ private fun readObjectMap(unpacker: MessageUnpacker): ObjectMap {
478490

479491
for (i in 0 until fieldCount) {
480492
val fieldName = unpacker.unpackString().intern()
481-
val fieldFormat = unpacker.getNextFormat()
493+
val fieldFormat = unpacker.nextFormat
482494

483495
if (fieldFormat == MessageFormat.NIL) {
484496
unpacker.unpackNil()
485497
continue
486498
}
487499

488500
when (fieldName) {
489-
"semantics" -> semantics = MapSemantics.entries.find { it.code == unpacker.unpackInt() }
490-
?: throw IllegalArgumentException("Unknown MapSemantics code")
501+
"semantics" -> {
502+
val semanticsCode = unpacker.unpackInt()
503+
semantics = MapSemantics.entries.find { it.code == semanticsCode }
504+
?: throw IllegalArgumentException("Unknown MapSemantics code: $semanticsCode")
505+
}
491506
"entries" -> {
492507
val mapSize = unpacker.unpackMapHeader()
493508
val tempMap = mutableMapOf<String, ObjectMapEntry>()
@@ -531,7 +546,7 @@ private fun readObjectCounter(unpacker: MessageUnpacker): ObjectCounter {
531546

532547
for (i in 0 until fieldCount) {
533548
val fieldName = unpacker.unpackString().intern()
534-
val fieldFormat = unpacker.getNextFormat()
549+
val fieldFormat = unpacker.nextFormat
535550

536551
if (fieldFormat == MessageFormat.NIL) {
537552
unpacker.unpackNil()
@@ -587,7 +602,7 @@ private fun readObjectMapEntry(unpacker: MessageUnpacker): ObjectMapEntry {
587602

588603
for (i in 0 until fieldCount) {
589604
val fieldName = unpacker.unpackString().intern()
590-
val fieldFormat = unpacker.getNextFormat()
605+
val fieldFormat = unpacker.nextFormat
591606

592607
if (fieldFormat == MessageFormat.NIL) {
593608
unpacker.unpackNil()
@@ -667,7 +682,7 @@ private fun readObjectData(unpacker: MessageUnpacker): ObjectData {
667682

668683
for (i in 0 until fieldCount) {
669684
val fieldName = unpacker.unpackString().intern()
670-
val fieldFormat = unpacker.getNextFormat()
685+
val fieldFormat = unpacker.nextFormat
671686

672687
if (fieldFormat == MessageFormat.NIL) {
673688
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) {

‎live-objects/src/test/kotlin/io/ably/lib/objects/unit/fixtures/ObjectMessageFixture.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ internal fun dummyObjectMessageWithJsonObjectData(): ObjectMessage {
142142
val jsonObjectMap = dummyObjectMap.copy(entries = mapOf("dummy-key" to jsonObjectMapEntry))
143143
val jsonObjectMapOp = dummyObjectMapOp.copy(data = dummyJsonObjectValue)
144144
val jsonObjectOperation = dummyObjectOperation.copy(
145+
action = ObjectOperationAction.MapSet,
145146
mapOp = jsonObjectMapOp,
146147
map = jsonObjectMap
147148
)
@@ -160,6 +161,7 @@ internal fun dummyObjectMessageWithJsonArrayData(): ObjectMessage {
160161
val jsonArrayMap = dummyObjectMap.copy(entries = mapOf("dummy-key" to jsonArrayMapEntry))
161162
val jsonArrayMapOp = dummyObjectMapOp.copy(data = dummyJsonArrayValue)
162163
val jsonArrayOperation = dummyObjectOperation.copy(
164+
action = ObjectOperationAction.MapSet,
163165
mapOp = jsonArrayMapOp,
164166
map = jsonArrayMap
165167
)

0 commit comments

Comments
 (0)