Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI #8

Merged
merged 1 commit into from Sep 30, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -15,3 +15,4 @@ spec/reports
test/tmp
test/version_tmp
tmp
/gemfiles/*.lock
16 changes: 9 additions & 7 deletions .travis.yml
@@ -1,12 +1,14 @@
sudo: false
rvm:
- 1.9.3
- 2.0.0
- 2.1.2
- 2.0
- 2.1
- 2.2
gemfile:
- spec/gemfiles/Gemfile.ar-3.2
- spec/gemfiles/Gemfile.ar-4.0
- spec/gemfiles/Gemfile.ar-4.1
- spec/gemfiles/Gemfile.ar-edge
- gemfiles/Gemfile.ar-3.2
- gemfiles/Gemfile.ar-4.0
- gemfiles/Gemfile.ar-4.1
- gemfiles/Gemfile.ar-4.2
- gemfiles/Gemfile.ar-edge
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the Gemfile outside of spec because otherwise gems were installed in spec/gemfiles/bundler and rspec was trying to runt he dependencies specs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep... thats for sure a better idea.


before_script:
- mysql -e 'create database rescue_from_duplicate;'
Expand Down
15 changes: 6 additions & 9 deletions Rakefile
@@ -1,4 +1,4 @@
require "bundler/gem_tasks"
require 'bundler/gem_tasks'
require 'rspec/core/rake_task'

RSpec::Core::RakeTask.new(:spec)
Expand All @@ -7,14 +7,11 @@ task :default => :spec

namespace :spec do
task :all do
%w(3.2 4.0 4.1 edge).each do |ar_version|
system(
{
"BUNDLE_GEMFILE" => "spec/gemfiles/Gemfile.ar-#{ar_version}",
"MYSQL" => "1"
},
"rspec"
)
Dir[File.expand_path('../gemfiles/*.lock', __FILE__)].each { |f| File.delete(f) }
Dir[File.expand_path('../gemfiles/*', __FILE__)].each do |gemfile|
env = {'BUNDLE_GEMFILE' => gemfile, 'MYSQL' => '1'}
system(env, 'bundle install')
system(env, 'bundle exec rspec')
end
end
end
2 changes: 1 addition & 1 deletion spec/gemfiles/Gemfile.ar-3.2 → gemfiles/Gemfile.ar-3.2
Expand Up @@ -6,5 +6,5 @@ gem 'rake'
gem 'rspec'
gem 'sqlite3'
gem 'pg', '~> 0.11'
gem 'mysql2'
gem 'mysql2', '~> 0.3.20'
gem 'simplecov', require: false
2 changes: 1 addition & 1 deletion spec/gemfiles/Gemfile.ar-4.0 → gemfiles/Gemfile.ar-4.0
Expand Up @@ -6,5 +6,5 @@ gem 'rake'
gem 'rspec'
gem 'sqlite3'
gem 'pg', '~> 0.11'
gem 'mysql2'
gem 'mysql2', '~> 0.3.20'
gem 'simplecov', require: false
2 changes: 1 addition & 1 deletion spec/gemfiles/Gemfile.ar-4.1 → gemfiles/Gemfile.ar-4.1
Expand Up @@ -6,5 +6,5 @@ gem 'rake'
gem 'rspec'
gem 'sqlite3'
gem 'pg', '~> 0.11'
gem 'mysql2'
gem 'mysql2', '~> 0.3.20'
gem 'simplecov', require: false
10 changes: 10 additions & 0 deletions gemfiles/Gemfile.ar-4.2
@@ -0,0 +1,10 @@
source 'https://rubygems.org'

gem 'activerecord', '~> 4.2.0'
gem 'bundler', '~> 1.3'
gem 'rake'
gem 'rspec'
gem 'sqlite3'
gem 'pg', '~> 0.11'
gem 'mysql2', '~> 0.3.20'
gem 'simplecov', require: false
2 changes: 1 addition & 1 deletion spec/gemfiles/Gemfile.ar-edge → gemfiles/Gemfile.ar-edge
Expand Up @@ -8,5 +8,5 @@ gem 'rake'
gem 'rspec'
gem 'sqlite3'
gem 'pg', '~> 0.11'
gem 'mysql2'
gem 'mysql2', '~> 0.3.20'
gem 'simplecov', require: false
30 changes: 18 additions & 12 deletions lib/rescue_from_duplicate/active_record/extension.rb
Expand Up @@ -24,50 +24,56 @@ def _rescue_from_duplicate_handlers

def create_or_update(*params, &block)
super
rescue ActiveRecord::RecordNotUnique => exception
handler = exception_handler(exception)
rescue ActiveRecord::RecordNotUnique, ActiveRecord::StatementInvalid => exception
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look like AR 3.2 raises a ActiveRecord::StatementInvalid in some cases. At least it does for SQLite 3.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how long should we support 3.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we want. But 3.2 will go EOL when Rails 5 is released around this winter or early 2016 worst case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also just bump rescue_form_duplicate and change the dependency constraints.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's call it "until the next time it breaks"

raise unless handle_unicity_error(exception)
false
end

private

raise exception unless handler
def handle_unicity_error(exception)
handler = exception_handler(exception)
return false unless handler

attribute = handler.attributes.first
options = handler.options.except(:case_sensitive, :scope).merge(value: self.send(:read_attribute_for_validation, attribute))

self.errors.add(attribute, :taken, options)
false
true
end

private

def exception_handler(exception)
columns = exception_columns(exception)
return unless columns
columns.sort!

self.class._rescue_from_duplicate_handlers.detect do |handler|
handler.rescue? && columns == handler.columns
end
end

def exception_columns(exception)
case
when exception.message =~ /SQLite3::ConstraintException/
if exception.message =~ /SQLite3::ConstraintException/
sqlite3_exception_columns(exception)
when exception.message =~ /PG::UniqueViolation/
elsif exception.message =~ /PG::UniqueViolation/
postgresql_exception_columns(exception)
else
other_exception_columns(exception)
end.sort
end
end

def postgresql_exception_columns(exception)
extract_columns(exception.message[/Key \((.*?)\)=\(.*?\) already exists./, 1])
end

def sqlite3_exception_columns(exception)
extract_columns(exception.message[/columns? (.*) (?:is|are) not unique/, 1])
extract_columns(exception.message[/columns? (.*) (?:is|are) not unique/, 1]) ||
extract_columns(exception.message[/UNIQUE constraint failed: ([^:]*)\:/, 1])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look like sqlite changed it's format. I don't know if there was a major update of sqlite or something, nor if we have a way to test both version :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol :/

end

def extract_columns(columns_string)
return unless columns_string
columns_string.split(",").map(&:strip)
columns_string.split(",").map { |column| column.split('.').last.strip }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. This is because in that new format, columns are in their "fully qualified names", e.g. table_name.column_name, not just column_name

end

def other_exception_columns(exception)
Expand Down
2 changes: 0 additions & 2 deletions spec/spec_helper.rb
Expand Up @@ -15,8 +15,6 @@
rescue LoadError
end



module RescueFromDuplicate
class Base
cattr_accessor :exception
Expand Down