From 371c1cdb5a1d07e8ef8d2eb830895bc9d0d63a23 Mon Sep 17 00:00:00 2001 From: Felix Kleinschmidt Date: Mon, 3 Mar 2025 11:34:10 -0200 Subject: [PATCH 1/4] replace Excon with Net::HTTP --- lib/quickpay/api/client.rb | 69 +++++++++---- quickpay-ruby-client.gemspec | 2 +- test/client.rb | 194 +++++++++-------------------------- 3 files changed, 98 insertions(+), 167 deletions(-) diff --git a/lib/quickpay/api/client.rb b/lib/quickpay/api/client.rb index a86bda6..5495309 100644 --- a/lib/quickpay/api/client.rb +++ b/lib/quickpay/api/client.rb @@ -1,5 +1,5 @@ -require "excon" require "json" +require "net/http" require "quickpay/api/error" require "quickpay/api/version" @@ -16,20 +16,28 @@ class Client Request = Struct.new(:method, :path, :body, :headers, :query) # rubocop:disable Lint/StructNewOverride def initialize(username: nil, password: nil, base_uri: "https://api.quickpay.net", options: {}) - opts = { - read_timeout: options.fetch(:read_timeout, 60), - write_timeout: options.fetch(:write_timeout, 60), - connect_timeout: options.fetch(:connect_timeout, 60), - json_opts: options.fetch(:json_opts, nil) - } - - opts[:username] = Excon::Utils.escape_uri(username) if username - opts[:password] = Excon::Utils.escape_uri(password) if password - - @connection = Excon.new(base_uri, opts) + @read_timeout = options.fetch(:read_timeout, 60) + @write_timeout = options.fetch(:write_timeout, 60) + @connect_timeout = options.fetch(:connect_timeout, 60) + @json_opts = options.fetch(:json_opts, nil) + + uri_parser = URI::Parser.new + @username = uri_parser.escape(username) if username + @password = uri_parser.escape(password) if password + @base_uri = base_uri end - %i[get post patch put delete head].each do |method| + HTTPS = "https".freeze + + [ + Net::HTTP::Get, + Net::HTTP::Post, + Net::HTTP::Patch, + Net::HTTP::Put, + Net::HTTP::Delete, + Net::HTTP::Head + ].each do |method_class| + method = method_class.to_s.split("::").last.downcase define_method(method) do |path, **options, &block| headers = DEFAULT_HEADERS.merge(options.fetch(:headers, {})) body = begin @@ -42,29 +50,48 @@ def initialize(username: nil, password: nil, base_uri: "https://api.quickpay.net end req = Request.new( - method, + method.to_sym, path, scrub_body(body.dup, headers["Content-Type"]), headers, - options.fetch(:query, {}) + options[:query] ).freeze - res = @connection.request(**req.to_h) - error = QuickPay::API::Error.by_status_code(res.status, res.body, res.headers, req) + uri = URI(@base_uri) + uri.path << req.path + if (query = req.query) && query.any? + uri.query = URI.encode_www_form(req.query) + end + net_req = method_class.new(uri, req.headers) + net_req.basic_auth(@username, @password) if @username || @password + net_req.body = req.body + res = Net::HTTP.start( + uri.hostname, + use_ssl: uri.scheme == HTTPS, + open_timeout: @connect_timeout, + read_timeout: @read_timeout, + write_timeout: @write_timeout + ) do |http| + http.request(net_req) + end + status_code = res.code.to_i + body = res.body + headers = res.each_header.to_h + error = QuickPay::API::Error.by_status_code(status_code, body, headers, req) - if !options.fetch(:raw, false) && res.headers["Content-Type"] =~ CONTENT_TYPE_JSON_REGEX - res.body = JSON.parse(res.body, options[:json_opts] || @connection.data[:json_opts]) + if !options.fetch(:raw, false) && res["content-type"] =~ CONTENT_TYPE_JSON_REGEX + body = JSON.parse(body, options[:json_opts] || @json_opts) end if block # Raise error if not specified as fourth block parameter raise error if error && block.parameters.size < 4 - block.call(res.body, res.status, res.headers, error) + block.call(body, status_code, headers, error) else raise error if error - [res.body, res.status, res.headers] + [body, status_code, headers] end end end diff --git a/quickpay-ruby-client.gemspec b/quickpay-ruby-client.gemspec index 4aa857d..c2c3efa 100644 --- a/quickpay-ruby-client.gemspec +++ b/quickpay-ruby-client.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency "rubocop" spec.add_development_dependency "simplecov" spec.add_development_dependency "simplecov-console" + spec.add_development_dependency "webmock" - spec.add_dependency "excon", ">= 0.79" spec.add_dependency "json", "~> 2", ">= 2.5" end diff --git a/test/client.rb b/test/client.rb index aa1fcb6..a932fd8 100644 --- a/test/client.rb +++ b/test/client.rb @@ -9,70 +9,41 @@ minimum_coverage 100 end -require "excon" -require "json" require "minitest/autorun" +require "webmock/minitest" require "quickpay/api/client" -Excon.defaults[:mock] = true - describe QuickPay::API::Client do before do - # Excon expects two hashes - Excon.stub({}, { body: "Unknown Stub", status: 500 }) - end - - after do - Excon.stubs.clear + stub_request(:any, //).to_return(body: "Unknown Stub", status: 500) end it "set default headers" do - Excon.stub( - { path: "/ping" }, - lambda do |request_params| - { - headers: request_params[:headers], - status: 200 - } - end - ) + stub_request(:get, %r{/ping}).to_return { |request| { headers: request.headers, status: 200 } } client = QuickPay::API::Client.new _, _, headers = *client.get("/ping") - _(headers["Accept-Version"]).must_equal "v10" - _(headers["User-Agent"]).must_equal "quickpay-ruby-client, v#{QuickPay::API::VERSION}" + _(headers["accept-version"]).must_equal "v10" + _(headers["user-agent"]).must_equal "quickpay-ruby-client, v#{QuickPay::API::VERSION}" end it "handles authentication" do - Excon.stub( - { path: "/ping" }, - lambda do |request_params| - { - headers: request_params[:headers], - status: 200 - } - end - ) + stub_request(:get, %r{/ping}).to_return { |request| { headers: request.headers, status: 200 } } client = QuickPay::API::Client.new(password: "secret") _, _, headers = *client.get("/ping") - _(headers["Authorization"]).must_equal "Basic OnNlY3JldA==" + _(headers["authorization"]).must_equal "Basic OnNlY3JldA==" end describe "JSON <=> Hash conversion of body" do subject { QuickPay::API::Client.new } it "returns JSON string if content type is not set" do - Excon.stub( - { path: "/ping" }, - lambda do |request_params| - { - body: request_params[:body], - status: 200 - } - end + stub_request(:post, %r{/ping}).to_return( + body: JSON.generate({ "foo" => "bar" }), + status: 200 ) # client return JSON string @@ -86,16 +57,9 @@ end it "returns ruby Hash if content type is set" do - Excon.stub( - { path: "/ping" }, - lambda do |request_params| - { - body: request_params[:body], - headers: { "Content-Type" => "application/json" }, - status: 200 - } - end - ) + stub_request(:post, %r{/ping}).to_return do |request| + { body: request.body, headers: { "Content-Type" => "application/json" }, status: 200 } + end # client returns Ruby Hash with string keys subject.post( @@ -118,16 +82,9 @@ end it "returns a ruby Hash if content type is weird application/json" do - Excon.stub( - { path: "/ping" }, - lambda do |request_params| - { - body: request_params[:body], - headers: { "Content-Type" => "application/stuff+json" }, - status: 200 - } - end - ) + stub_request(:post, %r{/ping}).to_return do |request| + { body: request.body, headers: { "Content-Type" => "application/json" }, status: 200 } + end # client returns Ruby Hash with string keys subject.post( @@ -144,19 +101,14 @@ subject { QuickPay::API::Client.new } it "is called for success" do - Excon.stub( - { path: "/ping" }, - { - status: 200, - body: %({"message":"pong"}), - headers: { "Content-Type" => "application/json" } - } - ) + stub_request(:get, %r{/ping}).to_return do + { body: %({"message":"pong"}), headers: { "Content-Type" => "application/json" }, status: 200 } + end called = subject.get("/ping", json_opts: { symbolize_names: true }) do |body, status, headers, error| _(body[:message]).must_equal "pong" _(status).must_equal 200 - _(headers["Content-Type"]).must_equal "application/json" + _(headers["content-type"]).must_equal "application/json" _(error).must_be :nil? true @@ -165,7 +117,7 @@ end it "is called for non success with error block param" do - Excon.stub({ path: "/ping" }, { status: 404 }) + stub_request(:get, %r{/ping}).to_return(status: 404) called = subject.get "/ping", json_opts: { symbolize_names: true } do |_, status, _, error| _(status).must_equal 404 @@ -177,7 +129,7 @@ end it "is not called for non success without error block param" do - Excon.stub({ path: "/ping" }, { status: 404 }) + stub_request(:get, %r{/ping}).to_return(status: 404) assert_raises QuickPay::API::Error::NotFound do subject.get "/ping", json_opts: { symbolize_names: true } do |_, status| @@ -191,74 +143,26 @@ it "raises predefined errors" do client = QuickPay::API::Client.new - assert_raises QuickPay::API::Error::BadRequest do - Excon.stub({ path: "/ping" }, { status: 400 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::Unauthorized do - Excon.stub({ path: "/ping" }, { status: 401 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::PaymentRequired do - Excon.stub({ path: "/ping" }, { status: 402 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::Forbidden do - Excon.stub({ path: "/ping" }, { status: 403 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::NotFound do - Excon.stub({ path: "/ping" }, { status: 404 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::MethodNotAllowed do - Excon.stub({ path: "/ping" }, { status: 405 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::NotAcceptable do - Excon.stub({ path: "/ping" }, { status: 406 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::Conflict do - Excon.stub({ path: "/ping" }, { status: 409 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::TooManyRequest do - Excon.stub({ path: "/ping" }, { status: 429 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::InternalServerError do - Excon.stub({ path: "/ping" }, { status: 500 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::BadGateway do - Excon.stub({ path: "/ping" }, { status: 502 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::ServiceUnavailable do - Excon.stub({ path: "/ping" }, { status: 503 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error::GatewayTimeout do - Excon.stub({ path: "/ping" }, { status: 504 }) - client.get("/ping") - end - - assert_raises QuickPay::API::Error do - Excon.stub({ path: "/ping" }, { status: 418 }) - client.get("/ping") + [ + [QuickPay::API::Error::BadRequest, 400], + [QuickPay::API::Error::Unauthorized, 401], + [QuickPay::API::Error::PaymentRequired, 402], + [QuickPay::API::Error::Forbidden, 403], + [QuickPay::API::Error::NotFound, 404], + [QuickPay::API::Error::MethodNotAllowed, 405], + [QuickPay::API::Error::NotAcceptable, 406], + [QuickPay::API::Error::Conflict, 409], + [QuickPay::API::Error::TooManyRequest, 429], + [QuickPay::API::Error::InternalServerError, 500], + [QuickPay::API::Error::BadGateway, 502], + [QuickPay::API::Error::ServiceUnavailable, 503], + [QuickPay::API::Error::GatewayTimeout, 504], + [QuickPay::API::Error, 418] + ].each do |error, status| + stub_request(:get, %r{/ping}).to_return(status: status) + assert_raises error do + client.get("/ping") + end end end @@ -266,7 +170,7 @@ client = QuickPay::API::Client.new e = assert_raises QuickPay::API::Error do - Excon.stub({ path: "/ping" }, { status: 409, body: "Conflict", headers: { "Foo" => "bar" } }) + stub_request(:post, %r{/ping}).to_return(status: 409, body: "Conflict", headers: { "Foo" => "bar" }) client.post( "/ping", body: "foo=bar&baz=qux", @@ -275,15 +179,15 @@ end _(e.status).must_equal 409 _(e.body).must_equal "Conflict" - _(e.headers).must_equal({ "Foo" => "bar" }) + _(e.headers).must_equal({ "foo" => "bar" }) _(e.request.method).must_equal :post _(e.request.body).must_equal "foo=bar&baz=qux" - _(e.request.headers.fetch("Accept-Version")).must_equal "v10" - _(e.request.headers.fetch("User-Agent")).must_equal "quickpay-ruby-client, v#{QuickPay::API::VERSION}" - _(e.request.query).must_equal({}) + _(e.request.headers["Accept-Version"]).must_equal "v10" + _(e.request.headers["User-Agent"]).must_equal "quickpay-ruby-client, v#{QuickPay::API::VERSION}" + _(e.request.query).must_equal(nil) e = assert_raises QuickPay::API::Error do - Excon.stub({ path: "/upload" }, { status: 409, body: "Conflict", headers: { "Foo" => "bar" } }) + stub_request(:post, %r{/upload}).to_return(status: 409, body: "Conflict", headers: { "Foo" => "bar" }) client.post( "/upload", body: "binary data", @@ -293,7 +197,7 @@ end _(e.inspect).must_equal <<~ERR.strip - #"bar"} \ + #"bar"} \ request=#"quickpay-ruby-client, v#{QuickPay::API::VERSION}", \ From 055b30f722a534da6d027f6fd58f5a876bd0d76b Mon Sep 17 00:00:00 2001 From: Felix Kleinschmidt Date: Mon, 3 Mar 2025 12:02:27 -0200 Subject: [PATCH 2/4] bump version to 4.0.0 --- lib/quickpay/api/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/quickpay/api/version.rb b/lib/quickpay/api/version.rb index 1eccabc..465a761 100644 --- a/lib/quickpay/api/version.rb +++ b/lib/quickpay/api/version.rb @@ -1,5 +1,5 @@ module QuickPay module API - VERSION = "3.0.2".freeze + VERSION = "4.0.0".freeze end end From 9fc010d9b86e17d57d01bf5443a156a48c16af5f Mon Sep 17 00:00:00 2001 From: Felix Kleinschmidt Date: Mon, 3 Mar 2025 12:06:04 -0200 Subject: [PATCH 3/4] move development dependecies to Gemfile --- Gemfile | 9 +++++++++ quickpay-ruby-client.gemspec | 9 --------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Gemfile b/Gemfile index a186537..a5d919d 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,13 @@ source "https://rubygems.org" +gem "bundler" +gem "minitest" +gem "pry" +gem "rake" +gem "rubocop" +gem "simplecov" +gem "simplecov-console" +gem "webmock" + # Specify your gem's dependencies in quickpay-ruby-client.gemspec gemspec diff --git a/quickpay-ruby-client.gemspec b/quickpay-ruby-client.gemspec index c2c3efa..c425b9f 100644 --- a/quickpay-ruby-client.gemspec +++ b/quickpay-ruby-client.gemspec @@ -19,14 +19,5 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - spec.add_development_dependency "bundler" - spec.add_development_dependency "minitest" - spec.add_development_dependency "pry" - spec.add_development_dependency "rake" - spec.add_development_dependency "rubocop" - spec.add_development_dependency "simplecov" - spec.add_development_dependency "simplecov-console" - spec.add_development_dependency "webmock" - spec.add_dependency "json", "~> 2", ">= 2.5" end From 3fce304a42c9dd0fd24179de244b04da2e9fd89d Mon Sep 17 00:00:00 2001 From: Felix Kleinschmidt Date: Mon, 3 Mar 2025 13:01:54 -0200 Subject: [PATCH 4/4] Remove test support for 2.6 --- .github/workflows/test_and_lint.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test_and_lint.yml b/.github/workflows/test_and_lint.yml index f800dd6..3622833 100644 --- a/.github/workflows/test_and_lint.yml +++ b/.github/workflows/test_and_lint.yml @@ -8,7 +8,6 @@ jobs: strategy: matrix: ruby: - - "2.6" - "2.7" - "3.0" - "3.1"