From 7eda570619e186cce5c54684d0b8f9a5bc87994a Mon Sep 17 00:00:00 2001 From: Felipe Sateler Date: Wed, 6 Jan 2021 12:08:19 -0300 Subject: [PATCH] fix: strip schema from sequence name sequence_name is shared across threads because it is stored in a class ivar. This means that on threaded servers, the sequence_name might point to a different tenant, if another tenant made the switch at the right time. This race is likely the cause of https://github.com/rails-on-services/apartment/issues/81 as well. This reverts commit f8eefc43767fd94a190b182d4ef58178bebebd11. --- lib/apartment.rb | 1 + .../active_record/postgresql_adapter.rb | 16 +++++++++++++++ lib/apartment/adapters/postgresql_adapter.rb | 20 ------------------- spec/examples/schema_adapter_examples.rb | 8 ++++++-- 4 files changed, 23 insertions(+), 22 deletions(-) create mode 100644 lib/apartment/active_record/postgresql_adapter.rb diff --git a/lib/apartment.rb b/lib/apartment.rb index 59a61cd1..427dbb3d 100644 --- a/lib/apartment.rb +++ b/lib/apartment.rb @@ -8,6 +8,7 @@ require_relative 'apartment/log_subscriber' +require_relative 'apartment/active_record/postgresql_adapter' if ActiveRecord.version.release >= Gem::Version.new('6.0') require_relative 'apartment/active_record/connection_handling' end diff --git a/lib/apartment/active_record/postgresql_adapter.rb b/lib/apartment/active_record/postgresql_adapter.rb new file mode 100644 index 00000000..5e52b147 --- /dev/null +++ b/lib/apartment/active_record/postgresql_adapter.rb @@ -0,0 +1,16 @@ +module Apartment::PostgreSqlAdapterPatch + def default_sequence_name(table, _column) + res = super + schema_prefix = "#{Apartment::Tenant.current}." + if res.starts_with?(schema_prefix) && Apartment.excluded_models.none?{|m| m.constantize.table_name == table} + res.delete_prefix!("#{Apartment::Tenant.current}.") + end + res + end +end + +require 'active_record/connection_adapters/postgresql_adapter' + +class ActiveRecord::ConnectionAdapters::PostgreSQLAdapter + include Apartment::PostgreSqlAdapterPatch +end \ No newline at end of file diff --git a/lib/apartment/adapters/postgresql_adapter.rb b/lib/apartment/adapters/postgresql_adapter.rb index 9540f016..9cc1fd4c 100644 --- a/lib/apartment/adapters/postgresql_adapter.rb +++ b/lib/apartment/adapters/postgresql_adapter.rb @@ -41,7 +41,6 @@ def default_tenant def reset @current = default_tenant Apartment.connection.schema_search_path = full_search_path - reset_sequence_names end def init @@ -83,7 +82,6 @@ def connect_to_new(tenant = nil) # there is a issue for prepared statement with changing search_path. # https://www.postgresql.org/docs/9.3/static/sql-prepare.html Apartment.connection.clear_cache! if postgresql_version < 90_300 - reset_sequence_names rescue *rescuable_exceptions raise TenantNotFound, "One of the following schema(s) is invalid: \"#{tenant}\" #{full_search_path}" end @@ -131,24 +129,6 @@ def postgresql_version # public from Rails 5.0. Apartment.connection.send(:postgresql_version) end - - def reset_sequence_names - # sequence_name contains the schema, so it must be reset after switch - # There is `reset_sequence_name`, but that method actually goes to the database - # to find out the new name. Therefore, we do this hack to only unset the name, - # and it will be dynamically found the next time it is needed - descendants_to_unset = ActiveRecord::Base.descendants - .select { |c| c.instance_variable_defined?(:@sequence_name) } - .reject do |c| - c.instance_variable_defined?(:@explicit_sequence_name) && - c.instance_variable_get(:@explicit_sequence_name) - end - descendants_to_unset.each do |c| - # NOTE: due to this https://github.com/rails-on-services/apartment/issues/81 - # unreproduceable error we're checking before trying to remove it - c.remove_instance_variable :@sequence_name if c.instance_variable_defined?(:@sequence_name) - end - end end # Another Adapter for Postgresql when using schemas and SQL diff --git a/spec/examples/schema_adapter_examples.rb b/spec/examples/schema_adapter_examples.rb index 447322e3..24045c11 100644 --- a/spec/examples/schema_adapter_examples.rb +++ b/spec/examples/schema_adapter_examples.rb @@ -27,6 +27,9 @@ Apartment::Tenant.init expect(Company.table_name).to eq('public.companies') + expect(User.table_name).to eq('users') + expect(Company.sequence_name).to eq('public.companies_id_seq') + expect(User.sequence_name).to eq('users_id_seq') end context 'with a default_tenant', default_tenant: true do @@ -34,6 +37,9 @@ Apartment::Tenant.init expect(Company.table_name).to eq("#{default_tenant}.companies") + expect(User.table_name).to eq('users') + expect(Company.sequence_name).to eq("#{default_tenant}.companies_id_seq") + expect(User.sequence_name).to eq('users_id_seq') end it 'sets the search_path correctly' do @@ -121,11 +127,9 @@ it 'connects and resets' do subject.switch(schema1) do expect(connection.schema_search_path).to start_with %("#{schema1}") - expect(User.sequence_name).to eq "#{schema1}.#{User.table_name}_id_seq" end expect(connection.schema_search_path).to start_with %("#{public_schema}") - expect(User.sequence_name).to eq "#{public_schema}.#{User.table_name}_id_seq" end end