diff options
author | rouslan@chromium.org <rouslan@chromium.org@38ededc0-08b8-5190-f2ac-b31f878777ad> | 2014-07-24 00:49:37 +0000 |
---|---|---|
committer | rouslan@chromium.org <rouslan@chromium.org@38ededc0-08b8-5190-f2ac-b31f878777ad> | 2014-07-24 00:49:37 +0000 |
commit | 7ba4ba2dc31f8577eddb6f28603bae97cc29cce9 (patch) | |
tree | e23d03bca6c43c01636960a55ff9c7e6967e737e | |
parent | d75941244491c922456bf0fa4a987c098f082a76 (diff) | |
download | src-7ba4ba2dc31f8577eddb6f28603bae97cc29cce9.tar.gz |
Make JSON methods pass thru to the implementation.
R=roubert@google.com
Review URL: https://codereview.appspot.com/115210043
git-svn-id: http://libaddressinput.googlecode.com/svn/trunk@317 38ededc0-08b8-5190-f2ac-b31f878777ad
-rw-r--r-- | cpp/src/preload_supplier.cc | 8 | ||||
-rw-r--r-- | cpp/src/util/json.cc | 163 | ||||
-rw-r--r-- | cpp/src/util/json.h | 15 | ||||
-rw-r--r-- | cpp/test/util/json_test.cc | 17 |
4 files changed, 95 insertions, 108 deletions
diff --git a/cpp/src/preload_supplier.cc b/cpp/src/preload_supplier.cc index 3a9cdc3..26f7e2b 100644 --- a/cpp/src/preload_supplier.cc +++ b/cpp/src/preload_supplier.cc @@ -114,13 +114,13 @@ class Helper { for (std::vector<std::string>::const_iterator it = json.GetKeys().begin(); it != json.GetKeys().end(); ++it) { - if (!json.HasDictionaryValueForKey(*it)) { + const Json* value; + if (!json.GetDictionaryValueForKey(*it, &value)) { success = false; goto callback; } - const Json& value = json.GetDictionaryValueForKey(*it); - if (!value.GetStringValueForKey("id", &id)) { + if (!value->GetStringValueForKey("id", &id)) { success = false; goto callback; } @@ -135,7 +135,7 @@ class Helper { // All rules on the COUNTRY level inherit from the default rule. rule->CopyFrom(Rule::GetDefault()); } - rule->ParseJsonRule(value); + rule->ParseJsonRule(*value); assert(id == rule->GetId()); // Sanity check. rule_storage_->push_back(rule); diff --git a/cpp/src/util/json.cc b/cpp/src/util/json.cc index 4f26069..14a9d11 100644 --- a/cpp/src/util/json.cc +++ b/cpp/src/util/json.cc @@ -36,75 +36,97 @@ using rapidjson::Value; class Json::JsonImpl { public: - // Takes ownership of |document|. - explicit JsonImpl(const Document* document) - : document_(document), value_(document), dictionaries_() { - assert(value_ != NULL); - assert(value_->IsObject()); - BuildKeyList(); - } - - // Does not take ownership of |value|. - explicit JsonImpl(const Value* value) - : document_(), value_(value), dictionaries_() { - assert(value_ != NULL); - assert(value_->IsObject()); - BuildKeyList(); + explicit JsonImpl(const std::string& json) + : document_(new Document), + value_(document_.get()), + dictionaries_(), + keys_(), + valid_(false) { + document_->Parse<kParseValidateEncodingFlag>(json.c_str()); + valid_ = !document_->HasParseError() && document_->IsObject(); + if (valid_) { + BuildKeyList(); + } } ~JsonImpl() { for (std::map<std::string, const Json*>::const_iterator it = dictionaries_.begin(); - it != dictionaries_.end(); ++it) { + it != dictionaries_.end(); + ++it) { delete it->second; } } - // The caller does not own the result. - const Value::Member* FindMember(const std::string& key) { - return value_->FindMember(key.c_str()); + bool valid() const { return valid_; } + + const std::vector<std::string>& GetKeys() const { return keys_; } + + bool GetStringValueForKey(const std::string& key, std::string* value) const { + assert(value != NULL); + + Value::ConstMemberIterator member = value_->FindMember(key.c_str()); + if (member == NULL || !member->value.IsString()) { + return false; + } + + value->assign(member->value.GetString(), + member->value.GetStringLength()); + return true; } - // The caller does not own the result. The result can be NULL if there's no - // dictionary for |key|. - const Json* FindDictionary(const std::string& key) const { - std::map<std::string, const Json*>::const_iterator it = + bool GetDictionaryValueForKey(const std::string& key, const Json** value) { + assert(value != NULL); + + std::map<std::string, const Json*>::const_iterator dict_it = dictionaries_.find(key); - return it != dictionaries_.end() ? it->second : NULL; - } + if (dict_it != dictionaries_.end()) { + *value = dict_it->second; + return true; + } + + Value::ConstMemberIterator member = value_->FindMember(key.c_str()); + if (member == NULL || !member->value.IsObject()) { + return false; + } - // Takes ownership of |dictionary|. Should be called only once per |key| and - // per |dictionary|. - void AddDictionary(const std::string& key, const Json* dictionary) { + Json* sub_dictionary = new Json; + sub_dictionary->impl_.reset(new JsonImpl(&member->value)); bool inserted = - dictionaries_.insert(std::make_pair(key, dictionary)).second; + dictionaries_.insert(std::make_pair(key, sub_dictionary)).second; // Cannot do work inside of assert(), because the compiler can optimize it // away. assert(inserted); // Avoid unused variable warning when assert() is optimized away. (void)inserted; - } - const std::vector<std::string>& GetKeys() const { return keys_; } + *value = sub_dictionary; + return true; + } private: + // Does not take ownership of |value|. + explicit JsonImpl(const Value* value) + : document_(), + value_(value), + dictionaries_(), + keys_(), + valid_(true) { + assert(value_ != NULL); + assert(value_->IsObject()); + BuildKeyList(); + } + void BuildKeyList() { assert(keys_.empty()); - for (Value::ConstMemberIterator it = value_->MemberBegin(); - it != value_->MemberEnd(); ++it) { - keys_.push_back(it->name.GetString()); + for (Value::ConstMemberIterator member = value_->MemberBegin(); + member != value_->MemberEnd(); ++member) { + keys_.push_back(member->name.GetString()); } } // An owned JSON document. Can be NULL if the JSON document is not owned. - // - // When a JsonImpl object is constructed using a Document object, then - // JsonImpl is supposed to take ownership of that object, making sure to - // delete it in its own destructor. But when a JsonImpl object is constructed - // using a Value object, then that object is owned by a Member object which is - // owned by a Document object, and should therefore not be deleted by - // JsonImpl. - const scoped_ptr<const Document> document_; + const scoped_ptr<Document> document_; // A JSON document that is not owned. Cannot be NULL. Can point to document_. const Value* const value_; @@ -112,8 +134,12 @@ class Json::JsonImpl { // Owned JSON objects. std::map<std::string, const Json*> dictionaries_; + // The list of keys with values in the JSON object. std::vector<std::string> keys_; + // True if the JSON object was parsed successfully. + bool valid_; + DISALLOW_COPY_AND_ASSIGN(JsonImpl); }; @@ -123,13 +149,11 @@ Json::~Json() {} bool Json::ParseObject(const std::string& json) { assert(impl_ == NULL); - scoped_ptr<Document> document(new Document); - document->Parse<kParseValidateEncodingFlag>(json.c_str()); - bool valid = !document->HasParseError() && document->IsObject(); - if (valid) { - impl_.reset(new JsonImpl(document.release())); + impl_.reset(new JsonImpl(json)); + if (!impl_->valid()) { + impl_.reset(); } - return valid; + return impl_ != NULL; } const std::vector<std::string>& Json::GetKeys() const { @@ -140,50 +164,13 @@ const std::vector<std::string>& Json::GetKeys() const { bool Json::GetStringValueForKey(const std::string& key, std::string* value) const { assert(impl_ != NULL); - assert(value != NULL); - - // Member is owned by impl_. - const Value::Member* member = impl_->FindMember(key.c_str()); - if (member == NULL || !member->value.IsString()) { - return false; - } - - value->assign(member->value.GetString(), member->value.GetStringLength()); - return true; -} - -bool Json::HasDictionaryValueForKey(const std::string& key) const { - assert(impl_ != NULL); - - // The value returned by FindDictionary() is owned by impl_. - if (impl_->FindDictionary(key) != NULL) { - return true; - } - - // Member is owned by impl_. - const Value::Member* member = impl_->FindMember(key); - return member != NULL && member->value.IsObject(); + return impl_->GetStringValueForKey(key, value); } -const Json& Json::GetDictionaryValueForKey(const std::string& key) const { +bool Json::GetDictionaryValueForKey(const std::string& key, + const Json** value) const { assert(impl_ != NULL); - - // Existing_dictionary is owned by impl_. - const Json* existing_dictionary = impl_->FindDictionary(key); - if (existing_dictionary != NULL) { - return *existing_dictionary; - } - - // Member is owned by impl_. - const Value::Member* member = impl_->FindMember(key); - assert(member != NULL); - assert(member->value.IsObject()); - - // Dictionary is owned by impl_. - Json* dictionary = new Json; - dictionary->impl_.reset(new JsonImpl(&member->value)); - impl_->AddDictionary(key, dictionary); - return *dictionary; + return impl_->GetDictionaryValueForKey(key, value); } } // namespace addressinput diff --git a/cpp/src/util/json.h b/cpp/src/util/json.h index 90565e6..1d66aeb 100644 --- a/cpp/src/util/json.h +++ b/cpp/src/util/json.h @@ -49,15 +49,12 @@ class Json { // parameter should not be NULL. bool GetStringValueForKey(const std::string& key, std::string* value) const; - // Returns true if the parsed JSON contains a dictionary value for |key|. The - // JSON object must be parsed successfully in ParseObject() before invoking - // this method. - bool HasDictionaryValueForKey(const std::string& key) const; - - // Returns the dictionary value for the |key|. The |key| must be present and - // its value must be of the dictionary type, i.e., HasDictionaryValueForKey() - // must return true before invoking this method. - const Json& GetDictionaryValueForKey(const std::string& key) const; + // Returns true if the parsed JSON contains a dictionary value for |key|. + // Points |value| to the dictionary value of the |key|. The JSON object must + // be parsed successfully in ParseObject() before invoking this method. The + // caller does not own |value|. + bool GetDictionaryValueForKey(const std::string& key, + const Json** value) const; private: class JsonImpl; diff --git a/cpp/test/util/json_test.cc b/cpp/test/util/json_test.cc index ff23ea6..2306ac3 100644 --- a/cpp/test/util/json_test.cc +++ b/cpp/test/util/json_test.cc @@ -120,16 +120,18 @@ TEST(JsonTest, NumberIsNotValid) { TEST(JsonTest, NoDictionaryFound) { Json json; ASSERT_TRUE(json.ParseObject("{\"key\":\"value\"}")); - EXPECT_FALSE(json.HasDictionaryValueForKey("key")); + const Json* sub_json; + EXPECT_FALSE(json.GetDictionaryValueForKey("key", &sub_json)); } TEST(JsonTest, DictionaryFound) { Json json; ASSERT_TRUE(json.ParseObject("{\"key\":{\"inner_key\":\"value\"}}")); - ASSERT_TRUE(json.HasDictionaryValueForKey("key")); - const Json& sub_json = json.GetDictionaryValueForKey("key"); + const Json* sub_json = NULL; + ASSERT_TRUE(json.GetDictionaryValueForKey("key", &sub_json)); + ASSERT_TRUE(sub_json != NULL); std::string value; - EXPECT_TRUE(sub_json.GetStringValueForKey("inner_key", &value)); + EXPECT_TRUE(sub_json->GetStringValueForKey("inner_key", &value)); EXPECT_EQ("value", value); } @@ -139,10 +141,11 @@ TEST(JsonTest, DictionariesHaveKeys) { std::vector<std::string> expected_keys(1, "key"); EXPECT_EQ(expected_keys, json.GetKeys()); - ASSERT_TRUE(json.HasDictionaryValueForKey("key")); - const Json& sub_json = json.GetDictionaryValueForKey("key"); + const Json* sub_json = NULL; + ASSERT_TRUE(json.GetDictionaryValueForKey("key", &sub_json)); + ASSERT_TRUE(sub_json != NULL); std::vector<std::string> expected_sub_keys(1, "inner_key"); - EXPECT_EQ(expected_sub_keys, sub_json.GetKeys()); + EXPECT_EQ(expected_sub_keys, sub_json->GetKeys()); } } // namespace |