Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define ActiveRecord::Base#id API for composite primary key models #47625

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions activerecord/lib/active_record/attribute_methods/primary_key.rb
Expand Up @@ -16,7 +16,9 @@ def to_key

# Returns the primary key column's value.
def id
_read_attribute(@primary_key)
return _read_attribute(@primary_key) unless @primary_key.is_a?(Array)

@primary_key.map { |pk| _read_attribute(pk) }
end

# Sets the primary key column's value.
Expand Down Expand Up @@ -120,12 +122,20 @@ def get_primary_key(base_name) # :nodoc:
#
# Project.primary_key # => "foo_id"
def primary_key=(value)
@primary_key = value && -value.to_s
@primary_key = derive_primary_key(value)
@quoted_primary_key = nil
@attributes_builder = nil
end

private
def derive_primary_key(value)
return unless value

return -value.to_s unless value.is_a?(Array)

value.map { |v| -v.to_s }.freeze
end

def inherited(base)
super
base.class_eval do
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/attribute_methods/read.rb
Expand Up @@ -29,7 +29,7 @@ def read_attribute(attr_name, &block)
name = attr_name.to_s
name = self.class.attribute_aliases[name] || name

name = @primary_key if name == "id" && @primary_key
name = @primary_key if name == "id" && @primary_key && !@primary_key.is_a?(Array)
@attributes.fetch_value(name, &block)
end

Expand Down
3 changes: 2 additions & 1 deletion activerecord/lib/active_record/fixtures.rb
Expand Up @@ -769,7 +769,8 @@ def [](key)
def find
raise FixtureClassNotFound, "No class attached to find." unless model_class
object = model_class.unscoped do
model_class.find(fixture[model_class.primary_key])
pk_clauses = fixture.slice(*Array(model_class.primary_key))
model_class.find_by!(pk_clauses)
end
# Fixtures can't be eagerly loaded
object.instance_variable_set(:@strict_loading, false)
Expand Down
16 changes: 16 additions & 0 deletions activerecord/test/cases/primary_keys_test.rb
Expand Up @@ -10,6 +10,7 @@
require "models/mixed_case_monkey"
require "models/dashboard"
require "models/non_primary_key"
require "models/cpk"

class PrimaryKeysTest < ActiveRecord::TestCase
fixtures :topics, :subscribers, :movies, :mixed_case_monkeys
Expand Down Expand Up @@ -331,6 +332,8 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase

self.use_transactional_tests = false

fixtures :cpk_books, :cpk_orders

def setup
@connection = ActiveRecord::Base.connection
@connection.schema_cache.clear!
Expand Down Expand Up @@ -387,6 +390,19 @@ def test_dumping_composite_primary_key_out_of_order
schema = dump_table_schema "barcodes_reverse"
assert_match %r{create_table "barcodes_reverse", primary_key: \["code", "region"\]}, schema
end

def test_model_with_a_composite_primary_key
assert_equal(["author_id", "number"], Cpk::Book.primary_key)
assert_equal(["shop_id", "id"], Cpk::Order.primary_key)
end

def test_id_is_not_defined_on_a_model_with_composite_primary_key
book = cpk_books(:cpk_great_author_first_book)
order = cpk_orders(:cpk_groceries_order_1)

assert_equal([book.author_id, book.number], book.id)
assert_equal([order.shop_id, order.read_attribute(:id)], order.id)
end
end

class PrimaryKeyIntegerNilDefaultTest < ActiveRecord::TestCase
Expand Down
20 changes: 20 additions & 0 deletions activerecord/test/fixtures/cpk_books.yml
@@ -0,0 +1,20 @@
_fixture:
model_class: Cpk::Book

cpk_great_author_first_book:
author_id: 1
number: 1
title: "The first book"
revision: 1

cpk_great_author_second_book:
author_id: 1
number: 2
title: "The second book"
revision: 1

cpk_famous_author_first_book:
author_id: 2
number: 1
title: "Ruby on Rails"
revision: 1
17 changes: 17 additions & 0 deletions activerecord/test/fixtures/cpk_orders.yml
@@ -0,0 +1,17 @@
_fixture:
model_class: Cpk::Order

cpk_groceries_order_1:
id: 1
shop_id: 1
status: "paid"

cpk_groceries_order_2:
id: 2
shop_id: 1
status: "paid"

cpk_coffee_order_1:
id: 3
shop_id: 2
status: "cancelled"
4 changes: 4 additions & 0 deletions activerecord/test/models/cpk.rb
@@ -0,0 +1,4 @@
# frozen_string_literal: true

require_relative "cpk/book"
require_relative "cpk/order"
8 changes: 8 additions & 0 deletions activerecord/test/models/cpk/book.rb
@@ -0,0 +1,8 @@
# frozen_string_literal: true

module Cpk
class Book < ActiveRecord::Base
self.table_name = :cpk_books
self.primary_key = [:author_id, :number]
end
end
8 changes: 8 additions & 0 deletions activerecord/test/models/cpk/order.rb
@@ -0,0 +1,8 @@
# frozen_string_literal: true

module Cpk
class Order < ActiveRecord::Base
self.table_name = :cpk_orders
self.primary_key = [:shop_id, :id]
end
end
13 changes: 13 additions & 0 deletions activerecord/test/schema/schema.rb
Expand Up @@ -239,6 +239,19 @@
t.references :citation
end

create_table :cpk_books, primary_key: [:author_id, :number], force: true do |t|
t.integer :author_id
t.integer :number
t.string :title
t.integer :revision
end

create_table :cpk_orders, primary_key: [:shop_id, :id], force: true do |t|
t.integer :shop_id
t.integer :id
t.string :status
end

create_table :clothing_items, force: true do |t|
t.string :clothing_type
t.string :color
Expand Down