diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1e901f0..ef68b81 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -2,9 +2,6 @@ name: Ruby on: push: - branches: - - main - pull_request: jobs: @@ -35,6 +32,7 @@ jobs: --health-retries 5 strategy: + fail-fast: false matrix: ruby: - 3.1 @@ -48,5 +46,7 @@ jobs: with: ruby-version: ${{ matrix.ruby }} bundler-cache: true - - name: Run tests - run: bundle exec rspec + - name: Install Appraisals + run: bundle exec appraisal install + - name: Run tests for ruby ${{ matrix.ruby }} + run: bundle exec appraisal rspec diff --git a/.gitignore b/.gitignore index e66038d..98ccab7 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ /tmp/ /.idea/ .env -/spec/internal/log \ No newline at end of file +/spec/internal/log +/gemfiles/ \ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml index 07c78e1..23d717b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,3 +4,6 @@ inherit_gem: AllCops: TargetRubyVersion: 3.1 + +Gp/OptArgParameters: + Enabled: false diff --git a/Appraisals b/Appraisals new file mode 100644 index 0000000..dbb065d --- /dev/null +++ b/Appraisals @@ -0,0 +1,7 @@ +appraise "activerecord-6.1" do + gem "activerecord", "~> 6.1" +end + +appraise "activerecord-7.1" do + gem "activerecord", ">= 7.1" +end diff --git a/Gemfile.lock b/Gemfile.lock index e2938ff..17b2fc1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/corp-gp/rubocop-gp.git - revision: e5bd900ce4b10077cf8f4ae88a5a6e32e7861b65 + revision: 1039b83ebd2f8334df6175d410f9a1b025d97384 specs: rubocop-gp (0.0.4) rubocop @@ -15,153 +15,220 @@ PATH remote: . specs: update_values_all (1.1.0) - activerecord (>= 4.2) GEM remote: https://rubygems.org/ specs: - actionpack (7.0.4.3) - actionview (= 7.0.4.3) - activesupport (= 7.0.4.3) - rack (~> 2.0, >= 2.2.0) + actionpack (7.2.2.1) + actionview (= 7.2.2.1) + activesupport (= 7.2.2.1) + nokogiri (>= 1.8.5) + racc + rack (>= 2.2.4, < 3.2) + rack-session (>= 1.0.1) rack-test (>= 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.2.0) - actionview (7.0.4.3) - activesupport (= 7.0.4.3) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + useragent (~> 0.16) + actionview (7.2.2.1) + activesupport (= 7.2.2.1) builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.1, >= 1.2.0) - activemodel (7.0.4.3) - activesupport (= 7.0.4.3) - activerecord (7.0.4.3) - activemodel (= 7.0.4.3) - activesupport (= 7.0.4.3) - activesupport (7.0.4.3) - concurrent-ruby (~> 1.0, >= 1.0.2) + erubi (~> 1.11) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + activesupport (7.2.2.1) + base64 + benchmark (>= 0.3) + bigdecimal + concurrent-ruby (~> 1.0, >= 1.3.1) + connection_pool (>= 2.2.5) + drb i18n (>= 1.6, < 2) + logger (>= 1.4.2) minitest (>= 5.1) - tzinfo (~> 2.0) - ast (2.4.2) - builder (3.2.4) - combustion (1.3.7) + securerandom (>= 0.3) + tzinfo (~> 2.0, >= 2.0.5) + appraisal (2.5.0) + bundler + rake + thor (>= 0.14.0) + ast (2.4.3) + base64 (0.2.0) + benchmark (0.4.0) + bigdecimal (3.1.9) + builder (3.3.0) + combustion (1.5.0) activesupport (>= 3.0.0) railties (>= 3.0.0) thor (>= 0.14.6) - concurrent-ruby (1.3.4) + concurrent-ruby (1.3.5) + connection_pool (2.5.0) crass (1.0.6) - diff-lcs (1.5.0) - dotenv (2.8.1) - dotenv-rails (2.8.1) - dotenv (= 2.8.1) - railties (>= 3.2) - erubi (1.12.0) - i18n (1.14.5) + date (3.4.1) + diff-lcs (1.6.1) + dotenv (3.1.7) + dotenv-rails (3.1.7) + dotenv (= 3.1.7) + railties (>= 6.1) + drb (2.2.1) + erubi (1.13.1) + i18n (1.14.7) concurrent-ruby (~> 1.0) - json (2.7.2) - language_server-protocol (3.17.0.3) - lefthook (1.3.8) - loofah (2.19.1) + io-console (0.8.0) + irb (1.15.2) + pp (>= 0.6.0) + rdoc (>= 4.0.0) + reline (>= 0.4.2) + json (2.10.2) + language_server-protocol (3.17.0.4) + lefthook (1.11.8) + lint_roller (1.1.0) + logger (1.7.0) + loofah (2.24.0) crass (~> 1.0.2) - nokogiri (>= 1.5.9) - method_source (1.0.0) - minitest (5.25.1) - nokogiri (1.16.7-arm64-darwin) + nokogiri (>= 1.12.0) + minitest (5.25.5) + nokogiri (1.18.7-aarch64-linux-gnu) + racc (~> 1.4) + nokogiri (1.18.7-aarch64-linux-musl) + racc (~> 1.4) + nokogiri (1.18.7-arm-linux-gnu) + racc (~> 1.4) + nokogiri (1.18.7-arm-linux-musl) racc (~> 1.4) - nokogiri (1.16.7-x86_64-linux) + nokogiri (1.18.7-arm64-darwin) + racc (~> 1.4) + nokogiri (1.18.7-x86_64-darwin) + racc (~> 1.4) + nokogiri (1.18.7-x86_64-linux-gnu) + racc (~> 1.4) + nokogiri (1.18.7-x86_64-linux-musl) racc (~> 1.4) parallel (1.26.3) - parser (3.3.4.2) + parser (3.3.7.4) ast (~> 2.4.1) racc - pg (1.4.6) + pg (1.5.9) + pp (0.6.2) + prettyprint + prettyprint (0.2.0) + prism (1.4.0) + psych (5.2.3) + date + stringio racc (1.8.1) - rack (2.2.9) - rack-test (2.1.0) + rack (3.1.12) + rack-session (2.1.0) + base64 (>= 0.1.0) + rack (>= 3.0.0) + rack-test (2.2.0) rack (>= 1.3) - rails-dom-testing (2.0.3) - activesupport (>= 4.2.0) + rackup (2.2.1) + rack (>= 3) + rails-dom-testing (2.2.0) + activesupport (>= 5.0.0) + minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.5.0) - loofah (~> 2.19, >= 2.19.1) - railties (7.0.4.3) - actionpack (= 7.0.4.3) - activesupport (= 7.0.4.3) - method_source + rails-html-sanitizer (1.6.2) + loofah (~> 2.21) + nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) + railties (7.2.2.1) + actionpack (= 7.2.2.1) + activesupport (= 7.2.2.1) + irb (~> 1.13) + rackup (>= 1.0.0) rake (>= 12.2) - thor (~> 1.0) - zeitwerk (~> 2.5) + thor (~> 1.0, >= 1.2.2) + zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.0.6) - regexp_parser (2.9.2) - rexml (3.3.6) - strscan - rspec (3.11.0) - rspec-core (~> 3.11.0) - rspec-expectations (~> 3.11.0) - rspec-mocks (~> 3.11.0) - rspec-core (3.11.0) - rspec-support (~> 3.11.0) - rspec-expectations (3.11.0) + rake (13.2.1) + rdoc (6.13.1) + psych (>= 4.0.0) + regexp_parser (2.10.0) + reline (0.6.1) + io-console (~> 0.5) + rspec (3.13.0) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.3) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.3) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-mocks (3.11.1) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.2) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-rails (6.0.1) - actionpack (>= 6.1) - activesupport (>= 6.1) - railties (>= 6.1) - rspec-core (~> 3.11) - rspec-expectations (~> 3.11) - rspec-mocks (~> 3.11) - rspec-support (~> 3.11) - rspec-support (3.11.0) - rubocop (1.65.1) + rspec-support (~> 3.13.0) + rspec-rails (7.1.1) + actionpack (>= 7.0) + activesupport (>= 7.0) + railties (>= 7.0) + rspec-core (~> 3.13) + rspec-expectations (~> 3.13) + rspec-mocks (~> 3.13) + rspec-support (~> 3.13) + rspec-support (3.13.2) + rubocop (1.75.2) json (~> 2.3) - language_server-protocol (>= 3.17.0) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 2.4, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.31.1, < 2.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.44.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.32.1) - parser (>= 3.3.1.0) - rubocop-capybara (2.21.0) - rubocop (~> 1.41) - rubocop-factory_bot (2.26.1) - rubocop (~> 1.61) - rubocop-performance (1.21.1) - rubocop (>= 1.48.1, < 2.0) - rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rails (2.26.0) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.44.0) + parser (>= 3.3.7.2) + prism (~> 1.4) + rubocop-capybara (2.22.1) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-factory_bot (2.27.1) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-performance (1.25.0) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rails (2.31.0) activesupport (>= 4.2.0) + lint_roller (~> 1.1) rack (>= 1.1) - rubocop (>= 1.52.0, < 2.0) - rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rspec (3.0.4) - rubocop (~> 1.61) - rubocop-rspec_rails (2.30.0) - rubocop (~> 1.61) - rubocop-rspec (~> 3, >= 3.0.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rspec (3.5.0) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-rspec_rails (2.31.0) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-rspec (~> 3.5) ruby-progressbar (1.13.0) - strscan (3.1.0) - thor (1.2.1) + securerandom (0.4.1) + stringio (3.1.6) + thor (1.3.2) tzinfo (2.0.6) concurrent-ruby (~> 1.0) - unicode-display_width (2.5.0) - zeitwerk (2.6.7) + unicode-display_width (3.1.4) + unicode-emoji (~> 4.0, >= 4.0.4) + unicode-emoji (4.0.4) + useragent (0.16.11) + zeitwerk (2.6.18) PLATFORMS - arm64-darwin-21 - arm64-darwin-23 - x86_64-linux + aarch64-linux-gnu + aarch64-linux-musl + arm-linux-gnu + arm-linux-musl + arm64-darwin + x86_64-darwin + x86_64-linux-gnu + x86_64-linux-musl DEPENDENCIES + appraisal combustion dotenv-rails lefthook @@ -172,4 +239,4 @@ DEPENDENCIES update_values_all! BUNDLED WITH - 2.4.6 + 2.5.7 diff --git a/lib/update_values_all/adapters/postgres.rb b/lib/update_values_all/adapters/postgres.rb index 120f251..b766eb9 100644 --- a/lib/update_values_all/adapters/postgres.rb +++ b/lib/update_values_all/adapters/postgres.rb @@ -4,8 +4,9 @@ module UpdateValuesAll module Adapters module Postgres - def pg_update_values_all(data, key_to_match:, touch: false, sql_update_expression: 'updated_at = CURRENT_TIMESTAMP') + def pg_update_values_all(data, key_to_match:, touch: false, sql_update_expression: "updated_at = CURRENT_TIMESTAMP") keys = data.first.keys + key_to_match = Array.wrap(key_to_match) sql_values = +'' data.each do |hash_row| @@ -23,26 +24,24 @@ def pg_update_values_all(data, key_to_match:, touch: false, sql_update_expressio end sql_values.chop! - updated_keys = keys.join(', ') + updated_keys = keys.join(", ") sql_types = data[0].keys.index_with { |column_name| column_for_attribute(column_name).sql_type_metadata.sql_type } set_expr = if block_given? +yield else - keys - .reject { |key| key == key_to_match } + (keys - key_to_match) .map { |key| "#{key} = updated_data.#{key}::#{sql_types[key]}" } - .join(', ') + .join(", ") end only_changed_expr = if touch - 'TRUE' + "TRUE" else - keys - .reject { |key| key == key_to_match } + (keys - key_to_match) .map { |key| "#{table_name}.#{key} IS DISTINCT FROM updated_data.#{key}::#{sql_types[key]}" } - .join(' OR ') + .join(" OR ") end if sql_update_expression.present? @@ -50,12 +49,12 @@ def pg_update_values_all(data, key_to_match:, touch: false, sql_update_expressio end existing_data_sql = - select("#{table_name}.#{key_to_match}") # rubocop:disable Gp/PotentialSqlInjection - .where("#{key_to_match} IN (SELECT #{key_to_match} FROM updated_data)") + select(primary_keys_sql(key_to_match, table_name)) # rubocop:disable Gp/PotentialSqlInjection + .where("(#{primary_keys_sql(key_to_match, table_name)}) IN (SELECT #{primary_keys_sql(key_to_match, 'updated_data')} FROM updated_data)") .to_sql changed_ids = - connection.query(<<~SQL).flatten # rubocop:disable Gp/PotentialSqlInjection + connection.query(<<~SQL) # rubocop:disable Gp/PotentialSqlInjection WITH updated_data(#{updated_keys}) AS ( VALUES #{sql_values} @@ -65,19 +64,23 @@ def pg_update_values_all(data, key_to_match:, touch: false, sql_update_expressio ) UPDATE #{table_name} SET #{set_expr} - FROM updated_data JOIN existing_data ON existing_data.#{key_to_match} = updated_data.#{key_to_match} + FROM updated_data JOIN existing_data ON (#{primary_keys_sql(key_to_match, 'existing_data')}) = (#{primary_keys_sql(key_to_match, 'updated_data')}) WHERE - updated_data.#{key_to_match} = #{table_name}.#{key_to_match} - AND #{table_name}.#{key_to_match} = existing_data.#{key_to_match} + (#{primary_keys_sql(key_to_match, 'updated_data')}) = (#{primary_keys_sql(key_to_match, table_name)}) + AND (#{primary_keys_sql(key_to_match, table_name)}) = (#{primary_keys_sql(key_to_match, 'existing_data')}) AND (#{only_changed_expr}) - RETURNING #{table_name}.#{key_to_match} + RETURNING #{primary_keys_sql(key_to_match, table_name)} SQL connection.query_cache.clear if connection.query_cache_enabled - changed_ids + # When the primary key is composite (multi-column), we return an array of arrays + key_to_match.size == 1 ? changed_ids.flatten : changed_ids end + private def primary_keys_sql(keys, table_name) + keys.map { |key| "#{table_name}.#{key}" }.join(", ") + end end end end diff --git a/spec/internal/app/models/application_record.rb b/spec/internal/app/models/application_record.rb index 1c9da9b..6c208d1 100644 --- a/spec/internal/app/models/application_record.rb +++ b/spec/internal/app/models/application_record.rb @@ -4,5 +4,4 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true - end diff --git a/spec/internal/app/models/product_user.rb b/spec/internal/app/models/product_user.rb new file mode 100644 index 0000000..594985d --- /dev/null +++ b/spec/internal/app/models/product_user.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ProductUser < ApplicationRecord + + self.primary_key = %i[product_id user_id] + +end diff --git a/spec/internal/db/schema.rb b/spec/internal/db/schema.rb index 0509cda..5c4c6da 100644 --- a/spec/internal/db/schema.rb +++ b/spec/internal/db/schema.rb @@ -7,4 +7,11 @@ t.jsonb :address t.timestamps end + + create_table(:product_users, id: false, force: true) do |t| + t.bigint :product_id + t.bigint :user_id + t.string :state + t.timestamps + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8f2d993..f12c7ef 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'bundler' +require 'logger' require 'rails' require 'dotenv/rails-now' require 'dotenv/load' diff --git a/spec/update_values_all/adapters/postgres_spec.rb b/spec/update_values_all/adapters/postgres_spec.rb index ab3d762..0eb81a4 100644 --- a/spec/update_values_all/adapters/postgres_spec.rb +++ b/spec/update_values_all/adapters/postgres_spec.rb @@ -122,4 +122,36 @@ expect(User.order(:id).pluck(:state)).to eq(%w[confirmed blocked]) end + + context 'when composite primary keys' do + before(:each) do + if ActiveRecord.gem_version < Gem::Version.new("7.1.0") + skip("Composite primary keys not supported in ActiveRecord < 7.1") + end + end + + it 'updates attributes for records with composite primary keys' do + ProductUser.create!(product_id: 1, user_id: 1, state: 'pending') + ProductUser.create!(product_id: 2, user_id: 2, state: 'pending') + + changed_ids = + ProductUser.update_values_all( + [ + { product_id: 1, user_id: 1, state: 'confirmed' }, + { product_id: 2, user_id: 2, state: 'blocked' } + ], + ) + + expect(changed_ids).to eq [[1, 1], [2, 2]] + expect(ProductUser.find(changed_ids).pluck(:state)).to eq(%w[confirmed blocked]) + end + + it 'raises an exception when one of the composite primary keys is missing' do + ProductUser.create!(product_id: 1, user_id: 1, state: 'pending') + + expect { + ProductUser.update_values_all([{ product_id: 1, state: 'confirmed' }]) + }.to raise_error(ActiveRecord::StatementInvalid) + end + end end diff --git a/update_values_all.gemspec b/update_values_all.gemspec index beec7a6..10051ee 100644 --- a/update_values_all.gemspec +++ b/update_values_all.gemspec @@ -29,7 +29,7 @@ Gem::Specification.new do |spec| end spec.require_paths = ['lib'] - spec.add_dependency 'activerecord', '>= 4.2' + spec.add_development_dependency "appraisal" # For more information and examples about making a new gem, check out our # guide at: https://bundler.io/guides/creating_gem.html