Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Resolves #26] Support configuration for skip checking of schema existence before switching #45

Merged
merged 5 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion lib/apartment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions lib/apartment/adapters/jdbc_postgresql_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions lib/apartment/adapters/postgresql_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/apartment/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions spec/examples/generic_adapter_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
before do
Apartment.prepend_environment = false
Apartment.append_environment = false
Apartment.tenant_presence_check = true
end

describe '#init' do
Expand Down
24 changes: 20 additions & 4 deletions spec/examples/schema_adapter_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand All @@ -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
Expand Down