HTTP server for testing Google Drive.

chrome/browser/chromeos/drive/test_servers/http_server.h

+// Copyright (c) 2012 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_DRIVE_TEST_SERVERS_HTTP_SERVER_H_
+#define CHROME_BROWSER_CHROMEOS_DRIVE_TEST_SERVERS_HTTP_SERVER_H_
+
+#include <string>
+#include <vector>
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "chrome/browser/chromeos/drive/test_servers/http_connection.h"
+#include "chrome/browser/chromeos/drive/test_servers/http_response.h"
+#include "chrome/browser/google_apis/gdata_test_util.h"
+#include "net/base/tcp_listen_socket.h"
+
+namespace drive {
+namespace test_servers {
+
+// This class is required to be able to have composition instead of inheritance,
+class HttpListenSocket: public net::TCPListenSocket {
+ public:
+  HttpListenSocket(const SocketDescriptor socket_descriptor,
+                   net::StreamListenSocket::Delegate* delegate);
+  virtual void Listen();
+
+ private:
+  virtual ~HttpListenSocket();
+};
+
+// Class providing a HTTP server for testing purpose. This is a basic server
+// providing only an essential subset of HTTP/1.1 protocol. Especially,
+// it assumes that the request syntax is correct. It *does not* support
+// a Chunked Transfer Encoding.
+//
+// The common use case is below:
+//
+// scoped_ptr<HttpServer> test_server_;
+// GURL hello_world_url_;
+// GURL file_url_;
+// (...)
+// void SetUp() {
+//   test_server_.reset(new HttpServer());
+//   hello_world_url = test_server->RegisterSimpleTextResponse(
+//       "<b>Hello world!</b>",
+//       "text/html");
+//   metadata_url = test_server->RegisterFileResponse(
+//       "metadata/file.doc")
+//       "testing/data/metadata.xml",
+//       "text/xml",
+//       200);
+// }
+class HttpServer : private net::StreamListenSocket::Delegate {
+ public:
+  typedef base::Callback<void(HttpServer* server)> CreateCallback;
+  typedef base::Callback<scoped_ptr<HttpResponse>(const HttpRequest* request)>
+      HandleRequestCallback;
+
+  ~HttpServer();
+
+  // Factory method. Created a server instance and returns a pointer or NULL in
+  // case of failure. Can be run from any thread. It is blocking.
+  static scoped_ptr<HttpServer> CreateForTesting();
+
+  // Factory method. Created a server instance and returns a pointer to it
+  // through the |callback|. Returns NULL in case of failure. Must be called
+  // from IO thread.
+  static void CreateOnIOThread(const CreateCallback& callback);
+
+  // Provides URL to the server which is useful when general purpose provider
+  // is registered.
+  GURL GetBaseURL();
+
+  // The most general purpose method. Any request processing can be added using
+  // this method. Takes ownership of the object. The |callback| is called
+  // on UI thread.
+  void RegisterRequestHandler(const HandleRequestCallback& callback);

[satorux1, 2012/11/13 05:28:34]

Hmm, I think this function should take |relative_path|. 

[mtomasz, 2012/11/13 12:23:07]

Callback decides if it will handle that request. I believe it is way more
flexible. I think we are going to use this method only if we want some very
special behavior, eg. depending on the url. If we set a relative path here, then
why would we use this method?

[satorux1, 2012/11/14 01:34:32]

I think it's good to make it clear what code handles a particular path. With
this API, it's hard to find out what code processes a particular path.

With my approach, you can replace the loop in HandleRequest() with a table like:

  iter = method_table_.find(path);
  if (iter != method_table.end()) {
    iter->second.Run(request);
  }

[mtomasz, 2012/11/14 03:23:35]

I agree that your idea is simpler. But I don't think mine is so bad :), it is
just common chain of responsibility pattern. One of requests in this crbug was
to make it extensible using eg. regexps. If we allow callbacks to decide if they
want to accept a request, then they can do regex stuff on the url. I think we
will need it. In the drive v2 api urls are built using file id, change id, etc,
and with the current approach we can do it with one handler, not with separate
one for each file id/change id:
http://developers.google.com/drive/v2/reference/.

However, I will change it if you want.

+
+  // Used to provide the same predefined response for the requests matching
+  // the |relative_path|. Should be used if any custom data, such as additional
+  // headers should be send from the server.
+  GURL RegisterDefaultResponse(
+      const std::string& relative_path,
+      const HttpResponse& default_response);
+
+  // Registers a simple text response.
+  GURL RegisterTextResponse(
+      const std::string& relative_path,
+      const std::string& content,
+      const std::string& content_type,
+      const ResponseCode response_code);

[satorux1, 2012/11/13 05:13:18]

let's add a blank line.

[mtomasz, 2012/11/13 12:23:07]

Done.

+  // Registers a simple text response with automatically generated unique url.
+  // The response code is always 200.
+  GURL RegisterSimpleTextResponse(const std::string& content,

[satorux1, 2012/11/13 05:28:34]

I think the "simple" version is unnecessary. I'd rather want to see less public
functions. 

Besides, we can add it later if we want.

[mtomasz, 2012/11/13 12:23:07]

The reason was that in most cases (I think in all current tests) we don't need a
specific relative_path nor any custom response_code. Deleted. Done.

+                            const std::string& content_type);

[satorux1, 2012/11/13 05:13:18]

indentation is off.

[mtomasz, 2012/11/13 12:23:07]

Done.

+
+  // Registers a simple file response. The file is loaded into memory.
+  GURL RegisterFileResponse(
+      const std::string& relative_path,
+      const FilePath& file_path,
+      const std::string& content_type,
+      const ResponseCode response_code);

[satorux1, 2012/11/13 05:13:18]

blank line.

[mtomasz, 2012/11/13 12:23:07]

Done.

+  // Registers a simple file response with automatically generated unique url.
+  // The response code is always 200.
+  GURL RegisterSimpleFileResponse(

[satorux1, 2012/11/13 05:28:34]

Likewise, I think we don't need the simple version.

[mtomasz, 2012/11/13 12:23:07]

Done.

+      const FilePath& file_path,
+      const std::string& content_type);
+
+ private:
+  // Creates and starts server and attaches it to |socket_descriptor| socket.
+  // Passed |port| should be a valid port name, on which the socket is
+  // listening. Use factory methods to create an instance of the server.
+  HttpServer(int port, const SocketDescriptor& socket_descriptor);
+
+  // Handles a request when it is parsed. It passes the request to registed
+  // request handlers and sends a http response.
+  void HandleRequest(HttpConnection* connection,
+                     scoped_ptr<HttpRequest> request);
+
+  // Generates unique identifier to make unique URL addresses.
+  std::string GenerateUniqueIdentifier();

[satorux1, 2012/11/13 05:28:34]

We can remove this if we get rid of the simple versions.

[mtomasz, 2012/11/13 12:23:07]

Done.

+
+  // |server| is the original listening Socket, connection is the new
+  // Socket that was created.  Ownership of |connection| is transferred
+  // to the delegate with this call.
+  void DidAccept(net::StreamListenSocket* server,
+                         net::StreamListenSocket* connection) OVERRIDE;
+  void DidRead(net::StreamListenSocket* connection,
+                       const char* data,
+                       int length) OVERRIDE;
+  void DidClose(net::StreamListenSocket* connection) OVERRIDE;
+
+  HttpConnection* FindConnection(net::StreamListenSocket* socket);
+
+  scoped_refptr<HttpListenSocket> listen_socket_;
+  int port_;
+  GURL base_url_;
+  int last_unique_id_;
+
+  // Owns the HttpConnection objects.
+  std::map<net::StreamListenSocket*, HttpConnection*> connections_;
+
+  // Vector of registered request handlers.
+  std::vector<HandleRequestCallback> request_handlers_;
+
+  base::WeakPtrFactory<HttpServer> weak_factory_;

[satorux1, 2012/11/13 05:28:34]

weak_ptr_factory_

Please also add:

  // Note: This should remain the last member so it'll be destroyed and         
  // invalidate its weak pointers before any other members are destroyed.       

[mtomasz, 2012/11/13 12:23:07]

Done.

+  DISALLOW_COPY_AND_ASSIGN(HttpServer);
+};
+
+}  // namespace test_servers
+}  // namespace drive
+
+#endif  // CHROME_BROWSER_CHROMEOS_DRIVE_TEST_SERVERS_HTTP_SERVER_H_