Rework response to have a common mixin.

lib/stripe/stripe_response.rb

@@ -1,63 +1,54 @@
 # frozen_string_literal: true
 
 module Stripe
-  # StripeResponse encapsulates some vitals of a response that came back from
-  # the Stripe API.
-  class StripeResponse
-    # Headers provides an access wrapper to an API response's header data. It
-    # mainly exists so that we don't need to expose the entire
-    # `Net::HTTPResponse` object while still getting some of its benefits like
-    # case-insensitive access to header names and flattening of header values.
-    class Headers
-      # Initializes a Headers object from a Net::HTTP::HTTPResponse object.
-      def self.from_net_http(resp)
-        new(resp.to_hash)
-      end
+  # Headers provides an access wrapper to an API response's header data. It
+  # mainly exists so that we don't need to expose the entire
+  # `Net::HTTPResponse` object while still getting some of its benefits like
+  # case-insensitive access to header names and flattening of header values.
+  class StripeResponseHeaders
+    # Initializes a Headers object from a Net::HTTP::HTTPResponse object.
+    def self.from_net_http(resp)
+      new(resp.to_hash)
+    end
 
-      # `hash` is expected to be a hash mapping header names to arrays of
-      # header values. This is the default format generated by calling
-      # `#to_hash` on a `Net::HTTPResponse` object because headers can be
-      # repeated multiple times. Using `#[]` will collapse values down to just
-      # the first.
-      def initialize(hash)
-        if !hash.is_a?(Hash) ||
-           !hash.keys.all? { |n| n.is_a?(String) } ||
-           !hash.values.all? { |a| a.is_a?(Array) } ||
-           !hash.values.all? { |a| a.all? { |v| v.is_a?(String) } }
-          raise ArgumentError,
-                "expect hash to be a map of string header names to arrays of " \
-                "header values"
-        end
-
-        @hash = {}
-
-        # This shouldn't be strictly necessary because `Net::HTTPResponse` will
-        # produce a hash with all headers downcased, but do it anyway just in
-        # case an object of this class was constructed manually.
-        #
-        # Also has the effect of duplicating the hash, which is desirable for a
-        # little extra object safety.
-        hash.each do |k, v|
-          @hash[k.downcase] = v
-        end
+    # `hash` is expected to be a hash mapping header names to arrays of
+    # header values. This is the default format generated by calling
+    # `#to_hash` on a `Net::HTTPResponse` object because headers can be
+    # repeated multiple times. Using `#[]` will collapse values down to just
+    # the first.
+    def initialize(hash)
+      if !hash.is_a?(Hash) ||
+         !hash.keys.all? { |n| n.is_a?(String) } ||
+         !hash.values.all? { |a| a.is_a?(Array) } ||
+         !hash.values.all? { |a| a.all? { |v| v.is_a?(String) } }
+        raise ArgumentError,
+              "expect hash to be a map of string header names to arrays of " \
+              "header values"
       end
 
-      def [](name)
-        values = @hash[name.downcase]
-        if values && values.count > 1
-          warn("Duplicate header values for `#{name}`; returning only first")
-        end
-        values ? values.first : nil
+      @hash = {}
+
+      # This shouldn't be strictly necessary because `Net::HTTPResponse` will
+      # produce a hash with all headers downcased, but do it anyway just in
+      # case an object of this class was constructed manually.
+      #
+      # Also has the effect of duplicating the hash, which is desirable for a
+      # little extra object safety.
+      hash.each do |k, v|
+        @hash[k.downcase] = v
       end
     end
 
-    # The data contained by the HTTP body of the response deserialized from
-    # JSON.
-    attr_accessor :data
-
-    # The raw HTTP body of the response.
-    attr_accessor :http_body
+    def [](name)
+      values = @hash[name.downcase]
+      if values && values.count > 1
+        warn("Duplicate header values for `#{name}`; returning only first")
+      end
+      values ? values.first : nil
+    end
+  end
 
+  module StripeResponseBase
     # A Hash of the HTTP headers of the response.
     attr_accessor :http_headers
 
@@ -67,41 +58,54 @@ def [](name)
     # The Stripe request ID of the response.
     attr_accessor :request_id
 
