[ledger] Allow creating a TreeNode from an Object
Also discard unused method TreeNode::GetChild, which removes the need to
keep a PageStorage reference in TreeNode.
Change-Id: Ibdc9ea1d4f7b04c0edd5b535ba9554c1f6ae8b62
Bug: LE-697
Tested: fx run-test ledger_tests -t ledger_unittests
diff --git a/src/ledger/bin/storage/impl/btree/tree_node.cc b/src/ledger/bin/storage/impl/btree/tree_node.cc
index 2201526..e6a4f9b 100644
--- a/src/ledger/bin/storage/impl/btree/tree_node.cc
+++ b/src/ledger/bin/storage/impl/btree/tree_node.cc
@@ -24,11 +24,10 @@
namespace storage {
namespace btree {
-TreeNode::TreeNode(PageStorage* page_storage, ObjectIdentifier identifier,
- uint8_t level, std::vector<Entry> entries,
+TreeNode::TreeNode(ObjectIdentifier identifier, uint8_t level,
+ std::vector<Entry> entries,
std::map<size_t, ObjectIdentifier> children)
- : page_storage_(page_storage),
- identifier_(std::move(identifier)),
+ : identifier_(std::move(identifier)),
level_(level),
entries_(std::move(entries)),
children_(std::move(children)) {
@@ -42,19 +41,36 @@
fit::function<void(Status, std::unique_ptr<const TreeNode>)> callback) {
page_storage->GetObject(
identifier, PageStorage::Location::NETWORK,
- [page_storage, identifier, callback = std::move(callback)](
+ [identifier, callback = std::move(callback)](
Status status, std::unique_ptr<const Object> object) mutable {
if (status != Status::OK) {
callback(status, nullptr);
return;
}
std::unique_ptr<const TreeNode> node;
- status = FromObject(page_storage, std::move(identifier),
- std::move(object), &node);
+ status = FromObject(*object, &node);
callback(status, std::move(node));
});
}
+Status TreeNode::FromObject(const Object& object,
+ std::unique_ptr<const TreeNode>* node) {
+ fxl::StringView data;
+ Status status = object.GetData(&data);
+ if (status != Status::OK) {
+ return status;
+ }
+ uint8_t level;
+ std::vector<Entry> entries;
+ std::map<size_t, ObjectIdentifier> children;
+ if (!DecodeNode(data, &level, &entries, &children)) {
+ return Status::FORMAT_ERROR;
+ }
+ node->reset(new TreeNode(object.GetIdentifier(), level, std::move(entries),
+ std::move(children)));
+ return Status::OK;
+}
+
void TreeNode::Empty(PageStorage* page_storage,
fit::function<void(Status, ObjectIdentifier)> callback) {
FromEntries(page_storage, 0u, std::vector<Entry>(),
@@ -95,19 +111,6 @@
return Status::OK;
}
-void TreeNode::GetChild(
- int index,
- fit::function<void(Status, std::unique_ptr<const TreeNode>)> callback)
- const {
- FXL_DCHECK(index >= 0 && index <= GetKeyCount());
- const auto it = children_.find(index);
- if (it == children_.end()) {
- callback(Status::NO_SUCH_CHILD, nullptr);
- return;
- }
- return FromIdentifier(page_storage_, it->second, std::move(callback));
-}
-
Status TreeNode::FindKeyOrChild(convert::ExtendedStringView key,
int* index) const {
if (key.empty()) {
@@ -133,25 +136,5 @@
const ObjectIdentifier& TreeNode::GetIdentifier() const { return identifier_; }
-Status TreeNode::FromObject(PageStorage* page_storage,
- ObjectIdentifier identifier,
- std::unique_ptr<const Object> object,
- std::unique_ptr<const TreeNode>* node) {
- fxl::StringView data;
- Status status = object->GetData(&data);
- if (status != Status::OK) {
- return status;
- }
- uint8_t level;
- std::vector<Entry> entries;
- std::map<size_t, ObjectIdentifier> children;
- if (!DecodeNode(data, &level, &entries, &children)) {
- return Status::FORMAT_ERROR;
- }
- node->reset(new TreeNode(page_storage, std::move(identifier), level,
- std::move(entries), std::move(children)));
- return Status::OK;
-}
-
} // namespace btree
} // namespace storage
diff --git a/src/ledger/bin/storage/impl/btree/tree_node.h b/src/ledger/bin/storage/impl/btree/tree_node.h
index 6365a3a..24a58bc 100644
--- a/src/ledger/bin/storage/impl/btree/tree_node.h
+++ b/src/ledger/bin/storage/impl/btree/tree_node.h
@@ -23,28 +23,33 @@
public:
~TreeNode();
- // Creates a |TreeNode| object for an existing node and calls the given
- // |callback| with the returned status and node.
+ // Creates a |TreeNode| object in |page_storage| for an existing node and
+ // calls the given |callback| with the returned status and node.
static void FromIdentifier(
PageStorage* page_storage, ObjectIdentifier identifier,
fit::function<void(Status, std::unique_ptr<const TreeNode>)> callback);
- // Creates a |TreeNode| object with the given entries and children. |children|
- // is a map from the index of the child to the identifier of the child. It
- // only contains non-empty children. It is expected that all child indexes are
- // between |0| and |size(entries)| (included). The |callback| will be called
- // with the success or error status and the id of the new node.
+ // Creates a |TreeNode| object in |page_storage| with the given entries and
+ // children. |children| is a map from the index of the child to the identifier
+ // of the child. It only contains non-empty children. It is expected that all
+ // child indexes are between |0| and |size(entries)| (included). The
+ // |callback| will be called with the success or error status and the id of
+ // the new node.
static void FromEntries(
PageStorage* page_storage, uint8_t level,
const std::vector<Entry>& entries,
const std::map<size_t, ObjectIdentifier>& children,
fit::function<void(Status, ObjectIdentifier)> callback);
- // Creates an empty node, i.e. a TreeNode with no entries and an empty child
- // at index 0 and calls the callback with the result.
+ // Creates an empty node in |page_storage|, i.e. a TreeNode with no entries
+ // and an empty child at index 0 and calls the callback with the result.
static void Empty(PageStorage* page_storage,
fit::function<void(Status, ObjectIdentifier)> callback);
+ // Initializes |node| with a |TreeNode| object for an existing |object|.
+ static Status FromObject(const Object& object,
+ std::unique_ptr<const TreeNode>* node);
+
// Returns the number of entries stored in this tree node.
int GetKeyCount() const;
@@ -52,14 +57,6 @@
// to be in [0, GetKeyCount() - 1].
Status GetEntry(int index, Entry* entry) const;
- // Finds the child node at position |index| and calls the |callback| with the
- // result. |index| has to be in [0, GetKeyCount()]. If the child at the given
- // index is empty |NO_SUCH_CHILD| is returned and the value of |child| is not
- // updated.
- void GetChild(int index,
- fit::function<void(Status, std::unique_ptr<const TreeNode>)>
- callback) const;
-
// Searches for the given |key| in this node. If it is found, |OK| is
// returned and index contains the index of the entry. If not,
// |INTERNAL_NOT_FOUND| is returned and index stores the index of the child
@@ -77,18 +74,10 @@
}
private:
- TreeNode(PageStorage* page_storage, ObjectIdentifier identifier,
- uint8_t level, std::vector<Entry> entries,
+ TreeNode(ObjectIdentifier identifier, uint8_t level,
+ std::vector<Entry> entries,
std::map<size_t, ObjectIdentifier> children);
- // Creates a |TreeNode| object for an existing |object| and stores it in the
- // given |node|.
- static Status FromObject(PageStorage* page_storage,
- ObjectIdentifier identifier,
- std::unique_ptr<const Object> object,
- std::unique_ptr<const TreeNode>* node);
-
- PageStorage* page_storage_;
ObjectIdentifier identifier_;
const uint8_t level_;
const std::vector<Entry> entries_;
diff --git a/src/ledger/bin/storage/impl/btree/tree_node_unittest.cc b/src/ledger/bin/storage/impl/btree/tree_node_unittest.cc
index 219d2dc..02be759 100644
--- a/src/ledger/bin/storage/impl/btree/tree_node_unittest.cc
+++ b/src/ledger/bin/storage/impl/btree/tree_node_unittest.cc
@@ -96,7 +96,7 @@
EXPECT_EQ(Status::INTERNAL_NOT_FOUND, status);
}
-TEST_F(TreeNodeTest, GetEntryChild) {
+TEST_F(TreeNodeTest, GetEntry) {
int size = 10;
std::vector<Entry> entries;
ASSERT_TRUE(CreateEntries(size, &entries));
@@ -106,19 +106,6 @@
for (int i = 0; i < size; ++i) {
EXPECT_EQ(entries[i], GetEntry(node.get(), i));
}
-
- bool called;
- for (int i = 0; i <= size; ++i) {
- Status status;
- std::unique_ptr<const TreeNode> child;
- node->GetChild(i, callback::Capture(callback::SetWhenCalled(&called),
- &status, &child));
- RunLoopFor(kSufficientDelay);
- ASSERT_TRUE(called);
- ASSERT_EQ(Status::NO_SUCH_CHILD, status);
- EXPECT_EQ(node->children_identifiers().find(i),
- node->children_identifiers().end());
- }
}
TEST_F(TreeNodeTest, FindKeyOrChild) {
