From b25fed39bd8ad00e5ede49b3c572efa94f7bc6f9 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 13:07:35 -0400 Subject: [PATCH 1/7] Testing on ActiveRecord 5.0 and passing (with lots of deprecations) --- Gemfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index d833204..64cd216 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ gemspec gem 'jruby-openssl', :platform => :jruby group :test do - gem 'activerecord', '~> 4.2.0' + gem 'activerecord', '~> 5.0.0' gem 'activerecord-jdbcsqlite3-adapter', :platform => [:jruby] gem 'libxml-ruby', :platform => [:ruby, :mswin] gem 'rake' @@ -13,7 +13,7 @@ group :test do gem 'redcarpet', :platform => :ruby # For fast, Github-like Markdown gem 'kramdown', :platform => :jruby # For Markdown without a C compiler gem 'test-unit' - # This version of sqlite3 required for activerecord 4.2, not more recent. + # This version of sqlite3 required for activerecord 5.0, not more recent. # When bumping AR, may have to/want to adjust this to more recent versions. - gem 'sqlite3', "~> 1.3.0", :platform => [:ruby, :mswin] + gem 'sqlite3', "~> 1.3.6", :platform => [:ruby, :mswin] end From 16cafe160f0227b9c8f2b8ab7a2ae3f0d5b8b4d2 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 13:11:16 -0400 Subject: [PATCH 2/7] avoid ActiveRecord 5.0 deprecations --- lib/oai/provider/model/activerecord_caching_wrapper.rb | 2 +- test/activerecord_provider/database/0001_oaipmh_tables.rb | 2 +- test/activerecord_provider/models/exclusive_set_dc_field.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/oai/provider/model/activerecord_caching_wrapper.rb b/lib/oai/provider/model/activerecord_caching_wrapper.rb index 5b1de80..4faf385 100755 --- a/lib/oai/provider/model/activerecord_caching_wrapper.rb +++ b/lib/oai/provider/model/activerecord_caching_wrapper.rb @@ -111,7 +111,7 @@ def select_partial(token) end def sweep_cache - OaiToken.destroy_all(["created_at < ?", Time.now - expire]) + OaiToken.where(["created_at < ?", Time.now - expire]).destroy_all end def hydrate_records(records) diff --git a/test/activerecord_provider/database/0001_oaipmh_tables.rb b/test/activerecord_provider/database/0001_oaipmh_tables.rb index 0cbad06..ce85642 100755 --- a/test/activerecord_provider/database/0001_oaipmh_tables.rb +++ b/test/activerecord_provider/database/0001_oaipmh_tables.rb @@ -1,4 +1,4 @@ -class OaipmhTables < ActiveRecord::Migration +class OaipmhTables < ActiveRecord::Migration[4.2] def self.up create_table :oai_tokens do |t| t.column :token, :string, :null => false diff --git a/test/activerecord_provider/models/exclusive_set_dc_field.rb b/test/activerecord_provider/models/exclusive_set_dc_field.rb index 45f9079..b892fdd 100644 --- a/test/activerecord_provider/models/exclusive_set_dc_field.rb +++ b/test/activerecord_provider/models/exclusive_set_dc_field.rb @@ -3,7 +3,7 @@ class ExclusiveSetDCField < ActiveRecord::Base def self.sets klass = Struct.new(:name, :spec) - self.uniq.pluck('`set`').compact.map do |spec| + self.distinct.pluck('`set`').compact.map do |spec| klass.new("Set #{spec}", spec) end end From d7cd43a91e30b2f7c71c54d4157f3110de6da924 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 13:55:14 -0400 Subject: [PATCH 3/7] rewrite for clarity and AR 5.1 compat In AR 5.1 find_or_create_by and new_record_before_save? interact differently, to break the original code. (I think maybe it was relying on a bug in new_record_before_save?, I don't really understand what new_record_before_save? is). I believe this is the same logic, rewritten more clearly and in a way that will work in Rails 5.1. Note that find_or_create_by is NOT atomic, in any Rails version. So we should not have introduced any race condition that wasn't there already, we're still creating by soon after find if not present, just a slight intervening check for token.last == 0, shoudn't be much racier than before. --- .../model/activerecord_caching_wrapper.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/oai/provider/model/activerecord_caching_wrapper.rb b/lib/oai/provider/model/activerecord_caching_wrapper.rb index 4faf385..4679f07 100755 --- a/lib/oai/provider/model/activerecord_caching_wrapper.rb +++ b/lib/oai/provider/model/activerecord_caching_wrapper.rb @@ -90,17 +90,16 @@ def next_set(token_string) # select a subset of the result set, and return it with a # resumption token to get the next subset def select_partial(token) - if 0 == token.last - oaitoken = OaiToken.find_or_create_by(token: token.to_s) - if oaitoken.new_record_before_save? - OaiToken.connection.execute("insert into " + - "#{OaiEntry.table_name} (oai_token_id, record_id) " + - "select #{oaitoken.id}, id from #{model.table_name} where " + - "#{OaiToken.sanitize_sql(token_conditions(token))}") - end + oaitoken = OaiToken.find_by(token: token.to_s) + + if 0 == token.last && oaitoken.nil? + oaitoken = OaiToken.create!(token: token.to_s) + OaiToken.connection.execute("insert into " + + "#{OaiEntry.table_name} (oai_token_id, record_id) " + + "select #{oaitoken.id}, id from #{model.table_name} where " + + "#{OaiToken.sanitize_sql(token_conditions(token))}") end - oaitoken = OaiToken.find_by_token(token.to_s) raise ResumptionTokenException.new unless oaitoken PartialResult.new( From ce380088c691e52e0d0376d289c4650ff395ca76 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 13:56:30 -0400 Subject: [PATCH 4/7] test under AR 5.1, green --- Gemfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 64cd216..165e8a8 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ gemspec gem 'jruby-openssl', :platform => :jruby group :test do - gem 'activerecord', '~> 5.0.0' + gem 'activerecord', '~> 5.1.0' gem 'activerecord-jdbcsqlite3-adapter', :platform => [:jruby] gem 'libxml-ruby', :platform => [:ruby, :mswin] gem 'rake' @@ -13,7 +13,7 @@ group :test do gem 'redcarpet', :platform => :ruby # For fast, Github-like Markdown gem 'kramdown', :platform => :jruby # For Markdown without a C compiler gem 'test-unit' - # This version of sqlite3 required for activerecord 5.0, not more recent. - # When bumping AR, may have to/want to adjust this to more recent versions. - gem 'sqlite3', "~> 1.3.6", :platform => [:ruby, :mswin] + + # This version of sqlite3 oughta be good for activerecord 5.1+ hopefully + gem 'sqlite3', ">= 1.4.0", "< 2.0", :platform => [:ruby, :mswin] end From 62e349e7903744cef244f187aff28d6fb49a20f1 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 13:59:42 -0400 Subject: [PATCH 5/7] tests pass under ActiveRecord 5.2 hooray --- Gemfile | 2 +- test/activerecord_provider/config/connection.rb | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 165e8a8..0c8b775 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ gemspec gem 'jruby-openssl', :platform => :jruby group :test do - gem 'activerecord', '~> 5.1.0' + gem 'activerecord', '~> 5.2.0' gem 'activerecord-jdbcsqlite3-adapter', :platform => [:jruby] gem 'libxml-ruby', :platform => [:ruby, :mswin] gem 'rake' diff --git a/test/activerecord_provider/config/connection.rb b/test/activerecord_provider/config/connection.rb index 0849ff5..ed99721 100755 --- a/test/activerecord_provider/config/connection.rb +++ b/test/activerecord_provider/config/connection.rb @@ -12,4 +12,7 @@ ActiveRecord::Migration.verbose = false ActiveRecord::Base.establish_connection :adapter => "sqlite3", :database => ":memory:" -ActiveRecord::Migrator.up File.join(File.dirname(__FILE__), '..', 'database') + +ActiveRecord::MigrationContext.new(File.join(File.dirname(__FILE__), '..', 'database')).migrate + + From 1de46252938f89824b53f575b01efbc2b668432c Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 14:08:19 -0400 Subject: [PATCH 6/7] avoid AR 5.2 deprecation warning --- test/activerecord_provider/models/exclusive_set_dc_field.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/activerecord_provider/models/exclusive_set_dc_field.rb b/test/activerecord_provider/models/exclusive_set_dc_field.rb index b892fdd..31b5cc1 100644 --- a/test/activerecord_provider/models/exclusive_set_dc_field.rb +++ b/test/activerecord_provider/models/exclusive_set_dc_field.rb @@ -3,7 +3,7 @@ class ExclusiveSetDCField < ActiveRecord::Base def self.sets klass = Struct.new(:name, :spec) - self.distinct.pluck('`set`').compact.map do |spec| + self.distinct.pluck(:set).compact.map do |spec| klass.new("Set #{spec}", spec) end end From ad055020b866c8134d720d17124e6ab3acfa3ebd Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Wed, 21 Aug 2019 14:14:42 -0400 Subject: [PATCH 7/7] rails 5.2 requires at least ruby 2.2.2 I don't know if there's a reason for us to test ruby 2.2 at all, but we are. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 673c01b..ee49a0e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: ruby rvm: - - 2.2.1 + - 2.2.10 - 2.5.5 - 2.6.3 matrix: