Skip to content

Commit eaf4855

Browse files
nan-linan-li
authored
Merge pull request #1470 from OneSignal/improve/delay_updates_after_creates
[Fix] Handle incorrect `404`, delay making requests to new IDs
2 parents caee127 + 529a148 commit eaf4855

27 files changed

+561
-139
lines changed

‎.swiftlint.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
1-
line_length: 150
1+
line_length: 150
2+
disabled_rules:
3+
# Track https://github.com/realm/SwiftLint/pull/5521
4+
- opening_brace

‎iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,9 @@
177177
3CEE93542B7C78EC008440BD /* OneSignalUser.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DE69E19B282ED8060090BB3D /* OneSignalUser.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
178178
3CEE93572B7C78FD008440BD /* OneSignalCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; };
179179
3CEE93582B7C78FE008440BD /* OneSignalCore.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = DE7D17E627026B95002D3A5D /* OneSignalCore.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
180+
3CF11E3D2C6D6155002856F5 /* UserExecutorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */; };
181+
3CF11E402C6E6DE2002856F5 /* MockNewRecordsState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF11E3F2C6E6DE2002856F5 /* MockNewRecordsState.swift */; };
182+
3CF1A5632C669EA40056B3AA /* OSNewRecordsState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF1A5622C669EA40056B3AA /* OSNewRecordsState.swift */; };
180183
3CF8629E28A183F900776CA4 /* OSIdentityModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF8629D28A183F900776CA4 /* OSIdentityModel.swift */; };
181184
3CF862A028A1964F00776CA4 /* OSPropertiesModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF8629F28A1964F00776CA4 /* OSPropertiesModel.swift */; };
182185
3CF862A228A197D200776CA4 /* OSPropertiesModelStoreListener.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3CF862A128A197D200776CA4 /* OSPropertiesModelStoreListener.swift */; };
@@ -1294,6 +1297,9 @@
12941297
3CE92279289FA88B001B1062 /* OSIdentityModelStoreListener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModelStoreListener.swift; sourceTree = "<group>"; };
12951298
3CEE90A62BFE6ABD00B0FB5B /* OSPropertiesSupportedProperty.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesSupportedProperty.swift; sourceTree = "<group>"; };
12961299
3CEE90A82C000BD500B0FB5B /* OneSignalRequest+UnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OneSignalRequest+UnitTests.swift"; sourceTree = "<group>"; };
1300+
3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserExecutorTests.swift; sourceTree = "<group>"; };
1301+
3CF11E3F2C6E6DE2002856F5 /* MockNewRecordsState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockNewRecordsState.swift; sourceTree = "<group>"; };
1302+
3CF1A5622C669EA40056B3AA /* OSNewRecordsState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSNewRecordsState.swift; sourceTree = "<group>"; };
12971303
3CF8629D28A183F900776CA4 /* OSIdentityModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSIdentityModel.swift; sourceTree = "<group>"; };
12981304
3CF8629F28A1964F00776CA4 /* OSPropertiesModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesModel.swift; sourceTree = "<group>"; };
12991305
3CF862A128A197D200776CA4 /* OSPropertiesModelStoreListener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSPropertiesModelStoreListener.swift; sourceTree = "<group>"; };
@@ -2051,6 +2057,7 @@
20512057
3C115188289ADEA300565C41 /* OSModelStore.swift */,
20522058
3C115186289ADE7700565C41 /* OSModelStoreListener.swift */,
20532059
3C115184289ADE4F00565C41 /* OSModel.swift */,
2060+
3CF1A5622C669EA40056B3AA /* OSNewRecordsState.swift */,
20542061
3C11518A289ADEEB00565C41 /* OSEventProducer.swift */,
20552062
3C11518C289AF5E800565C41 /* OSModelChangedHandler.swift */,
20562063
3C4F9E4328A4466C009F453A /* OSOperationRepo.swift */,
@@ -2070,6 +2077,7 @@
20702077
children = (
20712078
3C8544B82C5AEFF700F542A9 /* OneSignalOSCoreMocks.h */,
20722079
3C8544C22C5AF18B00F542A9 /* OSCoreMocks.swift */,
2080+
3CF11E3F2C6E6DE2002856F5 /* MockNewRecordsState.swift */,
20732081
);
20742082
path = OneSignalOSCoreMocks;
20752083
sourceTree = "<group>";
@@ -2150,6 +2158,7 @@
21502158
isa = PBXGroup;
21512159
children = (
21522160
3CDE664A2BFC2A55006DA114 /* OneSignalUserTests-Bridging-Header.h */,
2161+
3CF11E3E2C6D61AC002856F5 /* Executors */,
21532162
3CC063ED2B6D7FE8002BB07F /* OneSignalUserTests.swift */,
21542163
3CC890342C5BF9A7002CB4CC /* UserConcurrencyTests.swift */,
21552164
3C67F7792BEB2B710085A0F0 /* SwitchUserIntegrationTests.swift */,
@@ -2166,6 +2175,14 @@
21662175
path = Support;
21672176
sourceTree = "<group>";
21682177
};
2178+
3CF11E3E2C6D61AC002856F5 /* Executors */ = {
2179+
isa = PBXGroup;
2180+
children = (
2181+
3CF11E3C2C6D6155002856F5 /* UserExecutorTests.swift */,
2182+
);
2183+
path = Executors;
2184+
sourceTree = "<group>";
2185+
};
21692186
3E2400391D4FFC31008BDE70 /* OneSignalFramework */ = {
21702187
isa = PBXGroup;
21712188
children = (
@@ -4038,6 +4055,7 @@
40384055
3C115165289A259500565C41 /* OneSignalOSCore.docc in Sources */,
40394056
3C115189289ADEA300565C41 /* OSModelStore.swift in Sources */,
40404057
3C115185289ADE4F00565C41 /* OSModel.swift in Sources */,
4058+
3CF1A5632C669EA40056B3AA /* OSNewRecordsState.swift in Sources */,
40414059
3C448BA22936B474002F96BC /* OSBackgroundTaskManager.swift in Sources */,
40424060
3C115187289ADE7700565C41 /* OSModelStoreListener.swift in Sources */,
40434061
3CE5F9E3289D88DC004A156E /* OSModelStoreChangedHandler.swift in Sources */,
@@ -4054,6 +4072,7 @@
40544072
buildActionMask = 2147483647;
40554073
files = (
40564074
3C8544C32C5AF18B00F542A9 /* OSCoreMocks.swift in Sources */,
4075+
3CF11E402C6E6DE2002856F5 /* MockNewRecordsState.swift in Sources */,
40574076
);
40584077
runOnlyForDeploymentPostprocessing = 0;
40594078
};
@@ -4092,6 +4111,7 @@
40924111
isa = PBXSourcesBuildPhase;
40934112
buildActionMask = 2147483647;
40944113
files = (
4114+
3CF11E3D2C6D6155002856F5 /* UserExecutorTests.swift in Sources */,
40954115
3C67F77A2BEB2B710085A0F0 /* SwitchUserIntegrationTests.swift in Sources */,
40964116
3CC063EE2B6D7FE8002BB07F /* OneSignalUserTests.swift in Sources */,
40974117
3CC890352C5BF9A7002CB4CC /* UserConcurrencyTests.swift in Sources */,

‎iOS_SDK/OneSignalSDK/OneSignalCore/Source/OneSignalCommonDefines.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,13 @@ typedef enum {GET, POST, HEAD, PUT, DELETE, OPTIONS, CONNECT, TRACE, PATCH} HTTP
259259

260260
// Flush interval for operation repo in milliseconds
261261
#define POLL_INTERVAL_MS 5000
262+
263+
/**
264+
The number of seconds to delay after an operation completes that creates or changes IDs.
265+
This is a "cold down" period to avoid a caveat with OneSignal's backend replication, where you may
266+
incorrectlyget a 404 when attempting a GET or PATCH REST API call on something just after it is created.
267+
*/
268+
#define OP_REPO_POST_CREATE_DELAY_SECONDS 3
262269
#else
263270
// Test defines for API Client
264271
#define REATTEMPT_DELAY 0.004
@@ -279,6 +286,8 @@ typedef enum {GET, POST, HEAD, PUT, DELETE, OPTIONS, CONNECT, TRACE, PATCH} HTTP
279286
// Reduce flush interval for operation repo in tests
280287
#define POLL_INTERVAL_MS 100
281288

289+
// Reduce delay in tests
290+
#define OP_REPO_POST_CREATE_DELAY_SECONDS 0
282291
#endif
283292

284293
// A max timeout for a request, which might include multiple reattempts

‎iOS_SDK/OneSignalSDK/OneSignalCoreMocks/MockOneSignalClient.swift‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,6 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
9393
public func execute(_ request: OneSignalRequest, onSuccess successBlock: @escaping OSResultSuccessBlock, onFailure failureBlock: @escaping OSFailureBlock) {
9494
print("🧪 MockOneSignalClient execute called")
9595

96-
lock.withLock {
97-
executedRequests.append(request)
98-
}
99-
10096
if executeInstantaneously {
10197
finishExecutingRequest(request, onSuccess: successBlock, onFailure: failureBlock)
10298
} else {
@@ -127,6 +123,9 @@ public class MockOneSignalClient: NSObject, IOneSignalClient {
127123
print("🧪 completing HTTP request: \(request)")
128124

129125
// TODO: Check for existence of app_id in the request and fail if not.
126+
lock.withLock {
127+
executedRequests.append(request)
128+
}
130129

131130
self.didCompleteRequest(request)
132131

‎iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSNewRecordsState.swift‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
Modified MIT License
3+
4+
Copyright 2024 OneSignal
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
1. The above copyright notice and this permission notice shall be included in
14+
all copies or substantial portions of the Software.
15+
16+
2. All copies of substantial portions of the Software may only be used in connection
17+
with services provided by OneSignal.
18+
19+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
20+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
21+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
22+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
23+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
24+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
25+
THE SOFTWARE.
26+
*/
27+
28+
import OneSignalCore
29+
30+
/**
31+
* Purpose: Keeps track of IDs that were just created on the backend.
32+
* This list gets used to delay network calls to ensure upcoming
33+
* requests are ready to be accepted by the backend.
34+
*/
35+
public class OSNewRecordsState {
36+
/**
37+
Params:
38+
- Key - a string ID such as onesignal ID or subscription ID
39+
- Value - a Date timestamp of when the ID was created
40+
*/
41+
private var records: [String: Date] = [:]
42+
private let lock = NSRecursiveLock()
43+
44+
public init() { }
45+
46+
/**
47+
Only add a new record with the current timestamp if overwriting is requested, or it is not already present
48+
*/
49+
public func add(_ key: String, _ overwrite: Bool = false) {
50+
lock.withLock {
51+
if overwrite || records[key] == nil {
52+
records[key] = Date()
53+
}
54+
}
55+
}
56+
57+
public func canAccess(_ key: String) -> Bool {
58+
lock.withLock {
59+
guard let timeLastMovedOrCreated = records[key] else {
60+
return true
61+
}
62+
63+
let minimumTime = timeLastMovedOrCreated.addingTimeInterval(TimeInterval(OP_REPO_POST_CREATE_DELAY_SECONDS))
64+
65+
return Date() >= minimumTime
66+
}
67+
}
68+
}

‎iOS_SDK/OneSignalSDK/OneSignalOSCoreMocks/MockNewRecordsState.swift‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
Modified MIT License
3+
4+
Copyright 2024 OneSignal
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
1. The above copyright notice and this permission notice shall be included in
14+
all copies or substantial portions of the Software.
15+
16+
2. All copies of substantial portions of the Software may only be used in connection
17+
with services provided by OneSignal.
18+
19+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
20+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
21+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
22+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
23+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
24+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
25+
THE SOFTWARE.
26+
*/
27+
28+
@testable import OneSignalOSCore
29+
30+
public class MockNewRecordsState: OSNewRecordsState {
31+
public struct MockNewRecord {
32+
let key: String
33+
let overwrite: Bool
34+
}
35+
36+
public var records: [MockNewRecord] = []
37+
38+
override public func add(_ key: String, _ overwrite: Bool = false) {
39+
let record = MockNewRecord(key: key, overwrite: overwrite)
40+
records.append(record)
41+
42+
super.add(key, overwrite)
43+
}
44+
45+
override public func canAccess(_ key: String) -> Bool {
46+
return super.canAccess(key)
47+
}
48+
49+
public func get(_ key: String?) -> [MockNewRecord] {
50+
return records.filter { $0.key == key }
51+
}
52+
53+
public func contains(_ key: String?) -> Bool {
54+
return get(key).count > 0
55+
}
56+
57+
public func wasOverwritten(_ key: String?) -> Bool {
58+
return records.filter { $0.key == key && $0.overwrite }.count > 0
59+
}
60+
}

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSIdentityOperationExecutor.swift‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,13 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
3434
// To simplify uncaching, we maintain separate request queues for each type
3535
var addRequestQueue: [OSRequestAddAliases] = []
3636
var removeRequestQueue: [OSRequestRemoveAlias] = []
37+
let newRecordsState: OSNewRecordsState
3738

3839
// The Identity executor dispatch queue, serial. This synchronizes access to the delta and request queues.
3940
private let dispatchQueue = DispatchQueue(label: "OneSignal.OSIdentityOperationExecutor", target: .global())
4041

41-
init() {
42+
init(newRecordsState: OSNewRecordsState) {
43+
self.newRecordsState = newRecordsState
4244
// Read unfinished deltas and requests from cache, if any...
4345
uncacheDeltas()
4446
uncacheAddAliasRequests()
@@ -72,7 +74,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
7274
if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
7375
// 1. The model exists in the repo, so set it to be the Request's models
7476
request.identityModel = identityModel
75-
} else if request.prepareForExecution() {
77+
} else if request.prepareForExecution(newRecordsState: newRecordsState) {
7678
// 2. The request can be sent, add the model to the repo
7779
OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
7880
} else {
@@ -95,7 +97,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
9597
if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
9698
// 1. The model exists in the repo, so set it to be the Request's model
9799
request.identityModel = identityModel
98-
} else if request.prepareForExecution() {
100+
} else if request.prepareForExecution(newRecordsState: newRecordsState) {
99101
// 2. The request can be sent, add the model to the repo
100102
OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
101103
} else {
@@ -191,7 +193,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
191193
guard !request.sentToClient else {
192194
return
193195
}
194-
guard request.prepareForExecution() else {
196+
guard request.prepareForExecution(newRecordsState: newRecordsState) else {
195197
return
196198
}
197199
request.sentToClient = true
@@ -250,7 +252,7 @@ class OSIdentityOperationExecutor: OSOperationExecutor {
250252
guard !request.sentToClient else {
251253
return
252254
}
253-
guard request.prepareForExecution() else {
255+
guard request.prepareForExecution(newRecordsState: newRecordsState) else {
254256
return
255257
}
256258
request.sentToClient = true

‎iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSPropertyOperationExecutor.swift‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,13 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
6464
var supportedDeltas: [String] = [OS_UPDATE_PROPERTIES_DELTA]
6565
var deltaQueue: [OSDelta] = []
6666
var updateRequestQueue: [OSRequestUpdateProperties] = []
67+
let newRecordsState: OSNewRecordsState
6768

6869
// The property executor dispatch queue, serial. This synchronizes access to `deltaQueue` and `updateRequestQueue`.
6970
private let dispatchQueue = DispatchQueue(label: "OneSignal.OSPropertyOperationExecutor", target: .global())
7071

71-
init() {
72+
init(newRecordsState: OSNewRecordsState) {
73+
self.newRecordsState = newRecordsState
7274
// Read unfinished deltas and requests from cache, if any...
7375
// Note that we should only have deltas for the current user as old ones are flushed..
7476
uncacheDeltas()
@@ -98,7 +100,7 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
98100
if let identityModel = OneSignalUserManagerImpl.sharedInstance.getIdentityModel(request.identityModel.modelId) {
99101
// 1. The identity model exist in the repo, set it to be the Request's model
100102
request.identityModel = identityModel
101-
} else if request.prepareForExecution() {
103+
} else if request.prepareForExecution(newRecordsState: newRecordsState) {
102104
// 2. The request can be sent, add the model to the repo
103105
OneSignalUserManagerImpl.sharedInstance.addIdentityModelToRepo(request.identityModel)
104106
} else {
@@ -233,7 +235,7 @@ class OSPropertyOperationExecutor: OSOperationExecutor {
233235
guard !request.sentToClient else {
234236
return
235237
}
236-
guard request.prepareForExecution() else {
238+
guard request.prepareForExecution(newRecordsState: newRecordsState) else {
237239
return
238240
}
239241
request.sentToClient = true

0 commit comments

Comments
 (0)