Skip to content

Commit baaa948

Browse files
committed
Add some safeguards around calling .tenanted
1 parent 221104c commit baaa948

File tree

4 files changed

+29
-7
lines changed

4 files changed

+29
-7
lines changed

GUIDE.md

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,23 @@ TODO:
6060

6161
- Active Record class methods
6262
- [ ] `.tenanted`
63-
- extends with `Base`
64-
- sets `Tenant.base_class=`
65-
- must only be set ONCE in the application
63+
- [x] mixin `Tenant`
64+
- [x] should error if self is not an abstract base class
65+
- [x] `tenant_config_name` and `.tenanted?`
6666
- [ ] `.tenanted_with`
67-
- extends with `Sublet`
68-
- should error if self is not an abstract base class or if target is not tenanted abstract base class
69-
- is the name right? should we have to provide the name of the tenanted class?
70-
- [ ] `.tenanted?`
67+
- [ ] mixin `Subtenant`
68+
- [ ] should error if self is not an abstract base class or if target is not tenanted abstract base class
7169
- [ ] `.tenanted_class` nil or the abstract base class
7270
- [ ] all the creation and schema migration complications (we have existing tests for this)
7371
- think about race conditions here, maybe use a file lock to figure it out
7472
- running migrations (they are done in a transaction, but the second thread's migration may fail resulting in a 500?)
7573
- loading schemas (if the first thread loads the schema and inserts data, can the second thread accidentally drop/load causing data loss?)
7674
- [ ] feature to turn off automatic creation/migration
7775
- make sure we pay attention to Rails.config.active_record.migration_error when we turn off auto-migrating
76+
- thinking
77+
- should there be a global singleton `Tenant`? I'm not sure we need it or the limitations of a global.
78+
- if we do, though, then `.tenanted` should set `Tenant.base_class=`
79+
- and we need to add checks that `.tenanted` is called only ONCE in the application
7880

7981
- database tasks
8082
- [ ] RootConfig should conditionally re-enable database tasks ... when AR_TENANT is present?

lib/active_record/tenanted.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
module ActiveRecord
1010
module Tenanted
11+
class Error < StandardError; end
1112
end
1213
end
1314

lib/active_record/tenanted/base.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ def initialize(...)
1313
end
1414

1515
def tenanted(config_name = "primary")
16+
raise Error, "Class #{self} is already tenanted" if tenanted?
17+
raise Error, "Class #{self} is not an abstract connection class" unless abstract_class?
18+
1619
include Tenant
1720

1821
@tenanted_config_name = config_name

test/unit/base_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@
2020
end
2121
end
2222

23+
with_scenario(:vanilla, :tenanted_primary) do
24+
test "it can only be called once" do
25+
e = assert_raises(ActiveRecord::Tenanted::Error) do
26+
TenantedApplicationRecord.tenanted
27+
end
28+
assert_includes(e.message, "already tenanted")
29+
end
30+
31+
test "it can only be called on abstract classes" do
32+
e = assert_raises(ActiveRecord::Tenanted::Error) do
33+
Announcement.tenanted
34+
end
35+
assert_includes(e.message, "not an abstract connection class")
36+
end
37+
end
38+
2339
with_each_scenario do
2440
test "it includes the Tenant module" do
2541
assert_includes(TenantedApplicationRecord.ancestors, ActiveRecord::Tenanted::Tenant)

0 commit comments

Comments
 (0)