Break out abstract base class Supplier from MetadataLoader.

To make it possible to supply the AddressValidator with address metadata
in several different way, an interface for such a Supplier is defined
and the existing MetadataLoader is refactored to implement it.

This will be used to implement another Supplier that doesn't load any
data on demand, but only works with preloaded data.

R=rouslan@chromium.org
BUG=

Review URL: https://codereview.appspot.com/97650044

git-svn-id: http://libaddressinput.googlecode.com/svn/trunk@235 38ededc0-08b8-5190-f2ac-b31f878777ad

3 files changed, 36 insertions, 52 deletions

diff --git a/cpp/test/metadata_loader_test.cc b/cpp/test/metadata_loader_test.cc
index 3c72d17..75e4444 100644
--- a/cpp/test/metadata_loader_test.cc
+++ b/cpp/test/metadata_loader_test.cc
@@ -18,6 +18,7 @@
 #include <libaddressinput/callback.h>
 #include <libaddressinput/downloader.h>
 #include <libaddressinput/null_storage.h>
+#include <libaddressinput/supplier.h>
 #include <libaddressinput/util/basictypes.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
@@ -63,6 +64,7 @@ using i18n::addressinput::NullStorage;
 using i18n::addressinput::Retriever;
 using i18n::addressinput::Rule;
 using i18n::addressinput::scoped_ptr;
