Use Rack::Headers to avoid deprecation warning about Rack 3

[3UR]

This resolves #239

[olleolleolle]

Thanks, @3UR for updating the code.

Background: The Rack::Headers class was introduced in Rack rack/rack@79328de in 3.0.

Blocker: httpi's gemspec lists rack < 3, so this can not go in yet.

https://github.com/savonrb/httpi/blob/master/httpi.gemspec#L24

And, more context to that blocker: savonrb/savon#992

[pcai]

Thanks @3UR and @olleolleolle - correct, this can't go in as-is. This is unlikely to go in at all, since this is a major breaking change that affects a public API method, and we're focused on maintenance only at the moment - see also #238

[dorianmarie]

Couldn't we check the Rack version and use Rack::Utils::HeaderHash or Rack::Headers accordingly?

[pcai]

@dorianmarie we could, but I disagree with that design choice: A library’s public API should not depend on the version of a transitive dependency. A change in API should be a change in version.

[gbs4ever]

so how can we silence this Rack::Utils::HeaderHash is deprecated warning

[pcai]

@gbs4ever

presumably you could

• use rack 2.x
• if you need rack 3+, monkeypatch it in classic ruby style. In rails the (untested) pseudocode would probably look like:

# config/initializers/httpi.rb

# use rack 3.x interface to silence a warning
module HTTPI
  class Response
    def initialize(code, headers, body)
      self.code = code.to_i
      self.headers = Rack::Headers.new(headers)
      self.raw_body = body
    end
  end
  
  class Request
    def headers
      @headers ||= Rack::Headers.new
    end
  end
end

[gbs4ever]

Thank you @pcai
upon testing Rack::Headers.new does not take arguments which is why this is my idea

Rails.logger.warn "Rewrote Httpi due to Rack::Utils::HeaderHash.new being deprecated https://github.com/rack/rack/blob/main/UPGRADE-GUIDE.md?plain=1 "
module HTTPI
 class Response
   def initialize(code, headers, body)
     self.code = code.to_i
     self.headers = !headers.is_a?(Hash) ? Rack::Headers.new.merge!(headers) : headers
     self.raw_body = body
   end
 end

 class Request
   def headers
     @headers ||= Rack::Headers.new
   end

   def headers=(headers)
     @headers = Rack::Headers.new.merge!(headers)
   end
 end
end`

[Slotos]

@dorianmarie we could, but I disagree with that design choice: A library’s public API should not depend on the version of a transitive dependency. A change in API should be a change in version.

The problem lies in #headers leaking implementation details. Would it return normalized library specific headers object, the library would be free to pick and choose whichever implementation fits the bill.

[gbs4ever]

It seems to be incorrect this version @3UR and @pcai as Rack::Headers.new does not accept a hash as an argument see below

Rack::Headers.new(headers) => {} vs  Rack::Utils::HeaderHash.new(headers) =>  {
 "Content-Type" => "application/json",
 "Authorization" => "Bearer your_token_here",
}

 This would be more correct
 
  
  Rack::Headers.new.merge!(headers)

[pcai]

Thanks everyone for your contributions.

I took some time to investigate and this is now fixed in 56cd67b which ended up being relatively minor overall.

I couldn't use this PR - as others have pointed out, the code isn't correct, and it doesnt account for tests and other dependencies that need updating (namely puma 6.x)

[pcai]

also @Slotos re:

The problem lies in #headers leaking implementation details. Would it return normalized library specific headers object, the library would be free to pick and choose whichever implementation fits the bill.

I agree that this is a mistake in the initial API design. But there is no turning back the clock on this one unfortunately.

[pcai]

Since it turns out that rack 3.x and 2.x have disjoint APIs and httpi 4.x is a public API change anyway, I actually went and implemented the suggestion by @Slotos by vendoring the class: #245

I invite participants to review this change and release strategy.