Skip to content

Commit fbe9f74

Browse files
jinliu9508jinliu9508
committed
Resolve the deadlock and pass the unit test
1 parent c24f55f commit fbe9f74

File tree

3 files changed

+76
-55
lines changed

3 files changed

+76
-55
lines changed

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt

Lines changed: 71 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -80,33 +80,52 @@ open class Model(
8080
* @param jsonObject The [JSONObject] to initialize this model from.
8181
*/
8282
fun initializeFromJson(jsonObject: JSONObject) {
83-
data.clear()
84-
for (property in jsonObject.keys()) {
85-
val jsonValue = jsonObject.get(property)
86-
if (jsonValue is JSONObject) {
87-
val childModel = createModelForProperty(property, jsonValue)
88-
if (childModel != null) {
89-
data[property] = childModel
90-
}
91-
} else if (jsonValue is JSONArray) {
92-
val listOfItems = createListForProperty(property, jsonValue)
93-
if (listOfItems != null) {
94-
data[property] = listOfItems
95-
}
96-
} else {
97-
val method = this.javaClass.methods.firstOrNull { it.returnType != Void::class.java && it.name.contains(property, true) }
98-
99-
if (method == null) {
100-
data[property] = jsonObject.get(property)
83+
synchronized(data) {
84+
data.clear()
85+
for (property in jsonObject.keys()) {
86+
val jsonValue = jsonObject.get(property)
87+
if (jsonValue is JSONObject) {
88+
val childModel = createModelForProperty(property, jsonValue)
89+
if (childModel != null) {
90+
data[property] = childModel
91+
}
92+
} else if (jsonValue is JSONArray) {
93+
val listOfItems = createListForProperty(property, jsonValue)
94+
if (listOfItems != null) {
95+
data[property] = listOfItems
96+
}
10197
} else {
102-
when (method.returnType) {
103-
Double::class.java, java.lang.Double::class.java -> data[property] = jsonObject.getDouble(property)
104-
Long::class.java, java.lang.Long::class.java -> data[property] = jsonObject.getLong(property)
105-
Float::class.java, java.lang.Float::class.java -> data[property] = jsonObject.getDouble(property).toFloat()
106-
Int::class.java, java.lang.Integer::class.java -> data[property] = jsonObject.getInt(property)
107-
Boolean::class.java, java.lang.Boolean::class.java -> data[property] = jsonObject.getBoolean(property)
108-
String::class.java, java.lang.String::class.java -> data[property] = jsonObject.getString(property)
109-
else -> data[property] = jsonObject.get(property)
98+
val method = this.javaClass.methods.firstOrNull {
99+
it.returnType != Void::class.java && it.name.contains(
100+
property,
101+
true
102+
)
103+
}
104+
105+
if (method == null) {
106+
data[property] = jsonObject.get(property)
107+
} else {
108+
when (method.returnType) {
109+
Double::class.java, java.lang.Double::class.java -> data[property] =
110+
jsonObject.getDouble(property)
111+
112+
Long::class.java, java.lang.Long::class.java -> data[property] =
113+
jsonObject.getLong(property)
114+
115+
Float::class.java, java.lang.Float::class.java -> data[property] =
116+
jsonObject.getDouble(property).toFloat()
117+
118+
Int::class.java, java.lang.Integer::class.java -> data[property] =
119+
jsonObject.getInt(property)
120+
121+
Boolean::class.java, java.lang.Boolean::class.java -> data[property] =
122+
jsonObject.getBoolean(property)
123+
124+
String::class.java, java.lang.String::class.java -> data[property] =
125+
jsonObject.getString(property)
126+
127+
else -> data[property] = jsonObject.get(property)
128+
}
110129
}
111130
}
112131
}
@@ -141,8 +160,10 @@ open class Model(
141160
}
142161

143162
synchronized(initializationLock) {
144-
data.clear()
145-
data.putAll(newData)
163+
synchronized(data) {
164+
data.clear()
165+
data.putAll(newData)
166+
}
146167
}
147168
}
148169

@@ -436,8 +457,8 @@ open class Model(
436457
tag: String = ModelChangeTags.NORMAL,
437458
forceChange: Boolean = false,
438459
) {
460+
val oldValue = data[name]
439461
synchronized(data) {
440-
val oldValue = data[name]
441462
if (oldValue == value && !forceChange) {
442463
return
443464
}
@@ -447,9 +468,8 @@ open class Model(
447468
} else if (data.containsKey(name)) {
448469
data.remove(name)
449470
}
450-
451-
notifyChanged(name, name, tag, oldValue, value)
452471
}
472+
notifyChanged(name, name, tag, oldValue, value)
453473
}
454474

455475
/**
@@ -672,24 +692,28 @@ open class Model(
672692
fun toJSON(): JSONObject {
673693
synchronized(initializationLock) {
674694
val jsonObject = JSONObject()
675-
for (kvp in data) {
676-
when (val value = kvp.value) {
677-
is Model -> {
678-
jsonObject.put(kvp.key, value.toJSON())
679-
}
680-
is List<*> -> {
681-
val jsonArray = JSONArray()
682-
for (arrayItem in value) {
683-
if (arrayItem is Model) {
684-
jsonArray.put(arrayItem.toJSON())
685-
} else {
686-
jsonArray.put(arrayItem)
695+
synchronized(data) {
696+
for (kvp in data) {
697+
when (val value = kvp.value) {
698+
is Model -> {
699+
jsonObject.put(kvp.key, value.toJSON())
700+
}
701+
702+
is List<*> -> {
703+
val jsonArray = JSONArray()
704+
for (arrayItem in value) {
705+
if (arrayItem is Model) {
706+
jsonArray.put(arrayItem.toJSON())
707+
} else {
708+
jsonArray.put(arrayItem)
709+
}
687710
}
711+
jsonObject.put(kvp.key, jsonArray)
712+
}
713+
714+
else -> {
715+
jsonObject.put(kvp.key, value)
688716
}
689-
jsonObject.put(kvp.key, jsonArray)
690-
}
691-
else -> {
692-
jsonObject.put(kvp.key, value)
693717
}
694718
}
695719
}

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ abstract class ModelStore<TModel>(
8484
tag: String,
8585
) {
8686
persist()
87-
8887
changeSubscription.fire { it.onModelUpdated(args, tag) }
8988
}
9089

@@ -102,10 +101,11 @@ abstract class ModelStore<TModel>(
102101

103102
override fun clear(tag: String) {
104103
val localList = models.toList()
105-
models.clear()
104+
synchronized(models) {
105+
models.clear()
106106

107107
persist()
108-
108+
}
109109
for (item in localList) {
110110
// no longer listen for changes to this model
111111
item.unsubscribe(this)

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/subscriptions/SubscriptionManagerTests.kt

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
package com.onesignal.user.internal.subscriptions
22

33
import com.onesignal.common.modeling.IModelChangedHandler
4-
import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
54
import com.onesignal.common.modeling.ModelChangeTags
65
import com.onesignal.common.modeling.ModelChangedArgs
76
import com.onesignal.core.internal.application.IApplicationService
8-
import com.onesignal.core.internal.config.ConfigModel
9-
import com.onesignal.extensions.RobolectricTest
107
import com.onesignal.mocks.MockHelper
118
import com.onesignal.session.internal.session.ISessionService
129
import com.onesignal.user.internal.subscriptions.impl.SubscriptionManager
@@ -26,7 +23,6 @@ import io.mockk.verify
2623
import junit.framework.TestCase
2724
import org.junit.runner.RunWith
2825

29-
@RobolectricTest
3026
@RunWith(KotestTestRunner::class)
3127
class SubscriptionManagerTests : FunSpec({
3228

@@ -56,9 +52,10 @@ class SubscriptionManagerTests : FunSpec({
5652

5753
t1.start()
5854
t2.start()
55+
5956
// Give some time for the thread to complete the task
6057
Thread.sleep(1000)
61-
58+
// verify if the thread has been successfully terminated
6259
TestCase.assertEquals(Thread.State.TERMINATED, t1.state)
6360
}
6461

0 commit comments

Comments
 (0)