From: Aredridel Date: 2004-06-05T02:34:03+09:00 Subject: Re: [ANN] Active Record 0.8.2: Inheritable callback ques --=-gY1zHbz8GZv34OhNggLD Content-Type: text/plain Content-Transfer-Encoding: 7bit Raphael Bauduin wrote: > I just tested it and it seems really easy to link Classes to tables. > However, I encountered > a blocking problem: it seems the column that's the primary key has to > be named id. From the code > in the docs > (http://ar.rubyonrails.org/classes/ActiveRecord/Base.html#M000057) it > seems this is hardcoded. > Is it planned to have the name of the "id column" extracted from the > database, or at least to let the user specify a name. I have none of > my columns name id.... :-) Attached is a patch that adds a primary key class method, and alters the code to use it: class User < ActiveRecord::Base def self.primary_key "userid" end end User.find("jdoe") It removes support for automatically calling to_i on key parameters, and instead passes them to the database as strings, so it may interpret as it sees fit. Also added is a little extra debugging for the internal calls to eval. Ari --=-gY1zHbz8GZv34OhNggLD Content-Disposition: attachment; filename=ar-pkey.patch Content-Type: text/x-patch; name=ar-pkey.patch; charset=utf-8 Content-Transfer-Encoding: 7bit ? .svn ? Rakefile ? debug.log ? doc ? dev-utils/.svn ? diagrams/.svn ? diagrams/activeRecordSketch.gif ? examples/.svn ? lib/.svn ? lib/active_record/.svn ? lib/active_record/connection_adapters/.svn ? lib/active_record/support/.svn ? lib/active_record/value_objects/.svn ? lib/active_record/vendor/.svn ? test/.svn ? test/debug.log ? test/connections/.svn ? test/connections/dbi_mysql/.svn ? test/connections/mysql_ruby/.svn ? test/connections/native_mysql/.svn ? test/connections/native_postgresql/.svn ? test/connections/native_sqlite/.svn ? test/connections/ruby_mysql/.svn ? test/fixtures/.svn ? test/fixtures/accounts/.svn ? test/fixtures/companies/.svn ? test/fixtures/customers/.svn ? test/fixtures/db_definitions/.svn ? test/fixtures/developer_project/.svn ? test/fixtures/developers/.svn ? test/fixtures/developers_projects/.svn ? test/fixtures/projects/.svn ? test/fixtures/topics/.svn Index: dev-utils/eval_debugger.rb =================================================================== RCS file: /var/cvs/activerecord/activerecord/dev-utils/eval_debugger.rb,v retrieving revision 1.4 diff -u -r1.4 eval_debugger.rb --- dev-utils/eval_debugger.rb 30 May 2004 15:19:55 -0000 1.4 +++ dev-utils/eval_debugger.rb 4 Jun 2004 17:30:04 -0000 @@ -3,7 +3,7 @@ class Module alias :old_module_eval :module_eval def module_eval(*args, &block) - $stderr.puts("\n#{self.name} is 'module_eval'ing:\n#{args[0]}") if args[0] + puts("in #{self.name}, #{if args[1] then "file #{args[1]}" end} #{if args[2] then "on line #{args[2]}" end}:\n#{args[0]}") if args[0] old_module_eval(*args, &block) end end Index: lib/active_record/associations.rb =================================================================== RCS file: /var/cvs/activerecord/activerecord/lib/active_record/associations.rb,v retrieving revision 1.28 diff -u -r1.28 associations.rb --- lib/active_record/associations.rb 3 Jun 2004 09:43:45 -0000 1.28 +++ lib/active_record/associations.rb 4 Jun 2004 17:30:04 -0000 @@ -97,14 +97,14 @@ collection_finder = "#{collection_class_name}.find_by_sql(\"#{options[:finder_sql]}\")" collection_counter = "#{collection_class_name}.count_by_sql(\"#{counter_sql}\")" else - collection_finder = <<-end_eval + collection_finder = <<-"end_eval" #{collection_class_name}.find_all( - "#{class_primary_key_name} = \#{id}#{options[:conditions] ? " AND " + options[:conditions] : ""}", + "#{class_primary_key_name} = '\#{id}'#{options[:conditions] ? " AND " + options[:conditions] : ""}", #{options[:order] ? "\"" + options[:order] + "\"" : "nil" } ) end_eval - collection_counter = "#{collection_class_name}.count(\"#{class_primary_key_name} = \#{id}\")" + collection_counter = "#{collection_class_name}.count(\"#{class_primary_key_name} = '\#{id}'\")" end has_collection_method(collection_name) @@ -203,12 +203,12 @@ association_name, association_class_name, class_primary_key_name = associate_identification(association_id, options[:class_name], options[:foreign_key]) - association_class_primary_key_name = options[:foreign_key] || association_class_name.downcase + "_id" - + association_class_primary_key_name = options[:foreign_key] || association_class_name.gsub(/^.*::/, '').downcase! + "_id" + if options[:remote] association_finder = <<-"end_eval" #{association_class_name}.find_first( - "#{class_primary_key_name} = \#{id}#{options[:conditions] ? " AND " + options[:conditions] : ""}", + "#{class_primary_key_name} = '\#{id}'#{options[:conditions] ? " AND " + options[:conditions] : ""}", #{options[:order] ? "\"" + options[:order] + "\"" : "nil" } ) end_eval @@ -275,7 +275,7 @@ associate_identification(association_id, options[:class_name], options[:foreign_key]) - association_foreign_key = options[:foreign_key] || association_class_name.downcase + "_id" + association_foreign_key = options[:foreign_key] || association_class_name.gsub(/^.*::/, '').downcase + "_id" association_table_name = options[:table_name] || table_name(association_class_name) my_key = options[:key] || name.downcase + "_id" @@ -322,11 +322,11 @@ end def associate_identification(association_id, association_class_name, foreign_key) - return association_id.id2name, (association_class_name || class_name(table_name_prefix + association_id.id2name + table_name_suffix)), (foreign_key || name.downcase + "_id") + return association_id.id2name, (association_class_name || class_name(table_name_prefix + association_id.id2name + table_name_suffix)), (foreign_key || name.gsub(/.*::/, '').downcase + "_id") end def collection_reader_method(collection_name, collection_finder) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{collection_name}(force_reload = false) if @#{collection_name}.nil? || force_reload @#{collection_name} = #{collection_finder} @@ -338,7 +338,7 @@ end def has_collection_method(collection_name) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def has_#{collection_name}?(force_reload = false) #{collection_name}(force_reload).length > 0 end @@ -346,7 +346,7 @@ end def collection_count_method(collection_name, collection_counter) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{collection_name}_count(force_reload = false) if @#{collection_name}.nil? || force_reload #{collection_counter} @@ -358,7 +358,7 @@ end def association_reader_method(association_name, association_finder) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{association_name}(force_reload = false) if @#{association_name}.nil? || force_reload @#{association_name} = #{association_finder} @@ -370,7 +370,7 @@ end def association_comparison_method(association_name, association_class_name) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{association_name}?(comparison_object, force_reload = false) if comparison_object.kind_of?(#{association_class_name}) begin @@ -386,7 +386,7 @@ end def has_association_method(association_name) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def has_#{association_name}?(force_reload = false) begin !#{association_name}(force_reload).nil? @@ -398,7 +398,7 @@ end def build_method(method_prefix, collection_name, collection_class_name, class_primary_key_name) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{method_prefix + collection_name}(attributes = {}) association = #{collection_class_name}.new association.attributes = attributes.merge({ "#{class_primary_key_name}" => id}) @@ -408,7 +408,7 @@ end def create_method(method_prefix, collection_name, collection_class_name, class_primary_key_name) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def #{method_prefix + collection_name}(attributes = {}) association = #{collection_class_name}.new association.attributes = attributes.merge({ "#{class_primary_key_name}" => id}) @@ -419,17 +419,17 @@ end def find_in_collection_method(collection_name, collection_class_name, class_primary_key_name, conditions = nil) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def find_in_#{collection_name}(association_id) - #{collection_class_name}.find_on_conditions( - association_id, "#{class_primary_key_name} = \#{id}#{conditions ? " AND " + conditions : ""}" + #{collection_class_name}.find_with_constraint( + association_id, "#{class_primary_key_name} = '\#{id}'#{conditions ? " AND " + conditions : ""}" ) end end_eval end def add_association_relation(association_name, insert_sql) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def add_#{association_name}(*items) items.flatten.each { |item| connection.insert "#{insert_sql}" } @#{association_name} = nil @@ -438,7 +438,7 @@ end def remove_association_relation(association_name, foreign_key, delete_sql) - module_eval <<-end_eval + module_eval <<-"end_eval", __FILE__, __LINE__ def remove_#{association_name}(*items) if items.flatten.length < 1 connection.delete "#{delete_sql}" Index: lib/active_record/base.rb =================================================================== RCS file: /var/cvs/activerecord/activerecord/lib/active_record/base.rb,v retrieving revision 1.30 diff -u -r1.30 base.rb --- lib/active_record/base.rb 3 Jun 2004 16:50:21 -0000 1.30 +++ lib/active_record/base.rb 4 Jun 2004 17:30:05 -0000 @@ -98,8 +98,8 @@ ids = [ ids ].flatten if ids.length > 1 - ids_list = ids.map{ |id| id.to_i }.join(", ") - objects = find_all("id IN (#{ids_list})", "id") + ids_list = ids.map{ |id| "'#{id}'" }.join(", ") + objects = find_all("#{primary_key} IN (#{ids_list})", "id") if objects.length == ids.length return objects @@ -108,7 +108,7 @@ end else id = ids.first - sql = "SELECT * FROM #{table_name} WHERE id = #{id.to_i} " + sql = "SELECT * FROM #{table_name} WHERE #{primary_key} = '#{id}'" sql << "AND type = '#{name}'" unless descents_from_active_record? if record = connection.select_one(sql, "#{name} Find") @@ -124,8 +124,8 @@ # Example: # Person.find_on_conditions 5, "first_name LIKE '%dav%' AND last_name = 'heinemeier'" def find_on_conditions(id, conditions) - find_first("id = #{id.to_i} AND #{sanitize_conditions(conditions)}") || - raise(RecordNotFound, "Couldn't find #{name} with ID = #{id} on the condition of #{conditions}") + find_first("#{primary_key} = '#{id}'} AND #{sanitize_conditions(conditions)}") || + raise(RecordNotFound, "Couldn't find #{name} with #{primary_key} = #{id} on the condition of #{conditions}") end # Returns an array of all the objects that could be instantiated from the associated @@ -133,8 +133,9 @@ # such as by "color = 'red'", and arrangement of the selection can be done through +orderings+ (ORDER BY-part), # such as by "last_name, first_name DESC". A maximum of returned objects can be specified in +limit+. Example: # Project.find_all "category = 'accounts'", "last_accessed DESC", 15 - def find_all(conditions = nil, orderings = nil, limit = nil) + def find_all(conditions = nil, orderings = nil, limit = nil, joins = nil) sql = "SELECT * FROM #{table_name} " + sql << "#{joins} " if joins add_conditions!(sql, conditions) sql << "ORDER BY #{orderings} " unless orderings.nil? sql << "LIMIT #{limit} " unless limit.nil? @@ -286,12 +287,18 @@ table_name_prefix + undecorated_table_name(class_name) + table_name_suffix end + # Defines the primary key field -- can be overridden in subclasses. + def primary_key + "id" + end + # Turns the +table_name+ back into a class name following the reverse rules of +table_name+. def class_name(table_name) # :nodoc: # remove any prefix and/or suffix from the table name class_name = table_name[table_name_prefix.length..-(table_name_suffix.length + 1)] class_name = class_name.capitalize.gsub(/_(.)/) { |s| $1.capitalize } + class_name.gsub!(/.*::/, '') if pluralize_table_names if class_name[-3,3] == "ies" @@ -342,7 +349,7 @@ # Guesses the table name, but does not decorate it with prefix and suffix information. def undecorated_table_name(class_name = class_name_of_active_record_descendant(self)) - table_name = class_name.gsub(/([a-z])([A-Z])/, '\1_\2').downcase + table_name = class_name.gsub(/.*::/, '').gsub(/([a-z])([A-Z])/, '\1_\2').downcase if pluralize_table_names case table_name[-1,1] @@ -393,11 +400,12 @@ # Every Active Record class must use "id" as their primary ID. This getter overwrites the native # id method, which isn't being used in this context. def id - @attributes["id"] + @attributes[self.class.primary_key] end - def id=(value) # :nodoc: - @attributes["id"] = value.to_i + # Sets the primary ID. + def id=(value) + @attributes[self.class.primary_key] = value end # Returns true if this object hasn't been saved yet -- that is, a record for the object doesn't exist yet. @@ -415,7 +423,7 @@ # Deletes the record in the database and freezes this instance to reflect that no changes should # be made (since they can't be persisted). def destroy - connection.delete("DELETE FROM #{self.class.table_name} WHERE id = #{id}", "#{self.class.name} Destroy") unless new_record? + connection.delete("DELETE FROM #{self.class.table_name} WHERE #{self.class.primary_key} = '#{id}'", "#{self.class.name} Destroy") unless new_record? freeze end @@ -457,7 +465,7 @@ connection.update( "UPDATE #{self.class.table_name} " + "SET #{comma_pair_list(attributes_with_quotes)} " + - "WHERE id = #{id}", + "WHERE #{self.class.primary_key} = '#{id}'", "#{self.class.name} Update" ) end @@ -577,4 +585,4 @@ hash.inject([]) { |list, pair| list << "#{pair.first} = #{pair.last}" }.join(", ") end end -end \ No newline at end of file +end Index: test/associations_test.rb =================================================================== RCS file: /var/cvs/activerecord/activerecord/test/associations_test.rb,v retrieving revision 1.16 diff -u -r1.16 associations_test.rb --- test/associations_test.rb 3 Jun 2004 09:42:12 -0000 1.16 +++ test/associations_test.rb 4 Jun 2004 17:30:05 -0000 @@ -212,11 +212,12 @@ aridridel.add_projects([ Project.find(1), Project.find(2) ]) assert_equal 2, aridridel.projects_count end - - def xtest_module_spanning_associations + + def test_module_spanning_associations assert MyApplication::Business::Firm.find_first.has_clients? firm = MyApplication::Business::Firm.find_first + assert_nil firm.class.table_name.match('::') assert_equal 2, MyApplication::Business::firm.clients_count assert MyApplication::Billing::Account.find(1).has_firm?, "37signals account should be able to backtrack" end -end \ No newline at end of file +end --=-gY1zHbz8GZv34OhNggLD--