buffet: Fix command progress update JSON for GCD server
GCD server expects command progress updates to contain a device-specific
JSON object, while buffet was providing just an integer. Make GCD server
happy by creating an object instead by sticking another "progress" property
as in {"progress":{"progress":100}}.
Also added some unit tests to cover the rest of UpdateCommand notifications.
BUG=brillo:779
TEST=`FEATURES=test emerge-link libchromeos buffet`
Change-Id: I210de62abae7fa1c357449b0c07b5298e8606ec0
Reviewed-on: https://chromium-review.googlesource.com/264867
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/commands/cloud_command_proxy.cc b/buffet/commands/cloud_command_proxy.cc
index 341401e..4463c50 100644
--- a/buffet/commands/cloud_command_proxy.cc
+++ b/buffet/commands/cloud_command_proxy.cc
@@ -35,7 +35,8 @@
void CloudCommandProxy::OnProgressChanged(int progress) {
base::DictionaryValue patch;
- patch.SetInteger(commands::attributes::kCommand_Progress, progress);
+ patch.Set(commands::attributes::kCommand_Progress,
+ command_instance_->GetProgressJson().release());
// TODO(antonm): Consider batching progress change updates.
device_registration_info_->UpdateCommand(command_instance_->GetID(), patch);
}
diff --git a/buffet/commands/command_instance.cc b/buffet/commands/command_instance.cc
index c12f1ed..c67411f 100644
--- a/buffet/commands/command_instance.cc
+++ b/buffet/commands/command_instance.cc
@@ -152,7 +152,8 @@
TypedValueToJson(parameters_, nullptr).release());
json->Set(commands::attributes::kCommand_Results,
TypedValueToJson(results_, nullptr).release());
- json->SetInteger(commands::attributes::kCommand_Progress, progress_);
+ json->Set(commands::attributes::kCommand_Progress,
+ GetProgressJson().release());
json->SetString(commands::attributes::kCommand_State, status_);
return json;
@@ -219,5 +220,15 @@
queue_->DelayedRemove(GetID());
}
+std::unique_ptr<base::Value> CommandInstance::GetProgressJson() const {
+ // GCD server requires "progress" to be a JSON object. We will just make
+ // an object with a single field, "progress", so the patch request will
+ // look like this: {"progress": {"progress":100}}.
+ std::unique_ptr<base::DictionaryValue> progress_object{
+ new base::DictionaryValue};
+ progress_object->SetInteger(commands::attributes::kCommand_Progress,
+ progress_);
+ return std::move(progress_object);
+}
} // namespace buffet
diff --git a/buffet/commands/command_instance.h b/buffet/commands/command_instance.h
index 2b09829..e5e0f9f 100644
--- a/buffet/commands/command_instance.h
+++ b/buffet/commands/command_instance.h
@@ -95,6 +95,9 @@
int GetProgress() const { return progress_; }
const std::string& GetStatus() const { return status_; }
+ // Returns the "progress" portion of JSON command resource.
+ std::unique_ptr<base::Value> GetProgressJson() const;
+
// Values for command execution status.
static const char kStatusQueued[];
static const char kStatusInProgress[];
diff --git a/buffet/commands/command_instance_unittest.cc b/buffet/commands/command_instance_unittest.cc
index 48d54b9..e1d874b 100644
--- a/buffet/commands/command_instance_unittest.cc
+++ b/buffet/commands/command_instance_unittest.cc
@@ -209,7 +209,7 @@
json->MergeDictionary(CreateDictionaryValue(R"({
'id': 'testId',
- 'progress': 15,
+ 'progress': {'progress': 15},
'state': 'inProgress',
'results': {'testResult': 17}
})").get());
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index e3fb737..c800052 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -521,6 +521,8 @@
})");
EXPECT_TRUE(command_manager_->LoadCommands(*json_cmds, "", nullptr));
+ const std::string command_url = dev_reg_->GetServiceURL("commands/1234");
+
auto commands_json = buffet::unittests::CreateValue(R"([{
'name':'robot._jump',
'id':'1234',
@@ -538,22 +540,50 @@
{"status", string_type.CreateValue(std::string{"Ok"}, nullptr)}
};
- auto update_command = [](const ServerRequest& request,
- ServerResponse* response) {
- // Make sure we serialize the JSON back without any pretty print so
- // the string comparison works correctly.
- auto json = request.GetDataAsJson();
- EXPECT_NE(nullptr, json.get());
- std::string value;
- ASSERT_TRUE(base::JSONWriter::Write(json.get(), &value));
- EXPECT_EQ(R"({"results":{"status":"Ok"}})", value);
+ // UpdateCommand when setting command results.
+ auto update_command_results = [](const ServerRequest& request,
+ ServerResponse* response) {
+ EXPECT_EQ(R"({"results":{"status":"Ok"}})",
+ request.GetDataAsNormalizedJsonString());
};
- transport_->AddHandler(dev_reg_->GetServiceURL("commands/1234"),
- chromeos::http::request_type::kPatch,
- base::Bind(update_command));
+ transport_->AddHandler(command_url,
+ chromeos::http::request_type::kPatch,
+ base::Bind(update_command_results));
command->SetResults(results);
+
+ // UpdateCommand when setting command progress.
+ int count = 0; // This will be called twice...
+ auto update_command_progress = [&count](const ServerRequest& request,
+ ServerResponse* response) {
+ if (count++ == 0) {
+ EXPECT_EQ(R"({"state":"inProgress"})",
+ request.GetDataAsNormalizedJsonString());
+ } else {
+ EXPECT_EQ(R"({"progress":{"progress":18}})",
+ request.GetDataAsNormalizedJsonString());
+ }
+ };
+
+ transport_->AddHandler(command_url,
+ chromeos::http::request_type::kPatch,
+ base::Bind(update_command_progress));
+
+ command->SetProgress(18);
+
+ // UpdateCommand when changing command status.
+ auto update_command_state = [](const ServerRequest& request,
+ ServerResponse* response) {
+ EXPECT_EQ(R"({"state":"cancelled"})",
+ request.GetDataAsNormalizedJsonString());
+ };
+
+ transport_->AddHandler(command_url,
+ chromeos::http::request_type::kPatch,
+ base::Bind(update_command_state));
+
+ command->Cancel();
}
