Extract parsing into custom parser class

History

@@ -1,8 +1,13 @@
 == 0.4.6 master
 * bug fixes
   * inheritable attributes no longer mutable by subclasses (yyyc514)
+
+* major enhancements
+  * Custom Parsers via class or proc
+  * Deprecation warning on HTTParty::AllowedFormats
+    moved to HTTParty::Parser::SupportedFormats
+
 * minor enhancements
-  * Register a customized response parser
   * Curl inspired output when using the binary in verbose mode (alexvollmer)
   * raise UnsupportedURIScheme when scheme is not HTTP or HTTPS (djspinmonkey)
   * Allow SSL for ports other than 443 when scheme is HTTPS (stefankroes)

examples/custom_parsers.rb

@@ -0,0 +1,71 @@
+class ParseAtom
+  include HTTParty
+
+  # Support Atom along with the default parsers: xml, json, yaml, etc.
+  class Parser::Atom < HTTParty::Parser
+    def self.formats
+      super.merge({"application/atom+xml" => :atom})
+    end
+
+    protected
+
+    # perform atom parsing on body
+    def atom
+      body.to_atom
+    end
+  end
+
+  parser Parser::Atom
+end
+
+
+class OnlyParseAtom
+  include HTTParty
+
+  # Only support Atom
+  class Parser::OnlyAtom < HTTParty::Parser
+    def self.formats
+      {"application/atom+xml" => :atom}
+    end
+
+    protected
+
+    # perform atom parsing on body
+    def atom
+      body.to_atom
+    end
+  end
+
+  parser Parser::OnlyAtom
+end
+
+
+class SkipParsing
+  include HTTParty
+
+  # Parse the response body however you like
+  class Parser::Simple < HTTParty::Parser
+    def parse
+      body
+    end
+  end
+
+  parser Parser::Simple
+end
+
+
+class AdHocParsing
+  include HTTParty
+  parser(
+    Proc.new do |body, format|
+      case format
+      when :json
+        body.to_json
+      when :xml
+        body.to_xml
+      else
+        body
+      end
+    end
+  )
+end

lib/httparty.rb

@@ -11,19 +11,18 @@ require dir + 'httparty/module_inheritable_attributes'
 require dir + 'httparty/cookie_hash'
 
 module HTTParty
+  module AllowedFormatsDeprecation
+    def const_missing(const)
+      if const.to_s =~ /AllowedFormats$/
+        Kernel.warn("Deprecated: Use HTTParty::Parser::SupportedFormats")
+        HTTParty::Parser::SupportedFormats
+      else
+        super
+      end
+    end
+  end
 
-  AllowedFormats = {
+  extend AllowedFormatsDeprecation
-    'text/xml'               => :xml,
-    'application/xml'        => :xml,
-    'application/json'       => :json,
-    'text/json'              => :json,
-    'application/javascript' => :json,
-    'text/javascript'        => :json,
-    'text/html'              => :html,
-    'application/x-yaml'     => :yaml,
-    'text/yaml'              => :yaml,
-    'text/plain'             => :plain
-  } unless defined?(AllowedFormats)
 
   def self.included(base)
     base.extend ClassMethods
@@ -35,6 +34,8 @@ module HTTParty
   end
 
   module ClassMethods
+    extend AllowedFormatsDeprecation
+
     # Allows setting http proxy information to be used
     #
     #   class Foo
@@ -105,9 +106,14 @@ module HTTParty
     #     include HTTParty
     #     format :json
     #   end
-    def format(f)
+    def format(f = nil)
-      raise UnsupportedFormat, "Must be one of: #{AllowedFormats.values.map { |v| v.to_s }.uniq.sort.join(', ')}" unless AllowedFormats.value?(f)
+      if f.nil?
+        default_options[:format]
+      else
+        parser(Parser) if parser.nil?
         default_options[:format] = f
+        validate_format
+      end
     end
 
     # Allows setting a PEM file to be used
