From e13e845db51a08d3d3da328e35d787f2d3dbe0a3 Mon Sep 17 00:00:00 2001 From: Phill Baker Date: Fri, 25 Mar 2016 20:52:37 -0400 Subject: [PATCH] Migrations should use Vanity connection. Fixes #295. --- lib/generators/templates/vanity_migration.rb | 112 +++++++++++-------- lib/vanity/adapters.rb | 7 +- lib/vanity/autoconnect.rb | 33 +++--- lib/vanity/connection.rb | 4 +- test/adapters_test.rb | 17 +++ test/autoconnect_test.rb | 8 +- test/connection_test.rb | 6 - test/test_helper.rb | 6 +- 8 files changed, 116 insertions(+), 77 deletions(-) create mode 100644 test/adapters_test.rb diff --git a/lib/generators/templates/vanity_migration.rb b/lib/generators/templates/vanity_migration.rb index 2b762020..235a246f 100644 --- a/lib/generators/templates/vanity_migration.rb +++ b/lib/generators/templates/vanity_migration.rb @@ -1,55 +1,73 @@ class VanityMigration < ActiveRecord::Migration - def self.up - create_table :vanity_metrics do |t| - t.string :metric_id - t.datetime :updated_at - end - add_index :vanity_metrics, [:metric_id] + # Helper methods to ensure we're connecting to the right database, see + # https://github.com/assaf/vanity/issues/295. - create_table :vanity_metric_values do |t| - t.integer :vanity_metric_id - t.integer :index - t.integer :value - t.string :date - end - add_index :vanity_metric_values, [:vanity_metric_id, :date] - - create_table :vanity_experiments do |t| - t.string :experiment_id - t.integer :outcome - t.boolean :enabled - t.datetime :created_at - t.datetime :completed_at - end - add_index :vanity_experiments, [:experiment_id] + def connection + @connection ||= ActiveRecord::Base.connection + end + alias_method :default_connection, :connection - create_table :vanity_conversions do |t| - t.integer :vanity_experiment_id - t.integer :alternative - t.integer :conversions - end - add_index :vanity_conversions, [:vanity_experiment_id, :alternative], :name => "by_experiment_id_and_alternative" - - create_table :vanity_participants do |t| - t.string :experiment_id - t.string :identity - t.integer :shown - t.integer :seen - t.integer :converted - t.timestamps + def with_vanity_connection + @connection = Vanity::Adapters::ActiveRecordAdapter::VanityRecord.connection + yield + @connection = default_connection + end + + def up + with_vanity_connection do + create_table :vanity_metrics do |t| + t.string :metric_id + t.datetime :updated_at + end + add_index :vanity_metrics, [:metric_id] + + create_table :vanity_metric_values do |t| + t.integer :vanity_metric_id + t.integer :index + t.integer :value + t.string :date + end + add_index :vanity_metric_values, [:vanity_metric_id, :date] + + create_table :vanity_experiments do |t| + t.string :experiment_id + t.integer :outcome + t.boolean :enabled + t.datetime :created_at + t.datetime :completed_at + end + add_index :vanity_experiments, [:experiment_id] + + create_table :vanity_conversions do |t| + t.integer :vanity_experiment_id + t.integer :alternative + t.integer :conversions + end + add_index :vanity_conversions, [:vanity_experiment_id, :alternative], :name => "by_experiment_id_and_alternative" + + create_table :vanity_participants do |t| + t.string :experiment_id + t.string :identity + t.integer :shown + t.integer :seen + t.integer :converted + t.timestamps + end + add_index :vanity_participants, [:experiment_id] + add_index :vanity_participants, [:experiment_id, :identity], :name => "by_experiment_id_and_identity" + add_index :vanity_participants, [:experiment_id, :shown], :name => "by_experiment_id_and_shown" + add_index :vanity_participants, [:experiment_id, :seen], :name => "by_experiment_id_and_seen" + add_index :vanity_participants, [:experiment_id, :converted], :name => "by_experiment_id_and_converted" end - add_index :vanity_participants, [:experiment_id] - add_index :vanity_participants, [:experiment_id, :identity], :name => "by_experiment_id_and_identity" - add_index :vanity_participants, [:experiment_id, :shown], :name => "by_experiment_id_and_shown" - add_index :vanity_participants, [:experiment_id, :seen], :name => "by_experiment_id_and_seen" - add_index :vanity_participants, [:experiment_id, :converted], :name => "by_experiment_id_and_converted" end - def self.down - drop_table :vanity_metrics - drop_table :vanity_metric_values - drop_table :vanity_experiments - drop_table :vanity_conversions - drop_table :vanity_participants + def down + with_vanity_connection do + drop_table :vanity_metrics + drop_table :vanity_metric_values + drop_table :vanity_experiments + drop_table :vanity_conversions + drop_table :vanity_participants + end end end diff --git a/lib/vanity/adapters.rb b/lib/vanity/adapters.rb index 81a9749a..4ffbbf2d 100644 --- a/lib/vanity/adapters.rb +++ b/lib/vanity/adapters.rb @@ -3,17 +3,20 @@ module Adapters class << self # Creates new connection to underlying datastore and returns suitable # adapter (adapter object extends AbstractAdapter and wraps the - # connection). Vanity.playground.establish_connection uses this. + # connection). # # @since 1.4.0 def establish_connection(spec) + return unless Autoconnect.should_connect? || + (Autoconnect.schema_relevant? && spec[:adapter].to_s == 'active_record') + begin require "vanity/adapters/#{spec[:adapter]}_adapter" rescue LoadError raise "Could not find #{spec[:adapter]} in your load path" end adapter_method = "#{spec[:adapter]}_connection" - send adapter_method, spec + send(adapter_method, spec) end end end diff --git a/lib/vanity/autoconnect.rb b/lib/vanity/autoconnect.rb index 1e85e25d..ff97d832 100644 --- a/lib/vanity/autoconnect.rb +++ b/lib/vanity/autoconnect.rb @@ -42,22 +42,29 @@ module Autoconnect ] ENVIRONMENT_VANITY_DISABLED_FLAG = "VANITY_DISABLED" - def self.playground_should_autoconnect? - !environment_disabled? && !in_blacklisted_rake_task? - end + class << self + def should_connect? + !environment_disabled? && !in_blacklisted_rake_task? + end + alias_method :playground_should_autoconnect?, :should_connect? - def self.environment_disabled? - !!ENV[ENVIRONMENT_VANITY_DISABLED_FLAG] - end + def schema_relevant? + current_rake_tasks.any? { |task| task =~ /\Adb:/ } + end - def self.in_blacklisted_rake_task? - !(current_rake_tasks & BLACKLISTED_RAILS_RAKE_TASKS).empty? - end + def environment_disabled? + !!ENV[ENVIRONMENT_VANITY_DISABLED_FLAG] + end + + def in_blacklisted_rake_task? + !(current_rake_tasks & BLACKLISTED_RAILS_RAKE_TASKS).empty? + end - def self.current_rake_tasks - ::Rake.application.top_level_tasks - rescue => e - [] + def current_rake_tasks + ::Rake.application.top_level_tasks + rescue => e + [] + end end end end \ No newline at end of file diff --git a/lib/vanity/connection.rb b/lib/vanity/connection.rb index f87acc79..fbedd6b0 100644 --- a/lib/vanity/connection.rb +++ b/lib/vanity/connection.rb @@ -33,9 +33,7 @@ class InvalidSpecification < StandardError; end def initialize(specification=nil) @specification = specification || DEFAULT_SPECIFICATION - if Autoconnect.playground_should_autoconnect? - @adapter = setup_connection(@specification) - end + @adapter = setup_connection(@specification) end # Closes the current connection. diff --git a/test/adapters_test.rb b/test/adapters_test.rb new file mode 100644 index 00000000..a45b95ff --- /dev/null +++ b/test/adapters_test.rb @@ -0,0 +1,17 @@ +require "test_helper" + +describe Vanity::Adapters do + describe ".establish_connection" do + it "returns nil when not autoconnecting" do + Vanity::Autoconnect.stubs(:should_connect?).returns(false) + adapter = Vanity::Adapters.establish_connection({}) + assert_nil adapter + end + + it "returns nil using active record and migrating" do + Rake.stubs(:application).returns(stub(:top_level_tasks => ['db:migrate'])) + adapter = Vanity::Adapters.establish_connection(adapter: 'active_record') + refute_nil adapter + end + end +end diff --git a/test/autoconnect_test.rb b/test/autoconnect_test.rb index f6c1de77..b0533910 100644 --- a/test/autoconnect_test.rb +++ b/test/autoconnect_test.rb @@ -1,23 +1,23 @@ require "test_helper" describe Vanity::Autoconnect do - describe ".playground_should_autoconnect?" do + describe ".should_connect?" do it "returns true by default" do - autoconnect = Vanity::Autoconnect.playground_should_autoconnect? + autoconnect = Vanity::Autoconnect.should_connect? assert autoconnect == true end it "returns false if environment flag is set" do ENV['VANITY_DISABLED'] = '1' - autoconnect = Vanity::Autoconnect.playground_should_autoconnect? + autoconnect = Vanity::Autoconnect.should_connect? assert autoconnect == false ENV['VANITY_DISABLED'] = nil end it "returns false if in assets:precompile rake task" do Rake.expects(:application).returns(stub(:top_level_tasks => ['assets:precompile'])) - autoconnect = Vanity::Autoconnect.playground_should_autoconnect? + autoconnect = Vanity::Autoconnect.should_connect? assert autoconnect == false end end diff --git a/test/connection_test.rb b/test/connection_test.rb index 5c797069..48fd0e07 100644 --- a/test/connection_test.rb +++ b/test/connection_test.rb @@ -12,12 +12,6 @@ Vanity::Connection.new(adapter: "mock") end - it "can skip connection" do - Vanity::Autoconnect.stubs(:playground_should_autoconnect?).returns(false) - connection = Vanity::Connection.new(adapter: "mock") - assert !connection.connected? - end - it "parses from a string" do Vanity::Adapters.expects(:establish_connection).with( adapter: 'redis', diff --git a/test/test_helper.rb b/test/test_helper.rb index 663135fb..dedf200b 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -198,7 +198,9 @@ def setup_controller_request_and_response ActiveRecord::Base.establish_connection ActiveRecord::Base.logger = $logger + Vanity.connect!(VanityTestHelpers::DATABASE) require "generators/templates/vanity_migration" - VanityMigration.down rescue nil - VanityMigration.up + # Clear any existing tables. If we error because they don't exist, move on. + VanityMigration.new.migrate(:down) rescue SQLite3::SQLException + VanityMigration.new.migrate(:up) end