Extract new system version from Omaha responses am: 7dcdedf1b6
am: 80ded6b90a
Change-Id: I23fb9176f51c4615b9c1f3905c7235ca0e05f6a4
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index c3bbf9d..cce4287 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -392,6 +392,7 @@
string daystart_elapsed_seconds;
struct App {
+ string id;
vector<string> url_codebase;
string manifest_version;
map<string, string> action_postinstall_attrs;
@@ -430,6 +431,9 @@
if (data->current_path == "/response/app") {
data->apps.emplace_back();
+ if (attrs.find("appid") != attrs.end()) {
+ data->apps.back().id = attrs["appid"];
+ }
if (attrs.find("cohort") != attrs.end()) {
data->app_cohort_set = true;
data->app_cohort = attrs["cohort"];
@@ -983,10 +987,18 @@
ScopedActionCompleter* completer) {
map<string, string> attrs;
for (auto& app : parser_data->apps) {
- if (!app.manifest_version.empty() && output_object->version.empty())
+ if (app.id == params_->GetAppId()) {
+ // this is the app (potentially the only app)
output_object->version = app.manifest_version;
- if (!app.action_postinstall_attrs.empty() && attrs.empty())
+ } else if (!params_->system_app_id().empty() &&
+ app.id == params_->system_app_id()) {
+ // this is the system app (this check is intentionally skipped if there is
+ // no system_app_id set)
+ output_object->system_version = app.manifest_version;
+ }
+ if (!app.action_postinstall_attrs.empty() && attrs.empty()) {
attrs = app.action_postinstall_attrs;
+ }
}
if (output_object->version.empty()) {
LOG(ERROR) << "Omaha Response does not have version in manifest!";
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 9091031..7e38956 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -68,6 +68,7 @@
namespace {
const char kTestAppId[] = "test-app-id";
+const char kTestAppId2[] = "test-app2-id";
// This is a helper struct to allow unit tests build an update response with the
// values they care about.
@@ -89,7 +90,8 @@
"<ping status=\"ok\"/>"
"<updatecheck status=\"noupdate\"/></app>" +
(multi_app_no_update
- ? "<app><updatecheck status=\"noupdate\"/></app>"
+ ? "<app appid=\"" + app_id2 +
+ "\"><updatecheck status=\"noupdate\"/></app>"
: "") +
"</response>";
}
@@ -142,9 +144,10 @@
(disable_p2p_for_sharing ? "DisableP2PForSharing=\"true\" " : "") +
"/></actions></manifest></updatecheck></app>" +
(multi_app
- ? "<app><updatecheck status=\"ok\"><urls><url codebase=\"" +
- codebase2 +
- "\"/></urls><manifest><packages>"
+ ? "<app appid=\"" + app_id2 +
+ "\"><updatecheck status=\"ok\"><urls><url codebase=\"" +
+ codebase2 + "\"/></urls><manifest version=\"" + version2 +
+ "\"><packages>"
"<package name=\"package3\" size=\"333\" "
"hash_sha256=\"hash3\"/></packages>"
"<actions><action event=\"postinstall\" " +
@@ -166,7 +169,9 @@
}
string app_id = kTestAppId;
+ string app_id2 = kTestAppId2;
string version = "1.2.3.4";
+ string version2 = "2.3.4.5";
string more_info_url = "http://more/info";
string prompt = "true";
string codebase = "http://code/base/";
@@ -549,6 +554,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.more_info_url, response.more_info_url);
@@ -557,7 +563,7 @@
EXPECT_EQ(true, response.packages[0].is_delta);
EXPECT_EQ(fake_update_response_.prompt == "true", response.prompt);
EXPECT_EQ(fake_update_response_.deadline, response.deadline);
- // Omaha cohort attribets are not set in the response, so they should not be
+ // Omaha cohort attributes are not set in the response, so they should not be
// persisted.
EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohort));
EXPECT_FALSE(fake_prefs_.Exists(kPrefsOmahaCohortHint));
@@ -624,6 +630,40 @@
EXPECT_EQ(false, response.packages[1].is_delta);
}
+TEST_F(OmahaRequestActionTest, MultiAppAndSystemUpdateTest) {
+ OmahaResponse response;
+ fake_update_response_.multi_app = true;
+ // trigger the lining up of the app and system versions
+ request_params_.set_system_app_id(fake_update_response_.app_id2);
+
+ ASSERT_TRUE(TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kUpdateAvailable,
+ metrics::CheckReaction::kUpdating,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_TRUE(response.update_exists);
+ EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ(fake_update_response_.version2, response.system_version);
+ EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
+ response.packages[0].payload_urls[0]);
+ EXPECT_EQ(fake_update_response_.codebase2 + "package3",
+ response.packages[1].payload_urls[0]);
+ EXPECT_EQ(fake_update_response_.hash, response.packages[0].hash);
+ EXPECT_EQ(fake_update_response_.size, response.packages[0].size);
+ EXPECT_EQ(11u, response.packages[0].metadata_size);
+ EXPECT_EQ(true, response.packages[0].is_delta);
+ ASSERT_EQ(2u, response.packages.size());
+ EXPECT_EQ(string("hash3"), response.packages[1].hash);
+ EXPECT_EQ(333u, response.packages[1].size);
+ EXPECT_EQ(33u, response.packages[1].metadata_size);
+ EXPECT_EQ(false, response.packages[1].is_delta);
+}
+
TEST_F(OmahaRequestActionTest, MultiAppPartialUpdateTest) {
OmahaResponse response;
fake_update_response_.multi_app = true;
@@ -640,6 +680,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.hash, response.packages[0].hash);
@@ -668,6 +709,7 @@
nullptr));
EXPECT_TRUE(response.update_exists);
EXPECT_EQ(fake_update_response_.version, response.version);
+ EXPECT_EQ("", response.system_version);
EXPECT_EQ(fake_update_response_.GetPayloadUrl(),
response.packages[0].payload_urls[0]);
EXPECT_EQ(fake_update_response_.codebase + "package2",
@@ -1263,7 +1305,10 @@
string input_response =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response protocol=\"3.0\">"
"<daystart elapsed_seconds=\"100\"/>"
- "<app appid=\"xyz\" status=\"ok\">"
+ // the appid needs to match that in the request params
+ "<app appid=\"" +
+ fake_update_response_.app_id +
+ "\" status=\"ok\">"
"<updatecheck status=\"ok\">"
"<urls><url codebase=\"http://missing/field/test/\"/></urls>"
"<manifest version=\"10.2.3.4\">"
diff --git a/omaha_request_params.h b/omaha_request_params.h
index f8e9438..ba7f2c3 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -111,6 +111,9 @@
return image_props_.canary_product_id;
}
inline std::string system_app_id() const { return image_props_.system_id; }
+ inline void set_system_app_id(const std::string& system_app_id) {
+ image_props_.system_id = system_app_id;
+ }
inline void set_app_id(const std::string& app_id) {
image_props_.product_id = app_id;
image_props_.canary_product_id = app_id;
diff --git a/omaha_response.h b/omaha_response.h
index c702068..b973eb5 100644
--- a/omaha_response.h
+++ b/omaha_response.h
@@ -37,6 +37,7 @@
// These are only valid if update_exists is true:
std::string version;
+ std::string system_version;
struct Package {
// The ordered list of URLs in the Omaha response. Each item is a complete
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 189fe6b..75050e7 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -71,6 +71,7 @@
install_plan_.download_url = current_url;
install_plan_.version = response.version;
+ install_plan_.system_version = response.system_version;
OmahaRequestParams* const params = system_state_->request_params();
PayloadStateInterface* const payload_state = system_state_->payload_state();
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 75cd819..935ae5d 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -456,4 +456,28 @@
EXPECT_TRUE(install_plan.hash_checks_mandatory);
}
+TEST_F(OmahaResponseHandlerActionTest, SystemVersionTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.version = "a.b.c.d";
+ in.system_version = "b.c.d.e";
+ in.packages.push_back({.payload_urls = {"http://package/1"},
+ .size = 1,
+ .hash = kPayloadHashHex});
+ in.packages.push_back({.payload_urls = {"http://package/2"},
+ .size = 2,
+ .hash = kPayloadHashHex});
+ in.more_info_url = "http://more/info";
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "", &install_plan));
+ EXPECT_EQ(in.packages[0].payload_urls[0], install_plan.download_url);
+ EXPECT_EQ(2u, install_plan.payloads.size());
+ EXPECT_EQ(in.packages[0].size, install_plan.payloads[0].size);
+ EXPECT_EQ(in.packages[1].size, install_plan.payloads[1].size);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[0].hash);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[1].hash);
+ EXPECT_EQ(in.version, install_plan.version);
+ EXPECT_EQ(in.system_version, install_plan.system_version);
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index d5d745b..b0dff31 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -74,7 +74,14 @@
InstallPayloadTypeToString(payload.type).c_str());
}
+ string version_str = base::StringPrintf(", version: %s", version.c_str());
+ if (!system_version.empty()) {
+ version_str +=
+ base::StringPrintf(", system_version: %s", system_version.c_str());
+ }
+
LOG(INFO) << "InstallPlan: " << (is_resume ? "resume" : "new_update")
+ << version_str
<< ", source_slot: " << BootControlInterface::SlotName(source_slot)
<< ", target_slot: " << BootControlInterface::SlotName(target_slot)
<< ", url: " << download_url << payloads_str << partitions_str
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 6dd5a73..551f8c2 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -54,6 +54,8 @@
bool is_resume{false};
std::string download_url; // url to download from
std::string version; // version we are installing.
+ // system version, if present and separate from version
+ std::string system_version;
struct Payload {
uint64_t size = 0; // size of the payload