@@ -126,8 +132,13 @@ module HTTParty
     #     include HTTParty
     #     parser Proc.new {|data| ...}
     #   end
-    def parser(customer_parser)
+    def parser(customer_parser = nil)
+      if customer_parser.nil?
+        default_options[:parser]
+      else
         default_options[:parser] = customer_parser
+        validate_format
+      end
     end
 
     # Allows making a get request to a url.
@@ -183,6 +194,7 @@ module HTTParty
     end
 
     private
+
       def perform_request(http_method, path, options) #:nodoc:
         options = default_options.dup.merge(options)
         process_cookies(options)
@@ -194,6 +206,12 @@ module HTTParty
         options[:headers] ||= headers.dup
         options[:headers]["cookie"] = cookies.merge(options.delete(:cookies) || {}).to_cookie_string
       end
+
+      def validate_format
+        if format && parser.respond_to?(:supports_format?) && !parser.supports_format?(format)
+          raise UnsupportedFormat, "'#{format.inspect}' Must be one of: #{parser.supported_formats.map{|f| f.to_s}.sort.join(', ')}"
+        end
+      end
   end
 
   def self.normalize_base_uri(url) #:nodoc:
@@ -239,6 +257,7 @@ end
 
 require dir + 'httparty/core_extensions'
 require dir + 'httparty/exceptions'
+require dir + 'httparty/parser'
 require dir + 'httparty/request'
 require dir + 'httparty/response'
 

lib/httparty/parser.rb

@@ -0,0 +1,85 @@
+module HTTParty
+  class Parser
+    SupportedFormats = {
+      'text/xml'               => :xml,
+      'application/xml'        => :xml,
+      'application/json'       => :json,
+      'text/json'              => :json,
+      'application/javascript' => :json,
+      'text/javascript'        => :json,
+      'text/html'              => :html,
+      'application/x-yaml'     => :yaml,
+      'text/yaml'              => :yaml,
+      'text/plain'             => :plain
+    }
+
+    attr_reader :body, :format
+
+    def self.call(body, format)
+      new(body, format).parse
+    end
+
+    def self.formats
+      const_get(:SupportedFormats)
+    end
+
+    def self.format_from_mimetype(mimetype)
+      formats[formats.keys.detect {|k| mimetype.include?(k)}]
+    end
+
+    def self.supported_formats
+      formats.values.uniq
+    end
+
+    def self.supports_format?(format)
+      supported_formats.include?(format)
+    end
+
+    def initialize(body, format)
+      @body = body
+      @format = format
+    end
+    private_class_method :new
+
+    def parse
+      return nil if body.nil? || body.empty?
+      if supports_format?
+        parse_supported_format
+      else
+        body
+      end
+    end
+
+    protected
+
+    def xml
+      Crack::XML.parse(body)
+    end
+
+    def json
+      Crack::JSON.parse(body)
+    end
+
+    def yaml
+      YAML.load(body)
+    end
+
+    def html
+      body
+    end
+
+    def plain
+      body
+    end
+
+    def supports_format?
+      self.class.supports_format?(format)
+    end
+
+    def parse_supported_format
+      send(format)
+      rescue NoMethodError
+        raise NotImplementedError, "#{self.class.name} has not implemented a parsing method for the #{format.inspect} format."
+    end
+  end
+end

lib/httparty/parsers.rb

@@ -1,31 +0,0 @@
-module HTTParty
-  module Xml
-    def self.parse(body)
-      Crack::XML.parse(body)
-    end
-  end
-  
-  module Json
-    def self.parse(body)
-      Crack::JSON.parse(body)
-    end
-  end
-  
-  module Yaml
-    def self.parse(str)
-      ::YAML.load(str)
-    end
-  end
-  
-  module Html
-    def self.parse(str)
-      str
-    end
-  end
-  
-  module Text
-    def self.parse(str)
-      str
-    end
-  end
-end

lib/httparty/request.rb

@@ -1,9 +1,16 @@
 require 'uri'
 
 module HTTParty
-
   class Request #:nodoc:
