Merge pull request #102 from code4lib/activerecord-wrapper-table-name-prefix

Prefix column names with model's table name in ActiveRecord wrapper.

Author: barmintor

lib/oai/provider/model/activerecord_wrapper.rb

@@ -32,12 +32,12 @@ def initialize(model, options={})
     end
 
     def earliest
-      earliest_obj = model.order("#{timestamp_field} asc").first
+      earliest_obj = model.order("#{model.base_class.table_name}.#{timestamp_field} asc").first
       earliest_obj.nil? ? Time.at(0) : earliest_obj.send(timestamp_field)
     end
 
     def latest
-      latest_obj = model.order("#{timestamp_field} desc").first
+      latest_obj = model.order("#{model.base_class.table_name}.#{timestamp_field} desc").first
       latest_obj.nil? ? Time.now : latest_obj.send(timestamp_field)
     end
     # A model class is expected to provide a method Model.sets that
@@ -137,7 +137,7 @@ def next_set(find_scope, token_string)
     def select_partial(find_scope, token)
       records = find_scope.where(token_conditions(token))
         .limit(@limit)
-        .order("#{identifier_field} asc")
+        .order("#{model.base_class.table_name}.#{identifier_field} asc")
       raise OAI::ResumptionTokenException.new unless records
 
       total = find_scope.where(token_conditions(token)).count
@@ -158,7 +158,7 @@ def token_conditions(token)
 
       return sql if "0" == last_id
       # Now add last id constraint
-      sql.first << " AND #{identifier_field} > :id"
+      sql.first << " AND #{model.base_class.table_name}.#{identifier_field} > :id"
       sql.last[:id] = last_id
 
       return sql
@@ -169,12 +169,12 @@ def sql_conditions(opts)
       sql = []
       esc_values = {}
       if opts.has_key?(:from)
-        sql << "#{timestamp_field} >= :from"
+        sql << "#{model.base_class.table_name}.#{timestamp_field} >= :from"
         esc_values[:from] = parse_to_local(opts[:from])
       end
       if opts.has_key?(:until)
         # Handle databases which store fractions of a second by rounding up
-        sql << "#{timestamp_field} < :until"
+        sql << "#{model.base_class.table_name}.#{timestamp_field} < :until"
         esc_values[:until] = parse_to_local(opts[:until]) { |t| t + 1 }
       end
 
@@ -215,4 +215,3 @@ def parse_to_local(time)
 
   end
 end
-

test/activerecord_provider/database/0001_oaipmh_tables.rb

@@ -10,6 +10,12 @@ def self.up
       t.column :oai_token_id, :integer, :null => false
     end
 
+    create_table :dc_langs do |t|
+      t.column :name,           :string
+      t.column :updated_at,     :datetime
+      t.column :created_at,     :datetime
+    end
+
     dc_fields = proc do |t|
       t.column  :title,         :string
       t.column  :creator,       :string
@@ -21,7 +27,7 @@ def self.up
       t.column  :type,          :string
       t.column  :format,        :string
       t.column  :source,        :string
-      t.column  :language,      :string
+      t.column  :dc_lang_id,    :integer
       t.column  :relation,      :string
       t.column  :coverage,      :string
       t.column  :rights,        :string

test/activerecord_provider/helpers/providers.rb

@@ -56,11 +56,13 @@ def self.load
       File.join(File.dirname(__FILE__), '..', 'fixtures', 'dc.yml')
     )
     fixtures.keys.sort.each do |key|
-      DCField.create(fixtures[key])
+      lang = DCLang.create(name: fixtures[key].delete('language'))
+      DCField.create(fixtures[key].merge(dc_lang: lang))
     end
   end
 
   def self.unload
     DCField.delete_all
+    DCLang.delete_all
   end
 end

test/activerecord_provider/helpers/transactional_test_case.rb

@@ -19,7 +19,8 @@ def load_fixtures
     )
     disable_logging do
       fixtures.keys.sort.each do |key|
-        DCField.create(fixtures[key])
+        lang = DCLang.create(name: fixtures[key].delete('language'))
+        DCField.create(fixtures[key].merge(dc_lang: lang))
       end
     end
   end

test/activerecord_provider/models/dc_field.rb

@@ -4,4 +4,12 @@ class DCField < ActiveRecord::Base
     :join_table => "dc_fields_dc_sets",
     :foreign_key => "dc_field_id",
     :class_name => "DCSet"
+
+  belongs_to :dc_lang, class_name: "DCLang", optional: true
+
+  default_scope -> { left_outer_joins(:dc_lang) }
+
+  def language
+    dc_lang&.name
+  end
 end

test/activerecord_provider/models/dc_lang.rb

