[ledger] Fix LedgerManagerTest.PageIsClosedAndSynced*.
In the FakeLedgerStorage used in the test, DelayIsSyncedCallback
expected the PageStorage to be open. However, in most of the tests
PageIsClosedAnd* the page was already closed when the flag was set.
After this CL FakeLedgerStorage doesn't require the PageStorage to
be open to correctly set the flag.
Test: LedgerManagerTest.PageIsClosedAndSyncedCheckPageOpened
.PageIsClosedAndSyncedConcurrentCalls
Bug: LE-650 #done
Change-Id: I3d98437c9688ff0661e3ff37e194904cc2dade64
diff --git a/bin/ledger/app/ledger_manager_unittest.cc b/bin/ledger/app/ledger_manager_unittest.cc
index 977edac..ff80021 100644
--- a/bin/ledger/app/ledger_manager_unittest.cc
+++ b/bin/ledger/app/ledger_manager_unittest.cc
@@ -35,16 +35,31 @@
namespace ledger {
namespace {
+class DelayingCallbacksManager {
+ public:
+ DelayingCallbacksManager() {}
+ virtual ~DelayingCallbacksManager() {}
+
+ // Returns true if the PageStorage of the page with the given id should delay
+ // calling the callback of |IsSynced|.
+ virtual bool ShouldDelayIsSyncedCallback(storage::PageIdView page_id) = 0;
+
+ private:
+ FXL_DISALLOW_COPY_AND_ASSIGN(DelayingCallbacksManager);
+};
+
class DelayIsSyncedCallbackFakePageStorage
: public storage::fake::FakePageStorage {
public:
- explicit DelayIsSyncedCallbackFakePageStorage(Environment* environment,
- storage::PageId id)
- : storage::fake::FakePageStorage(environment, id) {}
+ explicit DelayIsSyncedCallbackFakePageStorage(
+ Environment* environment,
+ DelayingCallbacksManager* delaying_callbacks_manager, storage::PageId id)
+ : storage::fake::FakePageStorage(environment, id),
+ delaying_callbacks_manager_(delaying_callbacks_manager) {}
~DelayIsSyncedCallbackFakePageStorage() override {}
void IsSynced(fit::function<void(storage::Status, bool)> callback) override {
- if (!delay_callback_) {
+ if (!delaying_callbacks_manager_->ShouldDelayIsSyncedCallback(page_id_)) {
storage::fake::FakePageStorage::IsSynced(std::move(callback));
return;
}
@@ -57,20 +72,19 @@
bool IsOnline() override { return false; }
- void DelayIsSyncedCallback(bool delay_callback) {
- delay_callback_ = delay_callback;
- }
-
void CallIsSyncedCallback() {
storage::fake::FakePageStorage::IsSynced(std::move(is_synced_callback_));
}
private:
fit::function<void(storage::Status, bool)> is_synced_callback_;
- bool delay_callback_ = false;
+ DelayingCallbacksManager* delaying_callbacks_manager_;
+
+ FXL_DISALLOW_COPY_AND_ASSIGN(DelayIsSyncedCallbackFakePageStorage);
};
-class FakeLedgerStorage : public storage::LedgerStorage {
+class FakeLedgerStorage : public storage::LedgerStorage,
+ public DelayingCallbacksManager {
public:
explicit FakeLedgerStorage(Environment* environment)
: environment_(environment) {}
@@ -98,13 +112,10 @@
} else {
auto fake_page_storage =
std::make_unique<DelayIsSyncedCallbackFakePageStorage>(
- environment_, page_id);
+ environment_, this, page_id);
// If the page was opened before, restore the previous sync state.
fake_page_storage->set_synced(synced_pages_.find(page_id) !=
synced_pages_.end());
- fake_page_storage->DelayIsSyncedCallback(
- pages_with_delayed_callback.find(page_id) !=
- pages_with_delayed_callback.end());
page_storages_[std::move(page_id)] = fake_page_storage.get();
callback(storage::Status::OK, std::move(fake_page_storage));
}
@@ -127,14 +138,14 @@
if (delay_callback) {
pages_with_delayed_callback.insert(page_id.ToString());
} else {
- auto it = pages_with_delayed_callback.find(page_id.ToString());
- if (it != pages_with_delayed_callback.end()) {
- pages_with_delayed_callback.erase(it);
- }
+ pages_with_delayed_callback.erase(page_id.ToString());
}
- auto it = page_storages_.find(page_id.ToString());
- FXL_CHECK(it != page_storages_.end());
- it->second->DelayIsSyncedCallback(delay_callback);
+ }
+
+ // DelayingCallbacksManager:
+ bool ShouldDelayIsSyncedCallback(storage::PageIdView page_id) override {
+ return pages_with_delayed_callback.find(page_id.ToString()) !=
+ pages_with_delayed_callback.end();
}
void CallIsSyncedCallback(storage::PageIdView page_id) {
diff --git a/bin/ledger/storage/fake/fake_page_storage.cc b/bin/ledger/storage/fake/fake_page_storage.cc
index 56cd422..e06372a 100644
--- a/bin/ledger/storage/fake/fake_page_storage.cc
+++ b/bin/ledger/storage/fake/fake_page_storage.cc
@@ -27,8 +27,8 @@
FakePageStorage::FakePageStorage(ledger::Environment* environment,
PageId page_id)
- : environment_(environment),
- page_id_(std::move(page_id)),
+ : page_id_(std::move(page_id)),
+ environment_(environment),
encryption_service_(environment_->dispatcher()) {}
FakePageStorage::~FakePageStorage() {}
diff --git a/bin/ledger/storage/fake/fake_page_storage.h b/bin/ledger/storage/fake/fake_page_storage.h
index 2ca8f8c..bc6f3a1 100644
--- a/bin/ledger/storage/fake/fake_page_storage.h
+++ b/bin/ledger/storage/fake/fake_page_storage.h
@@ -93,6 +93,8 @@
// this method if they need valid object digests instead.
virtual ObjectDigest FakeDigest(fxl::StringView value) const;
+ PageId page_id_;
+
private:
void SendNextObject();
@@ -106,7 +108,6 @@
std::set<CommitId> heads_;
std::set<CommitWatcher*> watchers_;
std::vector<fit::closure> object_requests_;
- PageId page_id_;
encryption::FakeEncryptionService encryption_service_;
FXL_DISALLOW_COPY_AND_ASSIGN(FakePageStorage);
