wireframe / multitenant

making cross tenant data leaks a thing of the past.
http://blog.codecrate.com/2011/03/multitenant-locking-down-your-app-and.html
MIT License
164 stars 36 forks source link

Make current_tenant thread safe #19

Closed sapanakothari closed 3 years ago

wireframe commented 3 years ago

Thank you for your contribution! The code addition is super straightforward, as expected. The real trick is on the testing side. My recommendation is to simplify the testcase such that the new thread attempts to overwrite the current tenant variable. There are some good examples available here: https://stackoverflow.com/questions/6241384/how-do-i-manage-ruby-threads-so-they-finish-all-their-work

describe '#current_tenant=` do
  it 'is threadsafe' do
    Multitenant.current_tenant = :old_tenant
    overwriter = Thread.new { Multitenant.current_tenant = :new_tenant }
    overwriter.join
    expect(Multitenant.current_tenant).to eq :old_tenant
  end
end
sapanakothari commented 3 years ago

Thank you for your contribution! The code addition is super straightforward, as expected. The real trick is on the testing side. My recommendation is to simplify the testcase such that the new thread attempts to overwrite the current tenant variable. There are some good examples available here: https://stackoverflow.com/questions/6241384/how-do-i-manage-ruby-threads-so-they-finish-all-their-work

describe '#current_tenant=` do
  it 'is threadsafe' do
    Multitenant.current_tenant = :old_tenant
    overwriter = Thread.new { Multitenant.current_tenant = :new_tenant }
    overwriter.join
    expect(Multitenant.current_tenant).to eq :old_tenant
  end
end

Thankyou for your review and feedback. Changes are incorporated, kindly give another go.

wireframe commented 3 years ago

Thank you @sapanakothari for your contribution! This has been merged and released in the v1.0.0 release.

sapanakothari commented 3 years ago

Thanks @wireframe!