Rework the DB api access (#62)

CHANGELOG.md

@@ -1,7 +1,13 @@
 ## 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`
+* Breaking change: rework DB api exposition
+* Specs: rewrite all specs
+* Breaking change: endpoint methods declaration is simplified:
+    * based on URI templates
+    * argument and method names are unified
+    * one "main" internal method, `Endpoint#request`
+* 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
 

lib/scalingo/api/client.rb

@@ -9,10 +9,12 @@ module API
     class Client
       include TokenHolder
 
-      attr_reader :config, :token_holder, :url
+      attr_reader :config, :token_holder, :url, :region
 
-      def initialize(url, scalingo: nil, config: {})
+      def initialize(url, scalingo: nil, region: nil, config: {})
         @url = url
+        @region = region
+
         parent_config = Scalingo.config
 
         if scalingo
@@ -119,21 +121,6 @@ def authenticated_connection
           conn.adapter(config.faraday_adapter) if config.faraday_adapter
         }
       end
-
-      def database_connection(database_id)
-        raise Error::Unauthenticated unless token_holder.authenticated_for_database?(database_id)
-
-        @database_connections ||= {}
-        @database_connections[database_id] ||= Faraday.new(connection_options) { |conn|
-          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 }
-
-          conn.adapter(config.faraday_adapter) if config.faraday_adapter
-        }
-      end
     end
   end
 end

lib/scalingo/api/endpoint.rb

@@ -38,7 +38,6 @@ def initialize(client)
       end
 
       def_delegator :client, :connection
-      def_delegator :client, :database_connection
 
       # Perform a request to the API.
       # path can be an URI template; and faraday expect valid URIs - the parser raises when templates aren't fully expanded.

lib/scalingo/client.rb

@@ -2,53 +2,38 @@
 require "scalingo/auth"
 require "scalingo/billing"
 require "scalingo/regional"
-require "scalingo/regional_database"
+require "scalingo/database"
 
 module Scalingo
   class Client < CoreClient
+    URLS = {
+      auth: "https://auth.scalingo.com/v1",
+      billing: "https://cashmachine.scalingo.com",
+      regional: {
+        osc_fr1: "https://api.osc-fr1.scalingo.com/v1",
+        osc_secnum_fr1: "https://api.osc-secnum-fr1.scalingo.com/v1"
+      },
+      database: {
+        osc_fr1: "https://db-api.osc-fr1.scalingo.com/api",
+        osc_secnum_fr1: "https://db-api.osc-secnum-fr1.scalingo.com/api"
+      }
+    }
+
     ## API clients
     def auth
-      @auth ||= Auth.new(
-        "https://auth.scalingo.com/v1",
-        scalingo: self
-      )
+      @auth ||= Auth.new(URLS[:auth], scalingo: self)
     end
 
     def billing
-      @billing ||= Billing.new(
-        "https://cashmachine.scalingo.com",
-        scalingo: self
-      )
+      @billing ||= Billing.new(URLS[:billing], scalingo: self)
     end
 
     def osc_fr1
-      @osc_fr1 ||= Regional.new(
-        "https://api.osc-fr1.scalingo.com/v1",
-        scalingo: self
-      )
+      @osc_fr1 ||= Regional.new(URLS[:regional][:osc_fr1], scalingo: self, region: :osc_fr1)
     end
-    alias_method :apps_api_osc_fr1, :osc_fr1
 
     def osc_secnum_fr1
-      @osc_secnum_fr1 ||= Regional.new(
-        "https://api.osc-secnum-fr1.scalingo.com/v1",
-        scalingo: self
-      )
-    end
-    alias_method :apps_api_osc_secnum_fr1, :osc_secnum_fr1
-
-    def db_api_osc_fr1
-      @db_api_osc_fr1 ||= RegionalDatabase.new(
-        "https://db-api.osc-fr1.scalingo.com/api",
-        scalingo: self
-      )
-    end
-
-    def db_api_osc_secnum_fr1
-      @db_api_osc_secnum_fr1 ||= RegionalDatabase.new(
-        "https://db-api.osc-secnum-fr1.scalingo.com/api",
-        scalingo: self
-      )
+      @osc_secnum_fr1 ||= Regional.new(URLS[:regional][:osc_secnum_fr1], scalingo: self, region: :osc_secnum_fr1)
     end
 
     ## Helpers

