[Offline pages] Implementation of RQStore.UpdateRequests with StoreUpdateResult

components/offline_pages/background/request_queue_store_unittest.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_queue_store.h"
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ skipping 39 matching lines @@
 
   void PumpLoop();
   void ClearResults();
 
   // Callback used for get requests.
   void GetRequestsDone(bool result,
                        std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for add/update request.
   void AddOrUpdateDone(UpdateStatus result);
   void AddRequestDone(ItemActionStatus status);
+  void UpdateRequestDone(std::unique_ptr<UpdateRequestsResult> result);
   void UpdateMultipleRequestsDone(
       const RequestQueue::UpdateMultipleRequestResults& results,
       std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for remove requests.
   void RemoveDone(const RequestQueue::UpdateMultipleRequestResults& results,
                   std::vector<std::unique_ptr<SavePageRequest>> requests);
   // Callback used for reset.
   void ResetDone(bool result);
 
   LastResult last_result() const { return last_result_; }
   UpdateStatus last_update_status() const { return last_update_status_; }
   const RequestQueue::UpdateMultipleRequestResults&
   last_multiple_update_results() const {
     return last_multiple_update_results_;
   }
   const RequestQueue::UpdateMultipleRequestResults& last_remove_results()
       const {
     return last_remove_results_;
   }
   const std::vector<std::unique_ptr<SavePageRequest>>& last_requests() const {
     return last_requests_;
   }
   ItemActionStatus last_add_status() const { return last_add_status_; }
 
+  UpdateRequestsResult* last_update_result() const {
+    return last_update_result_.get();
+  }
+
  protected:
   base::ScopedTempDir temp_directory_;
 
  private:
   LastResult last_result_;
   UpdateStatus last_update_status_;
   ItemActionStatus last_add_status_;
+  std::unique_ptr<UpdateRequestsResult> last_update_result_;
   RequestQueue::UpdateMultipleRequestResults last_multiple_update_results_;
   RequestQueue::UpdateMultipleRequestResults last_remove_results_;
   std::vector<std::unique_ptr<SavePageRequest>> last_requests_;
 
   scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::ThreadTaskRunnerHandle task_runner_handle_;
 };
 
 RequestQueueStoreTestBase::RequestQueueStoreTestBase()
     : last_result_(LastResult::kNone),
@@ skipping 12 matching lines @@
 void RequestQueueStoreTestBase::PumpLoop() {
   task_runner_->RunUntilIdle();
 }
 
 void RequestQueueStoreTestBase::ClearResults() {
   last_result_ = LastResult::kNone;
   last_update_status_ = UpdateStatus::FAILED;
   last_add_status_ = ItemActionStatus::NOT_FOUND;
   last_remove_results_.clear();
   last_requests_.clear();
+  last_update_result_.reset(nullptr);
 }
 
 void RequestQueueStoreTestBase::GetRequestsDone(
     bool result,
     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_result_ = result ? LastResult::kTrue : LastResult::kFalse;
   last_requests_ = std::move(requests);
 }
 
 void RequestQueueStoreTestBase::AddOrUpdateDone(UpdateStatus status) {
   last_update_status_ = status;
 }
 
 void RequestQueueStoreTestBase::AddRequestDone(ItemActionStatus status) {
   last_add_status_ = status;
 }
 
+void RequestQueueStoreTestBase::UpdateRequestDone(
+    std::unique_ptr<UpdateRequestsResult> result) {
+  last_update_result_ = std::move(result);
+}
+
 void RequestQueueStoreTestBase::UpdateMultipleRequestsDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_multiple_update_results_ = results;
   last_requests_ = std::move(requests);

[Pete Williamson, 2016/09/22 00:12:50]

Let's remove UpdateMultipleRequestsDone - The idea was to have one function for
a single request, and one for multiple, and UpdateRequestDone now handles
multiples.

[fgorski, 2016/09/22 15:47:53]

Acknowledged. Separate patch.

 }
 
 void RequestQueueStoreTestBase::RemoveDone(
     const RequestQueue::UpdateMultipleRequestResults& results,
     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   last_remove_results_ = results;
   last_requests_ = std::move(requests);
 }
 
 void RequestQueueStoreTestBase::ResetDone(bool result) {
@@ skipping 97 matching lines @@
   // Check that there is still only one item in the store.
   this->ClearResults();
   store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
                                 base::Unretained(this)));
   ASSERT_EQ(LastResult::kNone, this->last_result());
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
 }
 
 TYPED_TEST(RequestQueueStoreTest, UpdateRequest) {

[Pete Williamson, 2016/09/22 00:12:50]

This test only tests a single request at a time, we should test updating more
than one at a time to make sure we fully exercise the code under test.

[fgorski, 2016/09/22 15:47:53]

Done. Updated to run both tests in one call to Update.

   std::unique_ptr<RequestQueueStore> store(this->BuildStore());
   base::Time creation_time = base::Time::Now();
   SavePageRequest original_request(
       kRequestId, kUrl, kClientId, creation_time, kUserRequested);
   store->AddRequest(original_request,
                     base::Bind(&RequestQueueStoreTestBase::AddRequestDone,
                                base::Unretained(this)));
   this->PumpLoop();
   this->ClearResults();
 
   base::Time new_creation_time =
       creation_time + base::TimeDelta::FromMinutes(1);
   base::Time activation_time = creation_time + base::TimeDelta::FromHours(6);
   SavePageRequest updated_request(kRequestId, kUrl, kClientId,
                                   new_creation_time, activation_time,
                                   kUserRequested);
-  store->AddOrUpdateRequest(
-      updated_request, base::Bind(&RequestQueueStoreTestBase::AddOrUpdateDone,
-                                  base::Unretained(this)));
-  ASSERT_EQ(UpdateStatus::FAILED, this->last_update_status());
+  std::vector<SavePageRequest> requests_to_update{updated_request};
+  store->UpdateRequests(
+      requests_to_update,
+      base::Bind(&RequestQueueStoreTestBase::UpdateRequestDone,
+                 base::Unretained(this)));
+  ASSERT_FALSE(this->last_update_result());
   this->PumpLoop();
-  ASSERT_EQ(UpdateStatus::UPDATED, this->last_update_status());
+  ASSERT_TRUE(this->last_update_result());
+  EXPECT_EQ(1UL, this->last_update_result()->item_statuses.size());
+  EXPECT_EQ(kRequestId,
+            this->last_update_result()->item_statuses.begin()->first);
+  EXPECT_EQ(ItemActionStatus::SUCCESS,
+            this->last_update_result()->item_statuses.begin()->second);
+  EXPECT_EQ(1UL, this->last_update_result()->updated_items.size());
+  EXPECT_EQ(updated_request,
+            *(this->last_update_result()->updated_items.begin()));
 
   // Verifying get reqeust results after a request was updated.
   this->ClearResults();
   store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
                                 base::Unretained(this)));
   ASSERT_EQ(LastResult::kNone, this->last_result());
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
   ASSERT_EQ(updated_request, *(this->last_requests()[0].get()));