@@ -0,0 +1,3 @@
+class DCLang < ActiveRecord::Base
+  has_many :dc_fields
+end

test/activerecord_provider/models/exclusive_set_dc_field.rb

@@ -8,4 +8,10 @@ def self.sets
     end
   end
 
+  belongs_to :dc_lang, class_name: "DCLang", optional: true
+
+  def language
+    dc_lang&.name
+  end
+
 end

test/activerecord_provider/tc_activerecord_wrapper.rb

@@ -5,50 +5,50 @@ def test_sql_conditions_from_date
     input = "2005-12-25"
     expected = input.dup
     sql_template, sql_opts = sql_conditions(from: input)
-    assert_equal "updated_at >= :from", sql_template
+    assert_equal "dc_fields.updated_at >= :from", sql_template
     assert_equal expected, sql_opts[:from]
     sql_template, sql_opts = sql_conditions(from: Date.strptime(input, "%Y-%m-%d"))
-    assert_equal "updated_at >= :from", sql_template
+    assert_equal "dc_fields.updated_at >= :from", sql_template
     assert_equal expected, sql_opts[:from]
   end
 
   def test_sql_conditions_from_time
     input = "2005-12-25T00:00:00Z"
     expected = "2005-12-25 00:00:00"
     sql_template, sql_opts = sql_conditions(from: input)
-    assert_equal "updated_at >= :from", sql_template
+    assert_equal "dc_fields.updated_at >= :from", sql_template
     assert_equal expected, sql_opts[:from]
     sql_template, sql_opts = sql_conditions(from: Time.strptime(input, "%Y-%m-%dT%H:%M:%S%Z"))
-    assert_equal "updated_at >= :from", sql_template
+    assert_equal "dc_fields.updated_at >= :from", sql_template
     assert_equal expected, sql_opts[:from]
   end
 
   def test_sql_conditions_until_date
     input = "2005-12-25"
     expected = "2005-12-26"
     sql_template, sql_opts = sql_conditions(until: input)
-    assert_equal "updated_at < :until", sql_template
+    assert_equal "dc_fields.updated_at < :until", sql_template
     assert_equal expected, sql_opts[:until]
     sql_template, sql_opts = sql_conditions(until: Date.strptime(input, "%Y-%m-%d"))
-    assert_equal "updated_at < :until", sql_template
+    assert_equal "dc_fields.updated_at < :until", sql_template
     assert_equal expected, sql_opts[:until]
   end
 
   def test_sql_conditions_until_time
     input = "2005-12-25T00:00:00Z"
     expected = "2005-12-25 00:00:01"
     sql_template, sql_opts = sql_conditions(until: input)
-    assert_equal "updated_at < :until", sql_template
+    assert_equal "dc_fields.updated_at < :until", sql_template
     assert_equal expected, sql_opts[:until]
     sql_template, sql_opts = sql_conditions(until: Time.strptime(input, "%Y-%m-%dT%H:%M:%S%Z"))
-    assert_equal "updated_at < :until", sql_template
+    assert_equal "dc_fields.updated_at < :until", sql_template
     assert_equal expected, sql_opts[:until]
   end
 
   def test_sql_conditions_both
     input = "2005-12-25"
     sql_template, sql_opts = sql_conditions(from: input, until: input)
-    assert_equal "updated_at >= :from AND updated_at < :until", sql_template
+    assert_equal "dc_fields.updated_at >= :from AND dc_fields.updated_at < :until", sql_template
   end
 
   def setup

test/activerecord_provider/tc_ar_provider.rb

@@ -81,9 +81,9 @@ def test_deleted
 
   def test_from
     first_id = DCField.order("id asc").first.id
-    DCField.where("id < #{first_id + 90}").update_all(updated_at: Time.parse("January 1 2005"))
+    DCField.where("dc_fields.id < #{first_id + 90}").update_all(updated_at: Time.parse("January 1 2005"))
 
-    DCField.where("id < #{first_id + 10}").update_all(updated_at: Time.parse("June 1 2005"))
+    DCField.where("dc_fields.id < #{first_id + 10}").update_all(updated_at: Time.parse("June 1 2005"))
 
 
     from_param = Time.parse("January 1 2006").getutc.iso8601
@@ -92,7 +92,7 @@ def test_from
       @provider.list_records(
         :metadata_prefix => 'oai_dc', :from => from_param)
     )
