diff --git a/spec/schema_auto_foreign_keys/foreign_key_spec.rb b/spec/schema_auto_foreign_keys/foreign_key_spec.rb new file mode 100644 index 0000000..99aa2be --- /dev/null +++ b/spec/schema_auto_foreign_keys/foreign_key_spec.rb @@ -0,0 +1,206 @@ +require 'spec_helper' + +describe "Foreign Key" do + + let(:migration) { ::ActiveRecord::Migration } + + context "created with table" do + before(:all) do + define_schema(:auto_create => true) do + create_table :users, :force => true do |t| + t.string :login + end + create_table :comments, :force => true do |t| + t.integer :user_id + t.foreign_key :user_id, :users + end + end + class User < ::ActiveRecord::Base ; end + class Comment < ::ActiveRecord::Base ; end + end + + it "should report foreign key constraints" do + expect(Comment.foreign_keys.collect(&:column).flatten).to eq([ "user_id" ]) + end + + it "should report reverse foreign key constraints" do + expect(User.reverse_foreign_keys.collect(&:column).flatten).to eq([ "user_id" ]) + end + + end + + context "modification" do + + before(:each) do + define_schema(:auto_create => false) do + create_table :users, :force => true do |t| + t.string :login + t.datetime :deleted_at + end + + create_table :posts, :force => true do |t| + t.text :body + t.integer :user_id + t.integer :author_id + end + + create_table :comments, :force => true do |t| + t.text :body + t.integer :post_id + t.foreign_key :post_id, :posts + end + end + class User < ::ActiveRecord::Base ; end + class Post < ::ActiveRecord::Base ; end + class Comment < ::ActiveRecord::Base ; end + Comment.reset_column_information + end + + + context "works", :sqlite3 => :skip do + + context "when is added", "posts(author_id)" do + + before(:each) do + add_foreign_key(:posts, :users, :column => :author_id, :on_update => :cascade, :on_delete => :restrict) + end + + it "references users(id)" do + expect(Post).to reference(:users, :id).on(:author_id) + end + + it "cascades on update" do + expect(Post).to reference(:users).on_update(:cascade) + end + + it "restricts on delete" do + expect(Post).to reference(:users).on_delete(:restrict) + end + + it "is available in Post.foreign_keys" do + expect(Post.foreign_keys.collect(&:column)).to include('author_id') + end + + it "is available in User.reverse_foreign_keys" do + expect(User.reverse_foreign_keys.collect(&:column)).to include('author_id') + end + + end + + context "when is dropped", "comments(post_id)" do + + let(:foreign_key_name) { fk = Comment.foreign_keys.detect(&its.column == 'post_id') and fk.name } + + before(:each) do + remove_foreign_key(:comments, name: foreign_key_name) + end + + it "doesn't reference posts(id)" do + expect(Comment).not_to reference(:posts).on(:post_id) + end + + it "is no longer available in Post.foreign_keys" do + expect(Comment.foreign_keys.collect(&:column)).not_to include('post_id') + end + + it "is no longer available in User.reverse_foreign_keys" do + expect(Post.reverse_foreign_keys.collect(&:column)).not_to include('post_id') + end + + end + + context "when drop using hash", "comments(post_id)" do + + let(:foreign_key_name) { fk = Comment.foreign_keys.detect(&its.column == 'post_id') and fk.name } + + it "finds by name" do + remove_foreign_key(:comments, name: foreign_key_name) + expect(Comment).not_to reference(:posts).on(:post_id) + end + + it "finds by column_names" do + remove_foreign_key(:comments, column: "post_id", to_table: "posts") + expect(Comment).not_to reference(:posts).on(:post_id) + end + end + + context "when attempt to drop nonexistent foreign key" do + it "raises error" do + expect{remove_foreign_key(:comments, "posts", column: "nonesuch")}.to raise_error(/no foreign key/i) + end + + it "does not error with :if_exists" do + expect{remove_foreign_key(:comments, "posts", column: "nonesuch", :if_exists => true)}.to_not raise_error + end + end + + context "when referencing column and column is removed" do + + let(:foreign_key_name) { Comment.foreign_keys.detect { |definition| definition.column == 'post_id' }.name } + + it "should remove foreign keys" do + remove_foreign_key(:comments, name: foreign_key_name) + expect(Post.reverse_foreign_keys.collect { |fk| fk.column == 'post_id' && fk.from_table == "comments" }).to be_empty + end + + end + + context "when table name is a reserved word" do + before(:each) do + migration.suppress_messages do + migration.create_table :references, :force => true do |t| + t.integer :post_id, :foreign_key => false + end + end + end + + it "can add, detect, and remove a foreign key without error" do + migration.suppress_messages do + expect { + migration.add_foreign_key(:references, :posts) + foreign_key = migration.foreign_keys(:references).detect{|definition| definition.column == "post_id"} + migration.remove_foreign_key(:references, name: foreign_key.name) + }.to_not raise_error + end + end + end + + end + + context "raises an exception", :sqlite3 => :only do + + it "when attempting to add" do + expect { + add_foreign_key(:posts, :users, :column => :author_id, :on_update => :cascade, :on_delete => :restrict) + }.to raise_error(NotImplementedError) + end + + it "when attempting to remove" do + expect { + remove_foreign_key(:posts, name: "dummy") + }.to raise_error(NotImplementedError) + end + + end + end + + protected + def add_foreign_key(*args) + migration.suppress_messages do + migration.add_foreign_key(*args) + end + User.reset_column_information + Post.reset_column_information + Comment.reset_column_information + end + + def remove_foreign_key(*args) + migration.suppress_messages do + migration.remove_foreign_key(*args) + end + User.reset_column_information + Post.reset_column_information + Comment.reset_column_information + end + +end diff --git a/spec/schema_auto_foreign_keys/migration_spec.rb b/spec/schema_auto_foreign_keys/migration_spec.rb new file mode 100644 index 0000000..6d8b68e --- /dev/null +++ b/spec/schema_auto_foreign_keys/migration_spec.rb @@ -0,0 +1,778 @@ +# encoding: utf-8 +require 'spec_helper' + +describe ActiveRecord::Migration do + + before(:each) do + define_schema(:auto_create => true) do + + create_table :users, :force => true do |t| + t.string :login, :index => { :unique => true } + end + + create_table :members, :force => true do |t| + t.string :login + end + + create_table :comments, :force => true do |t| + t.string :content + t.integer :user + t.integer :user_id + t.foreign_key :user_id, :users, :primary_key => :id + end + + create_table :posts, :force => true do |t| + t.string :content + end + end + class User < ::ActiveRecord::Base ; end + class Post < ::ActiveRecord::Base ; end + class Comment < ::ActiveRecord::Base ; end + end + + around(:each) do |example| + with_fk_config(:auto_create => true, :auto_index => true) { example.run } + end + + context "when table is created" do + + before(:each) do + @model = Post + end + + it "should create auto foreign keys" do + recreate_table(@model) do |t| + t.integer :user_id + end + expect(@model).to reference(:users, :id).on(:user_id) + end + + it "should create explicit foreign key with default reference" do + recreate_table(@model) do |t| + t.integer :user, :foreign_key => true + end + expect(@model).to reference(:users, :id).on(:user) + end + + it "should create foreign key with different reference" do + recreate_table(@model) do |t| + t.integer :author_id, :foreign_key => { :references => :users } + end + expect(@model).to reference(:users, :id).on(:author_id) + end + + it "should create foreign key without modifying input hash" do + hash = { :references => :users } + hash_original = hash.dup + recreate_table(@model) do |t| + t.integer :author_id, :foreign_key => hash + end + expect(hash).to eq(hash_original) + end + + it "should create foreign key without modifying input hash" do + hash = { :references => :users } + hash_original = hash.dup + recreate_table(@model) do |t| + t.references :author, :foreign_key => hash + end + expect(hash).to eq(hash_original) + end + + it "should create foreign key with different reference using shortcut" do + recreate_table(@model) do |t| + t.integer :author_id, :references => :users + end + expect(@model).to reference(:users, :id).on(:author_id) + end + + it "should create foreign key with default name" do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => true + end + expect(@model).to reference(:users, :id).with_name("fk_#{@model.table_name}_user_id") + end + + it "should create foreign key with specified name" do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :name => "wugga" } + end + expect(@model).to reference(:users, :id).with_name("wugga") + end + + it "should allow multiple foreign keys to be made" do + recreate_table(@model) do |t| + t.integer :user_id, :references => :users + t.integer :updater_id, :references => :users + end + expect(@model).to reference(:users, :id).on(:user_id) + expect(@model).to reference(:users, :id).on(:updater_id) + end + + it "should suppress foreign key" do + recreate_table(@model) do |t| + t.integer :member_id, :foreign_key => false + end + expect(@model).not_to reference.on(:member_id) + end + + it "should suppress foreign key using shortcut" do + recreate_table(@model) do |t| + t.integer :member_id, :references => nil + end + expect(@model).not_to reference.on(:member_id) + end + + it "should create foreign key using t.belongs_to" do + recreate_table(@model) do |t| + t.belongs_to :user + end + expect(@model).to reference(:users, :id).on(:user_id) + end + + it "should not create foreign key using t.belongs_to with :polymorphic => true" do + recreate_table(@model) do |t| + t.belongs_to :user, :polymorphic => true + end + expect(@model).not_to reference(:users, :id).on(:user_id) + end + + it "should create foreign key using t.references" do + recreate_table(@model) do |t| + t.references :user + end + expect(@model).to reference(:users, :id).on(:user_id) + end + + it "should not create foreign key using t.references with :foreign_key => false" do + recreate_table(@model) do |t| + t.references :user, :foreign_key => false + end + expect(@model).not_to reference(:users, :id).on(:user_id) + end + + it "should not create foreign key using t.references with :polymorphic => true" do + recreate_table(@model) do |t| + t.references :user, :polymorphic => true + end + expect(@model).not_to reference(:users, :id).on(:user_id) + end + + it "should create foreign key to the same table on parent_id" do + recreate_table(@model) do |t| + t.integer :parent_id + end + expect(@model).to reference(@model.table_name, :id).on(:parent_id) + end + + [:references, :belongs_to].each do |reftype| + + context "when define #{reftype}" do + + before(:each) do + @model = Comment + end + + it "should create foreign key" do + create_reference(reftype, :post) + expect(@model).to reference(:posts, :id).on(:post_id) + end + + it "should not create a foreign_key if polymorphic" do + create_reference(reftype, :post, :polymorphic => true) + expect(@model).not_to reference(:posts, :id).on(:post_id) + end + + it "should create an index implicitly" do + create_reference(reftype, :post) + expect(@model).to have_index.on(:post_id) + end + + it "should create exactly one index explicitly (#157)" do + create_reference(reftype, :post, :index => true) + expect(@model).to have_index.on(:post_id) + end + + it "should respect :unique (#157)" do + create_reference(reftype, :post, :index => :unique) + expect(@model).to have_unique_index.on(:post_id) + end + + it "should create a two-column index if polymophic and index requested" do + create_reference(reftype, :post, :polymorphic => true, :index => true) + expect(@model).to have_index.on([:post_id, :post_type]) + end + + protected + + def create_reference(reftype, column_name, *args) + recreate_table(@model) do |t| + t.send reftype, column_name, *args + end + end + + end + end + + it "should auto-index foreign keys only" do + recreate_table(@model) do |t| + t.integer :user_id + t.integer :application_id, :references => nil + t.integer :state + end + expect(@model).to have_index.on(:user_id) + expect(@model).not_to have_index.on(:application_id) + expect(@model).not_to have_index.on(:state) + end + + it "should override foreign key auto_create positively" do + with_fk_config(:auto_create => false) do + recreate_table @model, :foreign_keys => {:auto_create => true} do |t| + t.integer :user_id + end + expect(@model).to reference(:users, :id).on(:user_id) + end + end + + it "should override foreign key auto_create negatively" do + with_fk_config(:auto_create => true) do + recreate_table @model, :foreign_keys => {:auto_create => false} do |t| + t.integer :user_id + end + expect(@model).not_to reference.on(:user_id) + end + end + + it "should override foreign key auto_index positively" do + with_fk_config(:auto_index => false) do + recreate_table @model, :foreign_keys => {:auto_index => true} do |t| + t.integer :user_id + end + expect(@model).to have_index.on(:user_id) + end + end + + actions = [:cascade, :restrict, :nullify, :set_default, :no_action] + + actions.each do |action| + if action == :set_default + if_action_supported = { :mysql => :skip } + if_action_unsupported = { :mysql => :only } + else + if_action_supported = { :if => true } + if_action_unsupported = { :if => false } + end + + it "should create and detect on_update #{action.inspect}", if_action_supported do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_update => action } + end + expect(@model).to reference.on(:user_id).on_update(action) + end + + it "should create and detect on_update #{action.inspect} using shortcut", if_action_supported do + recreate_table @model do |t| + t.integer :user_id, :on_update => action + end + expect(@model).to reference.on(:user_id).on_update(action) + end + + it "should raise a not-implemented error for on_update => #{action}", if_action_unsupported do + expect { + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_update => action } + end + }.to raise_error(NotImplementedError) + end + + it "should create and detect on_delete #{action.inspect}", if_action_supported do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_delete => action } + end + expect(@model).to reference.on(:user_id).on_delete(action) + end + + it "should create and detect on_delete #{action.inspect} using shortcut", if_action_supported do + recreate_table @model do |t| + t.integer :user_id, :on_delete => action + end + expect(@model).to reference.on(:user_id).on_delete(action) + end + + it "should raise a not-implemented error for on_delete => #{action}", if_action_unsupported do + expect { + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_delete => action } + end + }.to raise_error(NotImplementedError) + end + + end + + [false, true, :initially_deferred].each do |status| + it "should create and detect deferrable #{status.inspect}", :mysql => :skip do + recreate_table @model do |t| + t.integer :user_id, :on_delete => :cascade, :deferrable => status + end + expect(@model).to reference.on(:user_id).deferrable(status) + end + end + + it "should use default on_delete action" do + with_fk_config(:on_delete => :cascade) do + recreate_table @model do |t| + t.integer :user_id + end + expect(@model).to reference.on(:user_id).on_delete(:cascade) + end + end + + it "should override on_update action per table" do + with_fk_config(:on_update => :cascade) do + recreate_table @model, :foreign_keys => {:on_update => :restrict} do |t| + t.integer :user_id + end + expect(@model).to reference.on(:user_id).on_update(:restrict) + end + end + + it "should override on_delete action per table" do + with_fk_config(:on_delete => :cascade) do + recreate_table @model, :foreign_keys => {:on_delete => :restrict} do |t| + t.integer :user_id + end + expect(@model).to reference.on(:user_id).on_delete(:restrict) + end + end + + it "should override on_update action per column" do + with_fk_config(:on_update => :cascade) do + recreate_table @model, :foreign_keys => {:on_update => :restrict} do |t| + t.integer :user_id, :foreign_key => { :on_update => :nullify } + end + expect(@model).to reference.on(:user_id).on_update(:nullify) + end + end + + it "should override on_delete action per column" do + with_fk_config(:on_delete => :cascade) do + recreate_table @model, :foreign_keys => {:on_delete => :restrict} do |t| + t.integer :user_id, :foreign_key => { :on_delete => :nullify } + end + expect(@model).to reference.on(:user_id).on_delete(:nullify) + end + end + + it "should raise an error for an invalid on_update action" do + expect { + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_update => :invalid } + end + }.to raise_error(ArgumentError) + end + + it "should raise an error for an invalid on_delete action" do + expect { + recreate_table @model do |t| + t.integer :user_id, :foreign_key => { :on_delete => :invalid } + end + }.to raise_error(ArgumentError) + end + + it "should override foreign key auto_index negatively", :mysql => :skip do + with_fk_config(:auto_index => true) do + recreate_table @model, :foreign_keys => {:auto_index => false} do |t| + t.integer :user_id + end + expect(@model).not_to have_index.on(:user_id) + end + end + + it "should disable auto-index for a column", :mysql => :skip do + with_fk_config(:auto_index => true) do + recreate_table @model do |t| + t.integer :user_id, :index => false + end + expect(@model).not_to have_index.on(:user_id) + end + end + + end + + context "when table is changed" do + before(:each) do + @model = Post + end + [false, true].each do |bulk| + suffix = bulk ? ' with :bulk option' : "" + + it "should create a foreign key constraint"+suffix, :sqlite3 => :skip do + change_table(@model, :bulk => bulk) do |t| + t.integer :user_id + end + expect(@model).to reference(:users, :id).on(:user_id) + end + + context "migrate down" do + it "should remove a foreign key constraint"+suffix, :sqlite3 => :skip do + Comment.reset_column_information + expect(Comment).to reference(:users, :id).on(:user_id) + migration = Class.new ::ActiveRecord::Migration do + define_method(:change) { + change_table("comments", :bulk => bulk) do |t| + t.integer :user_id + end + } + end + ActiveRecord::Migration.suppress_messages do + migration.migrate(:down) + end + Comment.reset_column_information + expect(Comment).not_to reference(:users, :id).on(:user_id) + end + end + + it "should create a foreign key constraint using :references"+suffix, :sqlite3 => :skip do + change_table(@model, :bulk => bulk) do |t| + t.references :user + end + expect(@model).to reference(:users, :id).on(:user_id) + end + + it "should accept index shorthand when using :references"+suffix, :sqlite3 => :skip do + with_fk_config(:auto_index => false) do + change_table(@model, :bulk => bulk) do |t| + t.references :user, :index => true + end + end + expect(@model).to have_index.on(:user_id) + end + + + it "should create a foreign key constraint using :belongs_to"+suffix, :sqlite3 => :skip do + change_table(@model, :bulk => bulk) do |t| + t.belongs_to :user + end + expect(@model).to reference(:users, :id).on(:user_id) + end + end + end + + context "when column is added", :sqlite3 => :skip do + + before(:each) do + @model = Comment + end + + it "should create foreign key" do + add_column(:post_id, :integer) do + expect(@model).to reference(:posts, :id).on(:post_id) + end + end + + it "should create foreign key to explicitly given table" do + add_column(:author_id, :integer, :foreign_key => { :references => :users }) do + expect(@model).to reference(:users, :id).on(:author_id) + end + end + + it "should create foreign key to explicitly given table using shortcut" do + add_column(:author_id, :integer, :references => :users) do + expect(@model).to reference(:users, :id).on(:author_id) + end + end + + it "should create foreign key to explicitly given table and column name" do + add_column(:author_login, :string, :foreign_key => { :references => [:users, :login]}) do + expect(@model).to reference(:users, :login).on(:author_login) + end + end + + it "should create foreign key to the same table on parent_id" do + add_column(:parent_id, :integer) do + expect(@model).to reference(@model.table_name, :id).on(:parent_id) + end + end + + it "shouldn't create foreign key if column doesn't look like foreign key" do + add_column(:views_count, :integer) do + expect(@model).not_to reference.on(:views_count) + end + end + + it "shouldn't create foreign key if specified explicitly" do + add_column(:post_id, :integer, :foreign_key => false) do + expect(@model).not_to reference.on(:post_id) + end + end + + it "shouldn't create foreign key if specified explicitly by shorthand" do + add_column(:post_id, :integer, :references => nil) do + expect(@model).not_to reference.on(:post_id) + end + end + + it "should auto-index if specified in global options" do + SchemaPlus::ForeignKeys.config.auto_index = true + add_column(:post_id, :integer) do + expect(@model).to have_index.on(:post_id) + end + SchemaPlus::ForeignKeys.config.auto_index = false + end + + it "should auto-index foreign keys only" do + SchemaPlus::ForeignKeys.config.auto_index = true + add_column(:state, :integer) do + expect(@model).not_to have_index.on(:state) + end + SchemaPlus::ForeignKeys.config.auto_index = false + end + + # MySQL creates an index on foreign key and we can't override that + it "should allow to overwrite auto_index options in column definition", :mysql => :skip do + SchemaPlus::ForeignKeys.config.auto_index = true + add_column(:post_id, :integer, :index => false) do + expect(@model).not_to have_index.on(:post_id) + end + SchemaPlus::ForeignKeys.config.auto_index = false + end + + it "should use default on_update action" do + SchemaPlus::ForeignKeys.config.on_update = :cascade + add_column(:post_id, :integer) do + expect(@model).to reference.on(:post_id).on_update(:cascade) + end + SchemaPlus::ForeignKeys.config.on_update = nil + end + + it "should use default on_delete action" do + SchemaPlus::ForeignKeys.config.on_delete = :cascade + add_column(:post_id, :integer) do + expect(@model).to reference.on(:post_id).on_delete(:cascade) + end + SchemaPlus::ForeignKeys.config.on_delete = nil + end + + it "should allow to overwrite default actions" do + SchemaPlus::ForeignKeys.config.on_delete = :cascade + SchemaPlus::ForeignKeys.config.on_update = :restrict + add_column(:post_id, :integer, :foreign_key => { :on_update => :nullify, :on_delete => :nullify}) do + expect(@model).to reference.on(:post_id).on_delete(:nullify).on_update(:nullify) + end + SchemaPlus::ForeignKeys.config.on_delete = nil + end + + it "should create foreign key with default name" do + add_column(:post_id, :integer) do + expect(@model).to reference(:posts, :id).with_name("fk_#{@model.table_name}_post_id") + end + end + + protected + def add_column(column_name, *args) + table = @model.table_name + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.add_column(table, column_name, *args) + @model.reset_column_information + yield if block_given? + ActiveRecord::Migration.remove_column(table, column_name) + end + end + + end + + + context "when column is changed" do + + before(:each) do + @model = Comment + end + + context "with foreign keys", :sqlite3 => :skip do + + it "should create foreign key" do + change_column :user, :string, :foreign_key => { :references => [:users, :login] } + expect(@model).to reference(:users, :login).on(:user) + end + + context "and initially references to users table" do + + before(:each) do + recreate_table @model do |t| + t.integer :user_id + end + end + + it "should have foreign key" do + expect(@model).to reference(:users) + end + + it "should drop foreign key if it is no longer valid" do + change_column :user_id, :integer, :foreign_key => { :references => :members } + expect(@model).not_to reference(:users) + end + + it "should drop foreign key if requested to do so" do + change_column :user_id, :integer, :foreign_key => { :references => nil } + expect(@model).not_to reference(:users) + end + + it "should remove auto-created index if foreign key is removed" do + expect(@model).to have_index.on(:user_id) # sanity check that index was auto-created + change_column :user_id, :integer, :foreign_key => { :references => nil } + expect(@model).not_to have_index.on(:user_id) + end + + it "should reference pointed table afterwards if new one is created" do + change_column :user_id, :integer, :foreign_key => { :references => :members } + expect(@model).to reference(:members) + end + + it "should maintain foreign key if it's unaffected by change" do + change_column :user_id, :integer, :default => 0 + expect(@model).to reference(:users) + end + + it "should maintain foreign key if it's unaffected by change, even if auto_index is off" do + with_fk_config(:auto_create => false) do + change_column :user_id, :integer, :default => 0 + expect(@model).to reference(:users) + end + end + + end + + context "if column defined without foreign key but with index" do + before(:each) do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => false, :index => true + end + end + + it "should create the index" do + expect(@model).to have_index.on(:user_id) + end + + it "adding foreign key should not fail due to attempt to auto-create existing index" do + expect { change_column :user_id, :integer, :foreign_key => true }.to_not raise_error + end + end + end + + context "without foreign keys" do + + it "doesn't auto-add foreign keys" do + recreate_table @model do |t| + t.integer :user_id, :foreign_key => false + t.string :other_column + end + with_fk_auto_create do + change_column :other_column, :text + end + expect(@model).to_not reference(:users) + end + + end + + protected + def change_column(column_name, *args) + table = @model.table_name + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.change_column(table, column_name, *args) + @model.reset_column_information + end + end + + end + + context "when column is removed", :sqlite3 => :skip do + before(:each) do + @model = Comment + recreate_table @model do |t| + t.integer :post_id + end + end + + it "should remove a foreign key" do + expect(@model).to reference(:posts) + remove_column(:post_id) + expect(@model).not_to reference(:posts) + end + + it "should remove an index" do + expect(@model).to have_index.on(:post_id) + remove_column(:post_id) + expect(@model).not_to have_index.on(:post_id) + end + + protected + def remove_column(column_name) + table = @model.table_name + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.remove_column(table, column_name) + @model.reset_column_information + end + end + end + + + context "when table is renamed" do + + before(:each) do + @model = Comment + recreate_table @model do |t| + t.integer :user_id + t.integer :xyz, :index => true + end + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.rename_table @model.table_name, :newname + end + end + + it "should rename fk indexes" do + index = ActiveRecord::Base.connection.indexes(:newname).find(&its.columns == ['user_id']) + expect(index.name).to match(/^fk__newname_/) + end + + it "should rename foreign key constraints", :sqlite3 => :skip do + expect(ActiveRecord::Base.connection.foreign_keys(:newname).first.name).to match(/newname/) + end + + end + + + context "when table with more than one fk constraint is renamed", :sqlite3 => :skip do + + before(:each) do + @model = Comment + recreate_table @model do |t| + t.integer :user_id + t.integer :member_id + end + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.rename_table @model.table_name, :newname + end + end + + it "should rename foreign key constraints" do + names = ActiveRecord::Base.connection.foreign_keys(:newname).map(&:name) + expect(names.grep(/newname/)).to eq(names) + end + end + + def recreate_table(model, opts={}, &block) + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.create_table model.table_name, opts.merge(:force => true), &block + end + model.reset_column_information + end + + def change_table(model, opts={}, &block) + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.change_table model.table_name, opts, &block + end + model.reset_column_information + end + +end + diff --git a/spec/schema_auto_foreign_keys/named_schemas_spec.rb b/spec/schema_auto_foreign_keys/named_schemas_spec.rb new file mode 100644 index 0000000..28d9718 --- /dev/null +++ b/spec/schema_auto_foreign_keys/named_schemas_spec.rb @@ -0,0 +1,136 @@ +require 'spec_helper' + +describe "with multiple schemas" do + def connection + ActiveRecord::Base.connection + end + + before(:all) do + newdb = case connection.adapter_name + when /^mysql/i then "CREATE SCHEMA IF NOT EXISTS schema_plus_test2" + when /^postgresql/i then "CREATE SCHEMA schema_plus_test2" + when /^sqlite/i then "ATTACH ':memory:' AS schema_plus_test2" + end + begin + ActiveRecord::Base.connection.execute newdb + rescue ActiveRecord::StatementInvalid => e + raise unless e.message =~ /already exists/ + end + + class User < ::ActiveRecord::Base ; end + end + + before(:each) do + define_schema(:auto_create => false) do + create_table :users, :force => true do |t| + t.string :login + end + end + + connection.execute 'DROP TABLE IF EXISTS schema_plus_test2.users' + connection.execute 'CREATE TABLE schema_plus_test2.users (id ' + case connection.adapter_name + when /^mysql/i then "integer primary key auto_increment" + when /^postgresql/i then "serial primary key" + when /^sqlite/i then "integer primary key autoincrement" + end + ", login varchar(255))" + end + + context "with foreign key in each schema" do + before(:each) do + class Comment < ::ActiveRecord::Base ; end + connection.execute 'DROP TABLE IF EXISTS schema_plus_test2.comments' + connection.execute 'CREATE TABLE schema_plus_test2.comments (user_id integer,' + case connection.adapter_name + when /^mysql/i then "foreign key (user_id) references schema_plus_test2.users (id))" + when /^postgresql/i then "foreign key (user_id) references schema_plus_test2.users (id))" + when /^sqlite/i then "foreign key (user_id) references users (id))" + end + end + + around(:each) do |example| + begin + example.run + ensure + connection.execute 'DROP TABLE IF EXISTS comments' + connection.execute 'DROP TABLE IF EXISTS schema_plus_test2.comments' + end + end + + it "should not find foreign keys in other schema" do + connection.create_table :comments, :force => true do |t| + t.integer :user_id, :foreign_key => false + end + Comment.reset_column_information + expect(Comment.foreign_keys.length).to eq(0) + User.reset_column_information + expect(User.reverse_foreign_keys.length).to eq(0) + end + + it "should find foreign keys in this schema" do + connection.create_table :comments, :force => true do |t| + t.integer :user_id, :foreign_key => true + end + Comment.reset_column_information + expect(Comment.foreign_keys.map(&:column).flatten).to eq(["user_id"]) + User.reset_column_information + expect(User.reverse_foreign_keys.map(&:column).flatten).to eq(["user_id"]) + end + + end + + context "foreign key migrations" do + before(:each) do + define_schema do + create_table "items", :force => true do |t| + end + create_table "schema_plus_test2.groups", :force => true do |t| + end + create_table "schema_plus_test2.members", :force => true do |t| + t.integer :item_id, :foreign_key => true unless SchemaDev::Rspec::Helpers.mysql? + t.integer :group_id, :foreign_key => { references: "schema_plus_test2.groups" } + end + end + class Group < ::ActiveRecord::Base + self.table_name = "schema_plus_test2.groups" + end + class Item < ::ActiveRecord::Base + self.table_name = "items" + end + class Member < ::ActiveRecord::Base + self.table_name = "schema_plus_test2.members" + end + end + + around(:each) do |example| + begin + example.run + ensure + connection.execute 'DROP TABLE IF EXISTS schema_plus_test2.members' + connection.execute 'DROP TABLE IF EXISTS schema_plus_test2.groups' + connection.execute 'DROP TABLE IF EXISTS items' + end + end + + it "should find foreign keys" do + expect(Member.foreign_keys).not_to be_empty + end + + it "should find reverse foreign keys" do + expect(Group.reverse_foreign_keys).not_to be_empty + end + + it "should reference table in same schema" do + expect(Member.foreign_keys.map(&:to_table)).to include "schema_plus_test2.groups" + end + + it "should reference table in default schema", :mysql => :skip do + expect(Member.foreign_keys.map(&:to_table)).to include "items" + end + + it "should include the schema in the constraint name" do + expected_names = ["fk_schema_plus_test2_members_group_id"] + expected_names << "fk_schema_plus_test2_members_item_id" unless SchemaDev::Rspec::Helpers.mysql? + expect(Member.foreign_keys.map(&:name).sort).to match_array(expected_names.sort) + end + end + +end diff --git a/spec/schema_auto_foreign_keys/schema_dumper_spec.rb b/spec/schema_auto_foreign_keys/schema_dumper_spec.rb new file mode 100644 index 0000000..582bc56 --- /dev/null +++ b/spec/schema_auto_foreign_keys/schema_dumper_spec.rb @@ -0,0 +1,256 @@ +require 'spec_helper' +require 'stringio' + +describe "Schema dump" do + + before(:all) do + SchemaPlus::ForeignKeys.setup do |config| + config.auto_create = false + end + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Schema.define do + connection.tables.each do |table| drop_table table, force: :cascade end + + create_table :users, :force => true do |t| + t.string :login + t.datetime :deleted_at + t.integer :first_post_id, index: { unique: true } + end + + create_table :posts, :force => true do |t| + t.text :body + t.integer :user_id + t.integer :first_comment_id + t.string :string_no_default + t.integer :short_id + t.string :str_short + t.integer :integer_col + t.float :float_col + t.decimal :decimal_col + t.datetime :datetime_col + t.timestamp :timestamp_col + t.time :time_col + t.date :date_col + t.binary :binary_col + t.boolean :boolean_col + end + + create_table :comments, :force => true do |t| + t.text :body + t.integer :post_id + t.integer :commenter_id + end + end + end + class ::User < ActiveRecord::Base ; end + class ::Post < ActiveRecord::Base ; end + class ::Comment < ActiveRecord::Base ; end + end + + it "should include foreign_key definition" do + with_foreign_key Post, :user_id, :users, :id do + expect(dump_posts).to match(%r{t.integer\s+"user_id".*foreign_key.*users}) + end + end + + it "should include foreign_key name" do + with_foreign_key Post, :user_id, :users, :id, :name => "yippee" do + expect(dump_posts).to match(/user_id.*foreign_key.*users.*name: "yippee"/) + end + end + + it "should respect foreign key's primary key" do + with_foreign_key Post, :user_id, :users, :first_post_id do + expect(dump_posts).to match(%r{t.integer\s+"user_id".*foreign_key.*primary_key: "first_post_id"}) + end + end + + + it "should include foreign_key exactly once" do + with_foreign_key Post, :user_id, :users, :id, :name => "yippee" do + expect(dump_posts.scan(/foreign_key.*yippee"/).length).to eq 1 + end + end + + + xit "should sort foreign_key definitions" do + with_foreign_keys Comment, [ [ :post_id, :posts, :id ], [ :commenter_id, :users, :id ]] do + expect(dump_schema).to match(/foreign_key.+commenter_id.+foreign_key.+post_id/m) + end + end + + context "with constraint dependencies" do + it "should sort in Posts => Comments direction" do + with_foreign_key Comment, :post_id, :posts, :id do + expect(dump_schema).to match(%r{create_table "posts".*create_table "comments"}m) + end + end + it "should sort in Comments => Posts direction" do + with_foreign_key Post, :first_comment_id, :comments, :id do + expect(dump_schema).to match(%r{create_table "comments".*create_table "posts"}m) + end + end + + it "should handle regexp in ignore_tables" do + with_foreign_key Comment, :post_id, :posts, :id do + dump = dump_schema(:ignore => /post/) + expect(dump).to match(/create_table "comments"/) + expect(dump).not_to match(/create_table "posts"/) + end + end + + end + + it "should include foreign_key options" do + with_foreign_key Post, :user_id, :users, :id, :on_update => :cascade, :on_delete => :nullify do + expect(dump_posts).to match(%q[t.integer\s*"user_id",.*foreign_key: {references: "users", name: "fk_posts_user_id", on_update: :cascade, on_delete: :nullify}]) + end + end + + context "with cyclic foreign key constraints", :sqlite3 => :skip do + before(:all) do + ActiveRecord::Base.connection.add_foreign_key(Comment.table_name, User.table_name, column: :commenter_id) + ActiveRecord::Base.connection.add_foreign_key(Comment.table_name, Post.table_name, column: :post_id) + ActiveRecord::Base.connection.add_foreign_key(Post.table_name, Comment.table_name, column: :first_comment_id) + ActiveRecord::Base.connection.add_foreign_key(Post.table_name, User.table_name, column: :user_id) + ActiveRecord::Base.connection.add_foreign_key(User.table_name, Post.table_name, column: :first_post_id) + end + + it "should not raise an error" do + expect { dump_schema }.to_not raise_error + end + + ["comments", "posts", "users"].each do |table| + it "should dump constraints for table #{table.inspect} after the table definition" do + dump = dump_schema.gsub(/#[^\n*]/m, '') + expect(dump =~ %r{create_table "#{table}"}).to be < (dump =~ %r{foreign_key.*"#{table}"}) + end + end + + ["comments", "posts"].each do |table| + qtable = table.inspect + it "should dump comments for delayed constraint definition referencing table #{qtable}" do + expect(dump_schema).to match(%r{# foreign key references #{qtable}.*create_table #{qtable}.*add_foreign_key \S+, #{qtable}}m) + end + end + + context 'with complicated schemas' do + before(:all) do + + SchemaPlus::ForeignKeys.setup do |config| + config.auto_create = false + end + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Schema.define do + connection.tables.each do |table| drop_table table, force: :cascade end + + create_table :grade_systems, force: true do |t| + t.string :name + t.integer :school_id + t.integer :parent_id + t.integer :profile_id + end + + create_table :schools, force: true do |t| + t.string :name + t.integer :default_grade_system_id + end + + create_table :academic_years, force: true do |t| + t.string :name + t.integer :school_id + end + + create_table :buildings, force: true do |t| + t.string :name + t.integer :school_id + end + + create_table :profiles, force: true do |t| + t.integer :school_id + t.integer :building_id + end + + end + end + + class ::AcademicYear < ActiveRecord::Base ; end + class ::Building < ActiveRecord::Base ; end + class ::GradeSystem < ActiveRecord::Base ; end + class ::Profile < ActiveRecord::Base ; end + class ::School < ActiveRecord::Base ; end + + ActiveRecord::Base.connection.add_foreign_key(School.table_name, GradeSystem.table_name, column: :default_grade_system_id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, School.table_name, column: :school_id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, GradeSystem.table_name, column: :parent_id) + ActiveRecord::Base.connection.add_foreign_key(GradeSystem.table_name, Profile.table_name, column: :profile_id) + ActiveRecord::Base.connection.add_foreign_key(Profile.table_name, Building.table_name, column: :building_id) + ActiveRecord::Base.connection.add_foreign_key(Profile.table_name, School.table_name, column: :school_id) + ActiveRecord::Base.connection.add_foreign_key(Building.table_name, School.table_name, column: :school_id) + ActiveRecord::Base.connection.add_foreign_key(AcademicYear.table_name, School.table_name, column: :school_id) + end + + it "should not raise an error" do + expect { dump_schema }.to_not raise_error + end + + ["buildings", "grade_systems", "profiles", "schools"].each do |table| + it "should dump constraints for table #{table.inspect} after the table definition" do + expect(dump_schema =~ %r{create_table "#{table}"}).to be < (dump_schema =~ %r{foreign_key.*"#{table}"}) + end + end + end + end + + protected + def to_regexp(string) + Regexp.new(Regexp.escape(string)) + end + + def with_foreign_key(model, columns, referenced_table_name, referenced_columns, options = {}, &block) + with_foreign_keys(model, [[columns, referenced_table_name, referenced_columns, options]], &block) + end + + def with_foreign_keys(model, columnsets) + table_columns = model.columns.reject{|column| column.name == 'id'} + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.create_table model.table_name, :force => true do |t| + table_columns.each do |column| + t.column column.name, column.type + end + columnsets.each do |columns, referenced_table_name, referenced_columns, options| + t.foreign_key columns, referenced_table_name, (options||{}).merge(primary_key: referenced_columns) + end + end + end + model.reset_column_information + begin + yield + ensure + ActiveRecord::Migration.suppress_messages do + ActiveRecord::Migration.create_table model.table_name, :force => true do |t| + table_columns.each do |column| + t.column column.name, column.type + end + end + end + end + end + + def determine_foreign_key_name(model, columns, options) + name = options[:name] + name ||= model.foreign_keys.detect { |fk| fk.from_table == model.table_name.to_s && Array.wrap(fk.column) == Array.wrap(columns).collect(&:to_s) }.name + end + + def dump_schema(opts={}) + stream = StringIO.new + ActiveRecord::SchemaDumper.ignore_tables = Array.wrap(opts[:ignore]) || [] + ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream) + stream.string + end + + def dump_posts + dump_schema(:ignore => %w[users comments]) + end + +end diff --git a/spec/schema_auto_foreign_keys/schema_spec.rb b/spec/schema_auto_foreign_keys/schema_spec.rb new file mode 100644 index 0000000..089c82e --- /dev/null +++ b/spec/schema_auto_foreign_keys/schema_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +describe ActiveRecord::Schema do + + let(:schema) { ActiveRecord::Schema } + + let(:connection) { ActiveRecord::Base.connection } + + context "defining with auto_index and auto_create" do + + around(:each) do |example| + with_auto_index do + with_auto_create do + example.run + end + end + end + + it "should pass" do + expect { do_schema }.to_not raise_error + end + + it "should create only explicity added indexes" do + do_schema + expected = SchemaDev::Rspec::Helpers.mysql? ? 2 : 1 + expect(connection.tables.collect { |table| connection.indexes(table) }.flatten.size).to eq(expected) + end + + it "should create only explicity added foriegn keys" do + do_schema + expect(connection.tables.collect { |table| connection.foreign_keys(table) }.flatten.size).to eq(2) + end + + protected + + def do_schema + define_schema do + + create_table :users, :force => true do + end + + create_table :colors, :force => true do + end + + create_table :shoes, :force => true do + end + + create_table :posts, :force => true do |t| + t.integer :user_id, :references => :users, :index => true + t.integer :shoe_id, :references => :shoes # should not have an index (except mysql) + t.integer :color_id # should not have a foreign key nor index + end + end + end + + end + + it "handles explicit foreign keys" do + expect { + with_auto_create(false) do + define_schema do + create_table :users, :force => :cascade do + end + + create_table :posts, :force => :cascade do |t| + t.integer :user_id + t.foreign_key :users + end + end + end + }.not_to raise_error + expect(connection.foreign_keys("posts").first.to_table).to eq "users" + end + + + protected + + + def with_auto_index(value = true) + old_value = SchemaPlus::ForeignKeys.config.auto_index + SchemaPlus::ForeignKeys.config.auto_index = value + begin + yield + ensure + SchemaPlus::ForeignKeys.config.auto_index = old_value + end + end + + def with_auto_create(value = true) + old_value = SchemaPlus::ForeignKeys.config.auto_create + SchemaPlus::ForeignKeys.config.auto_create = value + begin + yield + ensure + SchemaPlus::ForeignKeys.config.auto_create = old_value + end + end + +end