Add default_tenant to PSQL_DUMP_BLACKLISTED_STATEMENTS

rbngzlv commented 3 years ago

Steps to reproduce

Configure a default_tenant that isn't the public default and try to create a new tenant. This fails with:

ActiveRecord::StatementInvalid: PG::DuplicateSchema: ERROR:  schema "aplicacio" already exists

Seems that the problem is raised when cloning the schema to the new one, because the pg_dump includes the creation of the already existing schema (the default):

--
-- Name: aplicacio; Type: SCHEMA; Schema: -; Owner: -
--

CREATE SCHEMA aplicacio;

This doesn't happen when using public as the default tenant because this statements are blacklisted:

https://github.com/rails-on-services/apartment/blob/c733f35bf8da44fb4d9dda8499066f1409aa4b99/lib/apartment/adapters/postgresql_adapter.rb#L160-L169

Here is where the pg_dump output is "sanitized":

https://github.com/rails-on-services/apartment/blob/c733f35bf8da44fb4d9dda8499066f1409aa4b99/lib/apartment/adapters/postgresql_adapter.rb#L255-L263

WDYT if we move the blacklisting of the public statements from the constant PSQL_DUMP_BLACKLISTED_STATEMENTS to inside the patch_search_path method? Any other approach?

As a note, this method is doing more than patching the search path 😄 , are you open to rename the method?

Thank you.

Expected behavior

The creation of a new tenant doesn't raises any error

Actual behavior

The creation of a new tenant fails:

> Apartment::Tenant.create("new_tenant")

Caused by:
ActiveRecord::StatementInvalid: PG::DuplicateSchema: ERROR:  schema "aplicacio" already exists
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/postgresql/database_statements.rb:92:in `exec'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/postgresql/database_statements.rb:92:in `block (2 levels) in execute'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/dependencies/interlock.rb:48:in `block in permit_concurrent_loads'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/share_lock.rb:187:in `yield_shares'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/dependencies/interlock.rb:47:in `permit_concurrent_loads'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/postgresql/database_statements.rb:91:in `block in execute'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/abstract_adapter.rb:722:in `block (2 levels) in log'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/abstract_adapter.rb:721:in `block in log'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/abstract_adapter.rb:712:in `log'
/usr/local/bundle/gems/activerecord-6.0.3.6/lib/active_record/connection_adapters/postgresql/database_statements.rb:90:in `execute'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/postgresql_adapter.rb:191:in `clone_pg_schema'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/postgresql_adapter.rb:170:in `block in import_database_schema'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/postgresql_adapter.rb:183:in `preserving_search_path'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/postgresql_adapter.rb:169:in `import_database_schema'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/abstract_adapter.rb:28:in `block (2 levels) in create'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/abstract_adapter.rb:89:in `switch'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/abstract_adapter.rb:27:in `block in create'
/usr/local/bundle/gems/activesupport-6.0.3.6/lib/active_support/callbacks.rb:101:in `run_callbacks'
/usr/local/bundle/gems/ros-apartment-2.9.0/lib/apartment/adapters/abstract_adapter.rb:24:in `create'
/var/app/app/models/center.rb:15:in `create_tenant'
....

System configuration

Database: (Tell us what database and its version you use.) PostgreSQL 11.9
Apartment version: 2.9

Apartment config (in config/initializers/apartment.rb or so):

Apartment.configure do |config|
config.default_tenant = "aplicacio"
config.persistent_schemas = %w(extensions)
end

use_schemas: (true or false) true

Rails (or ActiveRecord) version: Rails 6.0.3.7
Ruby version: ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux-musl]

martinbarilik commented 5 months ago

Have you solved this ? I am facing the same issue when setting custom default_tenant = 'apartment_template'

rbngzlv commented 5 months ago

I checked a couple of projects and one of them is overriding the constant value in an initializer with a duplicate of the original array plus the following lines.

/CREATE SCHEMA #{Apartment.default_tenant}/i
/COMMENT ON SCHEMA #{Apartment.default_tenant}/i

rbngzlv commented 1 month ago

Hi @mnovelo! I would like to know your opinion on this to see if I can help with this. Thank you!

mnovelo commented 1 month ago

@rbngzlv I don't see a problem with this. I'm happy to review a PR!

rails-on-services / apartment