+using i18n::addressinput::Supplier;
 
 class MetadataLoaderTest : public testing::Test {
  protected:
@@ -75,13 +77,13 @@ class MetadataLoaderTest : public testing::Test {
             new Retriever(FakeDownloader::kFakeDataUrl,
                           new FakeDownloader,
                           new NullStorage)),
-        loaded_(BuildCallback(this, &MetadataLoaderTest::Loaded)) {}
+        supplied_(BuildCallback(this, &MetadataLoaderTest::Supplied)) {}
 
   virtual ~MetadataLoaderTest() {}
 
-  void Load() {
+  void Supply() {
     lookup_key_.FromAddress(address_);
-    loader_.Load(lookup_key_, *loaded_);
+    loader_.Supply(lookup_key_, *supplied_);
   }
 
   AddressData address_;
@@ -89,9 +91,9 @@ class MetadataLoaderTest : public testing::Test {
   bool called_;
 
  private:
-  void Loaded(bool success,
-              const LookupKey& lookup_key,
-              const MetadataLoader::RuleHierarchy& hierarchy) {
+  void Supplied(bool success,
+                const LookupKey& lookup_key,
+                const Supplier::RuleHierarchy& hierarchy) {
     ASSERT_TRUE(success);
     ASSERT_EQ(&lookup_key_, &lookup_key);
     std::memcpy(rule_, hierarchy.rule_, sizeof rule_);
@@ -100,7 +102,7 @@ class MetadataLoaderTest : public testing::Test {
 
   LookupKey lookup_key_;
   MetadataLoader loader_;
-  const scoped_ptr<const MetadataLoader::Callback> loaded_;
+  const scoped_ptr<Supplier::Callback> supplied_;
 
   DISALLOW_COPY_AND_ASSIGN(MetadataLoaderTest);
 };
@@ -108,7 +110,7 @@ class MetadataLoaderTest : public testing::Test {
 TEST_F(MetadataLoaderTest, Invalid) {
   address_.region_code = "QZ";
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] == NULL);
   EXPECT_TRUE(rule_[1] == NULL);
@@ -119,7 +121,7 @@ TEST_F(MetadataLoaderTest, Invalid) {
 TEST_F(MetadataLoaderTest, Valid) {
   address_.region_code = "SE";
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] == NULL);
@@ -135,7 +137,7 @@ TEST_F(MetadataLoaderTest, KeyDepthEqualsMaxDepth) {
   address_.region_code = "HK";
   address_.administrative_area = kKowloon;
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -148,7 +150,7 @@ TEST_F(MetadataLoaderTest, KeyDepthLargerThanMaxDepth) {
   address_.administrative_area = kKowloon;
   address_.locality = "bbb";  // Ignored!
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -159,7 +161,7 @@ TEST_F(MetadataLoaderTest, KeyDepthLargerThanMaxDepth) {
 TEST_F(MetadataLoaderTest, KeyDepthSmallerThanMaxDepth) {
   address_.region_code = "HK";
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] == NULL);
@@ -170,7 +172,7 @@ TEST_F(MetadataLoaderTest, KeyDepthSmallerThanMaxDepth) {
 TEST_F(MetadataLoaderTest, KeyDepth0) {
   address_.region_code = "CN";
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] == NULL);
@@ -182,7 +184,7 @@ TEST_F(MetadataLoaderTest, KeyDepth1) {
   address_.region_code = "CN";
   address_.administrative_area = kXinJiang;
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -195,7 +197,7 @@ TEST_F(MetadataLoaderTest, KeyDepth2) {
   address_.administrative_area = kXinJiang;
   address_.locality = kKashiDiqu;
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -209,7 +211,7 @@ TEST_F(MetadataLoaderTest, KeyDepth3) {
   address_.locality = kKashiDiqu;
   address_.dependent_locality = kKashiShi;
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -221,7 +223,7 @@ TEST_F(MetadataLoaderTest, RuleCache) {
   address_.region_code = "US";
   address_.administrative_area = "CA";
 
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_TRUE(rule_[0] != NULL);
   EXPECT_TRUE(rule_[1] != NULL);
@@ -229,15 +231,15 @@ TEST_F(MetadataLoaderTest, RuleCache) {
   EXPECT_TRUE(rule_[3] == NULL);
 
   // Make a copy of the currently returned pointers to the Rule objects (stored
-  // in the MetadataLoader cache) and verify that calling Load() again with the
-  // same LookupKey returns the same pointers again (and doesn't create any new
-  // Rule objects instead).
+  // in the MetadataLoader cache) and verify that calling Supply() again with
+  // the same LookupKey returns the same pointers again (and doesn't create any
+  // new Rule objects instead).
 
   const Rule* rule[arraysize(LookupKey::kHierarchy)];
   std::memcpy(rule, rule_, sizeof rule);
 
   called_ = false;
-  ASSERT_NO_FATAL_FAILURE(Load());
+  ASSERT_NO_FATAL_FAILURE(Supply());
   ASSERT_TRUE(called_);
   EXPECT_EQ(rule[0], rule_[0]);
   EXPECT_EQ(rule[1], rule_[1]);
@@ -275,7 +277,7 @@ class RuleHierarchyTest : public testing::Test {
  private:
   std::map<std::string, const Rule*> rule_cache_;
   const scoped_ptr<Retriever> retriever_;
-  const scoped_ptr<const MetadataLoader::Callback> loaded_;
+  const scoped_ptr<const Supplier::Callback> supplied_;
 
  protected:
   RuleHierarchyTest()
@@ -284,14 +286,14 @@ class RuleHierarchyTest : public testing::Test {
         retriever_(
             new Retriever(
                 MockDownloader::kMockDataUrl, downloader_, new NullStorage)),
-        loaded_(BuildCallback(this, &RuleHierarchyTest::Loaded)),
+        supplied_(BuildCallback(this, &RuleHierarchyTest::Supplied)),
         success_(true),
         lookup_key_(),
         rule_(),
         called_(false),
         hierarchy_(
             new MetadataLoader::RuleHierarchy(
-                lookup_key_, &rule_cache_, *loaded_)) {}
+                lookup_key_, &rule_cache_, *supplied_)) {}
 
   virtual ~RuleHierarchyTest() {
     for (std::map<std::string, const Rule*>::const_iterator
@@ -314,9 +316,9 @@ class RuleHierarchyTest : public testing::Test {
   bool called_;
 
  private:
-  void Loaded(bool success,
-              const LookupKey& lookup_key,
-              const MetadataLoader::RuleHierarchy& hierarchy) {
+  void Supplied(bool success,
+                const LookupKey& lookup_key,
+                const Supplier::RuleHierarchy& hierarchy) {
     ASSERT_EQ(success_, success);
     ASSERT_EQ(&lookup_key_, &lookup_key);
     ASSERT_EQ(hierarchy_, &hierarchy);
diff --git a/cpp/test/metadata_query_task_test.cc b/cpp/test/metadata_query_task_test.cc
index c568835..60efa80 100644
--- a/cpp/test/metadata_query_task_test.cc
+++ b/cpp/test/metadata_query_task_test.cc
@@ -48,7 +48,7 @@ class MetadataQueryTaskTest : public testing::Test {
     virtual ~MockTask() {}
 
    protected:
-    virtual bool Query(const MetadataLoader::RuleHierarchy& hierarchy) const {
+    virtual bool Query(const Supplier::RuleHierarchy& hierarchy) const {
       return answer_;
     }
 
@@ -64,24 +64,20 @@ class MetadataQueryTaskTest : public testing::Test {
         region_code_(),
         answer_(),
         called_(false),
-        loaded_(BuildCallback(this, &MetadataQueryTaskTest::Loaded)),
         answered_(BuildCallback(this, &MetadataQueryTaskTest::Answered)) {}
 
   virtual ~MetadataQueryTaskTest() {}
 
   void Run() {
-    std::map<std::string, const Rule*> rules;  // Stub.
-
     MetadataQueryTask* task = new MockTask(answer_, region_code_, *answered_);
 
-    MetadataLoader::RuleHierarchy* hierarchy =
-        new MetadataLoader::RuleHierarchy(*task->lookup_key_, &rules, *loaded_);
+    Supplier::RuleHierarchy* hierarchy = new Supplier::RuleHierarchy();
 
     if (use_default_rule_) {
       hierarchy->rule_[0] = &Rule::GetDefault();
     }
 
-    (*task->loaded_)(success_, *task->lookup_key_, *hierarchy);
+    (*task->supplied_)(success_, *task->lookup_key_, *hierarchy);
   }
 
   bool use_default_rule_;
@@ -91,10 +87,6 @@ class MetadataQueryTaskTest : public testing::Test {
   bool called_;
 
  private:
-  void Loaded(bool, const LookupKey&, const MetadataLoader::RuleHierarchy&) {
-    FAIL();  // RuleHierarchy::Retrieve() shouldn't be called in this test.
-  }
-
   void Answered(bool success,
                 const std::string& region_code,
                 const bool& answer) {
@@ -104,7 +96,6 @@ class MetadataQueryTaskTest : public testing::Test {
     called_ = true;
   }
 
-  const scoped_ptr<const MetadataLoader::Callback> loaded_;  // Stub.
   const scoped_ptr<const AddressValidator::BoolCallback> answered_;
 
   DISALLOW_COPY_AND_ASSIGN(MetadataQueryTaskTest);
diff --git a/cpp/test/validation_task_test.cc b/cpp/test/validation_task_test.cc
index 6bf2aa3..e0ae009 100644
--- a/cpp/test/validation_task_test.cc
+++ b/cpp/test/validation_task_test.cc
@@ -19,17 +19,16 @@
 #include <libaddressinput/address_problem.h>
 #include <libaddressinput/address_validator.h>
 #include <libaddressinput/callback.h>
+#include <libaddressinput/supplier.h>
 #include <libaddressinput/util/basictypes.h>
 #include <libaddressinput/util/scoped_ptr.h>
 
 #include <cstddef>
-#include <map>
-#include <string>
 #include <utility>
 
 #include <gtest/gtest.h>
 
-#include "metadata_loader.h"
+#include "lookup_key.h"
 #include "rule.h"
 
 namespace i18n {
@@ -49,14 +48,12 @@ class ValidationTaskTest : public testing::Test {
         problems_(),
         expected_(),
         called_(false),
-        loaded_(BuildCallback(this, &ValidationTaskTest::Loaded)),
         validated_(BuildCallback(this, &ValidationTaskTest::Validated)) {}
 
   virtual ~ValidationTaskTest() {}
 
   void Validate() {
     Rule rule[arraysize(json_)];
-    std::map<std::string, const Rule*> rules;  // Stub.
 
     ValidationTask* task = new ValidationTask(
         address_,
@@ -66,15 +63,14 @@ class ValidationTaskTest : public testing::Test {
         &problems_,
         *validated_);
 
-    MetadataLoader::RuleHierarchy* hierarchy =
-        new MetadataLoader::RuleHierarchy(*task->lookup_key_, &rules, *loaded_);
+    Supplier::RuleHierarchy* hierarchy = new Supplier::RuleHierarchy();
 
     for (size_t i = 0; i < arraysize(json_) && json_[i] != NULL; ++i) {
       ASSERT_TRUE(rule[i].ParseSerializedRule(json_[i]));
       hierarchy->rule_[i] = &rule[i];
     }
 
-    (*task->loaded_)(success_, *task->lookup_key_, *hierarchy);
+    (*task->supplied_)(success_, *task->lookup_key_, *hierarchy);
   }
 
   const char* json_[arraysize(LookupKey::kHierarchy)];
@@ -88,10 +84,6 @@ class ValidationTaskTest : public testing::Test {
   bool called_;
 
  private:
-  void Loaded(bool, const LookupKey&, const MetadataLoader::RuleHierarchy&) {
-    FAIL();  // RuleHierarchy::Retrieve() shouldn't be called in this test.
-  }
-
   void Validated(bool success,
                  const AddressData& address,
                  const FieldProblemMap& problems) {
@@ -101,7 +93,6 @@ class ValidationTaskTest : public testing::Test {
     called_ = true;
   }
 
-  const scoped_ptr<const MetadataLoader::Callback> loaded_;  // Stub.
   const scoped_ptr<const AddressValidator::Callback> validated_;
 
   DISALLOW_COPY_AND_ASSIGN(ValidationTaskTest);