+
+  this->ClearResults();

[Pete Williamson, 2016/09/22 00:12:50]

This section is a nice addition to the test, thanks!

[fgorski, 2016/09/22 15:47:53]

Acknowledged.

+  // Try to update a non-existing request.
+  SavePageRequest updated_request2(kRequestId2, kUrl, kClientId,
+                                   new_creation_time, activation_time,
+                                   kUserRequested);
+  requests_to_update.clear();
+  requests_to_update.push_back(updated_request2);
+  store->UpdateRequests(
+      requests_to_update,
+      base::Bind(&RequestQueueStoreTestBase::UpdateRequestDone,
+                 base::Unretained(this)));
+  this->PumpLoop();
+  ASSERT_TRUE(this->last_update_result());
+  EXPECT_EQ(1UL, this->last_update_result()->item_statuses.size());
+  EXPECT_EQ(kRequestId2,
+            this->last_update_result()->item_statuses.begin()->first);
+  EXPECT_EQ(ItemActionStatus::NOT_FOUND,
+            this->last_update_result()->item_statuses.begin()->second);
+  EXPECT_EQ(0UL, this->last_update_result()->updated_items.size());
 }
 
 TYPED_TEST(RequestQueueStoreTest, RemoveRequests) {
   std::unique_ptr<RequestQueueStore> store(this->BuildStore());
   base::Time creation_time = base::Time::Now();
   SavePageRequest request1(kRequestId, kUrl, kClientId, creation_time,
                            kUserRequested);
   store->AddRequest(request1,
                     base::Bind(&RequestQueueStoreTestBase::AddRequestDone,
                                base::Unretained(this)));
@@ skipping 160 matching lines @@
   store->GetRequests(base::Bind(&RequestQueueStoreTestBase::GetRequestsDone,
                                 base::Unretained(this)));
   ASSERT_EQ(LastResult::kNone, this->last_result());
   this->PumpLoop();
   ASSERT_EQ(LastResult::kTrue, this->last_result());
   ASSERT_EQ(1ul, this->last_requests().size());
   ASSERT_TRUE(original_request == *(this->last_requests().at(0).get()));
 }
 
 }  // offline_pages