Skip to content
New issue

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

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

Already on GitHub? # to your account

wrong ActiveRecord.sequence_name after switch tenant #508

Open
soupersoul opened this issue Dec 27, 2017 · 4 comments
Open

wrong ActiveRecord.sequence_name after switch tenant #508

soupersoul opened this issue Dec 27, 2017 · 4 comments

Comments

@soupersoul
Copy link

Steps to reproduce

source code:
test.rake

namespace :dev do
  task :test => :environment do
       t1 = Tenant.create!(name: 's99999')
       Apartment::Tenant.switch("s99999") do
          puts Book.sequence_name
       end
       t2 = Tenant.create!(name: 's88888')
       Apartment::Tenant.switch("s88888") do
          puts Book.sequence_name
       end
  end
end

Book

    create_table :books do |t|
      t.timestamps
    end

tenant

    create_table :tenants do |t|
      t.string  :name, null: false
      t.timestamps
    end

Expected behavior

output of rails dev:test is:

s99999.books_id_seq
s88888.books_id_seq

Actual behavior

output of rails dev:test is:

s99999.books_id_seq
s99999.books_id_seq

System configuration

  • Database: (Tell us what database and its version you use.)
    PostgresSQL9.6.6, compiled by Visual C++ build 1800, 64 bit
  • Apartment version:
    apartment (2.0.0)
  • Apartment config (in config/initializers/apartment.rb or so):
Apartment.configure do |config|
  config.excluded_models = %w[Tenant]
  config.tenant_names = -> { Tenant.pluck :name }
  config.use_schemas = true
  config.default_schema = 'public'
end

require 'apartment/adapters/postgresql_adapter'
require 'apartment/migrator'
module Apartment
  module Adapters
    class PostgresqlSchemaAdapter
      def import_database_schema
        Apartment::Migrator.migrate(Apartment::Tenant.current)
      end
    end
  end
end

  • use_schemas: (true or false)
    true

  • Rails (or ActiveRecord) version:
    5.1.0

  • Ruby version:
    2.4.2

@caifara
Copy link

caifara commented Mar 13, 2018

Same problem here on a Rails 4.2.x app.

Added this hackish piece of code to initializers/apartment.rb which does the job but is less than ideal:

module PostgresqlSequenceResetter
  def connect_to_new(tenant=nil)
    super

    model_names = ActiveRecord::Base.descendants.map(&:to_s)

    tables = ActiveRecord::Base.connection.tables
                                          .map(&:singularize)
                                          .map(&:camelize)

    resettable_tables = tables & model_names

    resettable_tables.each do |table|
      table.constantize.reset_sequence_name
    end
  end
end

require "apartment/adapters/postgresql_adapter"
Apartment::Adapters::PostgresqlSchemaAdapter.prepend PostgresqlSequenceResetter

@FUT
Copy link

FUT commented Dec 11, 2018

I would not recommend to use the answer from @caifara since it will just kill your application if it has 50+ models. The issue is that you will perform COUNT_OF_MODELS requests to database to reset sequences on each tenant switch. For my application with 180 models it was not a good idea.

You can achieve the same result in much easier way:

config/application.rb

    config.after_initialize do
      eager_load!
      ActiveRecord::Base.descendants.each do |model|
        seq_name = model.sequence_name
        if seq_name.present?
          tenant, seq_name = seq_name.include?('.') ? seq_name.split('.') : [nil, seq_name]
          if tenant != 'public'
            puts "Unsetting tenant for #{model.name}  =>  #{seq_name}" if Rails.env.development?
            model.sequence_name = seq_name
          end
        end
      end
    end

This way your database will search for sequence in current tenant and then in public tenant (or in the way you configured the search path).

@madzhuga
Copy link

@FUT thank you for your solution but there is a bug in it. If you update rake task following way:

namespace :dev do
  task :test => :environment do
    Apartment::Tenant.switch("first") do
      puts "Current tenant: #{Apartment::Tenant.current}"
      puts User.sequence_name
    end

    Apartment::Tenant.switch("public") do
      puts "Current tenant: #{Apartment::Tenant.current}"
      puts User.sequence_name
    end

    Apartment::Tenant.switch("first") do
      puts "Current tenant: #{Apartment::Tenant.current}"
      puts User.sequence_name
    end
  end
end

it will show that after switching to the public schema it does not update a sequence name properly.

I updated it a little + changed it to use tenant after switch hook. Put it to config/initializers/apartment.rb

module Apartment
  module Adapters
    class AbstractAdapter
      set_callback :switch, :after do |_object|

        ActiveRecord::Base.descendants.each do |model|
          seq_name = model.sequence_name

          next unless seq_name.present?

          _, seq_name = seq_name.include?('.') ? seq_name.split('.') : [nil, seq_name]
          model.sequence_name = Apartment::Tenant.current + '.' + seq_name
        end
      end
    end
  end
end

@fsateler
Copy link

The solution by @madzhuga will not work if you have models excluded from tenant. Currently I'm relying on an implementation detail of Rails, to avoid N database trips and this problem:

  ActiveRecord::Base.descendants.select{|c| c.instance_variable_defined?(:@sequence_name) }.each do |c|
    c.remove_instance_variable :@sequence_name unless c.instance_variable_defined?(:@explicit_sequence_name) && c.instance_variable_get(:@explicit_sequence_name)
  end

Yuck, I know 😉

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants