Skip to content

Commit dea2bd4

Browse files
committed
Use dedicated sequence record instead of deal with issues with ids and uuids when using the previous approach
1 parent 6cf8b3d commit dea2bd4

File tree

8 files changed

+91
-28
lines changed

8 files changed

+91
-28
lines changed

app/models/account.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,6 @@ def system_user
3939

4040
private
4141
def assign_external_account_id
42-
self.external_account_id ||= ExternalAccount.create!.id
42+
self.external_account_id ||= ExternalIdSequence.next
4343
end
4444
end
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Provides sequential IDs for +external_account_id+ when creating accounts without one.
2+
class Account::ExternalIdSequence < ApplicationRecord
3+
class << self
4+
def next
5+
with_lock do |sequence|
6+
sequence.increment!(:value).value
7+
end
8+
end
9+
10+
def value
11+
first&.value
12+
end
13+
14+
private
15+
def with_lock
16+
transaction do
17+
sequence = lock.first_or_create!(value: initial_value)
18+
yield sequence
19+
end
20+
end
21+
22+
def initial_value
23+
Account.maximum(:external_account_id) || 0
24+
end
25+
end
26+
end

app/models/external_account.rb

Lines changed: 0 additions & 5 deletions
This file was deleted.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
class CreateAccountExternalIdSequences < ActiveRecord::Migration[8.0]
2+
def change
3+
create_table :account_external_id_sequences do |t|
4+
t.bigint :value, null: false, default: 0
5+
6+
t.index :value, unique: true
7+
end
8+
end
9+
end

db/migrate/20251127000001_create_external_accounts.rb

Lines changed: 0 additions & 15 deletions
This file was deleted.

db/schema_sqlite.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#
1111
# It's strongly recommended that you check this file into your version control system.
1212

13-
ActiveRecord::Schema[8.2].define(version: 2025_11_25_130010) do
13+
ActiveRecord::Schema[8.2].define(version: 2025_11_27_000001) do
1414
create_table "accesses", id: :uuid, force: :cascade do |t|
1515
t.datetime "accessed_at"
1616
t.uuid "account_id", null: false
@@ -25,6 +25,11 @@
2525
t.index ["user_id"], name: "index_accesses_on_user_id"
2626
end
2727

28+
create_table "account_external_id_sequences", force: :cascade do |t|
29+
t.bigint "value", default: 0, null: false
30+
t.index ["value"], name: "index_account_external_id_sequences_on_value", unique: true
31+
end
32+
2833
create_table "account_join_codes", id: :uuid, force: :cascade do |t|
2934
t.uuid "account_id", null: false
3035
t.string "code", limit: 255, null: false
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
require "test_helper"
2+
3+
class Account::ExternalIdSequenceTest < ActiveSupport::TestCase
4+
setup do
5+
Account::ExternalIdSequence.delete_all
6+
end
7+
8+
test ".next returns sequential values" do
9+
first_value = Account::ExternalIdSequence.next
10+
second_value = Account::ExternalIdSequence.next
11+
third_value = Account::ExternalIdSequence.next
12+
13+
assert_equal first_value + 1, second_value
14+
assert_equal second_value + 1, third_value
15+
end
16+
17+
test ".next initializes from maximum external_account_id" do
18+
max_id = Account.maximum(:external_account_id) || 0
19+
20+
first_value = Account::ExternalIdSequence.next
21+
22+
assert_equal max_id + 1, first_value
23+
end
24+
25+
test ".next creates single sequence record" do
26+
3.times { Account::ExternalIdSequence.next }
27+
28+
assert_equal 1, Account::ExternalIdSequence.count
29+
end
30+
31+
test ".next is concurrency-safe" do
32+
values = 20.times.map do
33+
Thread.new do
34+
Account::ExternalIdSequence.next
35+
end
36+
end.map(&:value)
37+
38+
assert_equal 20, values.uniq.size, "All values should be unique"
39+
assert_equal values.min..values.max, values.sort.first..values.sort.last
40+
assert_equal 20, values.max - values.min + 1, "Values should be sequential with no gaps"
41+
end
42+
end

test/models/account_test.rb

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,13 @@ class AccountTest < ActiveSupport::TestCase
6969
assert_equal account1.external_account_id + 1, account2.external_account_id
7070
end
7171

72-
test "external_account_id can be overridden without creating sequence entry" do
73-
custom_id = Account.last.id + 99999
72+
test "external_account_id can be overridden" do
73+
custom_id = 999999
74+
sequence_value_before = Account::ExternalIdSequence.first_or_create!(value: 0).value
7475

75-
assert_no_difference -> { ExternalAccount.count } do
76-
account = Account.create!(name: "Custom ID Account", external_account_id: custom_id)
77-
assert_equal custom_id, account.external_account_id
78-
end
76+
account = Account.create!(name: "Custom ID Account", external_account_id: custom_id)
77+
78+
assert_equal custom_id, account.external_account_id
79+
assert_equal sequence_value_before, Account::ExternalIdSequence.value
7980
end
8081
end

0 commit comments

Comments
 (0)