From 4fbd8adf4824d48aea47a57047d90fb6510cee28 Mon Sep 17 00:00:00 2001 From: Christopher Meiklejohn Date: Mon, 14 Mar 2011 22:19:47 -0400 Subject: Don't quote ID's as Arel will quote them -- follow same conventions as the delete method. --- .../lib/active_record/locking/optimistic.rb | 2 +- activerecord/test/cases/locking_test.rb | 36 +++++++++++++++++++++- activerecord/test/fixtures/string_key_objects.yml | 7 +++++ activerecord/test/models/string_key_object.rb | 3 ++ activerecord/test/schema/schema.rb | 6 ++++ 5 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 activerecord/test/fixtures/string_key_objects.yml create mode 100644 activerecord/test/models/string_key_object.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 9a31675782..cdedcde0eb 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -94,7 +94,7 @@ module ActiveRecord relation = self.class.unscoped stmt = relation.where( - relation.table[self.class.primary_key].eq(quoted_id).and( + relation.table[self.class.primary_key].eq(id).and( relation.table[lock_col].eq(quote_value(previous_lock_value)) ) ).arel.compile_update(arel_attributes_values(false, false, attribute_names)) diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 636a709924..61baa55027 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -5,6 +5,7 @@ require 'models/job' require 'models/reader' require 'models/legacy_thing' require 'models/reference' +require 'models/string_key_object' class LockWithoutDefault < ActiveRecord::Base; end @@ -18,7 +19,40 @@ class ReadonlyFirstNamePerson < Person end class OptimisticLockingTest < ActiveRecord::TestCase - fixtures :people, :legacy_things, :references + fixtures :people, :legacy_things, :references, :string_key_objects + + def test_non_integer_lock_existing + s1 = StringKeyObject.find("record1") + s2 = StringKeyObject.find("record1") + assert_equal 0, s1.lock_version + assert_equal 0, s2.lock_version + + s1.name = 'updated record' + s1.save! + assert_equal 1, s1.lock_version + assert_equal 0, s2.lock_version + + s2.name = 'doubly updated record' + assert_raise(ActiveRecord::StaleObjectError) { s2.save! } + end + + def test_non_integer_lock_destroy + s1 = StringKeyObject.find("record1") + s2 = StringKeyObject.find("record1") + assert_equal 0, s1.lock_version + assert_equal 0, s2.lock_version + + s1.name = 'updated record' + s1.save! + assert_equal 1, s1.lock_version + assert_equal 0, s2.lock_version + assert_raise(ActiveRecord::StaleObjectError) { s2.destroy } + + assert s1.destroy + assert s1.frozen? + assert s1.destroyed? + assert_raises(ActiveRecord::RecordNotFound) { StringKeyObject.find("record1") } + end def test_lock_existing p1 = Person.find(1) diff --git a/activerecord/test/fixtures/string_key_objects.yml b/activerecord/test/fixtures/string_key_objects.yml new file mode 100644 index 0000000000..fa1299915b --- /dev/null +++ b/activerecord/test/fixtures/string_key_objects.yml @@ -0,0 +1,7 @@ +first: + id: record1 + name: first record + +second: + id: record2 + name: second record diff --git a/activerecord/test/models/string_key_object.rb b/activerecord/test/models/string_key_object.rb new file mode 100644 index 0000000000..f8d4c6e0e4 --- /dev/null +++ b/activerecord/test/models/string_key_object.rb @@ -0,0 +1,3 @@ +class StringKeyObject < ActiveRecord::Base + set_primary_key :id +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 9479242e4f..c20304c0e2 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -543,6 +543,12 @@ ActiveRecord::Schema.define do t.string :sponsorable_type end + create_table :string_key_objects, :id => false, :primary_key => :id, :force => true do |t| + t.string :id + t.string :name + t.integer :lock_version, :null => false, :default => 0 + end + create_table :students, :force => true do |t| t.string :name end -- cgit v1.2.3