-    assert_equal DCField.where(["updated_at >= ?", from_param]).size,
+    assert_equal DCField.where(["dc_fields.updated_at >= ?", from_param]).size,
       doc.elements['OAI-PMH/ListRecords'].size
 
     doc = REXML::Document.new(
@@ -103,8 +103,8 @@ def test_from
   end
 
   def test_until
-    first_id = DCField.order("id asc").first.id
-    DCField.where("id < #{first_id + 10}").update_all(updated_at: Time.parse("June 1 2005"))
+    first_id = DCField.order(id: :asc).first.id
+    DCField.where("dc_fields.id < ?", first_id + 10).update_all(updated_at: Time.parse("June 1 2005"))
 
     doc = REXML::Document.new(
       @provider.list_records(
@@ -114,10 +114,10 @@ def test_until
   end
 
   def test_from_and_until
-    first_id = DCField.order("id asc").first.id
+    first_id = DCField.order(id: :asc).first.id
     DCField.update_all(updated_at: Time.parse("June 1 2005"))
-    DCField.where("id < #{first_id + 50}").update_all(updated_at: Time.parse("June 15 2005"))
-    DCField.where("id < #{first_id + 10}").update_all(updated_at: Time.parse("June 30 2005"))
+    DCField.where("dc_fields.id < ?", first_id + 50).update_all(updated_at: Time.parse("June 15 2005"))
+    DCField.where("dc_fields.id < ?", first_id + 10).update_all(updated_at: Time.parse("June 30 2005"))
 
     doc = REXML::Document.new(
       @provider.list_records(
@@ -129,8 +129,8 @@ def test_from_and_until
   end
 
   def test_bad_until_raises_exception
-    DCField.order('id asc').limit(10).update_all(updated_at: 1.year.ago)
-    DCField.order('id desc').limit(10).update_all(updated_at: 1.year.from_now)
+    DCField.order(id: :asc).limit(10).update_all(updated_at: 1.year.ago)
+    DCField.order(id: :desc).limit(10).update_all(updated_at: 1.year.from_now)
     badTimes = [
       'junk',
       'February 92nd, 2015']
@@ -142,8 +142,8 @@ def test_bad_until_raises_exception
   end
 
   def test_bad_from_raises_exception
-    DCField.order('id asc').limit(10).update_all(updated_at: 1.year.ago)
-    DCField.order('id desc').limit(10).update_all(updated_at: 1.year.from_now)
+    DCField.order(id: :asc).limit(10).update_all(updated_at: 1.year.ago)
+    DCField.order(id: :desc).limit(10).update_all(updated_at: 1.year.from_now)
 
     badTimes = [
       'junk',

test/activerecord_provider/tc_ar_sets_provider.rb

@@ -51,22 +51,22 @@ def define_sets
     set_ab = DCSet.create(:name => "Set A:B", :spec => "A:B")
 
     next_id = 0
-    DCField.limit(10).order("id asc").each do |record|
+    DCField.limit(10).order(id: :asc).each do |record|
       set_a.dc_fields << record
       next_id = record.id
     end
 
-    DCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    DCField.where("dc_fields.id > ?", next_id).limit(10).order(id: :asc).each do |record|
       set_b.dc_fields << record
       next_id = record.id
     end
 
-    DCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    DCField.where("dc_fields.id > ?", next_id).limit(10).order(id: :asc).each do |record|
       set_ab.dc_fields << record
       next_id = record.id
     end
 
-    DCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    DCField.where("dc_fields.id > ?", next_id).limit(10).order(id: :asc).each do |record|
       set_a.dc_fields << record
       set_c.dc_fields << record
       next_id = record.id
@@ -117,25 +117,25 @@ def setup
   def define_sets
     next_id = 0
 
-    ExclusiveSetDCField.limit(10).order("id asc").each do |record|
+    ExclusiveSetDCField.limit(10).order(id: :asc).each do |record|
       record.set = "A"
       record.save!
       next_id = record.id
     end
 
-    ExclusiveSetDCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    ExclusiveSetDCField.where("id > ?", next_id).limit(10).order(id: :asc).each do |record|
       record.set = "B"
       record.save!
       next_id = record.id
     end
 
-    ExclusiveSetDCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    ExclusiveSetDCField.where("id > ?", next_id).limit(10).order(id: :asc).each do |record|
       record.set = "A:B"
       record.save!
       next_id = record.id
     end
 
-    ExclusiveSetDCField.where("id > #{next_id}").limit(10).order("id asc").each do |record|
+    ExclusiveSetDCField.where("id > ?", next_id).limit(10).order(id: :asc).each do |record|
       record.set = "A"
       record.save!
       next_id = record.id
@@ -150,7 +150,8 @@ def load_fixtures
     )
     disable_logging do
       fixtures.keys.sort.each do |key|
-        ExclusiveSetDCField.create(fixtures[key])
+        lang = DCLang.create(name: fixtures[key].delete('language'))
+        ExclusiveSetDCField.create(fixtures[key].merge(dc_lang: lang))
       end
     end
   end