Begin consolidation of request building in reference. Further changes coming to simplify batch + media handling

lib/google/api_client/batch.rb

@@ -121,7 +121,7 @@ module Google
           call_response = deserialize_call_response(part)
           callback = @callbacks[call_response.call_id]
           call = @calls[call_response.call_id]
-          result = Google::APIClient::Result.new(call, nil, call_response)
+          result = Google::APIClient::Result.new(call, call_response)
           callback.call(result) if callback
         end
       end
@@ -196,7 +196,7 @@ module Google
       #
       # @return [String] The request as a string in application/http format.
       def serialize_call(call)
-        http_request = call.to_request
+        http_request = call.to_http_request
         method = http_request.method.to_s.upcase
         path = http_request.path.to_s
         status_line = method + " " + path + " HTTP/1.1"

lib/google/api_client/discovery/method.rb

@@ -224,13 +224,6 @@ module Google
       # @return [Array] The generated HTTP request.
       def generate_request(parameters={}, body='', headers=[], options={})
         options[:connection] ||= Faraday.default_connection
-        if body.respond_to?(:string)
-          body = body.string
-        elsif body.respond_to?(:to_str)
-          body = body.to_str
-        else
-          raise TypeError, "Expected String or StringIO, got #{body.class}."
-        end
         if !headers.kind_of?(Array) && !headers.kind_of?(Hash)
           raise TypeError, "Expected Hash or Array, got #{headers.class}."
         end

lib/google/api_client/reference.rb

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-
 require 'faraday'
 require 'faraday/utils'
 require 'multi_json'
@@ -29,84 +28,20 @@ module Google
       MULTIPART_BOUNDARY = "-----------RubyApiMultipartPost".freeze
 
       def initialize(options={})
-        # We only need this to do lookups on method ID String values
-        # It's optional, but method ID lookups will fail if the client is
-        # omitted.
-        @client = options[:client]
-        @version = options[:version] || 'v1'
 
         self.connection = options[:connection] || Faraday.default_connection
         self.authorization = options[:authorization]
         self.api_method = options[:api_method]
+        
         self.parameters = options[:parameters] || {}
         # These parameters are handled differently because they're not
         # parameters to the API method, but rather to the API system.
-        if self.parameters.kind_of?(Array)
-          if options[:key]
-            self.parameters.reject! { |k, _| k == 'key' }
-            self.parameters << ['key', options[:key]]
-          end
-          if options[:user_ip]
-            self.parameters.reject! { |k, _| k == 'userIp' }
-            self.parameters << ['userIp', options[:user_ip]]
-          end
-        elsif self.parameters.kind_of?(Hash)
-          self.parameters['key'] ||= options[:key] if options[:key]
-          self.parameters['userIp'] ||= options[:user_ip] if options[:user_ip]
-          # Convert to Array, because they're easier to work with when
-          # repeated parameters are an issue.
-          self.parameters = self.parameters.to_a
-        else
-          raise TypeError,
-            "Expected Array or Hash, got #{self.parameters.class}."
-        end
+        self.parameters['key'] ||= options[:key] if options[:key]
+        self.parameters['userIp'] ||= options[:user_ip] if options[:user_ip]
+
         self.headers = options[:headers] || {}
         if options[:media]
-          self.media = options[:media]
-          upload_type = self.parameters.find { |(k, _)| ['uploadType', 'upload_type'].include?(k) }.last
-          case upload_type
-          when "media"
-            if options[:body] || options[:body_object]
-              raise ArgumentError,
-                "Can not specify body & body object for simple uploads."
-            end
-            self.headers['Content-Type'] ||= self.media.content_type
-            self.body = self.media
-          when "multipart"
-            unless options[:body_object]
-              raise ArgumentError, "Multipart requested but no body object."
-            end
-            # This is all a bit of a hack due to Signet requiring body to be a
-            # string. Ideally, update Signet to delay serialization so we can
-            # just pass streams all the way down through to the HTTP library.
-            metadata = StringIO.new(serialize_body(options[:body_object]))
-            env = {
-              :request_headers => {
-                'Content-Type' =>
-                  "multipart/related;boundary=#{MULTIPART_BOUNDARY}"
-              },
-              :request => {:boundary => MULTIPART_BOUNDARY}
-            }
-            multipart = Faraday::Request::Multipart.new
-            self.body = multipart.create_multipart(env, [
-              [nil, Faraday::UploadIO.new(
-                metadata, 'application/json', 'file.json'
-              )],
-              [nil, self.media]])
-            self.headers.update(env[:request_headers])
-          when "resumable"
-            file_length = self.media.length
-            self.headers['X-Upload-Content-Type'] = self.media.content_type
-            self.headers['X-Upload-Content-Length'] = file_length.to_s
-            if options[:body_object]
-              self.headers['Content-Type'] ||= 'application/json'
-              self.body = serialize_body(options[:body_object])
-            else
-              self.body = ''
-            end
-          else
-            raise ArgumentError, "Invalid uploadType for media."
-          end
+          self.initialize_media_upload
         elsif options[:body]
           self.body = options[:body]
         elsif options[:body_object]
@@ -119,15 +54,49 @@ module Google
           self.http_method = options[:http_method] || 'GET'
           self.uri = options[:uri]
           unless self.parameters.empty?
-            query_values = (self.uri.query_values(Array) || [])
-            self.uri.query = Addressable::URI.form_encode(
-              (query_values + self.parameters).sort
-            )
-            self.uri.query = nil if self.uri.query == ""
+            self.uri.query = Addressable::URI.form_encode(self.parameters)
           end
         end
       end
 
