Skip to content

Commit 4f0d3d1

Browse files
jinliu9508jinliu9508
committed
Remove unused code and fix format
1 parent 87b4300 commit 4f0d3d1

File tree

8 files changed

+195
-216
lines changed

8 files changed

+195
-216
lines changed

‎Examples/OneSignalDemo/app/src/main/java/com/onesignal/sdktest/model/MainActivityViewModel.java‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import android.widget.TextView;
2525
import com.onesignal.Continue;
2626
import com.onesignal.OneSignal;
27-
import com.onesignal.core.internal.config.ConfigModelStore;
2827
import com.onesignal.sdktest.adapter.SubscriptionRecyclerViewAdapter;
2928
import com.onesignal.user.subscriptions.IPushSubscription;
3029
import com.onesignal.sdktest.R;
@@ -274,11 +273,6 @@ public ActivityViewModel onActivityCreated(Context context) {
274273
getActivity().startActivity(new Intent(getActivity(), SecondaryActivity.class));
275274
});
276275

277-
Button createDeadlock = getActivity().findViewById(R.id.main_activity_deadlock_button);
278-
createDeadlock.setOnClickListener(v -> {
279-
createDeadlock();
280-
});
281-
282276
aliasSet = new HashMap<>();
283277
aliasArrayList = new ArrayList<>();
284278

@@ -937,44 +931,4 @@ private void refreshState() {
937931
// triggers are not persisted, they are always "starting from scratch"
938932
refreshTriggerRecyclerView();
939933
}
940-
941-
private void createDeadlock() {
942-
943-
// register test observers
944-
IPushSubscriptionObserver observer = state -> {
945-
try {
946-
Thread.sleep(1000);
947-
} catch (InterruptedException e) {
948-
throw new RuntimeException(e);
949-
}
950-
OneSignal.getUser().getPushSubscription().optIn();
951-
System.out.println("JinTest optedout");
952-
};
953-
OneSignal.getUser().getPushSubscription().addObserver(observer);
954-
955-
// calling login will call setOptAnyProperty, which locks model.data
956-
// then, any observer added will be fired, which will lock subscribers
957-
Thread t1 = new Thread() {
958-
@Override
959-
public void run() {
960-
OneSignal.getUser().getPushSubscription().optOut();
961-
OneSignal.getUser().getPushSubscription().optIn();
962-
System.out.println("JinTest optedin");
963-
}
964-
};
965-
966-
Thread t2 = new Thread() {
967-
@Override
968-
public void run() {
969-
OneSignal.logout();
970-
OneSignal.login("testJin");
971-
System.out.println("JinTest login");
972-
}
973-
};
974-
975-
t1.start();
976-
t2.start();
977-
978-
979-
}
980934
}

‎Examples/OneSignalDemo/app/src/main/res/layout/main_activity_layout.xml‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@
139139
android:orientation="vertical">
140140

141141
<!-- Application -->
142-
143142
<LinearLayout
144143
android:id="@+id/main_activity_account_linear_layout"
145144
android:layout_width="match_parent"
@@ -1269,30 +1268,6 @@
12691268
android:visibility="visible"/>
12701269

12711270
</LinearLayout>
1272-
1273-
<LinearLayout
1274-
android:layout_width="match_parent"
1275-
android:layout_height="match_parent"
1276-
android:layout_gravity="center"
1277-
android:layout_marginStart="12dp"
1278-
android:layout_marginTop="4dp"
1279-
android:layout_marginEnd="12dp"
1280-
android:layout_marginBottom="12dp"
1281-
android:background="@color/colorPrimary"
1282-
android:gravity="center"
1283-
android:orientation="vertical">
1284-
1285-
<Button
1286-
android:id="@+id/main_activity_deadlock_button"
1287-
android:layout_width="match_parent"
1288-
android:layout_height="match_parent"
1289-
android:background="@drawable/ripple_selector_white_red"
1290-
android:text="@string/create_deadlock"
1291-
android:textColor="@android:color/white"
1292-
android:textSize="19sp"
1293-
android:visibility="visible" />
1294-
1295-
</LinearLayout>
12961271
</LinearLayout>
12971272

12981273
</androidx.core.widget.NestedScrollView>

‎Examples/OneSignalDemo/app/src/main/res/values/strings.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
<string name="revoke_consent">Revoke Consent</string>
6464

