libweave: Add 'reason' to backup command fetch requests
When XMPP channel is active we periodically (once every 30 minutes)
do a command fetch from the server. If XMPP channel doesn't miss any
commands, this fetch of command queue should never return any commands.
To help debug this on the server side, added "reason=just_in_case" to
such "backup" requests so we can identify if ever the server has to
return something for it. In addition, other reasons are added for
different fetch request origins:
- device_start
- regular_pull
- new_command
- just_in_case
BUG: 23321447
Change-Id: I6f2daf30424bc54a7b6d7d06052509e902c7ce1a
Reviewed-on: https://weave-review.googlesource.com/1391
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc
index 458b57d..8945b66 100644
--- a/libweave/src/device_registration_info.cc
+++ b/libweave/src/device_registration_info.cc
@@ -43,6 +43,15 @@
const int kPollingPeriodSeconds = 7;
const int kBackupPollingPeriodMinutes = 30;
+namespace fetch_reason {
+
+const char kDeviceStart[] = "device_start"; // Initial queue fetch at startup.
+const char kRegularPull[] = "regular_pull"; // Regular fetch before XMPP is up.
+const char kNewCommand[] = "new_command"; // A new command is available.
+const char kJustInCase[] = "just_in_case"; // Backup fetch when XMPP is live.
+
+} // namespace fetch_reason
+
using provider::HttpClient;
inline void SetUnexpectedError(ErrorPtr* error) {
@@ -789,7 +798,7 @@
LOG(INFO) << "Device connected to cloud server";
connected_to_cloud_ = true;
FetchCommands(base::Bind(&DeviceRegistrationInfo::ProcessInitialCommandList,
- AsWeakPtr()));
+ AsWeakPtr()), fetch_reason::kDeviceStart);
// In case there are any pending state updates since we sent off the initial
// UpdateDeviceResource() request, update the server with any state changes.
PublishStateUpdates();
@@ -985,29 +994,33 @@
fetch_commands_request_sent_ = false;
// If we have additional requests queued, send them out now.
if (fetch_commands_request_queued_)
- FetchAndPublishCommands();
+ FetchAndPublishCommands(queued_fetch_reason_);
}
void DeviceRegistrationInfo::FetchCommands(
- const base::Callback<void(const base::ListValue&, ErrorPtr error)>&
- callback) {
+ const base::Callback<void(const base::ListValue&, ErrorPtr)>& callback,
+ const std::string& reason) {
fetch_commands_request_sent_ = true;
fetch_commands_request_queued_ = false;
DoCloudRequest(
HttpClient::Method::kGet,
- GetServiceURL("commands/queue", {{"deviceId", GetSettings().cloud_id}}),
+ GetServiceURL("commands/queue", {{"deviceId", GetSettings().cloud_id},
+ {"reason", reason}}),
nullptr, base::Bind(&DeviceRegistrationInfo::OnFetchCommandsDone,
AsWeakPtr(), callback));
}
-void DeviceRegistrationInfo::FetchAndPublishCommands() {
+void DeviceRegistrationInfo::FetchAndPublishCommands(
+ const std::string& reason) {
if (fetch_commands_request_sent_) {
fetch_commands_request_queued_ = true;
+ queued_fetch_reason_ = reason;
return;
}
FetchCommands(base::Bind(&DeviceRegistrationInfo::PublishCommands,
- weak_factory_.GetWeakPtr()));
+ weak_factory_.GetWeakPtr()),
+ reason);
}
void DeviceRegistrationInfo::ProcessInitialCommandList(
@@ -1200,7 +1213,7 @@
UpdateDeviceResource(
base::Bind(&IgnoreCloudErrorWithCallback,
base::Bind(&DeviceRegistrationInfo::FetchAndPublishCommands,
- AsWeakPtr())));
+ AsWeakPtr(), fetch_reason::kRegularPull)));
}
void DeviceRegistrationInfo::OnDisconnected() {
@@ -1222,7 +1235,8 @@
}
void DeviceRegistrationInfo::OnCommandCreated(
- const base::DictionaryValue& command) {
+ const base::DictionaryValue& command,
+ const std::string& channel_name) {
if (!connected_to_cloud_)
return;
@@ -1234,10 +1248,21 @@
PublishCommand(command);
return;
}
+
+ // If this request comes from a Pull channel while the primary notification
+ // channel (XMPP) is active, we are doing a backup poll, so mark the request
+ // appropriately.
+ bool just_in_case =
+ (channel_name == kPullChannelName) &&
+ (current_notification_channel_ == primary_notification_channel_.get());
+
+ std::string reason =
+ just_in_case ? fetch_reason::kJustInCase : fetch_reason::kNewCommand;
+
// If the command was too big to be delivered over a notification channel,
// or OnCommandCreated() was initiated from the Pull notification,
// perform a manual command fetch from the server here.
- FetchAndPublishCommands();
+ FetchAndPublishCommands(reason);
}
void DeviceRegistrationInfo::OnDeviceDeleted(const std::string& cloud_id) {
diff --git a/libweave/src/device_registration_info.h b/libweave/src/device_registration_info.h
index 8eb0ec3..f3bacc6 100644
--- a/libweave/src/device_registration_info.h
+++ b/libweave/src/device_registration_info.h
@@ -210,7 +210,8 @@
bool UpdateDeviceInfoTimestamp(const base::DictionaryValue& device_info);
void FetchCommands(
- const base::Callback<void(const base::ListValue&, ErrorPtr)>& callback);
+ const base::Callback<void(const base::ListValue&, ErrorPtr)>& callback,
+ const std::string& reason);
void OnFetchCommandsDone(
const base::Callback<void(const base::ListValue&, ErrorPtr)>& callback,
const base::DictionaryValue& json,
@@ -230,7 +231,9 @@
// Helper function to pull the pending command list from the server using
// FetchCommands() and make them available on D-Bus with PublishCommands().
- void FetchAndPublishCommands();
+ // |backup_fetch| is set to true when performing backup ("just-in-case")
+ // command fetch while XMPP channel is up and running.
+ void FetchAndPublishCommands(const std::string& reason);
void PublishStateUpdates();
void OnPublishStateDone(StateChangeQueueInterface::UpdateID update_id,
@@ -258,7 +261,8 @@
void OnConnected(const std::string& channel_name) override;
void OnDisconnected() override;
void OnPermanentFailure() override;
- void OnCommandCreated(const base::DictionaryValue& command) override;
+ void OnCommandCreated(const base::DictionaryValue& command,
+ const std::string& channel_name) override;
void OnDeviceDeleted(const std::string& cloud_id) override;
// Wipes out the device registration information and stops server connections.
@@ -316,6 +320,8 @@
// Set to true when another command queue fetch request is queued while
// another one was in flight.
bool fetch_commands_request_queued_{false};
+ // Specifies the reason for queued command fetch request.
+ std::string queued_fetch_reason_;
using ResourceUpdateCallbackList = std::vector<DoneCallback>;
// Callbacks for device resource update request currently in flight to the
diff --git a/libweave/src/notification/notification_delegate.h b/libweave/src/notification/notification_delegate.h
index feb9d17..719d76d 100644
--- a/libweave/src/notification/notification_delegate.h
+++ b/libweave/src/notification/notification_delegate.h
@@ -18,7 +18,8 @@
virtual void OnDisconnected() = 0;
virtual void OnPermanentFailure() = 0;
// Called when a new command is sent via the notification channel.
- virtual void OnCommandCreated(const base::DictionaryValue& command) = 0;
+ virtual void OnCommandCreated(const base::DictionaryValue& command,
+ const std::string& channel_name) = 0;
// Called when DEVICE_DELETED notification is received.
virtual void OnDeviceDeleted(const std::string& cloud_id) = 0;
diff --git a/libweave/src/notification/notification_parser.cc b/libweave/src/notification/notification_parser.cc
index 25c525f..0a27f1c 100644
--- a/libweave/src/notification/notification_parser.cc
+++ b/libweave/src/notification/notification_parser.cc
@@ -12,14 +12,15 @@
// Processes COMMAND_CREATED notifications.
bool ParseCommandCreated(const base::DictionaryValue& notification,
- NotificationDelegate* delegate) {
+ NotificationDelegate* delegate,
+ const std::string& channel_name) {
const base::DictionaryValue* command = nullptr;
if (!notification.GetDictionary("command", &command)) {
LOG(ERROR) << "COMMAND_CREATED notification is missing 'command' property";
return false;
}
- delegate->OnCommandCreated(*command);
+ delegate->OnCommandCreated(*command, channel_name);
return true;
}
@@ -39,7 +40,8 @@
} // anonymous namespace
bool ParseNotificationJson(const base::DictionaryValue& notification,
- NotificationDelegate* delegate) {
+ NotificationDelegate* delegate,
+ const std::string& channel_name) {
CHECK(delegate);
std::string kind;
@@ -57,7 +59,7 @@
}
if (type == "COMMAND_CREATED")
- return ParseCommandCreated(notification, delegate);
+ return ParseCommandCreated(notification, delegate, channel_name);
if (type == "DEVICE_DELETED")
return ParseDeviceDeleted(notification, delegate);
diff --git a/libweave/src/notification/notification_parser.h b/libweave/src/notification/notification_parser.h
index ed62d10..01932c7 100644
--- a/libweave/src/notification/notification_parser.h
+++ b/libweave/src/notification/notification_parser.h
@@ -17,7 +17,8 @@
// the appropriate method from the |delegate|.
// Returns false if unexpected or malformed notification is received.
bool ParseNotificationJson(const base::DictionaryValue& notification,
- NotificationDelegate* delegate);
+ NotificationDelegate* delegate,
+ const std::string& channel_name);
} // namespace weave
diff --git a/libweave/src/notification/notification_parser_unittest.cc b/libweave/src/notification/notification_parser_unittest.cc
index 146208f..d0b43fe 100644
--- a/libweave/src/notification/notification_parser_unittest.cc
+++ b/libweave/src/notification/notification_parser_unittest.cc
@@ -17,12 +17,17 @@
using test::CreateDictionaryValue;
+MATCHER_P(MatchDict, str, "") {
+ return arg.Equals(CreateDictionaryValue(str).get());
+}
+
class MockNotificationDelegate : public NotificationDelegate {
public:
MOCK_METHOD1(OnConnected, void(const std::string&));
MOCK_METHOD0(OnDisconnected, void());
MOCK_METHOD0(OnPermanentFailure, void());
- MOCK_METHOD1(OnCommandCreated, void(const base::DictionaryValue& command));
+ MOCK_METHOD2(OnCommandCreated, void(const base::DictionaryValue& command,
+ const std::string& channel_name));
MOCK_METHOD1(OnDeviceDeleted, void(const std::string&));
};
@@ -51,14 +56,6 @@
"commandId": "command_id"
})");
- base::DictionaryValue command_instance;
- auto on_command = [&command_instance](const base::DictionaryValue& command) {
- command_instance.MergeDictionary(&command);
- };
-
- EXPECT_CALL(delegate_, OnCommandCreated(_)).WillOnce(Invoke(on_command));
- EXPECT_TRUE(ParseNotificationJson(*json, &delegate_));
-
const char expected_json[] = R"({
"kind": "clouddevices#command",
"deviceId": "device_id",
@@ -71,7 +68,10 @@
"id": "command_id",
"creationTimeMs": "1403444174811"
})";
- EXPECT_JSON_EQ(expected_json, command_instance);
+
+ EXPECT_CALL(delegate_, OnCommandCreated(MatchDict(expected_json), "foo"))
+ .Times(1);
+ EXPECT_TRUE(ParseNotificationJson(*json, &delegate_, "foo"));
}
TEST_F(NotificationParserTest, DeviceDeleted) {
@@ -83,7 +83,7 @@
std::string device_id;
EXPECT_CALL(delegate_, OnDeviceDeleted(_)).WillOnce(SaveArg<0>(&device_id));
- EXPECT_TRUE(ParseNotificationJson(*json, &delegate_));
+ EXPECT_TRUE(ParseNotificationJson(*json, &delegate_, "foo"));
EXPECT_EQ("some_device_id", device_id);
}
@@ -106,7 +106,7 @@
"commandId": "command_id"
})");
- EXPECT_FALSE(ParseNotificationJson(*json, &delegate_));
+ EXPECT_FALSE(ParseNotificationJson(*json, &delegate_, "bar"));
}
TEST_F(NotificationParserTest, Failure_NoType) {
@@ -128,7 +128,7 @@
"commandId": "command_id"
})");
- EXPECT_FALSE(ParseNotificationJson(*json, &delegate_));
+ EXPECT_FALSE(ParseNotificationJson(*json, &delegate_, "baz"));
}
TEST_F(NotificationParserTest, IgnoredNotificationType) {
@@ -151,7 +151,7 @@
"commandId": "command_id"
})");
- EXPECT_TRUE(ParseNotificationJson(*json, &delegate_));
+ EXPECT_TRUE(ParseNotificationJson(*json, &delegate_, "quux"));
}
} // namespace weave
diff --git a/libweave/src/notification/pull_channel.cc b/libweave/src/notification/pull_channel.cc
index b192df6..378351a 100644
--- a/libweave/src/notification/pull_channel.cc
+++ b/libweave/src/notification/pull_channel.cc
@@ -12,12 +12,14 @@
namespace weave {
+const char kPullChannelName[] = "pull";
+
PullChannel::PullChannel(base::TimeDelta pull_interval,
provider::TaskRunner* task_runner)
: pull_interval_{pull_interval}, task_runner_{task_runner} {}
std::string PullChannel::GetName() const {
- return "pull";
+ return kPullChannelName;
}
bool PullChannel::IsConnected() const {
@@ -58,7 +60,7 @@
// Repost before delegate notification to give it a chance to stop channel.
RePost();
base::DictionaryValue empty_dict;
- delegate_->OnCommandCreated(empty_dict);
+ delegate_->OnCommandCreated(empty_dict, GetName());
}
} // namespace weave
diff --git a/libweave/src/notification/pull_channel.h b/libweave/src/notification/pull_channel.h
index a48b9f6..7f859e8 100644
--- a/libweave/src/notification/pull_channel.h
+++ b/libweave/src/notification/pull_channel.h
@@ -20,6 +20,8 @@
class TaskRunner;
} // namespace
+extern const char kPullChannelName[];
+
class PullChannel : public NotificationChannel {
public:
PullChannel(base::TimeDelta pull_interval, provider::TaskRunner* task_runner);
diff --git a/libweave/src/notification/xmpp_channel.cc b/libweave/src/notification/xmpp_channel.cc
index bca4c3b..ceb45ed 100644
--- a/libweave/src/notification/xmpp_channel.cc
+++ b/libweave/src/notification/xmpp_channel.cc
@@ -279,7 +279,7 @@
VLOG(2) << "XMPP push notification data: " << json_data;
auto json_dict = LoadJsonDict(json_data, nullptr);
if (json_dict && delegate_)
- ParseNotificationJson(*json_dict, delegate_);
+ ParseNotificationJson(*json_dict, delegate_, GetName());
}
void XmppChannel::CreateSslSocket() {
