Skip to content

Commit

Permalink
Add Value path functions for nested dictionaries
Browse files Browse the repository at this point in the history
The paths are specified as initializer lists to avoid parsing them. We will
likely need to add on a different capability to support callers that have
only dynamic paths, but I think we will want to support this format
either way.

Updates flat_set and flat_map documentation with a missing function.

Bug: 646113
Change-Id: I105b958afa7c514147f259c6be1e3608518d7734
Reviewed-on: https://chromium-review.googlesource.com/589779
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491563}
  • Loading branch information
Brett Wilson authored and Commit Bot committed Aug 3, 2017
1 parent 4c1b885 commit d16cf4e
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 17 deletions.
2 changes: 2 additions & 0 deletions base/containers/flat_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ struct GetKeyFromValuePairFirst {
// Mapped& operator[](key_type&&);
// pair<iterator, bool> insert(const value_type&);
// pair<iterator, bool> insert(value_type&&);
// iterator insert(const_iterator hint, const value_type&);
// iterator insert(const_iterator hint, value_type&&);
// void insert(InputIterator first, InputIterator last,
// FlatContainerDupes);
// pair<iterator, bool> emplace(Args&&...);
Expand Down
2 changes: 2 additions & 0 deletions base/containers/flat_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ namespace base {
// pair<iterator, bool> insert(key_type&&);
// void insert(InputIterator first, InputIterator last,
// FlatContainerDupes);
// iterator insert(const_iterator hint, const key_type&);
// iterator insert(const_iterator hint, key_type&&);
// pair<iterator, bool> emplace(Args&&...);
// iterator emplace_hint(const_iterator, Args&&...);
//
Expand Down
62 changes: 62 additions & 0 deletions base/values.cc
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,68 @@ Value::dict_iterator Value::SetKey(const char* key, Value value) {
return SetKey(StringPiece(key), std::move(value));
}

Value* Value::FindPath(std::initializer_list<const char*> path) {
return const_cast<Value*>(const_cast<const Value*>(this)->FindPath(path));
}

const Value* Value::FindPath(std::initializer_list<const char*> path) const {
const Value* cur = this;
for (const char* component : path) {
if (!cur->is_dict())
return nullptr;

auto found = cur->FindKey(component);
if (found == cur->DictEnd())
return nullptr;
cur = &found->second;
}
return cur;
}

Value* Value::FindPathOfType(std::initializer_list<const char*> path,
Type type) {
return const_cast<Value*>(
const_cast<const Value*>(this)->FindPathOfType(path, type));
}

const Value* Value::FindPathOfType(std::initializer_list<const char*> path,
Type type) const {
const Value* result = FindPath(path);
if (!result || !result->IsType(type))
return nullptr;
return result;
}

Value* Value::SetPath(std::initializer_list<const char*> path, Value value) {
DCHECK_NE(path.begin(), path.end()); // Can't be empty path.

// Walk/construct intermediate dictionaries. The last element requires
// special handling so skip it in this loop.
Value* cur = this;
const char* const* cur_path = path.begin();
for (; (cur_path + 1) < path.end(); ++cur_path) {
if (!cur->is_dict())
return nullptr;

// Use lower_bound to avoid doing the search twice for missing keys.
const char* path_component = *cur_path;
auto found = cur->dict_->lower_bound(path_component);
if (found == cur->dict_->end() || found->first != path_component) {
// No key found, insert one.
auto inserted = cur->dict_->emplace_hint(
found, path_component, MakeUnique<Value>(Type::DICTIONARY));
cur = inserted->second.get();
} else {
cur = found->second.get();
}
}

// "cur" will now contain the last dictionary to insert or replace into.
if (!cur->is_dict())
return nullptr;
return &cur->SetKey(*cur_path, std::move(value))->second;
}

Value::dict_iterator Value::DictEnd() {
CHECK(is_dict());
return dict_iterator(dict_->end());
Expand Down
34 changes: 34 additions & 0 deletions base/values.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,40 @@ class BASE_EXPORT Value {
// This overload is necessary to avoid ambiguity for const char* arguments.
dict_iterator SetKey(const char* key, Value value);

// Searches a hierarchy of dictionary values for a given value. If a path
// of dictionaries exist, returns the item at that path. If any of the path
// components do not exist or if any but the last path components are not
// dictionaries, returns nullptr.
//
// The type of the leaf Value is not checked.
//
// Implementation note: This can't return an iterator because the iterator
// will actually be into another Value, so it can't be compared to iterators
// from thise one (in particular, the DictEnd() iterator).
//
// Example:
// auto found = FindPath({"foo", "bar"});
Value* FindPath(std::initializer_list<const char*> path);
const Value* FindPath(std::initializer_list<const char*> path) const;

// Like FindPath but will only return the value if the leaf Value type
// matches the given type. Will return nullptr otherwise.
Value* FindPathOfType(std::initializer_list<const char*> path, Type type);
const Value* FindPathOfType(std::initializer_list<const char*> path,
Type type) const;

// Sets the given path, expanding and creating dictionary keys as necessary.
//
// The current value must be a dictionary. If path components do not exist,
// they will be created. If any but the last components matches a value that
// is not a dictionary, the function will fail (it will not overwrite the
// value) and return nullptr. The last path component will be unconditionally
// overwritten if it exists, and created if it doesn't.
//
// Example:
// value.SetPath({"foo", "bar"}, std::move(myvalue));
Value* SetPath(std::initializer_list<const char*> path, Value value);

// |DictEnd| returns the end iterator of the underlying dictionary. It is
// intended to be used with |FindKey| in order to determine whether a given
// key is present in the dictionary.
Expand Down
58 changes: 58 additions & 0 deletions base/values_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,64 @@ TEST(ValuesTest, SetKey) {
EXPECT_EQ(Value(std::move(storage)), dict);
}

TEST(ValuesTest, FindPath) {
// Construct a dictionary path {root}.foo.bar = 123
Value foo(Value::Type::DICTIONARY);
foo.SetKey("bar", Value(123));

Value root(Value::Type::DICTIONARY);
root.SetKey("foo", std::move(foo));

// No key (stupid but well-defined and takes work to prevent).
Value* found = root.FindPath({});
EXPECT_EQ(&root, found);

// Single not found key.
found = root.FindPath({"notfound"});
EXPECT_FALSE(found);

// Single found key.
found = root.FindPath({"foo"});
ASSERT_TRUE(found);
EXPECT_TRUE(found->is_dict());

// Double key, second not found.
found = root.FindPath({"foo", "notfound"});
EXPECT_FALSE(found);

// Double key, found.
found = root.FindPath({"foo", "bar"});
EXPECT_TRUE(found);
EXPECT_TRUE(found->is_int());
EXPECT_EQ(123, found->GetInt());
}

TEST(ValuesTest, SetPath) {
Value root(Value::Type::DICTIONARY);

Value* inserted = root.SetPath({"one"}, Value(123));
Value* found = root.FindPathOfType({"one"}, Value::Type::INTEGER);
ASSERT_TRUE(found);
EXPECT_EQ(inserted, found);
EXPECT_EQ(123, found->GetInt());

inserted = root.SetPath({"foo", "bar"}, Value(123));
found = root.FindPathOfType({"foo", "bar"}, Value::Type::INTEGER);
ASSERT_TRUE(found);
EXPECT_EQ(inserted, found);
EXPECT_EQ(123, found->GetInt());

// Overwrite with a different value.
root.SetPath({"foo", "bar"}, Value("hello"));
found = root.FindPathOfType({"foo", "bar"}, Value::Type::STRING);
ASSERT_TRUE(found);
EXPECT_EQ("hello", found->GetString());

// Can't change existing non-dictionary keys to dictionaries.
found = root.SetPath({"foo", "bar", "baz"}, Value(123));
EXPECT_FALSE(found);
}

TEST(ValuesTest, DictEnd) {
Value dict(Value::Type::DICTIONARY);
EXPECT_EQ(dict.DictItems().end(), dict.DictEnd());
Expand Down
19 changes: 2 additions & 17 deletions chrome/profiling/json_exporter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,6 @@ const base::Value* FindFirstPeriodicInterval(const base::Value& root) {
return nullptr;
}

const base::Value* ResolveValuePath(const base::Value& input,
std::initializer_list<const char*> path) {
const base::Value* cur = &input;
for (const char* component : path) {
if (!cur->is_dict())
return nullptr;

auto found = cur->FindKey(component);
if (found == cur->DictEnd())
return nullptr;
cur = &found->second;
}
return cur;
}

} // namespace

TEST(ProfilingJsonExporter, Simple) {
Expand Down Expand Up @@ -84,13 +69,13 @@ TEST(ProfilingJsonExporter, Simple) {
ASSERT_TRUE(periodic_interval) << "Array contains no periodic_interval";

const base::Value* heaps_v2 =
ResolveValuePath(*periodic_interval, {"args", "dumps", "heaps_v2"});
periodic_interval->GetPath({"args", "dumps", "heaps_v2"});
ASSERT_TRUE(heaps_v2);

// Counts should be a list of two items, a 1 and a 2 (in either order). The
// two matching 16-byte allocations should be coalesced to produce the 2.
const base::Value* counts =
ResolveValuePath(*heaps_v2, {"allocators", "malloc", "counts"});
heaps_v2->GetPath({"allocators", "malloc", "counts"});
ASSERT_TRUE(counts);
EXPECT_EQ(2u, counts->GetList().size());
EXPECT_TRUE((counts->GetList()[0].GetInt() == 1 &&
Expand Down

0 comments on commit d16cf4e

Please sign in to comment.