Add base_url to DriveApiUrlGenerator.

This is preparation for injecting server path to the drive api operations.
Its main usage is injecting local test server for unittests of classes in
drive_api_operations.{cc, h} (will be done in following CLs).
To share the testing utility GetBaseUrlForTesting, it is moved into test_util.

BUG=162155
TEST=Ran unit_tests and chromeos_unittests


Review URL: https://chromiumcodereview.appspot.com/11804004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175774 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/drive/drive_api_service.cc

@@ -122,10 +122,12 @@ void ParseAccounetMetadataAndRun(
 
 DriveAPIService::DriveAPIService(
     net::URLRequestContextGetter* url_request_context_getter,
+    const GURL& base_url,
     const std::string& custom_user_agent)
     : url_request_context_getter_(url_request_context_getter),
       profile_(NULL),
       runner_(NULL),
+      url_generator_(base_url),
       custom_user_agent_(custom_user_agent) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }

chrome/browser/chromeos/drive/drive_api_service.h

@@ -39,10 +39,12 @@ class DriveAPIService : public google_apis::DriveServiceInterface,
   // DriveFileSystem.
   //
   // |url_request_context_getter| is used to initialize URLFetcher.
+  // |base_url| is used to generate URLs for communication with the drive API.
   // |custom_user_agent| will be used for the User-Agent header in HTTP
   // requests issues through the service if the value is not empty.
   DriveAPIService(
       net::URLRequestContextGetter* url_request_context_getter,
+      const GURL& base_url,
       const std::string& custom_user_agent);
   virtual ~DriveAPIService();
 

chrome/browser/chromeos/drive/drive_system_service.cc

