diff --git a/README.markdown b/README.markdown index aa5397f7..ab4fafbe 100644 --- a/README.markdown +++ b/README.markdown @@ -30,31 +30,36 @@ The gem provides the following high-level features: ## Table of Contents -* [Examples](#examples) - * [Introduction](#introduction) - * [Columns and Arrays](#columns-and-arrays) - * [Hashes](#hashes) - * [ActiveRecord Models](#activerecord-models) - * [Batching](#batching) - * [Recursive](#recursive) -* [Options](#options) - * [Duplicate Key Ignore](#duplicate-key-ignore) - * [Duplicate Key Update](#duplicate-key-update) -* [Return Info](#return-info) -* [Counter Cache](#counter-cache) -* [ActiveRecord Timestamps](#activerecord-timestamps) -* [Callbacks](#callbacks) -* [Supported Adapters](#supported-adapters) -* [Additional Adapters](#additional-adapters) -* [Requiring](#requiring) - * [Autoloading via Bundler](#autoloading-via-bundler) - * [Manually Loading](#manually-loading) -* [Load Path Setup](#load-path-setup) -* [Conflicts With Other Gems](#conflicts-with-other-gems) -* [More Information](#more-information) -* [Contributing](#contributing) - * [Running Tests](#running-tests) - * [Issue Triage](#issue-triage) +- [Activerecord-Import ](#activerecord-import-) + - [Table of Contents](#table-of-contents) + - [Examples](#examples) + - [Introduction](#introduction) + - [Columns and Arrays](#columns-and-arrays) + - [Hashes](#hashes) + - [Import Using Hashes and Explicit Column Names](#import-using-hashes-and-explicit-column-names) + - [ActiveRecord Models](#activerecord-models) + - [Batching](#batching) + - [Recursive](#recursive) + - [Options](#options) + - [Duplicate Key Ignore](#duplicate-key-ignore) + - [Duplicate Key Update](#duplicate-key-update) + - [Return Info](#return-info) + - [Counter Cache](#counter-cache) + - [ActiveRecord Timestamps](#activerecord-timestamps) + - [Callbacks](#callbacks) + - [Supported Adapters](#supported-adapters) + - [Additional Adapters](#additional-adapters) + - [Requiring](#requiring) + - [Autoloading via Bundler](#autoloading-via-bundler) + - [Manually Loading](#manually-loading) + - [Load Path Setup](#load-path-setup) + - [Conflicts With Other Gems](#conflicts-with-other-gems) + - [More Information](#more-information) + - [Contributing](#contributing) + - [Running Tests](#running-tests) + - [Issue Triage ](#issue-triage-) +- [License](#license) +- [Author](#author) ### Examples @@ -277,6 +282,8 @@ Key | Options | Default | Descrip :batch_size | `Integer` | total # of records | Max number of records to insert per import :raise_error | `true`/`false` | `false` | Raises an exception at the first invalid record. This means there will not be a result object returned. The `import!` method is a shortcut for this. :all_or_none | `true`/`false` | `false` | Will not import any records if there is a record with validation errors. +:omit_columns | `Array`/`Hash`/`Proc` | `nil` | Array of columns to leave out of SQL statement, e.g `[:guid]`, or Hash of `{ Model => [:column_name] }` or a Proc `-> (model, column_names) { [:guid] }` returning columns to omit +:omit_columns_with_default_functions | `true` / `false` | `false` | Automatically omit columns that have a default function defined in the schema, such as non-PK uuid columns #### Duplicate Key Ignore diff --git a/lib/activerecord-import/import.rb b/lib/activerecord-import/import.rb index 11eeab7d..72529f1a 100644 --- a/lib/activerecord-import/import.rb +++ b/lib/activerecord-import/import.rb @@ -347,6 +347,13 @@ def supports_setting_primary_key_of_imported_objects? # newly imported objects. PostgreSQL only. # * +batch_size+ - an integer value to specify the max number of records to # include per insert. Defaults to the total number of records to import. + # * +omit_columns+ an array of column names to exclude from the import, + # or a proc that receives the class, and array of column names, and returns + # a new array of column names (for recursive imports). This is to avoid + # having to populate Model.ignored_columns, which can leak to other threads. + # * +omit_columns_with_default_functions+ - true|false, tells import to filter + # out all columns with a default function defined in the schema, such as uuid + # columns that have a default value of uuid_generate_v4(). Defaults to false. # # == Examples # class BlogPost < ActiveRecord::Base ; end @@ -548,9 +555,60 @@ def bulk_import!(*args) end alias import! bulk_import! unless ActiveRecord::Base.respond_to? :import! + # Filters column names for a model according to the options given, + # specifically the :omit_columns and :omit_columns_with_default_functions options + def omitted_column_names(column_names, options) + model = options[:model] + # binding.pry if model == Redirection + ignored_column_names = [] + + # Do nothing if none of these options are truthy + return ignored_column_names unless options.slice(:omit_columns, :omit_columns_with_default_functions).values.any? + + # Remove columns that have default functions defined if the option is given + ignored_column_names += columns_with_default_function.map(&:name).map(&:to_sym) if options[:omit_columns_with_default_functions] + + if (omit_columns = options[:omit_columns]) + # If the option is a Proc, which is useful for recursive imports + # where the model class is not known yet, call it with the model + # and the current set of column names and expect it to return + # columns to ignore + # Cast to array in case it returns a falsy value + case omit_columns + when Proc + ignored_column_names += Array(omit_columns.call(model, column_names)).map(&:to_sym) + when Array + ignored_column_names += omit_columns.map(&:to_sym) + when Hash + # ignore_columns could also be a hash of { Model => [:guid, :uuid], OtherModel => [:some_column] } + ignored_column_names += Array(omit_columns[model]).map(&:to_sym) if omit_columns[model] + end + end + ignored_column_names + end + + # Finds all columns that have a default function defined + # in the schema. These columns should not be forcibly set + # to NULL even if it's allowed. + # If options[:omit_columns_with_default_functions] is given, + # we use this list to remove these columns from the list and + # subsequently from the schema column hash. + def columns_with_default_function + columns.select do |column| + # We should probably not ignore the primary key? + # If we should, it's not the job of this method to do so, + # so don't return the primary key in this list. + next if column.name == primary_key + # Any columns that have a default function + next unless column.default_function + true + end + end + def import_helper( *args ) options = { model: self, validate: true, timestamps: true, track_validation_failures: false } options.merge!( args.pop ) if args.last.is_a? Hash + # making sure that current model's primary key is used options[:primary_key] = primary_key options[:locking_column] = locking_column if attribute_names.include?(locking_column) @@ -572,6 +630,9 @@ def import_helper( *args ) end end + # Filter column names according to the options given + # before proceeding to construct the array of attributes + column_names -= omitted_column_names(column_names, options).map(&:to_s) if models.first.id.nil? Array(primary_key).each do |c| if column_names.include?(c) && schema_columns_hash[c].type == :uuid @@ -622,15 +683,22 @@ def import_helper( *args ) allow_extra_hash_keys = false end + # When importing an array of hashes, we know `self` is the current model + omitted_column_names = omitted_column_names(column_names, options) + array_of_attributes = array_of_hashes.map do |h| error_message = validate_hash_import(h, column_names, allow_extra_hash_keys) raise ArgumentError, error_message if error_message - column_names.map do |key| + # Ensure column attributes are set to the filtered list after validation, + # but validate as if the original list was passed in so that we dont + # fail validation on columns that are going to be ignored + (column_names - omitted_column_names).map do |key| h[key] end end + # supports empty array elsif args.last.is_a?( Array ) && args.last.empty? return ActiveRecord::Import::Result.new([], 0, [], []) @@ -653,6 +721,10 @@ def import_helper( *args ) # Force the primary key col into the insert if it's not # on the list and we are using a sequence and stuff a nil # value for it into each row so the sequencer will fire later + + # Remove omitted columns + column_names -= omitted_column_names(column_names, options) + symbolized_column_names = Array(column_names).map(&:to_sym) symbolized_primary_key = Array(primary_key).map(&:to_sym) diff --git a/test/models/redirection.rb b/test/models/redirection.rb new file mode 100644 index 00000000..cf759c6b --- /dev/null +++ b/test/models/redirection.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class Redirection < ActiveRecord::Base +end diff --git a/test/schema/postgresql_schema.rb b/test/schema/postgresql_schema.rb index 27f0142f..309369ec 100644 --- a/test/schema/postgresql_schema.rb +++ b/test/schema/postgresql_schema.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true ActiveRecord::Schema.define do + enable_extension "pgcrypto" execute('CREATE extension IF NOT EXISTS "hstore";') execute('CREATE extension IF NOT EXISTS "pgcrypto";') execute('CREATE extension IF NOT EXISTS "uuid-ossp";') @@ -59,5 +60,16 @@ t.datetime :updated_at end + create_table :redirections, force: :cascade do |t| + t.uuid "guid", default: -> { "gen_random_uuid()" } + t.string :title, null: false + t.string :author_name + t.string :url + t.datetime :created_at + t.datetime :created_on + t.datetime :updated_at + t.datetime :updated_on + end + add_index :alarms, [:device_id, :alarm_type], unique: true, where: 'status <> 0' end diff --git a/test/support/postgresql/import_examples.rb b/test/support/postgresql/import_examples.rb index 111cb0b8..299e817c 100644 --- a/test/support/postgresql/import_examples.rb +++ b/test/support/postgresql/import_examples.rb @@ -251,6 +251,198 @@ def should_support_postgresql_import_functionality assert_equal id, vendor.id end end + + if ENV['AR_VERSION'].to_f >= 5.0 + describe 'with :omit_columns option' do + let(:values) do + [ + { + title: "Invictus", + author_name: "William Ernest Henley", + url: 'https://www.poetryfoundation.org/poems/51642/invictus', + guid: '' # guid is blank, but will be ignored + }, + { + title: 'If', + author_name: 'Rudyard Kipling', + guid: SecureRandom.uuid, # this will still be ignored + url: 'https://www.poetryfoundation.org/poems/46473/if---' + } + ] + end + + context 'with array of hashes' do + context ':omit_columns is a Proc' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import values, omit_columns: ->(model, _column_names) { [:guid] if model == Redirection } + end + end + + it "should generate guid on match" do + Redirection.import! values, omit_columns: ->(model, _column_names) { [:guid] if model == Redirection } + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, values.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! values, omit_columns: ->(model, _column_names) { [:guid] unless model == Redirection } + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + + context ':omit_columns is an Array' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import values, omit_columns: [:guid] + end + end + + it "should generate guid" do + Redirection.import! values, omit_columns: [:guid] + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, values.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! values + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + + context ':omit_columns is a Hash' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import values, omit_columns: { Redirection => [:guid] } + end + end + + it "should generate guid" do + Redirection.import! values, omit_columns: { Redirection => [:guid] } + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, values.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! values, omit_columns: { Book => [:guid] } + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + end + + context 'with array of models' do + let(:models) { values.map { |hash| Redirection.new(hash) } } + context ':omit_columns is a Proc' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import models, omit_columns: ->(model, _column_names) { [:guid] if model == Redirection } + end + end + + it "should generate guid on match" do + Redirection.import! models, omit_columns: ->(model, _column_names) { [:guid] if model == Redirection } + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, models.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! models, omit_columns: ->(model, _column_names) { [:guid] unless model == Redirection } + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + + context ':omit_columns is an Array' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import models, omit_columns: [:guid] + end + end + + it "should generate guid" do + Redirection.import! models, omit_columns: [:guid] + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, values.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! models + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + + context ':omit_columns is a Hash' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import models, omit_columns: { Redirection => [:guid] } + end + end + + it "should generate guid" do + Redirection.import! models, omit_columns: { Redirection => [:guid] } + assert_not_nil Redirection.last.guid + assert_not_equal Redirection.last.guid, models.last[:guid] + end + + it "should not generate guid when not ignored" do + Redirection.import! models, omit_columns: { Book => [:guid] } + assert_nil Redirection.first.guid + assert_equal values.last[:guid], Redirection.last.guid + end + end + end + end + + describe 'with :omit_columns_with_default_functions option' do + let(:values) do + [ + { + title: "Invictus", + author_name: "William Ernest Henley", + url: 'https://www.poetryfoundation.org/poems/51642/invictus', + guid: '' # guid is blank, but will be ignored + }, + { + title: 'If', + author_name: 'Rudyard Kipling', + guid: SecureRandom.uuid, # this will still be ignored + url: 'https://www.poetryfoundation.org/poems/46473/if---' + } + ] + end + + context 'with array of hashes' do + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import values, omit_columns_with_default_functions: true + end + end + + it "should generate guid" do + Redirection.import values, omit_columns_with_default_functions: true + assert_not_nil Redirection.last.guid + end + end + + context 'with array of models' do + let(:models) { values.map { |hash| Redirection.new(hash) } } + it "should succeed with guid being generated" do + assert_difference "Redirection.count", +2 do + Redirection.import models, omit_columns_with_default_functions: true + end + end + + it "should generate guid" do + Redirection.import models, omit_columns_with_default_functions: true + assert_not_nil Redirection.last.guid + end + end + end + end end describe "with store accessor fields" do