From dbbf99a7eb2bf8d0c4c2ddfa1d1562920ab632ed Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 16:59:37 +0100 Subject: [PATCH 1/9] refactor: remove custom Response object --- lib/scalingo/api/endpoint.rb | 7 - lib/scalingo/api/response.rb | 69 ------- spec/scalingo/api/endpoint_spec.rb | 23 --- spec/scalingo/api/response_spec.rb | 301 ----------------------------- 4 files changed, 400 deletions(-) delete mode 100644 lib/scalingo/api/response.rb delete mode 100644 spec/scalingo/api/response_spec.rb diff --git a/lib/scalingo/api/endpoint.rb b/lib/scalingo/api/endpoint.rb index 7f534e2..5698a23 100644 --- a/lib/scalingo/api/endpoint.rb +++ b/lib/scalingo/api/endpoint.rb @@ -1,5 +1,4 @@ require "forwardable" -require "scalingo/api/response" module Scalingo module API @@ -23,12 +22,6 @@ def inspect str << ">" str end - - private - - def unpack(*keys, &block) - Response.unpack(client, keys: keys, &block) - end end end end diff --git a/lib/scalingo/api/response.rb b/lib/scalingo/api/response.rb deleted file mode 100644 index f145663..0000000 --- a/lib/scalingo/api/response.rb +++ /dev/null @@ -1,69 +0,0 @@ -module Scalingo - module API - class Response - def self.unpack(client, key: nil, keys: nil) - response = yield - - body = response.body - has_hash_body = body.present? && body.respond_to?(:key) - - data = body - meta = nil - - if has_hash_body - keys = [key] if key.present? - - data = body.dig(*keys) if response.success? && keys.present? - - meta = body[:meta] - end - - new( - client: client, - status: response.status, - headers: response.headers, - data: data, - meta: meta, - full_body: body - ) - end - - attr_reader :client, :status, :headers, :data, :full_body, :meta - - def initialize(client:, status:, headers:, data:, full_body:, meta: nil) - @client = client - @status = status - @headers = headers - @data = data - @full_body = full_body - @meta = meta - end - - def inspect - %(<#{self.class}:0x#{object_id.to_s(16)} status:#{@status} data:#{@data} meta:#{@meta}>) - end - - def successful? - status >= 200 && status < 300 - end - - def paginated? - meta&.dig(:pagination).present? - end - - def operation - if operation? && client.respond_to?(:operations) - client.operations.get(operation_url) - end - end - - def operation_url - headers[:location] - end - - def operation? - operation_url.present? - end - end - end -end diff --git a/spec/scalingo/api/endpoint_spec.rb b/spec/scalingo/api/endpoint_spec.rb index 971d1b1..72fc111 100644 --- a/spec/scalingo/api/endpoint_spec.rb +++ b/spec/scalingo/api/endpoint_spec.rb @@ -19,27 +19,4 @@ expect(subject.connection).to eq :value end end - - describe "unpack" do - it "forwards unpack to Response without keys" do - mock = proc { 1 } - - expect(Scalingo::API::Response).to receive(:unpack).with(client, keys: [], &mock).and_return(:d).once - expect(subject.send(:unpack, &mock)).to eq :d - end - - it "forwards unpack to Response with a single key" do - mock = proc { 1 } - - expect(Scalingo::API::Response).to receive(:unpack).with(client, keys: [:a], &mock).and_return(:d).once - expect(subject.send(:unpack, :a, &mock)).to eq :d - end - - it "forwards unpack to Response with many keys" do - mock = proc { 1 } - - expect(Scalingo::API::Response).to receive(:unpack).with(client, keys: [:a, :b], &mock).and_return(:d).once - expect(subject.send(:unpack, :a, :b, &mock)).to eq :d - end - end end diff --git a/spec/scalingo/api/response_spec.rb b/spec/scalingo/api/response_spec.rb deleted file mode 100644 index 3a6318f..0000000 --- a/spec/scalingo/api/response_spec.rb +++ /dev/null @@ -1,301 +0,0 @@ -require "spec_helper" - -RSpec.describe Scalingo::API::Response do - let(:client) { regional } - let(:status) { 200 } - let(:headers) { {} } - let(:data) { "" } - let(:full_body) { "" } - let(:meta_object) { nil } - - subject { - described_class.new( - client: client, - status: status, - headers: headers, - data: data, - full_body: full_body, - meta: meta_object - ) - } - - describe "self.unpack" do - let(:body) { "" } - let(:success) { true } - - let(:response) { - OpenStruct.new( - body: body, - status: status, - headers: headers, - success?: success - ) - } - - it "passes the client supplied" do - object = described_class.unpack(:some_client) { response } - - expect(object.client).to eq :some_client - end - - it "passes the response status" do - object = described_class.unpack(:client) { response } - - expect(object.status).to eq status - end - - it "passes the response headers" do - object = described_class.unpack(:client) { response } - - expect(object.headers).to eq headers - end - - context "with an empty body" do - let(:body) { "" } - - it "without key" do - object = described_class.unpack(client) { response } - - expect(object.data).to eq "" - expect(object.full_body).to eq "" - expect(object.meta).to eq nil - end - - it "ignores key if supplied" do - object = described_class.unpack(client, key: :key) { response } - - expect(object.data).to eq "" - expect(object.full_body).to eq "" - expect(object.meta).to eq nil - end - end - - context "with a nil body" do - let(:body) { nil } - - it "without key" do - object = described_class.unpack(client) { response } - - expect(object.data).to eq nil - expect(object.full_body).to eq nil - expect(object.meta).to eq nil - end - - it "ignores key if supplied" do - object = described_class.unpack(client, key: :key) { response } - - expect(object.data).to eq nil - expect(object.full_body).to eq nil - expect(object.meta).to eq nil - end - end - - context "with a string body" do - let(:body) { "this is a string body, probably due to an error" } - - it "without key" do - object = described_class.unpack(client) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "ignores key if supplied" do - object = described_class.unpack(client, key: :key) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - end - - context "with an json (array) body" do - let(:body) { - [{key: :value}] - } - - it "without key" do - object = described_class.unpack(client) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "ignores key if supplied" do - object = described_class.unpack(client, key: :root) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - end - - context "with a json (hash) body" do - let(:body) { - {root: {key: :value}} - } - - it "without key" do - object = described_class.unpack(client) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "with valid key" do - object = described_class.unpack(client, key: :root) { response } - - expect(object.data).to eq({key: :value}) - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "with invalid key" do - object = described_class.unpack(client, key: :other) { response } - - expect(object.data).to eq nil - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "with valid keys" do - object = described_class.unpack(client, keys: [:root, :key]) { response } - - expect(object.data).to eq(:value) - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - it "with invalid keys" do - object = described_class.unpack(client, keys: [:root, :other]) { response } - - expect(object.data).to eq nil - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - - context "with meta" do - let(:body) { - {root: {key: :value}, meta: {meta1: :value}} - } - - it "extracts the meta object" do - object = described_class.unpack(client) { response } - - expect(object.meta).to eq({meta1: :value}) - end - end - end - - context "with an error response" do - let(:success) { false } - let(:body) { {root: {key: :value}} } - - it "does not dig in the response hash, even with a valid key" do - object = described_class.unpack(client, key: :root) { response } - - expect(object.data).to eq body - expect(object.full_body).to eq body - expect(object.meta).to eq nil - end - end - end - - describe "successful?" do - context "is true when 2XX" do - let(:status) { 200 } - it { expect(subject.successful?).to be true } - end - - context "is false when 3XX" do - let(:status) { 300 } - it { expect(subject.successful?).to be false } - end - - context "is false when 4XX" do - let(:status) { 400 } - it { expect(subject.successful?).to be false } - end - - context "is false when 5XX" do - let(:status) { 500 } - it { expect(subject.successful?).to be false } - end - end - - describe "paginated?" do - context "with pagination metadata" do - let(:meta_object) { - {pagination: {page: 1}} - } - - it { expect(subject.paginated?).to be true } - end - - context "without pagination metadata" do - let(:meta_object) { - {messages: []} - } - - it { expect(subject.paginated?).to be false } - end - end - - describe "operation" do - context "with an operation url" do - before do - load_meta!(api: :regional, folder: :operations) - register_stubs!("find-200", api: :regional, folder: :operations) - end - - let(:url) { - path = "/apps/#{meta[:app_id]}/operations/#{meta[:id]}" - File.join(Scalingo::ENDPOINTS[:regional], path) - } - - let(:headers) { - {location: url} - } - - it { expect(subject.operation?).to be true } - it { expect(subject.operation_url).to eq url } - - it "can request the operation" do - response = subject.operation - - expect(response).to be_successful - expect(response.data[:id]).to be_present - expect(response.data[:status]).to be_present - expect(response.data[:type]).to be_present - end - - it "delegates the operation to the given client" do - mock = double - - expect(subject.client).to receive(:operations).and_return(mock) - expect(mock).to receive(:get).with(url).and_return(:response) - - expect(subject.operation).to eq :response - end - - context "when the client doesn't know about operations" do - let(:client) { auth } - - it "fails silently" do - expect(subject.operation).to eq nil - end - end - end - - context "without an operation url" do - let(:meta_object) { {} } - - it { expect(subject.operation?).to be false } - it { expect(subject.operation_url).to eq nil } - it { expect(subject.operation).to eq nil } - end - end -end From 652855668e2318969c0e6312e770ecdff108b492 Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:02:02 +0100 Subject: [PATCH 2/9] refactor: enhance Faraday Response with helper methods --- lib/scalingo/api/client.rb | 2 ++ lib/scalingo/faraday/response.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 lib/scalingo/faraday/response.rb diff --git a/lib/scalingo/api/client.rb b/lib/scalingo/api/client.rb index 0d4d929..6b20d84 100644 --- a/lib/scalingo/api/client.rb +++ b/lib/scalingo/api/client.rb @@ -1,4 +1,6 @@ require "scalingo/token_holder" +require "scalingo/faraday/response" +require "active_support/core_ext/hash" module Scalingo module API diff --git a/lib/scalingo/faraday/response.rb b/lib/scalingo/faraday/response.rb new file mode 100644 index 0000000..a958e60 --- /dev/null +++ b/lib/scalingo/faraday/response.rb @@ -0,0 +1,29 @@ +require "faraday" +require "faraday/response" + +# Some additional methods for Faraday::Response in order to enhance expressiveness +module Faraday + class Response + # @deprecated Use {#success?} instead + def successful? + success? + end + + # @deprecated Use {#body} instead + def data + body + end + + def client_error? + RaiseError::ClientErrorStatuses.include?(status) + end + + def server_error? + RaiseError::ServerErrorStatuses.include?(status) + end + + def error? + !success? + end + end +end From d5db9ce875d802bd5e223e813f9860ecd11c65e7 Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:02:32 +0100 Subject: [PATCH 3/9] refactor: automatically extract meta and dig into root key --- lib/scalingo/api/client.rb | 4 +++ lib/scalingo/faraday/response.rb | 28 +++++++++++++++++++ lib/scalingo/faraday/unpack_middleware.rb | 33 +++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 lib/scalingo/faraday/unpack_middleware.rb diff --git a/lib/scalingo/api/client.rb b/lib/scalingo/api/client.rb index 6b20d84..a4ccb08 100644 --- a/lib/scalingo/api/client.rb +++ b/lib/scalingo/api/client.rb @@ -1,5 +1,6 @@ require "scalingo/token_holder" require "scalingo/faraday/response" +require "scalingo/faraday/unpack_middleware" require "active_support/core_ext/hash" module Scalingo @@ -86,6 +87,7 @@ def connection(fallback_to_guest: false) def unauthenticated_connection @unauthenticated_conn ||= Faraday.new(connection_options) { |conn| + conn.response :unpack conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json @@ -106,6 +108,7 @@ def authenticated_connection end @connection = Faraday.new(connection_options) { |conn| + conn.response :unpack conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json conn.request :authorization, "Bearer", -> { token_holder.token&.value } @@ -119,6 +122,7 @@ def database_connection(database_id) @database_connections ||= {} @database_connections[database_id] ||= Faraday.new(connection_options) { |conn| + conn.response :unpack conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json conn.request :authorization, "Bearer", -> { token_holder.database_tokens[database_id]&.value } diff --git a/lib/scalingo/faraday/response.rb b/lib/scalingo/faraday/response.rb index a958e60..d969f17 100644 --- a/lib/scalingo/faraday/response.rb +++ b/lib/scalingo/faraday/response.rb @@ -25,5 +25,33 @@ def server_error? def error? !success? end + + def meta + env[:response_meta] + end + + def meta? + meta.present? + end + + def pagination + return unless meta? + + meta[:pagination] + end + + def paginated? + pagination.present? + end + + def cursor_pagination + return unless meta? + + meta[:cursor_pagination] + end + + def cursor_paginated? + cursor_pagination.present? + end end end diff --git a/lib/scalingo/faraday/unpack_middleware.rb b/lib/scalingo/faraday/unpack_middleware.rb new file mode 100644 index 0000000..c7fb3fe --- /dev/null +++ b/lib/scalingo/faraday/unpack_middleware.rb @@ -0,0 +1,33 @@ +require "faraday" + +module Scalingo + class UnpackMiddleware < Faraday::Middleware + def on_complete(env) + # We only want to unpack the response for successful responses + return unless env.response.success? + + # Only hash-like objects are relevant to "unpack" + return unless env.body.is_a?(Hash) + + # Extract meta from response + if env.body[:meta] + env[:response_meta] = env.body.delete(:meta) + end + + # Extract cursor-based pagination + if env.body.key?(:next_cursor) + env[:response_meta] = { + cursor_pagination: { + next_cursor: env.body.delete(:next_cursor), + has_more: env.body.delete(:has_more) + }.compact + } + end + + # Dig the root key if it's the only remaining key in the body + env.body = env.body.values.first if env.body.size == 1 + end + end +end + +Faraday::Response.register_middleware(unpack: Scalingo::UnpackMiddleware) From 1cd8a92d4dab0255279ffca414f700190977b32d Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:43:06 +0100 Subject: [PATCH 4/9] refactor: remove unpack from API calls --- lib/scalingo/auth/keys.rb | 17 +++------- lib/scalingo/auth/scm_integrations.rb | 16 +++------ lib/scalingo/auth/tokens.rb | 20 +++--------- lib/scalingo/auth/two_factor_auth.rb | 16 +++------ lib/scalingo/auth/user.rb | 12 ++----- lib/scalingo/billing/profile.rb | 12 ++----- lib/scalingo/core_client.rb | 2 +- lib/scalingo/regional/addons.rb | 36 ++++++--------------- lib/scalingo/regional/apps.rb | 32 +++++------------- lib/scalingo/regional/autoscalers.rb | 20 +++--------- lib/scalingo/regional/collaborators.rb | 16 +++------ lib/scalingo/regional/containers.rb | 16 +++------ lib/scalingo/regional/deployments.rb | 12 ++----- lib/scalingo/regional/domains.rb | 20 +++--------- lib/scalingo/regional/environment.rb | 24 ++++---------- lib/scalingo/regional/events.rb | 16 +++------ lib/scalingo/regional/logs.rb | 8 ++--- lib/scalingo/regional/metrics.rb | 8 ++--- lib/scalingo/regional/notifiers.rb | 28 ++++------------ lib/scalingo/regional/operations.rb | 4 +-- lib/scalingo/regional/scm_repo_links.rb | 32 +++++------------- lib/scalingo/regional_database/backups.rb | 12 ++----- lib/scalingo/regional_database/databases.rb | 8 ++--- 23 files changed, 98 insertions(+), 289 deletions(-) diff --git a/lib/scalingo/auth/keys.rb b/lib/scalingo/auth/keys.rb index ef917c4..64f6449 100644 --- a/lib/scalingo/auth/keys.rb +++ b/lib/scalingo/auth/keys.rb @@ -5,52 +5,45 @@ class Auth::Keys < API::Endpoint def all(headers = nil, &block) data = nil - response = connection.get( + connection.get( "keys", data, headers, &block ) - - unpack(:keys) { response } end def show(id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "keys/#{id}", data, headers, &block ) - - unpack(:key) { response } end def create(payload, headers = nil, &block) data = {key: payload} - response = connection.post( + connection.post( "keys", data, headers, &block ) - - unpack(:key) { response } end def destroy(id, headers = nil, &block) data = nil - response = connection.delete( + + connection.delete( "keys/#{id}", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/auth/scm_integrations.rb b/lib/scalingo/auth/scm_integrations.rb index 1755bc6..224aa85 100644 --- a/lib/scalingo/auth/scm_integrations.rb +++ b/lib/scalingo/auth/scm_integrations.rb @@ -5,53 +5,45 @@ class Auth::ScmIntegrations < API::Endpoint def all(headers = nil, &block) data = nil - response = connection.get( + connection.get( "scm_integrations", data, headers, &block ) - - unpack(:scm_integrations) { response } end def show(id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "scm_integrations/#{id}", data, headers, &block ) - - unpack(:scm_integration) { response } end def create(payload, headers = nil, &block) data = {scm_integration: payload} - response = connection.post( + connection.post( "scm_integrations", data, headers, &block ) - - unpack(:scm_integration) { response } end def destroy(id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "scm_integrations/#{id}", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/auth/tokens.rb b/lib/scalingo/auth/tokens.rb index 6d1167f..f3cc5a0 100644 --- a/lib/scalingo/auth/tokens.rb +++ b/lib/scalingo/auth/tokens.rb @@ -13,66 +13,56 @@ def exchange(token, headers = nil, &block) request_headers.update(headers) if headers - response = client.unauthenticated_connection.post( + client.unauthenticated_connection.post( "tokens/exchange", data, request_headers, &block ) - - unpack { response } end def all(headers = nil, &block) data = nil - response = connection.get( + connection.get( "tokens", data, headers, &block ) - - unpack(:tokens) { response } end def create(payload, headers = nil, &block) data = {token: payload} - response = connection.post( + connection.post( "tokens", data, headers, &block ) - - unpack(:token) { response } end def renew(id, headers = nil, &block) data = nil - response = connection.patch( + connection.patch( "tokens/#{id}/renew", data, headers, &block ) - - unpack(:token) { response } end def destroy(id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "tokens/#{id}", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/auth/two_factor_auth.rb b/lib/scalingo/auth/two_factor_auth.rb index f244e65..ae5e7b3 100644 --- a/lib/scalingo/auth/two_factor_auth.rb +++ b/lib/scalingo/auth/two_factor_auth.rb @@ -9,53 +9,45 @@ class Auth::TwoFactorAuth < API::Endpoint def status(headers = nil, &block) data = nil - response = connection.get( + connection.get( "client/tfa", data, headers, &block ) - - unpack(:tfa) { response } end def initiate(provider = DEFAULT_PROVIDER, headers = nil, &block) data = {tfa: {provider: provider}} - response = connection.post( + connection.post( "client/tfa", data, headers, &block ) - - unpack(:tfa) { response } end def validate(attempt, headers = nil, &block) data = {tfa: {attempt: attempt}} - response = connection.post( + connection.post( "client/tfa/validate", data, headers, &block ) - - unpack(:tfa) { response } end def disable(headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "client/tfa", data, headers, &block ) - - unpack(:tfa) { response } end end end diff --git a/lib/scalingo/auth/user.rb b/lib/scalingo/auth/user.rb index 0498002..7557788 100644 --- a/lib/scalingo/auth/user.rb +++ b/lib/scalingo/auth/user.rb @@ -5,40 +5,34 @@ class Auth::User < API::Endpoint def self(headers = nil, &block) data = nil - response = connection.get( + connection.get( "users/self", data, headers, &block ) - - unpack(:user) { response } end def update(payload, headers = nil, &block) data = {user: payload} - response = connection.put( + connection.put( "users/account", data, headers, &block ) - - unpack(:user) { response } end def stop_free_trial(headers = nil, &block) data = nil - response = connection.post( + connection.post( "users/stop_free_trial", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/billing/profile.rb b/lib/scalingo/billing/profile.rb index d7f1a6a..362f3ba 100644 --- a/lib/scalingo/billing/profile.rb +++ b/lib/scalingo/billing/profile.rb @@ -5,40 +5,34 @@ class Billing::Profile < API::Endpoint def show(headers = nil, &block) data = nil - response = connection.get( + connection.get( "profile", data, headers, &block ) - - unpack(:profile) { response } end def create(payload = {}, headers = nil, &block) data = {profile: payload} - response = connection.post( + connection.post( "profiles", data, headers, &block ) - - unpack(:profile) { response } end def update(id, payload = {}, headers = nil, &block) data = {profile: payload} - response = connection.put( + connection.put( "profiles/#{id}", data, headers, &block ) - - unpack(:profile) { response } end alias_method :self, :show diff --git a/lib/scalingo/core_client.rb b/lib/scalingo/core_client.rb index 21c655c..d4c1c76 100644 --- a/lib/scalingo/core_client.rb +++ b/lib/scalingo/core_client.rb @@ -64,7 +64,7 @@ def authenticate_with(access_token: nil, bearer_token: nil, expires_at: nil) if response.successful? self.token = BearerToken.new( - response.data[:token], + response.data, expires_at: expiration, raise_on_expired: config.raise_on_expired_token ) diff --git a/lib/scalingo/regional/addons.rb b/lib/scalingo/regional/addons.rb index ac9967d..aceef10 100644 --- a/lib/scalingo/regional/addons.rb +++ b/lib/scalingo/regional/addons.rb @@ -5,79 +5,67 @@ class Regional::Addons < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/addons", data, headers, &block ) - - unpack(:addons) { response } end def find(app_id, addon_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/addons/#{addon_id}", data, headers, &block ) - - unpack(:addon) { response } end def provision(app_id, payload = {}, headers = nil, &block) data = {addon: payload} - response = connection.post( + connection.post( "apps/#{app_id}/addons", data, headers, &block ) - - unpack(:addon) { response } end def update(app_id, addon_id, payload = {}, headers = nil, &block) data = {addon: payload} - response = connection.patch( + connection.patch( "apps/#{app_id}/addons/#{addon_id}", data, headers, &block ) - - unpack(:addon) { response } end def destroy(app_id, addon_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/addons/#{addon_id}", data, headers, &block ) - - unpack { response } end def sso(app_id, addon_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/addons/#{addon_id}/sso", data, headers, &block ) - - unpack(:addon) { response } end def authenticate!(app_id, addon_id, headers = nil, &block) @@ -98,40 +86,34 @@ def authenticate!(app_id, addon_id, headers = nil, &block) def token(app_id, addon_id, headers = nil, &block) data = nil - response = connection.post( + connection.post( "apps/#{app_id}/addons/#{addon_id}/token", data, headers, &block ) - - unpack(:addon) { response } end def categories(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "addon_categories", data, headers, &block ) - - unpack(:addon_categories) { response } end def providers(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "addon_providers", data, headers, &block ) - - unpack(:addon_providers) { response } end end end diff --git a/lib/scalingo/regional/apps.rb b/lib/scalingo/regional/apps.rb index 5f615eb..bae29bd 100644 --- a/lib/scalingo/regional/apps.rb +++ b/lib/scalingo/regional/apps.rb @@ -5,27 +5,23 @@ class Regional::Apps < API::Endpoint def all(headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps", data, headers, &block ) - - unpack(:apps) { response } end def find(id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{id}", data, headers, &block ) - - unpack(:app) { response } end def create(payload = {}, headers = nil, &block) @@ -37,73 +33,61 @@ def create(payload = {}, headers = nil, &block) request_headers["X-Dry-Run"] = "true" if dry_run request_headers.update(headers) if headers - response = connection.post( + connection.post( "apps", data, request_headers, &block ) - - unpack(:app) { response } end def update(id, payload = {}, headers = nil, &block) data = {app: payload} - response = connection.patch( + connection.patch( "apps/#{id}", data, headers, &block ) - - unpack(:app) { response } end def logs_url(id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{id}/logs", data, headers, &block ) - - unpack(:logs_url) { response } end def destroy(id, payload = {}, headers = nil, &block) - response = connection.delete( + connection.delete( "apps/#{id}", payload, headers, &block ) - - unpack { response } end def rename(id, payload = {}, headers = nil, &block) - response = connection.post( + connection.post( "apps/#{id}/rename", payload, headers, &block ) - - unpack(:app) { response } end def transfer(id, payload = {}, headers = nil, &block) - response = connection.patch( + connection.patch( "apps/#{id}", payload, headers, &block ) - - unpack(:app) { response } end end end diff --git a/lib/scalingo/regional/autoscalers.rb b/lib/scalingo/regional/autoscalers.rb index 2270430..046c338 100644 --- a/lib/scalingo/regional/autoscalers.rb +++ b/lib/scalingo/regional/autoscalers.rb @@ -5,66 +5,56 @@ class Regional::Autoscalers < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/autoscalers", data, headers, &block ) - - unpack(:autoscalers) { response } end def find(app_id, autoscaler_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/autoscalers/#{autoscaler_id}", data, headers, &block ) - - unpack(:autoscaler) { response } end def create(app_id, payload = {}, headers = nil, &block) data = {autoscaler: payload} - response = connection.post( + connection.post( "apps/#{app_id}/autoscalers", data, headers, &block ) - - unpack(:autoscaler) { response } end def update(app_id, autoscaler_id, payload = {}, headers = nil, &block) data = {autoscaler: payload} - response = connection.patch( + connection.patch( "apps/#{app_id}/autoscalers/#{autoscaler_id}", data, headers, &block ) - - unpack(:autoscaler) { response } end def destroy(app_id, autoscaler_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/autoscalers/#{autoscaler_id}", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/regional/collaborators.rb b/lib/scalingo/regional/collaborators.rb index dc901b4..92307f0 100644 --- a/lib/scalingo/regional/collaborators.rb +++ b/lib/scalingo/regional/collaborators.rb @@ -5,53 +5,45 @@ class Regional::Collaborators < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/collaborators", data, headers, &block ) - - unpack(:collaborators) { response } end def destroy(app_id, collaborator_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/collaborators/#{collaborator_id}", data, headers, &block ) - - unpack { response } end def invite(app_id, payload = {}, headers = nil, &block) data = {collaborator: payload} - response = connection.post( + connection.post( "apps/#{app_id}/collaborators", data, headers, &block ) - - unpack(:collaborator) { response } end def accept(token, headers = nil, &block) data = {token: token} - response = connection.get( + connection.get( "apps/collaboration", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/regional/containers.rb b/lib/scalingo/regional/containers.rb index 446a177..f1e1954 100644 --- a/lib/scalingo/regional/containers.rb +++ b/lib/scalingo/regional/containers.rb @@ -5,53 +5,45 @@ class Regional::Containers < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/containers", data, headers, &block ) - - unpack(:containers) { response } end def scale(app_id, formation, headers = nil, &block) data = {containers: formation} - response = connection.post( + connection.post( "apps/#{app_id}/scale", data, headers, &block ) - - unpack(:containers) { response } end def restart(app_id, scope = [], headers = nil, &block) data = {scope: scope} - response = connection.post( + connection.post( "apps/#{app_id}/restart", data, headers, &block ) - - unpack { response } end def sizes(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "features/container_sizes", data, headers, &block ) - - unpack(:container_sizes) { response } end end end diff --git a/lib/scalingo/regional/deployments.rb b/lib/scalingo/regional/deployments.rb index 407a068..6605585 100644 --- a/lib/scalingo/regional/deployments.rb +++ b/lib/scalingo/regional/deployments.rb @@ -5,40 +5,34 @@ class Regional::Deployments < API::Endpoint def for(app_id, payload = {}, headers = nil, &block) data = payload.compact - response = connection.get( + connection.get( "apps/#{app_id}/deployments", data, headers, &block ) - - unpack(:deployments) { response } end def find(app_id, deployment_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/deployments/#{deployment_id}", data, headers, &block ) - - unpack(:deployment) { response } end def logs(app_id, deployment_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/deployments/#{deployment_id}/output", data, headers, &block ) - - unpack(:deployment) { response } end end end diff --git a/lib/scalingo/regional/domains.rb b/lib/scalingo/regional/domains.rb index 64f84ca..c97d660 100644 --- a/lib/scalingo/regional/domains.rb +++ b/lib/scalingo/regional/domains.rb @@ -5,66 +5,56 @@ class Regional::Domains < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/domains", data, headers, &block ) - - unpack(:domains) { response } end def find(app_id, domain_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/domains/#{domain_id}", data, headers, &block ) - - unpack(:domain) { response } end def create(app_id, payload = {}, headers = nil, &block) data = {domain: payload} - response = connection.post( + connection.post( "apps/#{app_id}/domains", data, headers, &block ) - - unpack(:domain) { response } end def update(app_id, domain_id, payload = {}, headers = nil, &block) data = {domain: payload} - response = connection.patch( + connection.patch( "apps/#{app_id}/domains/#{domain_id}", data, headers, &block ) - - unpack(:domain) { response } end def destroy(app_id, domain_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/domains/#{domain_id}", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/regional/environment.rb b/lib/scalingo/regional/environment.rb index 20ac83c..d9b0774 100644 --- a/lib/scalingo/regional/environment.rb +++ b/lib/scalingo/regional/environment.rb @@ -5,79 +5,67 @@ class Regional::Environment < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/variables", data, headers, &block ) - - unpack(:variables) { response } end def create(app_id, payload = {}, headers = nil, &block) data = {variable: payload} - response = connection.post( + connection.post( "apps/#{app_id}/variables", data, headers, &block ) - - unpack(:variable) { response } end def update(app_id, variable_id, value, headers = nil, &block) data = {variable: {value: value}} - response = connection.patch( + connection.patch( "apps/#{app_id}/variables/#{variable_id}", data, headers, &block ) - - unpack(:variable) { response } end def destroy(app_id, variable_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/variables/#{variable_id}", data, headers, &block ) - - unpack { response } end def bulk_update(app_id, variables, headers = nil, &block) data = {variables: variables} - response = connection.put( + connection.put( "apps/#{app_id}/variables", data, headers, &block ) - - unpack(:variables) { response } end def bulk_destroy(app_id, variable_ids, headers = nil, &block) data = {variable_ids: variable_ids} - response = connection.delete( + connection.delete( "apps/#{app_id}/variables", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/regional/events.rb b/lib/scalingo/regional/events.rb index d4d1684..582879c 100644 --- a/lib/scalingo/regional/events.rb +++ b/lib/scalingo/regional/events.rb @@ -3,53 +3,45 @@ class Regional::Events < API::Endpoint def all(payload = {}, headers = nil, &block) data = payload.compact - response = connection.get( + connection.get( "events", data, headers, &block ) - - unpack(:events) { response } end def for(app_id, payload = {}, headers = nil, &block) data = payload.compact - response = connection.get( + connection.get( "apps/#{app_id}/events", data, headers, &block ) - - unpack(:events) { response } end def types(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "event_types", data, headers, &block ) - - unpack(:event_types) { response } end def categories(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "event_categories", data, headers, &block ) - - unpack(:event_categories) { response } end end end diff --git a/lib/scalingo/regional/logs.rb b/lib/scalingo/regional/logs.rb index 9550a25..f86ffe1 100644 --- a/lib/scalingo/regional/logs.rb +++ b/lib/scalingo/regional/logs.rb @@ -3,27 +3,23 @@ class Regional::Logs < API::Endpoint def get(url, payload = {}, headers = nil, &block) data = payload.compact - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( url, data, headers, &block ) - - unpack { response } end def archives(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/logs_archives", data, headers, &block ) - - unpack(:archives) { response } end ## Helper method to avoid having to manually chain two operations diff --git a/lib/scalingo/regional/metrics.rb b/lib/scalingo/regional/metrics.rb index e735530..3b2169a 100644 --- a/lib/scalingo/regional/metrics.rb +++ b/lib/scalingo/regional/metrics.rb @@ -16,27 +16,23 @@ def for(app_id, payload = {}, headers = nil, &block) url = "#{url}/#{payload[:container_index]}" if payload[:container_index] end - response = connection.get( + connection.get( url, data, headers, &block ) - - unpack { response } end def types(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "features/metrics", data, headers, &block ) - - unpack(:metrics) { response } end end end diff --git a/lib/scalingo/regional/notifiers.rb b/lib/scalingo/regional/notifiers.rb index 336097f..0dc8ffa 100644 --- a/lib/scalingo/regional/notifiers.rb +++ b/lib/scalingo/regional/notifiers.rb @@ -5,92 +5,78 @@ class Regional::Notifiers < API::Endpoint def for(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/notifiers", data, headers, &block ) - - unpack(:notifiers) { response } end def find(app_id, notifier_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/notifiers/#{notifier_id}", data, headers, &block ) - - unpack(:notifier) { response } end def create(app_id, payload = {}, headers = nil, &block) data = {notifier: payload} - response = connection.post( + connection.post( "apps/#{app_id}/notifiers", data, headers, &block ) - - unpack(:notifier) { response } end def update(app_id, notifier_id, payload = {}, headers = nil, &block) data = {notifier: payload} - response = connection.patch( + connection.patch( "apps/#{app_id}/notifiers/#{notifier_id}", data, headers, &block ) - - unpack(:notifier) { response } end def destroy(app_id, notifier_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/notifiers/#{notifier_id}", data, headers, &block ) - - unpack { response } end def test(app_id, notifier_id, headers = nil, &block) data = nil - response = connection.post( + connection.post( "apps/#{app_id}/notifiers/#{notifier_id}/test", data, headers, &block ) - - unpack { response } end def platforms(headers = nil, &block) data = nil - response = connection(fallback_to_guest: true).get( + connection(fallback_to_guest: true).get( "notification_platforms", data, headers, &block ) - - unpack(:notification_platforms) { response } end end end diff --git a/lib/scalingo/regional/operations.rb b/lib/scalingo/regional/operations.rb index 8c2310d..15ef6c1 100644 --- a/lib/scalingo/regional/operations.rb +++ b/lib/scalingo/regional/operations.rb @@ -13,14 +13,12 @@ def find(app_id, operation_id, headers = nil, &block) def get(url, headers = nil, &block) data = nil - response = connection.get( + connection.get( url, data, headers, &block ) - - unpack(:operation) { response } end end end diff --git a/lib/scalingo/regional/scm_repo_links.rb b/lib/scalingo/regional/scm_repo_links.rb index 9b904c6..deb9443 100644 --- a/lib/scalingo/regional/scm_repo_links.rb +++ b/lib/scalingo/regional/scm_repo_links.rb @@ -5,105 +5,89 @@ class Regional::ScmRepoLinks < API::Endpoint def show(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/scm_repo_link", data, headers, &block ) - - unpack(:scm_repo_link) { response } end def create(app_id, payload = {}, headers = nil, &block) data = {scm_repo_link: payload} - response = connection.post( + connection.post( "apps/#{app_id}/scm_repo_link", data, headers, &block ) - - unpack(:scm_repo_link) { response } end def update(app_id, payload = {}, headers = nil, &block) data = {scm_repo_link: payload} - response = connection.patch( + connection.patch( "apps/#{app_id}/scm_repo_link", data, headers, &block ) - - unpack(:scm_repo_link) { response } end def destroy(app_id, headers = nil, &block) data = nil - response = connection.delete( + connection.delete( "apps/#{app_id}/scm_repo_link", data, headers, &block ) - - unpack(:scm_repo_link) { response } end def deploy(app_id, branch, headers = nil, &block) data = {branch: branch} - response = connection.post( + connection.post( "apps/#{app_id}/scm_repo_link/manual_deploy", data, headers, &block ) - - unpack(:deployment) { response } end def review_app(review_app, pull_request_id, headers = nil, &block) data = {pull_request_id: pull_request_id} - response = connection.post( + connection.post( "apps/#{app_id}/scm_repo_link/manual_review_app", data, headers, &block ) - - unpack(:review_app) { response } end def branches(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/scm_repo_link/branches", data, headers, &block ) - - unpack(:branches) { response } end def pulls(app_id, headers = nil, &block) data = nil - response = connection.get( + connection.get( "apps/#{app_id}/scm_repo_link/pulls", data, headers, &block ) - - unpack(:pulls) { response } end end end diff --git a/lib/scalingo/regional_database/backups.rb b/lib/scalingo/regional_database/backups.rb index 0b835b0..4ccafc1 100644 --- a/lib/scalingo/regional_database/backups.rb +++ b/lib/scalingo/regional_database/backups.rb @@ -5,40 +5,34 @@ class RegionalDatabase::Backups < API::Endpoint def create(addon_id, headers = nil, &block) data = nil - response = database_connection(addon_id).post( + database_connection(addon_id).post( "databases/#{addon_id}/backups", data, headers, &block ) - - unpack(:database_backup) { response } end def for(addon_id, headers = nil, &block) data = nil - response = database_connection(addon_id).get( + database_connection(addon_id).get( "databases/#{addon_id}/backups", data, headers, &block ) - - unpack(:database_backups) { response } end def archive(addon_id, backup_id, headers = nil, &block) data = nil - response = database_connection(addon_id).get( + database_connection(addon_id).get( "databases/#{addon_id}/backups/#{backup_id}/archive", data, headers, &block ) - - unpack { response } end end end diff --git a/lib/scalingo/regional_database/databases.rb b/lib/scalingo/regional_database/databases.rb index fb56f87..f2d0ddb 100644 --- a/lib/scalingo/regional_database/databases.rb +++ b/lib/scalingo/regional_database/databases.rb @@ -5,27 +5,23 @@ class RegionalDatabase::Databases < API::Endpoint def find(id, headers = nil, &block) data = nil - response = database_connection(id).get( + database_connection(id).get( "databases/#{id}", data, headers, &block ) - - unpack(:database) { response } end def upgrade(id, headers = nil, &block) data = nil - response = database_connection(id).post( + database_connection(id).post( "databases/#{id}/upgrade", data, headers, &block ) - - unpack(:database) { response } end end end From 38a9e5d9c51377a975a63313aab0bbcb64c7a5ae Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:43:43 +0100 Subject: [PATCH 5/9] refactor: update specs to reflect the changes in internal API --- spec/scalingo/auth/tokens_spec.rb | 4 +-- spec/scalingo/client_spec.rb | 2 +- spec/scalingo/regional/addons_spec.rb | 2 ++ spec/scalingo/regional/apps_spec.rb | 1 + spec/scalingo/regional/collaborators_spec.rb | 1 - spec/scalingo/regional/logs_spec.rb | 2 +- spec/support/shared.rb | 26 ++++++++++++-------- 7 files changed, 23 insertions(+), 15 deletions(-) diff --git a/spec/scalingo/auth/tokens_spec.rb b/spec/scalingo/auth/tokens_spec.rb index fa64a88..de798b3 100644 --- a/spec/scalingo/auth/tokens_spec.rb +++ b/spec/scalingo/auth/tokens_spec.rb @@ -9,8 +9,8 @@ let(:stub_pattern) { "exchange-200" } it "is successful" do - expect(response).to be_successful - expect(response.data[:token]).to be_present + expect(response).to be_success + expect(response.body).to be_present end end diff --git a/spec/scalingo/client_spec.rb b/spec/scalingo/client_spec.rb index 7ae6d31..0f44f6f 100644 --- a/spec/scalingo/client_spec.rb +++ b/spec/scalingo/client_spec.rb @@ -52,7 +52,7 @@ it "is successful with valid token" do fake_response = OpenStruct.new( successful?: true, - data: {token: "response token"} + data: "response token" ) expect(subject.auth.tokens).to receive(:exchange).and_return(fake_response) diff --git a/spec/scalingo/regional/addons_spec.rb b/spec/scalingo/regional/addons_spec.rb index 0491c9c..ac6ed2f 100644 --- a/spec/scalingo/regional/addons_spec.rb +++ b/spec/scalingo/regional/addons_spec.rb @@ -45,6 +45,7 @@ context "success" do let(:arguments) { [meta[:app_id], meta[:provision][:valid]] } let(:stub_pattern) { "provision-201" } + let(:expected_keys) { %i[addon message] } it_behaves_like "a singular object response", 201 end @@ -139,6 +140,7 @@ context "success" do let(:arguments) { [meta[:app_id], meta[:id], meta[:update][:valid]] } let(:stub_pattern) { "update-200" } + let(:expected_keys) { %i[addon message] } it_behaves_like "a singular object response" end diff --git a/spec/scalingo/regional/apps_spec.rb b/spec/scalingo/regional/apps_spec.rb index 316acbf..5c6bb17 100644 --- a/spec/scalingo/regional/apps_spec.rb +++ b/spec/scalingo/regional/apps_spec.rb @@ -61,6 +61,7 @@ context "success" do let(:arguments) { meta[:id] } let(:stub_pattern) { "logs_url" } + let(:expected_keys) { %i[app logs_url] } it_behaves_like "a singular object response" end diff --git a/spec/scalingo/regional/collaborators_spec.rb b/spec/scalingo/regional/collaborators_spec.rb index b304476..e2285d3 100644 --- a/spec/scalingo/regional/collaborators_spec.rb +++ b/spec/scalingo/regional/collaborators_spec.rb @@ -31,7 +31,6 @@ context "success" do let(:arguments) { meta[:accept][:valid] } let(:stub_pattern) { "accept-200" } - let(:expected_keys) { %i[app] } it_behaves_like "a singular object response" end diff --git a/spec/scalingo/regional/logs_spec.rb b/spec/scalingo/regional/logs_spec.rb index 99b5f2f..51ad0ef 100644 --- a/spec/scalingo/regional/logs_spec.rb +++ b/spec/scalingo/regional/logs_spec.rb @@ -33,7 +33,7 @@ let(:stub_pattern) { "archives-200" } it_behaves_like "a collection response" - it_behaves_like "a non-paginated collection" + it_behaves_like "a cursor paginated collection" end end end diff --git a/spec/support/shared.rb b/spec/support/shared.rb index 1c4428a..771fc70 100644 --- a/spec/support/shared.rb +++ b/spec/support/shared.rb @@ -32,30 +32,30 @@ end end -RSpec.shared_examples "a not found response" do - it "cannot be found" do - expect(response).not_to be_successful - expect(response.status).to eq 404 - end -end - RSpec.shared_examples "a client error" do it "is a generic client error" do - expect(response).not_to be_successful + expect(response).to be_client_error expect(response.status).to eq 400 end end RSpec.shared_examples "a server error" do it "is a generic server error" do - expect(response).not_to be_successful + expect(response).to be_server_error expect(response.status).to eq 500 end end +RSpec.shared_examples "a not found response" do + it "cannot be found" do + expect(response).to be_client_error + expect(response.status).to eq 404 + end +end + RSpec.shared_examples "an unprocessable request" do it "cannot be found" do - expect(response).not_to be_successful + expect(response).to be_client_error expect(response.status).to eq 422 end end @@ -119,6 +119,12 @@ end end +RSpec.shared_examples "a cursor paginated collection" do |code = 200| + it "is cursor paginated" do + expect(response).to be_cursor_paginated + end +end + RSpec.shared_examples "a non-paginated collection" do |code = 200| it "is not paginated" do expect(response).not_to be_paginated From 82c8d3683dd2101de57b0a962ca6862b5773e7bc Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:47:13 +0100 Subject: [PATCH 6/9] refactor: remove compat methods --- lib/scalingo/core_client.rb | 6 +++--- lib/scalingo/faraday/response.rb | 10 ---------- lib/scalingo/regional/addons.rb | 2 +- lib/scalingo/regional/logs.rb | 4 ++-- spec/scalingo/client_spec.rb | 6 +++--- spec/support/shared.rb | 16 ++++++++-------- 6 files changed, 17 insertions(+), 27 deletions(-) diff --git a/lib/scalingo/core_client.rb b/lib/scalingo/core_client.rb index d4c1c76..1a38eb4 100644 --- a/lib/scalingo/core_client.rb +++ b/lib/scalingo/core_client.rb @@ -62,15 +62,15 @@ def authenticate_with(access_token: nil, bearer_token: nil, expires_at: nil) expiration = Time.now + config.exchanged_token_validity response = auth.tokens.exchange(access_token) - if response.successful? + if response.success? self.token = BearerToken.new( - response.data, + response.body, expires_at: expiration, raise_on_expired: config.raise_on_expired_token ) end - return response.successful? + return response.success? end if bearer_token diff --git a/lib/scalingo/faraday/response.rb b/lib/scalingo/faraday/response.rb index d969f17..e7c8e7f 100644 --- a/lib/scalingo/faraday/response.rb +++ b/lib/scalingo/faraday/response.rb @@ -4,16 +4,6 @@ # Some additional methods for Faraday::Response in order to enhance expressiveness module Faraday class Response - # @deprecated Use {#success?} instead - def successful? - success? - end - - # @deprecated Use {#body} instead - def data - body - end - def client_error? RaiseError::ClientErrorStatuses.include?(status) end diff --git a/lib/scalingo/regional/addons.rb b/lib/scalingo/regional/addons.rb index aceef10..dbf1165 100644 --- a/lib/scalingo/regional/addons.rb +++ b/lib/scalingo/regional/addons.rb @@ -72,7 +72,7 @@ def authenticate!(app_id, addon_id, headers = nil, &block) response = token(app_id, addon_id, headers, &block) return response unless response.status == 200 - token = response.data[:token] + token = response.body[:token] client.token_holder.authenticate_database_with_bearer_token( addon_id, token, diff --git a/lib/scalingo/regional/logs.rb b/lib/scalingo/regional/logs.rb index f86ffe1..a7ad752 100644 --- a/lib/scalingo/regional/logs.rb +++ b/lib/scalingo/regional/logs.rb @@ -26,9 +26,9 @@ def archives(app_id, headers = nil, &block) def for(app_id, payload = {}, headers = nil, &block) logs_response = scalingo.apps.logs_url(app_id) - return logs_response unless logs_response.successful? + return logs_response unless logs_response.success? - get(logs_response.data, payload, headers, &block) + get(logs_response.body, payload, headers, &block) end end end diff --git a/spec/scalingo/client_spec.rb b/spec/scalingo/client_spec.rb index 0f44f6f..d7c5ed2 100644 --- a/spec/scalingo/client_spec.rb +++ b/spec/scalingo/client_spec.rb @@ -51,8 +51,8 @@ context "with access token" do it "is successful with valid token" do fake_response = OpenStruct.new( - successful?: true, - data: "response token" + success?: true, + body: "response token" ) expect(subject.auth.tokens).to receive(:exchange).and_return(fake_response) @@ -64,7 +64,7 @@ it "fails with invalid token" do fake_response = OpenStruct.new( - successful?: false + success?: false ) expect(subject.auth.tokens).to receive(:exchange).and_return(fake_response) diff --git a/spec/support/shared.rb b/spec/support/shared.rb index 771fc70..b652db7 100644 --- a/spec/support/shared.rb +++ b/spec/support/shared.rb @@ -3,7 +3,7 @@ let(:custom_headers) { {"X-Custom-Header" => "custom"} } it "is successful" do - expect(response).to be_successful + expect(response).to be_success expect(response.status).to eq code end @@ -67,11 +67,11 @@ let(:expected_keys) { %i[id] } unless method_defined?(:expected_keys) it "is an object of the expected type (and if applicable, the expected keys)" do - expect(response.data).to be_a_kind_of(expected_type) + expect(response.body).to be_a_kind_of(expected_type) - if response.data.respond_to?(:key?) + if response.body.respond_to?(:key?) expected_keys.each do |key| - expect(response.data.key?(key)).to be true + expect(response.body.key?(key)).to be true end end end @@ -81,7 +81,7 @@ it_behaves_like "a successful response", code it "is empty" do - expect(response.data).to eq("") + expect(response.body).to eq("") end end @@ -93,15 +93,15 @@ let(:expected_keys) { %i[id] } unless method_defined?(:expected_keys) it "is an array" do - expect(response.data).to be_a_kind_of(Array) + expect(response.body).to be_a_kind_of(Array) end it "contains the number of expected elements" do - expect(response.data.size).to eq(expected_count) + expect(response.body.size).to eq(expected_count) end it "items are of the expected type (and if applicable, the expected keys)" do - response.data.each do |item| + response.body.each do |item| expect(item).to be_a_kind_of(expected_type) if item.respond_to?(:key?) From 6f3bbc781af23ae109e54544823ff863cefbf986 Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 17:59:20 +0100 Subject: [PATCH 7/9] feat: console with client --- bin/console | 12 +++++++++++- scalingo.gemspec | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/bin/console b/bin/console index 5613a7a..a924bda 100755 --- a/bin/console +++ b/bin/console @@ -3,10 +3,20 @@ require "bundler/setup" require "scalingo" +begin + require "dotenv" + Dotenv.load(".env.local") +rescue LoadError + puts("dotenv not available - no .env.local loading") +end + # You can add fixtures and/or initialization code here to make experimenting # with your gem easier. You can also use a different console, if you like. # (If you use this, don't forget to add pry to your Gemfile!) require "pry" -Pry.start +client = Scalingo::Client.new +client.authenticate_with(access_token: ENV["SCALINGO_API_TOKEN"]) if ENV["SCALINGO_API_TOKEN"].present? + +Pry.start(binding, quiet: true) diff --git a/scalingo.gemspec b/scalingo.gemspec index e2fbfbd..c79c88c 100644 --- a/scalingo.gemspec +++ b/scalingo.gemspec @@ -37,6 +37,7 @@ Gem::Specification.new do |s| s.add_dependency "multi_json", ">= 1.0.3", "~> 1.0" s.add_development_dependency "bundler", "~> 2.0" + s.add_development_dependency "dotenv" s.add_development_dependency "rake", "~> 13.0" s.add_development_dependency "rspec", "~> 3.0" s.add_development_dependency "standard", "~> 1.32" From ce094af242775cf9caae25b13ee50cc67d3fd43d Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Sun, 11 Feb 2024 23:36:08 +0100 Subject: [PATCH 8/9] refactor(farady): split up middlewares --- lib/scalingo/api/client.rb | 12 ++- .../{unpack_middleware.rb => extract_meta.rb} | 12 +-- lib/scalingo/faraday/extract_root_value.rb | 18 ++++ lib/scalingo/faraday/response.rb | 3 +- spec/scalingo/faraday/extract_meta_spec.rb | 101 ++++++++++++++++++ .../faraday/extract_root_value_spec.rb | 48 +++++++++ 6 files changed, 180 insertions(+), 14 deletions(-) rename lib/scalingo/faraday/{unpack_middleware.rb => extract_meta.rb} (55%) create mode 100644 lib/scalingo/faraday/extract_root_value.rb create mode 100644 spec/scalingo/faraday/extract_meta_spec.rb create mode 100644 spec/scalingo/faraday/extract_root_value_spec.rb diff --git a/lib/scalingo/api/client.rb b/lib/scalingo/api/client.rb index a4ccb08..a7ba304 100644 --- a/lib/scalingo/api/client.rb +++ b/lib/scalingo/api/client.rb @@ -1,6 +1,7 @@ require "scalingo/token_holder" require "scalingo/faraday/response" -require "scalingo/faraday/unpack_middleware" +require "scalingo/faraday/extract_meta" +require "scalingo/faraday/extract_root_value" require "active_support/core_ext/hash" module Scalingo @@ -87,7 +88,8 @@ def connection(fallback_to_guest: false) def unauthenticated_connection @unauthenticated_conn ||= Faraday.new(connection_options) { |conn| - conn.response :unpack + conn.response :extract_root_value + conn.response :extract_meta conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json @@ -108,7 +110,8 @@ def authenticated_connection end @connection = Faraday.new(connection_options) { |conn| - conn.response :unpack + conn.response :extract_root_value + conn.response :extract_meta conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json conn.request :authorization, "Bearer", -> { token_holder.token&.value } @@ -122,7 +125,8 @@ def database_connection(database_id) @database_connections ||= {} @database_connections[database_id] ||= Faraday.new(connection_options) { |conn| - conn.response :unpack + conn.response :extract_root_value + conn.response :extract_meta conn.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} conn.request :json conn.request :authorization, "Bearer", -> { token_holder.database_tokens[database_id]&.value } diff --git a/lib/scalingo/faraday/unpack_middleware.rb b/lib/scalingo/faraday/extract_meta.rb similarity index 55% rename from lib/scalingo/faraday/unpack_middleware.rb rename to lib/scalingo/faraday/extract_meta.rb index c7fb3fe..be0304f 100644 --- a/lib/scalingo/faraday/unpack_middleware.rb +++ b/lib/scalingo/faraday/extract_meta.rb @@ -1,12 +1,9 @@ require "faraday" module Scalingo - class UnpackMiddleware < Faraday::Middleware + class ExtractMeta < Faraday::Middleware def on_complete(env) - # We only want to unpack the response for successful responses - return unless env.response.success? - - # Only hash-like objects are relevant to "unpack" + # Only hash-like objects are relevant return unless env.body.is_a?(Hash) # Extract meta from response @@ -23,11 +20,8 @@ def on_complete(env) }.compact } end - - # Dig the root key if it's the only remaining key in the body - env.body = env.body.values.first if env.body.size == 1 end end end -Faraday::Response.register_middleware(unpack: Scalingo::UnpackMiddleware) +Faraday::Response.register_middleware(extract_meta: Scalingo::ExtractMeta) diff --git a/lib/scalingo/faraday/extract_root_value.rb b/lib/scalingo/faraday/extract_root_value.rb new file mode 100644 index 0000000..ac23782 --- /dev/null +++ b/lib/scalingo/faraday/extract_root_value.rb @@ -0,0 +1,18 @@ +require "faraday" + +module Scalingo + class ExtractRootValue < Faraday::Middleware + def on_complete(env) + # We only want to unpack the response for successful responses + return unless env.response.success? + + # Only hash-like objects are relevant to "unpack" + return unless env.body.is_a?(Hash) + + # Dig the root key if it's the only remaining key in the body + env.body = env.body.values.first if env.body.size == 1 + end + end +end + +Faraday::Response.register_middleware(extract_root_value: Scalingo::ExtractRootValue) diff --git a/lib/scalingo/faraday/response.rb b/lib/scalingo/faraday/response.rb index e7c8e7f..078be99 100644 --- a/lib/scalingo/faraday/response.rb +++ b/lib/scalingo/faraday/response.rb @@ -1,7 +1,8 @@ require "faraday" require "faraday/response" -# Some additional methods for Faraday::Response in order to enhance expressiveness +# Note: the file is scalingo/faraday/response but we're reopning Faraday::Response: +# we define additional methods for Faraday::Response in order to enhance expressiveness module Faraday class Response def client_error? diff --git a/spec/scalingo/faraday/extract_meta_spec.rb b/spec/scalingo/faraday/extract_meta_spec.rb new file mode 100644 index 0000000..8f7627e --- /dev/null +++ b/spec/scalingo/faraday/extract_meta_spec.rb @@ -0,0 +1,101 @@ +require "spec_helper" +require "scalingo/faraday/extract_meta" + +RSpec.shared_examples "no meta extraction" do + it "does not extract meta from response" do + expect(subject.meta).to eq(nil) + expect(subject.meta?).to eq(false) + expect(subject.pagination).to eq(nil) + expect(subject.paginated?).to eq(false) + expect(subject.cursor_pagination).to eq(nil) + expect(subject.cursor_paginated?).to eq(false) + end +end + +RSpec.describe Scalingo::ExtractMeta do + let(:headers) { {"Content-Type" => content_type}.compact } + let(:content_type) { "application/json" } + let(:body) { nil } + + let(:client) do + Faraday.new do |b| + b.response :extract_meta + b.response :json, content_type: /\bjson$/, parser_options: {symbolize_names: true} + b.adapter :test do |stub| + stub.get("/") { [nil, headers, body] } + end + end + end + + subject { client.get("/") } + + context "with a non-json response" do + let(:content_type) { "text/html" } + let(:body) { "string".to_json } + + include_examples "no meta extraction" + end + + context "without a body" do + include_examples "no meta extraction" + end + + context "with a non indexable body" do + let(:body) { "string".to_json } + + include_examples "no meta extraction" + end + + context "with a non hash indexable body" do + let(:body) { [1, 2, 3].to_json } + + include_examples "no meta extraction" + end + + context "with a hash" do + context "without a meta key" do + let(:body) { {a: 1, b: 2}.to_json } + + include_examples "no meta extraction" + end + + context "with a generic meta key" do + let(:body) { {meta: {a: 1}}.to_json } + + it "extracts the meta" do + expect(subject.meta).to eq({a: 1}) + expect(subject.meta?).to eq(true) + expect(subject.pagination).to eq(nil) + expect(subject.paginated?).to eq(false) + expect(subject.cursor_pagination).to eq(nil) + expect(subject.cursor_paginated?).to eq(false) + end + end + + context "with a pagination meta key" do + let(:body) { {meta: {pagination: {page: 1}}}.to_json } + + it "extracts the meta" do + expect(subject.meta).to eq({pagination: {page: 1}}) + expect(subject.meta?).to eq(true) + expect(subject.pagination).to eq({page: 1}) + expect(subject.paginated?).to eq(true) + expect(subject.cursor_pagination).to eq(nil) + expect(subject.cursor_paginated?).to eq(false) + end + end + + context "with a cursor pagination" do + let(:body) { {next_cursor: 1, has_more: true}.to_json } + + it "extracts the meta" do + expect(subject.meta).to eq({cursor_pagination: {next_cursor: 1, has_more: true}}) + expect(subject.meta?).to eq(true) + expect(subject.pagination).to eq(nil) + expect(subject.paginated?).to eq(false) + expect(subject.cursor_pagination).to eq({next_cursor: 1, has_more: true}) + expect(subject.cursor_paginated?).to eq(true) + end + end + end +end diff --git a/spec/scalingo/faraday/extract_root_value_spec.rb b/spec/scalingo/faraday/extract_root_value_spec.rb new file mode 100644 index 0000000..30eb2be --- /dev/null +++ b/spec/scalingo/faraday/extract_root_value_spec.rb @@ -0,0 +1,48 @@ +require "spec_helper" +require "scalingo/faraday/extract_root_value" + +RSpec.describe Scalingo::ExtractRootValue do + let(:status) { 200 } + let(:body) { {a: 1} } + + let(:client) do + Faraday.new do |b| + b.response :extract_root_value + b.adapter :test do |stub| + stub.get("/") { [status, {}, body] } + end + end + end + + subject { client.get("/") } + + context "with a non-successful response" do + let(:status) { 300 } + + it "does not change the response body" do + expect(subject.body).to eq({a: 1}) + end + end + + context "with a non-hash response" do + let(:body) { [{a: 1}] } + + it "does not change the response body" do + expect(subject.body).to eq([{a: 1}]) + end + end + + context "with a multi keys hash" do + let(:body) { {a: 1, b: 2} } + + it "does not change the response body" do + expect(subject.body).to eq({a: 1, b: 2}) + end + end + + context "with a single key hash" do + it "extracts the response body" do + expect(subject.body).to eq(1) + end + end +end From 63a26052e5a8982e0d84f4fbbf7c91b7c0c3f9e9 Mon Sep 17 00:00:00 2001 From: Kevin Soltysiak Date: Mon, 12 Feb 2024 10:16:24 +0100 Subject: [PATCH 9/9] cleaner code, changelog entries --- CHANGELOG.md | 3 +++ lib/scalingo/faraday/extract_meta.rb | 36 ++++++++++++++++------------ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 406154e..d236569 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Unreleased +** Breaking change: automatic digging of the value if the reponse body is an object with a single key +** Breaking change: remove `Scalingo::API::Reponse` in favor of `Faraday::Response` + ## 3.5.0 - 2023-12-28 * Change: update Faraday to 2.x, released about two years ago. The public API of this gem doesn't change, therefore this is not a major release. However, if you manipulate directly faraday's objects, you may encounter breaking changes. Refer to [Faraday's website](https://lostisland.github.io/faraday/) for how to migrate. diff --git a/lib/scalingo/faraday/extract_meta.rb b/lib/scalingo/faraday/extract_meta.rb index be0304f..51b73ff 100644 --- a/lib/scalingo/faraday/extract_meta.rb +++ b/lib/scalingo/faraday/extract_meta.rb @@ -3,23 +3,29 @@ module Scalingo class ExtractMeta < Faraday::Middleware def on_complete(env) - # Only hash-like objects are relevant return unless env.body.is_a?(Hash) - # Extract meta from response - if env.body[:meta] - env[:response_meta] = env.body.delete(:meta) - end - - # Extract cursor-based pagination - if env.body.key?(:next_cursor) - env[:response_meta] = { - cursor_pagination: { - next_cursor: env.body.delete(:next_cursor), - has_more: env.body.delete(:has_more) - }.compact - } - end + extract_metadata(env) + extract_cursor_metadata(env) + end + + private + + def extract_metadata(env) + return unless env.body[:meta] + + env[:response_meta] = env.body.delete(:meta) + end + + def extract_cursor_metadata(env) + return unless env.body.key?(:next_cursor) + + env[:response_meta] = { + cursor_pagination: { + next_cursor: env.body.delete(:next_cursor), + has_more: env.body.delete(:has_more) + }.compact + } end end end