feat: alter `verify_connection` to just return boolean (#1115)

* feat: alter `verify_connection` to just return boolean

Closes #1114

CHANGELOG.md

@@ -22,6 +22,7 @@
 - iOS: Sync metadata can be reset using the `resetBookmarksMetadata` method
   ([#1092](https://github.com/mozilla/application-services/pull/1092))
 
+
 # v0.27.1 (_2019-04-26_)
 
 [Full Changelog](https://github.com/mozilla/application-services/compare/v0.27.0...v0.27.1)

CHANGES_UNRELEASED.md

@@ -4,3 +4,14 @@
 
 [Full Changelog](https://github.com/mozilla/application-services/compare/v0.27.2...master)
 
+## Push
+
+### Breaking Changes
+
+- `PushManager.verifyConnection()` now returns a boolean. `true`
+  indicates the connection is valid and no action required, `false`
+indicates that the connection is invalid. All existing subscriptions
+have been dropped. The caller should send a `pushsubscriptionchange`
+to all known apps. (This is due to the fact that the Push API does
+not have a way to send just the new endpoint to the client PWA.) 
+[#1114](https://github.com/mozilla/application-services/issues/1114) 

components/push/android/src/main/java/mozilla/appservices/push/LibPushFFI.kt

@@ -75,7 +75,7 @@ internal interface LibPushFFI : Library {
     fun push_verify_connection(
         mgr: PushManagerHandle,
         out_err: RustError.ByReference
-    ): Pointer?
+    ): Byte
 
     fun push_decrypt(
         mgr: PushManagerHandle,

components/push/android/src/main/java/mozilla/appservices/push/PushManager.kt

@@ -84,19 +84,11 @@ class PushManager(
         }.toInt() == 1
     }
 
-    override fun verifyConnection(): Map<String, String> {
-        val newEndpoints: MutableMap<String, String> = linkedMapOf()
-        val response = rustCallForString { error ->
+    override fun verifyConnection(): Boolean {
+        return rustCall { error ->
             LibPushFFI.INSTANCE.push_verify_connection(
                 this.handle.get(), error)
-        }
-        if (response.isNotEmpty()) {
-            val visited = JSONObject(response)
-            for (key in visited.keys()) {
-                newEndpoints[key] = visited[key] as String
-            }
-        }
-        return newEndpoints
+        }.toInt() == 1
     }
 
     override fun decrypt(
@@ -330,13 +322,12 @@ interface PushAPI : java.lang.AutoCloseable {
     fun update(registrationToken: String): Boolean
 
     /**
-     * Verifies the connection state. NOTE: If the internal check fails,
-     * endpoints will be re-registered and new endpoints will be returned for
-     * known ChannelIDs
+     * Verifies the connection state.
      *
-     * @return Map of ChannelID: Endpoint, be sure to notify apps registered to given channelIDs of the new Endpoint.
+     * @return bool indicating if connection state is valid (true) or if channels should get a
+     * `pushsubscriptionchange` event (false).
      */
-    fun verifyConnection(): Map<String, String>
+    fun verifyConnection(): Boolean
 
     /**
      * Decrypts a raw push message.

components/push/android/src/test/java/mozilla/appservices/push/PushTest.kt

@@ -218,8 +218,7 @@ class PushTest {
         manager.subscribe(testChannelid, "foo")
         // and call verifyConnection again to emulate a set value.
         val result = manager.verifyConnection()
-        val vv = result[testChannelid].toString()
-        assertEquals("Check changed endpoint", "http://push.example.com/test/obscure", vv)
+        assertEquals("Check changed endpoint", false, result)
     }
 
     @Test

components/push/ffi/src/lib.rs

@@ -136,21 +136,12 @@ pub extern "C" fn push_update(handle: u64, new_token: FfiStr<'_>, error: &mut Ex
 }
 
 // verify connection using channel list
-// Returns a JSON containing the new channel_ids => endpoints
-// NOTE: AC should notify processes associated with channel_ids of new endpoint
+// Returns a bool indicating if channel_ids should resubscribe.
 #[no_mangle]
-pub extern "C" fn push_verify_connection(handle: u64, error: &mut ExternError) -> *mut c_char {
+pub extern "C" fn push_verify_connection(handle: u64, error: &mut ExternError) -> u8 {
     log::debug!("push_verify");
     MANAGER.call_with_result_mut(error, handle, |mgr| -> Result<_> {
-        if !mgr.verify_connection()? {
-            let new_endpoints = mgr.regenerate_endpoints()?;
-            if !new_endpoints.is_empty() {
-                return serde_json::to_string(&new_endpoints).map_err(|e| {
-                    push::error::ErrorKind::TranscodingError(format!("{:?}", e)).into()
-                });
-            }
-        }
-        Ok(String::from(""))
+        mgr.verify_connection()
     })
 }
 
@@ -172,7 +163,7 @@ pub extern "C" fn push_decrypt(
         let r_salt: Option<&str> = salt.as_opt_str();
         let r_dh: Option<&str> = dh.as_opt_str();
         let uaid = mgr.conn.uaid.clone().unwrap();
-        mgr.decrypt(&uaid, r_chid, r_body, r_encoding, r_dh, r_salt)
+        mgr.decrypt(&uaid, r_chid, r_body, r_encoding, r_salt, r_dh)
     })
 }
 // TODO: modify these to be relevant.

components/push/src/communications.rs

@@ -234,7 +234,7 @@ impl Connection for ConnectHttp {
         })
     }
 
-    /// Drop a channel and stop recieving updates.
+    /// Drop a channel and stop receiving updates.
     fn unsubscribe(&self, channel_id: Option<&str>) -> error::Result<bool> {
         if self.auth.is_none() {
             return Err(CommunicationError("Connection is unauthorized".into()).into());
@@ -384,8 +384,7 @@ impl Connection for ConnectHttp {
 
     /// Verify that the server and client both have matching channel information. A "false"
     /// should force the client to drop the old UAID, request a new UAID from the server, and
-    /// resubscribe all channels, resulting in new endpoints. This will require sending the
-    /// new endpoints to the channel recipient functions.
+    /// resubscribe all channels, resulting in new endpoints.
     fn verify_connection(&self, channels: &[String]) -> error::Result<bool> {
         if self.auth.is_none() {
             return Err(CommunicationError("Connection uninitiated".to_owned()).into());
@@ -396,7 +395,12 @@ impl Connection for ConnectHttp {
         log::debug!("Getting Channel List");
         let remote = self.channel_list()?;
         // verify both lists match. Either side could have lost it's mind.
-        Ok(remote == channels.to_vec())
+        if remote != channels {
+            // Unsubscribe all the channels (just to be sure and avoid a loop)
+            self.unsubscribe(None)?;
+            return Ok(false);
+        }
+        Ok(true)
     }
 
     //impl TODO: Handle a Ping response with updated Broadcasts.

components/push/src/subscriber.rs

@@ -94,10 +94,13 @@ impl PushManager {
         if self.conn.uaid.is_none() {
             return Err(ErrorKind::GeneralError("No subscriptions created yet.".into()).into());
         }
-        let result = self.conn.unsubscribe(channel_id)?;
-        self.store
-            .delete_record(self.conn.uaid.as_ref().unwrap(), channel_id.unwrap())?;
-        Ok(result)
+        let uaid = self.conn.uaid.as_ref().unwrap();
+        Ok(if let Some(chid) = channel_id {
+            self.conn.unsubscribe(channel_id)? && self.store.delete_record(uaid, chid)?
+        } else {
+            self.store.delete_all_records(uaid)?;
+            self.conn.unsubscribe(None)?
+        })
     }
 
     pub fn update(&mut self, new_token: &str) -> error::Result<bool> {
@@ -119,7 +122,11 @@ impl PushManager {
         let channels = self
             .store
             .get_channel_list(self.conn.uaid.as_ref().unwrap())?;
-        self.conn.verify_connection(&channels)
+        let result = self.conn.verify_connection(&channels)?;
+        if !result {
+            self.unsubscribe(None)?;
+        }
+        Ok(result)
     }
 
     pub fn decrypt(
@@ -128,8 +135,8 @@ impl PushManager {
         chid: &str,
         body: &str,
         encoding: &str,
-        dh: Option<&str>,
         salt: Option<&str>,
+        dh: Option<&str>,
     ) -> Result<String> {
         match self.store.get_record(&uaid, chid) {
             Err(e) => Err(ErrorKind::StorageError(format!("{:?}", e)).into()),