Add unit test for fallback route of manifest cache

Test: Run atest
Change-Id: Icc6a2c809c571c3ad8e16a863c37afd8d4042ed6

7 files changed, 130 insertions, 18 deletions

diff --git a/Android.bp b/Android.bp
index ecf3585b..4e3e2484 100644
--- a/Android.bp
+++ b/Android.bp
@@ -671,6 +671,7 @@ cc_test {
         "payload_consumer/certificate_parser_android_unittest.cc",
         "payload_consumer/delta_performer_integration_test.cc",
         "payload_consumer/delta_performer_unittest.cc",
+        "payload_consumer/download_action_android_unittest.cc",
         "payload_consumer/extent_reader_unittest.cc",
         "payload_consumer/extent_writer_unittest.cc",
         "payload_consumer/fake_file_descriptor.cc",
diff --git a/common/action_pipe.h b/common/action_pipe.h
index 0c98ee13..4c568126 100644
--- a/common/action_pipe.h
+++ b/common/action_pipe.h
@@ -79,6 +79,8 @@ class ActionPipe {
 
  private:
   ObjectType contents_;
+  // Give unit test access
+  friend class DownloadActionTest;
 
   // The ctor is private. This is because this class should construct itself
   // via the static Bond() method.
diff --git a/common/action_processor.h b/common/action_processor.h
index 735a1063..ad98cc9c 100644
--- a/common/action_processor.h
+++ b/common/action_processor.h
@@ -89,7 +89,7 @@ class ActionProcessor {
   // But this call deletes the action if there no other object has a reference
   // to it, so in that case, the caller should not try to access any of its
   // member variables after this call.
-  void ActionComplete(AbstractAction* actionptr, ErrorCode code);
+  virtual void ActionComplete(AbstractAction* actionptr, ErrorCode code);
 
  private:
   FRIEND_TEST(ActionProcessorTest, ChainActionsTest);
diff --git a/common/mock_action_processor.h b/common/mock_action_processor.h
index 4c62109b..97857764 100644
--- a/common/mock_action_processor.h
+++ b/common/mock_action_processor.h
@@ -32,6 +32,8 @@ class MockActionProcessor : public ActionProcessor {
   MOCK_METHOD0(StartProcessing, void());
   MOCK_METHOD1(EnqueueAction, void(AbstractAction* action));
 
+  MOCK_METHOD2(ActionComplete, void(AbstractAction*, ErrorCode));
+
   // This is a legacy workaround described in:
   // https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#legacy-workarounds-for-move-only-types-legacymoveonly
   void EnqueueAction(std::unique_ptr<AbstractAction> action) override {
diff --git a/common/mock_http_fetcher.cc b/common/mock_http_fetcher.cc
index 10e3b9ef..1b3cd7d7 100644
--- a/common/mock_http_fetcher.cc
+++ b/common/mock_http_fetcher.cc
@@ -22,6 +22,7 @@
 #include <base/logging.h>
 #include <base/strings/string_util.h>
 #include <base/time/time.h>
+#include <brillo/message_loops/message_loop.h>
 #include <gtest/gtest.h>
 
 // This is a mock implementation of HttpFetcher which is useful for testing.
@@ -43,12 +44,12 @@ void MockHttpFetcher::BeginTransfer(const std::string& url) {
     SignalTransferComplete();
     return;
   }
-  if (sent_size_ < data_.size())
+  if (sent_offset_ < data_.size())
     SendData(true);
 }
 
 void MockHttpFetcher::SendData(bool skip_delivery) {
-  if (fail_transfer_ || sent_size_ == data_.size()) {
+  if (fail_transfer_ || sent_offset_ == data_.size()) {
     SignalTransferComplete();
     return;
   }
@@ -60,19 +61,22 @@ void MockHttpFetcher::SendData(bool skip_delivery) {
 
   // Setup timeout callback even if the transfer is about to be completed in
   // order to get a call to |TransferComplete|.
-  if (timeout_id_ == MessageLoop::kTaskIdNull) {
+  if (timeout_id_ == MessageLoop::kTaskIdNull && delay_) {
+    CHECK(MessageLoop::current());
     timeout_id_ = MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&MockHttpFetcher::TimeoutCallback, base::Unretained(this)),
         base::TimeDelta::FromMilliseconds(10));
   }
 
-  if (!skip_delivery) {
+  if (!skip_delivery || !delay_) {
     const size_t chunk_size =
-        min(kMockHttpFetcherChunkSize, data_.size() - sent_size_);
-    sent_size_ += chunk_size;
+        min(kMockHttpFetcherChunkSize, data_.size() - sent_offset_);
+    sent_offset_ += chunk_size;
+    bytes_sent_ += chunk_size;
     CHECK(delegate_);
-    delegate_->ReceivedBytes(this, &data_[sent_size_ - chunk_size], chunk_size);
+    delegate_->ReceivedBytes(
+        this, &data_[sent_offset_ - chunk_size], chunk_size);
   }
   // We may get terminated and deleted right after |ReceivedBytes| call, so we
   // should not access any class member variable after this call.
@@ -81,7 +85,7 @@ void MockHttpFetcher::SendData(bool skip_delivery) {
 void MockHttpFetcher::TimeoutCallback() {
   CHECK(!paused_);
   timeout_id_ = MessageLoop::kTaskIdNull;
-  CHECK_LE(sent_size_, data_.size());
+  CHECK_LE(sent_offset_, data_.size());
   // Same here, we should not access any member variable after this call.
   SendData(false);
 }
@@ -90,10 +94,15 @@ void MockHttpFetcher::TimeoutCallback() {
 // The transfer cannot be resumed.
 void MockHttpFetcher::TerminateTransfer() {
   LOG(INFO) << "Terminating transfer.";
-  // Kill any timeout, it is ok to call with kTaskIdNull.
-  MessageLoop::current()->CancelTask(timeout_id_);
-  timeout_id_ = MessageLoop::kTaskIdNull;
-  delegate_->TransferTerminated(this);
+  // During testing, MessageLoop may or may not be available.
+  // So don't call CancelTask() unless necessary.
+  if (timeout_id_ != MessageLoop::kTaskIdNull) {
+    MessageLoop::current()->CancelTask(timeout_id_);
+    timeout_id_ = MessageLoop::kTaskIdNull;
+  }
+  if (delegate_) {
+    delegate_->TransferTerminated(this);
+  }
 }
 
 void MockHttpFetcher::SetHeader(const std::string& header_name,
diff --git a/common/mock_http_fetcher.h b/common/mock_http_fetcher.h
index dec81b0c..b082bbd4 100644
--- a/common/mock_http_fetcher.h
+++ b/common/mock_http_fetcher.h
@@ -46,7 +46,7 @@ class MockHttpFetcher : public HttpFetcher {
                   size_t size,
                   ProxyResolver* proxy_resolver)
       : HttpFetcher(proxy_resolver),
-        sent_size_(0),
+        sent_offset_(0),
         timeout_id_(brillo::MessageLoop::kTaskIdNull),
         paused_(false),
         fail_transfer_(false),
@@ -64,7 +64,7 @@ class MockHttpFetcher : public HttpFetcher {
 
   // Ignores this.
   void SetOffset(off_t offset) override {
-    sent_size_ = offset;
+    sent_offset_ = offset;
     if (delegate_)
       delegate_->SeekToOffset(offset);
   }
@@ -77,7 +77,7 @@ class MockHttpFetcher : public HttpFetcher {
   void set_max_retry_count(int max_retry_count) override {}
 
   // No bytes were downloaded in the mock class.
-  size_t GetBytesDownloaded() override { return sent_size_; }
+  size_t GetBytesDownloaded() override { return bytes_sent_; }
 
   // Begins the transfer if it hasn't already begun.
   void BeginTransfer(const std::string& url) override;
@@ -113,6 +113,8 @@ class MockHttpFetcher : public HttpFetcher {
 
   const brillo::Blob& post_data() const { return post_data_; }
 
+  void set_delay(bool delay) { delay_ = delay; }
+
  private:
   // Sends data to the delegate and sets up a timeout callback if needed. There
   // must be a delegate. If |skip_delivery| is true, no bytes will be delivered,
@@ -129,8 +131,11 @@ class MockHttpFetcher : public HttpFetcher {
   // A full copy of the data we'll return to the delegate
   brillo::Blob data_;
 
-  // The number of bytes we've sent so far
-  size_t sent_size_;
+  // The current offset, marks the first byte that will be sent next
+  size_t sent_offset_;
+
+  // Total number of bytes transferred
+  size_t bytes_sent_;
 
   // The extra headers set.
   std::map<std::string, std::string> extra_headers_;
@@ -148,6 +153,9 @@ class MockHttpFetcher : public HttpFetcher {
   // Set to true if BeginTransfer should EXPECT fail.
   bool never_use_;
 
+  // Whether it should wait for 10ms before sending data to delegates
+  bool delay_{true};
+
   DISALLOW_COPY_AND_ASSIGN(MockHttpFetcher);
 };
 
diff --git a/payload_consumer/download_action_android_unittest.cc b/payload_consumer/download_action_android_unittest.cc
new file mode 100644
index 00000000..f78845f5
--- /dev/null
+++ b/payload_consumer/download_action_android_unittest.cc
@@ -0,0 +1,90 @@
+//
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "common/mock_action_processor.h"
+#include <gmock/gmock-actions.h>
+#include <gmock/gmock-function-mocker.h>
+#include <gmock/gmock-spec-builders.h>
+
+#include "payload_consumer/install_plan.h"
+#include "update_engine/common/action_pipe.h"
+#include "update_engine/common/boot_control_stub.h"
+#include "update_engine/common/constants.h"
+#include "update_engine/common/mock_http_fetcher.h"
+#include "update_engine/common/mock_prefs.h"
+#include "update_engine/common/test_utils.h"
+#include "update_engine/payload_consumer/download_action.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <memory>
+
+namespace chromeos_update_engine {
+using testing::_;
+using testing::DoAll;
+using testing::Return;
+using testing::SetArgPointee;
+
+class DownloadActionTest : public ::testing::Test {
+ public:
+  static constexpr int64_t METADATA_SIZE = 1024;
+  static constexpr int64_t SIGNATURE_SIZE = 256;
+  std::shared_ptr<ActionPipe<InstallPlan>> action_pipe{
+      new ActionPipe<InstallPlan>()};
+};
+
+TEST_F(DownloadActionTest, CacheManifestInvalid) {
+  std::string data(METADATA_SIZE + SIGNATURE_SIZE, '-');
+  MockPrefs prefs;
+  EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStatePayloadIndex, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(0L), Return(true)));
+  EXPECT_CALL(prefs, GetInt64(kPrefsManifestMetadataSize, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(METADATA_SIZE), Return(true)));
+  EXPECT_CALL(prefs, GetInt64(kPrefsManifestSignatureSize, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(SIGNATURE_SIZE), Return(true)));
+  EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextDataOffset, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(0L), Return(true)));
+  EXPECT_CALL(prefs, GetString(kPrefsManifestBytes, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(data), Return(true)));
+
+  BootControlStub boot_control;
+  MockHttpFetcher* http_fetcher =
+      new MockHttpFetcher(data.data(), data.size(), nullptr);
+  http_fetcher->set_delay(false);
+  InstallPlan install_plan;
+  auto& payload = install_plan.payloads.emplace_back();
+  install_plan.download_url = "http://fake_url.invalid";
+  payload.size = data.size();
+  payload.payload_urls.emplace_back("http://fake_url.invalid");
+  install_plan.is_resume = true;
+  action_pipe->set_contents(install_plan);
+
+  // takes ownership of passed in HttpFetcher
+  auto download_action =
+      std::make_unique<DownloadAction>(&prefs,
+                                       &boot_control,
+                                       nullptr,
+                                       nullptr,
+                                       http_fetcher,
+                                       false /* interactive */);
+  download_action->set_in_pipe(action_pipe);
+  MockActionProcessor mock_processor;
+  download_action->SetProcessor(&mock_processor);
+  download_action->PerformAction();
+  ASSERT_EQ(download_action->http_fetcher()->GetBytesDownloaded(), data.size());
+}
+
+}  // namespace chromeos_update_engine