From 77d11c6ca3f81537b80a882653432f9db1b7e309 Mon Sep 17 00:00:00 2001
From: "J. Morgan Lieberthal" <j.morgan.lieberthal@gmail.com>
Date: Fri, 30 Oct 2020 22:04:48 -0600
Subject: [PATCH] Fix #713

This PR fixes the issue(s) described in #713. To fix the logger option
issue, I simply delete `Request.options[:logger]` when dumping. To fix
the proc parser issue, I delete `Request.options[:parser]` if and only
if `Request.options[:logger]` is a proc. If `Request.options[:logger]`
is a regular class, `Marshal.dump` should proceed as normal. This should
not affect the `Marshal.dump` behavior described in issue #143 and fixed
by PR #618.

I have added a feature spec to make sure marshalling the request
works as intended, as well as a unit test to ensure
`Marshal.load(Marshal.dump(req))` works as it should.
---
 features/steps/httparty_steps.rb              | 17 +++++++++++++++
 ...s_marshalling_with_logger_and_proc.feature | 21 +++++++++++++++++++
 lib/httparty/request.rb                       | 12 +++++++++++
 spec/httparty/request_spec.rb                 |  9 ++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 features/supports_marshalling_with_logger_and_proc.feature

diff --git a/features/steps/httparty_steps.rb b/features/steps/httparty_steps.rb
index 5a83b43..cae5b2d 100644
--- a/features/steps/httparty_steps.rb
+++ b/features/steps/httparty_steps.rb
@@ -15,6 +15,15 @@ When /^I set my HTTParty header '(.*)' to value '(.*)'$/ do |name, value|
   @request_options[:headers][name] = value
 end
 
+When /I set my HTTParty logger option/ do
+  # TODO: make the IO something portable
+  @request_options[:logger] = Logger.new("/dev/null")
+end
+
+When /I set my HTTParty parser option to a proc/ do
+  @request_options[:parser] = proc { |body| body }
+end
+
 When /I call HTTParty#get with '(.*)'$/ do |url|
   begin
     @response_from_httparty = HTTParty.get("http://#{@host_and_port}#{url}", @request_options)
@@ -46,3 +55,11 @@ When /I call HTTParty#get with '(.*)' and a digest_auth hash:/ do |url, auth_tab
     digest_auth: { username: h["username"], password: h["password"] }
   )
 end
+
+When /I call Marshal\.dump on the response/ do
+  begin
+    Marshal.dump(@response_from_httparty)
+  rescue TypeError => e
+    @exception_from_httparty = e
+  end
+end
diff --git a/features/supports_marshalling_with_logger_and_proc.feature b/features/supports_marshalling_with_logger_and_proc.feature
new file mode 100644
index 0000000..e4a16b3
--- /dev/null
+++ b/features/supports_marshalling_with_logger_and_proc.feature
@@ -0,0 +1,21 @@
+Feature: Supports marshalling with request logger and/or proc parser
+  In order to support caching responses
+  As a developer
+  I want the request to be able to be marshalled if I have set up a custom
+  logger or have a proc as the response parser.
+
+  Scenario: Marshal response with request logger
+    Given a remote service that returns '{ "some": "data" }'
+    And that service is accessed at the path '/somedata.json'
+    When I set my HTTParty logger option
+    And I call HTTParty#get with '/somedata.json'
+    And I call Marshal.dump on the response
+    Then it should not raise a TypeError exception
+
+  Scenario: Marshal response with proc parser
+    Given a remote service that returns '{ "some": "data" }'
+    And that service is accessed at the path '/somedata.json'
+    When I set my HTTParty parser option to a proc
+    And I call HTTParty#get with '/somedata.json'
+    And I call Marshal.dump on the response
+    Then it should not raise a TypeError exception
diff --git a/lib/httparty/request.rb b/lib/httparty/request.rb
index aecc114..77d01ba 100644
--- a/lib/httparty/request.rb
+++ b/lib/httparty/request.rb
@@ -44,6 +44,11 @@ module HTTParty
       end.flatten.join('&')
     end
 
+    def self._load(data)
+      http_method, path, options = Marshal.load(data)
+      new(http_method, path, options)
+    end
+
     attr_accessor :http_method, :options, :last_response, :redirect, :last_uri
     attr_reader :path
 
@@ -173,6 +178,13 @@ module HTTParty
       @raw_request.body
     end
 
+    def _dump(_level)
+      opts = options.dup
+      opts.delete(:logger)
+      opts.delete(:parser) if opts[:parser] && opts[:parser].is_a?(Proc)
+      Marshal.dump([http_method, path, opts])
+    end
+
     private
 
     def http
diff --git a/spec/httparty/request_spec.rb b/spec/httparty/request_spec.rb
index 4b4c237..2ef6aef 100644
--- a/spec/httparty/request_spec.rb
+++ b/spec/httparty/request_spec.rb
@@ -1387,4 +1387,13 @@ RSpec.describe HTTParty::Request do
       expect(request.instance_variable_get(:@raw_request).decode_content).to eq(true)
     end
   end
+
+  describe "marshalling" do
+    it "properly marshals the request object" do
+      marshalled = Marshal.load(Marshal.dump(@request))
+      expect(marshalled.http_method).to eq @request.http_method
+      expect(marshalled.path).to eq @request.path
+      expect(marshalled.options).to eq @request.options
+    end
+  end
 end