DnsClient refactoring + features (timeout, suffix search, server rotation).

net/dns/async_host_resolver_unittest.cc

 // Copyright (c) 2011 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 "net/dns/async_host_resolver.h"
 
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "net/base/host_cache.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/sys_addrinfo.h"
 #include "net/base/test_completion_callback.h"
-#include "net/dns/dns_client.h"
 #include "net/dns/dns_query.h"
 #include "net/dns/dns_response.h"
 #include "net/dns/dns_test_util.h"
+#include "net/dns/dns_transaction.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
 
 namespace {
 
 const int kPortNum = 80;
 const size_t kMaxTransactions = 2;
 const size_t kMaxPendingRequests = 1;
 
@@ skipping 11 matching lines @@
     EXPECT_EQ(sizeof(struct sockaddr_in), ainfo->ai_addrlen);
 
     const struct sockaddr* sa = ainfo->ai_addr;
     const struct sockaddr_in* sa_in = (const struct sockaddr_in*) sa;
     EXPECT_TRUE(htons(port) == sa_in->sin_port);
     EXPECT_STREQ(*i, NetAddressToString(sa, ainfo->ai_addrlen).c_str());
   }
   ASSERT_EQ(static_cast<addrinfo*>(NULL), ainfo);
 }
 
-class MockDnsClient : public DnsClient,
-                      public base::SupportsWeakPtr<MockDnsClient> {
+class MockDnsTransactionFactory : public DnsTransactionFactory,
+                      public base::SupportsWeakPtr<MockDnsTransactionFactory> {

[cbentzel, 2012/01/21 03:41:16]

Nit: maybe just indent by 4 from line start here. 

  public:
   // Using WeakPtr to support cancellation.
   // All MockRequests succeed unless canceled or MockDnsClient is destroyed.
-  class MockRequest : public DnsClient::Request,
+  class MockRequest : public DnsTransaction,

[mmenke, 2012/01/24 18:27:01]

nit:  Might want to call this MockTransaction.

                       public base::SupportsWeakPtr<MockRequest> {
    public:
-    MockRequest(const base::StringPiece& qname,
+    MockRequest(const std::string& hostname,
                 uint16 qtype,
-                const RequestCallback& callback,
-                const base::WeakPtr<MockDnsClient>& client)
-        : Request(qname, qtype, callback), started_(false), client_(client) {
-    }
-
-    virtual int Start() OVERRIDE {
+                const DnsTransactionFactory::CallbackType& callback,
+                const base::WeakPtr<MockDnsTransactionFactory>& factory)
+        : hostname_(hostname),
+          qtype_(qtype),
+          callback_(callback),
+          started_(false),
+          factory_(factory) {
       EXPECT_FALSE(started_);
       started_ = true;
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&MockRequest::Finish, AsWeakPtr()));
+    }
+
+    virtual const std::string& GetHostname() const OVERRIDE {
+      return hostname_;
+    }
+
+    virtual uint16 GetType() const OVERRIDE {
+      return qtype_;
+    }
+
+    virtual int Start() OVERRIDE {
       return ERR_IO_PENDING;
     }
 
    private:
     void Finish() {
-      if (!client_) {
-        DoCallback(ERR_DNS_SERVER_FAILED, NULL);
+      if (!factory_) {
+        callback_.Run(this, ERR_DNS_SERVER_FAILED, NULL);
         return;
       }
-      DoCallback(OK, client_->responses[Key(qname(), qtype())]);
+      callback_.Run(this,
+                    OK,
+                    factory_->responses[Key(GetHostname(), GetType())]);
     }
 
+    const std::string hostname_;
+    const uint16 qtype_;
+    DnsTransactionFactory::CallbackType callback_;
     bool started_;
-    base::WeakPtr<MockDnsClient> client_;
+    const base::WeakPtr<MockDnsTransactionFactory> factory_;
   };
 
   typedef std::pair<std::string, uint16> Key;
 
-  MockDnsClient() : num_requests(0) {}
-  ~MockDnsClient() {
+  MockDnsTransactionFactory() : num_requests(0) {}
+  ~MockDnsTransactionFactory() {
     STLDeleteValues(&responses);
   }
 
-  Request* CreateRequest(const base::StringPiece& qname,
-                         uint16 qtype,
-                         const RequestCallback& callback,
-                         const BoundNetLog&) {
+  scoped_ptr<DnsTransaction> CreateTransaction(
+      const std::string& qname,
+      uint16 qtype,
+      const DnsTransactionFactory::CallbackType& callback,
+      const BoundNetLog&) {
     ++num_requests;
-    return new MockRequest(qname, qtype, callback, AsWeakPtr());
+    return scoped_ptr<DnsTransaction>(
+        new MockRequest(qname, qtype, callback, AsWeakPtr()));
   }
 
   int num_requests;
   std::map<Key, DnsResponse*> responses;

[mmenke, 2012/01/24 18:27:01]

While you're here, could you rename these to end in underscores?

 };
 
 }  // namespace
 
 
 // The following fixture sets up an environment for four different lookups
 // with their data defined in dns_test_util.h.  All tests make use of these
 // predefined variables instead of each defining their own, to avoid
 // boilerplate code in every test.  Assuming every coming query is for a
 // distinct hostname, as |kMaxTransactions| is set to 2 and
@@ skipping 15 matching lines @@
         ip_addresses2_(kT2IpAddresses,
             kT2IpAddresses + arraysize(kT2IpAddresses)),
         ip_addresses3_(kT3IpAddresses,
             kT3IpAddresses + arraysize(kT3IpAddresses)) {
     // AF_INET only for now.
     info0_.set_address_family(ADDRESS_FAMILY_IPV4);
     info1_.set_address_family(ADDRESS_FAMILY_IPV4);
     info2_.set_address_family(ADDRESS_FAMILY_IPV4);
     info3_.set_address_family(ADDRESS_FAMILY_IPV4);
 
-    client_.reset(new MockDnsClient());
+    client_ = new MockDnsTransactionFactory();
 
-    AddResponse(std::string(kT0DnsName, arraysize(kT0DnsName)), kT0Qtype,
+    AddResponse(kT0HostName, kT0Qtype,
         new DnsResponse(reinterpret_cast<const char*>(kT0ResponseDatagram),
                         arraysize(kT0ResponseDatagram),
                         arraysize(kT0QueryDatagram)));
 
-    AddResponse(std::string(kT1DnsName, arraysize(kT1DnsName)), kT1Qtype,
+    AddResponse(kT1HostName, kT1Qtype,
         new DnsResponse(reinterpret_cast<const char*>(kT1ResponseDatagram),
                         arraysize(kT1ResponseDatagram),
                         arraysize(kT1QueryDatagram)));
 
-    AddResponse(std::string(kT2DnsName, arraysize(kT2DnsName)), kT2Qtype,
+    AddResponse(kT2HostName, kT2Qtype,
         new DnsResponse(reinterpret_cast<const char*>(kT2ResponseDatagram),
                         arraysize(kT2ResponseDatagram),
                         arraysize(kT2QueryDatagram)));
 
-    AddResponse(std::string(kT3DnsName, arraysize(kT3DnsName)), kT3Qtype,
+    AddResponse(kT3HostName, kT3Qtype,
         new DnsResponse(reinterpret_cast<const char*>(kT3ResponseDatagram),
                         arraysize(kT3ResponseDatagram),
                         arraysize(kT3QueryDatagram)));
 
     resolver_.reset(
         new AsyncHostResolver(kMaxTransactions, kMaxPendingRequests,
               HostCache::CreateDefaultCache(),
-              client_.get(), NULL));
+              scoped_ptr<DnsTransactionFactory>(client_), NULL));
   }
 
   void AddResponse(const std::string& name, uint8 type, DnsResponse* response) {
-    client_->responses[MockDnsClient::Key(name, type)] = response;
+    client_->responses[MockDnsTransactionFactory::Key(name, type)] = response;
   }
 
  protected:
   AddressList addrlist0_, addrlist1_, addrlist2_, addrlist3_;
   HostResolver::RequestInfo info0_, info1_, info2_, info3_;
   std::vector<const char*> ip_addresses0_, ip_addresses1_,
     ip_addresses2_, ip_addresses3_;
   scoped_ptr<HostResolver> resolver_;
-  scoped_ptr<MockDnsClient> client_;
+  MockDnsTransactionFactory* client_;  // Owned by the AsyncHostResolver.
   TestCompletionCallback callback0_, callback1_, callback2_, callback3_;
 };
 
 TEST_F(AsyncHostResolverTest, EmptyHostLookup) {
   info0_.set_host_port_pair(HostPortPair("", kPortNum));
   int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL,
                               BoundNetLog());
   EXPECT_EQ(ERR_NAME_NOT_RESOLVED, rv);
 }
 
@@ skipping 29 matching lines @@
   EXPECT_EQ(OK, rv);
   VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_);
 
   // Now lookup |info0_| from cache only, store results in |addrlist1_|,
   // should succeed synchronously.
   rv = resolver_->ResolveFromCache(info0_, &addrlist1_, BoundNetLog());
   EXPECT_EQ(OK, rv);
   VerifyAddressList(ip_addresses0_, kPortNum, addrlist1_);
 }
 