6565
<string name="navigate_next_activity">Next activity</string>
66-
<string name="create_deadlock">Create deadlock</string>
6766

6867
<string name="onesignal_app_id">0ba9731b-33bd-43f4-8b59-61172e27447d</string>
6968

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

Lines changed: 38 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ open class Model(
5151
* specified, must also specify [_parentModel]
5252
*/
5353
private val _parentProperty: String? = null,
54-
private val initializationLock: Any = Any(),
5554
) : IEventNotifier<IModelChangedHandler> {
5655
/**
5756
* A unique identifier for this model.
@@ -95,35 +94,26 @@ open class Model(
9594
data[property] = listOfItems
9695
}
9796
} else {
98-
val method = this.javaClass.methods.firstOrNull {
99-
it.returnType != Void::class.java && it.name.contains(
100-
property,
101-
true
102-
)
103-
}
97+
val method =
98+
this.javaClass.methods.firstOrNull {
99+
it.returnType !=
100+
Void::class.java &&
101+
it.name.contains(
102+
property,
103+
true,
104+
)
105+
}
104106

105107
if (method == null) {
106108
data[property] = jsonObject.get(property)
107109
} else {
108110
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-
111+
Double::class.java, java.lang.Double::class.java -> data[property] = jsonObject.getDouble(property)
112+
Long::class.java, java.lang.Long::class.java -> data[property] = jsonObject.getLong(property)
113+
Float::class.java, java.lang.Float::class.java -> data[property] = jsonObject.getDouble(property).toFloat()
114+
Int::class.java, java.lang.Integer::class.java -> data[property] = jsonObject.getInt(property)
115+
Boolean::class.java, java.lang.Boolean::class.java -> data[property] = jsonObject.getBoolean(property)
116+
String::class.java, java.lang.String::class.java -> data[property] = jsonObject.getString(property)
127117
else -> data[property] = jsonObject.get(property)
128118
}
129119
}
@@ -159,11 +149,9 @@ open class Model(
159149
newData[::id.name] = id
160150
}
161151

162-
synchronized(initializationLock) {
163-
synchronized(data) {
164-
data.clear()
165-
data.putAll(newData)
166-
}
152+
synchronized(data) {
153+
data.clear()
154+
data.putAll(newData)
167155
}
168156
}
169157

@@ -690,42 +678,38 @@ open class Model(
690678
* @return The resulting [JSONObject].
691679
*/
692680
fun toJSON(): JSONObject {
693-
synchronized(initializationLock) {
694-
val jsonObject = JSONObject()
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-
}
681+
val jsonObject = JSONObject()
682+
synchronized(data) {
683+
for (kvp in data) {
684+
when (val value = kvp.value) {
685+
is Model -> {
686+
jsonObject.put(kvp.key, value.toJSON())
687+
}
701688

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-
}
689+
is List<*> -> {
690+
val jsonArray = JSONArray()
691+
for (arrayItem in value) {
692+
if (arrayItem is Model) {
693+
jsonArray.put(arrayItem.toJSON())
694+
} else {
695+
jsonArray.put(arrayItem)
710696
}
711-
jsonObject.put(kvp.key, jsonArray)
712697
}
698+
jsonObject.put(kvp.key, jsonArray)
699+
}
713700

714-
else -> {
715-
jsonObject.put(kvp.key, value)
716-
}
701+
else -> {
702+
jsonObject.put(kvp.key, value)
717703
}
718704
}
719705
}
720-
return jsonObject
721706
}
707+
return jsonObject
722708
}
723709

724710
override fun subscribe(handler: IModelChangedHandler) = changeNotifier.subscribe(handler)
725711

726-
override fun unsubscribe(handler: IModelChangedHandler) {
727-
changeNotifier.unsubscribe(handler)
728-
}
712+
override fun unsubscribe(handler: IModelChangedHandler) = changeNotifier.unsubscribe(handler)
729713

730714
override val hasSubscribers: Boolean
731715
get() = changeNotifier.hasSubscribers

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,8 @@ abstract class ModelStore<TModel>(
134134
model.subscribe(this)
135135

136136
persist()
137-
138-
changeSubscription.fire { it.onModelAdded(model, tag) }
139137
}
138+
changeSubscription.fire { it.onModelAdded(model, tag) }
140139
}
141140

