Merge pull request rails#46331 from Shopify/introduce-AR-query-by-configuration

Allow specifying columns to use in ActiveRecord::Base object queries

Author: eileencodes

activerecord/CHANGELOG.md

@@ -1,3 +1,18 @@
+*   Allow configuring columns list to be used in SQL queries issued by an `ActiveRecord::Base` object
+
+    It is now possible to configure columns list that will be used to build an SQL query clauses when
+    updating, deleting or reloading an `ActiveRecord::Base` object
+
+    ```ruby
+    class Developer < ActiveRecord::Base
+      query_constraints :company_id, :id
+    end
+    developer = Developer.first.update(name: "Bob")
+    # => UPDATE "developers" SET "name" = 'Bob' WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+    ```
+
+    *Nikita Vasilevsky*
+
 *   Adds `validate` to foreign keys and check constraints in schema.rb
 
     Previously, `schema.rb` would not record if `validate: false` had been used when adding a foreign key or check

activerecord/lib/active_record.rb

@@ -60,6 +60,7 @@ module ActiveRecord
   autoload :Persistence
   autoload :QueryCache
   autoload :Querying
+  autoload :QueryConstraints
   autoload :QueryLogs
   autoload :ReadonlyAttributes
   autoload :RecordInvalid, "active_record/validations"

activerecord/lib/active_record/locking/optimistic.rb

@@ -91,7 +91,7 @@ def _update_row(attribute_names, attempted_action = "update")
             locking_column = self.class.locking_column
             lock_attribute_was = @attributes[locking_column]
 
-            update_constraints = _primary_key_constraints_hash
+            update_constraints = _query_constraints_hash
             update_constraints[locking_column] = _lock_value_for_database(locking_column)
 
             attribute_names = attribute_names.dup if attribute_names.frozen?
@@ -122,7 +122,7 @@ def destroy_row
 
           locking_column = self.class.locking_column
 
-          delete_constraints = _primary_key_constraints_hash
+          delete_constraints = _query_constraints_hash
           delete_constraints[locking_column] = _lock_value_for_database(locking_column)
 
           affected_rows = self.class._delete_record(delete_constraints)

activerecord/lib/active_record/persistence.rb

@@ -6,6 +6,7 @@ module ActiveRecord
   # = Active Record \Persistence
   module Persistence
     extend ActiveSupport::Concern
+    include QueryConstraints
 
     module ClassMethods
       # Creates an object (or multiple objects) and saves it to the database, if validations pass.
@@ -834,7 +835,7 @@ def update_columns(attributes)
         verify_readonly_attribute(name) || name
       end
 
-      update_constraints = _primary_key_constraints_hash
+      update_constraints = _query_constraints_hash
       attributes = attributes.each_with_object({}) do |(k, v), h|
         h[k] = @attributes.write_cast_value(k, v)
         clear_attribute_change(k)
@@ -1051,8 +1052,12 @@ def apply_scoping?(options)
         (self.class.default_scopes?(all_queries: true) || self.class.global_current_scope)
     end
 
-    def _primary_key_constraints_hash
-      { @primary_key => id_in_database }
+    def _query_constraints_hash
+      return { @primary_key => id_in_database } unless self.class.query_constraints_list
+
+      self.class.query_constraints_list.index_with do |column_name|
+        attribute_in_database(column_name)
+      end
     end
 
     # A hook to be overridden by association modules.
@@ -1064,7 +1069,7 @@ def destroy_row
     end
 
     def _delete_row
-      self.class._delete_record(_primary_key_constraints_hash)
+      self.class._delete_record(_query_constraints_hash)
     end
 
     def _touch_row(attribute_names, time)
@@ -1080,7 +1085,7 @@ def _touch_row(attribute_names, time)
     def _update_row(attribute_names, attempted_action = "update")
       self.class._update_record(
         attributes_with_values(attribute_names),
-        _primary_key_constraints_hash
+        _query_constraints_hash
       )
     end
 

