Move directory code from chrome/service to chrome/browser.

Renamed RemotingDirectoryService to DirectoryAddRequest. It will be used in RemotingSetupFlow to register host. Also added unittests for this code.

BUG=67218
TEST=Unittests

Review URL: http://codereview.chromium.org/6036001

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

chrome/service/remoting/remoting_directory_service.cc → chrome/browser/remoting/directory_add_request.cc

@@ -2,45 +2,42 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/remoting/directory_add_request.h"
+
 #include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/values.h"
-#include "chrome/common/guid.h"
+#include "chrome/common/net/http_return.h"
 #include "chrome/common/net/url_request_context_getter.h"
-#include "chrome/service/net/service_url_request_context.h"
-#include "chrome/service/remoting/remoting_directory_service.h"
-#include "net/base/net_util.h"
 #include "net/http/http_request_headers.h"
-#include "remoting/host/host_key_pair.h"
+#include "net/url_request/url_request_status.h"
+
+namespace remoting {
 
 static const char kRemotingDirectoryUrl[] =
     "https://www.googleapis.com/chromoting/v1/@me/hosts";
 
-RemotingDirectoryService::RemotingDirectoryService(Client* client)
-    : client_(client) {
+DirectoryAddRequest::DirectoryAddRequest(URLRequestContextGetter* getter)
+    : getter_(getter) {
 }
 
-RemotingDirectoryService::~RemotingDirectoryService() {
+DirectoryAddRequest::~DirectoryAddRequest() {
   DCHECK(!fetcher_.get()) << "URLFetcher not destroyed.";
 }
 
-void RemotingDirectoryService::AddHost(const std::string& token) {
-  // TODO(hclam): This is a time consuming operation so we should run it on
-  // a separate thread.
-  host_key_pair_.reset(new remoting::HostKeyPair());
-  host_key_pair_->Generate();
-
-  // Get a host name and generate a UUID for the request.
-  host_id_ = guid::GenerateGUID();
-  host_name_ = net::GetHostName();
+void DirectoryAddRequest::AddHost(const remoting::ChromotingHostInfo& host_info,
+                                  const std::string& auth_token,
+                                  DoneCallback* done_callback) {
+  DCHECK(done_callback);
+  done_callback_.reset(done_callback);
 
   // Prepare the parameters for the request.
   DictionaryValue data;
-  data.SetString("hostId", host_id_);
-  data.SetString("hostName", host_name_);
-  data.SetString("publicKey", host_key_pair_->GetPublicKey());
+  data.SetString("hostId", host_info.host_id);
+  data.SetString("hostName", host_info.hostname);
+  data.SetString("publicKey", host_info.public_key);
 
   // Generate the final json query.
   DictionaryValue args;
@@ -50,10 +47,10 @@ void RemotingDirectoryService::AddHost(const std::string& token) {
 
   // Prepare the HTTP header for authentication.
   net::HttpRequestHeaders headers;
-  headers.SetHeader("Authorization", "GoogleLogin auth=" + token);
+  headers.SetHeader("Authorization", "GoogleLogin auth=" + auth_token);
   fetcher_.reset(
       new URLFetcher(GURL(kRemotingDirectoryUrl), URLFetcher::POST, this));
-  fetcher_->set_request_context(new ServiceURLRequestContextGetter());
+  fetcher_->set_request_context(getter_);
   fetcher_->set_upload_data("application/json", request_content);
   fetcher_->set_extra_request_headers(headers.ToString());
 
@@ -61,24 +58,68 @@ void RemotingDirectoryService::AddHost(const std::string& token) {
   fetcher_->Start();
 }
 
-void RemotingDirectoryService::CancelRequest() {
-  fetcher_.reset();
-}
-
-void RemotingDirectoryService::OnURLFetchComplete(
+void DirectoryAddRequest::OnURLFetchComplete(
     const URLFetcher* source,
     const GURL& url,
     const URLRequestStatus& status,
     int response_code,
     const ResponseCookies& cookies,
     const std::string& data) {
+  DCHECK_EQ(source, fetcher_.get());
+
   // Destroy the fetcher after the response has been received.
   fetcher_.reset();
 
-  // TODO(hclam): Simply checking 200 status is not enough.
-  if (response_code == 200) {
-    client_->OnRemotingHostAdded();
+  Result result;
+  std::string error_message;
+
+  if (status.is_success()) {
+    DictionaryValue* response = NULL;
+    scoped_ptr<Value> response_json(base::JSONReader::Read(data, true));
+    if (response_json != NULL &&
+        response_json->IsType(Value::TYPE_DICTIONARY)) {
+      response = static_cast<DictionaryValue*>(response_json.get());
+      response->GetString("error.message", &error_message);
+    }
+
+    switch (response_code) {
+      case RC_REQUEST_OK:
+        result = SUCCESS;
+        break;
+
+      case RC_BAD_REQUEST:
+        // TODO(sergeyu): Implement duplicate error detection that doesn't
+        // depend on error message.
+        if (error_message.find("duplicate") != std::string::npos) {
+          result = ERROR_EXISTS;
+        } else {
+          result = ERROR_INVALID_REQUEST;
+        }
+        break;
+
+      case RC_UNAUTHORIZED:
+        result = ERROR_AUTH;
+        break;
+
+      case RC_INTERNAL_SERVER_ERROR:
+        result = ERROR_SERVER;
+        break;
+
+      default:
+        result = ERROR_OTHER;
+    }
   } else {
-    client_->OnRemotingDirectoryError();
+    result = ERROR_OTHER;
   }
+
+  if (result != SUCCESS) {
+    LOG(WARNING) << "Received error when trying to register Chromoting host. "
+                 << "status.is_success(): " << status.is_success()
+                 << "  response_code: " << response_code
+                 << "  error_message: " << error_message;
+  }
+
+  done_callback_->Run(result, error_message);
 }
+
+}  // namespace remoting

chrome/service/remoting/remoting_directory_service.h → chrome/browser/remoting/directory_add_request.h

@@ -2,44 +2,55 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_SERVICE_REMOTING_REMOTING_DIRECTORY_SERVICE_H_
-#define CHROME_SERVICE_REMOTING_REMOTING_DIRECTORY_SERVICE_H_
+#ifndef CHROME_BROWSER_REMOTING_DIRECTORY_ADD_REQUEST_H_
+#define CHROME_BROWSER_REMOTING_DIRECTORY_ADD_REQUEST_H_
 
 #include <string>
 
+#include "base/callback.h"
 #include "base/scoped_ptr.h"
+#include "chrome/common/remoting/chromoting_host_info.h"
 #include "chrome/common/net/url_fetcher.h"
 #include "googleurl/src/gurl.h"
 
 namespace remoting {
-class HostKeyPair;
-}  // namespace remoting
 
-// A class to provide access to the remoting directory service.
-// TODO(hclam): Should implement this in Javascript.
-class RemotingDirectoryService : public URLFetcher::Delegate {
+// A class implements REST API insert call for the Chromoting directory service.
+class DirectoryAddRequest : public URLFetcher::Delegate {
  public:
-  // Client to receive events from the directory service.
-  class Client {
-   public:
-    virtual ~Client() {}
-
-    // Called when a remoting host was added.
-    virtual void OnRemotingHostAdded() {}
-
-    // Called when the last operation has failed.
-    virtual void OnRemotingDirectoryError() {}
+  enum Result {
+    // Host was added successfully.
+    SUCCESS,
+    // Invalid authentication token.
+    ERROR_AUTH,
+    // Server rejected request because it was invalid (e.g. specified
+    // public key is invalid).
+    ERROR_INVALID_REQUEST,
+    // Host is already registered.
+    ERROR_EXISTS,
+    // Internal server error.
+    ERROR_SERVER,
+    // Timeout expired.
+    ERROR_TIMEOUT_EXPIRED,
+    // Some other error, e.g. network failure.
+    ERROR_OTHER,
   };
 
-  explicit RemotingDirectoryService(Client* client);
-  ~RemotingDirectoryService();
+  // Callback called when request is finished. The second parameter
+  // contains error message in case of an error. The error message may
+  // not be localized, and should be used for logging, but not shown
+  // to the user.
+  typedef Callback2<Result, const std::string&>::Type DoneCallback;
 
-  // Add this computer as host. Use the token for authentication.
-  // TODO(hclam): Need more information for this method call.
-  void AddHost(const std::string& token);
+  explicit DirectoryAddRequest(URLRequestContextGetter* getter);
+  ~DirectoryAddRequest();
 
-  // Cancel the last requested operation.
-  void CancelRequest();
+  // Add this computer as a host. Use the token for
+  // authentication. |done_callback| is called when the request is
+  // finished. Request can be cancelled by destroying this object.
+  void AddHost(const remoting::ChromotingHostInfo& host_info,
+               const std::string& auth_token,
+               DoneCallback* done_callback);
 
   // URLFetcher::Delegate implementation.
   virtual void OnURLFetchComplete(const URLFetcher* source,
@@ -49,24 +60,16 @@ class RemotingDirectoryService : public URLFetcher::Delegate {
                                   const ResponseCookies& cookies,
                                   const std::string& data);
 
-  const std::string& host_id() const { return host_id_; }
-  const std::string& host_name() const { return host_name_; }
-  remoting::HostKeyPair* host_key_pair() const {
-    return host_key_pair_.get();
-  }
-
  private:
-  Client* client_;
-  scoped_ptr<URLFetcher> fetcher_;
+  friend class DirectoryAddRequestTest;
 
-  // Host key generated during host registration.
-  scoped_ptr<remoting::HostKeyPair> host_key_pair_;
-
-  // Host info used for registration.
-  std::string host_id_;
-  std::string host_name_;
+  URLRequestContextGetter* getter_;
+  scoped_ptr<DoneCallback> done_callback_;
+  scoped_ptr<URLFetcher> fetcher_;
 
-  DISALLOW_COPY_AND_ASSIGN(RemotingDirectoryService);
+  DISALLOW_COPY_AND_ASSIGN(DirectoryAddRequest);
 };
 
-#endif  // CHROME_SERVICE_REMOTING_REMOTING_DIRECTORY_SERVICE_H_
+}  // namespace remoting
+
+#endif  // CHROME_BROWSER_REMOTING_DIRECTORY_ADD_REQUEST_H_

chrome/browser/remoting/directory_add_request_unittest.cc

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/remoting/directory_add_request.h"
+#include "chrome/test/testing_profile.h"
+#include "net/url_request/url_request_status.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+namespace {
+class MockDoneCallback {
+ public:
+  MOCK_METHOD2(OnDone,
+               void(DirectoryAddRequest::Result,
+                    const std::string& message));
+};
+}  // namespace
+
+class DirectoryAddRequestTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    target_.reset(new DirectoryAddRequest(profile_.GetRequestContext()));
+  }
+
+  void TestResult(int response_code, const std::string& data,
+                  DirectoryAddRequest::Result expected_result,
+                  const std::string& expected_message) {
+    MockDoneCallback callback;
+    EXPECT_CALL(callback, OnDone(expected_result, expected_message))
+        .Times(1);
+
+    target_->done_callback_.reset(
+        NewCallback(&callback, &MockDoneCallback::OnDone));
+
+    GURL url;
+    URLRequestStatus status_ok;
+    ResponseCookies cookies;
+    target_->OnURLFetchComplete(NULL, url, status_ok, response_code,
+                               cookies, data);
+  }
+
+  TestingProfile profile_;
+  scoped_ptr<DirectoryAddRequest> target_;
+};
+
+TEST_F(DirectoryAddRequestTest, Success) {
+  TestResult(200, "{\"data\":{\"kind\":\"chromoting#host\","
+             "\"hostId\":\"e64906c9-fdc9-4921-80cb-563cf7f564f3\","
+             "\"hostName\":\"host_name\",\"publicKey\":\"PUBLIC+KEY\"}}",
+             DirectoryAddRequest::SUCCESS, "");
+}
+
+TEST_F(DirectoryAddRequestTest, Duplicate) {
+  TestResult(400, "{\"error\":{\"errors\":[{\"domain\":\"global\","
+             "\"reason\":\"invalid\",\"message\":\"Attempt to register "
+             "a duplicate host.\"}],\"code\":400,\"message\":\"Attempt to "
+             "register a duplicate host.\"}}",
+             DirectoryAddRequest::ERROR_EXISTS,
+             "Attempt to register a duplicate host.");
+}
+
+TEST_F(DirectoryAddRequestTest, InvalidRequest) {
+  TestResult(400, "{\"error\":{\"errors\":[{\"domain\":\"global\","
+             "\"reason\":\"invalid\",\"message\":\"Invalid Value\"}],"
+             "\"code\":400,\"message\":\"Invalid Value\"}}",
+             DirectoryAddRequest::ERROR_INVALID_REQUEST,
+             "Invalid Value");
+}
+
+TEST_F(DirectoryAddRequestTest, InvalidToken) {
+  TestResult(401, "{\"error\":{\"errors\":[{\"domain\":\"global\","
+             "\"reason\":\"invalid\",\"message\":\"Token invalid\","
+             "\"locationType\":\"header\",\"location\":\"Authorization\"}],"
+             "\"code\":401,\"message\":\"Token invalid\"}}",
+             DirectoryAddRequest::ERROR_AUTH,
+             "Token invalid");
+}
+
+}  // namespace remoting

chrome/chrome_browser.gypi

@@ -2106,6 +2106,8 @@
         'browser/profiles/profile_impl.h',
         'browser/profiles/profile_manager.cc',
         'browser/profiles/profile_manager.h',
+        'browser/remoting/directory_add_request.cc',
+        'browser/remoting/directory_add_request.h',
         'browser/remoting/remoting_resources_source.cc',
         'browser/remoting/remoting_resources_source.h',
         'browser/remoting/remoting_setup_flow.cc',

chrome/chrome_tests.gypi

@@ -1293,6 +1293,7 @@
         'browser/process_info_snapshot_mac_unittest.cc',
         'browser/process_singleton_mac_unittest.cc',
         'browser/profiles/profile_manager_unittest.cc',
+        'browser/remoting/directory_add_request_unittest.cc',
         'browser/renderer_host/audio_renderer_host_unittest.cc',
         'browser/renderer_host/gtk_im_context_wrapper_unittest.cc',
         'browser/renderer_host/gtk_key_bindings_handler_unittest.cc',

chrome/common/net/http_return.h

@@ -10,9 +10,10 @@
 // contains a few HTTP return codes. Add more HTTP return codes.
 enum HTTPReturnCode {
   RC_REQUEST_OK = 200,
+  RC_BAD_REQUEST = 400,
   RC_UNAUTHORIZED = 401,
   RC_FORBIDDEN = 403,
+  RC_INTERNAL_SERVER_ERROR = 500,
 };
 
 #endif  // CHROME_COMMON_NET_HTTP_RETURN_H_
-