Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add deviceId to AccountEvent.deviceDisconnected #2645

Merged
merged 1 commit into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGES_UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
- `.DeviceConnected` can be handled by showing a "<Device name> is connected to this account" notification.
- `.DeviceDisconnected` should be handled by showing a "re-auth" state to the user if `isLocalDevice` is true. There is no need to call `FirefoxAccount.disconnect` as it will fail.

- iOS: Added `FxAccountManager.getSessionToken`. Note that you should request the `.session` scope in the constructor for this to work properly ([#2638](https://github.com/mozilla/application-services/pull/2638))
- iOS: Added `FxAccountManager.getSessionToken`. Note that you should request the `.session` scope in the constructor for this to work properly. ([#2638](https://github.com/mozilla/application-services/pull/2638))

- Added a `deviceId` property to the `AccountEvent.deviceDisconnected` enum case. ([#2645](https://github.com/mozilla/application-services/pull/2645))

### Breaking changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ sealed class AccountEvent {
class AccountAuthStateChanged : AccountEvent()
class AccountDestroyed : AccountEvent()
class DeviceConnected(val deviceName: String) : AccountEvent()
class DeviceDisconnected(val isLocalDevice: Boolean) : AccountEvent()
class DeviceDisconnected(val deviceId: String, val isLocalDevice: Boolean) : AccountEvent()

companion object {
private fun fromMessage(msg: MsgTypes.AccountEvent): AccountEvent {
Expand All @@ -30,7 +30,8 @@ sealed class AccountEvent {
deviceName = msg.deviceConnectedName
)
MsgTypes.AccountEvent.AccountEventType.DEVICE_DISCONNECTED -> DeviceDisconnected(
isLocalDevice = msg.deviceDisconnectedIsLocal
deviceId = msg.deviceDisconnectedData.deviceId,
isLocalDevice = msg.deviceDisconnectedData.isLocalDevice
)
null -> throw NullPointerException("AccountEvent type cannot be null.")
}.exhaustive
Expand Down
7 changes: 5 additions & 2 deletions components/fxa-client/ios/FxAClient/FxAccountDevice.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public enum IncomingDeviceCommand {
public enum AccountEvent {
case incomingDeviceCommand(IncomingDeviceCommand)
case deviceConnected(deviceName: String)
case deviceDisconnected(isLocalDevice: Bool)
case deviceDisconnected(deviceId: String, isLocalDevice: Bool)

internal static func fromCollectionMsg(msg: MsgTypes_AccountEvents) -> [AccountEvent] {
msg.events.compactMap { AccountEvent.fromMsg(msg: $0) }
Expand All @@ -142,7 +142,10 @@ public enum AccountEvent {
return .deviceConnected(deviceName: msg.deviceConnectedName)
}
case .deviceDisconnected: do {
return .deviceDisconnected(isLocalDevice: msg.deviceDisconnectedIsLocal)
return .deviceDisconnected(
deviceId: msg.deviceDisconnectedData.deviceID,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is D in deviceID correct here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

isLocalDevice: msg.deviceDisconnectedData.isLocalDevice
)
}
// The following push messages are filtered upstream by the FxA server,
// because iOS requires all Push messages to show a UI notification to the user
Expand Down
12 changes: 9 additions & 3 deletions components/fxa-client/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,16 @@ impl From<AccountEvent> for msg_types::AccountEvent {
device_name,
)),
},
AccountEvent::DeviceDisconnected { is_local_device } => Self {
AccountEvent::DeviceDisconnected {
device_id,
is_local_device,
} => Self {
r#type: msg_types::account_event::AccountEventType::DeviceDisconnected as i32,
data: Some(msg_types::account_event::Data::DeviceDisconnectedIsLocal(
is_local_device,
data: Some(msg_types::account_event::Data::DeviceDisconnectedData(
msg_types::account_event::DeviceDisconnectedData {
device_id,
is_local_device,
},
)),
},
}
Expand Down
9 changes: 7 additions & 2 deletions components/fxa-client/src/fxa_msg_types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,20 @@ message AccountEvent {
PROFILE_UPDATED = 2;
DEVICE_CONNECTED = 3; // `data` set to `device_connected_name`.
ACCOUNT_AUTH_STATE_CHANGED = 4;
DEVICE_DISCONNECTED = 5; // `data` set to `device_disconnected_is_local`.
DEVICE_DISCONNECTED = 5; // `data` set to `device_disconnected_data`.
ACCOUNT_DESTROYED = 6;
}
required AccountEventType type = 1;

message DeviceDisconnectedData {
required string device_id = 1;
required bool is_local_device = 2;
}

oneof data {
IncomingDeviceCommand device_command = 2;
string device_connected_name = 3;
bool device_disconnected_is_local = 4;
DeviceDisconnectedData device_disconnected_data = 4;
}
}

Expand Down
9 changes: 7 additions & 2 deletions components/fxa-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,13 @@ pub enum AccountEvent {
ProfileUpdated,
AccountAuthStateChanged,
AccountDestroyed,
DeviceConnected { device_name: String },
DeviceDisconnected { is_local_device: bool },
DeviceConnected {
device_name: String,
},
DeviceDisconnected {
device_id: String,
is_local_device: bool,
},
}

pub enum IncomingDeviceCommand {
Expand Down
25 changes: 20 additions & 5 deletions components/fxa-client/src/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ impl FirefoxAccount {
if is_local_device {
self.disconnect();
}
Ok(vec![AccountEvent::DeviceDisconnected { is_local_device }])
Ok(vec![AccountEvent::DeviceDisconnected {
device_id,
is_local_device,
}])
}
PushPayload::AccountDestroyed(AccountDestroyedPushPayload { account_uid }) => {
let is_local_account = match &self.state.last_seen_profile {
Expand Down Expand Up @@ -161,8 +164,14 @@ mod tests {
let events = fxa.handle_push_message(json).unwrap();
assert!(fxa.state.refresh_token.is_none());
assert_eq!(events.len(), 1);
match events[0] {
AccountEvent::DeviceDisconnected { is_local_device } => assert!(is_local_device),
match &events[0] {
AccountEvent::DeviceDisconnected {
device_id,
is_local_device,
} => {
assert!(is_local_device);
assert_eq!(device_id, "my_id");
}
_ => unreachable!(),
};
}
Expand All @@ -174,8 +183,14 @@ mod tests {
let json = "{\"version\":1,\"command\":\"fxaccounts:device_disconnected\",\"data\":{\"id\":\"remote_id\"}}";
let events = fxa.handle_push_message(json).unwrap();
assert_eq!(events.len(), 1);
match events[0] {
AccountEvent::DeviceDisconnected { is_local_device } => assert!(!is_local_device),
match &events[0] {
AccountEvent::DeviceDisconnected {
device_id,
is_local_device,
} => {
assert!(!is_local_device);
assert_eq!(device_id, "remote_id");
}
_ => unreachable!(),
};
}
Expand Down