activerecord/lib/active_record/query_constraints.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  module QueryConstraints
+    extend ActiveSupport::Concern
+
+    included do
+      class_attribute :query_constraints_list, instance_writer: false
+    end
+
+    module ClassMethods
+      # Accepts a list of attribute names to be used in the WHERE clause
+      # of SELECT / UPDATE / DELETE queries.
+      #
+      #   class Developer < ActiveRecord::Base
+      #     query_constraints :company_id, :id
+      #   end
+      #
+      #   developer = Developer.first
+      #   developer.inspect # => #<Developer id: 1, company_id: 1, ...>
+      #
+      #   developer.update!(name: "Nikita")
+      #   # => UPDATE "developers" SET "name" = 'Nikita' WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+      #
+      #   It is possible to update attribute used in the query_by clause:
+      #   developer.update!(company_id: 2)
+      #   # => UPDATE "developers" SET "company_id" = 2 WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+      #
+      #   developer.name = "Bob"
+      #   developer.save!
+      #   # => UPDATE "developers" SET "name" = 'Bob' WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+      #
+      #   developer.destroy!
+      #   # => DELETE FROM "developers" WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+      #
+      #   developer.delete
+      #   # => DELETE FROM "developers" WHERE "developers"."company_id" = 1 AND "developers"."id" = 1
+      #
+      #   developer.reload
+      #   # => SELECT "developers".* FROM "developers" WHERE "developers"."company_id" = 1 AND "developers"."id" = 1 LIMIT 1
+      def query_constraints(*columns_list)
+        self.query_constraints_list = columns_list.map(&:to_s)
+      end
+    end
+  end
+end

activerecord/test/cases/persistence_test.rb

@@ -19,9 +19,11 @@
 require "models/ship"
 require "models/admin"
 require "models/admin/user"
+require "models/clothing_item"
 
 class PersistenceTest < ActiveRecord::TestCase
-  fixtures :topics, :companies, :developers, :accounts, :minimalistics, :authors, :author_addresses, :posts, :minivans
+  fixtures :topics, :companies, :developers, :accounts, :minimalistics, :authors, :author_addresses,
+    :posts, :minivans, :clothing_items
 
   def test_update_many
     topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } }
@@ -1339,4 +1341,51 @@ def test_reset_column_information_resets_children
     ActiveRecord::Base.connection.remove_column(:topics, :foo)
     Topic.reset_column_information
   end
+
+  def test_update_uses_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    sql = capture_sql { clothing_item.update(description: "Lovely green t-shirt")  }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+  end
+
+  def test_save_uses_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    clothing_item.description = "Lovely green t-shirt"
+    sql = capture_sql { clothing_item.save }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+  end
+
+  def test_destroy_uses_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    sql = capture_sql { clothing_item.destroy }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+  end
+
+  def test_delete_uses_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    sql = capture_sql { clothing_item.delete }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+  end
+
+  def test_update_attribute_uses_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    sql = capture_sql { clothing_item.update_attribute(:description, "Lovely green t-shirt") }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+  end
+
+  def test_it_is_possible_to_update_parts_of_the_query_constraints_config
+    clothing_item = clothing_items(:green_t_shirt)
+    clothing_item.color = "blue"
+    clothing_item.description = "Now it's a blue t-shirt"
+    sql = capture_sql { clothing_item.save }.first
+    assert_match(/WHERE .*clothing_type/, sql)
+    assert_match(/WHERE .*color/, sql)
+
+    assert_equal("blue", ClothingItem.find_by(id: clothing_item.id).color)
+  end
 end

activerecord/test/cases/query_constraints_test.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require "cases/helper"
+require "models/clothing_item"
+
+class QueryConstraintsTest < ActiveRecord::TestCase
+  def test_primary_key_stays_the_same
+    assert_equal("id", ClothingItem.primary_key)
+  end
+
+  def test_query_constraints_list_is_an_array_of_strings
+    assert_equal(["clothing_type", "color"], ClothingItem.query_constraints_list)
+  end
+end

activerecord/test/fixtures/clothing_items.yml

@@ -0,0 +1,14 @@
+green_pants:
+  color: green
+  clothing_type: pants
+  description: Cool green pants
+
+green_t_shirt:
+  color: green
+  clothing_type: t-shirt
+  description: Cool green t-shirt
+
+red_t_shirt:
+  color: red
+  clothing_type: t-shirt
+  description: Cool red t-shirt

activerecord/test/models/clothing_item.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ClothingItem < ActiveRecord::Base
+  query_constraints :clothing_type, :color
+end

activerecord/test/schema/schema.rb

@@ -232,6 +232,14 @@
     t.references :citation
   end
 
+  create_table :clothing_items, force: true do |t|
+    t.string :clothing_type
+    t.string :color
+    t.text :description
+
+    t.index [:clothing_type, :color], unique: true
+  end
+
   create_table :clubs, force: true do |t|
     t.string :name
     t.integer :category_id