From fe5e4111642ea04f5e35620ecea1dbe66291aa9d Mon Sep 17 00:00:00 2001 From: VitalinaVakulchyk Date: Fri, 2 Apr 2021 16:09:11 +0300 Subject: [PATCH] Add `ignore_blank` option to `field` method. Drop support for Ruby 2.5 (#778) --- .rubocop.yml | 1 + .rubocop_todo.yml | 23 ++-- CHANGELOG.md | 6 +- README.md | 23 +++- lib/chewy/fields/base.rb | 10 ++ lib/chewy/search/request.rb | 8 +- lib/chewy/type/syncer.rb | 30 ++--- lib/chewy/type/witchcraft.rb | 2 +- spec/chewy/fields/base_spec.rb | 237 +++++++++++++++++++++++++++++++++ spec/support/active_record.rb | 9 ++ 10 files changed, 312 insertions(+), 37 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 02efd02fa..0ba0eb272 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,7 @@ inherit_from: .rubocop_todo.yml AllCops: NewCops: enable + TargetRubyVersion: 2.6 Layout/AccessModifierIndentation: EnforcedStyle: outdent diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3462d1e3e..2b1cccd91 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2021-03-11 12:44:08 UTC using RuboCop version 1.11.0. +# on 2021-04-02 12:44:05 UTC using RuboCop version 1.11.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -13,16 +13,15 @@ Gemspec/RequiredRubyVersion: Exclude: - 'chewy.gemspec' -# Offense count: 3 +# Offense count: 2 # Configuration parameters: AllowedMethods. # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'lib/chewy.rb' - 'lib/chewy/rspec/update_index.rb' - 'spec/chewy/config_spec.rb' -# Offense count: 12 +# Offense count: 10 # Configuration parameters: AllowComments, AllowEmptyLambdas. Lint/EmptyBlock: Exclude: @@ -42,7 +41,7 @@ Lint/MissingSuper: - 'lib/chewy/type/adapter/object.rb' - 'lib/chewy/type/adapter/orm.rb' -# Offense count: 36 +# Offense count: 35 # Configuration parameters: IgnoredMethods, CountRepeatedAttributes. Metrics/AbcSize: Max: 41 @@ -50,14 +49,14 @@ Metrics/AbcSize: # Offense count: 4 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 269 + Max: 267 # Offense count: 13 # Configuration parameters: IgnoredMethods. Metrics/CyclomaticComplexity: Max: 12 -# Offense count: 44 +# Offense count: 43 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. Metrics/MethodLength: Max: 29 @@ -65,7 +64,7 @@ Metrics/MethodLength: # Offense count: 2 # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 174 + Max: 158 # Offense count: 18 # Configuration parameters: IgnoredMethods. @@ -89,7 +88,7 @@ Style/DocumentDynamicEvalDefinition: - 'lib/chewy/type/crutch.rb' - 'lib/chewy/type/witchcraft.rb' -# Offense count: 63 +# Offense count: 58 Style/Documentation: Enabled: false @@ -99,7 +98,7 @@ Style/EvalWithLocation: Exclude: - 'spec/chewy/index_spec.rb' -# Offense count: 208 +# Offense count: 191 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. # SupportedStyles: always, always_true, never @@ -113,7 +112,7 @@ Style/GuardClause: - 'lib/chewy.rb' - 'spec/support/active_record.rb' -# Offense count: 12 +# Offense count: 10 # Cop supports --auto-correct. Style/IfUnlessModifier: Exclude: @@ -125,7 +124,7 @@ Style/IfUnlessModifier: - 'lib/chewy/type/witchcraft.rb' - 'spec/support/active_record.rb' -# Offense count: 64 +# Offense count: 53 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https diff --git a/CHANGELOG.md b/CHANGELOG.md index 55a17fc4d..00995174a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,19 @@ ### New Features + * [#778](https://github.com/toptal/chewy/pull/778): Add `ignore_blank` option to `field` method ([@Vitalina-Vakulchyk][]): + * `true` by default for the `geo_point` type + * `false` by default for other types + ### Changes + * [#778](https://github.com/toptal/chewy/pull/778): **(Breaking)** Drop support for Ruby 2.5 ([@Vitalina-Vakulchyk][]): * [#776](https://github.com/toptal/chewy/pull/776): **(Breaking)** Removal of unnecessary features and integrations ([@Vitalina-Vakulchyk][]): * `aws-sdk-sqs` / `shoryuken` * `mongoid` * `sequel` * `will_paginate` * `resque` - * [#769](https://github.com/toptal/chewy/pull/769): **(Breaking)** Removal of deprecated methods and rake tasks ([@Vitalina-Vakulchyk][]): * `Chewy::Index.index_params` is removed, use `Chewy::Index.specification_hash` instead * `Chewy::Index.derivable_index_name` is removed, use `Chewy::Index.derivable_name` instead diff --git a/README.md b/README.md index 02c8b0297..387b3f88a 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ Chewy is an ODM (Object Document Mapper), built on top of the [the official Elas * [Witchcraft™ technology](#witchcraft-technology) * [Raw Import](#raw-import) * [Index creation during import](#index-creation-during-import) + * [Skip record fields during import](#skip-record-fields-during-import) + * [Default values for different types](#default-values-for-different-types) * [Journaling](#journaling) * [Types access](#types-access) * [Index manipulation](#index-manipulation) @@ -97,7 +99,7 @@ Or install it yourself as: ### Ruby -Chewy is compatible with MRI 2.5-3.0¹. +Chewy is compatible with MRI 2.6-3.0¹. > ¹ Ruby 3 is only supported with Rails 6.1 @@ -660,6 +662,25 @@ By default, when you perform import Chewy checks whether an index exists and cre You can turn off this feature to decrease Elasticsearch hits count. To do so you need to set `skip_index_creation_on_import` parameter to `false` in your `config/chewy.yml` +### Skip record fields during import + +You can use `ignore_blank: true` to skip fields that return `true` for the `.blank?` method: + +```ruby +define_type Country do + field :id + field :cities, ignore_blank: true do + field :id + field :name + field :surname, ignore_blank: true + field :description + end +end +``` + +#### Default values for different types + +By default `ignore_blank` is false on every type except `geo_point`. ### Journaling diff --git a/lib/chewy/fields/base.rb b/lib/chewy/fields/base.rb index e75c2c6df..18282bfa3 100644 --- a/lib/chewy/fields/base.rb +++ b/lib/chewy/fields/base.rb @@ -40,6 +40,8 @@ def mappings_hash def compose(*objects) result = evaluate(objects) + return {} if result.blank? && ignore_blank? + if children.present? && !multi_field? result = if result.respond_to?(:to_ary) result.to_ary.map { |item| compose_children(item, *objects) } @@ -53,6 +55,14 @@ def compose(*objects) private + def geo_point? + @options[:type].to_s == 'geo_point' + end + + def ignore_blank? + @options.fetch(:ignore_blank) { geo_point? } + end + def evaluate(objects) object = objects.first diff --git a/lib/chewy/search/request.rb b/lib/chewy/search/request.rb index ff4679e88..fbd5a6643 100644 --- a/lib/chewy/search/request.rb +++ b/lib/chewy/search/request.rb @@ -988,11 +988,9 @@ def reset def perform(additional = {}) request_body = render.merge(additional) ActiveSupport::Notifications.instrument 'search_query.chewy', notification_payload(request: request_body) do - begin - Chewy.client.search(request_body) - rescue Elasticsearch::Transport::Transport::Errors::NotFound - {} - end + Chewy.client.search(request_body) + rescue Elasticsearch::Transport::Transport::Errors::NotFound + {} end end diff --git a/lib/chewy/type/syncer.rb b/lib/chewy/type/syncer.rb index 67448234a..7ffc2ec39 100644 --- a/lib/chewy/type/syncer.rb +++ b/lib/chewy/type/syncer.rb @@ -127,12 +127,10 @@ def missing_ids def outdated_ids return [] if source_data.blank? || index_data.blank? || !@type.supports_outdated_sync? - @outdated_ids ||= begin - if @parallel - parallel_outdated_ids - else - linear_outdated_ids - end + @outdated_ids ||= if @parallel + parallel_outdated_ids + else + linear_outdated_ids end end @@ -147,19 +145,17 @@ def index_data end def source_and_index_data - @source_and_index_data ||= begin - if @parallel - ::ActiveRecord::Base.connection.close if defined?(::ActiveRecord::Base) - result = ::Parallel.map(%i[source index], @parallel, &SOURCE_OR_INDEX_DATA_WORKER.curry[self, @type]) - ::ActiveRecord::Base.connection.reconnect! if defined?(::ActiveRecord::Base) - if result.first.keys.first == :source - [result.first.values.first, result.second.values.first] - else - [result.second.values.first, result.first.values.first] - end + @source_and_index_data ||= if @parallel + ::ActiveRecord::Base.connection.close if defined?(::ActiveRecord::Base) + result = ::Parallel.map(%i[source index], @parallel, &SOURCE_OR_INDEX_DATA_WORKER.curry[self, @type]) + ::ActiveRecord::Base.connection.reconnect! if defined?(::ActiveRecord::Base) + if result.first.keys.first == :source + [result.first.values.first, result.second.values.first] else - [fetch_source_data, fetch_index_data] + [result.second.values.first, result.first.values.first] end + else + [fetch_source_data, fetch_index_data] end end diff --git a/lib/chewy/type/witchcraft.rb b/lib/chewy/type/witchcraft.rb index 3655d074d..a246cae1f 100644 --- a/lib/chewy/type/witchcraft.rb +++ b/lib/chewy/type/witchcraft.rb @@ -223,7 +223,7 @@ def replace_lvar(node, old_variable, new_variable) def replace_send(node, variable) if node.is_a?(Parser::AST::Node) if node.type == :send && node.children[0].nil? - node.updated(nil, [Parser::AST::Node.new(:lvar, [variable]), *node.children[1..-1]]) + node.updated(nil, [Parser::AST::Node.new(:lvar, [variable]), *node.children[1..]]) else node.updated(nil, node.children.map { |child| replace_send(child, variable) }) end diff --git a/spec/chewy/fields/base_spec.rb b/spec/chewy/fields/base_spec.rb index 897bd0efd..135559676 100644 --- a/spec/chewy/fields/base_spec.rb +++ b/spec/chewy/fields/base_spec.rb @@ -400,6 +400,243 @@ end end + context 'ignore_blank option for field method', :orm do + before do + stub_model(:location) + stub_model(:city) + stub_model(:country) + + City.belongs_to :country + Location.belongs_to :city + City.has_one :location, -> { order :id } + Country.has_many :cities, -> { order :id } + end + + context 'text fields with and without ignore_blank option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities do + field :id + field :name + field :historical_name, ignore_blank: false + field :description, ignore_blank: true + end + end + end + end + + let(:country_with_cities) do + cities = [ + City.create!(id: 1, name: '', historical_name: '', description: ''), + City.create!(id: 2, name: '', historical_name: '', description: '') + ] + + Country.create!(id: 1, cities: cities) + end + + specify do + expect(CountriesIndex::Country.root.compose(country_with_cities)).to eq( + 'id' => 1, 'cities' => [ + {'id' => 1, 'name' => '', 'historical_name' => ''}, + {'id' => 2, 'name' => '', 'historical_name' => ''} + ] + ) + end + end + + context 'nested fields' do + context 'with ignore_blank: true option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities, ignore_blank: true do + field :id + field :name + field :historical_name, ignore_blank: true + field :description + end + end + end + end + + let(:country) { Country.create!(id: 1, cities: cities) } + context('without cities') do + let(:cities) { [] } + specify do + expect(CountriesIndex::Country.root.compose(country)) + .to eq('id' => 1) + end + end + context('with cities') do + let(:cities) { [City.create!(id: 1, name: '', historical_name: '')] } + specify do + expect(CountriesIndex::Country.root.compose(country)).to eq( + 'id' => 1, 'cities' => [ + {'id' => 1, 'name' => '', 'description' => nil} + ] + ) + end + end + end + + context 'with ignore_blank: false option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities, ignore_blank: false do + field :id + field :name + field :historical_name + field :description + end + end + end + end + + let(:country_with_cities) { Country.create!(id: 1) } + + specify do + expect(CountriesIndex::Country.root.compose(country_with_cities)) + .to eq('id' => 1, 'cities' => []) + end + end + + context 'without ignore_blank: true option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities do + field :id + field :name + field :historical_name + field :description + end + end + end + end + + let(:country_with_cities) { Country.create!(id: 1) } + + specify do + expect(CountriesIndex::Country.root.compose(country_with_cities)) + .to eq('id' => 1, 'cities' => []) + end + end + end + + context 'geo_point field type' do + context 'with ignore_blank: true option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities do + field :id + field :name + field :location, type: :geo_point, ignore_blank: true do + field :lat + field :lon + end + end + end + end + end + + specify do + expect( + CountriesIndex::Country.root.compose({ + 'id' => 1, + 'cities' => [ + {'id' => 1, 'name' => 'City1', 'location' => {}}, + {'id' => 2, 'name' => 'City2', 'location' => {}} + ] + }) + ).to eq( + 'id' => 1, 'cities' => [ + {'id' => 1, 'name' => 'City1'}, + {'id' => 2, 'name' => 'City2'} + ] + ) + end + end + + context 'without ignore_blank option' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities do + field :id + field :name + field :location, type: :geo_point do + field :lat + field :lon + end + end + end + end + end + + specify do + expect( + CountriesIndex::Country.root.compose({ + 'id' => 1, + 'cities' => [ + {'id' => 1, 'name' => 'City1', 'location' => {}}, + {'id' => 2, 'name' => 'City2', 'location' => {}} + ] + }) + ).to eq( + 'id' => 1, 'cities' => [ + {'id' => 1, 'name' => 'City1'}, + {'id' => 2, 'name' => 'City2'} + ] + ) + end + end + + context 'with ignore_blank: false flag' do + before do + stub_index(:countries) do + define_type Country do + field :id + field :cities do + field :id + field :name + field :location, type: :geo_point, ignore_blank: false do + field :lat + field :lon + end + end + end + end + end + + specify do + expect( + CountriesIndex::Country.root.compose({ + 'id' => 1, + 'cities' => [ + {'id' => 1, 'location' => {}, 'name' => 'City1'}, + {'id' => 2, 'location' => '', 'name' => 'City2'} + ] + }) + ).to eq( + 'id' => 1, 'cities' => [ + {'id' => 1, 'location' => {}, 'name' => 'City1'}, + {'id' => 2, 'location' => '', 'name' => 'City2'} + ] + ) + end + end + end + end + context 'objects and scopes', :orm do before do stub_model(:city) diff --git a/spec/support/active_record.rb b/spec/support/active_record.rb index ffe8b49ce..45c3c501b 100644 --- a/spec/support/active_record.rb +++ b/spec/support/active_record.rb @@ -8,6 +8,7 @@ ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS 'countries'") ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS 'cities'") +ActiveRecord::Base.connection.execute("DROP TABLE IF EXISTS 'locations'") ActiveRecord::Schema.define do create_table :countries do |t| t.column :name, :string @@ -19,9 +20,17 @@ create_table :cities do |t| t.column :country_id, :integer t.column :name, :string + t.column :historical_name, :string + t.column :description, :string t.column :rating, :integer t.column :updated_at, :datetime end + + create_table :locations do |t| + t.column :city_id, :integer + t.column :lat, :string + t.column :lon, :string + end end module ActiveRecordClassHelpers