Support multiple JSON mime types

[thestrabusiness]

🧰 What's being changed?

This commit expands the content type check in the
Har::ResponseSerializer to accept multiple JSON mime types.

🧪 Testing

Using the following sample application:

$LOAD_PATH << File.expand_path("../../metrics-sdks/packages/ruby/lib", __FILE__)
require "readme/metrics"

class ApiApp
  def call(env)
    [
      200,
      {"Content-Length" => "20", "Content-Type" => "application/x-json"},
      [{response: "value"}.to_json]
    ]
  end
end

map "/api" do
  options = {
    api_key: "api_key",
    buffer_length: 1
  }
  use Readme::Metrics, options do |env|
    {id: "anthonym", label: "Anthony M.", email: "[email protected]"}
  end
  run ApiApp.new
end

And the following curl command:

curl 'http://username:password@localhost:9292/api/foo' -X POST

A request was successfully added to the metrics dashboard with a newly added json mime type.

Automated test coverage was also expanded

[JoelQ]

How would you feel about moving this to something like HttpRequest#has_json_body? It would be easier to unit-test over there and it fits with the generic request responsibility of the object?

[thestrabusiness]

That sounds good 👍

[thestrabusiness]

Oh huh. This doesn't fit with the HttpRequest object. @response here is a Rack::Response. Does it make sense to wrap that in a new class to add this has_json_body? functionality and test it there?

[JoelQ]

Oh, I'm getting confused between request and response 🤦‍♂️

Would it make sense to extract a HttpResponse object similar to what was done with HttpRequest? Or maybe leave everything in here.

[thestrabusiness]

Looking at this again I think that it makes sense to just leave everything here. It doesn't seem like we gain much by wrapping the response.

[JoelQ]

Was this test red before you implemented the change? My guess is that this would be true no matter the mime type as long as there is some kind of body

[thestrabusiness]

This test was red. The JSON content type was hardcoded as "application/json"

[JoelQ]

Oh I see, so it failed for application/json but passed for the others.