[rac] Validate an address.

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

 // Copyright (C) 2013 Google Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 // http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #include <libaddressinput/address_validator.h>
 
+#include <libaddressinput/address_data.h>
 #include <libaddressinput/downloader.h>
 #include <libaddressinput/load_rules_delegate.h>
 #include <libaddressinput/localization.h>
 #include <libaddressinput/storage.h>
 #include <libaddressinput/util/basictypes.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
+#include <algorithm>
 #include <cassert>
+#include <cstddef>
 #include <map>
 #include <string>
 #include <utility>
 
 #include "country_rules_retriever.h"
 #include "lookup_key.h"
 #include "retriever.h"
+#include "rule.h"
 #include "rule_retriever.h"
 #include "util/stl_util.h"
 #include "validating_storage.h"
 
 namespace i18n {
 namespace addressinput {
 
 namespace {
 
+// Returns the next smaller size field type for |field|. For example, returns
+// ADMIN_AREA for COUNTRY. The |field| can only be COUNTRY, ADMIN_AREA, and
+// LOCALITY.
+AddressField GetSubField(AddressField field) {
+  switch (field) {
+    case COUNTRY:
+      return ADMIN_AREA;

[Evan Stade, 2013/12/19 02:12:38]

you've now encoded this ordering twice in one CL.

[please use gerrit instead, 2014/01/08 00:55:52]

Fixed.

+    case ADMIN_AREA:
+      return LOCALITY;
+    case LOCALITY:
+      return DEPENDENT_LOCALITY;
+    default:
+      assert(false);
+      return field;
+  }
+}
+
+// Returns the |field| value from |address|. The |field| can be only ADMIN_AREA,
+// LOCALITY, and DEPENDENT_LOCALITY.
+const std::string& GetValue(const AddressData& address, AddressField field) {

[Evan Stade, 2013/12/19 02:12:38]

didn't I read this function elsewhere in this CL?

[please use gerrit instead, 2014/01/08 00:55:52]

Moved to AddressData.

+  switch (field) {
+    case ADMIN_AREA:
+      return address.administrative_area;
+    case LOCALITY:
+      return address.locality;
+    case DEPENDENT_LOCALITY:
+      return address.dependent_locality;
+    default:
+      assert(false);
+      static const std::string kEmpty;
+      return kEmpty;
+  }
+}
+
+// Returns true if |filter| allows the |field| to have a |problem_type|.
+bool FilterAllows(const AddressProblemFilter& filter,
+                  AddressField field,
+                  AddressProblem::Type problem_type) {
+  if (filter.empty()) {
+    return true;
+  }
+  std::pair<AddressProblemFilter::const_iterator,
+            AddressProblemFilter::const_iterator> range =
+                filter.equal_range(field);

[Evan Stade, 2013/12/19 02:12:38]

while this is interesting, I think the following is easier to read because it's
more idiomatic (and has no worse time complexity):

for (AddressProblemFilter::const_iterator it = filter.begin();
     it != filter.end(); ++it) {
  if (it->first == field && it->second == problem_type) {
    return true;
  }
}

[please use gerrit instead, 2014/01/08 00:55:52]

Done.

+  for (AddressProblemFilter::const_iterator it = range.first;
+       it != range.second; ++it) {
+    if (it->second == problem_type) {
+      return true;
+    }
+  }
+  return false;
+}
+
 // Contains all rules loaded for a country code.
 struct CountryRules {
   CountryRules(const std::string& country_code,
                scoped_ptr<CountryRulesRetriever::Callback> rules_ready)
     : status(AddressValidator::RULES_NOT_READY),
       root(country_code),
       rules_ready(rules_ready.Pass()) {}
 
   ~CountryRules() {}
 
@@ skipping 49 matching lines @@
                                &country_rules->root);
     }
   }
 
   // AddressValidator implementation.
   virtual Status ValidateAddress(
       const AddressData& address,
       const AddressProblemFilter& filter,
       const Localization& localization,
       AddressProblems* problems) const {
-    return RULES_UNAVAILABLE;
+    CountryRulesMap::const_iterator country_rule_it =
+        rules_.find(address.country_code);
+    if (country_rule_it == rules_.end()) {
+      return RULES_UNAVAILABLE;
+    }
+    assert(country_rule_it->second != NULL);
+    if (country_rule_it->second->status != SUCCESS) {
+      return country_rule_it->second->status;
+    }
+    std::map<AddressField, const Rule*> field_rule_map =

[Evan Stade, 2013/12/19 02:12:38]

this function nee4ds some vertical whitespace somewhere.

[please use gerrit instead, 2014/01/08 00:55:52]

Done.

+        country_rule_it->second->root.BuildFieldRuleMap(address);
+    for (std::map<AddressField, const Rule*>::const_iterator
+         field_rule_it = field_rule_map.begin();

[Evan Stade, 2013/12/19 02:12:38]

indent by 4 more

[please use gerrit instead, 2014/01/08 00:55:52]

The rest of libaddressinput uses this format when "it = collection.begin()" does
not fit on the first line of the for-loop:

for (CollectionType::const_iterator
     it = collection.begin();
     it != collection.end();
     ++it) {
  ...
}

I think this makes the loop easier to reed. WYT?

[Evan Stade, 2014/01/08 21:29:34]

Why do you think it makes it easier to read? I think it makes it harder because
it's less obvious that the second line is a continuation of the first.

[please use gerrit instead, 2014/01/09 19:42:00]

Done.

+         field_rule_it != field_rule_map.end();
+         ++field_rule_it) {
+      AddressField field = field_rule_it->first;
+      const Rule& rule = *field_rule_it->second;
+      if (field < DEPENDENT_LOCALITY) {

[Evan Stade, 2013/12/19 02:12:38]

nit: invert this check and continue;, that keeps the total indentation down for
the rest of the function (indentation roughly aligns with how much context one
has to remember when reading code)

[please use gerrit instead, 2014/01/08 00:55:52]

No longer applicable.

+        AddressField sub_field = GetSubField(field);
+        const std::string& sub_value = GetValue(address, sub_field);
+        if (!rule.GetSubKeys().empty() && !sub_value.empty() &&

[Evan Stade, 2013/12/19 02:12:38]

nit: easier to read with one condition per line (when you are wrapping any
conditions, wrap all conditions)

[please use gerrit instead, 2014/01/08 00:55:52]

Done.

+            std::find(rule.GetSubKeys().begin(),
+                      rule.GetSubKeys().end(),
+                      sub_value) == rule.GetSubKeys().end() &&
+            FilterAllows(filter, sub_field, AddressProblem::UNKNOWN_VALUE)) {
+          AddressProblem problem = {sub_field,
+                                    AddressProblem::UNKNOWN_VALUE,
+                                    std::string()};
+          problems->push_back(problem);
+        }
+      }
+    }
+    return SUCCESS;
   }
 
  private:
   // Called when CountryRulesRetriever::RetrieveRules finishes loading all rules
   // for the |country_code|.
   void OnRulesLoaded(bool success,
                      const std::string& country_code,
                      const LookupKey& lookup_key) {
     CountryRulesMap::const_iterator rules_it = rules_.find(country_code);
     assert(rules_it != rules_.end());
@@ skipping 30 matching lines @@
 scoped_ptr<AddressValidator> AddressValidator::Build(
     scoped_ptr<const Downloader> downloader,
     scoped_ptr<Storage> storage,
     LoadRulesDelegate* load_rules_delegate) {
   return scoped_ptr<AddressValidator>(new AddressValidatorImpl(
       downloader.Pass(), storage.Pass(), load_rules_delegate));
 }
 
 }  // namespace addressinput
 }  // namespace i18n