diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c412a2..a417909 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased - xxxx-xx-xx +- Fix `validates_db_uniqueness_of` raising `NoMethodError` on PostgreSQL when the table has row-level security. PostgreSQL omits the `Key (...)=(...)` detail from the unique violation there, so the error is now matched by its index name only ## [2.1.1] - 2026-03-31 - `lint_roller` is removed as dependency. It is not needed diff --git a/lib/database_validations/lib/adapters/postgresql_adapter.rb b/lib/database_validations/lib/adapters/postgresql_adapter.rb index f1fe802..0ff4fdc 100644 --- a/lib/database_validations/lib/adapters/postgresql_adapter.rb +++ b/lib/database_validations/lib/adapters/postgresql_adapter.rb @@ -8,8 +8,11 @@ def unique_index_name(error) error.message[/unique constraint "([^"]+)"/, 1] end + # PostgreSQL leaves the `DETAIL: Key (...)=(...) already exists.` line out of a unique violation on a + # table the current role is subject to row-level security on, because the values could belong to a row the + # role cannot see. The columns are then unknown (nil) and the error is matched by its index name instead. def unique_error_columns(error) - error.message[/Key \((.+)\)=/, 1].split(', ') + error.message[/Key \((.+)\)=/, 1]&.split(', ') end def foreign_key_error_column(error) diff --git a/spec/adapters/postgresql_unique_error_columns_spec.rb b/spec/adapters/postgresql_unique_error_columns_spec.rb new file mode 100644 index 0000000..3e0f5e5 --- /dev/null +++ b/spec/adapters/postgresql_unique_error_columns_spec.rb @@ -0,0 +1,25 @@ +RSpec.describe 'Adapter.unique_error_columns' do + def build_error(message) + ActiveRecord::RecordNotUnique.new(message) + end + + describe DatabaseValidations::Adapters::PostgresqlAdapter do + it 'extracts the columns from the error detail' do + error = build_error( + 'PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_entities_on_a_and_b" ' \ + 'DETAIL: Key (a, b)=(1, x) already exists.' + ) + + expect(described_class.unique_error_columns(error)).to eq(%w[a b]) + end + + it 'returns nil when the error has no detail' do + error = build_error( + 'PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_entities_on_a_and_b"' + ) + + expect(described_class.unique_error_columns(error)).to be_nil + expect(described_class.unique_index_name(error)).to eq('index_entities_on_a_and_b') + end + end +end diff --git a/spec/validations/validates_db_uniqueness_of_row_level_security_spec.rb b/spec/validations/validates_db_uniqueness_of_row_level_security_spec.rb new file mode 100644 index 0000000..5069493 --- /dev/null +++ b/spec/validations/validates_db_uniqueness_of_row_level_security_spec.rb @@ -0,0 +1,98 @@ +RSpec.describe '.validates_db_uniqueness_of with PostgreSQL row-level security' do + # End to end for a unique violation that carries no `DETAIL` line: see `PostgresqlAdapter.unique_error_columns`. + let(:role) { 'database_validations_rls_probe' } + let(:connection) { ActiveRecord::Base.connection } + + before do + define_database(postgresql_configuration) + + can_create_roles = connection.select_value( + 'SELECT rolsuper OR rolcreaterole FROM pg_roles WHERE rolname = current_user' + ) + skip 'needs a role that can create roles' unless can_create_roles + + connection.execute("DROP ROLE IF EXISTS #{role}") + connection.execute("CREATE ROLE #{role}") + + define_table(:rls_entities) do |t| + t.string :name + t.index :name, unique: true, name: 'index_rls_entities_on_name' + end + define_table(:rls_scoped_entities) do |t| + t.string :name + t.string :scope_key + t.index %i[scope_key name], unique: true, name: 'index_rls_scoped_entities_on_scope_key_and_name' + end + + %w[rls_entities rls_scoped_entities].each do |table| + connection.execute("GRANT SELECT, INSERT, UPDATE, DELETE ON #{table} TO #{role}") + connection.execute("GRANT USAGE, SELECT ON SEQUENCE #{table}_id_seq TO #{role}") + connection.execute("ALTER TABLE #{table} ENABLE ROW LEVEL SECURITY") + connection.execute("CREATE POLICY allow_all ON #{table} USING (true) WITH CHECK (true)") + end + connection.execute("SET ROLE #{role}") + end + + after do + connection.execute('RESET ROLE') + connection.execute("DROP OWNED BY #{role}") + connection.execute("DROP ROLE #{role}") + end + + it 'is run under a role that row-level security applies to (the premise of the other examples)' do + connection.execute("INSERT INTO rls_entities (name) VALUES ('taken')") + + error = begin + connection.execute("INSERT INTO rls_entities (name) VALUES ('taken')") + rescue ActiveRecord::RecordNotUnique => e + e + end + + expect(error.message).to include('unique constraint "index_rls_entities_on_name"') + expect(error.message).not_to match(/Key \(/) + end + + shared_examples 'turns the collision into a validation error' do + it 'adds the error to the record instead of raising' do + klass.create!(attrs) + + record = klass.create(attrs) + + expect(record.errors.details).to eq(error_details) + end + + it 'raises RecordInvalid from the bang methods' do + klass.create!(attrs) + + expect { klass.create!(attrs) }.to raise_error(ActiveRecord::RecordInvalid) + end + end + + context 'when the unique index is found by the attribute' do + let(:klass) { define_class(ActiveRecord::Base, :rls_entities) { validates_db_uniqueness_of :name } } + let(:attrs) { { name: 'taken' } } + let(:error_details) { { name: [{ error: :taken, value: 'taken' }] } } + + include_examples 'turns the collision into a validation error' + end + + context 'when the unique index is found by the attribute and its scope' do + let(:klass) { define_class(ActiveRecord::Base, :rls_scoped_entities) { validates_db_uniqueness_of :name, scope: :scope_key } } + let(:attrs) { { name: 'taken', scope_key: 'one' } } + let(:error_details) { { name: [{ error: :taken, value: 'taken' }] } } + + include_examples 'turns the collision into a validation error' + end + + context 'when the unique index is named' do + let(:klass) do + define_class(ActiveRecord::Base, :rls_entities) do + validates_db_uniqueness_of :name, index_name: :index_rls_entities_on_name + end + end + let(:attrs) { { name: 'taken' } } + let(:error_details) { { name: [{ error: :taken, value: 'taken' }] } } + + include_examples 'turns the collision into a validation error' + end +end