[core/xml] Splits SAX parsing from Connection

This creates a new `Fog::Core::Connection` class that wraps around HTTP
connections/requests but does not presume SAX parsing of the API response.

A new `Fog::XML::SAXParserConnection` is available which implements the
original behaviour with a clearer interface.

`Fog::Connection` subclasses `SAXParserConnection` to be backwards
compatible.

Further testing and deprecation warnings are needed.

Since mock testing occurs at a higher level the changed code is not
exercised by the tests and I do not have access to an XML based API to
debug quickly with.

lib/fog/core.rb

@@ -34,6 +34,8 @@ require 'fog/core/wait_for'
 require 'fog/xml'
 require 'fog/json'
 
+# deprecation wrappers
+require 'fog/core/deprecated/connection'
 
 # service wrappers
 require 'fog/compute'

lib/fog/core/connection.rb

@@ -1,88 +1,70 @@
 module Fog
+  module Core
 
-  # The Connection class is a wrapper around an instance of Excon::Connection
-  # supporting {#request} and {#reset} only.
-  #
-  # {#request} includes an option to perform SAX parsing for XML APIs.
-  #
-  # @see https://github.com/geemus/excon/blob/master/lib/excon/connection.rb
-  #
-  class Connection
-
-    # Prepares the connection and sets defaults for any future requests.
     #
-    # @param [String] url The destination URL
-    # @param persistent [Boolean]
-    # @param [Hash] params
-    # @option params [String] :body Default text to be sent over a socket. Only used if :body absent in Connection#request params
-    # @option params [Hash<Symbol, String>] :headers The default headers to supply in a request. Only used if params[:headers] is not supplied to Connection#request
-    # @option params [String] :host The destination host's reachable DNS name or IP, in the form of a String
-    # @option params [String] :path Default path; appears after 'scheme://host:port/'. Only used if params[:path] is not supplied to Connection#request
-    # @option params [Fixnum] :port The port on which to connect, to the destination host
-    # @option params [Hash]   :query Default query; appended to the 'scheme://host:port/path/' in the form of '?key=value'. Will only be used if params[:query] is not supplied to Connection#request
-    # @option params [String] :scheme The protocol; 'https' causes OpenSSL to be used
-    # @option params [String] :proxy Proxy server; e.g. 'http://myproxy.com:8888'
-    # @option params [Fixnum] :retry_limit Set how many times we'll retry a failed request.  (Default 4)
-    # @option params [Class] :instrumentor Responds to #instrument as in ActiveSupport::Notifications
-    # @option params [String] :instrumentor_name Name prefix for #instrument events.  Defaults to 'excon'
-    # @option params [Nokogiri::XML::SAX::Document] :parser
-    #
-    def initialize(url, persistent=false, params={})
-      unless params.has_key?(:debug_response)
-        params[:debug_response] = true
-      end
-      params[:headers] ||= {}
-      params[:headers]['User-Agent'] ||= "fog/#{Fog::VERSION}"
-      @excon = Excon.new(url, params)
-      @persistent = persistent
-    end
-
-    # Makes a request using the connection using Excon
-    #
-    # @param [Hash] params
-    # @option params [String] :body text to be sent over a socket
-    # @option params [Hash<Symbol, String>] :headers The default headers to supply in a request
-    # @option params [String] :host The destination host's reachable DNS name or IP, in the form of a String
-    # @option params [String] :path appears after 'scheme://host:port/'
-    # @option params [Fixnum] :port The port on which to connect, to the destination host
-    # @option params [Hash]   :query appended to the 'scheme://host:port/path/' in the form of '?key=value'
-    # @option params [String] :scheme The protocol; 'https' causes OpenSSL to be used
-    # @option params [Proc] :response_block
-    # @option params [Nokogiri::XML::SAX::Document] :parser
-    #
-    # @return [Excon::Response]
-    #
-    # @raise [Excon::Errors::StubNotFound]
-    # @raise [Excon::Errors::Timeout]
-    # @raise [Excon::Errors::SocketError]
-    #
-    def request(params, &block)
-      unless @persistent
-        reset
-      end
-      unless block_given?
-        if (parser = params.delete(:parser))
-          body = Nokogiri::XML::SAX::PushParser.new(parser)
-          params[:response_block] = lambda do |chunk, remaining, total|
-            body << chunk
-          end
+    class Connection
+      # Prepares the connection and sets defaults for any future requests.
+      #
+      # @param [String] url The destination URL
+      # @param persistent [Boolean]
+      # @param [Hash] params
+      # @option params [String] :body Default text to be sent over a socket. Only used if :body absent in Connection#request params
+      # @option params [Hash<Symbol, String>] :headers The default headers to supply in a request. Only used if params[:headers] is not supplied to Connection#request
+      # @option params [String] :host The destination host's reachable DNS name or IP, in the form of a String
+      # @option params [String] :path Default path; appears after 'scheme://host:port/'. Only used if params[:path] is not supplied to Connection#request
+      # @option params [Fixnum] :port The port on which to connect, to the destination host
+      # @option params [Hash]   :query Default query; appended to the 'scheme://host:port/path/' in the form of '?key=value'. Will only be used if params[:query] is not supplied to Connection#request
+      # @option params [String] :scheme The protocol; 'https' causes OpenSSL to be used
+      # @option params [String] :proxy Proxy server; e.g. 'http://myproxy.com:8888'
+      # @option params [Fixnum] :retry_limit Set how many times we'll retry a failed request.  (Default 4)
+      # @option params [Class] :instrumentor Responds to #instrument as in ActiveSupport::Notifications
+      # @option params [String] :instrumentor_name Name prefix for #instrument events.  Defaults to 'excon'
+      # @option params [Nokogiri::XML::SAX::Document] :parser
+      #
+      def initialize(url, persistent=false, params={})
+        unless params.has_key?(:debug_response)
+          params[:debug_response] = true
         end