+    def self.populate_for_net_http(resp, http_resp)
+      resp.http_headers = StripeResponseHeaders.from_net_http(http_resp)
+      resp.http_status = http_resp.code.to_i
+      resp.request_id = http_resp["request-id"]
+    end
+  end
+
+  # StripeResponse encapsulates some vitals of a response that came back from
+  # the Stripe API.
+  class StripeResponse
+    include StripeResponseBase
+    # The data contained by the HTTP body of the response deserialized from
+    # JSON.
+    attr_accessor :data
+
+    # The raw HTTP body of the response.
+    attr_accessor :http_body
+
     # Initializes a StripeResponse object from a Net::HTTP::HTTPResponse
     # object.
     def self.from_net_http(http_resp)
       resp = StripeResponse.new
       resp.data = JSON.parse(http_resp.body, symbolize_names: true)
       resp.http_body = http_resp.body
-      resp.http_headers = Headers.from_net_http(http_resp)
-      resp.http_status = http_resp.code.to_i
-      resp.request_id = http_resp["request-id"]
+      StripeResponseBase.populate_for_net_http(resp, http_resp)
       resp
     end
   end
 
+  # We have to alias StripeResponseHeaders to StripeResponse::Headers, as this
+  # class used to be embedded within StripeResponse and we want to be backwards
+  # compatible.
+  StripeResponse::Headers = StripeResponseHeaders
+
   # StripeStreamResponse includes header-related vitals of the
-  # response as well as an IO stream of the body content.
+  # response as well as as the raw HTTP body content.
   class StripeStreamResponse
-    attr_accessor :io
-
-    # A Hash of the HTTP headers of the response.
-    attr_accessor :http_headers
-
-    # The integer HTTP status code of the response.
-    attr_accessor :http_status
+    include StripeResponseBase
 
-    # The Stripe request ID of the response.
-    attr_accessor :request_id
+    # The raw HTTP body of the response.
+    attr_accessor :http_body
 
     # Initializes a StripeStreamResponse object from a Net::HTTP::HTTPResponse
     # object.
     def self.from_net_http(http_resp)
       resp = StripeStreamResponse.new
-      resp.io = StringIO.new http_resp.body
-      resp.http_headers = StripeResponse::Headers.from_net_http(http_resp)
-      resp.http_status = http_resp.code.to_i
-      resp.request_id = http_resp["request-id"]
+      resp.http_body = http_resp.body
+      StripeResponseBase.populate_for_net_http(resp, http_resp)
       resp
     end
   end
@@ -111,22 +115,13 @@ def self.from_net_http(http_resp)
   # directly in a block and we explicitly don't want to store the body of the
   # response in memory.
   class StripeHeadersOnlyResponse
-    # A Hash of the HTTP headers of the response.
-    attr_accessor :http_headers
-
-    # The integer HTTP status code of the response.
-    attr_accessor :http_status
-
-    # The Stripe request ID of the response.
-    attr_accessor :request_id
+    include StripeResponseBase
 
     # Initializes a StripeHeadersOnlyResponse object from a
     # Net::HTTP::HTTPResponse object.
     def self.from_net_http(http_resp)
       resp = StripeHeadersOnlyResponse.new
-      resp.http_headers = StripeResponse::Headers.from_net_http(http_resp)
-      resp.http_status = http_resp.code.to_i
-      resp.request_id = http_resp["request-id"]
+      StripeResponseBase.populate_for_net_http(resp, http_resp)
       resp
     end
   end

test/stripe/api_resource_test.rb

@@ -662,7 +662,7 @@ def read_stream(params = {}, opts = {}, &read_body_chunk_block)
         resp = StreamTestAPIResource.new(id: "hi_123").read_stream({ foo: "bar" }, stripe_account: "acct_hi")
 
         assert_instance_of Stripe::StripeStreamResponse, resp
-        assert_equal "response body", resp.io.read
+        assert_equal "response body", resp.http_body
       end
     end
 

test/stripe/stripe_client_test.rb

@@ -1114,7 +1114,7 @@ class StripeClientTest < Test::Unit::TestCase
         resp, = client.execute_request_stream(:post, "/v1/charges")
 
         assert_instance_of Stripe::StripeStreamResponse, resp
-        assert_equal "response body", resp.io.read
+        assert_equal "response body", resp.http_body
       end
 
       should "executes the read_body_chunk_block when passed" do

test/stripe/stripe_response_test.rb

@@ -4,47 +4,47 @@
 
 module Stripe
   class StripeResponseTest < Test::Unit::TestCase
-    context "Headers" do
+    context "StripeResponseHeaders" do
       should "allow case-insensitive header access" do
         headers = { "Request-Id" => "request-id" }
         http_resp = create_net_http_resp(200, "", headers)
 
