diff --git a/lib/chrono_model/adapter.rb b/lib/chrono_model/adapter.rb index afdfee69..173cd5db 100644 --- a/lib/chrono_model/adapter.rb +++ b/lib/chrono_model/adapter.rb @@ -41,6 +41,20 @@ def initialize(*) end end + class TableDefinition < ActiveRecord::ConnectionAdapters::PostgreSQL::TableDefinition + def foreign_key(to_table, **options) + if !@conn.is_schema_qualified?(to_table) && (@conn.current_schema == TEMPORAL_SCHEMA) != @conn.is_chrono?(to_table) + to_table = @conn.qualified_table_name(to_table) + end + + super + end + end + + def create_table_definition(name, **options) + ChronoModel::Adapter::TableDefinition.new(self, name, **options) + end + # Returns true whether the connection adapter supports our # implementation of temporal tables. Currently, Chronomodel # is supported starting with PostgreSQL 9.3 (90300 in PostgreSQL's @@ -112,13 +126,18 @@ def on_history_schema(&block) # Evaluates the given block in the given +schema+ search path. # - # Recursion works by saving the old_path the function closure + # Recursion works by saving the old_path in the method closure # at each recursive call. # + # @original_schema is set to the current schema on the first call, + # and restored to +nil+ when the recursion ends. Use outer_schema() + # to get the original schema. + # # See specs for examples and behaviour. # def on_schema(schema, recurse: :follow) old_path = schema_search_path + @original_schema ||= current_schema count_recursions do if (recurse == :follow) || (Thread.current['recursions'] == 1) @@ -144,6 +163,29 @@ def on_schema(schema, recurse: :follow) else self.schema_search_path = old_path end + + @original_schema = nil if Thread.current['recursions'].zero? + end + + # The current schema or the schema that was the current schema before + # on_schema() was called. + # + def outer_schema + @original_schema || current_schema + end + + def foreign_key_column_for(table_name, *args) # :nodoc: + _schema, table_name = extract_schema_qualified_name(table_name) + super + end + + def qualified_table_name(table_name) + schema = is_chrono?(table_name) ? TEMPORAL_SCHEMA : outer_schema + "#{schema}.#{table_name}" + end + + def is_schema_qualified?(name) + name.to_s.include?('.') end # Returns true if the given name references a temporal table. diff --git a/lib/chrono_model/adapter/migrations_modules/stable.rb b/lib/chrono_model/adapter/migrations_modules/stable.rb index 7bb6a80f..7d423036 100644 --- a/lib/chrono_model/adapter/migrations_modules/stable.rb +++ b/lib/chrono_model/adapter/migrations_modules/stable.rb @@ -33,6 +33,26 @@ def remove_index(table_name, column_name = nil, **options) on_history_schema { super } end end + + def add_foreign_key(from_table, to_table, **options) + if !is_schema_qualified?(to_table) && is_chrono?(to_table) != is_chrono?(from_table) + to_table = qualified_table_name(to_table) + end + + return super unless is_chrono?(from_table) + + on_temporal_schema { super } + end + + def remove_foreign_key(from_table, to_table = nil, **options) + if !is_schema_qualified?(to_table) && is_chrono?(to_table) != is_chrono?(from_table) + to_table = qualified_table_name(to_table) + end + + return super unless is_chrono?(from_table) + + on_temporal_schema { super } + end end end end diff --git a/spec/chrono_model/adapter/base_spec.rb b/spec/chrono_model/adapter/base_spec.rb index 7a9b6409..7296677d 100644 --- a/spec/chrono_model/adapter/base_spec.rb +++ b/spec/chrono_model/adapter/base_spec.rb @@ -108,6 +108,24 @@ end end + describe '.outer_schema' do + subject(:outer_schema) { adapter.outer_schema } + + context 'when outside of on_schema()' do + it { is_expected.to eq(adapter.current_schema) } + end + + context 'when inside on_schema()' do + it 'returns the original schema' do + original_schema = adapter.current_schema + + adapter.on_schema('test_schema') do + expect(outer_schema).to eq(original_schema) + end + end + end + end + describe '.is_chrono?' do subject(:is_chrono?) { adapter.is_chrono?(table) } diff --git a/spec/chrono_model/adapter/migrations_spec.rb b/spec/chrono_model/adapter/migrations_spec.rb index 42d10b8d..3189be2a 100644 --- a/spec/chrono_model/adapter/migrations_spec.rb +++ b/spec/chrono_model/adapter/migrations_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' require 'support/adapter/structure' +require 'support/time_machine/structure' # For the structure of these tables, please see spec/support/adabters/structure.rb. @@ -40,13 +41,86 @@ describe '.create_table' do context 'with temporal tables' do include_context 'with temporal tables' + it_behaves_like 'temporal table' + + context 'when adding references to temporal tables' do + let(:columns) do + native = [%w[bar_id bigint]] + + def native.to_proc + proc { |t| t.references :bar, foreign_key: true } + end + + native + end + + it_behaves_like 'temporal table' + + it 'creates a foreign key constraint to the temporal schema' do + expect(table).to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'when adding references to plain tables' do + let(:columns) do + native = [%w[baz_id bigint]] + + def native.to_proc + proc { |t| t.references :baz, foreign_key: true } + end + + native + end + + it_behaves_like 'temporal table' + + it 'creates a foreign key constraint to the public schema' do + expect(table).to have_temporal_foreign_key_constraint('bazs', 'public') + end + end end context 'with plain tables' do include_context 'with plain tables' it_behaves_like 'plain table' + + context 'when adding references to temporal tables' do + let(:columns) do + native = [%w[bar_id bigint]] + + def native.to_proc + proc { |t| t.references :bar, foreign_key: true } + end + + native + end + + it_behaves_like 'plain table' + + it 'creates a foreign key constraint to the temporal schema' do + expect(table).to have_foreign_key_constraint('bars', 'temporal') + end + end + + context 'when adding references to plain tables' do + let(:columns) do + native = [%w[baz_id bigint]] + + def native.to_proc + proc { |t| t.references :baz, foreign_key: true } + end + + native + end + + it_behaves_like 'plain table' + + it 'creates a foreign key constraint to the public schema' do + expect(table).to have_foreign_key_constraint('bazs', 'public') + end + end end end @@ -114,6 +188,34 @@ it { is_expected.to have_temporal_columns([%w[new_column integer]]) } it { is_expected.to have_history_columns([%w[new_column integer]]) } end + + context 'when adding references to temporal tables' do + before do + adapter.change_table table do |t| + t.references :bar, foreign_key: true + end + end + + it_behaves_like 'temporal table' + + it 'creates a foreign key constraint to the temporal schema' do + expect(table).to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'when adding references to plain tables' do + before do + adapter.change_table table do |t| + t.references :baz, foreign_key: true + end + end + + it_behaves_like 'temporal table' + + it 'creates a foreign key constraint to the public schema' do + expect(table).to have_temporal_foreign_key_constraint('bazs', 'public') + end + end end context 'with plain tables' do @@ -174,6 +276,34 @@ expect(indexes - history_indexes.map(&:name).sort).to be_empty end end + + context 'when when adding references to temporal tables' do + before do + adapter.change_table table do |t| + t.references :bar, foreign_key: true + end + end + + it_behaves_like 'plain table' + + it 'creates a foreign key constraint to the temporal schema' do + expect(table).to have_foreign_key_constraint('bars', 'temporal') + end + end + + context 'when when adding references to plain tables' do + before do + adapter.change_table table do |t| + t.references :baz, foreign_key: true + end + end + + it_behaves_like 'plain table' + + it 'creates a foreign key constraint to the public schema' do + expect(table).to have_foreign_key_constraint('bazs', 'public') + end + end end # https://github.com/ifad/chronomodel/issues/91 @@ -491,4 +621,160 @@ it { is_expected.to have_columns([['foo', 'double precision']]) } end end + + describe '.add_foreign_key' do + context 'with temporal tables' do + include_context 'with temporal tables' + + it 'adds foreign key constraints on plain tables' do + adapter.add_reference table, :baz, foreign_key: false + adapter.add_foreign_key table, :bazs + + expect(table).to have_temporal_foreign_key_constraint('bazs', 'public') + end + + it 'adds foreign key constraints on other temporal tables' do + adapter.add_reference table, :bar, foreign_key: false + adapter.add_foreign_key table, :bars + + expect(table).to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'with plain tables' do + include_context 'with plain tables' + + it 'adds foreign key constraints on other plain tables' do + adapter.add_reference table, :baz, foreign_key: false + adapter.add_foreign_key table, :bazs + + expect(table).to have_foreign_key_constraint('bazs', 'public') + end + + it 'adds foreign key constraints on temporal tables' do + adapter.add_reference table, :bar, foreign_key: false + adapter.add_foreign_key table, :bars + + expect(table).to have_foreign_key_constraint('bars', 'temporal') + end + end + end + + describe '.remove_foreign_key' do + context 'with temporal tables' do + include_context 'with temporal tables' + + it 'removes foreign key constraints on plain tables' do + adapter.add_reference table, :baz, foreign_key: true + adapter.remove_foreign_key table, :bazs + + expect(table).not_to have_temporal_foreign_key_constraint('bazs', 'public') + end + + it 'removes foreign key constraints on other temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + adapter.remove_foreign_key table, :bars + + expect(table).not_to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'with plain tables' do + include_context 'with plain tables' + + it 'removes foreign key constraint on other plain tables' do + adapter.add_reference table, :baz, foreign_key: true + adapter.remove_foreign_key table, :bazs + + expect(table).not_to have_foreign_key_constraint('bazs', 'public') + end + + it 'removes foreign key constraints on temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + adapter.remove_foreign_key table, :bars + + expect(table).not_to have_foreign_key_constraint('bars', 'temporal') + end + end + end + + describe '.add_reference' do + context 'with temporal tables' do + include_context 'with temporal tables' + + it 'adds a foreign key column and constraint to plain tables' do + adapter.add_reference table, :baz, foreign_key: true + + expect(table).to have_columns([%w[baz_id bigint]]) + expect(table).to have_temporal_foreign_key_constraint('bazs', 'public') + end + + it 'adds a foreign key column and constraint to other temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + + expect(table).to have_columns([%w[bar_id bigint]]) + expect(table).to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'with plain tables' do + include_context 'with plain tables' + + it 'adds a foreign key column and constraint to other plain tables' do + adapter.add_reference table, :baz, foreign_key: true + + expect(table).to have_columns([%w[baz_id bigint]]) + expect(table).to have_foreign_key_constraint('bazs', 'public') + end + + it 'adds a foreign key column and constraint to temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + + expect(table).to have_columns([%w[bar_id bigint]]) + expect(table).to have_foreign_key_constraint('bars', 'temporal') + end + end + end + + describe '.remove_reference' do + context 'with temporal tables' do + include_context 'with temporal tables' + + it 'removes the foreign key column and constraint from plain tables' do + adapter.add_reference table, :baz, foreign_key: true + adapter.remove_reference table, :baz + + expect(table).not_to have_columns([%w[baz_id bigint]]) + expect(table).not_to have_temporal_foreign_key_constraint('bazs', 'public') + end + + it 'removes the foreign key column and constraint from other temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + adapter.remove_reference table, :bar + + expect(table).not_to have_columns([%w[bar_id bigint]]) + expect(table).not_to have_temporal_foreign_key_constraint('bars', 'temporal') + end + end + + context 'with plain tables' do + include_context 'with plain tables' + + it 'removes the foreign key column and constraint from other plain tables' do + adapter.add_reference table, :baz, foreign_key: true + adapter.remove_reference table, :baz + + expect(table).not_to have_columns([%w[baz_id bigint]]) + expect(table).not_to have_foreign_key_constraint('bazs', 'public') + end + + it 'removes the foreign key column and constraint from temporal tables' do + adapter.add_reference table, :bar, foreign_key: true + adapter.remove_reference table, :bar + + expect(table).not_to have_columns([%w[bar_id bigint]]) + expect(table).not_to have_foreign_key_constraint('bars', 'temporal') + end + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5dd06ada..624b7e70 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,7 @@ require_relative 'support/matchers/column' require_relative 'support/matchers/function' require_relative 'support/matchers/index' +require_relative 'support/matchers/foreign_key_constraint' require_relative 'support/matchers/schema' require_relative 'support/matchers/source' require_relative 'support/matchers/table' @@ -40,6 +41,7 @@ config.include(ChronoTest::Matchers::Column) config.include(ChronoTest::Matchers::Function) config.include(ChronoTest::Matchers::Index) + config.include(ChronoTest::Matchers::ForeignKeyConstraint) config.include(ChronoTest::Matchers::Schema) config.include(ChronoTest::Matchers::Source) config.include(ChronoTest::Matchers::Table) diff --git a/spec/support/matchers/foreign_key_constraint.rb b/spec/support/matchers/foreign_key_constraint.rb new file mode 100644 index 00000000..93b2a329 --- /dev/null +++ b/spec/support/matchers/foreign_key_constraint.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require_relative 'base' + +module ChronoTest + module Matchers + module ForeignKeyConstraint + class HaveForeignKeyConstraint < ChronoTest::Matchers::Base + attr_reader :ref_table, :ref_schema, :schema + + def initialize(ref_table, ref_schema, schema = 'public') + @ref_table = ref_table + @ref_schema = ref_schema + @schema = schema + end + + def description + 'have foreign key constraint' + end + + def matches?(table) + super + + select_values(<<~SQL.squish, [table, schema, ref_table, ref_schema], 'Check foreign key constraint').any? + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON c.conrelid = t.oid + JOIN pg_class r ON c.confrelid = r.oid + JOIN pg_namespace tn ON t.relnamespace = tn.oid + JOIN pg_namespace rn ON r.relnamespace = rn.oid + WHERE t.relname = ? + AND tn.nspname = ? + AND r.relname = ? + AND rn.nspname = ? + AND c.contype = 'f'; + SQL + end + + def failure_message + "expected #{schema}.#{table} to have a foreign key constraint on #{ref_schema}.#{ref_table}" + end + + def failure_message_when_negated + "expected #{schema}.#{table} to not have a foreign key constraint on #{ref_schema}.#{ref_table}" + end + end + + def have_foreign_key_constraint(*args) + HaveForeignKeyConstraint.new(*args) + end + + class HaveTemporalForeignKeyConstraint < HaveForeignKeyConstraint + def initialize(ref_table, ref_schema) + super(ref_table, ref_schema, temporal_schema) + end + end + + def have_temporal_foreign_key_constraint(*args) + HaveTemporalForeignKeyConstraint.new(*args) + end + end + end +end