-TEST_F(AsyncHostResolverTest, InvalidHostNameLookup) {
+// TODO(szym): This tests DnsClient not AsyncHostResolver. Remove or move to
+// dns_client_unittest.cc
+TEST_F(AsyncHostResolverTest, DISABLED_InvalidHostNameLookup) {
   const std::string kHostName1(64, 'a');
   info0_.set_host_port_pair(HostPortPair(kHostName1, kPortNum));
   int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL,
                               BoundNetLog());
-  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, rv);
+  EXPECT_EQ(ERR_INVALID_ARGUMENT, rv);
 
   const std::string kHostName2(4097, 'b');
   info0_.set_host_port_pair(HostPortPair(kHostName2, kPortNum));
   rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL,
                           BoundNetLog());
-  EXPECT_EQ(ERR_NAME_NOT_RESOLVED, rv);
+  EXPECT_EQ(ERR_INVALID_ARGUMENT, rv);
 }
 
 TEST_F(AsyncHostResolverTest, Lookup) {
   int rv = resolver_->Resolve(info0_, &addrlist0_, callback0_.callback(), NULL,
                               BoundNetLog());
   EXPECT_EQ(ERR_IO_PENDING, rv);
   rv = callback0_.WaitForResult();
   EXPECT_EQ(OK, rv);
   VerifyAddressList(ip_addresses0_, kPortNum, addrlist0_);
 }
@@ skipping 260 matching lines @@
   rv_fail = callback_fail.WaitForResult();
   EXPECT_EQ(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, rv_fail);
   EXPECT_EQ(static_cast<addrinfo*>(NULL), addrlist_fail.head());
 
   rv2 = callback2_.WaitForResult();
   EXPECT_EQ(OK, rv2);
   VerifyAddressList(ip_addresses2_, kPortNum, addrlist2_);
 }
 
 }  // namespace net