-        headers = StripeResponse::Headers.from_net_http(http_resp)
+        headers = StripeResponseHeaders.from_net_http(http_resp)
 
         assert_equal "request-id", headers["request-id"]
         assert_equal "request-id", headers["Request-Id"]
         assert_equal "request-id", headers["Request-ID"]
       end
 
       should "initialize without error" do
-        StripeResponse::Headers.new({})
-        StripeResponse::Headers.new("Request-Id" => [])
-        StripeResponse::Headers.new("Request-Id" => ["request-id"])
+        StripeResponseHeaders.new({})
+        StripeResponseHeaders.new("Request-Id" => [])
+        StripeResponseHeaders.new("Request-Id" => ["request-id"])
       end
 
       should "initialize with error on a malformed hash" do
         assert_raises(ArgumentError) do
-          StripeResponse::Headers.new(nil)
+          StripeResponseHeaders.new(nil)
         end
 
         assert_raises(ArgumentError) do
-          StripeResponse::Headers.new(1 => [])
+          StripeResponseHeaders.new(1 => [])
         end
 
         assert_raises(ArgumentError) do
-          StripeResponse::Headers.new("Request-Id" => 1)
+          StripeResponseHeaders.new("Request-Id" => 1)
         end
 
         assert_raises(ArgumentError) do
-          StripeResponse::Headers.new("Request-Id" => [1])
+          StripeResponseHeaders.new("Request-Id" => [1])
         end
       end
 
       should "warn on duplicate header values" do
         old_stderr = $stderr
         $stderr = StringIO.new
         begin
-          headers = StripeResponse::Headers.new("Duplicated" => %w[a b])
+          headers = StripeResponseHeaders.new("Duplicated" => %w[a b])
           assert_equal "a", headers["Duplicated"]
           assert_equal "Duplicate header values for `Duplicated`; returning only first",
                        $stderr.string.rstrip
@@ -54,20 +54,61 @@ class StripeResponseTest < Test::Unit::TestCase
       end
     end
 
-    context ".from_net_http" do
-      should "converts to StripeResponse" do
-        code = 200
-        body = '{"foo": "bar"}'
-        headers = { "Request-Id" => "request-id" }
-        http_resp = create_net_http_resp(code, body, headers)
+    [StripeResponse, StripeStreamResponse, StripeHeadersOnlyResponse].each do |response_class|
+      context "StripeResponseBase mixin for #{response_class}" do
+        context ".from_net_http" do
+          should "populate the base fields" do
+            code = 200
+            body = '{"foo": "bar"}'
+            headers = { "Request-Id" => "request-id" }
+            http_resp = create_net_http_resp(code, body, headers)
+
+            resp = response_class.from_net_http(http_resp)
+
+            assert_equal "request-id", resp.http_headers["Request-ID"]
+            assert_equal code, resp.http_status
+            assert_equal "request-id", resp.request_id
+          end
+        end
+      end
+    end
+
+    context "#StripeResponse" do
+      context ".from_net_http" do
+        should "converts to StripeResponse" do
+          code = 200
+          body = '{"foo": "bar"}'
+          http_resp = create_net_http_resp(code, body, {})
+
+          resp = StripeResponse.from_net_http(http_resp)
+
+          assert_instance_of StripeResponse, resp
+          assert_equal JSON.parse(body, symbolize_names: true), resp.data
+          assert_equal body, resp.http_body
+        end
+      end
+
+      context "Headers backwards compatibility" do
+        should "alias StripeResponseHeaders" do
+          headers = StripeResponse::Headers.new("Request-Id" => ["request-id"])
+
+          assert_instance_of StripeResponseHeaders, headers
+        end
+      end
+    end
 
-        resp = StripeResponse.from_net_http(http_resp)
+    context "#StripeStreamResponse" do
+      context ".from_net_http" do
+        should "converts to StripeStreamResponse" do
+          code = 200
+          body = '{"foo": "bar"}'
+          http_resp = create_net_http_resp(code, body, {})
 
-        assert_equal JSON.parse(body, symbolize_names: true), resp.data
-        assert_equal body, resp.http_body
-        assert_equal "request-id", resp.http_headers["Request-ID"]
-        assert_equal code, resp.http_status
-        assert_equal "request-id", resp.request_id
+          resp = StripeStreamResponse.from_net_http(http_resp)
+
+          assert_instance_of StripeStreamResponse, resp
+          assert_equal body, resp.http_body
+        end
       end
     end