From 0075191c493c9f49bf837d8f19af285170b3c051 Mon Sep 17 00:00:00 2001 From: Polo Date: Mon, 9 Jul 2018 15:07:33 -0700 Subject: [PATCH 1/2] pl - fix gem sqlite3-ruby warning Hello! The sqlite3-ruby gem has changed it's name to just sqlite3. Rather than installing `sqlite3-ruby`, you should install `sqlite3`. Please update your dependencies accordingly. Thanks from the Ruby sqlite3 team! <3 <3 <3 <3 --- Gemfile | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Gemfile b/Gemfile index 8247135..67610d0 100644 --- a/Gemfile +++ b/Gemfile @@ -7,7 +7,3 @@ gem 'timecop' gem 'appraisal' gem 'sqlite3' gem 'nokogiri' - -group :active_record do - gem 'sqlite3-ruby', :require => 'sqlite3' -end From 089fe281392dfe5c5ceac88d2c1041d3545336fa Mon Sep 17 00:00:00 2001 From: Polo Date: Tue, 10 Jul 2018 16:50:07 -0700 Subject: [PATCH 2/2] pl - add support for Rails 5.1, 5.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - change gemspec - appraisal - generate gemfiles --- - fix `active_record.time_zone_aware_types` error happen in 5.1 and 5.2 ``` DEPRECATION WARNING: Time columns will become time zone aware in Rails 5.1. This still causes `String`s to be parsed as if they were in `Time.zone`, and `Time`s to be converted to `Time.zone`. To keep the old behavior, you must add the following to your initializer: config.active_record.time_zone_aware_types = [:datetime] To silence this deprecation warning, add the following: config.active_record.time_zone_aware_types = [:datetime, :time] ``` What does this even mean? I found an article explain it well http://engineering.liefery.com/2017/10/25/times-in-rails-5.html Here is a quick summary: ``` Here’s a good way of thinking about it: which columns in your application should return an ActiveSupport::TimeWithZone object? To keep the old behaviour and only have datetime columns return an ActiveSupport::TimeWithZone object, use [:datetime]. To use the new behaviour and have time columns also return an ActiveSupport::TimeWithZone object, use [:datetime, :time]. ``` Here is where I found how to config it without `Rails.config` http://api.rubyonrails.org/classes/ActiveRecord/Timestamp.html As the testcases suggest, it expects the old behaviour, i.e. only `datetime` is time zone aware. https://github.com/appfolio/validates_timeliness/blob/4c80b403e500f36ca3d0d93cf312c0a30828b71c/spec/validates_timeliness/orm/active_record_spec.rb#L90-L105 I changed `spec_helper.rb` to set `time_zone_aware_types`, but I don't know how to enforce it in a Rails app. --- In `lib/validates_timeliness/orm/active_record.rb`, the `timeliness_column_for_attribute` function depends on the internal implementation of ActiveRecord. https://github.com/appfolio/validates_timeliness/blob/4c80b403e500f36ca3d0d93cf312c0a30828b71c/lib/validates_timeliness/orm/active_record.rb#L17-L33 It gave me a really hard time to figure out how to fix it. I read the active_record source and here is what I found. I put them here as the reference. I didn't dig into the details why this funtion has to depend on the internal implementation of ActiveRecord. It's out of the scope of this upgrade. I got the error when I run the test: ``` NoMethodError: undefined method `new_column' for # # ./lib/validates_timeliness/orm/active_record.rb:32:in `block in timeliness_column_for_attribute' # ./lib/validates_timeliness/orm/active_record.rb:18:in `fetch' # ./lib/validates_timeliness/orm/active_record.rb:18:in `timeliness_column_for_attribute' # ./lib/validates_timeliness/orm/active_record.rb:10:in `timeliness_attribute_timezone_aware?' # ./spec/validates_timeliness/orm/active_record_spec.rb:100:in `block (4 levels) in ' ``` - rails https://github.com/rails/rails/releases/tag/v4.2.0.beta1 commit: introduce `new_column` method https://github.com/rails/rails/commit/98a7dde064172dbf9b9c441175fcd45dca70d8db - rails https://github.com/rails/rails/releases/tag/v5.1.0.beta1 commit: refactor column initialization into `new_column_from_field ` https://github.com/rails/rails/commit/3dce96a6b1242452860e3a9560ccfb357cede0b1 ``` abstract_mysql_adapter has `new_column`
https://github.com/rails/rails/blob/3dce96a6b1242452860e3a9560ccfb357cede0b1/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L172
postgresql doesn’t have `new_column` anymore
https://github.com/rails/rails/commit/3dce96a6b1242452860e3a9560ccfb357cede0b1#diff-6e377cf054cb4121e6f207bd7c14b492
sqlite3 just use base class `AbstractAdapter #new_column`
https://github.com/rails/rails/blob/3dce96a6b1242452860e3a9560ccfb357cede0b1/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb#L328 ``` - rails https://github.com/rails/rails/releases/tag/v5.2.0.beta1 commit: Make internal methods to private https://github.com/rails/rails/commit/e4108fc619e0f1c28cdec6049d31f2db01d56dfd ``` abstract_adapter doesn’t have `new_column` and `lookup_case_type` methods anymore
https://github.com/rails/rails/commit/e4108fc619e0f1c28cdec6049d31f2db01d56dfd#diff-c226a4680f86689c3c170d4bc5911e96L478 in connection_adapters/abstract/schema_statements.rb, it called `new_column_from_field`, but doesn't have definition for it https://github.com/rails/rails/blob/e4108fc619e0f1c28cdec6049d31f2db01d56dfd/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb#L108-L114 mysql_adapter has `new_column_from_field` in its schema_statements file https://github.com/rails/rails/blob/e4108fc619e0f1c28cdec6049d31f2db01d56dfd/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb#L14 sqlite3_adapter has `new_column_from_field` in its schema_statements file https://github.com/rails/rails/commit/e4108fc619e0f1c28cdec6049d31f2db01d56dfd#diff-cf85d787d5dfda96e016b6121426460cR14 postgresql_adapter has `new_column_from_field` in its schema_statements file https://github.com/rails/rails/commit/e4108fc619e0f1c28cdec6049d31f2db01d56dfd#diff-6e377cf054cb4121e6f207bd7c14b492R578 The `new_column_from_field` method in the above are private method ``` - rails 4.2.10 lookup_cast_type https://github.com/rails/rails/blob/v4.2.10/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L475 - rails 5.0 fetch_type_metadata https://github.com/rails/rails/blob/v5.0.0.beta1/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L857 - rails 5.1 fetch_type_metadata https://github.com/rails/rails/blob/v5.1.0/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb#L653 - rails 5.2 fetch_type_metadata https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb#L1295 --- Appraisals | 8 ++++++++ ae-validates_timeliness.gemspec | 2 +- gemfiles/rails_4_0.gemfile | 6 +----- gemfiles/rails_4_1.gemfile | 6 +----- gemfiles/rails_4_2.gemfile | 6 +----- gemfiles/rails_5_0.gemfile | 6 +----- gemfiles/rails_5_1.gemfile | 11 +++++++++++ gemfiles/rails_5_2.gemfile | 11 +++++++++++ lib/validates_timeliness/orm/active_record.rb | 16 ++++++++++++---- spec/spec_helper.rb | 3 +++ 10 files changed, 50 insertions(+), 25 deletions(-) create mode 100644 gemfiles/rails_5_1.gemfile create mode 100644 gemfiles/rails_5_2.gemfile diff --git a/Appraisals b/Appraisals index ccafefa..8c30e5b 100644 --- a/Appraisals +++ b/Appraisals @@ -13,3 +13,11 @@ end appraise "rails_5_0" do gem "rails", "~> 5.0.0" end + +appraise "rails_5_1" do + gem "rails", "~> 5.1.0" +end + +appraise "rails_5_2" do + gem "rails", "~> 5.2.0" +end diff --git a/ae-validates_timeliness.gemspec b/ae-validates_timeliness.gemspec index 45c9f34..9cddf4b 100644 --- a/ae-validates_timeliness.gemspec +++ b/ae-validates_timeliness.gemspec @@ -15,7 +15,7 @@ Gem::Specification.new do |s| s.executables = s.files.grep(%r{^bin/}) { |f| File.basename(f) } s.require_paths = ["lib"] - s.add_runtime_dependency(%q, [">= 4.0", "< 5.1"]) + s.add_runtime_dependency(%q, [">= 4.0", "< 5.3"]) s.add_runtime_dependency(%q, ["~> 0.3.7"]) s.add_development_dependency "coveralls" diff --git a/gemfiles/rails_4_0.gemfile b/gemfiles/rails_4_0.gemfile index c2990b6..06a5cd6 100644 --- a/gemfiles/rails_4_0.gemfile +++ b/gemfiles/rails_4_0.gemfile @@ -8,8 +8,4 @@ gem "appraisal" gem "sqlite3" gem "nokogiri" -group :active_record do - gem "sqlite3-ruby", :require => "sqlite3" -end - -gemspec :path => "../" +gemspec path: "../" diff --git a/gemfiles/rails_4_1.gemfile b/gemfiles/rails_4_1.gemfile index 6311646..010e9bc 100644 --- a/gemfiles/rails_4_1.gemfile +++ b/gemfiles/rails_4_1.gemfile @@ -8,8 +8,4 @@ gem "appraisal" gem "sqlite3" gem "nokogiri" -group :active_record do - gem "sqlite3-ruby", :require => "sqlite3" -end - -gemspec :path => "../" +gemspec path: "../" diff --git a/gemfiles/rails_4_2.gemfile b/gemfiles/rails_4_2.gemfile index b989715..353d93b 100644 --- a/gemfiles/rails_4_2.gemfile +++ b/gemfiles/rails_4_2.gemfile @@ -8,8 +8,4 @@ gem "appraisal" gem "sqlite3" gem "nokogiri" -group :active_record do - gem "sqlite3-ruby", :require => "sqlite3" -end - -gemspec :path => "../" +gemspec path: "../" diff --git a/gemfiles/rails_5_0.gemfile b/gemfiles/rails_5_0.gemfile index 8b6bfff..a2ef538 100644 --- a/gemfiles/rails_5_0.gemfile +++ b/gemfiles/rails_5_0.gemfile @@ -8,8 +8,4 @@ gem "appraisal" gem "sqlite3" gem "nokogiri" -group :active_record do - gem "sqlite3-ruby", :require => "sqlite3" -end - -gemspec :path => "../" +gemspec path: "../" diff --git a/gemfiles/rails_5_1.gemfile b/gemfiles/rails_5_1.gemfile new file mode 100644 index 0000000..12156c0 --- /dev/null +++ b/gemfiles/rails_5_1.gemfile @@ -0,0 +1,11 @@ +# This file was generated by Appraisal + +source "http://rubygems.org" + +gem "rails", "~> 5.1.0" +gem "timecop" +gem "appraisal" +gem "sqlite3" +gem "nokogiri" + +gemspec path: "../" diff --git a/gemfiles/rails_5_2.gemfile b/gemfiles/rails_5_2.gemfile new file mode 100644 index 0000000..7588d1d --- /dev/null +++ b/gemfiles/rails_5_2.gemfile @@ -0,0 +1,11 @@ +# This file was generated by Appraisal + +source "http://rubygems.org" + +gem "rails", "~> 5.2.0" +gem "timecop" +gem "appraisal" +gem "sqlite3" +gem "nokogiri" + +gemspec path: "../" diff --git a/lib/validates_timeliness/orm/active_record.rb b/lib/validates_timeliness/orm/active_record.rb index a799767..5629ae0 100644 --- a/lib/validates_timeliness/orm/active_record.rb +++ b/lib/validates_timeliness/orm/active_record.rb @@ -22,12 +22,20 @@ def timeliness_column_for_attribute(attr_name) ::ActiveRecord::ConnectionAdapters::Column.new(attr_name, nil, validation_type.to_s) else connection = ::ActiveRecord::Base.connection - arguments = if ::ActiveRecord.version > ::Gem::Version.new('4.3') - [attr_name, nil, connection.send(:fetch_type_metadata, validation_type.to_s), validation_type.to_s, table_name] + sql_type = validation_type.to_s + cast_type = begin + if ::ActiveRecord.version < ::Gem::Version.new('5') + connection.send(:lookup_cast_type, sql_type) else - [attr_name, nil, connection.lookup_cast_type(validation_type.to_s), validation_type.to_s] + connection.send(:fetch_type_metadata, sql_type) end - connection.new_column(*arguments) + end + + if ::ActiveRecord.version < ::Gem::Version.new('5.1') + connection.send(:new_column, attr_name, nil, cast_type, sql_type, table_name) + else + connection.send(:new_column_from_field, table_name, { 'name' => attr_name, 'type' => sql_type }) + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c7d8f9c..0b6059b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -69,6 +69,9 @@ class PersonWithShim < Person ActiveRecord::Base.default_timezone = :utc ActiveRecord::Base.time_zone_aware_attributes = true +if ActiveRecord.version >= Gem::Version.new('5') + ActiveRecord::Base.time_zone_aware_types = [:datetime] +end ActiveRecord::Base.establish_connection({:adapter => 'sqlite3', :database => ':memory:'}) ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define(:version => 1) do