-    SupportedHTTPMethods = [Net::HTTP::Get, Net::HTTP::Post, Net::HTTP::Put, Net::HTTP::Delete, Net::HTTP::Head, Net::HTTP::Options]
+    SupportedHTTPMethods = [
+      Net::HTTP::Get,
+      Net::HTTP::Post,
+      Net::HTTP::Put,
+      Net::HTTP::Delete,
+      Net::HTTP::Head,
+      Net::HTTP::Options
+    ]
+
     SupportedURISchemes  = [URI::HTTP, URI::HTTPS]
 
     attr_accessor :http_method, :path, :options
@@ -14,6 +21,7 @@ module HTTParty
       self.options = {
         :limit => o.delete(:no_follow) ? 0 : 5,
         :default_params => {},
+        :parser => Parser
       }.merge(o)
     end
 
@@ -40,6 +48,11 @@ module HTTParty
       options[:format]
     end
 
+    def parser
+      options[:parser]
+    end
+
+
     def perform
       validate
       setup_raw_request
@@ -126,31 +139,12 @@ module HTTParty
             capture_cookies(response)
             perform
           else
-            parsed_response = parse_response(response.body)
+            Response.new(parse_response(response.body), response.body, response.code, response.message, response.to_hash)
-            Response.new(parsed_response, response.body, response.code, response.message, response.to_hash)
           end
       end
 
       def parse_response(body)
-        return nil if body.nil? or body.empty?
+        parser.call(body, format)
-        if options[:parser].blank?
-          case format
-            when :xml
-              Crack::XML.parse(body)
-            when :json
-              Crack::JSON.parse(body)
-            when :yaml
-              YAML::load(body)
-            else
-              body
-            end
-        else
-          if options[:parser].is_a?(Proc)
-            options[:parser].call(body)
-          else
-            body
-          end
-        end
       end
 
       def capture_cookies(response)
@@ -162,11 +156,13 @@ module HTTParty
         options[:headers]['Cookie'] = cookies_hash.to_cookie_string
       end
 
-      # Uses the HTTP Content-Type header to determine the format of the response
+      # Uses the HTTP Content-Type header to determine the format of the
-      # It compares the MIME type returned to the types stored in the AllowedFormats hash
+      # response It compares the MIME type returned to the types stored in the
+      # SupportedFormats hash
       def format_from_mimetype(mimetype)
-        return nil if mimetype.nil?
+        if mimetype && parser.respond_to?(:format_from_mimetype)
-        AllowedFormats.each { |k, v| return v if mimetype.include?(k) }
+          parser.format_from_mimetype(mimetype)
+        end
       end
 
       def validate

spec/httparty/parser_spec.rb