@@ -124,6 +124,7 @@ DriveSystemService::DriveSystemService(
   } else if (google_apis::util::IsDriveV2ApiEnabled()) {
     drive_service_.reset(new DriveAPIService(
         g_browser_process->system_request_context(),
+        GURL(google_apis::DriveApiUrlGenerator::kBaseUrlForProduction),
         GetDriveUserAgent()));
   } else {
     drive_service_.reset(new google_apis::GDataWapiService(

chrome/browser/google_apis/drive_api_url_generator.cc

@@ -13,18 +13,16 @@ namespace google_apis {
 namespace {
 
 // Hard coded URLs for communication with a google drive server.
-const char kDriveV2AboutUrl[] = "https://www.googleapis.com/drive/v2/about";
-const char kDriveV2ApplistUrl[] = "https://www.googleapis.com/drive/v2/apps";
-const char kDriveV2ChangelistUrl[] =
-    "https://www.googleapis.com/drive/v2/changes";
-
-const char kDriveV2FilelistUrl[] = "https://www.googleapis.com/drive/v2/files";
-const char kDriveV2FileUrlFormat[] =
-    "https://www.googleapis.com/drive/v2/files/%s";
+const char kDriveV2AboutUrl[] = "/drive/v2/about";
+const char kDriveV2ApplistUrl[] = "/drive/v2/apps";
+const char kDriveV2ChangelistUrl[] = "/drive/v2/changes";
+const char kDriveV2FilelistUrl[] = "/drive/v2/files";
+const char kDriveV2FileUrlFormat[] = "/drive/v2/files/%s";
 
 }  // namespace
 
-DriveApiUrlGenerator::DriveApiUrlGenerator() {
+DriveApiUrlGenerator::DriveApiUrlGenerator(const GURL& base_url)
+    : base_url_(base_url) {
   // Do nothing.
 }
 
@@ -32,20 +30,24 @@ DriveApiUrlGenerator::~DriveApiUrlGenerator() {
   // Do nothing.
 }
 
+const char DriveApiUrlGenerator::kBaseUrlForProduction[] =
+    "https://www.googleapis.com";
+
 GURL DriveApiUrlGenerator::GetAboutUrl() const {
-  return GURL(kDriveV2AboutUrl);
+  return base_url_.Resolve(kDriveV2AboutUrl);
 }
 
 GURL DriveApiUrlGenerator::GetApplistUrl() const {
-  return GURL(kDriveV2ApplistUrl);
+  return base_url_.Resolve(kDriveV2ApplistUrl);
 }
 
 GURL DriveApiUrlGenerator::GetChangelistUrl(
     const GURL& override_url, int64 start_changestamp) const {
   // Use override_url if not empty,
-  // otherwise use the default url (kDriveV2Changelisturl).
-  const GURL& url =
-      override_url.is_empty() ? GURL(kDriveV2ChangelistUrl) : override_url;
+  // otherwise use the default url (kDriveV2Changelisturl based on base_url_).
+  const GURL& url = override_url.is_empty() ?
+      base_url_.Resolve(kDriveV2ChangelistUrl) :
+      override_url;
   return start_changestamp ?
       chrome_common_net::AppendOrReplaceQueryParameter(
           url, "startChangeId", base::Int64ToString(start_changestamp)) :
@@ -55,10 +57,10 @@ GURL DriveApiUrlGenerator::GetChangelistUrl(
 GURL DriveApiUrlGenerator::GetFilelistUrl(
     const GURL& override_url, const std::string& search_string) const {
   // Use override_url if not empty,
-  // otherwise use the default url (kDriveV2FilelistUrl).
-  const GURL& url =
-      override_url.is_empty() ? GURL(kDriveV2FilelistUrl) : override_url;
-
+  // otherwise use the default url (kDriveV2FilelistUrl based on base_url_).
+  const GURL& url = override_url.is_empty() ?
+      base_url_.Resolve(kDriveV2FilelistUrl) :
+      override_url;
   return search_string.empty() ?
       url :
       chrome_common_net::AppendOrReplaceQueryParameter(
@@ -66,7 +68,8 @@ GURL DriveApiUrlGenerator::GetFilelistUrl(
 }
 
 GURL DriveApiUrlGenerator::GetFileUrl(const std::string& file_id) const {
-  return GURL(base::StringPrintf(kDriveV2FileUrlFormat, file_id.c_str()));
+  return base_url_.Resolve(
+      base::StringPrintf(kDriveV2FileUrlFormat, file_id.c_str()));
 }
 
 }  // namespace google_apis

chrome/browser/google_apis/drive_api_url_generator.h

@@ -15,11 +15,14 @@ namespace google_apis {
 // servers for production, and a local server for testing.
 class DriveApiUrlGenerator {
  public:
-  // TODO(hidehiko): Pass server name to a constructor in order to inject
-  // server path for testing.
-  DriveApiUrlGenerator();
+  // |base_url| is the path to the target drive api server.
+  // Note that this is an injecting point for a testing server.
+  explicit DriveApiUrlGenerator(const GURL& base_url);
   ~DriveApiUrlGenerator();
 
+  // The base URL for communicating with the production drive api server.
+  static const char kBaseUrlForProduction[];
+
   // Returns a URL to fetch "about" data.
   GURL GetAboutUrl() const;
 
@@ -45,6 +48,9 @@ class DriveApiUrlGenerator {
   // Returns a URL to fecth a file content.
   GURL GetFileUrl(const std::string& file_id) const;
 
+ private:
+  const GURL base_url_;
+
   // This class is copyable hence no DISALLOW_COPY_AND_ASSIGN here.
 };
 

chrome/browser/google_apis/drive_api_url_generator_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/google_apis/drive_api_url_generator.h"
 
+#include "chrome/browser/google_apis/test_util.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -11,23 +12,29 @@ namespace google_apis {
 
 class DriveApiUrlGeneratorTest : public testing::Test {
  public:
-  DriveApiUrlGeneratorTest() {
+  DriveApiUrlGeneratorTest()
+      : url_generator_(GURL(DriveApiUrlGenerator::kBaseUrlForProduction)),
+        test_url_generator_(test_util::GetBaseUrlForTesting(12345)) {
   }
 
  protected:
   DriveApiUrlGenerator url_generator_;
+  DriveApiUrlGenerator test_url_generator_;
 };
 
 // Make sure the hard-coded urls are returned.
-// TODO(hidehiko): More detailed tests when we support server path injecting.
 TEST_F(DriveApiUrlGeneratorTest, GetAboutUrl) {
   EXPECT_EQ("https://www.googleapis.com/drive/v2/about",
             url_generator_.GetAboutUrl().spec());
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/about",
+            test_url_generator_.GetAboutUrl().spec());
 }
 
 TEST_F(DriveApiUrlGeneratorTest, GetApplistUrl) {
   EXPECT_EQ("https://www.googleapis.com/drive/v2/apps",
             url_generator_.GetApplistUrl().spec());
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/apps",
+            test_url_generator_.GetApplistUrl().spec());
 }
 
 TEST_F(DriveApiUrlGeneratorTest, GetChangelistUrl) {
@@ -48,6 +55,14 @@ TEST_F(DriveApiUrlGeneratorTest, GetChangelistUrl) {
   EXPECT_EQ("https://localhost/drive/v2/changes?startChangeId=200",
             url_generator_.GetChangelistUrl(
                 GURL("https://localhost/drive/v2/changes"), 200).spec());
+
+  // For test server, the given base url should be used,
+  // but if |override_url| is given, |override_url| should be used.
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/changes?startChangeId=100",
+            test_url_generator_.GetChangelistUrl(GURL(), 100).spec());
+  EXPECT_EQ("https://localhost/drive/v2/changes?startChangeId=200",
+            test_url_generator_.GetChangelistUrl(
+                GURL("https://localhost/drive/v2/changes"), 200).spec());
 }
 
 TEST_F(DriveApiUrlGeneratorTest, GetFilelistUrl) {
@@ -68,6 +83,14 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilelistUrl) {
   EXPECT_EQ("https://localhost/drive/v2/files?q=query",
             url_generator_.GetFilelistUrl(
                 GURL("https://localhost/drive/v2/files"), "query").spec());
+
+  // For test server, the given base url should be used,
+  // but if |override_url| is given, |override_url| should be used.
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/files?q=query",
+            test_url_generator_.GetFilelistUrl(GURL(), "query").spec());
+  EXPECT_EQ("https://localhost/drive/v2/files?q=query",
+            test_url_generator_.GetFilelistUrl(
+                GURL("https://localhost/drive/v2/files"), "query").spec());
 }
 
 TEST_F(DriveApiUrlGeneratorTest, GetFileUrl) {
@@ -76,6 +99,11 @@ TEST_F(DriveApiUrlGeneratorTest, GetFileUrl) {
             url_generator_.GetFileUrl("0ADK06pfg").spec());
   EXPECT_EQ("https://www.googleapis.com/drive/v2/files/0Bz0bd074",
             url_generator_.GetFileUrl("0Bz0bd074").spec());
+
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/files/0ADK06pfg",
+            test_url_generator_.GetFileUrl("0ADK06pfg").spec());
+  EXPECT_EQ("http://127.0.0.1:12345/drive/v2/files/0Bz0bd074",
+            test_url_generator_.GetFileUrl("0Bz0bd074").spec());
 }
 
 }  // namespace google_apis

chrome/browser/google_apis/gdata_wapi_operations_unittest.cc

@@ -188,7 +188,7 @@ class GDataWapiOperationsTest : public testing::Test {
                    base::Unretained(this)));
 
     url_generator_.reset(new GDataWapiUrlGenerator(
-        GDataWapiUrlGenerator::GetBaseUrlForTesting(test_server_.port())));
+        test_util::GetBaseUrlForTesting(test_server_.port())));
   }
 
   virtual void TearDown() OVERRIDE {

chrome/browser/google_apis/gdata_wapi_url_generator.cc

@@ -55,11 +55,6 @@ const char kGetChangesListURL[] = "/feeds/default/private/changes";
 const char GDataWapiUrlGenerator::kBaseUrlForProduction[] =
     "https://docs.google.com/";
 
-// static
-GURL GDataWapiUrlGenerator::GetBaseUrlForTesting(int port) {
-  return GURL(base::StringPrintf("http://127.0.0.1:%d/", port));
-}
-
 // static
 GURL GDataWapiUrlGenerator::AddStandardUrlParams(const GURL& url) {
   GURL result =

chrome/browser/google_apis/gdata_wapi_url_generator.h

@@ -23,10 +23,6 @@ class GDataWapiUrlGenerator {
   // The base URL for communicating with the WAPI server for production.
   static const char kBaseUrlForProduction[];
 
-  // Gets the base URL for communicating with the local test server for
-  // testing, running at the specified port number.
-  static GURL GetBaseUrlForTesting(int port);
-
   // Adds additional parameters for API version, output content type and to
   // show folders in the feed are added to document feed URLs.
   static GURL AddStandardUrlParams(const GURL& url);

chrome/browser/google_apis/gdata_wapi_url_generator_unittest.cc

@@ -19,11 +19,6 @@ class GDataWapiUrlGeneratorTest : public testing::Test {
   GDataWapiUrlGenerator url_generator_;
 };
 
-TEST_F(GDataWapiUrlGeneratorTest, GetBaseUrlForTesting) {
-  EXPECT_EQ("http://127.0.0.1:12345/",
-            GDataWapiUrlGenerator::GetBaseUrlForTesting(12345).spec());
-}
-
 TEST_F(GDataWapiUrlGeneratorTest, AddStandardUrlParams) {
   EXPECT_EQ("http://www.example.com/?v=3&alt=json",
             GDataWapiUrlGenerator::AddStandardUrlParams(

chrome/browser/google_apis/test_util.cc

@@ -9,11 +9,13 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
+#include "base/stringprintf.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/google_apis/gdata_wapi_parser.h"
 #include "chrome/browser/google_apis/test_server/http_server.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
+#include "googleurl/src/gurl.h"
 
 namespace google_apis {
 namespace test_util {
@@ -47,6 +49,10 @@ FilePath GetTestFilePath(const std::string& relative_path) {
   return path;
 }
 
+GURL GetBaseUrlForTesting(int port) {
+  return GURL(base::StringPrintf("http://127.0.0.1:%d/", port));
+}
+
 void RunBlockingPoolTask() {
   while (true) {
     content::BrowserThread::GetBlockingPool()->FlushForTesting();

chrome/browser/google_apis/test_util.h

@@ -9,6 +9,7 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/google_apis/gdata_errorcode.h"
+#include "googleurl/src/gurl.h"
 
 class FilePath;
 
@@ -41,6 +42,10 @@ void RunBlockingPoolTask();
 // chrome/test/data/chromeos.
 FilePath GetTestFilePath(const std::string& relative_path);
 
+// Returns the base URL for communicating with the local test server for
+// testing, running at the specified port number.
+GURL GetBaseUrlForTesting(int port);
+
 // Loads a test JSON file as a base::Value, from a test file stored under
 // chrome/test/data/chromeos.
 scoped_ptr<base::Value> LoadJSONFile(const std::string& relative_path);