From 0469f5df6d0a95760f30e41a7405b889481a947b Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:34:02 +0200 Subject: [PATCH 01/56] Remove the default from the AttributeInstruction#operator method This will be handled by the filtering strategies themselves in the future --- lib/active_set/attribute_instruction.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/active_set/attribute_instruction.rb b/lib/active_set/attribute_instruction.rb index 72e9fd1..21cf6e2 100644 --- a/lib/active_set/attribute_instruction.rb +++ b/lib/active_set/attribute_instruction.rb @@ -32,11 +32,11 @@ def attribute @attribute = attribute end - def operator(default: '==') + def operator return @operator if defined? @operator attribute_instruction = @keypath.last - @operator = (attribute_instruction[operator_regex, 1] || default).to_sym + @operator = attribute_instruction[operator_regex, 1]&.to_sym end def options From dd044148eef6499d7cd360628f1d2cf6e6e174df Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:35:14 +0200 Subject: [PATCH 02/56] Move the filtering strategies into directory/module namespaces Since we will be adding other stategy-specific files/modules soon --- .../filtering/active_record/strategy.rb | 87 +++++++++++++++++++ .../filtering/active_record_strategy.rb | 85 ------------------ .../filtering/enumerable/strategy.rb | 63 ++++++++++++++ .../filtering/enumerable_strategy.rb | 79 ----------------- lib/active_set/filtering/operation.rb | 8 +- 5 files changed, 154 insertions(+), 168 deletions(-) create mode 100644 lib/active_set/filtering/active_record/strategy.rb delete mode 100644 lib/active_set/filtering/active_record_strategy.rb create mode 100644 lib/active_set/filtering/enumerable/strategy.rb delete mode 100644 lib/active_set/filtering/enumerable_strategy.rb diff --git a/lib/active_set/filtering/active_record/strategy.rb b/lib/active_set/filtering/active_record/strategy.rb new file mode 100644 index 0000000..71dde7f --- /dev/null +++ b/lib/active_set/filtering/active_record/strategy.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require_relative './set_instruction' +require 'active_support/core_ext/module/delegation' + +class ActiveSet + module Filtering + module ActiveRecord + class Strategy + delegate :attribute_model, + :arel_column, + :arel_operator, + :arel_value, + :arel_type, + :initial_relation, + :attribute, + to: :@set_instruction + + def initialize(set, attribute_instruction) + @set = set + @attribute_instruction = attribute_instruction + @set_instruction = SetInstruction.new(attribute_instruction, set) + end + + def execute + return false unless @set.respond_to? :to_sql + + if execute_filter_operation? + statement = filter_operation + elsif execute_intersect_operation? + begin + statement = intersect_operation + rescue ArgumentError # thrown if merging a non-ActiveRecord::Relation + return false + end + else + return false + end + + statement + end + + private + + def execute_filter_operation? + return false unless attribute_model + return false unless attribute_model.respond_to?(:attribute_names) + return false unless attribute_model.attribute_names.include?(attribute) + + true + end + + def execute_intersect_operation? + return false unless attribute_model + return false unless attribute_model.respond_to?(attribute) + return false if attribute_model.method(attribute).arity.zero? + + true + end + + def filter_operation + initial_relation + .where( + arel_column.send( + arel_operator, + arel_value + ) + ) + end + + def intersect_operation + # NOTE: If merging relations that contain duplicate column conditions, + # the second condition will replace the first. + # e.g. Thing.where(id: [1,2]).merge(Thing.where(id: [2,3])) + # => [Thing<2>, Thing<3>] NOT [Thing<2>] + initial_relation + .merge( + attribute_model.public_send( + attribute, + arel_value + ) + ) + end + end + end + end +end diff --git a/lib/active_set/filtering/active_record_strategy.rb b/lib/active_set/filtering/active_record_strategy.rb deleted file mode 100644 index cb69872..0000000 --- a/lib/active_set/filtering/active_record_strategy.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require_relative '../active_record_set_instruction' -require 'active_support/core_ext/module/delegation' - -class ActiveSet - module Filtering - class ActiveRecordStrategy - delegate :attribute_model, - :arel_column, - :arel_operator, - :arel_value, - :arel_type, - :initial_relation, - :attribute, - to: :@set_instruction - - def initialize(set, attribute_instruction) - @set = set - @attribute_instruction = attribute_instruction - @set_instruction = ActiveRecordSetInstruction.new(attribute_instruction, set) - end - - def execute - return false unless @set.respond_to? :to_sql - - if execute_filter_operation? - statement = filter_operation - elsif execute_intersect_operation? - begin - statement = intersect_operation - rescue ArgumentError # thrown if merging a non-ActiveRecord::Relation - return false - end - else - return false - end - - statement - end - - private - - def execute_filter_operation? - return false unless attribute_model - return false unless attribute_model.respond_to?(:attribute_names) - return false unless attribute_model.attribute_names.include?(attribute) - - true - end - - def execute_intersect_operation? - return false unless attribute_model - return false unless attribute_model.respond_to?(attribute) - return false if attribute_model.method(attribute).arity.zero? - - true - end - - def filter_operation - initial_relation - .where( - arel_column.send( - arel_operator, - arel_value - ) - ) - end - - def intersect_operation - # NOTE: If merging relations that contain duplicate column conditions, - # the second condition will replace the first. - # e.g. Thing.where(id: [1,2]).merge(Thing.where(id: [2,3])) - # => [Thing<2>, Thing<3>] NOT [Thing<2>] - initial_relation - .merge( - attribute_model.public_send( - attribute, - arel_value - ) - ) - end - end - end -end diff --git a/lib/active_set/filtering/enumerable/strategy.rb b/lib/active_set/filtering/enumerable/strategy.rb new file mode 100644 index 0000000..1887f8e --- /dev/null +++ b/lib/active_set/filtering/enumerable/strategy.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require_relative './set_instruction' + +class ActiveSet + module Filtering + module Enumerable + class Strategy + def initialize(set, attribute_instruction) + @set = set + @attribute_instruction = attribute_instruction + @set_instruction = SetInstruction.new(attribute_instruction, set) + end + + def execute + return false unless @set.respond_to? :select + + if execute_filter_operation? + set = filter_operation + elsif execute_intersect_operation? + begin + set = intersect_operation + rescue TypeError # thrown if intersecting with a non-Array + return false + end + else + return false + end + + set + end + + private + + def execute_filter_operation? + return false if not @set_instruction.attribute_instance + return false if not @set_instruction.attribute_instance.respond_to?(@set_instruction.attribute) + return false if @set_instruction.attribute_instance.method(@set_instruction.attribute).arity.positive? + + true + end + + def execute_intersect_operation? + return false if not @set_instruction.attribute_class + return false if not @set_instruction.attribute_class.respond_to?(@set_instruction.attribute) + return false if @set_instruction.attribute_class.method(@set_instruction.attribute).arity.zero? + + true + end + + def filter_operation + @set.select do |item| + @set_instruction.item_matches_query?(item) + end + end + + def intersect_operation + @set & @set_instruction.other_set + end + end + end + end +end diff --git a/lib/active_set/filtering/enumerable_strategy.rb b/lib/active_set/filtering/enumerable_strategy.rb deleted file mode 100644 index 5bc50bc..0000000 --- a/lib/active_set/filtering/enumerable_strategy.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -require_relative '../enumerable_set_instruction' -require 'active_support/core_ext/module/delegation' - -class ActiveSet - module Filtering - class EnumerableStrategy - delegate :attribute_instance, - :attribute_class, - :attribute_value, - :attribute_value_for, - :operator, - :attribute, - to: :@set_instruction - - def initialize(set, attribute_instruction) - @set = set - @attribute_instruction = attribute_instruction - @set_instruction = EnumerableSetInstruction.new(attribute_instruction, set) - end - - def execute - return false unless @set.respond_to? :select - - if execute_filter_operation? - set = filter_operation - elsif execute_intersect_operation? - begin - set = intersect_operation - rescue TypeError # thrown if intersecting with a non-Array - return false - end - else - return false - end - - set - end - - private - - def execute_filter_operation? - return false unless attribute_instance - return false unless attribute_instance.respond_to?(attribute) - return false if attribute_instance.method(attribute).arity.positive? - - true - end - - def execute_intersect_operation? - return false unless attribute_class - return false unless attribute_class.respond_to?(attribute) - return false if attribute_class.method(attribute).arity.zero? - - true - end - - def filter_operation - @set.select do |item| - attribute_value_for(item) - .public_send( - operator, - attribute_value - ) - end - end - - def intersect_operation - other_set = attribute_class - .public_send( - attribute, - attribute_value - ) - @set & other_set - end - end - end -end diff --git a/lib/active_set/filtering/operation.rb b/lib/active_set/filtering/operation.rb index dfdc580..e524f97 100644 --- a/lib/active_set/filtering/operation.rb +++ b/lib/active_set/filtering/operation.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true require_relative '../attribute_instruction' -require_relative './enumerable_strategy' -require_relative './active_record_strategy' +require_relative './enumerable/strategy' +require_relative './active_record/strategy' class ActiveSet module Filtering @@ -17,7 +17,7 @@ def execute .map { |k, v| AttributeInstruction.new(k, v) } activerecord_filtered_set = attribute_instructions.reduce(@set) do |set, attribute_instruction| - maybe_set_or_false = ActiveRecordStrategy.new(set, attribute_instruction).execute + maybe_set_or_false = ActiveRecord::Strategy.new(set, attribute_instruction).execute next set unless maybe_set_or_false attribute_instruction.processed = true @@ -27,7 +27,7 @@ def execute return activerecord_filtered_set if attribute_instructions.all?(&:processed?) attribute_instructions.reject(&:processed?).reduce(activerecord_filtered_set) do |set, attribute_instruction| - maybe_set_or_false = EnumerableStrategy.new(set, attribute_instruction).execute + maybe_set_or_false = Enumerable::Strategy.new(set, attribute_instruction).execute next set unless maybe_set_or_false attribute_instruction.processed = true From 5c277af73d5fa0d89ad4a17f95d330723c3d8f33 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:35:36 +0200 Subject: [PATCH 03/56] Add the base predicate constants for filtering --- lib/active_set/filtering/constants.rb | 83 +++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 lib/active_set/filtering/constants.rb diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb new file mode 100644 index 0000000..30895a6 --- /dev/null +++ b/lib/active_set/filtering/constants.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +class ActiveSet + module Filtering + module Constants + BASE_PREDICATES = { + EQ: { + type: :binary, + compound: false, + behavior: :inclusive, + shorthand: '=='.to_sym + }, + NOT_EQ: { + type: :binary, + compound: false, + behavior: :exclusive, + shorthand: '!='.to_sym + }, + IN: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: '<<'.to_sym + }, + NOT_IN: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: '!<'.to_sym + }, + MATCHES: { + type: :binary, + compound: false, + behavior: :inclusive, + shorthand: '=~'.to_sym + }, + DOES_NOT_MATCH: { + type: :binary, + compound: false, + behavior: :exclusive, + shorthand: '!~'.to_sym + }, + LT: { + type: :binary, + compound: false, + behavior: :exclusive, + shorthand: '<'.to_sym + }, + LTEQ: { + type: :binary, + compound: false, + behavior: :inclusive, + shorthand: '<='.to_sym + }, + GT: { + type: :binary, + compound: false, + behavior: :exclusive, + shorthand: '>'.to_sym + }, + GTEQ: { + type: :binary, + compound: false, + behavior: :inclusive, + shorthand: '>='.to_sym + }, + BETWEEN: { + type: :binary, + compound: true, + behavior: :inconclusive, + shorthand: '..'.to_sym + }, + NOT_BETWEEN: { + type: :binary, + compound: true, + behavior: :inconclusive, + shorthand: '!.'.to_sym + } + }.freeze + end + end +end + From 4eeff53aab8078544fbe392429fdfa533cdd9983 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:36:12 +0200 Subject: [PATCH 04/56] Add the strategy-specific operator resolution logic --- .../filtering/active_record/operators.rb | 59 +++++++++++++++++ .../filtering/enumerable/operators.rb | 64 +++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 lib/active_set/filtering/active_record/operators.rb create mode 100644 lib/active_set/filtering/enumerable/operators.rb diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb new file mode 100644 index 0000000..9411d62 --- /dev/null +++ b/lib/active_set/filtering/active_record/operators.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require_relative '../constants' + +class ActiveSet + module Filtering + module ActiveRecord + module Operators + BASE_PREDICATES = { + EQ: { + operator: :eq + }, + NOT_EQ: { + operator: :not_eq + }, + IN: { + operator: :in + }, + NOT_IN: { + operator: :not_in + }, + MATCHES: { + operator: :matches + }, + DOES_NOT_MATCH: { + operator: :does_not_match + }, + LT: { + operator: :lt + }, + LTEQ: { + operator: :lteq + }, + GT: { + operator: :gt + }, + GTEQ: { + operator: :gteq + }, + BETWEEN: { + operator: :between + }, + NOT_BETWEEN: { + operator: :not_between + } + }.freeze + + def self.get(operator_name) + operator_key = operator_name.to_s.upcase.to_sym + + base_operator_hash = Constants::BASE_PREDICATES.fetch(operator_key, {}) + this_operator_hash = Operators::BASE_PREDICATES.fetch(operator_key, {}) + + base_operator_hash.merge(this_operator_hash) + end + end + end + end +end diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb new file mode 100644 index 0000000..5b21916 --- /dev/null +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require_relative '../constants' + +class ActiveSet + module Filtering + module Enumerable + module Operators + BASE_PREDICATES = { + EQ: { + operator: :'==' + }, + NOT_EQ: { + operator: :'!=' + }, + IN: { + operator: :presence_in + }, + NOT_IN: { + operator: :presence_in, + result_transformer: ->(result) { !result } + }, + MATCHES: { + operator: :'=~', + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{attribute}/ } + }, + DOES_NOT_MATCH: { + operator: :'!~', + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{attribute}/ } + }, + LT: { + operator: :'<' + }, + LTEQ: { + operator: :'<=' + }, + GT: { + operator: :'>' + }, + GTEQ: { + operator: :'>=' + }, + BETWEEN: { + operator: :between + }, + NOT_BETWEEN: { + operator: :not_between + } + }.freeze + + def self.get(operator_name) + operator_key = operator_name.to_s.upcase.to_sym + + base_operator_hash = Constants::BASE_PREDICATES.fetch(operator_key, {}) + this_operator_hash = Operators::BASE_PREDICATES.fetch(operator_key, {}) + + base_operator_hash.merge(this_operator_hash) + end + end + end + end +end From eac3288bb8a48850996cfd4971676ce4b505d533 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:37:01 +0200 Subject: [PATCH 05/56] Implement filtering-specific SetInstruction classes These can manage all of the filtering-specific logic bound to the set and the AttributeInstruction --- .../active_record_set_instruction.rb | 4 - lib/active_set/enumerable_set_instruction.rb | 5 +- .../active_record/set_instruction.rb | 20 +++++ .../filtering/enumerable/set_instruction.rb | 76 +++++++++++++++++++ 4 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 lib/active_set/filtering/active_record/set_instruction.rb create mode 100644 lib/active_set/filtering/enumerable/set_instruction.rb diff --git a/lib/active_set/active_record_set_instruction.rb b/lib/active_set/active_record_set_instruction.rb index ee4ab37..e4c3a07 100644 --- a/lib/active_set/active_record_set_instruction.rb +++ b/lib/active_set/active_record_set_instruction.rb @@ -40,10 +40,6 @@ def arel_column end # rubocop:enable Lint/UnderscorePrefixedVariableName - def arel_operator - @attribute_instruction.operator(default: :eq) - end - # rubocop:disable Lint/UnderscorePrefixedVariableName def arel_value _arel_value = @attribute_instruction.value diff --git a/lib/active_set/enumerable_set_instruction.rb b/lib/active_set/enumerable_set_instruction.rb index 56ad0ff..e20288e 100644 --- a/lib/active_set/enumerable_set_instruction.rb +++ b/lib/active_set/enumerable_set_instruction.rb @@ -29,8 +29,11 @@ def case_insensitive_operation_for?(value) value.is_a?(String) || value.is_a?(Symbol) end + def set_item + @set.find(&:present?) + end + def attribute_instance - set_item = @set.find(&:present?) return set_item if @attribute_instruction.associations_array.empty? return @attribute_model if defined? @attribute_model diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb new file mode 100644 index 0000000..efbb12a --- /dev/null +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require_relative '../../active_record_set_instruction' +require_relative './operators' + +class ActiveSet + module Filtering + module ActiveRecord + class SetInstruction < ActiveRecordSetInstruction + def arel_operator + instruction_operator = @attribute_instruction.operator + return :eq unless instruction_operator + + operator_hash = Operators.get(instruction_operator) + operator_hash&.dig(:operator) + end + end + end + end +end diff --git a/lib/active_set/filtering/enumerable/set_instruction.rb b/lib/active_set/filtering/enumerable/set_instruction.rb new file mode 100644 index 0000000..02842fe --- /dev/null +++ b/lib/active_set/filtering/enumerable/set_instruction.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require_relative '../../enumerable_set_instruction' +require_relative './operators' + +class ActiveSet + module Filtering + module Enumerable + class SetInstruction < EnumerableSetInstruction + def item_matches_query?(item) + result = object_attribute_for(item) + .public_send( + operator_method, + query_attribute + ) + + return result unless operator_hash.key?(:result_transformer) + + operator_hash[:result_transformer].call(result) + end + + def other_set + other_set = attribute_class.public_send( + attribute, + attribute_value + ) + if attribute_class != set_item.class + other_set = begin + @set.select { |item| resource_for(item: item)&.presence_in other_set } + rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil + nil + end + end + + other_set + end + + private + + def object_attribute_for(item) + attribute = guarantee_attribute_type(attribute_value_for(item)) + return attribute unless operator_hash.key?(:object_attribute_transformer) + + operator_hash[:object_attribute_transformer].call(attribute) + end + + def query_attribute + attribute = guarantee_attribute_type(attribute_value) + return attribute unless operator_hash.key?(:query_attribute_transformer) + + operator_hash[:query_attribute_transformer].call(attribute) + end + + def operator_method + operator_hash.dig(:operator) || :'==' + end + + def operator_hash + instruction_operator = @attribute_instruction.operator + + Operators.get(instruction_operator) + end + + def guarantee_attribute_type(attribute) + # Booleans don't respond to many operator methods, + # so we cast them to integers + return 1 if attribute == true + return 0 if attribute == false + return attribute.map { |a| guarantee_attribute_type(a) } if attribute.respond_to?(:each) + + attribute + end + end + end + end +end From 6df19a2edf5cdc5094ac2a4fe299f75018c51643 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:37:34 +0200 Subject: [PATCH 06/56] Change the default spec running behavior Must manually turn on coverage and failure reports --- spec/database_cleaner_helper.rb | 22 ++++++++++++++++++++ spec/inspect_failure_helper.rb | 21 +++++++++++++++++++ spec/simplecov_helper.rb | 2 +- spec/spec_helper.rb | 37 ++------------------------------- 4 files changed, 46 insertions(+), 36 deletions(-) create mode 100644 spec/database_cleaner_helper.rb create mode 100644 spec/inspect_failure_helper.rb diff --git a/spec/database_cleaner_helper.rb b/spec/database_cleaner_helper.rb new file mode 100644 index 0000000..c07559d --- /dev/null +++ b/spec/database_cleaner_helper.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +RSpec.configure do |config| + config.before(:suite) do + DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.strategy = :transaction + end + + config.before(:all) do + DatabaseCleaner.start + end + + config.around(:each) do |example| + DatabaseCleaner.cleaning do + example.run + end + end + + config.after(:all) do + DatabaseCleaner.clean + end +end diff --git a/spec/inspect_failure_helper.rb b/spec/inspect_failure_helper.rb new file mode 100644 index 0000000..bade4e4 --- /dev/null +++ b/spec/inspect_failure_helper.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +if ENV['INSPECT_FAILURE'] == 'true' + RSpec.configure do |config| + # Give a detailed report of all relevant data if a spec fails + # http://bensnape.com/2014/08/01/rspec-after-failure-hook/ + config.after(:each) do |example| + next unless example.exception + + let_data = @__memoized.instance_variable_get('@memoized') + ivar_data = instance_variables + .reject { |v| v.to_s.start_with?('@_') } + .reject { |v| v.presence_in %i[@example @fixture_cache @fixture_connections @connection_subscriber @loaded_fixtures] } + .map { |v| [v, instance_variable_get(v)] } + .to_h + + # https://www.jvt.me/posts/2019/03/29/pretty-printing-json-ruby/ + jj let_data.merge(ivar_data).transform_values(&:inspect) + end + end +end diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb index cfdabbd..5a52d8d 100644 --- a/spec/simplecov_helper.rb +++ b/spec/simplecov_helper.rb @@ -5,7 +5,7 @@ require 'simplecov-console' require 'codecov' -unless ENV['COVERAGE'] == 'false' +if ENV['COVERAGE'] == 'true' ROOT = File.expand_path('..', __dir__) # SimpleCov.minimum_coverage 99 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2b818f0..6d84168 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'simplecov_helper' +require 'database_cleaner_helper' +require 'inspect_failure_helper' require 'bundler' require 'combustion' @@ -46,39 +48,4 @@ nil end end - - config.before(:suite) do - DatabaseCleaner.clean_with(:truncation) - DatabaseCleaner.strategy = :transaction - end - - config.before(:all) do - DatabaseCleaner.start - end - - config.around(:each) do |example| - DatabaseCleaner.cleaning do - example.run - end - end - - config.after(:all) do - DatabaseCleaner.clean - end - - # Give a detailed report of all relevant data if a spec fails - # http://bensnape.com/2014/08/01/rspec-after-failure-hook/ - config.after(:each) do |example| - next unless example.exception - - let_data = @__memoized.instance_variable_get('@memoized') - ivar_data = instance_variables - .reject { |v| v.to_s.start_with?('@_') } - .reject { |v| v.presence_in %i[@example @fixture_cache @fixture_connections @connection_subscriber @loaded_fixtures] } - .map { |v| [v, instance_variable_get(v)] } - .to_h - - # https://www.jvt.me/posts/2019/03/29/pretty-printing-json-ruby/ - jj let_data.merge(ivar_data).transform_values(&:inspect) - end end From 55144bcd4de4efefaa8f400cf1e8187874b59d77 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:38:54 +0200 Subject: [PATCH 07/56] Update the filtering specs Test both ActiveRecord and Enumerable sets Fix the scope spec when dealing with computed scopes Make the predicate specs work off of the filtering predicate constants --- spec/active_set/filter/predicates_spec.rb | 334 +++++++++++----------- spec/active_set/filter/scopes_spec.rb | 40 +-- spec/active_set/filter/types_spec.rb | 22 +- 3 files changed, 202 insertions(+), 194 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index 56858e0..96f7cac 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -2,189 +2,197 @@ require 'spec_helper' +# PREDICATE_OPERATORS = ActiveSet::Filtering::ActiveRecord::Constants::PREDICATE_OPERATORS +# INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS +# .select do |o| +# o[:type] == :unary && +# o[:matching_behavior] == :inclusive +# end +# EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS +# .select do |o| +# o[:type] == :unary && +# o[:matching_behavior] == :exclusive +# end +# INCONCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS +# .select do |o| +# o[:type] == :unary && +# o[:matching_behavior] == :inconclusive +# end + + RSpec.describe ActiveSet do + PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::BASE_PREDICATES + INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :inclusive + end.map(&:first) + EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :exclusive + end.map(&:first) + INCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :inclusive + end.map(&:first) + EXCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :exclusive + end.map(&:first) + before(:all) do @thing_1 = FactoryBot.create(:thing) @thing_2 = FactoryBot.create(:thing) - @active_set = ActiveSet.new(Thing.all) end describe '#filter' do - let(:results) { @active_set.filter(instructions) } - let(:result_ids) { results.map(&:id) } - - ApplicationRecord::DB_FIELD_TYPES.each do |type| - [1, 2].each do |id| - # single value inclusive operators - %i[ - eq - lteq - gteq - matches - ].each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| - context "{ #{path}: }" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - let(:instruction_single_value) do - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) - end - let(:instructions) do - { - path => instruction_single_value - } - end - - it { expect(result_ids).to include matching_item.id } - end - end + ['itself', 'to_a'].each do |relation_mutator| + context "/#{ relation_mutator == 'itself' ? 'ActiveRecord' : 'Enumberable' }/" do + before(:all) do + @active_set = ActiveSet.new(Thing.all.public_send(relation_mutator)) end + let(:results) { @active_set.filter(instructions) } + let(:result_ids) { results.map(&:id) } - # single value exlusive operators - %i[ - not_eq - lt - gt - does_not_match - ].each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| - context "{ #{path}: }" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - let(:instruction_single_value) do - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) - end - let(:instructions) do - { - path => instruction_single_value - } - end + ApplicationRecord::DB_FIELD_TYPES.each do |type| + [1, 2].each do |id| + INCLUSIVE_UNARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:instruction_single_value) do + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) + end + let(:instructions) do + { + path => instruction_single_value + } + end - it { expect(result_ids).not_to include matching_item.id } + it { expect(result_ids).to include matching_item.id } + end + end end - end - end - # multi value inclusive operators - %i[ - eq_any - not_eq_any - in - in_any - not_in_any - lteq_any - gteq_any - matches_any - does_not_match_any - ].each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| - context "{ #{path}: }" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - let(:other_thing) do - guaranteed_unique_object_for(matching_item, - only: guaranteed_unique_object_for(matching_item.only)) - end - let(:instruction_multi_value) do - [ - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - ] - end - let(:instructions) do - { - path => instruction_multi_value - } - end + EXCLUSIVE_UNARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:instruction_single_value) do + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) + end + let(:instructions) do + { + path => instruction_single_value + } + end - it { expect(result_ids).to include matching_item.id } + it { expect(result_ids).not_to include matching_item.id } + end + end end - end - end - # multi value exclusive operators - %i[ - eq_all - not_eq_all - not_in - in_all - not_in_all - lt_all - gt_all - matches_all - does_not_match_all - ].each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| - context "{ #{path}: }" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - let(:other_thing) do - guaranteed_unique_object_for(matching_item, - only: guaranteed_unique_object_for(matching_item.only)) - end - let(:instruction_multi_value) do - [ - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - ] - end - let(:instructions) do - { - path => instruction_multi_value - } + INCLUSIVE_BINARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:other_thing) do + guaranteed_unique_object_for(matching_item, + only: guaranteed_unique_object_for(matching_item.only)) + end + let(:instruction_multi_value) do + [ + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) + ] + end + let(:instructions) do + { + path => instruction_multi_value + } + end + + it { expect(result_ids).to include matching_item.id } + end end + end - it { expect(result_ids).not_to include matching_item.id } + EXCLUSIVE_BINARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:other_thing) do + guaranteed_unique_object_for(matching_item, + only: guaranteed_unique_object_for(matching_item.only)) + end + let(:instruction_multi_value) do + [ + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), + ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) + ] + end + let(:instructions) do + { + path => instruction_multi_value + } + end + + it { expect(result_ids).not_to include matching_item.id } + end + end end + + # multi value mixed operators + # %i[ + # lt_any + # lteq_all + # gt_any + # gteq_all + # ].each do |operator| + # %W[ + # #{type}(#{operator}) + # only.#{type}(#{operator}) + # ].each do |path| + # context "{ #{path}: }" do + # let(:matching_item) { instance_variable_get("@thing_#{id}") } + # let(:other_thing) do + # FactoryBot.build(:thing, + # boolean: !matching_item.boolean, + # only: FactoryBot.build(:only, + # boolean: !matching_item.only.boolean)) + # end + # let(:instruction_multi_value) do + # [ + # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), + # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) + # ] + # end + # let(:instructions) do + # { + # path => instruction_multi_value + # } + # end + + # if type.presence_in(%i[binary datetime decimal float integer]) && operator == :gt_any + # it { expect(result_ids).not_to include matching_item.id } + # else + # end + # end + # end + # end end end - - # multi value mixed operators - # %i[ - # lt_any - # lteq_all - # gt_any - # gteq_all - # ].each do |operator| - # %W[ - # #{type}(#{operator}) - # only.#{type}(#{operator}) - # ].each do |path| - # context "{ #{path}: }" do - # let(:matching_item) { instance_variable_get("@thing_#{id}") } - # let(:other_thing) do - # FactoryBot.build(:thing, - # boolean: !matching_item.boolean, - # only: FactoryBot.build(:only, - # boolean: !matching_item.only.boolean)) - # end - # let(:instruction_multi_value) do - # [ - # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - # ] - # end - # let(:instructions) do - # { - # path => instruction_multi_value - # } - # end - - # if type.presence_in(%i[binary datetime decimal float integer]) && operator == :gt_any - # it { expect(result_ids).not_to include matching_item.id } - # else - # end - # end - # end - # end end end end diff --git a/spec/active_set/filter/scopes_spec.rb b/spec/active_set/filter/scopes_spec.rb index 4518fac..a3e3493 100644 --- a/spec/active_set/filter/scopes_spec.rb +++ b/spec/active_set/filter/scopes_spec.rb @@ -6,32 +6,34 @@ before(:all) do @thing_1 = FactoryBot.create(:thing) @thing_2 = FactoryBot.create(:thing) - @active_set = ActiveSet.new(Thing.all) end describe '#filter' do - let(:result) { @active_set.filter(instructions) } + ['itself', 'to_a'].each do |relation_mutator| + context "/#{ relation_mutator == 'itself' ? 'ActiveRecord' : 'Enumberable' }/" do + before(:all) do + @active_set = ActiveSet.new(Thing.all.public_send(relation_mutator)) + end + let(:result) { @active_set.filter(instructions) } - ApplicationRecord::DB_FIELD_TYPES.each do |type| - [1, 2].each do |id| - let(:matching_item) { instance_variable_get("@thing_#{id}") } + ApplicationRecord::DB_FIELD_TYPES.each do |type| + [1, 2].each do |id| + let(:matching_item) { instance_variable_get("@thing_#{id}") } - all_possible_scope_paths_for(type).each do |path| - context "{ #{path}: }" do - let(:instructions) do - { - path => filter_value_for(object: matching_item, path: path) - } - end + all_possible_scope_paths_for(type).each do |path| + context "{ #{path}: }" do + let(:instructions) do + { + path => filter_value_for(object: matching_item, path: path) + } + end - if path.end_with?('_collection_method', '_scope_method') - if path.include?('computed_') - it { expect(result.map(&:id)).to eq [] } - else - it { expect(result.map(&:id)).to eq [matching_item.id] } + if path.end_with?('_collection_method', '_scope_method') + it { expect(result.map(&:id)).to eq [matching_item.id] } + else + it { expect(result.map(&:id)).to eq Thing.pluck(:id) } + end end - else - it { expect(result.map(&:id)).to eq Thing.pluck(:id) } end end end diff --git a/spec/active_set/filter/types_spec.rb b/spec/active_set/filter/types_spec.rb index 2dd2aef..a9c9cb6 100644 --- a/spec/active_set/filter/types_spec.rb +++ b/spec/active_set/filter/types_spec.rb @@ -44,20 +44,18 @@ ApplicationRecord::FILTERABLE_TYPES.combination(2).each do |type_1, type_2| [1, 2].each do |id| - context "matching @thing_#{id}" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - - all_possible_path_combinations_for(type_1, type_2).each do |path_1, path_2| - context "{ #{path_1}:, #{path_2} }" do - let(:instructions) do - { - path_1 => filter_value_for(object: matching_item, path: path_1), - path_2 => filter_value_for(object: matching_item, path: path_2) - } - end + let(:matching_item) { instance_variable_get("@thing_#{id}") } - it { expect(result.map(&:id)).to eq [matching_item.id] } + all_possible_path_combinations_for(type_1, type_2).each do |path_1, path_2| + context "{ #{path_1}:, #{path_2} }" do + let(:instructions) do + { + path_1 => filter_value_for(object: matching_item, path: path_1), + path_2 => filter_value_for(object: matching_item, path: path_2) + } end + + it { expect(result.map(&:id)).to eq [matching_item.id] } end end end From 006b312bf12a2238cff8898001cdff84c879bad2 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:42:38 +0200 Subject: [PATCH 08/56] Move the filtering strategies into directory/module namespaces --- .../filtering/active_record/strategy.rb | 87 +++++++++++++++++++ .../filtering/active_record_strategy.rb | 85 ------------------ .../filtering/enumerable/strategy.rb | 81 +++++++++++++++++ .../filtering/enumerable_strategy.rb | 79 ----------------- lib/active_set/filtering/operation.rb | 8 +- 5 files changed, 172 insertions(+), 168 deletions(-) create mode 100644 lib/active_set/filtering/active_record/strategy.rb delete mode 100644 lib/active_set/filtering/active_record_strategy.rb create mode 100644 lib/active_set/filtering/enumerable/strategy.rb delete mode 100644 lib/active_set/filtering/enumerable_strategy.rb diff --git a/lib/active_set/filtering/active_record/strategy.rb b/lib/active_set/filtering/active_record/strategy.rb new file mode 100644 index 0000000..440f06c --- /dev/null +++ b/lib/active_set/filtering/active_record/strategy.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require_relative '../active_record_set_instruction' +require 'active_support/core_ext/module/delegation' + +class ActiveSet + module Filtering + module ActiveRecord + class Strategy + delegate :attribute_model, + :arel_column, + :arel_operator, + :arel_value, + :arel_type, + :initial_relation, + :attribute, + to: :@set_instruction + + def initialize(set, attribute_instruction) + @set = set + @attribute_instruction = attribute_instruction + @set_instruction = ActiveRecordSetInstruction.new(attribute_instruction, set) + end + + def execute + return false unless @set.respond_to? :to_sql + + if execute_filter_operation? + statement = filter_operation + elsif execute_intersect_operation? + begin + statement = intersect_operation + rescue ArgumentError # thrown if merging a non-ActiveRecord::Relation + return false + end + else + return false + end + + statement + end + + private + + def execute_filter_operation? + return false unless attribute_model + return false unless attribute_model.respond_to?(:attribute_names) + return false unless attribute_model.attribute_names.include?(attribute) + + true + end + + def execute_intersect_operation? + return false unless attribute_model + return false unless attribute_model.respond_to?(attribute) + return false if attribute_model.method(attribute).arity.zero? + + true + end + + def filter_operation + initial_relation + .where( + arel_column.send( + arel_operator, + arel_value + ) + ) + end + + def intersect_operation + # NOTE: If merging relations that contain duplicate column conditions, + # the second condition will replace the first. + # e.g. Thing.where(id: [1,2]).merge(Thing.where(id: [2,3])) + # => [Thing<2>, Thing<3>] NOT [Thing<2>] + initial_relation + .merge( + attribute_model.public_send( + attribute, + arel_value + ) + ) + end + end + end + end +end diff --git a/lib/active_set/filtering/active_record_strategy.rb b/lib/active_set/filtering/active_record_strategy.rb deleted file mode 100644 index cb69872..0000000 --- a/lib/active_set/filtering/active_record_strategy.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require_relative '../active_record_set_instruction' -require 'active_support/core_ext/module/delegation' - -class ActiveSet - module Filtering - class ActiveRecordStrategy - delegate :attribute_model, - :arel_column, - :arel_operator, - :arel_value, - :arel_type, - :initial_relation, - :attribute, - to: :@set_instruction - - def initialize(set, attribute_instruction) - @set = set - @attribute_instruction = attribute_instruction - @set_instruction = ActiveRecordSetInstruction.new(attribute_instruction, set) - end - - def execute - return false unless @set.respond_to? :to_sql - - if execute_filter_operation? - statement = filter_operation - elsif execute_intersect_operation? - begin - statement = intersect_operation - rescue ArgumentError # thrown if merging a non-ActiveRecord::Relation - return false - end - else - return false - end - - statement - end - - private - - def execute_filter_operation? - return false unless attribute_model - return false unless attribute_model.respond_to?(:attribute_names) - return false unless attribute_model.attribute_names.include?(attribute) - - true - end - - def execute_intersect_operation? - return false unless attribute_model - return false unless attribute_model.respond_to?(attribute) - return false if attribute_model.method(attribute).arity.zero? - - true - end - - def filter_operation - initial_relation - .where( - arel_column.send( - arel_operator, - arel_value - ) - ) - end - - def intersect_operation - # NOTE: If merging relations that contain duplicate column conditions, - # the second condition will replace the first. - # e.g. Thing.where(id: [1,2]).merge(Thing.where(id: [2,3])) - # => [Thing<2>, Thing<3>] NOT [Thing<2>] - initial_relation - .merge( - attribute_model.public_send( - attribute, - arel_value - ) - ) - end - end - end -end diff --git a/lib/active_set/filtering/enumerable/strategy.rb b/lib/active_set/filtering/enumerable/strategy.rb new file mode 100644 index 0000000..faf0780 --- /dev/null +++ b/lib/active_set/filtering/enumerable/strategy.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require_relative '../enumerable_set_instruction' +require 'active_support/core_ext/module/delegation' + +class ActiveSet + module Filtering + module Enumerable + class Strategy + delegate :attribute_instance, + :attribute_class, + :attribute_value, + :attribute_value_for, + :operator, + :attribute, + to: :@set_instruction + + def initialize(set, attribute_instruction) + @set = set + @attribute_instruction = attribute_instruction + @set_instruction = EnumerableSetInstruction.new(attribute_instruction, set) + end + + def execute + return false unless @set.respond_to? :select + + if execute_filter_operation? + set = filter_operation + elsif execute_intersect_operation? + begin + set = intersect_operation + rescue TypeError # thrown if intersecting with a non-Array + return false + end + else + return false + end + + set + end + + private + + def execute_filter_operation? + return false unless attribute_instance + return false unless attribute_instance.respond_to?(attribute) + return false if attribute_instance.method(attribute).arity.positive? + + true + end + + def execute_intersect_operation? + return false unless attribute_class + return false unless attribute_class.respond_to?(attribute) + return false if attribute_class.method(attribute).arity.zero? + + true + end + + def filter_operation + @set.select do |item| + attribute_value_for(item) + .public_send( + operator, + attribute_value + ) + end + end + + def intersect_operation + other_set = attribute_class + .public_send( + attribute, + attribute_value + ) + @set & other_set + end + end + end + end +end diff --git a/lib/active_set/filtering/enumerable_strategy.rb b/lib/active_set/filtering/enumerable_strategy.rb deleted file mode 100644 index 5bc50bc..0000000 --- a/lib/active_set/filtering/enumerable_strategy.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -require_relative '../enumerable_set_instruction' -require 'active_support/core_ext/module/delegation' - -class ActiveSet - module Filtering - class EnumerableStrategy - delegate :attribute_instance, - :attribute_class, - :attribute_value, - :attribute_value_for, - :operator, - :attribute, - to: :@set_instruction - - def initialize(set, attribute_instruction) - @set = set - @attribute_instruction = attribute_instruction - @set_instruction = EnumerableSetInstruction.new(attribute_instruction, set) - end - - def execute - return false unless @set.respond_to? :select - - if execute_filter_operation? - set = filter_operation - elsif execute_intersect_operation? - begin - set = intersect_operation - rescue TypeError # thrown if intersecting with a non-Array - return false - end - else - return false - end - - set - end - - private - - def execute_filter_operation? - return false unless attribute_instance - return false unless attribute_instance.respond_to?(attribute) - return false if attribute_instance.method(attribute).arity.positive? - - true - end - - def execute_intersect_operation? - return false unless attribute_class - return false unless attribute_class.respond_to?(attribute) - return false if attribute_class.method(attribute).arity.zero? - - true - end - - def filter_operation - @set.select do |item| - attribute_value_for(item) - .public_send( - operator, - attribute_value - ) - end - end - - def intersect_operation - other_set = attribute_class - .public_send( - attribute, - attribute_value - ) - @set & other_set - end - end - end -end diff --git a/lib/active_set/filtering/operation.rb b/lib/active_set/filtering/operation.rb index dfdc580..e524f97 100644 --- a/lib/active_set/filtering/operation.rb +++ b/lib/active_set/filtering/operation.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true require_relative '../attribute_instruction' -require_relative './enumerable_strategy' -require_relative './active_record_strategy' +require_relative './enumerable/strategy' +require_relative './active_record/strategy' class ActiveSet module Filtering @@ -17,7 +17,7 @@ def execute .map { |k, v| AttributeInstruction.new(k, v) } activerecord_filtered_set = attribute_instructions.reduce(@set) do |set, attribute_instruction| - maybe_set_or_false = ActiveRecordStrategy.new(set, attribute_instruction).execute + maybe_set_or_false = ActiveRecord::Strategy.new(set, attribute_instruction).execute next set unless maybe_set_or_false attribute_instruction.processed = true @@ -27,7 +27,7 @@ def execute return activerecord_filtered_set if attribute_instructions.all?(&:processed?) attribute_instructions.reject(&:processed?).reduce(activerecord_filtered_set) do |set, attribute_instruction| - maybe_set_or_false = EnumerableStrategy.new(set, attribute_instruction).execute + maybe_set_or_false = Enumerable::Strategy.new(set, attribute_instruction).execute next set unless maybe_set_or_false attribute_instruction.processed = true From e22fc213541b79b80839dfc0b043cf7bceccbb02 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 21:05:58 +0200 Subject: [PATCH 09/56] Begin separating global set_instructions from operation-specific set-instructions with the filtering operation --- .../active_record_set_instruction.rb | 44 ++++++++++--------- lib/active_set/enumerable_set_instruction.rb | 39 ++++++---------- .../active_record/set_instruction.rb | 12 +++++ .../filtering/active_record/strategy.rb | 6 +-- .../filtering/enumerable/set_instruction.rb | 32 ++++++++++++++ .../filtering/enumerable/strategy.rb | 12 ++--- 6 files changed, 89 insertions(+), 56 deletions(-) create mode 100644 lib/active_set/filtering/active_record/set_instruction.rb create mode 100644 lib/active_set/filtering/enumerable/set_instruction.rb diff --git a/lib/active_set/active_record_set_instruction.rb b/lib/active_set/active_record_set_instruction.rb index ee4ab37..3c40d71 100644 --- a/lib/active_set/active_record_set_instruction.rb +++ b/lib/active_set/active_record_set_instruction.rb @@ -14,23 +14,6 @@ def initial_relation @set.eager_load(@attribute_instruction.associations_hash) end - def arel_type - attribute_model - .columns_hash[@attribute_instruction.attribute] - .type - end - - def arel_table - # This is to work around an bug in ActiveRecord, - # where BINARY fields aren't found properly when using - # the `arel_table` class method to build an ARel::Node - if arel_type == :binary - Arel::Table.new(attribute_model.table_name) - else - attribute_model.arel_table - end - end - # rubocop:disable Lint/UnderscorePrefixedVariableName def arel_column _arel_column = arel_table[@attribute_instruction.attribute] @@ -53,10 +36,6 @@ def arel_value end # rubocop:enable Lint/UnderscorePrefixedVariableName - def case_insensitive_operation? - @attribute_instruction.case_insensitive? && arel_type.presence_in(%i[string text]) - end - def attribute_model return @set.klass if @attribute_instruction.associations_array.empty? return @attribute_model if defined? @attribute_model @@ -67,5 +46,28 @@ def attribute_model obj.reflections[assoc.to_s]&.klass end end + + private + + def arel_type + attribute_model + .columns_hash[@attribute_instruction.attribute] + .type + end + + def arel_table + # This is to work around an bug in ActiveRecord, + # where BINARY fields aren't found properly when using + # the `arel_table` class method to build an ARel::Node + if arel_type == :binary + Arel::Table.new(attribute_model.table_name) + else + attribute_model.arel_table + end + end + + def case_insensitive_operation? + @attribute_instruction.case_insensitive? && arel_type.presence_in(%i[string text]) + end end end diff --git a/lib/active_set/enumerable_set_instruction.rb b/lib/active_set/enumerable_set_instruction.rb index 56ad0ff..30ac506 100644 --- a/lib/active_set/enumerable_set_instruction.rb +++ b/lib/active_set/enumerable_set_instruction.rb @@ -9,40 +9,27 @@ def initialize(attribute_instruction, set) end def attribute_value_for(item) - item_value = @attribute_instruction - .value_for(item: item) - item_value = item_value.downcase if case_insensitive_operation_for?(item_value) - item_value + @item_values ||= Hash.new do |h, key| + item_value = @attribute_instruction.value_for(item: key) + item_value = item_value.downcase if case_insensitive_operation_for?(item_value) + h[key] = item_value + end + + @item_values[item] end - # rubocop:disable Lint/UnderscorePrefixedVariableName - def attribute_value - _attribute_value = @attribute_instruction.value - _attribute_value = _attribute_value.downcase if case_insensitive_operation_for?(_attribute_value) - _attribute_value + def instruction_value + return @instruction_value if defined? @instruction_value + + instruction_value = @attribute_instruction.value + instruction_value = instruction_value.downcase if case_insensitive_operation_for?(instruction_value) + @instruction_value = instruction_value end - # rubocop:enable Lint/UnderscorePrefixedVariableName def case_insensitive_operation_for?(value) return false unless @attribute_instruction.case_insensitive? value.is_a?(String) || value.is_a?(Symbol) end - - def attribute_instance - set_item = @set.find(&:present?) - return set_item if @attribute_instruction.associations_array.empty? - return @attribute_model if defined? @attribute_model - - @attribute_model = @attribute_instruction - .associations_array - .reduce(set_item) do |obj, assoc| - obj.public_send(assoc) - end - end - - def attribute_class - attribute_instance&.class - end end end diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb new file mode 100644 index 0000000..d3ebfba --- /dev/null +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require_relative '../../active_record_set_instruction' + +class ActiveSet + module Filtering + module ActiveRecord + class SetInstruction < ActiveRecordSetInstruction + end + end + end +end diff --git a/lib/active_set/filtering/active_record/strategy.rb b/lib/active_set/filtering/active_record/strategy.rb index 440f06c..18f32a3 100644 --- a/lib/active_set/filtering/active_record/strategy.rb +++ b/lib/active_set/filtering/active_record/strategy.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative '../active_record_set_instruction' +require_relative './set_instruction' require 'active_support/core_ext/module/delegation' class ActiveSet @@ -14,12 +14,12 @@ class Strategy :arel_type, :initial_relation, :attribute, - to: :@set_instruction + to: :@set_instruction def initialize(set, attribute_instruction) @set = set @attribute_instruction = attribute_instruction - @set_instruction = ActiveRecordSetInstruction.new(attribute_instruction, set) + @set_instruction = SetInstruction.new(attribute_instruction, set) end def execute diff --git a/lib/active_set/filtering/enumerable/set_instruction.rb b/lib/active_set/filtering/enumerable/set_instruction.rb new file mode 100644 index 0000000..bdd95b7 --- /dev/null +++ b/lib/active_set/filtering/enumerable/set_instruction.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require_relative '../../enumerable_set_instruction' + +class ActiveSet + module Filtering + module Enumerable + class SetInstruction < EnumerableSetInstruction + def set_item + return @set_item if defined? @set_item + + @set_item = @set.find(&:present?) + end + + def attribute_instance + return set_item if @attribute_instruction.associations_array.empty? + return @attribute_model if defined? @attribute_model + + @attribute_model = @attribute_instruction + .associations_array + .reduce(set_item) do |obj, assoc| + obj.public_send(assoc) + end + end + + def attribute_class + attribute_instance&.class + end + end + end + end +end diff --git a/lib/active_set/filtering/enumerable/strategy.rb b/lib/active_set/filtering/enumerable/strategy.rb index faf0780..1d90b63 100644 --- a/lib/active_set/filtering/enumerable/strategy.rb +++ b/lib/active_set/filtering/enumerable/strategy.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative '../enumerable_set_instruction' +require_relative './set_instruction' require 'active_support/core_ext/module/delegation' class ActiveSet @@ -9,16 +9,16 @@ module Enumerable class Strategy delegate :attribute_instance, :attribute_class, - :attribute_value, + :instruction_value, :attribute_value_for, :operator, :attribute, - to: :@set_instruction + to: :@set_instruction def initialize(set, attribute_instruction) @set = set @attribute_instruction = attribute_instruction - @set_instruction = EnumerableSetInstruction.new(attribute_instruction, set) + @set_instruction = SetInstruction.new(attribute_instruction, set) end def execute @@ -62,7 +62,7 @@ def filter_operation attribute_value_for(item) .public_send( operator, - attribute_value + instruction_value ) end end @@ -71,7 +71,7 @@ def intersect_operation other_set = attribute_class .public_send( attribute, - attribute_value + instruction_value ) @set & other_set end From 6e53b4841a85a22af4ab5b4fa0739bc57d507328 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 21:12:38 +0200 Subject: [PATCH 10/56] Fix enumerable intersection filtering when working across associations --- lib/active_set/enumerable_set_instruction.rb | 5 ++++- lib/active_set/filtering/enumerable_strategy.rb | 13 +++++++++++-- spec/active_set/filter/scopes_spec.rb | 6 +----- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/active_set/enumerable_set_instruction.rb b/lib/active_set/enumerable_set_instruction.rb index 56ad0ff..70c71bb 100644 --- a/lib/active_set/enumerable_set_instruction.rb +++ b/lib/active_set/enumerable_set_instruction.rb @@ -30,7 +30,6 @@ def case_insensitive_operation_for?(value) end def attribute_instance - set_item = @set.find(&:present?) return set_item if @attribute_instruction.associations_array.empty? return @attribute_model if defined? @attribute_model @@ -41,6 +40,10 @@ def attribute_instance end end + def set_item + @set.find(&:present?) + end + def attribute_class attribute_instance&.class end diff --git a/lib/active_set/filtering/enumerable_strategy.rb b/lib/active_set/filtering/enumerable_strategy.rb index 5bc50bc..286bd32 100644 --- a/lib/active_set/filtering/enumerable_strategy.rb +++ b/lib/active_set/filtering/enumerable_strategy.rb @@ -12,6 +12,8 @@ class EnumerableStrategy :attribute_value_for, :operator, :attribute, + :set_item, + :resource_for, to: :@set_instruction def initialize(set, attribute_instruction) @@ -67,11 +69,18 @@ def filter_operation end def intersect_operation - other_set = attribute_class - .public_send( + other_set = attribute_class.public_send( attribute, attribute_value ) + if attribute_class != set_item.class + other_set = begin + @set.select { |item| resource_for(item: item)&.presence_in other_set } + rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil + nil + end + end + @set & other_set end end diff --git a/spec/active_set/filter/scopes_spec.rb b/spec/active_set/filter/scopes_spec.rb index 4518fac..b39960b 100644 --- a/spec/active_set/filter/scopes_spec.rb +++ b/spec/active_set/filter/scopes_spec.rb @@ -25,11 +25,7 @@ end if path.end_with?('_collection_method', '_scope_method') - if path.include?('computed_') - it { expect(result.map(&:id)).to eq [] } - else - it { expect(result.map(&:id)).to eq [matching_item.id] } - end + it { expect(result.map(&:id)).to eq [matching_item.id] } else it { expect(result.map(&:id)).to eq Thing.pluck(:id) } end From 4dd26f310cddee57acecccfccaf61b138d93c812 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 20:37:34 +0200 Subject: [PATCH 11/56] Change the default spec running behavior Must manually turn on coverage and failure reports --- spec/database_cleaner_helper.rb | 22 ++++++++++++++++++++ spec/inspect_failure_helper.rb | 21 +++++++++++++++++++ spec/simplecov_helper.rb | 2 +- spec/spec_helper.rb | 37 ++------------------------------- 4 files changed, 46 insertions(+), 36 deletions(-) create mode 100644 spec/database_cleaner_helper.rb create mode 100644 spec/inspect_failure_helper.rb diff --git a/spec/database_cleaner_helper.rb b/spec/database_cleaner_helper.rb new file mode 100644 index 0000000..c07559d --- /dev/null +++ b/spec/database_cleaner_helper.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +RSpec.configure do |config| + config.before(:suite) do + DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.strategy = :transaction + end + + config.before(:all) do + DatabaseCleaner.start + end + + config.around(:each) do |example| + DatabaseCleaner.cleaning do + example.run + end + end + + config.after(:all) do + DatabaseCleaner.clean + end +end diff --git a/spec/inspect_failure_helper.rb b/spec/inspect_failure_helper.rb new file mode 100644 index 0000000..bade4e4 --- /dev/null +++ b/spec/inspect_failure_helper.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +if ENV['INSPECT_FAILURE'] == 'true' + RSpec.configure do |config| + # Give a detailed report of all relevant data if a spec fails + # http://bensnape.com/2014/08/01/rspec-after-failure-hook/ + config.after(:each) do |example| + next unless example.exception + + let_data = @__memoized.instance_variable_get('@memoized') + ivar_data = instance_variables + .reject { |v| v.to_s.start_with?('@_') } + .reject { |v| v.presence_in %i[@example @fixture_cache @fixture_connections @connection_subscriber @loaded_fixtures] } + .map { |v| [v, instance_variable_get(v)] } + .to_h + + # https://www.jvt.me/posts/2019/03/29/pretty-printing-json-ruby/ + jj let_data.merge(ivar_data).transform_values(&:inspect) + end + end +end diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb index cfdabbd..5a52d8d 100644 --- a/spec/simplecov_helper.rb +++ b/spec/simplecov_helper.rb @@ -5,7 +5,7 @@ require 'simplecov-console' require 'codecov' -unless ENV['COVERAGE'] == 'false' +if ENV['COVERAGE'] == 'true' ROOT = File.expand_path('..', __dir__) # SimpleCov.minimum_coverage 99 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2b818f0..6d84168 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'simplecov_helper' +require 'database_cleaner_helper' +require 'inspect_failure_helper' require 'bundler' require 'combustion' @@ -46,39 +48,4 @@ nil end end - - config.before(:suite) do - DatabaseCleaner.clean_with(:truncation) - DatabaseCleaner.strategy = :transaction - end - - config.before(:all) do - DatabaseCleaner.start - end - - config.around(:each) do |example| - DatabaseCleaner.cleaning do - example.run - end - end - - config.after(:all) do - DatabaseCleaner.clean - end - - # Give a detailed report of all relevant data if a spec fails - # http://bensnape.com/2014/08/01/rspec-after-failure-hook/ - config.after(:each) do |example| - next unless example.exception - - let_data = @__memoized.instance_variable_get('@memoized') - ivar_data = instance_variables - .reject { |v| v.to_s.start_with?('@_') } - .reject { |v| v.presence_in %i[@example @fixture_cache @fixture_connections @connection_subscriber @loaded_fixtures] } - .map { |v| [v, instance_variable_get(v)] } - .to_h - - # https://www.jvt.me/posts/2019/03/29/pretty-printing-json-ruby/ - jj let_data.merge(ivar_data).transform_values(&:inspect) - end end From e6a4fbcf5b733eb23e0a163e5bd3766a7acf0c08 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 21:48:47 +0200 Subject: [PATCH 12/56] Fix the generic factory date sequence to never output an invalid date (like 19XX-04-31) --- spec/factories/generic.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories/generic.rb b/spec/factories/generic.rb index 86e09a1..964f3cc 100644 --- a/spec/factories/generic.rb +++ b/spec/factories/generic.rb @@ -15,9 +15,9 @@ def fit_to_minmax(number, max:, min: 1) sequence(:boolean, &:even?) sequence(:date) do |n| [ - "19#{rand(99).to_s.rjust(2, '0')}", + "19#{fit_to_minmax(n, max: 99).to_s.rjust(2, '0')}", fit_to_minmax(n, max: 12).to_s.rjust(2, '0'), - fit_to_minmax(n, max: 31).to_s.rjust(2, '0') + fit_to_minmax(n, max: 28).to_s.rjust(2, '0') ].join('-') end sequence(:datetime) do |n| From 675ed605085da05786a9d4f29e6ae1d327438615 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 21:49:17 +0200 Subject: [PATCH 13/56] Update the Rakefile to have TravisCI run the full spec setup --- Rakefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 82bb534..859e9e4 100644 --- a/Rakefile +++ b/Rakefile @@ -5,4 +5,10 @@ require 'rspec/core/rake_task' RSpec::Core::RakeTask.new(:spec) -task default: :spec +task :full_spec do + ENV['COVERAGE'] = 'true' + ENV['INSPECT_FAILURE'] = 'true' + Rake::Task["spec"].invoke +end + +task default: :full_spec From ddf9a5876f88529f8f1e7103e3d9a80288127551 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 21:57:48 +0200 Subject: [PATCH 14/56] Fix rubocop issues --- Rakefile | 2 +- lib/action_set.rb | 2 ++ lib/action_set/attribute_value.rb | 2 +- lib/active_set/filtering/enumerable_strategy.rb | 10 +++++----- lib/helpers/flatten_keys_of.rb | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Rakefile b/Rakefile index 859e9e4..c08f09e 100644 --- a/Rakefile +++ b/Rakefile @@ -8,7 +8,7 @@ RSpec::Core::RakeTask.new(:spec) task :full_spec do ENV['COVERAGE'] = 'true' ENV['INSPECT_FAILURE'] = 'true' - Rake::Task["spec"].invoke + Rake::Task['spec'].invoke end task default: :full_spec diff --git a/lib/action_set.rb b/lib/action_set.rb index df0d3f1..c266ffd 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -76,6 +76,7 @@ def paginate_instructions paginate_params.transform_values(&:to_i) end + # rubocop:disable Metrics/AbcSize def export_instructions {}.tap do |struct| struct[:format] = export_params[:format] || request.format.symbol @@ -97,6 +98,7 @@ def export_instructions end end end + # rubocop:enable Metrics/AbcSize def filter_params params.fetch(:filter, {}).to_unsafe_hash diff --git a/lib/action_set/attribute_value.rb b/lib/action_set/attribute_value.rb index 62aa42d..95483d3 100644 --- a/lib/action_set/attribute_value.rb +++ b/lib/action_set/attribute_value.rb @@ -11,7 +11,7 @@ def initialize(value) def cast(to:) adapters.reduce(nil) do |_, adapter| mayble_value_or_nil = adapter.new(@raw, to).process - next if mayble_value_or_nil.nil? + next nil if mayble_value_or_nil.nil? return mayble_value_or_nil end diff --git a/lib/active_set/filtering/enumerable_strategy.rb b/lib/active_set/filtering/enumerable_strategy.rb index 286bd32..9c3f992 100644 --- a/lib/active_set/filtering/enumerable_strategy.rb +++ b/lib/active_set/filtering/enumerable_strategy.rb @@ -70,14 +70,14 @@ def filter_operation def intersect_operation other_set = attribute_class.public_send( - attribute, - attribute_value - ) + attribute, + attribute_value + ) if attribute_class != set_item.class other_set = begin @set.select { |item| resource_for(item: item)&.presence_in other_set } - rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil - nil + rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil + nil end end diff --git a/lib/helpers/flatten_keys_of.rb b/lib/helpers/flatten_keys_of.rb index 9e9bad5..a7e3ad7 100644 --- a/lib/helpers/flatten_keys_of.rb +++ b/lib/helpers/flatten_keys_of.rb @@ -24,7 +24,7 @@ # => { "key"=>"value", "nested.key"=>"nested_value", "array.0"=>0, "array.1"=>1, "array.2"=>2 } # refactored from https://stackoverflow.com/a/23861946/2884386 -def flatten_keys_of(input, keys = [], output = {}, flattener: ->(*keys) { keys }, flatten_arrays: false) +def flatten_keys_of(input, keys = [], output = {}, flattener: ->(*k) { k }, flatten_arrays: false) if input.is_a?(Hash) input.each do |key, value| flatten_keys_of( From 7c92767fa445cc288b42c2a98ad762fc802d92ec Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 22:40:40 +0200 Subject: [PATCH 15/56] Get the Enumerable filtering strategy working with the new SetInstruction --- lib/active_set/filtering/enumerable/strategy.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/active_set/filtering/enumerable/strategy.rb b/lib/active_set/filtering/enumerable/strategy.rb index ffc2d47..8e825c2 100644 --- a/lib/active_set/filtering/enumerable/strategy.rb +++ b/lib/active_set/filtering/enumerable/strategy.rb @@ -13,6 +13,8 @@ class Strategy :attribute_value_for, :operator, :attribute, + :set_item, + :resource_for, to: :@set_instruction def initialize(set, attribute_instruction) @@ -70,7 +72,7 @@ def filter_operation def intersect_operation other_set = attribute_class.public_send( attribute, - attribute_value + instruction_value ) if attribute_class != set_item.class other_set = begin From f5de6bfa5111ac05125c215ac0055f6fa6e110aa Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sat, 29 Jun 2019 22:41:17 +0200 Subject: [PATCH 16/56] Move filtering specific SetInstruction from the top-level class into the namespaced class --- .../active_record_set_instruction.rb | 29 +++++++------------ .../active_record/set_instruction.rb | 14 +++++++++ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/lib/active_set/active_record_set_instruction.rb b/lib/active_set/active_record_set_instruction.rb index 3c40d71..bb4c8e0 100644 --- a/lib/active_set/active_record_set_instruction.rb +++ b/lib/active_set/active_record_set_instruction.rb @@ -9,32 +9,23 @@ def initialize(attribute_instruction, set) end def initial_relation - return @set if @attribute_instruction.associations_array.empty? + return @initial_relation if defined? @initial_relation - @set.eager_load(@attribute_instruction.associations_hash) + @initial_relation = if @attribute_instruction.associations_array.empty? + @set + else + @set.eager_load(@attribute_instruction.associations_hash) + end end - # rubocop:disable Lint/UnderscorePrefixedVariableName def arel_column - _arel_column = arel_table[@attribute_instruction.attribute] - return _arel_column.lower if case_insensitive_operation? + return @arel_column if defined? @arel_column - _arel_column - end - # rubocop:enable Lint/UnderscorePrefixedVariableName - - def arel_operator - @attribute_instruction.operator(default: :eq) - end - - # rubocop:disable Lint/UnderscorePrefixedVariableName - def arel_value - _arel_value = @attribute_instruction.value - return _arel_value.downcase if case_insensitive_operation? + arel_column = arel_table[@attribute_instruction.attribute] + arel_column = arel_column.lower if case_insensitive_operation? - _arel_value + @arel_column = arel_column end - # rubocop:enable Lint/UnderscorePrefixedVariableName def attribute_model return @set.klass if @attribute_instruction.associations_array.empty? diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb index d3ebfba..88e3403 100644 --- a/lib/active_set/filtering/active_record/set_instruction.rb +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -6,6 +6,20 @@ class ActiveSet module Filtering module ActiveRecord class SetInstruction < ActiveRecordSetInstruction + def arel_value + return @arel_value if defined? @arel_value + + arel_value = @attribute_instruction.value + arel_value = arel_value.downcase if case_insensitive_operation? + + @arel_value = arel_value + end + + def arel_operator + return @arel_operator if defined? @arel_operator + + @arel_operator = @attribute_instruction.operator(default: :eq) + end end end end From 7075da3e0996f872f173c6028cc04a4222a7cf55 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 08:11:26 +0200 Subject: [PATCH 17/56] Add the various 'reducer' predicates --- lib/active_set/filtering/constants.rb | 155 +++++++++++++++++++++++--- 1 file changed, 140 insertions(+), 15 deletions(-) diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index 30895a6..717463b 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -3,78 +3,203 @@ class ActiveSet module Filtering module Constants - BASE_PREDICATES = { + PREDICATES = { EQ: { type: :binary, compound: false, behavior: :inclusive, - shorthand: '=='.to_sym + shorthand: :'==' }, NOT_EQ: { type: :binary, compound: false, behavior: :exclusive, - shorthand: '!='.to_sym + shorthand: :'!=' }, + EQ_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E==' + }, + EQ_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A==' + }, + NOT_EQ_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E!=' + }, + NOT_EQ_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A!=' + }, + IN: { type: :binary, compound: true, behavior: :inclusive, - shorthand: '<<'.to_sym + shorthand: :'<<' }, NOT_IN: { type: :binary, compound: true, behavior: :exclusive, - shorthand: '!<'.to_sym + shorthand: :'!<' }, + IN_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E<<' + }, + IN_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A<<' + }, + NOT_IN_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E!<' + }, + NOT_IN_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A!<' + }, + MATCHES: { type: :binary, compound: false, behavior: :inclusive, - shorthand: '=~'.to_sym + shorthand: :'=~' }, DOES_NOT_MATCH: { type: :binary, compound: false, behavior: :exclusive, - shorthand: '!~'.to_sym + shorthand: :'!~' }, + MATCHES_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E=~' + }, + MATCHES_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A=~' + }, + DOES_NOT_MATCH_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E!~' + }, + DOES_NOT_MATCH_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A!~' + }, + LT: { type: :binary, compound: false, behavior: :exclusive, - shorthand: '<'.to_sym + shorthand: :'<' }, LTEQ: { type: :binary, compound: false, behavior: :inclusive, - shorthand: '<='.to_sym + shorthand: :'<=' + }, + LT_ANY: { + type: :binary, + compound: true, + behavior: :inconclusive, + shorthand: :'E<' + }, + LT_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A<' + }, + LTEQ_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E<=' + }, + LTEQ_ALL: { + type: :binary, + compound: true, + behavior: :inconclusive, + shorthand: :'A<=' }, + GT: { type: :binary, compound: false, behavior: :exclusive, - shorthand: '>'.to_sym + shorthand: :'>' }, GTEQ: { type: :binary, compound: false, behavior: :inclusive, - shorthand: '>='.to_sym + shorthand: :'>=' }, - BETWEEN: { + GT_ANY: { type: :binary, compound: true, behavior: :inconclusive, - shorthand: '..'.to_sym + shorthand: :'E>' }, - NOT_BETWEEN: { + GT_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A>' + }, + GTEQ_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E>=' + }, + GTEQ_ALL: { type: :binary, compound: true, behavior: :inconclusive, - shorthand: '!.'.to_sym + shorthand: :'A>=' + }, + + BETWEEN: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'..' + }, + NOT_BETWEEN: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'!.' } }.freeze end From b6a9abb786e593acf79cd3208362b43f0c7c771d Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 08:11:48 +0200 Subject: [PATCH 18/56] Add the various 'reducer' predicates to the 2 strategy operators --- .../filtering/active_record/operators.rb | 77 +++++++++++- .../filtering/enumerable/operators.rb | 112 ++++++++++++++++-- 2 files changed, 177 insertions(+), 12 deletions(-) diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index 9411d62..7033e5f 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -6,50 +6,117 @@ class ActiveSet module Filtering module ActiveRecord module Operators - BASE_PREDICATES = { + PREDICATES = { EQ: { operator: :eq }, NOT_EQ: { operator: :not_eq }, + EQ_ANY: { + operator: :eq_any + }, + EQ_ALL: { + operator: :eq_all + }, + NOT_EQ_ANY: { + operator: :not_eq_any + }, + NOT_EQ_ALL: { + operator: :not_eq_all + }, + IN: { operator: :in }, NOT_IN: { operator: :not_in }, + IN_ANY: { + operator: :in_any + }, + IN_ALL: { + operator: :in_all + }, + NOT_IN_ANY: { + operator: :not_in_any + }, + NOT_IN_ALL: { + operator: :not_in_all + }, + MATCHES: { operator: :matches }, DOES_NOT_MATCH: { operator: :does_not_match }, + MATCHES_ANY: { + operator: :matches_any + }, + MATCHES_ALL: { + operator: :matches_all + }, + DOES_NOT_MATCH_ANY: { + operator: :does_not_match_any + }, + DOES_NOT_MATCH_ALL: { + operator: :does_not_match_all + }, + LT: { operator: :lt }, LTEQ: { operator: :lteq }, + LT_ANY: { + operator: :lt_any + }, + LT_ALL: { + operator: :lt_all + }, + LTEQ_ANY: { + operator: :lteq_any + }, + LTEQ_ALL: { + operator: :lteq_all + }, + GT: { operator: :gt }, GTEQ: { operator: :gteq }, + GT_ANY: { + operator: :gt_any + }, + GT_ALL: { + operator: :gt_all + }, + GTEQ_ANY: { + operator: :gteq_any + }, + GTEQ_ALL: { + operator: :gteq_all + }, + BETWEEN: { - operator: :between + operator: :between, + query_attribute_transformer: ->(query) { Range.new(*query.sort) } }, NOT_BETWEEN: { - operator: :not_between + operator: :not_between, + query_attribute_transformer: ->(query) { Range.new(*query.sort) } } }.freeze def self.get(operator_name) operator_key = operator_name.to_s.upcase.to_sym - base_operator_hash = Constants::BASE_PREDICATES.fetch(operator_key, {}) - this_operator_hash = Operators::BASE_PREDICATES.fetch(operator_key, {}) + base_operator_hash = Constants::PREDICATES.fetch(operator_key, {}) + this_operator_hash = Operators::PREDICATES.fetch(operator_key, {}) base_operator_hash.merge(this_operator_hash) end diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index 5b21916..0cbf470 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -6,13 +6,30 @@ class ActiveSet module Filtering module Enumerable module Operators - BASE_PREDICATES = { + PREDICATES = { EQ: { operator: :'==' }, NOT_EQ: { operator: :'!=' }, + EQ_ANY: { + operator: :'==', + reducer: :any? + }, + EQ_ALL: { + operator: :'==', + reducer: :all? + }, + NOT_EQ_ANY: { + operator: :'!=', + reducer: :any? + }, + NOT_EQ_ALL: { + operator: :'!=', + reducer: :all? + }, + IN: { operator: :presence_in }, @@ -20,41 +37,122 @@ module Operators operator: :presence_in, result_transformer: ->(result) { !result } }, + IN_ANY: { + operator: :presence_in, + reducer: :any? + }, + IN_ALL: { + operator: :presence_in, + reducer: :all? + }, + NOT_IN_ANY: { + operator: :presence_in, + reducer: :any?, + result_transformer: ->(result) { !result } + }, + NOT_IN_ALL: { + operator: :presence_in, + reducer: :all?, + result_transformer: ->(result) { !result } + }, + MATCHES: { operator: :'=~', object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{attribute}/ } + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } }, DOES_NOT_MATCH: { operator: :'!~', object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{attribute}/ } + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + }, + MATCHES_ANY: { + operator: :'=~', + reducer: :any?, + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + }, + MATCHES_ALL: { + operator: :'=~', + reducer: :all?, + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + }, + DOES_NOT_MATCH_ANY: { + operator: :'!~', + reducer: :any?, + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } }, + DOES_NOT_MATCH_ALL: { + operator: :'!~', + reducer: :all?, + object_attribute_transformer: ->(attribute) { attribute.to_s }, + query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + }, + LT: { operator: :'<' }, LTEQ: { operator: :'<=' }, + LT_ANY: { + operator: :'<', + reducer: :any? + }, + LT_ALL: { + operator: :'<', + reducer: :all? + }, + LTEQ_ANY: { + operator: :'<=', + reducer: :any? + }, + LTEQ_ALL: { + operator: :'<=', + reducer: :all? + }, + GT: { operator: :'>' }, GTEQ: { operator: :'>=' }, + GT_ANY: { + operator: :'>', + reducer: :any? + }, + GT_ALL: { + operator: :'>', + reducer: :all? + }, + GTEQ_ANY: { + operator: :'>=', + reducer: :any? + }, + GTEQ_ALL: { + operator: :'>=', + reducer: :all? + }, + BETWEEN: { - operator: :between + operator: :cover?, + query_attribute_transformer: ->(query) { Range.new(*query.sort) } }, NOT_BETWEEN: { - operator: :not_between + operator: :cover?, + query_attribute_transformer: ->(query) { Range.new(*query.sort) }, + result_transformer: ->(result) { !result } } }.freeze def self.get(operator_name) operator_key = operator_name.to_s.upcase.to_sym - base_operator_hash = Constants::BASE_PREDICATES.fetch(operator_key, {}) - this_operator_hash = Operators::BASE_PREDICATES.fetch(operator_key, {}) + base_operator_hash = Constants::PREDICATES.fetch(operator_key, {}) + this_operator_hash = Operators::PREDICATES.fetch(operator_key, {}) base_operator_hash.merge(this_operator_hash) end From 4c22c19f93ec4cdab22a5815b30e34080060dcd8 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 08:12:19 +0200 Subject: [PATCH 19/56] Update the various SetInstruction classes to work with all of the predicates now --- .../active_record_set_instruction.rb | 4 +- .../active_record/set_instruction.rb | 34 +++++++-- .../filtering/enumerable/set_instruction.rb | 74 ++++++++++++------- 3 files changed, 76 insertions(+), 36 deletions(-) diff --git a/lib/active_set/active_record_set_instruction.rb b/lib/active_set/active_record_set_instruction.rb index bb4c8e0..d723efa 100644 --- a/lib/active_set/active_record_set_instruction.rb +++ b/lib/active_set/active_record_set_instruction.rb @@ -42,8 +42,8 @@ def attribute_model def arel_type attribute_model - .columns_hash[@attribute_instruction.attribute] - .type + &.columns_hash[@attribute_instruction.attribute] + &.type end def arel_table diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb index 81a7d81..077d7f4 100644 --- a/lib/active_set/filtering/active_record/set_instruction.rb +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -8,21 +8,43 @@ module Filtering module ActiveRecord class SetInstruction < ActiveRecordSetInstruction def arel_operator - instruction_operator = @attribute_instruction.operator - return :eq unless instruction_operator - - operator_hash = Operators.get(instruction_operator) - operator_hash&.dig(:operator) + operator_hash.fetch(:operator, :eq) end def arel_value return @arel_value if defined? @arel_value - arel_value = @attribute_instruction.value + arel_value = guarantee_attribute_type(@attribute_instruction.value) + arel_value = query_attribute_for(arel_value) arel_value = arel_value.downcase if case_insensitive_operation? @arel_value = arel_value end + + private + + def query_attribute_for(value) + return value unless operator_hash.key?(:query_attribute_transformer) + + operator_hash[:query_attribute_transformer].call(value) + end + + def operator_hash + instruction_operator = @attribute_instruction.operator + + Operators.get(instruction_operator) + end + + def guarantee_attribute_type(attribute) + # Booleans don't respond to many operator methods, + # so we cast them to however the database expects them + return attribute if not arel_type == :boolean + return attribute.map { |a| guarantee_attribute_type(a) } if attribute.respond_to?(:each) + + sql_value = Arel::Nodes::Casted.new(attribute, arel_column).to_sql + sql_value = sql_value[/'(.*?)'/, 1] if sql_value.match? /'.*?'/ + sql_value + end end end end diff --git a/lib/active_set/filtering/enumerable/set_instruction.rb b/lib/active_set/filtering/enumerable/set_instruction.rb index b5f2316..2ef9857 100644 --- a/lib/active_set/filtering/enumerable/set_instruction.rb +++ b/lib/active_set/filtering/enumerable/set_instruction.rb @@ -8,11 +8,50 @@ module Filtering module Enumerable class SetInstruction < EnumerableSetInstruction def item_matches_query?(item) - result = object_attribute_for(item) - .public_send( - operator_method, - query_attribute - ) + return query_result_for(item, query_attribute_for(instruction_value)) if not operator_hash.key?(:reducer) + + instruction_value.public_send(operator_hash[:reducer]) do |value| + query_result_for(item, query_attribute_for(value)) + end + end + + def set_item + return @set_item if defined? @set_item + + @set_item = @set.find(&:present?) + end + + def attribute_instance + return set_item if @attribute_instruction.associations_array.empty? + return @attribute_model if defined? @attribute_model + + @attribute_model = @attribute_instruction + .associations_array + .reduce(set_item) do |obj, assoc| + obj.public_send(assoc) + end + end + + def attribute_class + attribute_instance&.class + end + + private + + def query_result_for(item, value) + result = if operator_method == :cover? && value.is_a?(Range) + value + .public_send( + operator_method, + object_attribute_for(item) + ) + else + object_attribute_for(item) + .public_send( + operator_method, + value + ) + end return result unless operator_hash.key?(:result_transformer) @@ -26,8 +65,8 @@ def object_attribute_for(item) operator_hash[:object_attribute_transformer].call(attribute) end - def query_attribute - attribute = guarantee_attribute_type(instruction_value) + def query_attribute_for(value) + attribute = guarantee_attribute_type(value) return attribute unless operator_hash.key?(:query_attribute_transformer) operator_hash[:query_attribute_transformer].call(attribute) @@ -52,27 +91,6 @@ def guarantee_attribute_type(attribute) attribute end - - def set_item - return @set_item if defined? @set_item - - @set_item = @set.find(&:present?) - end - - def attribute_instance - return set_item if @attribute_instruction.associations_array.empty? - return @attribute_model if defined? @attribute_model - - @attribute_model = @attribute_instruction - .associations_array - .reduce(set_item) do |obj, assoc| - obj.public_send(assoc) - end - end - - def attribute_class - attribute_instance&.class - end end end end From 27c7bceb6d401386864b2ac55b7a121455cb5a7c Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 08:12:52 +0200 Subject: [PATCH 20/56] Update the filtering predicates spec to properly test inclusion operators --- spec/active_set/filter/predicates_spec.rb | 50 ++++++++--------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index 96f7cac..ea2091f 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -2,26 +2,8 @@ require 'spec_helper' -# PREDICATE_OPERATORS = ActiveSet::Filtering::ActiveRecord::Constants::PREDICATE_OPERATORS -# INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :inclusive -# end -# EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :exclusive -# end -# INCONCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :inconclusive -# end - - RSpec.describe ActiveSet do - PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::BASE_PREDICATES + PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::PREDICATES INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| o[:compound] == false && o[:behavior] == :inclusive @@ -108,16 +90,17 @@ guaranteed_unique_object_for(matching_item, only: guaranteed_unique_object_for(matching_item.only)) end + let(:matching_value) { ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) } + let(:other_value) { ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) } let(:instruction_multi_value) do - [ - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - ] + if operator.to_s.include? 'IN_' + [ [matching_value], [other_value] ] + else + [ matching_value, other_value ] + end end let(:instructions) do - { - path => instruction_multi_value - } + { path => instruction_multi_value } end it { expect(result_ids).to include matching_item.id } @@ -136,16 +119,17 @@ guaranteed_unique_object_for(matching_item, only: guaranteed_unique_object_for(matching_item.only)) end + let(:matching_value) { ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item) } + let(:other_value) { ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) } let(:instruction_multi_value) do - [ - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - ] + if operator.to_s.include? 'IN_' + [ [matching_value], [other_value] ] + else + [ matching_value, other_value ] + end end let(:instructions) do - { - path => instruction_multi_value - } + { path => instruction_multi_value } end it { expect(result_ids).not_to include matching_item.id } From c4517e9caa1b1d05e64af3adacf695a7749c9e1c Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 08:21:18 +0200 Subject: [PATCH 21/56] Rubocop fixes --- .../filtering/active_record/operators.rb | 2 ++ .../active_record/set_instruction.rb | 4 +-- .../filtering/active_record/strategy.rb | 2 +- lib/active_set/filtering/constants.rb | 3 ++- .../filtering/enumerable/operators.rb | 2 ++ .../filtering/enumerable/set_instruction.rb | 26 +++++++++---------- .../filtering/enumerable/strategy.rb | 14 +++++----- 7 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index 7033e5f..b072ab8 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -5,6 +5,7 @@ class ActiveSet module Filtering module ActiveRecord + # rubocop:disable Metrics/ModuleLength module Operators PREDICATES = { EQ: { @@ -121,6 +122,7 @@ def self.get(operator_name) base_operator_hash.merge(this_operator_hash) end end + # rubocop:enable Metrics/ModuleLength end end end diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb index 077d7f4..65762f9 100644 --- a/lib/active_set/filtering/active_record/set_instruction.rb +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -38,11 +38,11 @@ def operator_hash def guarantee_attribute_type(attribute) # Booleans don't respond to many operator methods, # so we cast them to however the database expects them - return attribute if not arel_type == :boolean + return attribute if arel_type != :boolean return attribute.map { |a| guarantee_attribute_type(a) } if attribute.respond_to?(:each) sql_value = Arel::Nodes::Casted.new(attribute, arel_column).to_sql - sql_value = sql_value[/'(.*?)'/, 1] if sql_value.match? /'.*?'/ + sql_value = sql_value[/'(.*?)'/, 1] if sql_value.match?(/'.*?'/) sql_value end end diff --git a/lib/active_set/filtering/active_record/strategy.rb b/lib/active_set/filtering/active_record/strategy.rb index 18f32a3..71dde7f 100644 --- a/lib/active_set/filtering/active_record/strategy.rb +++ b/lib/active_set/filtering/active_record/strategy.rb @@ -14,7 +14,7 @@ class Strategy :arel_type, :initial_relation, :attribute, - to: :@set_instruction + to: :@set_instruction def initialize(set, attribute_instruction) @set = set diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index 717463b..e4237db 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -2,6 +2,7 @@ class ActiveSet module Filtering + # rubocop:disable Metrics/ModuleLength module Constants PREDICATES = { EQ: { @@ -203,6 +204,6 @@ module Constants } }.freeze end + # rubocop:enable Metrics/ModuleLength end end - diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index 0cbf470..33c219e 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -5,6 +5,7 @@ class ActiveSet module Filtering module Enumerable + # rubocop:disable Metrics/ModuleLength module Operators PREDICATES = { EQ: { @@ -157,6 +158,7 @@ def self.get(operator_name) base_operator_hash.merge(this_operator_hash) end end + # rubocop:enable Metrics/ModuleLength end end end diff --git a/lib/active_set/filtering/enumerable/set_instruction.rb b/lib/active_set/filtering/enumerable/set_instruction.rb index 2ef9857..c717d88 100644 --- a/lib/active_set/filtering/enumerable/set_instruction.rb +++ b/lib/active_set/filtering/enumerable/set_instruction.rb @@ -8,7 +8,7 @@ module Filtering module Enumerable class SetInstruction < EnumerableSetInstruction def item_matches_query?(item) - return query_result_for(item, query_attribute_for(instruction_value)) if not operator_hash.key?(:reducer) + return query_result_for(item, query_attribute_for(instruction_value)) unless operator_hash.key?(:reducer) instruction_value.public_send(operator_hash[:reducer]) do |value| query_result_for(item, query_attribute_for(value)) @@ -40,18 +40,18 @@ def attribute_class def query_result_for(item, value) result = if operator_method == :cover? && value.is_a?(Range) - value - .public_send( - operator_method, - object_attribute_for(item) - ) - else - object_attribute_for(item) - .public_send( - operator_method, - value - ) - end + value + .public_send( + operator_method, + object_attribute_for(item) + ) + else + object_attribute_for(item) + .public_send( + operator_method, + value + ) + end return result unless operator_hash.key?(:result_transformer) diff --git a/lib/active_set/filtering/enumerable/strategy.rb b/lib/active_set/filtering/enumerable/strategy.rb index 442e054..d668846 100644 --- a/lib/active_set/filtering/enumerable/strategy.rb +++ b/lib/active_set/filtering/enumerable/strategy.rb @@ -15,7 +15,7 @@ class Strategy :attribute, :set_item, :resource_for, - to: :@set_instruction + to: :@set_instruction def initialize(set, attribute_instruction) @set = set @@ -69,18 +69,16 @@ def intersect_operation @set & other_set end - private - def other_set other_set = attribute_class.public_send( - attribute, - instruction_value - ) + attribute, + instruction_value + ) if attribute_class != set_item.class other_set = begin @set.select { |item| resource_for(item: item)&.presence_in other_set } - rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil - nil + rescue ArgumentError # thrown if other_set is doesn't respond to #include?, like when nil + nil end end From 3b6c51521c4898bb0c4dbe47de9e02d2dbd6eac7 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 12:30:01 +0200 Subject: [PATCH 22/56] Get the specs for inconclusive predicates working --- spec/active_set/filter/predicates_spec.rb | 80 ++++++++++++----------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index ea2091f..ccbb5b1 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -20,6 +20,10 @@ o[:compound] == true && o[:behavior] == :exclusive end.map(&:first) + INCONCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :inconclusive + end.map(&:first) before(:all) do @thing_1 = FactoryBot.create(:thing) @@ -137,44 +141,46 @@ end end - # multi value mixed operators - # %i[ - # lt_any - # lteq_all - # gt_any - # gteq_all - # ].each do |operator| - # %W[ - # #{type}(#{operator}) - # only.#{type}(#{operator}) - # ].each do |path| - # context "{ #{path}: }" do - # let(:matching_item) { instance_variable_get("@thing_#{id}") } - # let(:other_thing) do - # FactoryBot.build(:thing, - # boolean: !matching_item.boolean, - # only: FactoryBot.build(:only, - # boolean: !matching_item.only.boolean)) - # end - # let(:instruction_multi_value) do - # [ - # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: matching_item), - # ActiveSet::AttributeInstruction.new(path, nil).value_for(item: other_thing) - # ] - # end - # let(:instructions) do - # { - # path => instruction_multi_value - # } - # end + INCONCLUSIVE_BINARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:other_thing) do + FactoryBot.build(:thing, + boolean: !matching_item.boolean, + only: FactoryBot.build(:only, + boolean: !matching_item.only.boolean)) + end + let(:matching_value) { value_for(object: matching_item, path: path) } + let(:other_value) { value_for(object: other_thing, path: path) } + let(:instruction_multi_value) do + [ + matching_value, + other_value + ] + end + let(:instructions) do + { + path => instruction_multi_value + } + end - # if type.presence_in(%i[binary datetime decimal float integer]) && operator == :gt_any - # it { expect(result_ids).not_to include matching_item.id } - # else - # end - # end - # end - # end + # By default, we expect these operators to return nothing. + # If, however, they do return something, we guarantee it is a logical result + it do + set_instruction = ActiveSet::Filtering::Enumerable::SetInstruction + .new( + ActiveSet::AttributeInstruction.new(path, instruction_multi_value), + @active_set.set) + + expect(results.map { |obj| set_instruction.item_matches_query?(obj) }).to all( be true ) + end + end + end + end end end end From 7453cd4e4f5e3ba6522235c8037ca1bde2ac01e0 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 13:36:13 +0200 Subject: [PATCH 23/56] Don't cover codepaths we can't reach --- lib/action_set.rb | 2 ++ lib/action_set/attribute_value.rb | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/lib/action_set.rb b/lib/action_set.rb index c266ffd..55ecd22 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -94,7 +94,9 @@ def export_instructions elsif respond_to?(:export_set_columns, true) send(:export_set_columns) else + # :nocov: [{}] + # :nocov: end end end diff --git a/lib/action_set/attribute_value.rb b/lib/action_set/attribute_value.rb index 95483d3..c6d8e54 100644 --- a/lib/action_set/attribute_value.rb +++ b/lib/action_set/attribute_value.rb @@ -62,7 +62,9 @@ class ActiveModelAdapter begin require 'active_model/type' rescue LoadError + # :nocov: require 'active_record/type' + # :nocov: end def initialize(raw, target) @@ -106,13 +108,17 @@ def can_typecast?(const_name) def init_typecaster(const_name) type_class.const_get(const_name).new rescue StandardError + # :nocov: nil + # :nocov: end def type_class ActiveModel::Type rescue NameError + # :nocov: ActiveRecord::Type + # :nocov: end end From 287018b00af46f5707cdebda6dc2befeaa99fb66 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 13:36:20 +0200 Subject: [PATCH 24/56] Add tests for the type caster --- spec/action_set/attribute_value_spec.rb | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 spec/action_set/attribute_value_spec.rb diff --git a/spec/action_set/attribute_value_spec.rb b/spec/action_set/attribute_value_spec.rb new file mode 100644 index 0000000..4da3e16 --- /dev/null +++ b/spec/action_set/attribute_value_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ActionSet::AttributeValue do + before(:all) do + @thing = FactoryBot.create(:thing) + end + + describe '#cast' do + ApplicationRecord::FIELD_TYPES.each do |type| + it do + type_value = @thing.public_send(type) + string_value = type_value.to_s + attribute_value = ActionSet::AttributeValue.new(string_value) + cast_value = attribute_value.cast(to: type_value.class) + + expect(cast_value).to eql type_value + end + + it do + type_value = @thing.public_send(type) + string_value = type_value.to_s + attribute_value = ActionSet::AttributeValue.new(string_value) + cast_value = attribute_value.cast(to: Module.new) + + expect(cast_value).to eql string_value + end + end + end +end From 2dba63745fdbba34ba29c29336f2b271f8c00542 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 13:37:43 +0200 Subject: [PATCH 25/56] Fix the filtering predicates request spec --- spec/requests/filtering/predicates_spec.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index 223cee4..6e0b4ea 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -28,10 +28,10 @@ gteq matches ].sample].each do |operator| - %W[ + [%W[ #{type}(#{operator}) only.#{type}(#{operator}) - ].sample do |path| + ].sample].each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -55,10 +55,10 @@ gt does_not_match ].sample].each do |operator| - %W[ + [%W[ #{type}(#{operator}) only.#{type}(#{operator}) - ].sample do |path| + ].sample].each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -87,10 +87,10 @@ matches_any does_not_match_any ].sample].each do |operator| - %W[ + [%W[ #{type}(#{operator}) only.#{type}(#{operator}) - ].sample do |path| + ].sample].each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -126,10 +126,10 @@ matches_all does_not_match_all ].sample].each do |operator| - %W[ + [%W[ #{type}(#{operator}) only.#{type}(#{operator}) - ].sample do |path| + ].sample].each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -160,10 +160,10 @@ # gt_any # gteq_all # ].sample].each do |operator| - # %W[ + # [%W[ # #{type}(#{operator}) # only.#{type}(#{operator}) - # ].sample do |path| + # ].sample].each do |path| # context "{ #{path}: }" do # let(:matching_item) { instance_variable_get("@thing_#{id}") } # let(:other_thing) do From e14d96bc4c7c2ff5b0df12ab716586ad0f0071e5 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 13:45:55 +0200 Subject: [PATCH 26/56] Use the operator constants in both predicate specs --- spec/active_set/filter/predicates_spec.rb | 36 ------------------- spec/requests/filtering/predicates_spec.rb | 42 +++------------------- spec/support/filtering_helpers.rb | 18 ++++++++++ 3 files changed, 22 insertions(+), 74 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index 96f7cac..096c46b 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -2,43 +2,7 @@ require 'spec_helper' -# PREDICATE_OPERATORS = ActiveSet::Filtering::ActiveRecord::Constants::PREDICATE_OPERATORS -# INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :inclusive -# end -# EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :exclusive -# end -# INCONCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS -# .select do |o| -# o[:type] == :unary && -# o[:matching_behavior] == :inconclusive -# end - - RSpec.describe ActiveSet do - PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::BASE_PREDICATES - INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == false && - o[:behavior] == :inclusive - end.map(&:first) - EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == false && - o[:behavior] == :exclusive - end.map(&:first) - INCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == true && - o[:behavior] == :inclusive - end.map(&:first) - EXCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == true && - o[:behavior] == :exclusive - end.map(&:first) - before(:all) do @thing_1 = FactoryBot.create(:thing) @thing_2 = FactoryBot.create(:thing) diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index 6e0b4ea..d785c03 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -21,13 +21,7 @@ ApplicationRecord::DB_FIELD_TYPES.each do |type| [1, 2].each do |id| - # single value inclusive operators - [%i[ - eq - lteq - gteq - matches - ].sample].each do |operator| + [INCLUSIVE_UNARY_OPERATORS.sample].each do |operator| [%W[ #{type}(#{operator}) only.#{type}(#{operator}) @@ -48,13 +42,7 @@ end end - # single value exlusive operators - [%i[ - not_eq - lt - gt - does_not_match - ].sample].each do |operator| + [EXCLUSIVE_UNARY_OPERATORS.sample].each do |operator| [%W[ #{type}(#{operator}) only.#{type}(#{operator}) @@ -75,18 +63,7 @@ end end - # multi value inclusive operators - [%i[ - eq_any - not_eq_any - in - in_any - not_in_any - lteq_any - gteq_any - matches_any - does_not_match_any - ].sample].each do |operator| + [INCLUSIVE_BINARY_OPERATORS.sample].each do |operator| [%W[ #{type}(#{operator}) only.#{type}(#{operator}) @@ -114,18 +91,7 @@ end end - # multi value exclusive operators - [%i[ - eq_all - not_eq_all - not_in - in_all - not_in_all - lt_all - gt_all - matches_all - does_not_match_all - ].sample].each do |operator| + [EXCLUSIVE_BINARY_OPERATORS.sample].each do |operator| [%W[ #{type}(#{operator}) only.#{type}(#{operator}) diff --git a/spec/support/filtering_helpers.rb b/spec/support/filtering_helpers.rb index b163b1c..0178601 100644 --- a/spec/support/filtering_helpers.rb +++ b/spec/support/filtering_helpers.rb @@ -1,6 +1,24 @@ # frozen_string_literal: true module FilteringHelpers + PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::BASE_PREDICATES + INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :inclusive + end.map(&:first) + EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :exclusive + end.map(&:first) + INCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :inclusive + end.map(&:first) + EXCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :exclusive + end.map(&:first) + def all_possible_scope_paths_for(type) %W[ #{type}_scope_method From 064e60cdbe808aa92eca7fa42c9445609be30b9a Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 14:06:49 +0200 Subject: [PATCH 27/56] Get the filtering specs in full working order --- spec/requests/filtering/predicates_spec.rb | 4 +++- spec/support/filtering_helpers.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index 729aa78..819ad77 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -157,8 +157,10 @@ .new( ActiveSet::AttributeInstruction.new(path, instruction_multi_value), @active_set.set) + result_objs = Thing.where(id: result_ids) - expect(results.map { |obj| set_instruction.item_matches_query?(obj) }).to all( be true ) + expect(result_objs.map { |obj| set_instruction.item_matches_query?(obj) }) + .to all( be true ) end end end diff --git a/spec/support/filtering_helpers.rb b/spec/support/filtering_helpers.rb index 4efbd32..52fd3a8 100644 --- a/spec/support/filtering_helpers.rb +++ b/spec/support/filtering_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module FilteringHelpers - PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::BASE_PREDICATES + PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::PREDICATES INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| o[:compound] == false && o[:behavior] == :inclusive From c2662c80d65f48d0e6f2ab713ad065d9541d1f12 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 14:41:34 +0200 Subject: [PATCH 28/56] Add is_true, is_false, is_null, not_null, is_present, is_blank operators --- .../filtering/active_record/operators.rb | 41 ++++++++++++++-- .../active_record/set_instruction.rb | 2 +- lib/active_set/filtering/constants.rb | 49 ++++++++++++++++++- .../filtering/enumerable/operators.rb | 25 +++++++++- .../filtering/enumerable/set_instruction.rb | 3 +- 5 files changed, 113 insertions(+), 7 deletions(-) diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index b072ab8..e5aa34c 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -105,12 +105,47 @@ module Operators BETWEEN: { operator: :between, - query_attribute_transformer: ->(query) { Range.new(*query.sort) } + query_attribute_transformer: ->(query, _) { Range.new(*query.sort) } }, NOT_BETWEEN: { operator: :not_between, - query_attribute_transformer: ->(query) { Range.new(*query.sort) } - } + query_attribute_transformer: ->(query, _) { Range.new(*query.sort) } + }, + + IS_TRUE: { + operator: :eq + }, + IS_FALSE: { + operator: :eq + }, + + IS_NULL: { + operator: :eq + }, + NOT_NULL: { + operator: :not_eq + }, + + IS_PRESENT: { + operator: :not_eq_all, + query_attribute_transformer: proc do |_, type| + if type.presence_in %i[date float integer] + [nil] + else + Constants::BLANK_VALUES + end + end + }, + IS_BLANK: { + operator: :eq_any, + query_attribute_transformer: proc do |_, type| + if type.presence_in %i[date float integer] + [nil] + else + Constants::BLANK_VALUES + end + end + }, }.freeze def self.get(operator_name) diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb index 65762f9..ace078f 100644 --- a/lib/active_set/filtering/active_record/set_instruction.rb +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -26,7 +26,7 @@ def arel_value def query_attribute_for(value) return value unless operator_hash.key?(:query_attribute_transformer) - operator_hash[:query_attribute_transformer].call(value) + operator_hash[:query_attribute_transformer].call(value, arel_type) end def operator_hash diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index e4237db..29020d4 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -4,6 +4,8 @@ class ActiveSet module Filtering # rubocop:disable Metrics/ModuleLength module Constants + BLANK_VALUES = [nil, ''].freeze + PREDICATES = { EQ: { type: :binary, @@ -201,7 +203,52 @@ module Constants compound: true, behavior: :exclusive, shorthand: :'!.' - } + }, + + IS_TRUE: { + type: :unary, + operator: :eq, + compound: false, + behavior: :inconclusive, + query_attribute_transformer: proc { |_| true } + }, + IS_FALSE: { + type: :unary, + operator: :eq, + compound: false, + behavior: :inconclusive, + query_attribute_transformer: proc { |_| false } + }, + + IS_NULL: { + type: :unary, + operator: :eq, + compound: false, + behavior: :exclusive, + query_attribute_transformer: proc { |_| nil } + }, + NOT_NULL: { + type: :unary, + operator: :not_eq, + compound: false, + behavior: :inclusive, + query_attribute_transformer: proc { |_| nil } + }, + + IS_PRESENT: { + type: :unary, + operator: :not_eq_all, + compound: false, + behavior: :inclusive, + query_attribute_transformer: proc { |_| BLANK_VALUES } + }, + IS_BLANK: { + type: :unary, + operator: :eq_any, + compound: false, + behavior: :exclusive, + query_attribute_transformer: proc { |_| BLANK_VALUES } + }, }.freeze end # rubocop:enable Metrics/ModuleLength diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index 33c219e..c193897 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -146,7 +146,30 @@ module Operators operator: :cover?, query_attribute_transformer: ->(query) { Range.new(*query.sort) }, result_transformer: ->(result) { !result } - } + }, + + IS_TRUE: { + operator: :'==' + }, + IS_FALSE: { + operator: :'==' + }, + + IS_NULL: { + operator: :'==' + }, + NOT_NULL: { + operator: :'!=' + }, + + IS_PRESENT: { + operator: :'!=', + reducer: :all? + }, + IS_BLANK: { + operator: :'==', + reducer: :any? + }, }.freeze def self.get(operator_name) diff --git a/lib/active_set/filtering/enumerable/set_instruction.rb b/lib/active_set/filtering/enumerable/set_instruction.rb index c717d88..3bd91cc 100644 --- a/lib/active_set/filtering/enumerable/set_instruction.rb +++ b/lib/active_set/filtering/enumerable/set_instruction.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'active_support/core_ext/array/wrap' require_relative '../../enumerable_set_instruction' require_relative './operators' @@ -10,7 +11,7 @@ class SetInstruction < EnumerableSetInstruction def item_matches_query?(item) return query_result_for(item, query_attribute_for(instruction_value)) unless operator_hash.key?(:reducer) - instruction_value.public_send(operator_hash[:reducer]) do |value| + Array.wrap(instruction_value).public_send(operator_hash[:reducer]) do |value| query_result_for(item, query_attribute_for(value)) end end From 90615c119df049d3ea0ea142757ab6c6d8424cbc Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Sun, 30 Jun 2019 16:45:58 +0200 Subject: [PATCH 29/56] Test the is_true and is_false predicates properly --- .../filtering/active_record/operators.rb | 8 +++-- lib/active_set/filtering/constants.rb | 12 +++---- .../filtering/enumerable/operators.rb | 12 ++++--- spec/active_set/filter/predicates_spec.rb | 30 +++++++++++++++++ spec/requests/filtering/predicates_spec.rb | 32 +++++++++++++++++++ spec/support/filtering_helpers.rb | 4 +++ 6 files changed, 83 insertions(+), 15 deletions(-) diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index e5aa34c..206a9c6 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -113,14 +113,16 @@ module Operators }, IS_TRUE: { - operator: :eq + operator: :eq, + query_attribute_transformer: proc { |_| true } }, IS_FALSE: { - operator: :eq + operator: :eq, + query_attribute_transformer: proc { |_| false } }, IS_NULL: { - operator: :eq + operator: :eq, }, NOT_NULL: { operator: :not_eq diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index 29020d4..e40cd8e 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -209,15 +209,13 @@ module Constants type: :unary, operator: :eq, compound: false, - behavior: :inconclusive, - query_attribute_transformer: proc { |_| true } + behavior: :inconclusive }, IS_FALSE: { type: :unary, operator: :eq, compound: false, - behavior: :inconclusive, - query_attribute_transformer: proc { |_| false } + behavior: :inconclusive }, IS_NULL: { @@ -239,15 +237,13 @@ module Constants type: :unary, operator: :not_eq_all, compound: false, - behavior: :inclusive, - query_attribute_transformer: proc { |_| BLANK_VALUES } + behavior: :inclusive }, IS_BLANK: { type: :unary, operator: :eq_any, compound: false, - behavior: :exclusive, - query_attribute_transformer: proc { |_| BLANK_VALUES } + behavior: :exclusive }, }.freeze end diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index c193897..f6f58dc 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -149,10 +149,12 @@ module Operators }, IS_TRUE: { - operator: :'==' + operator: :'==', + query_attribute_transformer: proc { |_| 1 } }, IS_FALSE: { - operator: :'==' + operator: :'==', + query_attribute_transformer: proc { |_| 0 } }, IS_NULL: { @@ -164,11 +166,13 @@ module Operators IS_PRESENT: { operator: :'!=', - reducer: :all? + reducer: :all?, + query_attribute_transformer: proc { |_| Constants::BLANK_VALUES } }, IS_BLANK: { operator: :'==', - reducer: :any? + reducer: :any?, + query_attribute_transformer: proc { |_| Constants::BLANK_VALUES } }, }.freeze diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index 68cb4ed..ff329b9 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -61,6 +61,36 @@ end end + INCONCLUSIVE_UNARY_OPERATORS.each do |operator| + %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:instruction_single_value) do + value_for(object: matching_item, path: path) + end + let(:instructions) do + { + path => instruction_single_value + } + end + + # By default, we expect these operators to return nothing. + # If, however, they do return something, we guarantee it is a logical result + it do + set_instruction = ActiveSet::Filtering::Enumerable::SetInstruction + .new( + ActiveSet::AttributeInstruction.new(path, instruction_single_value), + @active_set.set) + + expect(results.map { |obj| set_instruction.item_matches_query?(obj) }).to all( be true ) + end + end + end + end + INCLUSIVE_BINARY_OPERATORS.each do |operator| %W[ #{type}(#{operator}) diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index 819ad77..a09f1bf 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -63,6 +63,38 @@ end end + [INCONCLUSIVE_UNARY_OPERATORS.sample].each do |operator| + [%W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ].sample].each do |path| + context "{ #{path}: }" do + let(:matching_item) { instance_variable_get("@thing_#{id}") } + let(:instruction_single_value) do + value_for(object: matching_item, path: path) + end + let(:instructions) do + { + path => instruction_single_value + } + end + + # By default, we expect these operators to return nothing. + # If, however, they do return something, we guarantee it is a logical result + it do + set_instruction = ActiveSet::Filtering::Enumerable::SetInstruction + .new( + ActiveSet::AttributeInstruction.new(path, instruction_single_value), + @active_set.set) + result_objs = Thing.where(id: result_ids) + + expect(result_objs.map { |obj| set_instruction.item_matches_query?(obj) }) + .to all( be true ) + end + end + end + end + [INCLUSIVE_BINARY_OPERATORS.sample].each do |operator| [%W[ #{type}(#{operator}) diff --git a/spec/support/filtering_helpers.rb b/spec/support/filtering_helpers.rb index 52fd3a8..1f97572 100644 --- a/spec/support/filtering_helpers.rb +++ b/spec/support/filtering_helpers.rb @@ -10,6 +10,10 @@ module FilteringHelpers o[:compound] == false && o[:behavior] == :exclusive end.map(&:first) + INCONCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :inconclusive + end.map(&:first) INCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| o[:compound] == true && o[:behavior] == :inclusive From 1f33b2bec95aa958883745061b99526b5cf8b77b Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 9 Jul 2019 12:29:09 +0200 Subject: [PATCH 30/56] Make the filtering predicate spec handle the varous IN predicates properly --- spec/active_set/filter/predicates_spec.rb | 4 ++-- spec/requests/filtering/predicates_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index ff329b9..9988b3d 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -105,7 +105,7 @@ let(:matching_value) { value_for(object: matching_item, path: path) } let(:other_value) { value_for(object: other_thing, path: path) } let(:instruction_multi_value) do - if operator.to_s.include? 'IN_' + if operator.to_s.split('_').include?('IN') && (operator.to_s.split('_') & %w[ANY ALL]).any? [ [matching_value], [other_value] ] else [ matching_value, other_value ] @@ -134,7 +134,7 @@ let(:matching_value) { value_for(object: matching_item, path: path) } let(:other_value) { value_for(object: other_thing, path: path) } let(:instruction_multi_value) do - if operator.to_s.include? 'IN_' + if operator.to_s.split('_').include?('IN') && (operator.to_s.split('_') & %w[ANY ALL]).any? [ [matching_value], [other_value] ] else [ matching_value, other_value ] diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index a09f1bf..c919a88 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -109,7 +109,7 @@ let(:matching_value) { value_for(object: matching_item, path: path) } let(:other_value) { value_for(object: other_thing, path: path) } let(:instruction_multi_value) do - if operator.to_s.include? 'IN_' + if operator.to_s.split('_').include?('IN') && (operator.to_s.split('_') & %w[ANY ALL]).any? [ [matching_value], [other_value] ] else [ matching_value, other_value ] @@ -140,7 +140,7 @@ let(:matching_value) { value_for(object: matching_item, path: path) } let(:other_value) { value_for(object: other_thing, path: path) } let(:instruction_multi_value) do - if operator.to_s.include? 'IN_' + if operator.to_s.split('_').include?('IN') && (operator.to_s.split('_') & %w[ANY ALL]).any? [ [matching_value], [other_value] ] else [ matching_value, other_value ] From aa22a6cf990c4a1c9792a442eb83941eae18de57 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 9 Jul 2019 12:30:04 +0200 Subject: [PATCH 31/56] Add computed matcher predicates (start, end, contain) --- .../filtering/active_record/operators.rb | 156 ++++++++++++++--- lib/active_set/filtering/constants.rb | 97 +++++++++++ .../filtering/enumerable/operators.rb | 159 ++++++++++++++++-- 3 files changed, 372 insertions(+), 40 deletions(-) diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index 206a9c6..dda3d90 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -7,6 +7,51 @@ module Filtering module ActiveRecord # rubocop:disable Metrics/ModuleLength module Operators + RANGE_TRANSFORMER = proc do |raw:, sql:, type:| + if type.presence_in %i[boolean] + Range.new(*sql.sort) + else + Range.new(*raw.sort) + end + end + BLANK_TRANSFORMER = proc do |type:, **ctx| + if type.presence_in %i[date float integer] + [nil] + else + Constants::BLANK_VALUES + end + end + MATCHER_TRANSFORMER = proc do |sql:, type:, **ctx| + next sql.map { |str| MATCHER_TRANSFORMER.call(sql: str, type: type, **ctx) } if sql.respond_to?(:map) + + next sql if not type == :decimal + next sql[0..-3] if sql.ends_with?('.0') + next sql[0..-4] + '%' if sql.ends_with?('.0%') + + sql + end + START_MATCHER_TRANSFORMER = proc do |sql:, type:, **ctx| + next sql.map { |str| START_MATCHER_TRANSFORMER.call(sql: str, type: type, **ctx) } if sql.respond_to?(:map) + + str = MATCHER_TRANSFORMER.call(sql: sql, type: type, **ctx) + + str + '%' + end + END_MATCHER_TRANSFORMER = proc do |sql:, type:, **ctx| + next sql.map { |str| END_MATCHER_TRANSFORMER.call(sql: str, type: type, **ctx) } if sql.respond_to?(:map) + + str = MATCHER_TRANSFORMER.call(sql: sql, type: type, **ctx) + + '%' + str + end + CONTAIN_MATCHER_TRANSFORMER = proc do |sql:, type:, **ctx| + next sql.map { |str| CONTAIN_MATCHER_TRANSFORMER.call(sql: str, type: type, **ctx) } if sql.respond_to?(:map) + + str = MATCHER_TRANSFORMER.call(sql: sql, type: type, **ctx) + + '%' + str + '%' + end + PREDICATES = { EQ: { operator: :eq @@ -47,22 +92,28 @@ module Operators }, MATCHES: { - operator: :matches + operator: :matches, + query_attribute_transformer: MATCHER_TRANSFORMER }, DOES_NOT_MATCH: { - operator: :does_not_match + operator: :does_not_match, + query_attribute_transformer: MATCHER_TRANSFORMER }, MATCHES_ANY: { - operator: :matches_any + operator: :matches_any, + query_attribute_transformer: MATCHER_TRANSFORMER }, MATCHES_ALL: { - operator: :matches_all + operator: :matches_all, + query_attribute_transformer: MATCHER_TRANSFORMER }, DOES_NOT_MATCH_ANY: { - operator: :does_not_match_any + operator: :does_not_match_any, + query_attribute_transformer: MATCHER_TRANSFORMER }, DOES_NOT_MATCH_ALL: { - operator: :does_not_match_all + operator: :does_not_match_all, + query_attribute_transformer: MATCHER_TRANSFORMER }, LT: { @@ -105,11 +156,11 @@ module Operators BETWEEN: { operator: :between, - query_attribute_transformer: ->(query, _) { Range.new(*query.sort) } + query_attribute_transformer: RANGE_TRANSFORMER }, NOT_BETWEEN: { operator: :not_between, - query_attribute_transformer: ->(query, _) { Range.new(*query.sort) } + query_attribute_transformer: RANGE_TRANSFORMER }, IS_TRUE: { @@ -130,24 +181,85 @@ module Operators IS_PRESENT: { operator: :not_eq_all, - query_attribute_transformer: proc do |_, type| - if type.presence_in %i[date float integer] - [nil] - else - Constants::BLANK_VALUES - end - end + query_attribute_transformer: BLANK_TRANSFORMER }, IS_BLANK: { operator: :eq_any, - query_attribute_transformer: proc do |_, type| - if type.presence_in %i[date float integer] - [nil] - else - Constants::BLANK_VALUES - end - end + query_attribute_transformer: BLANK_TRANSFORMER }, + + MATCH_START: { + operator: :matches, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_START_ANY: { + operator: :matches_any, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_START_ALL: { + operator: :matches_all, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_NOT_START: { + operator: :does_not_match, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_NOT_START_ANY: { + operator: :does_not_match_any, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_NOT_START_ALL: { + operator: :does_not_match_all, + query_attribute_transformer: START_MATCHER_TRANSFORMER, + }, + MATCH_END: { + operator: :matches, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_END_ANY: { + operator: :matches_any, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_END_ALL: { + operator: :matches_all, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_NOT_END: { + operator: :does_not_match, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_NOT_END_ANY: { + operator: :does_not_match_any, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_NOT_END_ALL: { + operator: :does_not_match_all, + query_attribute_transformer: END_MATCHER_TRANSFORMER, + }, + MATCH_CONTAIN: { + operator: :matches, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + }, + MATCH_CONTAIN_ANY: { + operator: :matches_any, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + }, + MATCH_CONTAIN_ALL: { + operator: :matches_all, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + }, + MATCH_NOT_CONTAIN: { + operator: :does_not_match, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + }, + MATCH_NOT_CONTAIN_ANY: { + operator: :does_not_match_any, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + }, + MATCH_NOT_CONTAIN_ALL: { + operator: :does_not_match_all, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + } }.freeze def self.get(operator_name) diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index e40cd8e..1374e06 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -245,6 +245,103 @@ module Constants compound: false, behavior: :exclusive }, + + MATCH_START: { + type: :binary, + compound: false, + behavior: :inclusive, + }, + MATCH_START_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + }, + MATCH_START_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + }, + MATCH_NOT_START: { + type: :binary, + compound: false, + behavior: :exclusive, + }, + MATCH_NOT_START_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + }, + MATCH_NOT_START_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + }, + MATCH_END: { + type: :binary, + compound: false, + behavior: :inclusive, + }, + MATCH_END_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + }, + MATCH_END_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + }, + MATCH_NOT_END: { + type: :binary, + compound: false, + behavior: :exclusive, + }, + MATCH_NOT_END_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + }, + MATCH_NOT_END_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + }, + MATCH_CONTAIN: { + type: :binary, + compound: false, + behavior: :inclusive, + shorthand: :'~*' + }, + MATCH_CONTAIN_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E~*' + }, + MATCH_CONTAIN_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A~*' + }, + MATCH_NOT_CONTAIN: { + type: :binary, + compound: false, + behavior: :exclusive, + shorthand: :'!*' + }, + MATCH_NOT_CONTAIN_ANY: { + type: :binary, + compound: true, + behavior: :inclusive, + shorthand: :'E!*' + }, + MATCH_NOT_CONTAIN_ALL: { + type: :binary, + compound: true, + behavior: :exclusive, + shorthand: :'A!*' + } }.freeze end # rubocop:enable Metrics/ModuleLength diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index f6f58dc..e689129 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -7,6 +7,26 @@ module Filtering module Enumerable # rubocop:disable Metrics/ModuleLength module Operators + RANGE_TRANSFORMER = ->(query) { Range.new(*query.sort) } + NOT_TRANSFORMER = ->(result) { !result } + STRING_TRANSFORMER = ->(attribute) { attribute.to_s } + REGEXP_TRANSFORMER = ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + START_MATCHER_TRANSFORMER = proc do |string_or_strings| + next string_or_strings.map { |str| START_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) + + /#{Regexp.quote(string_or_strings.to_s) + '.*'}/ + end + END_MATCHER_TRANSFORMER = proc do |string_or_strings| + next string_or_strings.map { |str| END_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) + + /#{'.*' + Regexp.quote(string_or_strings.to_s)}/ + end + CONTAIN_MATCHER_TRANSFORMER = proc do |string_or_strings| + next string_or_strings.map { |str| CONTAIN_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) + + /#{'.*' + Regexp.quote(string_or_strings.to_s) + '.*'}/ + end + PREDICATES = { EQ: { operator: :'==' @@ -36,7 +56,7 @@ module Operators }, NOT_IN: { operator: :presence_in, - result_transformer: ->(result) { !result } + result_transformer: NOT_TRANSFORMER }, IN_ANY: { operator: :presence_in, @@ -49,47 +69,47 @@ module Operators NOT_IN_ANY: { operator: :presence_in, reducer: :any?, - result_transformer: ->(result) { !result } + result_transformer: NOT_TRANSFORMER }, NOT_IN_ALL: { operator: :presence_in, reducer: :all?, - result_transformer: ->(result) { !result } + result_transformer: NOT_TRANSFORMER }, MATCHES: { operator: :'=~', - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, DOES_NOT_MATCH: { operator: :'!~', - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, MATCHES_ANY: { operator: :'=~', reducer: :any?, - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, MATCHES_ALL: { operator: :'=~', reducer: :all?, - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, DOES_NOT_MATCH_ANY: { operator: :'!~', reducer: :any?, - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, DOES_NOT_MATCH_ALL: { operator: :'!~', reducer: :all?, - object_attribute_transformer: ->(attribute) { attribute.to_s }, - query_attribute_transformer: ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: REGEXP_TRANSFORMER }, LT: { @@ -140,12 +160,12 @@ module Operators BETWEEN: { operator: :cover?, - query_attribute_transformer: ->(query) { Range.new(*query.sort) } + query_attribute_transformer: RANGE_TRANSFORMER }, NOT_BETWEEN: { operator: :cover?, - query_attribute_transformer: ->(query) { Range.new(*query.sort) }, - result_transformer: ->(result) { !result } + query_attribute_transformer: RANGE_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, IS_TRUE: { @@ -174,6 +194,109 @@ module Operators reducer: :any?, query_attribute_transformer: proc { |_| Constants::BLANK_VALUES } }, + + MATCH_START: { + operator: :'=~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_START_ANY: { + operator: :'=~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_START_ALL: { + operator: :'=~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_NOT_START: { + operator: :'!~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_NOT_START_ANY: { + operator: :'!~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_NOT_START_ALL: { + operator: :'!~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER + }, + MATCH_END: { + operator: :'=~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_END_ANY: { + operator: :'=~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_END_ALL: { + operator: :'=~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_NOT_END: { + operator: :'!~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_NOT_END_ANY: { + operator: :'!~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_NOT_END_ALL: { + operator: :'!~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER + }, + MATCH_CONTAIN: { + operator: :'=~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + }, + MATCH_CONTAIN_ANY: { + operator: :'=~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + }, + MATCH_CONTAIN_ALL: { + operator: :'=~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + }, + MATCH_NOT_CONTAIN: { + operator: :'!~', + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + }, + MATCH_NOT_CONTAIN_ANY: { + operator: :'!~', + reducer: :any?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + }, + MATCH_NOT_CONTAIN_ALL: { + operator: :'!~', + reducer: :all?, + object_attribute_transformer: STRING_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + } }.freeze def self.get(operator_name) From 79f127ffdbe3aaff3454d5859c508fc7f8d4c0dd Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 9 Jul 2019 12:30:49 +0200 Subject: [PATCH 32/56] Update the ActiveRecord strategy organization to clean up handling the various edge-cases --- .../filtering/active_record/query_column.rb | 35 ++++++++++++++ .../filtering/active_record/query_value.rb | 47 +++++++++++++++++++ .../active_record/set_instruction.rb | 32 ++----------- .../filtering/active_record/strategy.rb | 10 ++-- 4 files changed, 92 insertions(+), 32 deletions(-) create mode 100644 lib/active_set/filtering/active_record/query_column.rb create mode 100644 lib/active_set/filtering/active_record/query_value.rb diff --git a/lib/active_set/filtering/active_record/query_column.rb b/lib/active_set/filtering/active_record/query_column.rb new file mode 100644 index 0000000..be42aef --- /dev/null +++ b/lib/active_set/filtering/active_record/query_column.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class ActiveSet + module Filtering + module ActiveRecord + module QueryColumn + def query_column + return @query_column if defined? @query_column + + @query_column = if must_cast_numerical_column? + column_cast_as_char + else + arel_column + end + end + + private + + def column_cast_as_char + # In order to use LIKE, we must CAST the numeric column as a CHAR column. + # NOTE: this is can be quite inefficient, as it forces the DB engine to perform that cast on all rows. + # https://www.ryadel.com/en/like-operator-equivalent-integer-numeric-columns-sql-t-sql-database/ + Arel::Nodes::NamedFunction.new('CAST', [arel_column.as('CHAR')]) + end + + def must_cast_numerical_column? + # The LIKE operator can’t be used if the column hosts numeric types. + return false if not arel_type.presence_in(%i[integer float]) + + arel_operator.to_s.downcase.include?('match') + end + end + end + end +end diff --git a/lib/active_set/filtering/active_record/query_value.rb b/lib/active_set/filtering/active_record/query_value.rb new file mode 100644 index 0000000..844ad07 --- /dev/null +++ b/lib/active_set/filtering/active_record/query_value.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class ActiveSet + module Filtering + module ActiveRecord + module QueryValue + def query_value + return @query_value if defined? @query_value + + query_value = @attribute_instruction.value + query_value = query_attribute_for(query_value) + query_value = query_value.downcase if case_insensitive_operation? + + @query_value = query_value + end + + private + + def query_attribute_for(value) + return value unless operator_hash.key?(:query_attribute_transformer) + + context = { + raw: value, + sql: to_sql_str(value), + type: arel_type + } + + operator_hash[:query_attribute_transformer].call(context) + end + + def to_sql_str(value) + return value.map { |a| to_sql_str(a) } if value.respond_to?(:map) + + arel_node = Arel::Nodes::Casted.new(value, arel_column) + sql_value = arel_node.to_sql + unwrap_sql_str(sql_value) + end + + def unwrap_sql_str(sql_str) + return sql_str if not sql_str[0] == "'" && sql_str[-1] == "'" + + sql_str[1..-2] + end + end + end + end +end diff --git a/lib/active_set/filtering/active_record/set_instruction.rb b/lib/active_set/filtering/active_record/set_instruction.rb index ace078f..c94eb6f 100644 --- a/lib/active_set/filtering/active_record/set_instruction.rb +++ b/lib/active_set/filtering/active_record/set_instruction.rb @@ -2,49 +2,27 @@ require_relative '../../active_record_set_instruction' require_relative './operators' +require_relative './query_value' +require_relative './query_column' class ActiveSet module Filtering module ActiveRecord class SetInstruction < ActiveRecordSetInstruction + include QueryValue + include QueryColumn + def arel_operator operator_hash.fetch(:operator, :eq) end - def arel_value - return @arel_value if defined? @arel_value - - arel_value = guarantee_attribute_type(@attribute_instruction.value) - arel_value = query_attribute_for(arel_value) - arel_value = arel_value.downcase if case_insensitive_operation? - - @arel_value = arel_value - end - private - def query_attribute_for(value) - return value unless operator_hash.key?(:query_attribute_transformer) - - operator_hash[:query_attribute_transformer].call(value, arel_type) - end - def operator_hash instruction_operator = @attribute_instruction.operator Operators.get(instruction_operator) end - - def guarantee_attribute_type(attribute) - # Booleans don't respond to many operator methods, - # so we cast them to however the database expects them - return attribute if arel_type != :boolean - return attribute.map { |a| guarantee_attribute_type(a) } if attribute.respond_to?(:each) - - sql_value = Arel::Nodes::Casted.new(attribute, arel_column).to_sql - sql_value = sql_value[/'(.*?)'/, 1] if sql_value.match?(/'.*?'/) - sql_value - end end end end diff --git a/lib/active_set/filtering/active_record/strategy.rb b/lib/active_set/filtering/active_record/strategy.rb index 71dde7f..56044da 100644 --- a/lib/active_set/filtering/active_record/strategy.rb +++ b/lib/active_set/filtering/active_record/strategy.rb @@ -8,9 +8,9 @@ module Filtering module ActiveRecord class Strategy delegate :attribute_model, - :arel_column, + :query_column, :arel_operator, - :arel_value, + :query_value, :arel_type, :initial_relation, :attribute, @@ -61,9 +61,9 @@ def execute_intersect_operation? def filter_operation initial_relation .where( - arel_column.send( + query_column.send( arel_operator, - arel_value + query_value ) ) end @@ -77,7 +77,7 @@ def intersect_operation .merge( attribute_model.public_send( attribute, - arel_value + query_value ) ) end From dab5c6fbd776ce13b772adb33fc806cc1141fe7b Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 9 Jul 2019 12:38:28 +0200 Subject: [PATCH 33/56] Simplify the float and decimal factory generators --- spec/factories/generic.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories/generic.rb b/spec/factories/generic.rb index 964f3cc..2997cfa 100644 --- a/spec/factories/generic.rb +++ b/spec/factories/generic.rb @@ -23,8 +23,8 @@ def fit_to_minmax(number, max:, min: 1) sequence(:datetime) do |n| "#{date}T#{time}:#{fit_to_minmax(n, max: 60).to_s.rjust(2, '0')}+00:00" end - sequence(:decimal) { |n| Faker::Number.decimal(2).to_f + n } - sequence(:float) { |n| Faker::Number.decimal(2).to_f + n } + sequence(:decimal) { |n| (n.to_s + '.' + Faker::Number.number(2)).to_f } + sequence(:float) { |n| (n.to_s + '.' + Faker::Number.number(2)).to_f } sequence(:integer, &:to_i) sequence(:string) do |n| n.hash.abs.to_s.split('').map { |i| ('a'..'z').to_a.shuffle[i.to_i] }.join From cf9a67da5ae455f2f6fca003c535322127a658dc Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Thu, 11 Jul 2019 16:41:07 +0200 Subject: [PATCH 34/56] Use the built in Ruby start_with?, end_with?, and include? string methods for the computed matcher predicates --- .../filtering/enumerable/operators.rb | 102 +++++++++--------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/lib/active_set/filtering/enumerable/operators.rb b/lib/active_set/filtering/enumerable/operators.rb index e689129..fe0ade1 100644 --- a/lib/active_set/filtering/enumerable/operators.rb +++ b/lib/active_set/filtering/enumerable/operators.rb @@ -7,25 +7,10 @@ module Filtering module Enumerable # rubocop:disable Metrics/ModuleLength module Operators - RANGE_TRANSFORMER = ->(query) { Range.new(*query.sort) } NOT_TRANSFORMER = ->(result) { !result } - STRING_TRANSFORMER = ->(attribute) { attribute.to_s } - REGEXP_TRANSFORMER = ->(attribute) { /#{Regexp.quote(attribute.to_s)}/ } - START_MATCHER_TRANSFORMER = proc do |string_or_strings| - next string_or_strings.map { |str| START_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) - - /#{Regexp.quote(string_or_strings.to_s) + '.*'}/ - end - END_MATCHER_TRANSFORMER = proc do |string_or_strings| - next string_or_strings.map { |str| END_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) - - /#{'.*' + Regexp.quote(string_or_strings.to_s)}/ - end - CONTAIN_MATCHER_TRANSFORMER = proc do |string_or_strings| - next string_or_strings.map { |str| CONTAIN_MATCHER_TRANSFORMER.call(str) } if string_or_strings.is_a?(Array) - - /#{'.*' + Regexp.quote(string_or_strings.to_s) + '.*'}/ - end + RANGE_TRANSFORMER = ->(value) { Range.new(*value.sort) } + REGEXP_TRANSFORMER = ->(value) { /#{Regexp.quote(value.to_s)}/ } + STRING_TRANSFORMER = ->(value) { value.to_s } PREDICATES = { EQ: { @@ -196,106 +181,115 @@ module Operators }, MATCH_START: { - operator: :'=~', + operator: :'start_with?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_START_ANY: { - operator: :'=~', + operator: :'start_with?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_START_ALL: { - operator: :'=~', + operator: :'start_with?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_NOT_START: { - operator: :'!~', + operator: :'start_with?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_START_ANY: { - operator: :'!~', + operator: :'start_with?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_START_ALL: { - operator: :'!~', + operator: :'start_with?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: START_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_END: { - operator: :'=~', + operator: :'end_with?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_END_ANY: { - operator: :'=~', + operator: :'end_with?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_END_ALL: { - operator: :'=~', + operator: :'end_with?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_NOT_END: { - operator: :'!~', + operator: :'end_with?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_END_ANY: { - operator: :'!~', + operator: :'end_with?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_END_ALL: { - operator: :'!~', + operator: :'end_with?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: END_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_CONTAIN: { - operator: :'=~', + operator: :'include?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_CONTAIN_ANY: { - operator: :'=~', + operator: :'include?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_CONTAIN_ALL: { - operator: :'=~', + operator: :'include?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER }, MATCH_NOT_CONTAIN: { - operator: :'!~', + operator: :'include?', object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_CONTAIN_ANY: { - operator: :'!~', + operator: :'include?', reducer: :any?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER }, MATCH_NOT_CONTAIN_ALL: { - operator: :'!~', + operator: :'include?', reducer: :all?, object_attribute_transformer: STRING_TRANSFORMER, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER + query_attribute_transformer: STRING_TRANSFORMER, + result_transformer: NOT_TRANSFORMER } }.freeze From 9885dfca69f4bb5669f4a1a2c6caa13e2219807a Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 31 Jul 2019 09:53:39 +0200 Subject: [PATCH 35/56] Allow sort params to be passed in short or long form e.g. { attribute: x, direction: x } or { attribute: direction } --- lib/action_set.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 55ecd22..9a1c4bd 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -30,7 +30,7 @@ def filter_set(set) def sort_set(set) active_set = ensure_active_set(set) - active_set = active_set.sort(sort_params) if sort_params.any? + active_set = active_set.sort(sort_instructions) if sort_params.any? active_set end @@ -72,6 +72,14 @@ def filter_typecasted_value_for(keypath, value, set) .cast(to: item_value.class) end + def sort_instructions + if sort_params.key?(:attribute) && sort_params.key?(:direction) + { sort_params[:attribute] => sort_params[:direction] } + else + sort_params.transform_values { |v| v.remove('ending') } + end + end + def paginate_instructions paginate_params.transform_values(&:to_i) end From 3920a4fa5de6db0e26d85629a87b9fa8a034da0b Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 31 Jul 2019 09:55:36 +0200 Subject: [PATCH 36/56] Add view helpers for the range of records shown on the current page --- lib/action_set/helpers/helper_methods.rb | 3 ++- .../record_description_for_helper.rb | 20 ++++++++++++++ .../pagination/record_first_for_helper.rb | 20 ++++++++++++++ .../pagination/record_last_for_helper.rb | 26 +++++++++++++++++++ .../pagination/record_range_for_helper.rb | 19 ++++++++++++++ .../pagination/record_size_for_helper.rb | 9 +++++++ .../pagination/total_pages_for_helper.rb | 4 ++- 7 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 lib/action_set/helpers/pagination/record_description_for_helper.rb create mode 100644 lib/action_set/helpers/pagination/record_first_for_helper.rb create mode 100644 lib/action_set/helpers/pagination/record_last_for_helper.rb create mode 100644 lib/action_set/helpers/pagination/record_range_for_helper.rb create mode 100644 lib/action_set/helpers/pagination/record_size_for_helper.rb diff --git a/lib/action_set/helpers/helper_methods.rb b/lib/action_set/helpers/helper_methods.rb index ea53af2..a303db3 100644 --- a/lib/action_set/helpers/helper_methods.rb +++ b/lib/action_set/helpers/helper_methods.rb @@ -2,7 +2,7 @@ require_relative './sort/link_for_helper' require_relative './pagination/links_for_helper' -require_relative './pagination/path_for_helper' +require_relative './pagination/record_description_for_helper' require_relative './params/form_for_object_helper' require_relative './export/path_for_helper' @@ -11,6 +11,7 @@ module Helpers module HelperMethods include Sort::LinkForHelper include Pagination::LinksForHelper + include Pagination::RecordDescriptionForHelper include Params::FormForObjectHelper include Export::PathForHelper end diff --git a/lib/action_set/helpers/pagination/record_description_for_helper.rb b/lib/action_set/helpers/pagination/record_description_for_helper.rb new file mode 100644 index 0000000..2a2ac89 --- /dev/null +++ b/lib/action_set/helpers/pagination/record_description_for_helper.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require_relative './record_range_for_helper' +require_relative './record_size_for_helper' + +module Pagination + module RecordDescriptionForHelper + include Pagination::RecordRangeForHelper + include Pagination::RecordSizeForHelper + + def pagination_record_description_for(set) + [ + pagination_record_range_for(set), + 'of', + "#{pagination_record_size_for(set)}", + 'records' + ].join(' ').html_safe + end + end +end diff --git a/lib/action_set/helpers/pagination/record_first_for_helper.rb b/lib/action_set/helpers/pagination/record_first_for_helper.rb new file mode 100644 index 0000000..363e39e --- /dev/null +++ b/lib/action_set/helpers/pagination/record_first_for_helper.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require_relative './current_page_for_helper' +require_relative './page_size_for_helper' + +module Pagination + module RecordFirstForHelper + include Pagination::CurrentPageForHelper + include Pagination::PageSizeForHelper + + def pagination_record_first_for(set) + current_page = pagination_current_page_for(set) + page_size = pagination_page_size_for(set) + + return 1 if current_page == 1 + + ((current_page - 1) * page_size) + 1 + end + end +end diff --git a/lib/action_set/helpers/pagination/record_last_for_helper.rb b/lib/action_set/helpers/pagination/record_last_for_helper.rb new file mode 100644 index 0000000..396e654 --- /dev/null +++ b/lib/action_set/helpers/pagination/record_last_for_helper.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require_relative './current_page_for_helper' +require_relative './record_size_for_helper' +require_relative './page_size_for_helper' +require_relative './total_pages_for_helper' + +module Pagination + module RecordLastForHelper + include Pagination::RecordSizeForHelper + include Pagination::CurrentPageForHelper + include Pagination::PageSizeForHelper + include Pagination::TotalPagesForHelper + + def pagination_record_last_for(set) + record_size = pagination_record_size_for(set) + current_page = pagination_current_page_for(set) + page_size = pagination_page_size_for(set) + total_pages = pagination_total_pages_for(set) + + return record_size if current_page == total_pages + + current_page * page_size + end + end +end diff --git a/lib/action_set/helpers/pagination/record_range_for_helper.rb b/lib/action_set/helpers/pagination/record_range_for_helper.rb new file mode 100644 index 0000000..979b173 --- /dev/null +++ b/lib/action_set/helpers/pagination/record_range_for_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require_relative './record_first_for_helper' +require_relative './record_last_for_helper' + +module Pagination + module RecordRangeForHelper + include Pagination::RecordFirstForHelper + include Pagination::RecordLastForHelper + + def pagination_record_range_for(set) + [ + pagination_record_first_for(set), + '–', + pagination_record_last_for(set), + ].join(' ').html_safe + end + end +end diff --git a/lib/action_set/helpers/pagination/record_size_for_helper.rb b/lib/action_set/helpers/pagination/record_size_for_helper.rb new file mode 100644 index 0000000..85ba182 --- /dev/null +++ b/lib/action_set/helpers/pagination/record_size_for_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Pagination + module RecordSizeForHelper + def pagination_record_size_for(set) + set.instructions.dig(:paginate, :count) + end + end +end diff --git a/lib/action_set/helpers/pagination/total_pages_for_helper.rb b/lib/action_set/helpers/pagination/total_pages_for_helper.rb index 4a977e3..658b867 100644 --- a/lib/action_set/helpers/pagination/total_pages_for_helper.rb +++ b/lib/action_set/helpers/pagination/total_pages_for_helper.rb @@ -1,13 +1,15 @@ # frozen_string_literal: true +require_relative './record_size_for_helper' require_relative './page_size_for_helper' module Pagination module TotalPagesForHelper + include Pagination::RecordSizeForHelper include Pagination::PageSizeForHelper def pagination_total_pages_for(set) - total_set_size = set.instructions.dig(:paginate, :count) + total_set_size = pagination_record_size_for(set) return 1 if total_set_size.zero? (total_set_size.to_f / pagination_page_size_for(set)).ceil From b5f974232046669c85af744c6bc2ae6b73bd1b41 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 31 Jul 2019 15:07:34 +0200 Subject: [PATCH 37/56] bundle exec rubocop --safe-auto-correct . --- lib/action_set.rb | 4 +- .../pagination/record_range_for_helper.rb | 2 +- .../filtering/active_record/operators.rb | 42 +++++++++---------- .../filtering/active_record/query_column.rb | 2 +- .../filtering/active_record/query_value.rb | 2 +- lib/active_set/filtering/constants.rb | 24 +++++------ 6 files changed, 38 insertions(+), 38 deletions(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 9a1c4bd..43f001f 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -102,9 +102,9 @@ def export_instructions elsif respond_to?(:export_set_columns, true) send(:export_set_columns) else - # :nocov: + # :nocov: [{}] - # :nocov: + # :nocov: end end end diff --git a/lib/action_set/helpers/pagination/record_range_for_helper.rb b/lib/action_set/helpers/pagination/record_range_for_helper.rb index 979b173..a370281 100644 --- a/lib/action_set/helpers/pagination/record_range_for_helper.rb +++ b/lib/action_set/helpers/pagination/record_range_for_helper.rb @@ -12,7 +12,7 @@ def pagination_record_range_for(set) [ pagination_record_first_for(set), '–', - pagination_record_last_for(set), + pagination_record_last_for(set) ].join(' ').html_safe end end diff --git a/lib/active_set/filtering/active_record/operators.rb b/lib/active_set/filtering/active_record/operators.rb index dda3d90..c1954fb 100644 --- a/lib/active_set/filtering/active_record/operators.rb +++ b/lib/active_set/filtering/active_record/operators.rb @@ -14,7 +14,7 @@ module Operators Range.new(*raw.sort) end end - BLANK_TRANSFORMER = proc do |type:, **ctx| + BLANK_TRANSFORMER = proc do |type:, **_ctx| if type.presence_in %i[date float integer] [nil] else @@ -24,7 +24,7 @@ module Operators MATCHER_TRANSFORMER = proc do |sql:, type:, **ctx| next sql.map { |str| MATCHER_TRANSFORMER.call(sql: str, type: type, **ctx) } if sql.respond_to?(:map) - next sql if not type == :decimal + next sql if type != :decimal next sql[0..-3] if sql.ends_with?('.0') next sql[0..-4] + '%' if sql.ends_with?('.0%') @@ -173,7 +173,7 @@ module Operators }, IS_NULL: { - operator: :eq, + operator: :eq }, NOT_NULL: { operator: :not_eq @@ -190,75 +190,75 @@ module Operators MATCH_START: { operator: :matches, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_START_ANY: { operator: :matches_any, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_START_ALL: { operator: :matches_all, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_NOT_START: { operator: :does_not_match, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_NOT_START_ANY: { operator: :does_not_match_any, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_NOT_START_ALL: { operator: :does_not_match_all, - query_attribute_transformer: START_MATCHER_TRANSFORMER, + query_attribute_transformer: START_MATCHER_TRANSFORMER }, MATCH_END: { operator: :matches, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_END_ANY: { operator: :matches_any, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_END_ALL: { operator: :matches_all, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_NOT_END: { operator: :does_not_match, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_NOT_END_ANY: { operator: :does_not_match_any, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_NOT_END_ALL: { operator: :does_not_match_all, - query_attribute_transformer: END_MATCHER_TRANSFORMER, + query_attribute_transformer: END_MATCHER_TRANSFORMER }, MATCH_CONTAIN: { operator: :matches, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER }, MATCH_CONTAIN_ANY: { operator: :matches_any, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER }, MATCH_CONTAIN_ALL: { operator: :matches_all, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER }, MATCH_NOT_CONTAIN: { operator: :does_not_match, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER }, MATCH_NOT_CONTAIN_ANY: { operator: :does_not_match_any, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER }, MATCH_NOT_CONTAIN_ALL: { operator: :does_not_match_all, - query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER, + query_attribute_transformer: CONTAIN_MATCHER_TRANSFORMER } }.freeze diff --git a/lib/active_set/filtering/active_record/query_column.rb b/lib/active_set/filtering/active_record/query_column.rb index be42aef..cb2f4c0 100644 --- a/lib/active_set/filtering/active_record/query_column.rb +++ b/lib/active_set/filtering/active_record/query_column.rb @@ -25,7 +25,7 @@ def column_cast_as_char def must_cast_numerical_column? # The LIKE operator can’t be used if the column hosts numeric types. - return false if not arel_type.presence_in(%i[integer float]) + return false unless arel_type.presence_in(%i[integer float]) arel_operator.to_s.downcase.include?('match') end diff --git a/lib/active_set/filtering/active_record/query_value.rb b/lib/active_set/filtering/active_record/query_value.rb index 844ad07..b125301 100644 --- a/lib/active_set/filtering/active_record/query_value.rb +++ b/lib/active_set/filtering/active_record/query_value.rb @@ -37,7 +37,7 @@ def to_sql_str(value) end def unwrap_sql_str(sql_str) - return sql_str if not sql_str[0] == "'" && sql_str[-1] == "'" + return sql_str unless sql_str[0] == "'" && sql_str[-1] == "'" sql_str[1..-2] end diff --git a/lib/active_set/filtering/constants.rb b/lib/active_set/filtering/constants.rb index 1374e06..77b5037 100644 --- a/lib/active_set/filtering/constants.rb +++ b/lib/active_set/filtering/constants.rb @@ -249,62 +249,62 @@ module Constants MATCH_START: { type: :binary, compound: false, - behavior: :inclusive, + behavior: :inclusive }, MATCH_START_ANY: { type: :binary, compound: true, - behavior: :inclusive, + behavior: :inclusive }, MATCH_START_ALL: { type: :binary, compound: true, - behavior: :exclusive, + behavior: :exclusive }, MATCH_NOT_START: { type: :binary, compound: false, - behavior: :exclusive, + behavior: :exclusive }, MATCH_NOT_START_ANY: { type: :binary, compound: true, - behavior: :inclusive, + behavior: :inclusive }, MATCH_NOT_START_ALL: { type: :binary, compound: true, - behavior: :exclusive, + behavior: :exclusive }, MATCH_END: { type: :binary, compound: false, - behavior: :inclusive, + behavior: :inclusive }, MATCH_END_ANY: { type: :binary, compound: true, - behavior: :inclusive, + behavior: :inclusive }, MATCH_END_ALL: { type: :binary, compound: true, - behavior: :exclusive, + behavior: :exclusive }, MATCH_NOT_END: { type: :binary, compound: false, - behavior: :exclusive, + behavior: :exclusive }, MATCH_NOT_END_ANY: { type: :binary, compound: true, - behavior: :inclusive, + behavior: :inclusive }, MATCH_NOT_END_ALL: { type: :binary, compound: true, - behavior: :exclusive, + behavior: :exclusive }, MATCH_CONTAIN: { type: :binary, From 0db1eaaad7e69d69fe0c4530df42e8374b005a20 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 31 Jul 2019 15:51:59 +0200 Subject: [PATCH 38/56] use AR's data dictionary to look up database column types or allow app to define type hints for filter attributes --- lib/action_set.rb | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 43f001f..2406b55 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -64,14 +64,6 @@ def filter_instructions_for(set) end end - def filter_typecasted_value_for(keypath, value, set) - instruction = ActiveSet::AttributeInstruction.new(keypath, value) - item_with_value = set.find { |i| !instruction.value_for(item: i).nil? } - item_value = instruction.value_for(item: item_with_value) - ActionSet::AttributeValue.new(value) - .cast(to: item_value.class) - end - def sort_instructions if sort_params.key?(:attribute) && sort_params.key?(:direction) { sort_params[:attribute] => sort_params[:direction] } @@ -110,6 +102,31 @@ def export_instructions end # rubocop:enable Metrics/AbcSize + def filter_typecasted_value_for(keypath, value, set) + klass = klass_for_keypath(keypath, value, set) + ActionSet::AttributeValue.new(value) + .cast(to: klass) + end + + def klass_for_keypath(keypath, value, set) + if respond_to?(:filter_set_types, true) + type_declarations = public_send(:filter_set_types) + types = type_declarations['types'] || type_declarations[:types] + klass = types[keypath.join('.')] + return klass if klass + end + + if set.is_a?(ActiveRecord::Relation) || set.view.is_a?(ActiveRecord::Relation) + klass_type = set.model.columns_hash.fetch(keypath, nil)&.type + return klass_type.class if klass_type + end + + instruction = ActiveSet::AttributeInstruction.new(keypath, value) + item_with_value = set.find { |i| !instruction.value_for(item: i).nil? } + item_value = instruction.value_for(item: item_with_value) + item_value.class + end + def filter_params params.fetch(:filter, {}).to_unsafe_hash end From 7b709b96cb45b8f0eb53dbdf964195056a756f5c Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 14:20:29 +0200 Subject: [PATCH 39/56] Add specs (with some fixes) for the new pagination record helpers --- .../pagination/record_last_for_helper.rb | 2 +- .../pagination/record_range_for_helper.rb | 6 + .../record_description_for_helper_spec.rb | 139 ++++++++++++++++++ .../record_range_for_helper_spec.rb | 139 ++++++++++++++++++ 4 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 spec/helpers/pagination/record_description_for_helper_spec.rb create mode 100644 spec/helpers/pagination/record_range_for_helper_spec.rb diff --git a/lib/action_set/helpers/pagination/record_last_for_helper.rb b/lib/action_set/helpers/pagination/record_last_for_helper.rb index 396e654..8dd7b30 100644 --- a/lib/action_set/helpers/pagination/record_last_for_helper.rb +++ b/lib/action_set/helpers/pagination/record_last_for_helper.rb @@ -18,7 +18,7 @@ def pagination_record_last_for(set) page_size = pagination_page_size_for(set) total_pages = pagination_total_pages_for(set) - return record_size if current_page == total_pages + return record_size if current_page >= total_pages current_page * page_size end diff --git a/lib/action_set/helpers/pagination/record_range_for_helper.rb b/lib/action_set/helpers/pagination/record_range_for_helper.rb index a370281..be6435d 100644 --- a/lib/action_set/helpers/pagination/record_range_for_helper.rb +++ b/lib/action_set/helpers/pagination/record_range_for_helper.rb @@ -5,10 +5,16 @@ module Pagination module RecordRangeForHelper + include Pagination::CurrentPageForHelper + include Pagination::TotalPagesForHelper include Pagination::RecordFirstForHelper include Pagination::RecordLastForHelper def pagination_record_range_for(set) + current_page = pagination_current_page_for(set) + total_pages = pagination_total_pages_for(set) + return 'None' if current_page > total_pages + [ pagination_record_first_for(set), '–', diff --git a/spec/helpers/pagination/record_description_for_helper_spec.rb b/spec/helpers/pagination/record_description_for_helper_spec.rb new file mode 100644 index 0000000..0e27e78 --- /dev/null +++ b/spec/helpers/pagination/record_description_for_helper_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Pagination::RecordDescriptionForHelper, type: :helper do + before(:each) do + allow(helper).to receive(:params).and_return(params.merge(_recall: { controller: 'things', action: 'index' })) + end + let(:params) do + {} + end + let(:set) { [1, 2, 3, 4, 5, 6, 7, 8, 9, 0] } + + describe '.pagination_record_description_for' do + subject { helper.pagination_record_description_for(active_set) } + + context 'size: 10' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: set.size) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 10 of 10 records') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None of 10 records') } + end + end + + context 'size: 5' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 5) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 5 of 10 records') } + end + + context 'page_number: 2' do + let(:page_number) { 2 } + + it { should eql('6 – 10 of 10 records') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None of 10 records') } + end + end + + context 'size: 4' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 4) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 4 of 10 records') } + end + + context 'page_number: 2' do + let(:page_number) { 2 } + + it { should eql('5 – 8 of 10 records') } + end + + context 'page_number: 3' do + let(:page_number) { 3 } + + it { should eql('9 – 10 of 10 records') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None of 10 records') } + end + end + + context 'size: 2' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 2) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 2 of 10 records') } + end + + context 'page_number: 3' do + let(:page_number) { 3 } + + it { should eql('5 – 6 of 10 records') } + end + + context 'page_number: 5' do + let(:page_number) { 5 } + + it { should eql('9 – 10 of 10 records') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None of 10 records') } + end + end + + context 'size: 1' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 1) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 1 of 10 records') } + end + + context 'page_number: 5' do + let(:page_number) { 5 } + + it { should eql('5 – 5 of 10 records') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('10 – 10 of 10 records') } + end + + context 'page_number: 20' do + let(:page_number) { 20 } + + it { should eql('None of 10 records') } + end + end + end +end diff --git a/spec/helpers/pagination/record_range_for_helper_spec.rb b/spec/helpers/pagination/record_range_for_helper_spec.rb new file mode 100644 index 0000000..d098982 --- /dev/null +++ b/spec/helpers/pagination/record_range_for_helper_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Pagination::RecordRangeForHelper, type: :helper do + before(:each) do + allow(helper).to receive(:params).and_return(params.merge(_recall: { controller: 'things', action: 'index' })) + end + let(:params) do + {} + end + let(:set) { [1, 2, 3, 4, 5, 6, 7, 8, 9, 0] } + + describe '.pagination_record_range_for' do + subject { helper.pagination_record_range_for(active_set) } + + context 'size: 10' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: set.size) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 10') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None') } + end + end + + context 'size: 5' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 5) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 5') } + end + + context 'page_number: 2' do + let(:page_number) { 2 } + + it { should eql('6 – 10') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None') } + end + end + + context 'size: 4' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 4) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 4') } + end + + context 'page_number: 2' do + let(:page_number) { 2 } + + it { should eql('5 – 8') } + end + + context 'page_number: 3' do + let(:page_number) { 3 } + + it { should eql('9 – 10') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None') } + end + end + + context 'size: 2' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 2) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 2') } + end + + context 'page_number: 3' do + let(:page_number) { 3 } + + it { should eql('5 – 6') } + end + + context 'page_number: 5' do + let(:page_number) { 5 } + + it { should eql('9 – 10') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('None') } + end + end + + context 'size: 1' do + let(:active_set) { ActiveSet.new(set).paginate(page: page_number, size: 1) } + + context 'page_number: 1' do + let(:page_number) { 1 } + + it { should eql('1 – 1') } + end + + context 'page_number: 5' do + let(:page_number) { 5 } + + it { should eql('5 – 5') } + end + + context 'page_number: 10' do + let(:page_number) { 10 } + + it { should eql('10 – 10') } + end + + context 'page_number: 20' do + let(:page_number) { 20 } + + it { should eql('None') } + end + end + end +end From 4e94261f39718111d62e0915fab5fe88ecaea702 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 14:21:38 +0200 Subject: [PATCH 40/56] Fix the sort request spec to actually test something --- spec/requests/sort_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/requests/sort_spec.rb b/spec/requests/sort_spec.rb index 377ba14..e29bc67 100644 --- a/spec/requests/sort_spec.rb +++ b/spec/requests/sort_spec.rb @@ -48,8 +48,10 @@ end ApplicationRecord::SORTABLE_TYPES.each do |type| - all_possible_sort_instructions_for(type).sample do |instruction| + [all_possible_sort_instructions_for(type).sample].each do |instruction| context instruction do + let(:instructions) { instruction } + it_should_behave_like 'a sorted collection', instruction do let(:result) { @active_set.sort(instruction) } end @@ -58,8 +60,10 @@ end ApplicationRecord::SORTABLE_TYPES.combination(2).each do |type_1, type_2| - all_possible_sort_instruction_combinations_for(type_1, type_2).sample do |instructions| + [all_possible_sort_instruction_combinations_for(type_1, type_2).sample].each do |instructions| context instructions do + let(:instructions) { instructions } + it_should_behave_like 'a sorted collection', instructions do let(:result) { @active_set.sort(instructions) } end From 746be6e72a0f56cde8131d44a6839faaa6418166 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 16:13:40 +0200 Subject: [PATCH 41/56] Add tests to cover the new #filter_set_types method for short-circuiting the type inferencing when filtering a set --- spec/requests/filtering/types_spec.rb | 54 ++++++++++++++++++++------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/spec/requests/filtering/types_spec.rb b/spec/requests/filtering/types_spec.rb index 802a61e..def2ae1 100644 --- a/spec/requests/filtering/types_spec.rb +++ b/spec/requests/filtering/types_spec.rb @@ -14,6 +14,11 @@ let(:result_ids) { results.map { |f| f['id'] } } before(:each) do + allow_any_instance_of(ThingsController) + .to receive(:filter_set_types) + .and_return({ + types: instructions.transform_values(&:class) + }) if defined?(filter_set_types) get things_path(format: :json), params: { filter: instructions } end @@ -33,27 +38,48 @@ it { expect(result_ids).to eq [matching_item.id] } end + + context "{ #{path}: } #filter_set_types" do + let(:filter_set_types) { true } + let(:instructions) do + { + path => filter_value_for(object: matching_item, path: path) + } + end + + it { expect(result_ids).to eq [matching_item.id] } + end end end end ApplicationRecord::FIELD_TYPES.combination(2).each do |type_1, type_2| [1, 2].each do |id| - context "matching @thing_#{id}" do - let(:matching_item) { instance_variable_get("@thing_#{id}") } - - paths = all_possible_path_combinations_for(type_1, type_2) - paths.shuffle.take(paths.size / 2).each do |path_1, path_2| - context "{ #{path_1}:, #{path_2} }" do - let(:instructions) do - { - path_1 => filter_value_for(object: matching_item, path: path_1), - path_2 => filter_value_for(object: matching_item, path: path_2) - } - end - - it { expect(result_ids).to eq [matching_item.id] } + let(:matching_item) { instance_variable_get("@thing_#{id}") } + + paths = all_possible_path_combinations_for(type_1, type_2) + paths.shuffle.take(paths.size / 2).each do |path_1, path_2| + context "{ #{path_1}:, #{path_2} }" do + let(:instructions) do + { + path_1 => filter_value_for(object: matching_item, path: path_1), + path_2 => filter_value_for(object: matching_item, path: path_2) + } + end + + it { expect(result_ids).to eq [matching_item.id] } + end + + context "{ #{path_1}:, #{path_2} } #filter_set_types" do + let(:filter_set_types) { true } + let(:instructions) do + { + path_1 => filter_value_for(object: matching_item, path: path_1), + path_2 => filter_value_for(object: matching_item, path: path_2) + } end + + it { expect(result_ids).to eq [matching_item.id] } end end end From a27e65b4bdaaefa298c44f9b8fd7bf56b5ea0c42 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 16:29:34 +0200 Subject: [PATCH 42/56] Bump version and update CHANGELOG --- CHANGELOG | 22 ++++++++++++++++++++++ actionset.gemspec | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 29f6cfe..452dfbe 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,25 @@ +v 0.9.0 + - Add Enumerable support for the base filtering operations + + EQ, NOT_EQ, EQ_ANY, EQ_ALL, NOT_EQ_ANY, NOT_EQ_ALL + + IN, NOT_IN, IN_ANY, IN_ALL, NOT_IN_ANY, NOT_IN_ALL + + MATCHES, DOES_NOT_MATCH, MATCHES_ANY, MATCHES_ALL, DOES_NOT_MATCH_ANY, DOES_NOT_MATCH_ALL + + LT, LTEQ, LT_ANY, LT_ALL, LTEQ_ANY, LTEQ_ALL + + GT, GTEQ, GT_ANY, GT_ALL, GTEQ_ANY, GTEQ_ALL + + BETWEEN, NOT_BETWEEN + - Add unary predicate filtering operators, for both ActiveRecord and Enumerable + + IS_TRUE, IS_FALSE + + IS_NULL, NOT_NULL + + IS_PRESENT, IS_BLANK + - Add computed matcher filtering operators, for both ActiveRecord and Enumerable + + MATCH_START, MATCH_START_ANY, MATCH_START_ALL, MATCH_NOT_START, MATCH_NOT_START_ANY, MATCH_NOT_START_ALL + + MATCH_END, MATCH_END_ANY, MATCH_END_ALL, MATCH_NOT_END, MATCH_NOT_END_ANY, MATCH_NOT_END_ALL + + MATCH_CONTAIN, MATCH_CONTAIN_ANY, MATCH_CONTAIN_ALL, MATCH_NOT_CONTAIN, MATCH_NOT_CONTAIN_ANY, MATCH_NOT_CONTAIN_ALL + - Add view helpers for the range of records shown on the current page + - Use ActiveRecord's data dictionary to look up database column types when converting filter params to filter instructions + - Allow app to define type hints for filter attributes when converting filter params to filter instructions + - Allow sort params to be passed in short or long form + + e.g. { attribute: x, direction: x } or { attribute: direction } + - Fix enumerable intersection filtering when working across associations v 0.8.2 - add `ActiveSet.configuration.on_asc_sort_nils_come` configuration v 0.8.1 diff --git a/actionset.gemspec b/actionset.gemspec index 412412d..3a6b61e 100644 --- a/actionset.gemspec +++ b/actionset.gemspec @@ -6,7 +6,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) Gem::Specification.new do |spec| spec.platform = Gem::Platform::RUBY spec.name = 'actionset' - spec.version = '0.8.2' + spec.version = '0.9.0' spec.authors = ['Stephen Margheim'] spec.email = ['stephen.margheim@gmail.com'] From 840f0a38a395a2f70da70def4148e53029ee59e7 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 16:29:34 +0200 Subject: [PATCH 43/56] Bump version and update CHANGELOG --- CHANGELOG | 22 ++++++++++++++++++++++ Gemfile.lock | 2 +- actionset.gemspec | 2 +- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 29f6cfe..452dfbe 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,25 @@ +v 0.9.0 + - Add Enumerable support for the base filtering operations + + EQ, NOT_EQ, EQ_ANY, EQ_ALL, NOT_EQ_ANY, NOT_EQ_ALL + + IN, NOT_IN, IN_ANY, IN_ALL, NOT_IN_ANY, NOT_IN_ALL + + MATCHES, DOES_NOT_MATCH, MATCHES_ANY, MATCHES_ALL, DOES_NOT_MATCH_ANY, DOES_NOT_MATCH_ALL + + LT, LTEQ, LT_ANY, LT_ALL, LTEQ_ANY, LTEQ_ALL + + GT, GTEQ, GT_ANY, GT_ALL, GTEQ_ANY, GTEQ_ALL + + BETWEEN, NOT_BETWEEN + - Add unary predicate filtering operators, for both ActiveRecord and Enumerable + + IS_TRUE, IS_FALSE + + IS_NULL, NOT_NULL + + IS_PRESENT, IS_BLANK + - Add computed matcher filtering operators, for both ActiveRecord and Enumerable + + MATCH_START, MATCH_START_ANY, MATCH_START_ALL, MATCH_NOT_START, MATCH_NOT_START_ANY, MATCH_NOT_START_ALL + + MATCH_END, MATCH_END_ANY, MATCH_END_ALL, MATCH_NOT_END, MATCH_NOT_END_ANY, MATCH_NOT_END_ALL + + MATCH_CONTAIN, MATCH_CONTAIN_ANY, MATCH_CONTAIN_ALL, MATCH_NOT_CONTAIN, MATCH_NOT_CONTAIN_ANY, MATCH_NOT_CONTAIN_ALL + - Add view helpers for the range of records shown on the current page + - Use ActiveRecord's data dictionary to look up database column types when converting filter params to filter instructions + - Allow app to define type hints for filter attributes when converting filter params to filter instructions + - Allow sort params to be passed in short or long form + + e.g. { attribute: x, direction: x } or { attribute: direction } + - Fix enumerable intersection filtering when working across associations v 0.8.2 - add `ActiveSet.configuration.on_asc_sort_nils_come` configuration v 0.8.1 diff --git a/Gemfile.lock b/Gemfile.lock index b4cea8e..1a860f0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - actionset (0.8.2) + actionset (0.9.0) activesupport (>= 4.0.2) railties diff --git a/actionset.gemspec b/actionset.gemspec index 412412d..3a6b61e 100644 --- a/actionset.gemspec +++ b/actionset.gemspec @@ -6,7 +6,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) Gem::Specification.new do |spec| spec.platform = Gem::Platform::RUBY spec.name = 'actionset' - spec.version = '0.8.2' + spec.version = '0.9.0' spec.authors = ['Stephen Margheim'] spec.email = ['stephen.margheim@gmail.com'] From 7a03c2ab44b7bda9fe4ce8f625500366274c3d15 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 2 Aug 2019 16:40:18 +0200 Subject: [PATCH 44/56] Bump to version 0.9.1 to ensure a good build --- CHANGELOG | 2 ++ Gemfile.lock | 2 +- actionset.gemspec | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 452dfbe..fc6b7ab 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ +v 0.9.1 + - ensure that our RubyGems build is stable, since we had a TravisCI build problem in the last version v 0.9.0 - Add Enumerable support for the base filtering operations + EQ, NOT_EQ, EQ_ANY, EQ_ALL, NOT_EQ_ANY, NOT_EQ_ALL diff --git a/Gemfile.lock b/Gemfile.lock index 1a860f0..1473b99 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - actionset (0.9.0) + actionset (0.9.1) activesupport (>= 4.0.2) railties diff --git a/actionset.gemspec b/actionset.gemspec index 3a6b61e..0768e01 100644 --- a/actionset.gemspec +++ b/actionset.gemspec @@ -6,7 +6,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) Gem::Specification.new do |spec| spec.platform = Gem::Platform::RUBY spec.name = 'actionset' - spec.version = '0.9.0' + spec.version = '0.9.1' spec.authors = ['Stephen Margheim'] spec.email = ['stephen.margheim@gmail.com'] From f9ea0227e6edd9628ae85d71db67ae64df36793f Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 6 Aug 2019 17:54:23 +0200 Subject: [PATCH 45/56] Allow the sort param to accept form-friendly structures { foo: :desc, bar: :asc } or { '1': { attribute: :bar, operator: :asc, query: 'foo' }, '0': { attribute: :foo, operator: :desc, query: 'foo' } } --- lib/action_set.rb | 16 ++++++++---- spec/requests/sort_spec.rb | 51 +++++++++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 2406b55..2ac90b8 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -65,11 +65,17 @@ def filter_instructions_for(set) end def sort_instructions - if sort_params.key?(:attribute) && sort_params.key?(:direction) - { sort_params[:attribute] => sort_params[:direction] } - else - sort_params.transform_values { |v| v.remove('ending') } - end + instructions_hash = if sort_params.key?(:'0') || sort_params.key?('0') + ordered_instructions = sort_params.sort_by(&:first) + array_of_instructions = ordered_instructions.map {|_, h| [h[:attribute], h[:direction]] } + Hash[array_of_instructions] + elsif sort_params.key?(:attribute) && sort_params.key?(:direction) + { sort_params[:attribute] => sort_params[:direction] } + else + sort_params + end + + instructions_hash.transform_values { |v| v.remove('ending') } end def paginate_instructions diff --git a/spec/requests/sort_spec.rb b/spec/requests/sort_spec.rb index e29bc67..dc73c39 100644 --- a/spec/requests/sort_spec.rb +++ b/spec/requests/sort_spec.rb @@ -52,8 +52,34 @@ context instruction do let(:instructions) { instruction } - it_should_behave_like 'a sorted collection', instruction do - let(:result) { @active_set.sort(instruction) } + it_should_behave_like 'a sorted collection', instruction + end + end + + [all_possible_paths_for(type).sample].each do |path| + [:asc, 'desc'].each do |dir| + context "{ 0: { attribute: #{path}, direction: #{dir} } }" do + let(:instructions) do + { + '0': { + attribute: path, + direction: dir + } + } + end + + it_should_behave_like 'a sorted collection', { path => dir } + end + + context "{ attribute: #{path}, direction: #{dir} }" do + let(:instructions) do + { + attribute: path, + direction: dir + } + end + + it_should_behave_like 'a sorted collection', { path => dir } end end end @@ -64,9 +90,26 @@ context instructions do let(:instructions) { instructions } - it_should_behave_like 'a sorted collection', instructions do - let(:result) { @active_set.sort(instructions) } + it_should_behave_like 'a sorted collection', instructions + end + end + + [all_possible_path_combinations_for(type_1, type_2).sample].each do |path_1, path_2| + context "{ 0: { attribute: #{path_1}, direction: asc }, 1: { attribute: #{path_2}, direction: desc } }" do + let(:instructions) do + { + '0': { + attribute: path_1, + direction: 'asc' + }, + '1': { + attribute: path_2, + direction: :desc + } + } end + + it_should_behave_like 'a sorted collection', { path_1 => 'asc', path_2 => :desc } end end end From 7ce176fb768d7e02df7a3dbffbf4495563ef8988 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Tue, 6 Aug 2019 17:54:52 +0200 Subject: [PATCH 46/56] Allow the filter params to accept form-friendly structures { :foo=>:desc, :bar=>:asc } or { '1': { attribute: :bar, operator: :asc, query: 'foo' }, '0': { attribute: :foo, operator: :desc, query: 'foo' } } --- lib/action_set.rb | 12 ++++++- spec/requests/filtering/types_spec.rb | 51 ++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 2ac90b8..67b331f 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -53,7 +53,17 @@ def export_set(set) private def filter_instructions_for(set) - flatten_keys_of(filter_params).reject { |_, v| v.try(:empty?) }.each_with_object({}) do |(keypath, value), memo| + instructions_hash = if filter_params.key?(:'0') || filter_params.key?('0') + ordered_instructions = filter_params.sort_by(&:first) + array_of_instructions = ordered_instructions.map {|_, h| ["#{h[:attribute]}(#{h[:operator]})", h[:query]] } + Hash[array_of_instructions] + elsif filter_params.key?(:attribute) || filter_params.key?('attribute') + { "#{filter_params[:attribute]}(#{filter_params[:operator]})" => filter_params[:query] } + else + filter_params + end + flattened_instructions = flatten_keys_of(instructions_hash).reject { |_, v| v.try(:empty?) } + flattened_instructions.each_with_object({}) do |(keypath, value), memo| typecast_value = if value.respond_to?(:each) value.map { |v| filter_typecasted_value_for(keypath, v, set) } else diff --git a/spec/requests/filtering/types_spec.rb b/spec/requests/filtering/types_spec.rb index def2ae1..0d63785 100644 --- a/spec/requests/filtering/types_spec.rb +++ b/spec/requests/filtering/types_spec.rb @@ -27,8 +27,7 @@ [1, 2].each do |id| let(:matching_item) { instance_variable_get("@thing_#{id}") } - paths = all_possible_paths_for(type) - paths.shuffle.take(paths.size / 2).each do |path| + [all_possible_paths_for(type).sample].each do |path| context "{ #{path}: }" do let(:instructions) do { @@ -49,6 +48,32 @@ it { expect(result_ids).to eq [matching_item.id] } end + + context "{ 0: { attribute: #{path} } }" do + let(:instructions) do + { + '0': { + attribute: path, + operator: 'EQ', + query: filter_value_for(object: matching_item, path: path) + } + } + end + + it { expect(result_ids).to eq [matching_item.id] } + end + + context "{ { attribute: #{path} } }" do + let(:instructions) do + { + attribute: path, + operator: 'EQ', + query: filter_value_for(object: matching_item, path: path) + } + end + + it { expect(result_ids).to eq [matching_item.id] } + end end end end @@ -57,8 +82,7 @@ [1, 2].each do |id| let(:matching_item) { instance_variable_get("@thing_#{id}") } - paths = all_possible_path_combinations_for(type_1, type_2) - paths.shuffle.take(paths.size / 2).each do |path_1, path_2| + [all_possible_path_combinations_for(type_1, type_2).sample].each do |path_1, path_2| context "{ #{path_1}:, #{path_2} }" do let(:instructions) do { @@ -81,6 +105,25 @@ it { expect(result_ids).to eq [matching_item.id] } end + + context "{ 0: { attribute: #{path_1} }, 1: { attribute: #{path_2} } }" do + let(:instructions) do + { + '0': { + attribute: path_1, + operator: 'EQ', + query: filter_value_for(object: matching_item, path: path_1) + }, + '1': { + attribute: path_2, + operator: 'EQ', + query: filter_value_for(object: matching_item, path: path_2) + } + } + end + + it { expect(result_ids).to eq [matching_item.id] } + end end end end From bc2ae8b48d026c3634fc216f9f65465643f16779 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 7 Aug 2019 21:35:01 +0200 Subject: [PATCH 47/56] Fix some Rubocop issues --- lib/action_set.rb | 4 ++-- lib/active_set/filtering/active_record/query_column.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/action_set.rb b/lib/action_set.rb index 67b331f..7f93023 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -55,7 +55,7 @@ def export_set(set) def filter_instructions_for(set) instructions_hash = if filter_params.key?(:'0') || filter_params.key?('0') ordered_instructions = filter_params.sort_by(&:first) - array_of_instructions = ordered_instructions.map {|_, h| ["#{h[:attribute]}(#{h[:operator]})", h[:query]] } + array_of_instructions = ordered_instructions.map { |_, h| ["#{h[:attribute]}(#{h[:operator]})", h[:query]] } Hash[array_of_instructions] elsif filter_params.key?(:attribute) || filter_params.key?('attribute') { "#{filter_params[:attribute]}(#{filter_params[:operator]})" => filter_params[:query] } @@ -77,7 +77,7 @@ def filter_instructions_for(set) def sort_instructions instructions_hash = if sort_params.key?(:'0') || sort_params.key?('0') ordered_instructions = sort_params.sort_by(&:first) - array_of_instructions = ordered_instructions.map {|_, h| [h[:attribute], h[:direction]] } + array_of_instructions = ordered_instructions.map { |_, h| [h[:attribute], h[:direction]] } Hash[array_of_instructions] elsif sort_params.key?(:attribute) && sort_params.key?(:direction) { sort_params[:attribute] => sort_params[:direction] } diff --git a/lib/active_set/filtering/active_record/query_column.rb b/lib/active_set/filtering/active_record/query_column.rb index cb2f4c0..c5a8666 100644 --- a/lib/active_set/filtering/active_record/query_column.rb +++ b/lib/active_set/filtering/active_record/query_column.rb @@ -24,7 +24,7 @@ def column_cast_as_char end def must_cast_numerical_column? - # The LIKE operator can’t be used if the column hosts numeric types. + # The LIKE operator can't be used if the column hosts numeric types. return false unless arel_type.presence_in(%i[integer float]) arel_operator.to_s.downcase.include?('match') From 01b05744543c02e77b8282f6fe4dda9803b6f285 Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 7 Aug 2019 22:38:01 +0200 Subject: [PATCH 48/56] Move the logic for transforming sorting and filtering request params into instructions into service classes --- lib/action_set.rb | 68 ++----------------------- lib/action_set/filter_instructions.rb | 72 +++++++++++++++++++++++++++ lib/action_set/sort_instructions.rb | 44 ++++++++++++++++ 3 files changed, 120 insertions(+), 64 deletions(-) create mode 100644 lib/action_set/filter_instructions.rb create mode 100644 lib/action_set/sort_instructions.rb diff --git a/lib/action_set.rb b/lib/action_set.rb index 7f93023..5ed81bd 100644 --- a/lib/action_set.rb +++ b/lib/action_set.rb @@ -5,7 +5,8 @@ require 'active_support/lazy_load_hooks' require 'active_set' -require_relative './action_set/attribute_value' +require_relative './action_set/filter_instructions' +require_relative './action_set/sort_instructions' require_relative './action_set/helpers/helper_methods' module ActionSet @@ -24,13 +25,13 @@ def process_set(set) def filter_set(set) active_set = ensure_active_set(set) - active_set = active_set.filter(filter_instructions_for(set)) if filter_params.any? + active_set = active_set.filter(FilterInstructions.new(filter_params, set, self).get) if filter_params.any? active_set end def sort_set(set) active_set = ensure_active_set(set) - active_set = active_set.sort(sort_instructions) if sort_params.any? + active_set = active_set.sort(SortInstructions.new(sort_params, set, self).get) if sort_params.any? active_set end @@ -52,42 +53,6 @@ def export_set(set) private - def filter_instructions_for(set) - instructions_hash = if filter_params.key?(:'0') || filter_params.key?('0') - ordered_instructions = filter_params.sort_by(&:first) - array_of_instructions = ordered_instructions.map { |_, h| ["#{h[:attribute]}(#{h[:operator]})", h[:query]] } - Hash[array_of_instructions] - elsif filter_params.key?(:attribute) || filter_params.key?('attribute') - { "#{filter_params[:attribute]}(#{filter_params[:operator]})" => filter_params[:query] } - else - filter_params - end - flattened_instructions = flatten_keys_of(instructions_hash).reject { |_, v| v.try(:empty?) } - flattened_instructions.each_with_object({}) do |(keypath, value), memo| - typecast_value = if value.respond_to?(:each) - value.map { |v| filter_typecasted_value_for(keypath, v, set) } - else - filter_typecasted_value_for(keypath, value, set) - end - - memo[keypath] = typecast_value - end - end - - def sort_instructions - instructions_hash = if sort_params.key?(:'0') || sort_params.key?('0') - ordered_instructions = sort_params.sort_by(&:first) - array_of_instructions = ordered_instructions.map { |_, h| [h[:attribute], h[:direction]] } - Hash[array_of_instructions] - elsif sort_params.key?(:attribute) && sort_params.key?(:direction) - { sort_params[:attribute] => sort_params[:direction] } - else - sort_params - end - - instructions_hash.transform_values { |v| v.remove('ending') } - end - def paginate_instructions paginate_params.transform_values(&:to_i) end @@ -118,31 +83,6 @@ def export_instructions end # rubocop:enable Metrics/AbcSize - def filter_typecasted_value_for(keypath, value, set) - klass = klass_for_keypath(keypath, value, set) - ActionSet::AttributeValue.new(value) - .cast(to: klass) - end - - def klass_for_keypath(keypath, value, set) - if respond_to?(:filter_set_types, true) - type_declarations = public_send(:filter_set_types) - types = type_declarations['types'] || type_declarations[:types] - klass = types[keypath.join('.')] - return klass if klass - end - - if set.is_a?(ActiveRecord::Relation) || set.view.is_a?(ActiveRecord::Relation) - klass_type = set.model.columns_hash.fetch(keypath, nil)&.type - return klass_type.class if klass_type - end - - instruction = ActiveSet::AttributeInstruction.new(keypath, value) - item_with_value = set.find { |i| !instruction.value_for(item: i).nil? } - item_value = instruction.value_for(item: item_with_value) - item_value.class - end - def filter_params params.fetch(:filter, {}).to_unsafe_hash end diff --git a/lib/action_set/filter_instructions.rb b/lib/action_set/filter_instructions.rb new file mode 100644 index 0000000..9502393 --- /dev/null +++ b/lib/action_set/filter_instructions.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require_relative './attribute_value' + +module ActionSet + class FilterInstructions + def initialize(params, set, controller) + @params = params + @set = set + @controller = controller + end + + def get + instructions_hash = if form_friendly_complex_params? + form_friendly_complex_params_to_hash + elsif form_friendly_simple_params? + form_friendly_simple_params_to_hash + else + @params + end + flattened_instructions = flatten_keys_of(instructions_hash).reject { |_, v| v.try(:empty?) } + flattened_instructions.each_with_object({}) do |(keypath, value), memo| + memo[keypath] = if value.respond_to?(:map) + value.map { |v| AttributeValue.new(v).cast(to: klass_for_keypath(keypath, v, @set)) } + else + AttributeValue.new(value).cast(to: klass_for_keypath(keypath, value, @set)) + end + end + end + + private + + def form_friendly_complex_params? + @params.key?(:'0') + end + + def form_friendly_simple_params? + @params.key?(:attribute) && + @params.key?(:operator) && + @params.key?(:query) + end + + def form_friendly_complex_params_to_hash + ordered_instructions = @params.sort_by(&:first) + array_of_instructions = ordered_instructions.map { |_, h| ["#{h[:attribute]}(#{h[:operator]})", h[:query]] } + Hash[array_of_instructions] + end + + def form_friendly_simple_params_to_hash + { "#{@params[:attribute]}(#{@params[:operator]})" => @params[:query] } + end + + def klass_for_keypath(keypath, value, set) + if @controller.respond_to?(:filter_set_types, true) + type_declarations = @controller.public_send(:filter_set_types) + types = type_declarations['types'] || type_declarations[:types] + klass = types[keypath.join('.')] + return klass if klass + end + + if set.is_a?(ActiveRecord::Relation) || set.view.is_a?(ActiveRecord::Relation) + klass_type = set.model.columns_hash.fetch(keypath, nil)&.type + return klass_type.class if klass_type + end + + instruction = ActiveSet::AttributeInstruction.new(keypath, value) + item_with_value = set.find { |i| !instruction.value_for(item: i).nil? } + item_value = instruction.value_for(item: item_with_value) + item_value.class + end + end +end diff --git a/lib/action_set/sort_instructions.rb b/lib/action_set/sort_instructions.rb new file mode 100644 index 0000000..eee06ab --- /dev/null +++ b/lib/action_set/sort_instructions.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module ActionSet + class SortInstructions + def initialize(params, set, controller) + @params = params + @set = set + @controller = controller + end + + def get + instructions_hash = if form_friendly_complex_params? + form_friendly_complex_params_to_hash + elsif form_friendly_simple_params? + form_friendly_simple_params_to_hash + else + @params + end + + instructions_hash.transform_values { |v| v.remove('ending') } + end + + private + + def form_friendly_complex_params? + @params.key?(:'0') + end + + def form_friendly_simple_params? + @params.key?(:attribute) && + @params.key?(:direction) + end + + def form_friendly_complex_params_to_hash + ordered_instructions = @params.sort_by(&:first) + array_of_instructions = ordered_instructions.map { |_, h| [h[:attribute], h[:direction]] } + Hash[array_of_instructions] + end + + def form_friendly_simple_params_to_hash + { @params[:attribute] => @params[:direction] } + end + end +end From f8f65032bb33ae37cb5ca226704ee6be0e4ef99d Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Wed, 7 Aug 2019 22:48:00 +0200 Subject: [PATCH 49/56] Bump to version 0.9.2 and update CHANGELOG --- CHANGELOG | 34 ++++++++++++++++++++++++++++++++++ Gemfile.lock | 2 +- actionset.gemspec | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index fc6b7ab..ea935e7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,37 @@ +v 0.9.2 + - allow for form-friendly filter parameters + ``` + { + '0': { + attribute: :foo, + operator: :EQ, + query: 'query' + }, + '1': { + attribute: :bar, + operator: :GT, + query: 'value' + } + } + + { foo: 'query', bar(GT): 'value' } + ``` + - allow for form-friendly sort parameters + ``` + { + '0': { + attribute: :foo, + direction: :desc + }, + '1': { + attribute: :bar, + direction: :asc + } + } + + { foo: 'desc', bar: 'asc' } + ``` + v 0.9.1 - ensure that our RubyGems build is stable, since we had a TravisCI build problem in the last version v 0.9.0 diff --git a/Gemfile.lock b/Gemfile.lock index 1473b99..390726a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - actionset (0.9.1) + actionset (0.9.2) activesupport (>= 4.0.2) railties diff --git a/actionset.gemspec b/actionset.gemspec index 0768e01..62024bc 100644 --- a/actionset.gemspec +++ b/actionset.gemspec @@ -6,7 +6,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) Gem::Specification.new do |spec| spec.platform = Gem::Platform::RUBY spec.name = 'actionset' - spec.version = '0.9.1' + spec.version = '0.9.2' spec.authors = ['Stephen Margheim'] spec.email = ['stephen.margheim@gmail.com'] From 3cac0e60f79f71173cd8d588a636063d8dda342f Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 9 Aug 2019 11:10:59 +0200 Subject: [PATCH 50/56] Clean up the filtering and sorting specs to have helpers determine whether to run all possible paths or only a randomly selected path based on an ENV variable --- spec/active_set/filter/predicates_spec.rb | 42 +++-------- spec/requests/filtering/predicates_spec.rb | 42 +++-------- spec/requests/filtering/types_spec.rb | 4 +- spec/requests/sort_spec.rb | 8 +- spec/support/filtering_helpers.rb | 86 ++++++++++++++++------ spec/support/path_helpers.rb | 21 ++++-- spec/support/sorting_helpers.rb | 8 +- 7 files changed, 114 insertions(+), 97 deletions(-) diff --git a/spec/active_set/filter/predicates_spec.rb b/spec/active_set/filter/predicates_spec.rb index 9988b3d..226ee48 100644 --- a/spec/active_set/filter/predicates_spec.rb +++ b/spec/active_set/filter/predicates_spec.rb @@ -19,11 +19,8 @@ ApplicationRecord::DB_FIELD_TYPES.each do |type| [1, 2].each do |id| - INCLUSIVE_UNARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + inclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -40,11 +37,8 @@ end end - EXCLUSIVE_UNARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + exclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -61,11 +55,8 @@ end end - INCONCLUSIVE_UNARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + inconclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -91,11 +82,8 @@ end end - INCLUSIVE_BINARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + inclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -120,11 +108,8 @@ end end - EXCLUSIVE_BINARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + exclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -149,11 +134,8 @@ end end - INCONCLUSIVE_BINARY_OPERATORS.each do |operator| - %W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].each do |path| + inconclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do diff --git a/spec/requests/filtering/predicates_spec.rb b/spec/requests/filtering/predicates_spec.rb index c919a88..f672bc8 100644 --- a/spec/requests/filtering/predicates_spec.rb +++ b/spec/requests/filtering/predicates_spec.rb @@ -21,11 +21,8 @@ ApplicationRecord::DB_FIELD_TYPES.each do |type| [1, 2].each do |id| - [INCLUSIVE_UNARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + inclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -42,11 +39,8 @@ end end - [EXCLUSIVE_UNARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + exclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -63,11 +57,8 @@ end end - [INCONCLUSIVE_UNARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + inconclusive_unary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:instruction_single_value) do @@ -95,11 +86,8 @@ end end - [INCLUSIVE_BINARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + inclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -126,11 +114,8 @@ end end - [EXCLUSIVE_BINARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + exclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do @@ -157,11 +142,8 @@ end end - [INCONCLUSIVE_BINARY_OPERATORS.sample].each do |operator| - [%W[ - #{type}(#{operator}) - only.#{type}(#{operator}) - ].sample].each do |path| + inconclusive_binary_operators.each do |operator| + all_possible_type_operator_paths_for(type, operator).each do |path| context "{ #{path}: }" do let(:matching_item) { instance_variable_get("@thing_#{id}") } let(:other_thing) do diff --git a/spec/requests/filtering/types_spec.rb b/spec/requests/filtering/types_spec.rb index 0d63785..6aad621 100644 --- a/spec/requests/filtering/types_spec.rb +++ b/spec/requests/filtering/types_spec.rb @@ -27,7 +27,7 @@ [1, 2].each do |id| let(:matching_item) { instance_variable_get("@thing_#{id}") } - [all_possible_paths_for(type).sample].each do |path| + all_possible_paths_for(type).each do |path| context "{ #{path}: }" do let(:instructions) do { @@ -82,7 +82,7 @@ [1, 2].each do |id| let(:matching_item) { instance_variable_get("@thing_#{id}") } - [all_possible_path_combinations_for(type_1, type_2).sample].each do |path_1, path_2| + all_possible_path_combinations_for(type_1, type_2).each do |path_1, path_2| context "{ #{path_1}:, #{path_2} }" do let(:instructions) do { diff --git a/spec/requests/sort_spec.rb b/spec/requests/sort_spec.rb index dc73c39..598ff8a 100644 --- a/spec/requests/sort_spec.rb +++ b/spec/requests/sort_spec.rb @@ -48,7 +48,7 @@ end ApplicationRecord::SORTABLE_TYPES.each do |type| - [all_possible_sort_instructions_for(type).sample].each do |instruction| + all_possible_sort_instructions_for(type).each do |instruction| context instruction do let(:instructions) { instruction } @@ -56,7 +56,7 @@ end end - [all_possible_paths_for(type).sample].each do |path| + all_possible_paths_for(type).each do |path| [:asc, 'desc'].each do |dir| context "{ 0: { attribute: #{path}, direction: #{dir} } }" do let(:instructions) do @@ -86,7 +86,7 @@ end ApplicationRecord::SORTABLE_TYPES.combination(2).each do |type_1, type_2| - [all_possible_sort_instruction_combinations_for(type_1, type_2).sample].each do |instructions| + all_possible_sort_instruction_combinations_for(type_1, type_2).each do |instructions| context instructions do let(:instructions) { instructions } @@ -94,7 +94,7 @@ end end - [all_possible_path_combinations_for(type_1, type_2).sample].each do |path_1, path_2| + all_possible_path_combinations_for(type_1, type_2).each do |path_1, path_2| context "{ 0: { attribute: #{path_1}, direction: asc }, 1: { attribute: #{path_2}, direction: desc } }" do let(:instructions) do { diff --git a/spec/support/filtering_helpers.rb b/spec/support/filtering_helpers.rb index 1f97572..375cd1d 100644 --- a/spec/support/filtering_helpers.rb +++ b/spec/support/filtering_helpers.rb @@ -2,30 +2,60 @@ module FilteringHelpers PREDICATE_OPERATORS = ActiveSet::Filtering::Constants::PREDICATES - INCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == false && - o[:behavior] == :inclusive - end.map(&:first) - EXCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == false && - o[:behavior] == :exclusive - end.map(&:first) - INCONCLUSIVE_UNARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == false && - o[:behavior] == :inconclusive - end.map(&:first) - INCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == true && - o[:behavior] == :inclusive - end.map(&:first) - EXCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == true && - o[:behavior] == :exclusive - end.map(&:first) - INCONCLUSIVE_BINARY_OPERATORS = PREDICATE_OPERATORS.select do |_, o| - o[:compound] == true && - o[:behavior] == :inconclusive - end.map(&:first) + + def inclusive_unary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :inclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end + + def exclusive_unary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :exclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end + + def inconclusive_unary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == false && + o[:behavior] == :inconclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end + + def inclusive_binary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :inclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end + + def exclusive_binary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :exclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end + + def inconclusive_binary_operators + operators_for_test = PREDICATE_OPERATORS.select do |_, o| + o[:compound] == true && + o[:behavior] == :inconclusive + end.map(&:first) + + relevant_paths_for_testing_given_environment(operators_for_test) + end def all_possible_scope_paths_for(type) %W[ @@ -38,6 +68,14 @@ def all_possible_scope_paths_for(type) end end + def all_possible_type_operator_paths_for(type, operator) + paths_for_test = %W[ + #{type}(#{operator}) + only.#{type}(#{operator}) + ] + relevant_paths_for_testing_given_environment(paths_for_test) + end + def filter_value_for(object:, path:) path = path.remove('_scope_method') path = path.remove('_collection_method') diff --git a/spec/support/path_helpers.rb b/spec/support/path_helpers.rb index 405e4f0..efd36cd 100644 --- a/spec/support/path_helpers.rb +++ b/spec/support/path_helpers.rb @@ -1,12 +1,20 @@ # frozen_string_literal: true module PathHelpers + def relevant_paths_for_testing_given_environment(paths_for_test) + if ENV['LOGICALLY_EXHAUSTIVE_REQUEST_SPECS'] == 'true' + paths_for_test + else + [paths_for_test.sample] + end + end + def all_possible_paths_for(type, options = {}) association = 'only' computed_field = "computed_#{type}" computed_association = "computed_#{association}" - [].tap do |paths| + paths_for_test = [].tap do |paths| paths << type paths << computed_field unless options[:computed_fields] == false unless options[:associations] == false @@ -18,13 +26,16 @@ def all_possible_paths_for(type, options = {}) paths << [computed_association, computed_field].join('.') unless options[:computed_fields] == false end end + + relevant_paths_for_testing_given_environment(paths_for_test) end def all_possible_path_combinations_for(type_1, type_2) - all_possible_paths_for(type_1) - .product( - all_possible_paths_for(type_2) - ) + paths_for_test = all_possible_paths_for(type_1) + .product( + all_possible_paths_for(type_2) + ) + relevant_paths_for_testing_given_environment(paths_for_test) end def value_for(object:, path:) diff --git a/spec/support/sorting_helpers.rb b/spec/support/sorting_helpers.rb index a422075..49c57af 100644 --- a/spec/support/sorting_helpers.rb +++ b/spec/support/sorting_helpers.rb @@ -2,19 +2,23 @@ module SortingHelpers def all_possible_sort_instructions_for(type) - all_possible_paths_for(type) + paths_for_test = all_possible_paths_for(type) .product([:asc, 'desc']) .map { |instruction_tuple| Hash[*instruction_tuple] } + + relevant_paths_for_testing_given_environment(paths_for_test) end def all_possible_sort_instruction_combinations_for(type_1, type_2) - all_possible_path_combinations_for(type_1, type_2) + paths_for_test = all_possible_path_combinations_for(type_1, type_2) .to_enum .with_index .map do |paths, index| directions = index.odd? ? ['asc', :desc] : [:desc, 'asc'] Hash[paths.zip(directions)] end + + relevant_paths_for_testing_given_environment(paths_for_test) end def sort_value_for(object:, path:) From 645cef87a726ce6ffc67c9021e8ceb1f175331ca Mon Sep 17 00:00:00 2001 From: fractaledmind Date: Fri, 9 Aug 2019 11:11:17 +0200 Subject: [PATCH 51/56] Ensure that the CI run rake task runs all possible tests --- Rakefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Rakefile b/Rakefile index c08f09e..7cc2370 100644 --- a/Rakefile +++ b/Rakefile @@ -8,6 +8,7 @@ RSpec::Core::RakeTask.new(:spec) task :full_spec do ENV['COVERAGE'] = 'true' ENV['INSPECT_FAILURE'] = 'true' + ENV['LOGICALLY_EXHAUSTIVE_REQUEST_SPECS'] = 'true' Rake::Task['spec'].invoke end From 9b877a1286fb9c25f925075962bd21d1a3b0cc31 Mon Sep 17 00:00:00 2001 From: Gregory Barendt Date: Fri, 9 Aug 2019 08:51:39 -0400 Subject: [PATCH 52/56] Use a DBMS-agnostic version of NULL sorting --- lib/active_set/sorting/active_record_strategy.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/active_set/sorting/active_record_strategy.rb b/lib/active_set/sorting/active_record_strategy.rb index 90aceff..db912db 100644 --- a/lib/active_set/sorting/active_record_strategy.rb +++ b/lib/active_set/sorting/active_record_strategy.rb @@ -65,13 +65,11 @@ def direction_operator(direction) end def nil_sorter_for(column, direction) - nil_sorter_operator = if ActiveSet.configuration.on_asc_sort_nils_come == :last - direction == :asc ? :eq : :not_eq - else - direction == :asc ? :not_eq : :eq - end - - column.send(nil_sorter_operator, nil) + if ActiveSet.configuration.on_asc_sort_nils_come == :last + "CASE WHEN #{column.relation.name}.#{column.name} IS NULL THEN 0 ELSE 1 END" + else + "CASE WHEN #{column.relation.name}.#{column.name} IS NULL THEN 1 ELSE 0 END" + end end end end From 9e7d9e49d52f15c0c15e2f7dd1a50f054efb685e Mon Sep 17 00:00:00 2001 From: Elika Molayi Date: Tue, 3 Mar 2020 17:17:28 -0500 Subject: [PATCH 53/56] Add `arel_column_name` method to ActiveRecordSetInstruction --- lib/active_set/active_record_set_instruction.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/active_set/active_record_set_instruction.rb b/lib/active_set/active_record_set_instruction.rb index ee4ab37..1d0be08 100644 --- a/lib/active_set/active_record_set_instruction.rb +++ b/lib/active_set/active_record_set_instruction.rb @@ -40,6 +40,10 @@ def arel_column end # rubocop:enable Lint/UnderscorePrefixedVariableName + def arel_column_name + arel_table[@attribute_instruction.attribute].name + end + def arel_operator @attribute_instruction.operator(default: :eq) end From 038ff6d3384419a27e57d9ac216d93bd8588178c Mon Sep 17 00:00:00 2001 From: Elika Molayi Date: Tue, 3 Mar 2020 17:18:06 -0500 Subject: [PATCH 54/56] Fix nil_sorter_for method to account for direction. --- .../sorting/active_record_strategy.rb | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/active_set/sorting/active_record_strategy.rb b/lib/active_set/sorting/active_record_strategy.rb index db912db..fa14be9 100644 --- a/lib/active_set/sorting/active_record_strategy.rb +++ b/lib/active_set/sorting/active_record_strategy.rb @@ -54,7 +54,9 @@ def order_operation_for(set_instruction) arel_column = set_instruction.arel_column arel_direction = direction_operator(set_instruction.value) - attribute_model.order(nil_sorter_for(arel_column, arel_direction)) + attribute_model.order(nil_sorter_for(set_instruction.arel_table, + set_instruction.arel_column_name, + arel_direction)) .order(arel_column.send(arel_direction)) end @@ -64,12 +66,16 @@ def direction_operator(direction) :asc end - def nil_sorter_for(column, direction) - if ActiveSet.configuration.on_asc_sort_nils_come == :last - "CASE WHEN #{column.relation.name}.#{column.name} IS NULL THEN 0 ELSE 1 END" - else - "CASE WHEN #{column.relation.name}.#{column.name} IS NULL THEN 1 ELSE 0 END" - end + def nil_sorter_for(model, column, direction) + first = 'THEN 0 ELSE 1 END' + last = 'THEN 1 ELSE 0 END' + then_statement = if ActiveSet.configuration.on_asc_sort_nils_come == :last + direction == :asc ? last : first + else + direction == :asc ? first : last + end + + "CASE WHEN #{model.table_name}.#{column} IS NULL #{then_statement}" end end end From 1fadd785082d4da14b23e4d54ded863ebd69551b Mon Sep 17 00:00:00 2001 From: Elika Molayi Date: Tue, 3 Mar 2020 17:18:17 -0500 Subject: [PATCH 55/56] Bump ruby version --- .ruby-version | 2 +- .travis.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ruby-version b/.ruby-version index 005119b..3f5987a 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.4.1 +2.4.9 diff --git a/.travis.yml b/.travis.yml index 37864eb..2671429 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ sudo: false language: ruby rvm: - - 2.4.1 + - 2.4.9 before_install: - gem install bundler -v 1.15.4 - export TZ=America/New_York From b424cb367e242cb5f5340cb1080c30af3b6e71ef Mon Sep 17 00:00:00 2001 From: Elika Molayi Date: Wed, 4 Mar 2020 18:57:13 -0500 Subject: [PATCH 56/56] Refactor sorting active_record_strategy methods --- .../sorting/active_record_strategy.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/active_set/sorting/active_record_strategy.rb b/lib/active_set/sorting/active_record_strategy.rb index fa14be9..d1d4ad7 100644 --- a/lib/active_set/sorting/active_record_strategy.rb +++ b/lib/active_set/sorting/active_record_strategy.rb @@ -67,15 +67,21 @@ def direction_operator(direction) end def nil_sorter_for(model, column, direction) + "CASE WHEN #{model.table_name}.#{column} IS NULL #{nil_sorter_then_statement(direction)}" + end + + def nil_sorter_then_statement(direction) first = 'THEN 0 ELSE 1 END' last = 'THEN 1 ELSE 0 END' - then_statement = if ActiveSet.configuration.on_asc_sort_nils_come == :last - direction == :asc ? last : first - else - direction == :asc ? first : last - end + if ActiveSet.configuration.on_asc_sort_nils_come == :last + return last if direction == :asc - "CASE WHEN #{model.table_name}.#{column} IS NULL #{then_statement}" + return first + else + return first if direction == :asc + + return last + end end end end