Support for processing multiple URLs in update_engine.
Main changes:
1. Added a new PayloadState class which encapsulates all the persisted
state we use for multiple URLs, back-off (TBD), etc.
2. Added support for handling multiple URLs stored in the OmahaResponse in
OmahaRequestAction and OmahaResponseHandlerAction code.
3. Added support for picking the right URL in OmahaResponseHandlerAction
and putting it in the install_plan. This way, the rest of the code that
uses the install_plan is oblivious to the presence of multiple URLs :-)
4. Added support for advancing to next URL when an update fails. The full
error classification is a new work item (chromium-os:37206). Right now,
it's a basic round-robin on every error.
5. Updated the conditions for determining when hash checks are mandatory.
Previously since there was only one URL, if it was HTTPS, the checks were
waived. Now, even if there's one HTTP URL, we make hash checks mandatory
even if other HTTPS URLs are present.
6. Added new unit tests for PayloadState and the new logic added to other
places.
Noisy changes:
1. Instead of passing PrefsInterface to OmahaRequestAction and
OmahaResponseHandlerAction, we're now passing SystemState which will now
contain PrefsInterface and the newly added PayloadState object that these
actions need to do their work.
2. Renamed a bunch of setters/getters to set_x() and x() instead of SetX()
and GetX() methods - this was pending from Gilad's old CR. As I'm
adding new methods in the correct style, I went ahead and fixed it to
avoid the confusing styles.
3. Updated all existing unit tests to reflect these changes.
BUG=chromium-os:36807
TEST=All Single/Multiple URL scenarios work fine on my ZGB as expected.
TEST=Old and new unit tests run fine.
Change-Id: Id31f9ccb220471f3ec3a475f624dc03c16119144
Reviewed-on: https://gerrit.chromium.org/gerrit/39638
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 937a70f..23e60fe 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -12,6 +12,7 @@
#include "update_engine/filesystem_copier_action.h"
#include "update_engine/mock_dbus_interface.h"
#include "update_engine/mock_http_fetcher.h"
+#include "update_engine/mock_payload_state.h"
#include "update_engine/mock_system_state.h"
#include "update_engine/postinstall_runner_action.h"
#include "update_engine/prefs.h"
@@ -37,22 +38,22 @@
// methods.
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
- explicit UpdateAttempterUnderTest(MockDbusGlib* dbus)
- : UpdateAttempter(NULL, dbus, NULL, NULL) {}
+ explicit UpdateAttempterUnderTest(MockSystemState* mock_system_state,
+ MockDbusGlib* dbus)
+ : UpdateAttempter(mock_system_state, dbus, NULL) {}
};
class UpdateAttempterTest : public ::testing::Test {
protected:
UpdateAttempterTest()
- : attempter_(&dbus_),
+ : attempter_(&mock_system_state_, &dbus_),
mock_connection_manager(&mock_system_state_),
loop_(NULL) {
- mock_system_state_.SetConnectionManager(&mock_connection_manager);
+ mock_system_state_.set_connection_manager(&mock_connection_manager);
}
virtual void SetUp() {
EXPECT_EQ(NULL, attempter_.dbus_service_);
- EXPECT_EQ(NULL, attempter_.prefs_);
- EXPECT_EQ(NULL, attempter_.system_state_);
+ EXPECT_TRUE(attempter_.system_state_ != NULL);
EXPECT_EQ(NULL, attempter_.update_check_scheduler_);
EXPECT_EQ(0, attempter_.http_response_code_);
EXPECT_EQ(utils::kCpuSharesNormal, attempter_.shares_);
@@ -65,8 +66,7 @@
EXPECT_EQ(0, attempter_.new_payload_size_);
processor_ = new ActionProcessorMock();
attempter_.processor_.reset(processor_); // Transfers ownership.
- attempter_.prefs_ = &prefs_;
- attempter_.system_state_ = &mock_system_state_;
+ prefs_ = mock_system_state_.mock_prefs();
}
void QuitMainLoop();
@@ -102,11 +102,11 @@
static gboolean StaticNoScatteringDoneDuringManualUpdateTestStart(
gpointer data);
+ MockSystemState mock_system_state_;
MockDbusGlib dbus_;
UpdateAttempterUnderTest attempter_;
ActionProcessorMock* processor_;
- NiceMock<PrefsMock> prefs_;
- MockSystemState mock_system_state_;
+ NiceMock<PrefsMock>* prefs_; // shortcut to mock_system_state_->mock_prefs()
MockConnectionManager mock_connection_manager;
GMainLoop* loop_;
};
@@ -114,8 +114,8 @@
TEST_F(UpdateAttempterTest, ActionCompletedDownloadTest) {
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(503); // Sets the HTTP response code.
- DownloadAction action(&prefs_, NULL, fetcher.release());
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
+ DownloadAction action(prefs_, NULL, fetcher.release());
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
EXPECT_EQ(503, attempter_.http_response_code());
EXPECT_EQ(UPDATE_STATUS_FINALIZING, attempter_.status());
@@ -126,7 +126,7 @@
ActionMock action;
EXPECT_CALL(action, Type()).WillRepeatedly(Return("ActionMock"));
attempter_.status_ = UPDATE_STATUS_DOWNLOADING;
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false));
attempter_.ActionCompleted(NULL, &action, kActionCodeError);
ASSERT_TRUE(attempter_.error_event_.get() != NULL);
@@ -136,7 +136,8 @@
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(500); // Sets the HTTP response code.
OmahaRequestParams params;
- OmahaRequestAction action(&prefs_, ¶ms, NULL, fetcher.release(), false);
+ OmahaRequestAction action(&mock_system_state_, ¶ms, NULL,
+ fetcher.release(), false);
ObjectCollectorAction<OmahaResponse> collector_action;
BondActions(&action, &collector_action);
OmahaResponse response;
@@ -144,7 +145,7 @@
action.SetOutputObject(response);
UpdateCheckScheduler scheduler(&attempter_, NULL, &mock_system_state_);
attempter_.set_update_check_scheduler(&scheduler);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
EXPECT_EQ(500, attempter_.http_response_code());
EXPECT_EQ(UPDATE_STATUS_IDLE, attempter_.status());
@@ -156,8 +157,7 @@
extern const char* kUpdateCompletedMarker;
const FilePath kMarker(kUpdateCompletedMarker);
EXPECT_EQ(0, file_util::WriteFile(kMarker, "", 0));
- MockDbusGlib dbus;
- UpdateAttempterUnderTest attempter(&dbus);
+ UpdateAttempterUnderTest attempter(&mock_system_state_, &dbus_);
EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter.status());
EXPECT_TRUE(file_util::Delete(kMarker, false));
}
@@ -169,10 +169,12 @@
GetErrorCodeForAction(NULL, kActionCodeSuccess));
OmahaRequestParams params;
- OmahaRequestAction omaha_request_action(NULL, ¶ms, NULL, NULL, false);
+ MockSystemState mock_system_state;
+ OmahaRequestAction omaha_request_action(&mock_system_state, ¶ms, NULL,
+ NULL, false);
EXPECT_EQ(kActionCodeOmahaRequestError,
GetErrorCodeForAction(&omaha_request_action, kActionCodeError));
- OmahaResponseHandlerAction omaha_response_handler_action(&prefs_);
+ OmahaResponseHandlerAction omaha_response_handler_action(&mock_system_state_);
EXPECT_EQ(kActionCodeOmahaResponseHandlerError,
GetErrorCodeForAction(&omaha_response_handler_action,
kActionCodeError));
@@ -191,40 +193,40 @@
TEST_F(UpdateAttempterTest, DisableDeltaUpdateIfNeededTest) {
attempter_.omaha_request_params_.delta_okay = true;
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_TRUE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures - 1),
Return(true)));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_TRUE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures),
Return(true)));
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_FALSE(attempter_.omaha_request_params_.delta_okay);
- EXPECT_CALL(prefs_, GetInt64(_, _)).Times(0);
+ EXPECT_CALL(*prefs_, GetInt64(_, _)).Times(0);
attempter_.DisableDeltaUpdateIfNeeded();
EXPECT_FALSE(attempter_.omaha_request_params_.delta_okay);
}
TEST_F(UpdateAttempterTest, MarkDeltaUpdateFailureTest) {
- EXPECT_CALL(prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
+ EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _))
.WillOnce(Return(false))
.WillOnce(DoAll(SetArgumentPointee<1>(-1), Return(true)))
.WillOnce(DoAll(SetArgumentPointee<1>(1), Return(true)))
.WillOnce(DoAll(
SetArgumentPointee<1>(UpdateAttempter::kMaxDeltaUpdateFailures),
Return(true)));
- EXPECT_CALL(prefs_, SetInt64(Ne(kPrefsDeltaUpdateFailures), _))
+ EXPECT_CALL(*prefs_, SetInt64(Ne(kPrefsDeltaUpdateFailures), _))
.WillRepeatedly(Return(true));
- EXPECT_CALL(prefs_, SetInt64(kPrefsDeltaUpdateFailures, 1)).Times(2);
- EXPECT_CALL(prefs_, SetInt64(kPrefsDeltaUpdateFailures, 2)).Times(1);
- EXPECT_CALL(prefs_, SetInt64(kPrefsDeltaUpdateFailures,
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 1)).Times(2);
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures, 2)).Times(1);
+ EXPECT_CALL(*prefs_, SetInt64(kPrefsDeltaUpdateFailures,
UpdateAttempter::kMaxDeltaUpdateFailures + 1))
.Times(1);
for (int i = 0; i < 4; i ++)
@@ -234,7 +236,14 @@
TEST_F(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest) {
EXPECT_CALL(*processor_, EnqueueAction(_)).Times(0);
EXPECT_CALL(*processor_, StartProcessing()).Times(0);
+ EXPECT_CALL(*mock_system_state_.mock_payload_state(), UpdateFailed(_))
+ .Times(0);
+ OmahaResponse response;
+ response.payload_urls.push_back("http://url");
+ response.payload_urls.push_back("https://url");
+ mock_system_state_.mock_payload_state()->SetResponse(response);
attempter_.ScheduleErrorEventAction();
+ EXPECT_EQ(0, mock_system_state_.mock_payload_state()->GetUrlIndex());
}
TEST_F(UpdateAttempterTest, ScheduleErrorEventActionTest) {
@@ -243,10 +252,16 @@
OmahaRequestAction::StaticType())))
.Times(1);
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ OmahaResponse response;
+ response.payload_urls.push_back("http://url");
+ response.payload_urls.push_back("https://url");
+ mock_system_state_.mock_payload_state()->SetResponse(response);
+ ActionExitCode err = kActionCodeError;
attempter_.error_event_.reset(new OmahaEvent(OmahaEvent::kTypeUpdateComplete,
OmahaEvent::kResultError,
- kActionCodeError));
+ err));
attempter_.ScheduleErrorEventAction();
+ EXPECT_EQ(1, mock_system_state_.mock_payload_state()->GetUrlIndex());
EXPECT_EQ(UPDATE_STATUS_REPORTING_ERROR_EVENT, attempter_.status());
}
@@ -427,7 +442,7 @@
TEST_F(UpdateAttempterTest, CreatePendingErrorEventResumedTest) {
OmahaResponseHandlerAction *response_action =
- new OmahaResponseHandlerAction(&prefs_);
+ new OmahaResponseHandlerAction(&mock_system_state_);
response_action->install_plan_.is_resume = true;
attempter_.response_handler_action_.reset(response_action);
ActionMock action;
@@ -545,7 +560,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
@@ -592,7 +607,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))
@@ -659,7 +674,7 @@
attempter_.policy_provider_.reset(new policy::PolicyProvider(device_policy));
EXPECT_CALL(*device_policy, LoadPolicy()).WillRepeatedly(Return(true));
- EXPECT_CALL(mock_system_state_, GetDevicePolicy()).WillRepeatedly(
+ EXPECT_CALL(mock_system_state_, device_policy()).WillRepeatedly(
Return(device_policy));
EXPECT_CALL(*device_policy, GetScatterFactorInSeconds(_))