Reworked the device API

- Defined the `LocalDevice` struct, which contains the info about the
  client's device.
- Many device methods now return a LocalDevice value constructed from
  the server response.
- Use the last `LocalDevice` returned from the server to cache the
  capabilities.  This improves on the previous system, since it
  differentiates between no known info and the empty list.
- One drawback is that this will always be unset when users migrate, so
  they may hit the server an extra time when `ensure_capabilities` is
  first called.

components/fxa-client/src/device.rs

@@ -51,7 +51,7 @@ impl FirefoxAccount {
         name: &str,
         device_type: DeviceType,
         supported_capabilities: Vec<DeviceCapability>,
-    ) -> ApiResult<()> {
+    ) -> ApiResult<LocalDevice> {
         // UniFFI doesn't have good handling of lists of references, work around it.
         let supported_capabilities: Vec<_> =
             supported_capabilities.into_iter().map(Into::into).collect();
@@ -141,7 +141,7 @@ impl FirefoxAccount {
     ///    - Device registration is only available to applications that have been
     ///      granted the `https://identity.mozilla.com/apps/oldsync` scope.
     #[handle_error(Error)]
-    pub fn set_device_name(&self, display_name: &str) -> ApiResult<()> {
+    pub fn set_device_name(&self, display_name: &str) -> ApiResult<LocalDevice> {
         self.internal.lock().set_device_name(display_name)
     }
 
@@ -187,7 +187,7 @@ impl FirefoxAccount {
     pub fn ensure_capabilities(
         &self,
         supported_capabilities: Vec<DeviceCapability>,
-    ) -> ApiResult<()> {
+    ) -> ApiResult<LocalDevice> {
         let supported_capabilities: Vec<_> =
             supported_capabilities.into_iter().map(Into::into).collect();
         self.internal
@@ -196,6 +196,17 @@ impl FirefoxAccount {
     }
 }
 
+/// Local device that's connecting to FxA
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct LocalDevice {
+    pub id: String,
+    pub display_name: String,
+    pub device_type: sync15::DeviceType,
+    pub capabilities: Vec<DeviceCapability>,
+    pub push_subscription: Option<DevicePushSubscription>,
+    pub push_endpoint_expired: bool,
+}
+
 /// A device connected to the user's account.
 ///
 /// This struct provides metadata about a device connected to the user's account.

components/fxa-client/src/fxa_client.udl

@@ -308,7 +308,7 @@ interface FirefoxAccount {
   //      granted the `https://identity.mozilla.com/apps/oldsync` scope.
   //
   [Throws=FxaError]
-  void initialize_device([ByRef] string name,  DeviceType device_type,  sequence<DeviceCapability> supported_capabilities );
+  LocalDevice initialize_device([ByRef] string name,  DeviceType device_type,  sequence<DeviceCapability> supported_capabilities );
 
 
   // Get the device id registered for this application.
@@ -383,7 +383,7 @@ interface FirefoxAccount {
   //      granted the `https://identity.mozilla.com/apps/oldsync` scope.
   //
   [Throws=FxaError]
-  void set_device_name([ByRef] string display_name );
+  LocalDevice set_device_name([ByRef] string display_name );
 
 
   // Clear any custom display name used for this application instance.
@@ -426,7 +426,7 @@ interface FirefoxAccount {
   //      granted the `https://identity.mozilla.com/apps/oldsync` scope.
   //
   [Throws=FxaError]
-  void ensure_capabilities( sequence<DeviceCapability> supported_capabilities );
+  LocalDevice ensure_capabilities( sequence<DeviceCapability> supported_capabilities );
 
 
   // Set or update a push subscription endpoint for this device.
@@ -449,7 +449,7 @@ interface FirefoxAccount {
   //      granted the `https://identity.mozilla.com/apps/oldsync` scope.
   //
   [Throws=FxaError]
-  void set_push_subscription( DevicePushSubscription subscription );
+  LocalDevice set_push_subscription( DevicePushSubscription subscription );
 
 
   // Process and respond to a server-delivered account update message
@@ -808,6 +808,19 @@ dictionary Device {
   i64? last_access_time;
 };
 
+// Local device that's connecting to FxA
+//
+// This is returned by the device update methods and represents the server's view of the local
+// device.
+dictionary LocalDevice {
+  string id;
+  string display_name;
+  DeviceType device_type;
+  sequence<DeviceCapability> capabilities;
+  DevicePushSubscription? push_subscription;
+  boolean push_endpoint_expired;
+};
+
 // Details of a web-push subscription endpoint.
 //
 // This struct encapsulates the details of a web-push subscription endpoint,

components/fxa-client/src/internal/device.rs

@@ -9,10 +9,12 @@ pub use super::http_client::{
 };
 use super::{
     commands::{self, IncomingDeviceCommand},
-    http_client::{DeviceUpdateRequest, DeviceUpdateRequestBuilder, PendingCommand},
+    http_client::{
+        DeviceUpdateRequest, DeviceUpdateRequestBuilder, PendingCommand, UpdateDeviceResponse,
+    },
     telemetry, util, CachedResponse, FirefoxAccount,
 };
-use crate::{DeviceCapability, Error, Result};
+use crate::{DeviceCapability, Error, LocalDevice, Result};
 use sync15::DeviceType;
 
 // An devices response is considered fresh for `DEVICES_FRESHNESS_THRESHOLD` ms.
@@ -82,11 +84,6 @@ impl FirefoxAccount {
                 }
             }
         }
-        // Remember what capabilities we've registered, so we don't register the same ones again.
-        // We write this to internal state before we've actually written the new device record,
-        // but roll it back if the server update fails.
-        self.state
-            .update_last_sent_device_capabilities(capabilities_set);
         Ok(commands)
     }
 
@@ -99,7 +96,7 @@ impl FirefoxAccount {
         name: &str,
         device_type: DeviceType,
         capabilities: &[DeviceCapability],
-    ) -> Result<()> {
+    ) -> Result<LocalDevice> {
         let commands = self.register_capabilities(capabilities)?;
         let update = DeviceUpdateRequestBuilder::new()
             .display_name(name)
@@ -116,19 +113,15 @@ impl FirefoxAccount {
     /// encrypt the Send Tab command data.
     ///
     /// **💾 This method alters the persisted account state.**
-    pub fn ensure_capabilities(&mut self, capabilities: &[DeviceCapability]) -> Result<()> {
+    pub fn ensure_capabilities(
+        &mut self,
+        capabilities: &[DeviceCapability],
+    ) -> Result<LocalDevice> {
         // Don't re-register if we already have exactly those capabilities.
-        // Because of the way that our state object defaults `device_capabilities` to empty,
-        // we can't tell the difference between "have never registered capabilities" and
-        // have "explicitly registered an empty set of capabilities", so it's simpler to
-        // just always re-register in that case.
-        if !self.state.last_sent_device_capabilities().is_empty()
-            && self.state.last_sent_device_capabilities().len() == capabilities.len()
-            && capabilities
-                .iter()
-                .all(|c| self.state.last_sent_device_capabilities().contains(c))
-        {
-            return Ok(());
+        if let Some(local_device) = self.state.server_local_device_info() {
+            if capabilities == local_device.capabilities {
+                return Ok(local_device.clone());
+            }
         }
         let commands = self.register_capabilities(capabilities)?;
         let update = DeviceUpdateRequestBuilder::new()
@@ -139,17 +132,13 @@ impl FirefoxAccount {
 
     /// Re-register the device capabilities, this should only be used internally.
     pub(crate) fn reregister_current_capabilities(&mut self) -> Result<()> {
-        let current_capabilities: Vec<DeviceCapability> = self
-            .state
-            .last_sent_device_capabilities()
-            .clone()
-            .into_iter()
-            .collect();
-        let commands = self.register_capabilities(&current_capabilities)?;
-        let update = DeviceUpdateRequestBuilder::new()
-            .available_commands(&commands)
-            .build();
-        self.update_device(update)?;
+        if let Some(local_device) = self.state.server_local_device_info().cloned() {
+            let commands = self.register_capabilities(&local_device.capabilities)?;
+            let update = DeviceUpdateRequestBuilder::new()
+                .available_commands(&commands)
+                .build();
+            self.update_device(update)?;
+        }
         Ok(())
     }
 
@@ -264,7 +253,7 @@ impl FirefoxAccount {
         }
     }
 
-    pub fn set_device_name(&mut self, name: &str) -> Result<()> {
+    pub fn set_device_name(&mut self, name: &str) -> Result<LocalDevice> {
         let update = DeviceUpdateRequestBuilder::new().display_name(name).build();
         self.update_device(update)
     }
@@ -273,10 +262,14 @@ impl FirefoxAccount {
         let update = DeviceUpdateRequestBuilder::new()
             .clear_display_name()
             .build();
-        self.update_device(update)
+        self.update_device(update)?;
+        Ok(())
     }
 
-    pub fn set_push_subscription(&mut self, push_subscription: PushSubscription) -> Result<()> {
+    pub fn set_push_subscription(
+        &mut self,
+        push_subscription: PushSubscription,
+    ) -> Result<LocalDevice> {
         let update = DeviceUpdateRequestBuilder::new()
             .push_subscription(&push_subscription)
             .build();
@@ -290,31 +283,35 @@ impl FirefoxAccount {
         push_subscription: &Option<PushSubscription>,
         commands: &HashMap<String, String>,
     ) -> Result<()> {
-        self.state.clear_last_sent_device_capabilities();
+        self.state.clear_server_local_device_info();
         let mut builder = DeviceUpdateRequestBuilder::new()
             .display_name(display_name)
             .device_type(device_type)
             .available_commands(commands);
         if let Some(push_subscription) = push_subscription {
             builder = builder.push_subscription(push_subscription)
         }
-        self.update_device(builder.build())
+        self.update_device(builder.build())?;
+        Ok(())
     }
 
-    fn update_device(&mut self, update: DeviceUpdateRequest<'_>) -> Result<()> {
+    fn update_device(&mut self, update: DeviceUpdateRequest<'_>) -> Result<LocalDevice> {
         let refresh_token = self.get_refresh_token()?;
         let res = self
             .client
             .update_device_record(self.state.config(), refresh_token, update);
         match res {
             Ok(resp) => {
-                self.state.set_current_device_id(resp.id);
-                Ok(())
+                self.state.set_current_device_id(resp.id.clone());
+                let local_device = LocalDevice::from(resp);
+                self.state
+                    .update_server_local_device_info(local_device.clone());
+                Ok(local_device)
             }
             Err(err) => {
                 // We failed to write an update to the server.
                 // Clear local state so that we'll be sure to retry later.
-                self.state.clear_last_sent_device_capabilities();
+                self.state.clear_server_local_device_info();
                 Err(err)
             }
         }
@@ -329,6 +326,40 @@ impl FirefoxAccount {
     }
 }
 
+impl TryFrom<String> for DeviceCapability {
+    type Error = Error;
+
+    fn try_from(command: String) -> Result<Self> {
+        match command.as_str() {
+            commands::send_tab::COMMAND_NAME => Ok(DeviceCapability::SendTab),
+            _ => Err(Error::UnknownCommand(command)),
+        }
+    }
+}
+
+impl From<UpdateDeviceResponse> for LocalDevice {
+    fn from(resp: UpdateDeviceResponse) -> Self {
+        Self {
+            id: resp.id,
+            display_name: resp.display_name,
+            device_type: resp.device_type,
+            capabilities: resp
+                .available_commands
+                .into_keys()
+                .filter_map(|command| match command.try_into() {
+                    Ok(capability) => Some(capability),
+                    Err(e) => {
+                        log::warn!("While parsing UpdateDeviceResponse: {e}");
+                        None
+                    }
+                })
+                .collect(),
+            push_subscription: resp.push_subscription.map(Into::into),
+            push_endpoint_expired: resp.push_endpoint_expired,
+        }
+    }
+}
+
 impl TryFrom<Device> for crate::Device {
     type Error = Error;
     fn try_from(d: Device) -> Result<Self> {
@@ -399,7 +430,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));
@@ -459,7 +493,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));
@@ -482,7 +519,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         restored.set_client(Arc::new(client));
@@ -509,7 +549,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));
@@ -578,7 +621,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));
@@ -630,7 +676,7 @@ mod tests {
         )
         .unwrap();
 
-        assert!(fxa.state.last_sent_device_capabilities().is_empty());
+        assert!(fxa.state.server_local_device_info().is_none());
 
         // Do another call with the same capabilities.
         // It should re-register, as server-side state may have changed.
@@ -646,7 +692,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));
@@ -691,7 +740,10 @@ mod tests {
                 display_name: "".to_string(),
                 device_type: DeviceType::Desktop,
                 push_subscription: None,
-                available_commands: HashMap::default(),
+                available_commands: HashMap::from([(
+                    commands::send_tab::COMMAND_NAME.to_owned(),
+                    "fake-command-data".to_owned(),
+                )]),
                 push_endpoint_expired: false,
             }));
         fxa.set_client(Arc::new(client));