142141
private fun removeItem(
@@ -150,9 +149,8 @@ abstract class ModelStore<TModel>(
150149
model.unsubscribe(this)
151150

152151
persist()
153-
154-
changeSubscription.fire { it.onModelRemoved(model, tag) }
155152
}
153+
changeSubscription.fire { it.onModelRemoved(model, tag) }
156154
}
157155

158156
protected fun load() {

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/common/ModelingTests.kt‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package com.onesignal.common
2+
3+
import com.onesignal.common.events.EventProducer
4+
import com.onesignal.common.modeling.IModelChangedHandler
5+
import com.onesignal.common.modeling.IModelStoreChangeHandler
6+
import com.onesignal.common.modeling.ModelChangedArgs
7+
import com.onesignal.mocks.MockHelper
8+
import com.onesignal.mocks.MockPreferencesService
9+
import com.onesignal.user.internal.subscriptions.SubscriptionModel
10+
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
11+
import io.kotest.core.spec.style.FunSpec
12+
import io.kotest.runner.junit4.KotestTestRunner
13+
import junit.framework.TestCase
14+
import org.junit.runner.RunWith
15+
16+
@RunWith(KotestTestRunner::class)
17+
class ModelingTests : FunSpec({
18+
19+
test("Deadlock related to Model.setOptAnyProperty") {
20+
// Given
21+
val modelStore = MockHelper.configModelStore()
22+
val model = modelStore.model
23+
24+
val t1 =
25+
Thread {
26+
// acquire "model.data", then trigger the onChanged event
27+
model.setOptAnyProperty("key1", "value1")
28+
}
29+
30+
val t2 =
31+
Thread {
32+
// acquire "model.initializationLock", then wait for "model.data" to be released
33+
model.initializeFromModel("", MockHelper.configModelStore().model)
34+
}
35+
36+
model.subscribe(
37+
object : IModelChangedHandler {
38+
// will be executed in t1
39+
override fun onChanged(
40+
args: ModelChangedArgs,
41+
tag: String,
42+
) {
43+
Thread.sleep(200)
44+
// waiting for "model.initializationLock"
45+
model.toJSON()
46+
}
47+
},
48+
)
49+
50+
t1.start()
51+
t2.start()
52+
53+
// Set 1s timeout for t2 to complete the task
54+
t2.join(1000)
55+
56+
// verify if the thread has been successfully terminated
57+
TestCase.assertEquals(Thread.State.TERMINATED, t2.state)
58+
}
59+
60+
test("Deadlock related to ModelSstore add() or remove()") {
61+
// Given
62+
val modelStore = SubscriptionModelStore(MockPreferencesService())
63+
val event = EventProducer<SubscriptionModel>()
64+
val oldSubscriptionModel = SubscriptionModel()
65+
val newSubscriptionModel = SubscriptionModel()
66+
oldSubscriptionModel.id = "oldModel"
67+
newSubscriptionModel.id = "newModel"
68+
modelStore.add(oldSubscriptionModel)
69+
70+
val t1 =
71+
Thread {
72+
// acquire "ModelStore.models", then trigger the onChanged event
73+
System.out.println("1")
74+
modelStore.add(newSubscriptionModel)
75+
}
76+
77+
val t2 =
78+
Thread {
79+
System.out.println("2")
80+
// acquire "model.data", then wait for "ModelStore.models"
81+
newSubscriptionModel.toJSON()
82+
}
83+
84+
modelStore.subscribe(
85+
object : IModelStoreChangeHandler<SubscriptionModel> {
86+
override fun onModelAdded(
87+
model: SubscriptionModel,
88+
tag: String,
89+
) {
90+
// waiting for "model.data"
91+
model.initializeFromModel("", MockHelper.configModelStore().model)
92+
}
93+
94+
override fun onModelUpdated(
95+
args: ModelChangedArgs,
96+
tag: String,
97+
) {
98+
// left empty in purpose
99+
}
100+
101+
override fun onModelRemoved(
102+
model: SubscriptionModel,
103+
tag: String,
104+
) {
105+
// left empty in purpose
106+
}
107+
},
108+
)
109+
110+
t1.start()
111+
t2.start()
112+
113+
// Set 1s timeout for t2 to complete the task
114+
t2.join(1000)
115+
116+
// verify if the thread has been successfully terminated
117+
TestCase.assertEquals(Thread.State.TERMINATED, t2.state)
118+
}
119+
})

0 commit comments

Comments
 (0)