Skip to content

Commit

Permalink
Make migrations work for Microsoft SQL Server
Browse files Browse the repository at this point in the history
The activerecord-sqlserver-adapter gem sets IDENTITY for primary keys,
and SQL Server disallows writing values into IDENTITY columns, which
breaks most features in Rodauth that use tables where primary key is
also a foreign key to the accounts table. IDENTITY columns are part of
the SQL standard, and if the PostgreSQL adapter were to switch to
IDENTITY columns for primary keys (like Sequel does), I believe it would
have the same issue.

The activerecord-sqlserver-adapter gem internally has mechanisms for
temporarily allowing setting IDENTITY around insertion, but that's only
activated when executing SQL queries via Active Record. We could call
this API directly, but it's better to avoid IDENTITY columns altogether,
like the Sequel migrations do.

Creating the `id` column directly and passing `primary_key: true` as a
secondary option appears to solve the issue, as it prevents the SQL
Server adapter from setting the IDENTITY clause. The new migrations
appear to produce the same or similar enough SQL on other adapters, so
it should be a safe change.

See #129
  • Loading branch information
janko committed Sep 14, 2022
1 parent 31d6ffe commit bbbb94c
Show file tree
Hide file tree
Showing 16 changed files with 45 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## HEAD

* Avoid creating IDENTITY columns for primary foreign keys on SQL Server with Active Record (@janko)

* Make configuration name argument required in generated `RodauthMailer` (@janko)

* Make the Rails integration work without Action Mailer loaded (@janko)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the account expiration feature
create_table :account_activity_times<%= primary_key_type %> do |t|
create_table :account_activity_times, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.datetime :last_activity_at, null: false
t.datetime :last_login_at, null: false
Expand Down
4 changes: 2 additions & 2 deletions lib/generators/rodauth/migration/active_record/email_auth.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Used by the email auth feature
create_table :account_email_auth_keys<%= primary_key_type %> do |t|
t.foreign_key :accounts, column: :id
create_table :account_email_auth_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.string :key, null: false
t.datetime :deadline, null: false
t.datetime :email_last_sent, null: false, default: <%= current_timestamp %>
Expand Down
6 changes: 4 additions & 2 deletions lib/generators/rodauth/migration/active_record/lockout.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# Used by the lockout feature
create_table :account_login_failures<%= primary_key_type %> do |t|
create_table :account_login_failures, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.integer :number, null: false, default: 1
end
create_table :account_lockouts<%= primary_key_type %> do |t|
create_table :account_lockouts, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.datetime :deadline, null: false
Expand Down
3 changes: 2 additions & 1 deletion lib/generators/rodauth/migration/active_record/otp.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the otp feature
create_table :account_otp_keys<%= primary_key_type %> do |t|
create_table :account_otp_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.integer :num_failures, null: false, default: 0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the password expiration feature
create_table :account_password_change_times<%= primary_key_type %> do |t|
create_table :account_password_change_times, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.datetime :changed_at, null: false, default: <%= current_timestamp %>
end
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Used by the recovery codes feature
create_table :account_recovery_codes, primary_key: [:id, :code] do |t|
t.column :id, :<%= primary_key_type(nil) || :bigint %>
t.<%= primary_key_type(nil) %> :id
t.foreign_key :accounts, column: :id
t.string :code
end
3 changes: 2 additions & 1 deletion lib/generators/rodauth/migration/active_record/remember.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the remember me feature
create_table :account_remember_keys<%= primary_key_type %> do |t|
create_table :account_remember_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.datetime :deadline, null: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the password reset feature
create_table :account_password_reset_keys<%= primary_key_type %> do |t|
create_table :account_password_reset_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.datetime :deadline, null: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the single session feature
create_table :account_session_keys<%= primary_key_type %> do |t|
create_table :account_session_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
end
3 changes: 2 additions & 1 deletion lib/generators/rodauth/migration/active_record/sms_codes.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the sms codes feature
create_table :account_sms_codes<%= primary_key_type %> do |t|
create_table :account_sms_codes, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :phone_number, null: false
t.integer :num_failures
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the account verification feature
create_table :account_verification_keys<%= primary_key_type %> do |t|
create_table :account_verification_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.datetime :requested_at, null: false, default: <%= current_timestamp %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the verify login change feature
create_table :account_login_change_keys<%= primary_key_type %> do |t|
create_table :account_login_change_keys, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :key, null: false
t.string :login, null: false
Expand Down
3 changes: 2 additions & 1 deletion lib/generators/rodauth/migration/active_record/webauthn.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Used by the webauthn feature
create_table :account_webauthn_user_ids<%= primary_key_type %> do |t|
create_table :account_webauthn_user_ids, id: false do |t|
t.<%= primary_key_type(nil) %> :id, primary_key: true
t.foreign_key :accounts, column: :id
t.string :webauthn_id, null: false
end
Expand Down
14 changes: 10 additions & 4 deletions lib/generators/rodauth/migration_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,18 @@ def primary_key_type(key = :id)
generators = ::Rails.application.config.generators
column_type = generators.options[:active_record][:primary_key_type]

return unless column_type

if key
", #{key}: :#{column_type}"
", #{key}: :#{column_type}" if column_type
else
column_type || default_primary_key_type
end
end

def default_primary_key_type
if ActiveRecord.version >= Gem::Version.new("5.1") && activerecord_adapter != "sqlite3"
:bigint
else
column_type
:integer
end
end

Expand Down
14 changes: 6 additions & 8 deletions test/generators/migration_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
migration_file = "db/migrate/create_rodauth_otp_sms_codes_recovery_codes.rb"

assert_migration migration_file, /class CreateRodauthOtpSmsCodesRecoveryCodes < ActiveRecord::Migration#{migration_version}/
assert_migration migration_file, /create_table :account_otp_keys do/
assert_migration migration_file, /create_table :account_sms_codes do/
assert_migration migration_file, /create_table :account_otp_keys, id: false do/
assert_migration migration_file, /create_table :account_sms_codes, id: false do/
assert_migration migration_file, /create_table :account_recovery_codes, primary_key: \[:id, :code\] do/
assert_migration migration_file, /t\.column :id, :bigint/
assert_migration migration_file, /t\.integer :id, primary_key: true/
end

test "migration name" do
Expand All @@ -33,13 +33,11 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
g.orm :active_record, primary_key_type: :uuid
end

run_generator %w[otp sms_codes recovery_codes]
run_generator %w[otp]

migration_file = "db/migrate/create_rodauth_otp_sms_codes_recovery_codes.rb"
migration_file = "db/migrate/create_rodauth_otp.rb"

assert_migration migration_file, /create_table :account_otp_keys, id: :uuid do/
assert_migration migration_file, /create_table :account_sms_codes, id: :uuid do/
assert_migration migration_file, /t\.column :id, :uuid/
assert_migration migration_file, /t\.uuid :id, primary_key: true/

Rails.application.config.generators.options[:active_record].delete(:primary_key_type)
end
Expand Down

0 comments on commit bbbb94c

Please sign in to comment.