Deprecate set_primary_key in favour of self.primary_key=

activerecord/CHANGELOG.md

@@ -5,6 +5,7 @@
       * `set_table_name`
       * `set_inheritance_column`
       * `set_sequence_name`
+      * `set_primary_key`
 
     Use an assignment method instead. For example, instead of `set_table_name`, use `self.table_name=`:
 

activerecord/lib/active_record/attribute_methods/primary_key.rb

@@ -22,11 +22,11 @@ module ActiveRecord
         end
 
         def reset_primary_key #:nodoc:
-          key = self == base_class ? get_primary_key(base_class.name) :
-            base_class.primary_key
-
-          set_primary_key(key)
-          key
+          if self == base_class
+            self.primary_key = get_primary_key(base_class.name)
+          else
+            self.primary_key = base_class.primary_key
+          end
         end
 
         def get_primary_key(base_name) #:nodoc:
@@ -46,27 +46,33 @@ module ActiveRecord
           end
         end
 
-        attr_accessor :original_primary_key
-
-        # Attribute writer for the primary key column
-        def primary_key=(value)
-          @quoted_primary_key = nil
-          @primary_key = value
+        def original_primary_key #:nodoc:
+          deprecated_original_property_getter :primary_key
         end
 
-        # Sets the name of the primary key column to use to the given value,
-        # or (if the value is nil or false) to the value returned by the given
-        # block.
+        # Sets the name of the primary key column.
         #
         #   class Project < ActiveRecord::Base
-        #     set_primary_key "sysid"
+        #     self.primary_key = "sysid"
         #   end
-        def set_primary_key(value = nil, &block)
+        #
+        # You can also define the primary_key method yourself:
+        #
+        #   class Project < ActiveRecord::Base
+        #     def self.primary_key
+        #       "foo_" + super
+        #     end
+        #   end
+        #   Project.primary_key # => "foo_id"
+        def primary_key=(value)
+          @original_primary_key = @primary_key if defined?(@primary_key)
+          @primary_key          = value && value.to_s
+          @quoted_primary_key   = nil
+        end
+
+        def set_primary_key(value = nil, &block) #:nodoc:
+          deprecated_property_setter :primary_key, value, block
           @quoted_primary_key = nil
-          @primary_key ||= ''
-          self.original_primary_key = @primary_key
-          value &&= value.to_s
-          self.primary_key = block_given? ? instance_eval(&block) : value
         end
       end
     end

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -114,7 +114,7 @@ module ActiveRecord
       #   Defaults to +id+. If <tt>:id</tt> is false this option is ignored.
       #
       #   Also note that this just sets the primary key in the table. You additionally
-      #   need to configure the primary key in the model via the +set_primary_key+ macro.
+      #   need to configure the primary key in the model via +self.primary_key=+.
       #   Models do NOT auto-detect the primary key from their table definition.
       #
       # [<tt>:options</tt>]

activerecord/test/cases/base_test.rb

@@ -1498,17 +1498,41 @@ class BasicsTest < ActiveRecord::TestCase
     k = Class.new( ActiveRecord::Base )
     k.primary_key = "foo"
     assert_equal "foo", k.primary_key
-    k.set_primary_key "bar"
+
+    assert_deprecated do
+      k.set_primary_key "bar"
+    end
     assert_equal "bar", k.primary_key
   end
 
   def test_set_primary_key_with_block
     k = Class.new( ActiveRecord::Base )
     k.primary_key = 'id'
-    k.set_primary_key { "sys_" + original_primary_key }
+
+    assert_deprecated do
+      k.set_primary_key { "sys_" + original_primary_key }
+    end
     assert_equal "sys_id", k.primary_key
   end
 
+  def test_original_primary_key
+    k = Class.new(ActiveRecord::Base)
+    def k.name; "Foo"; end
+    k.primary_key = "bar"
+
+    assert_deprecated do
+      assert_equal "id", k.original_primary_key
+    end
+
+    k = Class.new(ActiveRecord::Base)
+    k.primary_key = "omg"
+    k.primary_key = "wtf"
+
+    assert_deprecated do
+      assert_equal "omg", k.original_primary_key
+    end
+  end
+
   def test_set_inheritance_column_with_value
     k = Class.new( ActiveRecord::Base )
     k.inheritance_column = "foo"

activerecord/test/cases/finder_test.rb

