Merge pull request #48 from rails-on-services/development

- [Resolves #42]
- [Resolves #26]
- [Resolves #41]
- [Resolves #37]
- Updated github actions configuration to run on PRs as well

.github/workflows/.rubocop-linter.yml

@@ -1,6 +1,6 @@
 name: Rubocop Lint
 
-on: [push]
+on: [push, pull_request]
 
 jobs:
   build:

.rubocop_todo.yml

@@ -21,7 +21,7 @@ Metrics/BlockLength:
 # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
-  Max: 237
+  Max: 200
 
 # Offense count: 4
 # Configuration parameters: CountComments, ExcludedMethods.

README.md

@@ -324,6 +324,20 @@ Apartment.configure do |config|
 end
 ```
 
+### Skip tenant schema check
+
+This is configurable by setting: `tenant_presence_check`. It defaults to true
+in order to maintain the original gem behavior. This is only checked when using one of the PostgreSQL adapters.
+The original gem behavior, when running `switch` would look for the existence of the schema before switching. This adds an extra query on every context switch. While in the default simple scenarios this is a valid check, in high volume platforms this adds some unnecessary overhead which can be detected in some other ways on the application level.
+
+Setting this configuration value to `false` will disable the schema presence check before trying to switch the context.
+
+```ruby
+Apartment.configure do |config|
+  tenant_presence_check = false
+end
+```
+
 ### Excluding models
 
 If you have some models that should always access the 'public' tenant, you can specify this by configuring Apartment using `Apartment.configure`. This will yield a config object for you. You can set excluded models like so:

lib/apartment.rb

@@ -20,7 +20,7 @@ class << self
     extend Forwardable
 
     ACCESSOR_METHODS = %i[use_schemas use_sql seed_after_create prepend_environment
-                          append_environment with_multi_server_setup].freeze
+                          append_environment with_multi_server_setup tenant_presence_check].freeze
 
     WRITER_METHODS = %i[tenant_names database_schema_file excluded_models
                         default_schema persistent_schemas connection_class

lib/apartment/adapters/abstract_adapter.rb

@@ -68,8 +68,6 @@ def drop(tenant)
       #
       def switch!(tenant = nil)
         run_callbacks :switch do
-          return reset if tenant.nil?
-
           connect_to_new(tenant).tap do
             Apartment.connection.clear_query_cache
           end
@@ -130,14 +128,12 @@ def seed_data
       #   @return {String} tenant name with Rails environment *optionally* prepended
       #
       def environmentify(tenant)
-        if !tenant.include?(Rails.env)
-          if Apartment.prepend_environment
-            "#{Rails.env}_#{tenant}"
-          elsif Apartment.append_environment
-            "#{tenant}_#{Rails.env}"
-          else
-            tenant
-          end
+        return tenant if tenant.nil? || tenant.include?(Rails.env)
+
+        if Apartment.prepend_environment
+          "#{Rails.env}_#{tenant}"
+        elsif Apartment.append_environment
+          "#{tenant}_#{Rails.env}"
         else
           tenant
         end
@@ -175,6 +171,8 @@ def create_tenant_command(conn, tenant)
       #   @param {String} tenant Database name
       #
       def connect_to_new(tenant)
+        return reset if tenant.nil?
+
         query_cache_enabled = ActiveRecord::Base.connection.query_cache_enabled
 
         Apartment.establish_connection multi_tenantify(tenant)

lib/apartment/adapters/jdbc_postgresql_adapter.rb

@@ -37,19 +37,24 @@ class JDBCPostgresqlSchemaAdapter < PostgresqlSchemaAdapter
       #
       def connect_to_new(tenant = nil)
         return reset if tenant.nil?
-        # rubocop:disable Style/RaiseArgs
-        raise ActiveRecord::StatementInvalid.new("Could not find schema #{tenant}") unless Apartment.connection.all_schemas.include? tenant.to_s
 
-        # rubocop:enable Style/RaiseArgs
+        tenant = tenant.to_s
+        raise ActiveRecord::StatementInvalid, "Could not find schema #{tenant}" unless tenant_exists?(tenant)
 
-        @current = tenant.to_s
+        @current = tenant
         Apartment.connection.schema_search_path = full_search_path
       rescue ActiveRecord::StatementInvalid, ActiveRecord::JDBCError
         raise TenantNotFound, "One of the following schema(s) is invalid: #{full_search_path}"
       end
 
       private
 
+      def tenant_exists?(tenant)
+        return true unless Apartment.tenant_presence_check
+
+        Apartment.connection.all_schemas.include? tenant
+      end
+
       def rescue_from
         ActiveRecord::JDBCError
       end

lib/apartment/adapters/postgresql_adapter.rb

@@ -62,12 +62,11 @@ def drop_command(conn, tenant)
       #
       def connect_to_new(tenant = nil)
         return reset if tenant.nil?
-        # rubocop:disable Style/RaiseArgs
-        raise ActiveRecord::StatementInvalid.new("Could not find schema #{tenant}") unless Apartment.connection.schema_exists?(tenant.to_s)
 
-        # rubocop:enable Style/RaiseArgs
+        tenant = tenant.to_s
+        raise ActiveRecord::StatementInvalid, "Could not find schema #{tenant}" unless tenant_exists?(tenant)
 
-        @current = tenant.to_s
+        @current = tenant
         Apartment.connection.schema_search_path = full_search_path
 
         # When the PostgreSQL version is < 9.3,
@@ -80,6 +79,12 @@ def connect_to_new(tenant = nil)
 
       private
 
+      def tenant_exists?(tenant)
+        return true unless Apartment.tenant_presence_check
+
+        Apartment.connection.schema_exists?(tenant)
+      end
+
       def create_tenant_command(conn, tenant)
         conn.execute(%(CREATE SCHEMA "#{tenant}"))
       end

lib/apartment/adapters/sqlite3_adapter.rb

@@ -33,6 +33,8 @@ def current
       protected
 
       def connect_to_new(tenant)
+        return reset if tenant.nil?
+
         unless File.exist?(database_file(tenant))
           raise TenantNotFound,
                 "The tenant #{environmentify(tenant)} cannot be found."

lib/apartment/console.rb

@@ -35,3 +35,10 @@ def tenant_list
   tenant_list += Apartment.tenant_names
   tenant_list.uniq
 end
+
+def tenant_info_msg
+  # rubocop:disable Rails/Output
+  puts "Available Tenants: #{tenant_list}\n"
+  puts "Use `st 'tenant'` to switch tenants & `tenant_list` to see list\n"
+  # rubocop:enable Rails/Output
+end

lib/apartment/custom_console.rb

@@ -10,17 +10,31 @@ module CustomConsole
       # rubocop:enable Rails/Output
     end
 
-    if Pry::Prompt.respond_to?(:add)
-      desc = "Includes the current Rails environment and project folder name.\n" \
-            '[1] [project_name][Rails.env][Apartment::Tenant.current] pry(main)>'
+    desc = "Includes the current Rails environment and project folder name.\n" \
+          '[1] [project_name][Rails.env][Apartment::Tenant.current] pry(main)>'
 
+    prompt_procs = [
+      proc { |target_self, nest_level, pry| prompt_contents(pry, target_self, nest_level, '>') },
+      proc { |target_self, nest_level, pry| prompt_contents(pry, target_self, nest_level, '*') }
+    ]
+
+    if Gem::Version.new(Pry::VERSION) >= Gem::Version.new('0.13')
+      Pry.config.prompt = Pry::Prompt.new 'ros', desc, prompt_procs
+    else
       Pry::Prompt.add 'ros', desc, %w[> *] do |target_self, nest_level, pry, sep|
-        "[#{pry.input_ring.size}] [#{PryRails::Prompt.formatted_env}][#{Apartment::Tenant.current}] " \
-        "#{pry.config.prompt_name}(#{Pry.view_clip(target_self)})" \
-        "#{":#{nest_level}" unless nest_level.zero?}#{sep} "
+        prompt_contents(pry, target_self, nest_level, sep)
       end
-
       Pry.config.prompt = Pry::Prompt[:ros][:value]
     end
+
+    Pry.config.hooks.add_hook(:when_started, 'startup message') do
+      tenant_info_msg
+    end
+
+    def self.prompt_contents(pry, target_self, nest_level, sep)
+      "[#{pry.input_ring.size}] [#{PryRails::Prompt.formatted_env}][#{Apartment::Tenant.current}] " \
+      "#{pry.config.prompt_name}(#{Pry.view_clip(target_self)})" \
+      "#{":#{nest_level}" unless nest_level.zero?}#{sep} "
+    end
   end
 end

lib/apartment/railtie.rb

@@ -18,6 +18,7 @@ class Railtie < Rails::Railtie
         config.seed_after_create = false
         config.prepend_environment = false
         config.append_environment = false
+        config.tenant_presence_check = true
       end
 
       ActiveRecord::Migrator.migrations_paths = Rails.application.paths['db/migrate'].to_a

lib/apartment/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Apartment
-  VERSION = '2.5.0'
+  VERSION = '2.6.0'
 end

spec/adapters/jdbc_mysql_adapter_spec.rb

@@ -14,6 +14,7 @@ def tenant_names
 
     let(:default_tenant) { subject.switch { ActiveRecord::Base.connection.current_database } }
 
+    it_should_behave_like 'a generic apartment adapter callbacks'
     it_should_behave_like 'a generic apartment adapter'
     it_should_behave_like 'a connection based apartment adapter'
   end

spec/adapters/jdbc_postgresql_adapter_spec.rb

@@ -8,6 +8,8 @@
   describe Apartment::Adapters::JDBCPostgresqlAdapter, database: :postgresql do
     subject { Apartment::Tenant.jdbc_postgresql_adapter config.symbolize_keys }
 
+    it_should_behave_like 'a generic apartment adapter callbacks'
+
     context 'using schemas' do
       before { Apartment.use_schemas = true }
 

spec/adapters/mysql2_adapter_spec.rb

@@ -14,6 +14,8 @@ def tenant_names
 
     let(:default_tenant) { subject.switch { ActiveRecord::Base.connection.current_database } }
 
+    it_should_behave_like 'a generic apartment adapter callbacks'
+
     context 'using - the equivalent of - schemas' do
       before { Apartment.use_schemas = true }
 

spec/adapters/postgresql_adapter_spec.rb

@@ -8,6 +8,8 @@
 
     subject { Apartment::Tenant.postgresql_adapter config }
 
+    it_should_behave_like 'a generic apartment adapter callbacks'
+
     context 'using schemas with schema.rb' do
       before { Apartment.use_schemas = true }
 

spec/adapters/sqlite3_adapter_spec.rb

@@ -8,6 +8,8 @@
 
     subject { Apartment::Tenant.sqlite3_adapter config }
 
+    it_should_behave_like 'a generic apartment adapter callbacks'
+
     context 'using connections' do
       def tenant_names
         db_dir = File.expand_path('../dummy/db', __dir__)

spec/examples/generic_adapter_examples.rb

@@ -8,6 +8,7 @@
   before do
     Apartment.prepend_environment = false
     Apartment.append_environment = false
+    Apartment.tenant_presence_check = true
   end
 
   describe '#init' do

spec/examples/generic_adapters_callbacks_examples.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples_for 'a generic apartment adapter callbacks' do
+  class MyProc
+    def self.call(tenant_name); end
+  end
+
+  include Apartment::Spec::AdapterRequirements
+
+  before do
+    Apartment.prepend_environment = false
+    Apartment.append_environment = false
+  end
+
+  describe '#switch!' do
+    before do
+      Apartment::Adapters::AbstractAdapter.set_callback :switch, :before do
+        MyProc.call(Apartment::Tenant.current)
+      end
+
+      Apartment::Adapters::AbstractAdapter.set_callback :switch, :after do
+        MyProc.call(Apartment::Tenant.current)
+      end
+
+      allow(MyProc).to receive(:call)
+    end
+
+    # NOTE: Part of the test setup creates and switches tenants, so we need
+    # to reset the callbacks to ensure that each test run has the correct
+    # counts
+    after do
+      Apartment::Adapters::AbstractAdapter.reset_callbacks :switch
+    end
+
+    context 'when tenant is nil' do
+      before do
+        Apartment::Tenant.switch!(nil)
+      end
+
+      it 'runs both before and after callbacks' do
+        expect(MyProc).to have_received(:call).twice
+      end
+    end
+
+    context 'when tenant is not nil' do
+      before do
+        Apartment::Tenant.switch!(db1)
+      end
+
+      it 'runs both before and after callbacks' do
+        expect(MyProc).to have_received(:call).twice
+      end
+    end
+  end
+end

spec/examples/schema_adapter_examples.rb

@@ -164,6 +164,10 @@
   end
 
   describe '#switch!' do
+    let(:tenant_presence_check) { true }
+
+    before { Apartment.tenant_presence_check = tenant_presence_check }
+
     it 'should connect to new schema' do
       subject.switch!(schema1)
       expect(connection.schema_search_path).to start_with %("#{schema1}")
@@ -174,10 +178,22 @@
       expect(connection.schema_search_path).to eq(%("#{public_schema}"))
     end
 
-    it 'should raise an error if schema is invalid' do
-      expect do
-        subject.switch! 'unknown_schema'
-      end.to raise_error(Apartment::TenantNotFound)
+    context 'when configuration checks for tenant presence before switching' do
+      it 'should raise an error if schema is invalid' do
+        expect do
+          subject.switch! 'unknown_schema'
+        end.to raise_error(Apartment::TenantNotFound)
+      end
+    end
+
+    context 'when configuration skips tenant presence check before switching' do
+      let(:tenant_presence_check) { false }
+
+      it 'should not raise any errors' do
+        expect do
+          subject.switch! 'unknown_schema'
+        end.to_not raise_error(Apartment::TenantNotFound)
+      end
     end
 
     context 'numeric databases' do