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

Revert unsafe initializer #118

Merged
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
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,17 @@ module Apartment
end
```

## Running rails console without a connection

Before this fork, running rails with the gem installed would connect to the database
which is different than the default behavior. To disable this initial
connection, just run with `APARTMENT_DISABLE_INIT` set to something:

```shell
$ APARTMENT_DISABLE_INIT=true DATABASE_URL=postgresql://localhost:1234/buk_development bin/rails runner 'puts 1'
# 1
```

## Contributing

* In both `spec/dummy/config` and `spec/config`, you will see `database.yml.sample` files
Expand Down
2 changes: 1 addition & 1 deletion lib/apartment/active_record/log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module ActiveRecord
# Supports the logging configuration to prepend the database and schema in the ActiveRecord log
class LogSubscriber
class LogSubscriber < ActiveSupport::LogSubscriber
def apartment_log
return unless Apartment.active_record_log

Expand Down
40 changes: 17 additions & 23 deletions lib/apartment/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

module Apartment
class Railtie < Rails::Railtie

#
# Set up our default config options
# Do this before the app initializers run so we don't override custom settings
Expand All @@ -25,28 +26,24 @@ class Railtie < Rails::Railtie
ActiveRecord::Migrator.migrations_paths = Rails.application.paths['db/migrate'].to_a
end

# Make sure Apartment is reconfigured when the code is reloaded
# Hook into ActionDispatch::Reloader to ensure Apartment is properly initialized
# Note that this doens't entirely work as expected in Development, because this is called before classes are reloaded
# See the middleware/console declarations below to help with this. Hope to fix that soon.
#
config.to_prepare do
Apartment::Tenant.reload!
end
next if ARGV.any? { |arg| arg =~ /\Aassets:(?:precompile|clean)\z/ }
next if ARGV.any? { |arg| arg == 'webpacker:compile' }
next if ENV["APARTMENT_DISABLE_INIT"]

#
# Ensure that Apartment::Tenant.init is called when
# a new connection is requested.
#
module ApartmentInitializer
def connection
super.tap do
Apartment::Tenant.init_once
begin
Apartment.connection_class.connection_pool.with_connection do
Apartment::Tenant.init
end
end

def arel_table
Apartment::Tenant.init_once
super
rescue ::ActiveRecord::NoDatabaseError
# Since `db:create` and other tasks invoke this block from Rails 5.2.0,
# we need to swallow the error to execute `db:create` properly.
end
end
ActiveRecord::Base.singleton_class.prepend ApartmentInitializer

#
# Ensure rake tasks are loaded
Expand All @@ -57,10 +54,8 @@ def arel_table
end

#
# The following initializers are a workaround to the fact that I can't
# properly hook into the rails reloader
# Note this is technically valid for any environment where cache_classes
# is false, for us, it's just development
# The following initializers are a workaround to the fact that I can't properly hook into the rails reloader
# Note this is technically valid for any environment where cache_classes is false, for us, it's just development
#
if Rails.env.development?

Expand All @@ -69,8 +64,7 @@ def arel_table
app.config.middleware.use Apartment::Reloader
end

# Overrides reload! to also call Apartment::Tenant.init as well so that the
# reloaded classes have the proper table_names
# Overrides reload! to also call Apartment::Tenant.init as well so that the reloaded classes have the proper table_names
console do
require 'apartment/console'
end
Expand Down
15 changes: 0 additions & 15 deletions lib/apartment/tenant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,6 @@ module Tenant

attr_writer :config

def init_once
return if @already_initialized

# To avoid infinite loops in work init is doing,
# we need to set @already_initialized to true
# before init is called
@already_initialized = true
init
end

def reinitialize
@already_initialized = false
end

# Fetch the proper multi-tenant adapter based on Rails config
#
# @return {subclass of Apartment::AbstractAdapter}
Expand Down Expand Up @@ -63,7 +49,6 @@ def adapter
#
def reload!(config = nil)
Thread.current[:apartment_adapter] = nil
reinitialize
@config = config
end

Expand Down
24 changes: 17 additions & 7 deletions spec/examples/generic_adapter_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,25 @@
.instance_variable_get(:@queue)
.size

expect(num_available_connections).to eq(0)
expect(num_available_connections).to eq(1)
end

# NOTE: Bug report #92 - when the adapter is set before the railtie is actually run, the adapter class might be
# incorrect. This tests intends to assure that the railtie init resets the adapter to the correct one
it 'ensures apartment_adapter is properly set during init' do
Thread.current[:apartment_adapter] = Apartment::Adapters::AbstractAdapter.new(config)
Apartment::Railtie.config.to_prepare_blocks.map(&:call)
expect(Apartment::Tenant.adapter.class.name).to eq subject.class.name
it 'should not connect if env var is set' do
ENV["APARTMENT_DISABLE_INIT"] = "true"
begin
ActiveRecord::Base.connection_pool.disconnect!

Apartment::Railtie.config.to_prepare_blocks.map(&:call)

num_available_connections = Apartment.connection_class.connection_pool
.instance_variable_get(:@available)
.instance_variable_get(:@queue)
.size

expect(num_available_connections).to eq(0)
ensure
ENV.delete("APARTMENT_DISABLE_INIT")
end
end
end

Expand Down