+        params[:headers] ||= {}
+        params[:headers]['User-Agent'] ||= "fog/#{Fog::VERSION}"
+        @excon = Excon.new(url, params)
+        @persistent = persistent
       end
 
-      response = @excon.request(params, &block)
-
-      if parser
-        body.finish
-        response.body = parser.response
+      # Makes a request using the connection using Excon
+      #
+      # @param [Hash] params
+      # @option params [String] :body text to be sent over a socket
+      # @option params [Hash<Symbol, String>] :headers The default headers to supply in a request
+      # @option params [String] :host The destination host's reachable DNS name or IP, in the form of a String
+      # @option params [String] :path appears after 'scheme://host:port/'
+      # @option params [Fixnum] :port The port on which to connect, to the destination host
+      # @option params [Hash]   :query appended to the 'scheme://host:port/path/' in the form of '?key=value'
+      # @option params [String] :scheme The protocol; 'https' causes OpenSSL to be used
+      # @option params [Proc] :response_block
+      #
+      # @return [Excon::Response]
+      #
+      # @raise [Excon::Errors::StubNotFound]
+      # @raise [Excon::Errors::Timeout]
+      # @raise [Excon::Errors::SocketError]
+      #
+      def request(params, &block)
+        reset unless @persistent
+        @excon.request(params, &block)
       end
 
-      response
-    end
+      # Make {#request} available even when it has been overidden by a subclass
+      # to allow backwards compatibility.
+      #
+      alias_method :original_request, :request
+      protected :original_request
 
-    # Closes the connection
-    #
-    def reset
-      @excon.reset
+      # Closes the connection
+      #
+      def reset
+        @excon.reset
+      end
     end
   end
 end

lib/fog/core/deprecated/connection.rb

@@ -0,0 +1,24 @@
+require "fog/xml"
+
+module Fog
+
+  # @deprecated Use {Fog::Core::Connection} or {XML::SAXParserConnection} if you
+  #   require the response body to be parsed.
+  #
+  # The Connection class is a wrapper around an instance of Excon::Connection
+  # supporting {#request} and {#reset} only.
+  #
+  # {#request} includes an option to perform SAX parsing for XML APIs.
+  #
+  # @see https://github.com/geemus/excon/blob/master/lib/excon/connection.rb
+  #
+  class Connection < Fog::XML::SAXParserConnection
+    def request(params, &block)
+      if (parser = params.delete(:parser))
+        super(parser, params)
+      else
+        original_request(params)
+      end
+    end
+  end
+end

lib/fog/xml.rb

@@ -17,3 +17,5 @@ module Fog
   module XML
   end
 end
+
+require "fog/xml/sax_parser_connection"

lib/fog/xml/sax_parser_connection.rb

@@ -0,0 +1,43 @@
+module Fog
+  module XML
+    class SAXParserConnection < Fog::Core::Connection
+
+      # Makes a request using the connection using Excon
+      #
+      # @param [Hash] params
+      # @option params [String] :body text to be sent over a socket
+      # @option params [Hash<Symbol, String>] :headers The default headers to supply in a request
+      # @option params [String] :host The destination host's reachable DNS name or IP, in the form of a String
+      # @option params [String] :path appears after 'scheme://host:port/'
+      # @option params [Fixnum] :port The port on which to connect, to the destination host
+      # @option params [Hash]   :query appended to the 'scheme://host:port/path/' in the form of '?key=value'
+      # @option params [String] :scheme The protocol; 'https' causes OpenSSL to be used
+      # @option params [Proc] :response_block
+      # @option params [Nokogiri::XML::SAX::Document] :parser
+      #
+      # @return [Excon::Response]
+      #
+      # @raise [Excon::Errors::StubNotFound]
+      # @raise [Excon::Errors::Timeout]
+      # @raise [Excon::Errors::SocketError]
+      #
+      def request(parser, params)
+        reset unless @persistent
+
+        # Prepare the SAX parser
+        data_stream = Nokogiri::XML::SAX::PushParser.new(parser)
+        params[:response_block] = lambda do |chunk, remaining, total|
+          data_stream << chunk
+        end
+
+        # Make request which read chunks into parser
+        response = @excon.request(params)
+
+        # Cease parsing and override response.body with parsed data
+        data_stream.finish
+        response.body = parser.response
+        response
+      end
+    end
+  end
+end