+      def initialize_media_upload
+        self.media = options[:media]
+        case self.upload_type
+        when "media"
+          if options[:body] || options[:body_object] 
+            raise ArgumentError, "Can not specify body & body object for simple uploads"
+          end
+          self.headers['Content-Type'] ||= self.media.content_type
+          self.body = self.media
+        when "multipart"
+          unless options[:body_object] 
+            raise ArgumentError, "Multipart requested but no body object"              
+          end
+          metadata = StringIO.new(serialize_body(options[:body_object]))
+          env = {
+            :request_headers => {'Content-Type' => "multipart/related;boundary=#{MULTIPART_BOUNDARY}"},
+            :request => { :boundary => MULTIPART_BOUNDARY }
+          }
+          multipart = Faraday::Request::Multipart.new
+          self.body = multipart.create_multipart(env, [
+            [nil,Faraday::UploadIO.new(metadata, 'application/json', 'file.json')], 
+            [nil, self.media]])
+          self.headers.update(env[:request_headers])
+        when "resumable"
+          file_length = self.media.length
+          self.headers['X-Upload-Content-Type'] = self.media.content_type
+          self.headers['X-Upload-Content-Length'] = file_length.to_s
+          if options[:body_object]
+            self.headers['Content-Type'] ||= 'application/json'
+            self.body = serialize_body(options[:body_object]) 
+          else
+            self.body = ''
+          end
+        else
+          raise ArgumentError, "Invalid uploadType for media"
+        end
+      end
+
       def serialize_body(body)
         return body.to_json if body.respond_to?(:to_json)
         return MultiJson.dump(options[:body_object].to_hash) if body.respond_to?(:to_hash)
@@ -142,6 +111,10 @@ module Google
       def media=(media)
         @media = (media)
       end
+      
+      def upload_type
+        return self.parameters['uploadType'] || self.parameters['upload_type']
+      end
 
       def authorization
         return @authorization
@@ -172,29 +145,6 @@ module Google
         if new_api_method.kind_of?(Google::APIClient::Method) ||
             new_api_method == nil
           @api_method = new_api_method
-        elsif new_api_method.respond_to?(:to_str) ||
-            new_api_method.kind_of?(Symbol)
-          unless @client
-            raise ArgumentError,
-              "API method lookup impossible without client instance."
-          end
-          new_api_method = new_api_method.to_s
-          # This method of guessing the API is unreliable. This will fail for
-          # APIs where the first segment of the RPC name does not match the
-          # service name. However, this is a fallback mechanism anyway.
-          # Developers should be passing in a reference to the method, rather
-          # than passing in a string or symbol. This should raise an error
-          # in the case of a mismatch.
-          api = new_api_method[/^([^.]+)\./, 1]
-          @api_method = @client.discovered_method(
-            new_api_method, api, @version
-          )
-          if @api_method
-            # Ditch the client reference, we won't need it again.
-            @client = nil
-          else
-            raise ArgumentError, "API method could not be found."
-          end
         else
           raise TypeError,
             "Expected Google::APIClient::Method, got #{new_api_method.class}."
@@ -206,8 +156,7 @@ module Google
       end
 
       def parameters=(new_parameters)
-        # No type-checking needed, the Method class handles this.
-        @parameters = new_parameters
+        @parameters = Hash[new_parameters]
       end
 
       def body
@@ -215,19 +164,7 @@ module Google
       end
 
       def body=(new_body)
-        if new_body.respond_to?(:to_str)
-          @body = new_body.to_str
-        elsif new_body.respond_to?(:read)
-          @body = new_body.read()
-        elsif new_body.respond_to?(:inject)
-          @body = (new_body.inject(StringIO.new) do |accu, chunk|
-            accu.write(chunk)
-            accu
-          end).string
-        else
-          raise TypeError,
-            "Expected body to be String, IO, or Enumerable chunks."
-        end
+        @body = new_body
       end
 
       def headers
@@ -248,9 +185,9 @@ module Google
 
       def http_method=(new_http_method)
         if new_http_method.kind_of?(Symbol)
-          @http_method = new_http_method.to_s.upcase
+          @http_method = new_http_method.to_s.downcase.to_sym
         elsif new_http_method.respond_to?(:to_str)
-          @http_method = new_http_method.to_str.upcase
+          @http_method = new_http_method.to_s.downcase.to_sym
         else
           raise TypeError,
             "Expected String or Symbol, got #{new_http_method.class}."
@@ -265,21 +202,27 @@ module Google
         @uri = Addressable::URI.parse(new_uri)
       end
 
-      def to_request
-        if self.api_method
-          return self.api_method.generate_request(
-            self.parameters, self.body, self.headers,
+      def to_http_request
+        request = ( 
+          if self.api_method
+            self.api_method.generate_request(
+              self.parameters, self.body, self.headers, :connection => self.connection
+            )
+          else
+            self.connection.build_request(self.http_method) do |req|
+              req.url(self.uri.to_str)
+              req.headers.update(self.headers)
+              req.body = self.body
+            end
+          end)
+        
+        if self.authorization.respond_to?(:generate_authenticated_request)
+          request = self.authorization.generate_authenticated_request(
+            :request => request,
             :connection => self.connection
           )
-        else
-          return self.connection.build_request(
-            self.http_method.to_s.downcase.to_sym
-          ) do |req|
-            req.url(Addressable::URI.parse(self.uri).normalize.to_s)
-            req.headers = Faraday::Utils::Headers.new(self.headers)
-            req.body = self.body
-          end
         end
+        return request
       end
 
       def to_hash

lib/google/api_client/result.rb

@@ -18,16 +18,13 @@ module Google
     ##
     # This class wraps a result returned by an API call.
     class Result
-      def initialize(reference, request, response)
+      def initialize(reference, response)
         @reference = reference
-        @request = request
         @response = response
       end
 
       attr_reader :reference
 
-      attr_reader :request
-
       attr_reader :response
 
       def status