make fixes to the push subscription model

* Push observer will receive the push subscription object, so it has `optedIn`, not `enabled`.
* Update the logic in firePushSubscriptionChanged as a result of the above.
* Keep the `enabled` property for internal use to send to the server
* Cache the notificationTypes property
* Fix a bug in hydrating `enabled`. Add TODO for how to hydrate `enabled`, since it's not clear.
* For now, don't hydrate token, because it can lead to wonky states where it can overwrite the updated correct token and fire the push observer. Tokens should not be changed from the server anyway.

Author: nan-li

iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSSubscriptionModel.swift

@@ -38,15 +38,14 @@ import OneSignalNotifications
 
 @objc
 public class OSPushSubscriptionState: NSObject {
-    // TODO: Decide if optedIn will be observed
     @objc public let id: String?
     @objc public let token: String?
-    @objc public let enabled: Bool
+    @objc public let optedIn: Bool
 
-    init(id: String?, token: String?, enabled: Bool) {
+    init(id: String?, token: String?, optedIn: Bool) {
         self.id = id
         self.token = token
-        self.enabled = enabled
+        self.optedIn = optedIn
     }
 
     func toDictionary() -> NSDictionary {
@@ -55,7 +54,7 @@ public class OSPushSubscriptionState: NSObject {
         return [
             "id": id,
             "token": token,
-            "enabled": enabled
+            "optedIn": optedIn
         ]
     }
 }
@@ -125,21 +124,22 @@ class OSSubscriptionModel: OSModel {
         }
     }
 
-    var enabled: Bool { // Note: This behaves like the current `isSubscribed`
+    // Internal property to send to server, not meant for outside access
+    var enabled: Bool { // Does not consider subscription_id in the calculation
         get {
-            return calculateIsSubscribed(subscriptionId: subscriptionId, address: address, accepted: _accepted, isDisabled: _isDisabled)
+            return calculateIsEnabled(address: address, accepted: _accepted, isDisabled: _isDisabled)
         }
     }
 
     var optedIn: Bool {
         // optedIn = permission + userPreference
         get {
-            return _accepted && !_isDisabled
+            return calculateIsOptedIn(accepted: _accepted, isDisabled: _isDisabled)
         }
     }
 
     // Push Subscription Only
-    // Initialize to be -1, so not to deal with unwrapping every time
+    // Initialize to be -1, so not to deal with unwrapping every time, and simplifies caching
     var notificationTypes = -1 {
         didSet {
             guard self.type == .push && notificationTypes != oldValue else {
@@ -200,6 +200,7 @@ class OSSubscriptionModel: OSModel {
         coder.encode(subscriptionId, forKey: "subscriptionId")
         coder.encode(_accepted, forKey: "_accepted")
         coder.encode(_isDisabled, forKey: "_isDisabled")
+        coder.encode(notificationTypes, forKey: "notificationTypes")
     }
 
     required init?(coder: NSCoder) {
@@ -215,6 +216,7 @@ class OSSubscriptionModel: OSModel {
         self.subscriptionId = coder.decodeObject(forKey: "subscriptionId") as? String
         self._accepted = coder.decodeBool(forKey: "_accepted")
         self._isDisabled = coder.decodeBool(forKey: "_isDisabled")
+        self.notificationTypes = coder.decodeInteger(forKey: "notificationTypes")
         super.init(coder: coder)
     }
 
@@ -228,12 +230,13 @@ class OSSubscriptionModel: OSModel {
                 if let type = OSSubscriptionType(rawValue: property.value as? String ?? "") {
                     self.type = type
                 }
-            case "token":
-                self.address = property.value as? String
+            // case "token":
+                // TODO: For now, don't hydrate token
+                // self.address = property.value as? String
             case "enabled":
                 if let enabled = property.value as? Bool {
-                    if self.enabled != enabled {
-                        _isDisabled = enabled
+                    if self.enabled != enabled { // TODO: Is this right?
+                        _isDisabled = !enabled
                     }
                 }
             case "notification_types":
@@ -253,13 +256,14 @@ extension OSSubscriptionModel {
     var currentPushSubscriptionState: OSPushSubscriptionState {
         return OSPushSubscriptionState(id: self.subscriptionId,
                                        token: self.address,
-                                       enabled: self.enabled
+                                       optedIn: self.optedIn
         )
     }
-
-    // Calculates if the device is currently able to receive a push notification.
-    func calculateIsSubscribed(subscriptionId: String?, address: String?, accepted: Bool, isDisabled: Bool) -> Bool {
-        return (self.subscriptionId != nil) && (self.address != nil) && _accepted && !_isDisabled
+    
+    // Calculates if the device is opted in to push notification.
+    // Must have permission and not be opted out.
+    func calculateIsOptedIn(accepted: Bool, isDisabled: Bool) -> Bool {
+        return _accepted && !_isDisabled
     }
 
     // Calculates if push notifications are enabled on the device.
@@ -280,46 +284,47 @@ extension OSSubscriptionModel {
     }
 
     func firePushSubscriptionChanged(_ changedProperty: OSPushPropertyChanged) {
-        var prevIsSubscribed = true
+        var prevIsOptedIn = true
         var prevIsEnabled = true
-        var prevSubscriptionState = OSPushSubscriptionState(id: "", token: "", enabled: true)
+        var prevSubscriptionState = OSPushSubscriptionState(id: "", token: "", optedIn: true)
 
         switch changedProperty {
         case .subscriptionId(let oldValue):
             prevIsEnabled = calculateIsEnabled(address: address, accepted: _accepted, isDisabled: _isDisabled)
-            prevIsSubscribed = calculateIsSubscribed(subscriptionId: oldValue, address: address, accepted: _accepted, isDisabled: _isDisabled)
-            prevSubscriptionState = OSPushSubscriptionState(id: oldValue, token: address, enabled: prevIsSubscribed)
+            prevIsOptedIn = calculateIsOptedIn(accepted: _accepted, isDisabled: _isDisabled)
+            prevSubscriptionState = OSPushSubscriptionState(id: oldValue, token: address, optedIn: prevIsOptedIn)
 
         case .accepted(let oldValue):
             prevIsEnabled = calculateIsEnabled(address: address, accepted: oldValue, isDisabled: _isDisabled)
-            prevIsSubscribed = calculateIsSubscribed(subscriptionId: subscriptionId, address: address, accepted: oldValue, isDisabled: _isDisabled)
-            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, enabled: prevIsSubscribed)
+            prevIsOptedIn = calculateIsOptedIn(accepted: oldValue, isDisabled: _isDisabled)
+            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, optedIn: prevIsOptedIn)
 
         case .isDisabled(let oldValue):
             prevIsEnabled = calculateIsEnabled(address: address, accepted: _accepted, isDisabled: oldValue)
-            prevIsSubscribed = calculateIsSubscribed(subscriptionId: subscriptionId, address: address, accepted: _accepted, isDisabled: oldValue)
-            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, enabled: prevIsSubscribed)
+            prevIsOptedIn = calculateIsOptedIn(accepted: _accepted, isDisabled: oldValue)
+            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, optedIn: prevIsOptedIn)
 
         case .address(let oldValue):
             prevIsEnabled = calculateIsEnabled(address: oldValue, accepted: _accepted, isDisabled: _isDisabled)
-            prevIsSubscribed = calculateIsSubscribed(subscriptionId: subscriptionId, address: oldValue, accepted: _accepted, isDisabled: _isDisabled)
-            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: oldValue, enabled: prevIsSubscribed)
+            prevIsOptedIn = calculateIsOptedIn(accepted: _accepted, isDisabled: _isDisabled)
+            prevSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: oldValue, optedIn: prevIsOptedIn)
         }
 
-        let newIsSubscribed = calculateIsSubscribed(subscriptionId: subscriptionId, address: address, accepted: _accepted, isDisabled: _isDisabled)
+        let newIsOptedIn = calculateIsOptedIn(accepted: _accepted, isDisabled: _isDisabled)
 
         let newIsEnabled = calculateIsEnabled(address: address, accepted: _accepted, isDisabled: _isDisabled)
 
         if prevIsEnabled != newIsEnabled {
             self.set(property: "enabled", newValue: newIsEnabled)
         }
 
-        let newSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, enabled: newIsSubscribed)
+        let newSubscriptionState = OSPushSubscriptionState(id: subscriptionId, token: address, optedIn: newIsOptedIn)
 
         let stateChanges = OSPushSubscriptionStateChanges(to: newSubscriptionState, from: prevSubscriptionState)
 
         // TODO: Don't fire observer until server is udated
-        print("🔥 firePushSubscriptionChanged from \(prevSubscriptionState) to \(newSubscriptionState)")
+        print("🔥 firePushSubscriptionChanged from \(prevSubscriptionState.toDictionary()) to \(newSubscriptionState.toDictionary())")
+        
         OneSignalUserManagerImpl.sharedInstance.pushSubscriptionStateChangesObserver.notifyChange(stateChanges)
     }
 }

iOS_SDK/OneSignalSDK/OneSignalUser/Source/OSUserRequests.swift

@@ -854,6 +854,7 @@ class OSRequestUpdateSubscription: OneSignalRequest, OSUserRequest {
     }
 
     // TODO: just need the sub model and send it
+    // But the model may be outdated or not sync with the subscriptionObject
     init(subscriptionObject: [String: Any], subscriptionModel: OSSubscriptionModel) {
         self.subscriptionModel = subscriptionModel
         self.stringDescription = "OSRequestUpdateSubscription with subscriptionObject: \(subscriptionObject)"
@@ -866,6 +867,10 @@ class OSRequestUpdateSubscription: OneSignalRequest, OSUserRequest {
         subscriptionParams["token"] = subscriptionModel.address ?? ""
         subscriptionParams["notification_types"] = subscriptionModel.notificationTypes
         subscriptionParams["enabled"] = subscriptionModel.enabled
+        // TODO: The above is not quite right. If we hydrate, we will over-write any pending updates
+//        subscriptionParams["token"] = subscriptionObject["address"]
+//        subscriptionParams["notification_types"] = subscriptionObject["notificationTypes"]
+//        subscriptionParams["enabled"] = subscriptionObject["enabled"]
         self.parameters = ["subscription": subscriptionParams]
         self.method = PATCH
         _ = prepareForExecution() // sets the path property