fix: Support Addressable::URI in request patterns

[ixti]

As of 3.24.0 release it became impossible to pass Addressable::URI as pattern, while Addressable::Template is still supported. This patch brings back previous behaviour of Addressable::URI.

Regression: f651c25

[bblimke]

Hey @ixti,

Thank you for the PR.

If passing Addressable::URI as a request pattern worked, it was only accidental, as this was never officially supported. The change introduced in f651c25 was added as a safety check to ensure WebMock doesn't accept any unsupported argument types.

Addressable::Template is supported because it offers functionality for URI templates as per RFC 6570, which can't be expressed by any native Ruby type or string.

Since WebMock depends on Addressable::URI, I can consider supporting Addressable::URI for request patterns in the future. However, it would be helpful if you could explain why adding support for this is worthwhile and why existing types are not sufficient.

[ixti]

If passing Addressable::URI as a request pattern worked, it was only accidental, as this was never officially supported. The change introduced in f651c25 was added as a safety check to ensure WebMock doesn't accept any unsupported argument types.

LOL. Good to know. :D

Addressable::Template is supported because it offers functionality for URI templates as per RFC 6570, which can't be expressed by any native Ruby type or string.

When I saw that Addressable::Template was supported, I've assumed removal of Addressable::URI is a regression.

Since WebMock depends on Addressable::URI, I can consider supporting Addressable::URI for request patterns in the future. However, it would be helpful if you could explain why adding support for this is worthwhile and why existing types are not sufficient.

Just upgraded webmock and got a bunch of specs failing as we're using Addressable::URI in specs:

module Example
  class Application
    ADMIN_ORIGIN = Addressable::URI.parse(ENV.fetch("ADMIN_ORIGIN"))
  end
end

it "works like a charm" do
  stub_request(:get, ADMIN_ORIGIN.join("/some/path")).to_return(status: 200)

  ...
end

But as Addressable::URI was not meant to be supported, I can simply change my specs to call #to_s before passing the URI

[ixti]

After giving it another thought, if that's expected behaviour (no accepting Addressable::URI for any reason in fact), then I'm cool with that. Simply thought it was a regression, as gem depends on Addressable. :D

Closing the PR, as it's not a problem (adding #to_s on my end is not problematic, and frankly helped to spot a couple of problematic places to improve specs readability).

PS: On the other hand, if Addressable::Template is supported, it seems like supporting Addressable::URI is pretty OK-ish (at least in my head).

[bblimke]

@ixti Thank you. Supporting Addressable::URI is ok. My main consideration was avoiding unnecessary feature additions that would increase maintenance overhead. If there's significant user demand for Addressable::URI support, I'm happy to merge this PR and maintain the feature. To be honest, it's not a huge overhead to maintain it, but still I prefer to avoid accumulating features that are not necessary.

Support Addressable::Template on the other hand was essential, as there is no alternative for defining URI templates.

[postmodern]

If stub_request allows String values, why not automatically convert URI and Addressable::URI objects to String using .to_s?

[ixti]

@postmodern @bblimke WDYT about:

  elsif uri.respond_to?(:to_str)
    URIStringPattern.new(uri)
  else

That way it will support Addressable::URI, String, and anything that effectively representation of a String :D

See: 449c0a3