Using API key specified from js

components/copresence/rpc/http_post.h

 // Copyright 2014 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 COMPONENTS_COPRESENCE_RPC_HTTP_POST_H_
 #define COMPONENTS_COPRESENCE_RPC_HTTP_POST_H_
 
 #include <string>
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "net/url_request/url_fetcher_delegate.h"
+#include "url/gurl.h"
 
 namespace google {
 namespace protobuf {
 class MessageLite;
 }
 }
 
 namespace net {
 class URLRequestContextGetter;
 }
 
 namespace copresence {
 
 // This class handles all Apiary calls to the Copresence server.
 // It configures the HTTP request appropriately and reports any errors.
 // If deleted, the HTTP request is cancelled.
 //
 // TODO(ckehoe): Add retry logic.
 class HttpPost : public net::URLFetcherDelegate {
  public:
   // Callback to receive the HTTP status code and body of the response
   // (if any). A pointer to this HttpPost object is also passed along.
   typedef base::Callback<void(int, const std::string&)>
       ResponseCallback;
 
-  // An id for testing url fetching.
-  static const int kUrlFetcherId = 1;
-
-  // The query parameter to send Apiary tracing tokens.
-  static const char kTracingTokenField[];
-
-  // Create a request to the Copresence server. |url_context_getter|
-  // is owned by the caller, and the context it provides must be available
-  // until the request completes.
+  // Create a request to the Copresence server.
+  // |url_context_getter| is owned by the caller,
+  // and the context it provides must be available until the request completes.
   HttpPost(net::URLRequestContextGetter* url_context_getter,
            const std::string& server_host,
-           const std::string& rpc_name,
-           const std::string& tracing_token,

[rkc, 2014/08/13 21:45:31]

Why is this taken out of the constructor?

[Charlie, 2014/08/13 22:53:37]

All parameters to the constructor are now required. If clients want to add a
tracing token, they should use a separate mutator.

The request proto has been taken out as part of the refactoring, at xiyuan's
suggestion. I can put it back in, but I don't think it's needed.

[rkc, 2014/08/13 23:11:36]

I am not happy about using mutators instead of passing arguments through the
constructor. In general, the internal details of a class should be abstracted
out and anything it needs for its operation should be given to the constructor.

In case this class decides to not save the API key but rather to simply setup
the URL Request in the constructor, now any code that is setting the API key
would now be non-functional. The same argument goes for the tracing token.

In fact, a majority of the implementation had to be moved to the Start method
because we actually now need this space between construction and 'real'
construction where a consumer of this class can set these values.

We do an empty string check on both values anyway (the consumer can just pass in
an empty string to the constructor for those values). I guess I am seeing cons
but I am not seeing any pros of having this as a mutator.

[xiyuan, 2014/08/13 23:15:59]

If we decided to add back |request_proto| to ctor, I would prefer to extract
info out of it instead of storing the pointer, e.g. store the serialized request
data.

-           const google::protobuf::MessageLite& request_proto);
+           const std::string& rpc_name);
 
   // HTTP requests are cancelled on delete.
   virtual ~HttpPost();
 
+  // Add an Apiary tracing token to the request.
+  void set_tracing_token(const std::string& tracing_token);
+
+  // Add an API key to the request. If none is specified,
+  // we attempt to use Chrome's key.
+  void set_api_key(const std::string& api_key) { api_key_ = api_key; }
+
   // Send an HttpPost request.
-  void Start(const ResponseCallback& response_callback);
+  void Start(const ResponseCallback& response_callback,
+             const google::protobuf::MessageLite& request_proto);
 
  private:
+  static const int kUrlFetcherId = 1;
+  static const char kApiKeyField[];
+  static const char kTracingTokenField[];
+
+  friend class HttpPostTest;
+
   // Overridden from net::URLFetcherDelegate.
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
+  net::URLRequestContextGetter* url_context_getter_;
+  GURL url_;
+
   ResponseCallback response_callback_;
+
   scoped_ptr<net::URLFetcher> url_fetcher_;
+  std::string api_key_;
 
   DISALLOW_COPY_AND_ASSIGN(HttpPost);
 };
 
 }  // namespace copresence
 
 #endif  // COMPONENTS_COPRESENCE_RPC_HTTP_POST_H_