[rac] Download country code data in a single HTTP request.

third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc

Index: third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc
diff --git a/third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc b/third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc
index e268f86dd3722544f90612b76d3d1a6e6c36e0d8..fd13ac2d0aa08c10ce5bc6a7318b6697df4ac370 100644
--- a/third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc
+++ b/third_party/libaddressinput/chromium/cpp/src/country_rules_aggregator.cc
@@ -19,65 +19,113 @@
 #include <libaddressinput/util/basictypes.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <algorithm>
 #include <cassert>
 #include <cstddef>
-#include <map>
 #include <string>
-#include <utility>
+#include <vector>
 
 #include "retriever.h"
 #include "rule.h"
 #include "ruleset.h"
-#include "util/stl_util.h"
+#include "util/json.h"
 
 namespace i18n {
 namespace addressinput {
 
-// Information about data requests sent to Retriever. This data is not returned
-// as part of the ruleset, but is useful in constructing the ruleset
-// asynchronously.
-struct CountryRulesAggregator::RequestData {
-  // Does not take ownership of |parent|.
-  RequestData(Ruleset* parent,
-              AddressField level,
-              bool is_language_code,
-              const std::string& id)
-      : parent(parent),
-        level(level),
-        is_language_code(is_language_code),
-        id(id) {
-    assert(parent != NULL || level == COUNTRY);
+namespace {
+
+// Stores the shared data for parsing multiple rules into a ruleset.
+class RulesetBuilder {
+ public:
+  explicit RulesetBuilder(scoped_ptr<Json> json)
+      : json_(json.Pass()),
+        languages_() {
+    assert(json_ != NULL);
   }
 
-  ~RequestData() {}
+  ~RulesetBuilder() {}
+
+  // Builds and returns the ruleset for |key| at |field| level. Returns NULL on
+  // failure, e.g. missing sub-region data in JSON.
+  scoped_ptr<Ruleset> Build(const std::string& key, AddressField field) {
+    scoped_ptr<Rule> rule = ParseRule(key, field);
+    if (rule == NULL) {
+      return scoped_ptr<Ruleset>();
+    }
+
+    if (field == COUNTRY) {
+      languages_ = rule->GetLanguages();
+      std::vector<std::string>::iterator default_language_it =
+          std::find(languages_.begin(), languages_.end(), rule->GetLanguage());
+      if (default_language_it != languages_.end()) {
+        languages_.erase(default_language_it);
+      }
+    }
+
+    scoped_ptr<Ruleset> ruleset(new Ruleset(field, rule.Pass()));
+
+    for (std::vector<std::string>::const_iterator lang_it = languages_.begin();

[Evan Stade, 2014/01/18 19:14:32]

please add short comments to each of these loops

[please use gerrit instead, 2014/01/21 18:43:54]

Done.

+         lang_it != languages_.end(); ++lang_it) {
+      scoped_ptr<Rule> lang_rule = ParseRule(key + "--" + *lang_it, field);
+      if (lang_rule == NULL) {
+        return scoped_ptr<Ruleset>();
+      }
+      ruleset->AddLanguageCodeRule(*lang_it, lang_rule.Pass());
+    }
+
+    if (field == DEPENDENT_LOCALITY) {

[Evan Stade, 2014/01/18 19:14:32]

why is this necessary? Wouldn't the dependent locality already not have subkeys?

[please use gerrit instead, 2014/01/21 18:43:54]

True. I've changed this statement to an assert(), which would be triggered only
in Debug builds.

+      return ruleset.Pass();
+    }
 
-  // The parent ruleset of the data being downloaded. If NULL, then this is the
-  // root ruleset at the COUNTRY level. The language-specific and sub-region
-  // rules are added to this ruleset. Owned by |CountryRulesRetriever|.
-  Ruleset* parent;
+    for (std::vector<std::string>::const_iterator
+             subkey_it = ruleset->rule().GetSubKeys().begin();
+         subkey_it != ruleset->rule().GetSubKeys().end(); ++subkey_it) {
+      scoped_ptr<Ruleset> sub_ruleset =
+          Build(key + "/" + *subkey_it, static_cast<AddressField>(field + 1));
+      if (sub_ruleset == NULL) {
+        return scoped_ptr<Ruleset>();
+      }
+      ruleset->AddSubRegionRuleset(*subkey_it, sub_ruleset.Pass());
+    }
 
-  // The level of the data being requested. Ranges from COUNTRY to
-  // DEPENDENT_LOCALITY. If COUNTRY, then the rule should use default rules from
-  // Rule::GetDefault().
-  AddressField level;
+    return ruleset.Pass();
+  }
 
-  // If true, then |id| is a language. The data received for this request should
-  // be placed into a language-specific rule.
-  bool is_language_code;
+ private:
+  // Builds and returns the rule for |key| at |field| level. Returns NULL if
+  // |key| is not in JSON.
+  scoped_ptr<Rule> ParseRule(const std::string& key, AddressField field) {
+    Json* value = NULL;
+    json_->GetJsonValueForKey(key, &value);

[Evan Stade, 2014/01/18 19:14:32]

why did you make GetJsonValueForKey return a bool if you don't need it?

[please use gerrit instead, 2014/01/21 18:43:54]

Using the bool now.

+    if (value == NULL) {
+      return scoped_ptr<Rule>();
+    }
+    scoped_ptr<Json> value_scoped_ptr(value);
+    scoped_ptr<Rule> rule(new Rule);
+    if (field == COUNTRY) {
+      rule->CopyFrom(Rule::GetDefault());
+    }
+    rule->ParseJsonRule(*value_scoped_ptr);
+    return rule.Pass();
+  }
+
+  // The collection of rules for a country code.
+  scoped_ptr<Json> json_;
 
-  // Can be a region name (e.g. "CA") or a language (e.g. "fr"). Used to add a
-  // sub-region or a language-specific rule to |parent|.
-  std::string id;
+  // The non-default languages that have custom rules.
+  std::vector<std::string> languages_;

[Evan Stade, 2014/01/18 19:14:32]

nit: non_default_languages_? other_languages_?

[please use gerrit instead, 2014/01/21 18:43:54]

non_default_languages_.

+
+  DISALLOW_COPY_AND_ASSIGN(RulesetBuilder);
 };
 
+}  // namespace
+
 CountryRulesAggregator::CountryRulesAggregator(scoped_ptr<Retriever> retriever)
     : retriever_(retriever.Pass()),
-      requests_(),
       country_code_(),
-      rules_ready_(),
-      root_(),
-      default_language_(),
-      languages_() {
+      key_(),
+      rules_ready_() {
   assert(retriever_ != NULL);
 }
 
@@ -92,118 +140,35 @@ void CountryRulesAggregator::AggregateRules(const std::string& country_code,
   // Key construction:
   // https://code.google.com/p/libaddressinput/wiki/AddressValidationMetadata
   // Example of a country-level key: "data/CA".
-  std::string key = "data/" + country_code_;
-  requests_.insert(std::make_pair(
-      key, RequestData(NULL, COUNTRY, false, std::string())));
-
+  key_ = "data/" + country_code_;
   retriever_->Retrieve(
-      key, BuildCallback(this, &CountryRulesAggregator::OnDataReady));
+      key_, BuildCallback(this, &CountryRulesAggregator::OnDataReady));
 }
 
 void CountryRulesAggregator::OnDataReady(bool success,
                                          const std::string& key,
                                          const std::string& data) {
-  std::map<std::string, RequestData>::iterator request_it =
-      requests_.find(key);
-  if (request_it == requests_.end()) {
+  if (key != key_) {
     return;  // An abandoned request.
   }
 
-  if (!success) {
+  scoped_ptr<Json> json(Json::Build());
+  if (!success || !json->ParseObject(data)) {
     (*rules_ready_)(false, country_code_, scoped_ptr<Ruleset>());
     Reset();
     return;
   }
 
-  RequestData request = request_it->second;
-  requests_.erase(request_it);
-
-  // All country-level rules are based on the default rule.
-  scoped_ptr<Rule> rule(new Rule);
-  if (request.level == COUNTRY) {
-    rule->CopyFrom(Rule::GetDefault());
-  }
-
-  if (!rule->ParseSerializedRule(data)) {
-    (*rules_ready_)(false, country_code_, scoped_ptr<Ruleset>());
-    Reset();
-    return;
-  }
-
-  // Place the rule in the correct location in the ruleset.
-  Ruleset* ruleset = NULL;
-  if (request.is_language_code) {
-    assert(request.parent != NULL);
-    request.parent->AddLanguageCodeRule(request.id, rule.Pass());
-    ruleset = request.parent;
-  } else if (request.level == COUNTRY) {
-    // The default language and all supported languages for the country code are
-    // in the country-level rule without a language code identifier. For
-    // example: "data/CA".
-    default_language_ = rule->GetLanguage();
-    languages_ = rule->GetLanguages();
-
-    root_.reset(new Ruleset(request.level, rule.Pass()));
-    ruleset = root_.get();
-  } else {
-    assert(request.parent != NULL);
-    ruleset = new Ruleset(request.level, rule.Pass());
-    request.parent->AddSubRegionRuleset(
-        request.id, scoped_ptr<Ruleset>(ruleset));
-  }
-
-  if (!request.is_language_code) {
-    // Retrieve the language-specific rules for this region.
-    for (std::vector<std::string>::const_iterator
-             lang_it = languages_.begin();
-         lang_it != languages_.end();
-         ++lang_it) {
-      if (*lang_it == default_language_) {
-        continue;
-      }
-      // Example of a language-specific key: "data/CA--fr".
-      std::string language_code_key = key + "--" + *lang_it;
-      requests_.insert(std::make_pair(
-          key, RequestData(ruleset, request.level, true, *lang_it)));
-      retriever_->Retrieve(
-          language_code_key,
-          BuildCallback(this, &CountryRulesAggregator::OnDataReady));
-    }
-
-    if (request.level < DEPENDENT_LOCALITY) {
-      // Retrieve the sub-region rules for this region.
-      for (std::vector<std::string>::const_iterator
-               subkey_it = ruleset->rule().GetSubKeys().begin();
-           subkey_it != ruleset->rule().GetSubKeys().end();
-           ++subkey_it) {
-        // Example of a sub-region key: "data/CA/AB".
-        std::string sub_region_key = key + "/" + *subkey_it;
-        requests_.insert(std::make_pair(
-            key,
-            RequestData(ruleset,
-                        static_cast<AddressField>(request.level + 1),
-                        false,
-                        *subkey_it)));
-        retriever_->Retrieve(
-            sub_region_key,
-            BuildCallback(this, &CountryRulesAggregator::OnDataReady));
-      }
-    }
-  }
-
-  if (requests_.empty()) {
-    (*rules_ready_)(true, country_code_, root_.Pass());
-    Reset();
-  }
+  RulesetBuilder builder(json.Pass());
+  scoped_ptr<Ruleset> ruleset = builder.Build(key_, COUNTRY);
+  (*rules_ready_)(ruleset != NULL, country_code_, ruleset.Pass());
+  Reset();
 }
 
 void CountryRulesAggregator::Reset() {
-  requests_.clear();
   country_code_.clear();
+  key_.clear();
   rules_ready_.reset();
-  root_.reset();
-  default_language_.clear();
-  languages_.clear();
 }
 
 }  // namespace addressinput