lib/scalingo/core_client.rb

@@ -42,10 +42,6 @@ def region(name = nil)
       public_send(name || config.default_region)
     end
 
-    def database_region(name = nil)
-      public_send(name || "db_api_#{config.default_region}")
-    end
-
     ## Authentication helpers / Token management
     def authenticate_with(access_token: nil, bearer_token: nil, expires_at: nil)
       if !access_token && !bearer_token
@@ -107,8 +103,5 @@ def authenticate_with(access_token: nil, bearer_token: nil, expires_at: nil)
     def_delegator :region, :notifiers
     def_delegator :region, :operations
     def_delegator :region, :scm_repo_links
-
-    def_delegator :database_region, :databases
-    def_delegator :database_region, :backups
   end
 end

lib/scalingo/regional_database.rb → lib/scalingo/database.rb

@@ -1,9 +1,9 @@
 require "scalingo/api/client"
 
 module Scalingo
-  class RegionalDatabase < API::Client
-    require "scalingo/regional_database/databases"
-    require "scalingo/regional_database/backups"
+  class Database < API::Client
+    require "scalingo/database/databases"
+    require "scalingo/database/backups"
 
     register_handlers!(
       databases: Databases,

lib/scalingo/regional_database/backups.rb → lib/scalingo/database/backups.rb

@@ -1,7 +1,7 @@
 require "scalingo/api/endpoint"
 
 module Scalingo
-  class RegionalDatabase::Backups < API::Endpoint
+  class Database::Backups < API::Endpoint
     get :list, "databases/{addon_id}/backups"
     post :create, "databases/{addon_id}/backups"
     get :archive, "databases/{addon_id}/backups/{id}/archive"

lib/scalingo/regional_database/databases.rb → lib/scalingo/database/databases.rb

@@ -1,7 +1,7 @@
 require "scalingo/api/endpoint"
 
 module Scalingo
-  class RegionalDatabase::Databases < API::Endpoint
+  class Database::Databases < API::Endpoint
     get :find, "databases/{id}"
     post :upgrade, "databases/{id}/upgrade"
   end

lib/scalingo/regional/addons.rb

@@ -12,18 +12,16 @@ class Regional::Addons < API::Endpoint
     get :categories, "addon_categories", connected: false
     get :providers, "addon_providers", connected: false
 
-    def authenticate!(**params, &block)
-      response = token(**params, &block)
-      return response unless response.status == 200
+    def database_client_for(app_id:, id:)
+      response = token(app_id: app_id, id: id)
 
-      client.token_holder.authenticate_database_with_bearer_token(
-        params[:id],
-        response.body[:token],
-        expires_at: Time.now + 1.hour,
-        raise_on_expired_token: client.config.raise_on_expired_token
-      )
+      return nil unless response.status == 200
 
-      response
+      db_url = Scalingo::Client::URLS.fetch(:database).fetch(client.region)
+
+      db_client = Scalingo::Database.new(db_url, region: client.region)
+      db_client.token = response.body.fetch(:token)
+      db_client
     end
   end
 end

lib/scalingo/token_holder.rb

@@ -4,45 +4,22 @@ module Scalingo
   module TokenHolder
     def self.included(base)
       base.attr_reader :token
-      base.attr_reader :database_tokens
     end
 
     def token=(input)
       @token = bearer_token(input)
     end
 
-    def add_database_token(database_id, token)
-      @database_tokens ||= {}
-      @database_tokens[database_id] = bearer_token(token)
-    end
-
     def authenticated?
-      valid?(token)
-    end
-
-    def authenticated_for_database?(database_id)
-      return false if database_tokens.nil?
-      return false unless database_tokens.has_key?(database_id)
-
-      valid?(database_tokens[database_id])
+      token.present? && !token.expired?
     end
 
     def authenticate_with_bearer_token(bearer_token, expires_at:, raise_on_expired_token:)
       self.token = build_bearer_token(bearer_token, expires_at: expires_at, raise_on_expired_token: raise_on_expired_token)
     end
 
-    def authenticate_database_with_bearer_token(database_id, bearer_token, expires_at:, raise_on_expired_token:)
-      bearer_token = build_bearer_token(bearer_token, expires_at: expires_at, raise_on_expired_token: raise_on_expired_token)
-
-      add_database_token(database_id, bearer_token)
-    end
-
     private
 
-    def valid?(token)
-      token.present? && !token.expired?
-    end
-
     def bearer_token(token)
       token.is_a?(BearerToken) ? token : BearerToken.new(token.to_s, raise_on_expired: config.raise_on_expired_token)
     end

spec/scalingo/api/client_spec.rb

@@ -66,6 +66,13 @@
         subject
       end
     end
+
+    describe "region" do
+      it "keeps track of the region if supplied" do
+        instance = described_class.new(:url, region: "my-region")
+        expect(instance.region).to eq("my-region")
+      end
+    end
   end
 
   describe "self.register_handler(s)!" do
@@ -179,53 +186,6 @@
     end
   end
 
-  describe "database_connection" do
-    let(:database_id) { "db-id-1234" }
-
-    context "without bearer token" do
-      let(:bearer_token) { nil }
-
-      it "raises" do
-        expect {
-          subject.database_connection(database_id)
-        }.to raise_error(Scalingo::Error::Unauthenticated)
-      end
-    end
-
-    context "with bearer token" do
-      before { stub_request(:any, "localhost") }
-
-      it "has an authentication header set with a bearer scheme" do
-        scalingo.authenticate_database_with_bearer_token(
-          database_id,
-          "1234",
-          expires_at: Time.now + 1.hour,
-          raise_on_expired_token: false
-        )
-
-        request_headers = subject.database_connection(database_id).get("/").env.request_headers
-        expected = "Bearer #{subject.token_holder.database_tokens[database_id].value}"
-
-        expect(request_headers["Authorization"]).to eq(expected)
-      end
-    end
-
-    context "with wrong bearer token" do
-      it "raises" do
-        database_id_2 = "db-id-5678"
-        scalingo.authenticate_database_with_bearer_token(
-          database_id_2,
-          "1234",
-          expires_at: Time.now + 1.hour,
-          raise_on_expired_token: false
-        )
-        expect {
-          subject.database_connection(database_id)
-        }.to raise_error(Scalingo::Error::Unauthenticated)
-      end
-    end
-  end
-
   describe "connection" do
     context "when logged" do
       context "without fallback to guest" do

spec/scalingo/regional_database/backups_spec.rb → spec/scalingo/database/backups_spec.rb

@@ -1,12 +1,8 @@
 require "spec_helper"
 
-RSpec.describe Scalingo::RegionalDatabase::Backups, type: :endpoint do
+RSpec.describe Scalingo::Database::Backups, type: :endpoint do
   let(:addon_id) { "the-addon-id" }
 
-  before do
-    scalingo_client.add_database_token(addon_id, "the-bearer-token")
-  end
-
   describe "list" do
     subject(:response) { instance.list(**arguments) }
 

spec/scalingo/regional_database/databases_spec.rb → spec/scalingo/database/databases_spec.rb

@@ -1,12 +1,8 @@
 require "spec_helper"
 
-RSpec.describe Scalingo::RegionalDatabase::Databases, type: :endpoint do
+RSpec.describe Scalingo::Database::Databases, type: :endpoint do
   let(:id) { "database-id" }
 
-  before do
-    scalingo_client.add_database_token(id, "the-bearer-token")
-  end
-
   describe "find" do
     subject(:response) { instance.find(**arguments) }
 

spec/scalingo/regional_database_spec.rb → spec/scalingo/database_spec.rb

@@ -1,6 +1,6 @@
 require "spec_helper"
 
-RSpec.describe Scalingo::RegionalDatabase do
+RSpec.describe Scalingo::Database do
   subject { described_class.new("url") }
 
   %w[databases backups].each do |section|