@@ -0,0 +1,147 @@
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'spec_helper'))
+
+class CustomParser < HTTParty::Parser; end
+
+describe HTTParty::Parser do
+  describe ".SupportedFormats" do
+    it "returns a hash" do
+      HTTParty::Parser::SupportedFormats.should be_instance_of(Hash)
+    end
+  end
+
+  describe ".call" do
+    it "generates an HTTParty::Parser instance with the given body and format" do
+      HTTParty::Parser.should_receive(:new).with('body', :plain).and_return(stub(:parse => nil))
+      HTTParty::Parser.call('body', :plain)
+    end
+
+    it "calls #parse on the parser" do
+      parser = mock('Parser')
+      parser.should_receive(:parse)
+      HTTParty::Parser.stub(:new => parser)
+      parser = HTTParty::Parser.call('body', :plain)
+    end
+  end
+
+  describe ".formats" do
+    it "returns the SupportedFormats constant" do
+      HTTParty::Parser.formats.should == HTTParty::Parser::SupportedFormats
+    end
+  end
+
+  describe ".format_from_mimetype" do
+    it "returns a symbol representing the format mimetype" do
+      HTTParty::Parser.format_from_mimetype("text/plain").should == :plain
+    end
+
+    it "returns nil when the mimetype is not supported" do
+      HTTParty::Parser.format_from_mimetype("application/atom+xml").should be_nil
+    end
+  end
+
+  describe ".supported_formats" do
+    it "returns a unique set of supported formats represented by symbols" do
+      HTTParty::Parser.supported_formats.should == HTTParty::Parser::SupportedFormats.values.uniq
+    end
+  end
+
+  describe ".supports_format?" do
+    it "returns true for a supported format" do
+      HTTParty::Parser.stub(:supported_formats => [:json])
+      HTTParty::Parser.supports_format?(:json).should be_true
+    end
+
+    it "returns false for an unsupported format" do
+      HTTParty::Parser.stub(:supported_formats => [])
+      HTTParty::Parser.supports_format?(:json).should be_false
+    end
+  end
+
+  describe "#parse" do
+    before do
+      @parser = HTTParty::Parser.new('body', :json)
+    end
+
+    it "attempts to parse supported formats" do
+      @parser.stub(:supports_format? => true)
+      @parser.should_receive(:parse_supported_format)
+      @parser.parse
+    end
+
+    it "returns the unparsed body when the format is unsupported" do
+      @parser.stub(:supports_format? => false)
+      @parser.parse.should == @parser.body
+    end
+
+    it "returns nil for an empty body" do
+      @parser.stub(:body => '')
+      @parser.parse.should be_nil
+    end
+
+    it "returns nil for a nil body" do
+      @parser.stub(:body => nil)
+      @parser.parse.should be_nil
+    end
+  end
+
+  describe "#supports_format?" do
+    it "utilizes the class method to determine if the format is supported" do
+      HTTParty::Parser.should_receive(:supports_format?).with(:json)
+      parser = HTTParty::Parser.new('body', :json)
+      parser.send(:supports_format?)
+    end
+  end
+
+  describe "#parse_supported_format" do
+    it "calls the parser for the given format" do
+      parser = HTTParty::Parser.new('body', :json)
+      parser.should_receive(:json)
+      parser.send(:parse_supported_format)
+    end
+
+    context "when a parsing method does not exist for the given format" do
+      it "raises an exception" do
+        parser = HTTParty::Parser.new('body', :atom)
+        expect do
+          parser.send(:parse_supported_format)
+        end.to raise_error(NotImplementedError, "HTTParty::Parser has not implemented a parsing method for the :atom format.")
+      end
+
+      it "raises a useful exception message for subclasses" do
+        parser = CustomParser.new('body', :atom)
+        expect do
+          parser.send(:parse_supported_format)
+        end.to raise_error(NotImplementedError, "CustomParser has not implemented a parsing method for the :atom format.")
+      end
+    end
+  end
+
+  context "parsers" do
+    subject do
+      HTTParty::Parser.new('body', nil)
+    end
+
+    it "parses xml with Crack" do
+      Crack::XML.should_receive(:parse).with('body')
+      subject.send(:xml)
+    end
+
+    it "parses json with Crack" do
+      Crack::JSON.should_receive(:parse).with('body')
+      subject.send(:json)
+    end
+
+    it "parses yaml" do
+      YAML.should_receive(:load).with('body')
+      subject.send(:yaml)
+    end
+
+    it "parses html by simply returning the body" do
+      subject.send(:html).should == 'body'
+    end
+
+    it "parses plain text by simply returning the body" do
+      subject.send(:plain).should == 'body'
+    end
+  end
+end

spec/httparty/request_spec.rb

@@ -19,6 +19,19 @@ describe HTTParty::Request do
     @request = HTTParty::Request.new(Net::HTTP::Get, 'http://api.foo.com/v1', :format => :xml)
   end
 
+  describe "initialization" do
+    it "sets parser to HTTParty::Parser" do
+      request = HTTParty::Request.new(Net::HTTP::Get, 'http://google.com')
+      request.parser.should == HTTParty::Parser
+    end
+
+    it "sets parser to the optional parser" do
+      my_parser = lambda {}
+      request = HTTParty::Request.new(Net::HTTP::Get, 'http://google.com', :parser => my_parser)
+      request.parser.should == my_parser
+    end
+  end
+
   describe "#format" do
     it "should return the correct parsing format" do
       @request.format.should == :xml