@@ -1163,7 +1163,7 @@ class FinderTest < ActiveRecord::TestCase
   end
 
   def test_find_one_message_with_custom_primary_key
-    Toy.set_primary_key :name
+    Toy.primary_key = :name
     begin
       Toy.find 'Hello World!'
     rescue ActiveRecord::RecordNotFound => e

activerecord/test/cases/primary_keys_test.rb

@@ -142,8 +142,6 @@ class PrimaryKeysTest < ActiveRecord::TestCase
     assert_equal k.connection.quote_column_name("id"), k.quoted_primary_key
     k.primary_key = "foo"
     assert_equal k.connection.quote_column_name("foo"), k.quoted_primary_key
-    k.set_primary_key "bar"
-    assert_equal k.connection.quote_column_name("bar"), k.quoted_primary_key
   end
 end
 
@@ -155,9 +153,8 @@ class PrimaryKeyWithNoConnectionTest < ActiveRecord::TestCase
 
     connection = ActiveRecord::Base.remove_connection
 
-    model = Class.new(ActiveRecord::Base) do
-      set_primary_key 'foo'
-    end
+    model = Class.new(ActiveRecord::Base)
+    model.primary_key = 'foo'
 
     assert_equal 'foo', model.primary_key
 

activerecord/test/models/country.rb

@@ -1,6 +1,6 @@
 class Country < ActiveRecord::Base
 
-  set_primary_key :country_id
+  self.primary_key = :country_id
 
   has_and_belongs_to_many :treaties
 

activerecord/test/models/dashboard.rb

@@ -1,3 +1,3 @@
 class Dashboard < ActiveRecord::Base
-  set_primary_key :dashboard_id
-end
+  self.primary_key = :dashboard_id
+end

activerecord/test/models/keyboard.rb

@@ -1,3 +1,3 @@
 class Keyboard < ActiveRecord::Base
-  set_primary_key 'key_number'
+  self.primary_key = 'key_number'
 end

activerecord/test/models/minivan.rb

@@ -1,5 +1,5 @@
 class Minivan < ActiveRecord::Base
-  set_primary_key :minivan_id
+  self.primary_key = :minivan_id
 
   belongs_to :speedometer
   has_one :dashboard, :through => :speedometer

activerecord/test/models/mixed_case_monkey.rb

@@ -1,3 +1,3 @@
 class MixedCaseMonkey < ActiveRecord::Base
-  set_primary_key 'monkeyID'
+  self.primary_key = 'monkeyID'
 end

activerecord/test/models/owner.rb

@@ -1,5 +1,5 @@
 class Owner < ActiveRecord::Base
-  set_primary_key :owner_id
+  self.primary_key = :owner_id
   has_many :pets
   has_many :toys, :through => :pets
 end

activerecord/test/models/pet.rb

@@ -2,7 +2,7 @@ class Pet < ActiveRecord::Base
 
   attr_accessor :current_user
 
-  set_primary_key :pet_id
+  self.primary_key = :pet_id
   belongs_to :owner, :touch => true
   has_many :toys
 

activerecord/test/models/speedometer.rb

@@ -1,4 +1,4 @@
 class Speedometer < ActiveRecord::Base
-  set_primary_key :speedometer_id
+  self.primary_key = :speedometer_id
   belongs_to :dashboard
-end
+end

activerecord/test/models/string_key_object.rb

@@ -1,3 +1,3 @@
 class StringKeyObject < ActiveRecord::Base
-  set_primary_key :id
+  self.primary_key = :id
 end

activerecord/test/models/subscriber.rb

@@ -1,5 +1,5 @@
 class Subscriber < ActiveRecord::Base
-  set_primary_key 'nick'
+  self.primary_key = 'nick'
   has_many :subscriptions
   has_many :books, :through => :subscriptions
 end

activerecord/test/models/toy.rb

@@ -1,5 +1,5 @@
 class Toy < ActiveRecord::Base
-  set_primary_key :toy_id
+  self.primary_key = :toy_id
   belongs_to :pet
 
   scope :with_pet, joins(:pet)

activerecord/test/models/treaty.rb

@@ -1,6 +1,6 @@
 class Treaty < ActiveRecord::Base
 
-  set_primary_key :treaty_id
+  self.primary_key = :treaty_id
 
   has_and_belongs_to_many :countries