From bbcc946f33227973a8954179a0a386cbc010dda6 Mon Sep 17 00:00:00 2001
From: Bob Aman <bobaman@google.com>
Date: Tue, 28 Sep 2010 23:09:07 +0000
Subject: [PATCH] Improved coverage in tests and fixed a URI join bug.

git-svn-id: https://google-api-ruby-client.googlecode.com/svn/trunk@36 c1d61fac-ed7f-fcc1-18f7-ff78120a04ef
---
 lib/google/api_client.rb                 | 21 +++++--
 lib/google/api_client/discovery.rb       |  5 +-
 spec/google/api_client/discovery_spec.rb | 72 ++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/lib/google/api_client.rb b/lib/google/api_client.rb
index 369db35be..c5c5d475e 100644
--- a/lib/google/api_client.rb
+++ b/lib/google/api_client.rb
@@ -18,9 +18,16 @@ require 'json'
 require 'google/api_client/discovery'
 
 module Google #:nodoc:
+  # TODO(bobaman): Document all this stuff.
+
   ##
   # This class manages communication with a single API.
   class APIClient
+    ##
+    # An error which is raised when there is an unexpected response or other
+    # transport error that prevents an operation from succeeding.
+    class TransmissionError < StandardError
+    end
 
     def initialize(options={})
       @options = {
@@ -68,13 +75,19 @@ module Google #:nodoc:
       end)
     end
 
+    ##
+    # Returns the URI for the discovery document.
+    #
+    # @return [Addressable::URI] The URI of the discovery document.
     def discovery_uri
       return @options[:discovery_uri] ||= (begin
         if @options[:service]
           service_id = @options[:service]
           service_version = @options[:service_version] || 'v1'
-          "http://www.googleapis.com/discovery/0.1/describe" +
-          "?api=#{service_id}"
+          Addressable::URI.parse(
+            "http://www.googleapis.com/discovery/0.1/describe" +
+            "?api=#{service_id}"
+          )
         else
           raise ArgumentError,
             'Missing required configuration value, :discovery_uri.'
@@ -84,10 +97,10 @@ module Google #:nodoc:
 
     def discovery_document
       return @discovery_document ||= (begin
-        request = ['GET', self.discovery_uri, [], []]
+        request = ['GET', self.discovery_uri.to_s, [], []]
         response = self.transmit_request(request)
         status, headers, body = response
-        if status == 200
+        if status == 200 # TODO(bobaman) Better status code handling?
           merged_body = StringIO.new
           body.each do |chunk|
             merged_body.write(chunk)
diff --git a/lib/google/api_client/discovery.rb b/lib/google/api_client/discovery.rb
index 3f89b3688..3e999b7c7 100644
--- a/lib/google/api_client/discovery.rb
+++ b/lib/google/api_client/discovery.rb
@@ -168,8 +168,11 @@ module Google #:nodoc:
       end
 
       def uri_template
+        # TODO(bobaman) We shouldn't be calling #to_s here, this should be
+        # a join operation on a URI, but we have to treat these as Strings
+        # because of the way the discovery document provides the URIs.
         return @uri_template ||=
-          Addressable::Template.new(base + self.description['pathUrl'])
+          Addressable::Template.new(base.to_s + self.description['pathUrl'])
       end
 
       def normalize_parameters(parameters={})
diff --git a/spec/google/api_client/discovery_spec.rb b/spec/google/api_client/discovery_spec.rb
index b168ee89c..a9254affa 100644
--- a/spec/google/api_client/discovery_spec.rb
+++ b/spec/google/api_client/discovery_spec.rb
@@ -21,11 +21,40 @@ require 'google/api_client'
 require 'google/api_client/version'
 require 'google/api_client/parsers/json_parser'
 
+describe Google::APIClient, 'unconfigured' do
+  before do
+    @client = Google::APIClient.new
+  end
+
+  it 'should not be able to determine the discovery URI' do
+    (lambda do
+      @client.discovery_uri
+    end).should raise_error(ArgumentError)
+  end
+end
+
+describe Google::APIClient, 'configured for a bogus API' do
+  before do
+    @client = Google::APIClient.new(:service => 'bogus')
+  end
+
+  it 'should not be able to retrieve the discovery document' do
+    (lambda do
+      @client.discovery_document
+    end).should raise_error(Google::APIClient::TransmissionError)
+  end
+end
+
 describe Google::APIClient, 'configured for the prediction API' do
   before do
     @client = Google::APIClient.new(:service => 'prediction')
   end
 
+  it 'should correctly determine the discovery URI' do
+    @client.discovery_uri.should ===
+      'http://www.googleapis.com/discovery/0.1/describe?api=prediction'
+  end
+
   it 'should have multiple versions available' do
     @client.discovered_services.size.should > 1
   end
@@ -68,4 +97,47 @@ describe Google::APIClient, 'configured for the prediction API' do
     # Sanity check the algorithm
     @client.latest_service('bogus').should == nil
   end
+
+  it 'should generate requests against the correct URIs' do
+    request = @client.generate_request(
+      'prediction.training.insert',
+      {'query' => '12345'},
+      '',
+      [],
+      {:signed => false}
+    )
+    method, uri, headers, body = request
+    uri.should ==
+      'https://www.googleapis.com/prediction/v1/training?query=12345'
+  end
+
+  it 'should generate signed requests' do
+    @client.authorization.token_credential_key = '12345'
+    @client.authorization.token_credential_secret = '12345'
+    request = @client.generate_request(
+      'prediction.training.insert',
+      {'query' => '12345'},
+      '',
+      [],
+      {:signed => true}
+    )
+    method, uri, headers, body = request
+    headers = Hash[headers]
+    headers.keys.should include('Authorization')
+    headers['Authorization'].should =~ /^OAuth/
+  end
+
+  it 'should not be able to execute improperly authorized requests' do
+    @client.authorization.token_credential_key = '12345'
+    @client.authorization.token_credential_secret = '12345'
+    response = @client.execute(
+      'prediction.training.insert',
+      {'query' => '12345'},
+      '',
+      [],
+      {:signed => true}
+    )
+    status, headers, body = response
+    status.should == 401
+  end
 end