@@ -161,6 +174,15 @@ describe HTTParty::Request do
         @request.send(:format_from_mimetype, ct).should == :json
       end
     end
+
+    it "returns nil for an unrecognized mimetype" do
+      @request.send(:format_from_mimetype, "application/atom+xml").should be_nil
+    end
+
+    it "returns nil when using a default parser" do
+      @request.options[:parser] = lambda {}
+      @request.send(:format_from_mimetype, "text/json").should be_nil
+    end
   end
 
   describe 'parsing responses' do

spec/httparty_spec.rb

@@ -12,6 +12,20 @@ describe HTTParty do
     @klass.instance_eval { include HTTParty }
   end
 
+  describe "AllowedFormats deprecated" do
+    before do
+      Kernel.stub(:warn)
+    end
+    it "warns with a deprecation message" do
+      Kernel.should_receive(:warn).with("Deprecated: Use HTTParty::Parser::SupportedFormats")
+      HTTParty::AllowedFormats
+    end
+
+    it "returns HTTPart::Parser::SupportedFormats" do
+      HTTParty::AllowedFormats.should == HTTParty::Parser::SupportedFormats
+    end
+  end
+
   describe "base uri" do
     before(:each) do
       @klass.base_uri('api.foo.com/v1')
@@ -197,20 +211,38 @@ describe HTTParty do
   end
 
   describe "parser" do
-    before(:each) do
+    let(:parser) do
-      @parser = Proc.new{ |data| CustomParser.parse(data) }
+      Proc.new{ |data, format| CustomParser.parse(data) }
-      @klass.parser @parser
     end
 
     it "should set parser options" do
-      @klass.default_options[:parser].should == @parser
+      @klass.parser parser
+      @klass.default_options[:parser].should == parser
     end
 
     it "should be able parse response with custom parser" do
+      @klass.parser parser
       FakeWeb.register_uri(:get, 'http://twitter.com/statuses/public_timeline.xml', :body => 'tweets')
       custom_parsed_response = @klass.get('http://twitter.com/statuses/public_timeline.xml')
       custom_parsed_response[:sexy].should == true
     end
+
+    it "raises UnsupportedFormat when the parser cannot handle the format" do
+      @klass.format :json
+      class MyParser < HTTParty::Parser
+        SupportedFormats = {}
+      end
+      expect do
+        @klass.parser MyParser
+      end.to raise_error(HTTParty::UnsupportedFormat)
+    end
+
+    it 'does not validate format whe custom parser is a proc' do
+      expect do
+        @klass.format :json
+        @klass.parser lambda {|body, format|}
+      end.to_not raise_error(HTTParty::UnsupportedFormat)
+    end
   end
 
   describe "format" do
@@ -243,9 +275,21 @@ describe HTTParty do
     it 'should only print each format once with an exception' do
       lambda do
         @klass.format :foobar
-      end.should raise_error(HTTParty::UnsupportedFormat, "Must be one of: html, json, plain, xml, yaml")
+      end.should raise_error(HTTParty::UnsupportedFormat, "':foobar' Must be one of: html, json, plain, xml, yaml")
     end
 
+    it 'sets the default parser' do
+      @klass.default_options[:parser].should be_nil
+      @klass.format :json
+      @klass.default_options[:parser].should == HTTParty::Parser
+    end
+
+    it 'does not reset parser to the default parser' do
+      my_parser = lambda {}
+      @klass.parser my_parser
+      @klass.format :json
+      @klass.parser.should == my_parser
+    end
   end
 
   describe "with explicit override of automatic redirect handling" do
@@ -378,7 +422,7 @@ describe HTTParty do
     it "should parse empty response fine" do
       stub_http_response_with('empty.xml')
       result = HTTParty.get('http://foobar.com')
-      result.should == nil
+      result.should be_nil
     end
 
     it "should accept http URIs" do