From aa512c4d0d15a2dc2b406606e9b50e544fe5b91c Mon Sep 17 00:00:00 2001 From: Mattias Pfeiffer Date: Tue, 4 Oct 2022 16:16:42 +0200 Subject: [PATCH 1/3] Refactor `Relation` and restructure condition matching --- README.md | 6 +- lib/active_hash.rb | 2 + lib/active_hash/base.rb | 46 +--------- lib/active_hash/condition.rb | 44 ++++++++++ lib/active_hash/conditions.rb | 21 +++++ lib/active_hash/relation.rb | 161 ++++++++++++++++++---------------- spec/active_hash/base_spec.rb | 58 +++++++++++- 7 files changed, 214 insertions(+), 124 deletions(-) create mode 100644 lib/active_hash/condition.rb create mode 100644 lib/active_hash/conditions.rb diff --git a/README.md b/README.md index abad3376..4908ad68 100644 --- a/README.md +++ b/README.md @@ -211,11 +211,11 @@ Country#name= # => sets the name The ActiveHash::Base.all method functions like an in-memory data store. You can save your records as ActiveHash::Relation object by using standard ActiveRecord create and save methods: ```ruby Country.all -=> # +=> # Country.create => #1}> Country.all -=> #1}>], @query_hash={}, @records_dirty=false> +=> #1}>], @conditions=[..], @records_dirty=false> country = Country.new => # country.new_record? @@ -225,7 +225,7 @@ country.save country.new_record? # => false Country.all -=> #1}>, #2}>], @query_hash={}, @records_dirty=false> +=> #1}>, #2}>], @conditions=[..], @records_dirty=false> ``` Notice that when adding records to the collection, it will auto-increment the id for you by default. If you use string ids, it will not auto-increment the id. Available methods are: ``` diff --git a/lib/active_hash.rb b/lib/active_hash.rb index eceb6041..44032b6e 100644 --- a/lib/active_hash.rb +++ b/lib/active_hash.rb @@ -13,6 +13,8 @@ require 'active_hash/base' require 'active_hash/relation' +require 'active_hash/condition' +require 'active_hash/conditions' require 'active_file/multiple_files' require 'active_file/hash_and_array_files' require 'active_file/base' diff --git a/lib/active_hash/base.rb b/lib/active_hash/base.rb index a48294df..597ba237 100644 --- a/lib/active_hash/base.rb +++ b/lib/active_hash/base.rb @@ -21,50 +21,8 @@ class FileTypeMismatchError < StandardError end class Base - class_attribute :_data, :dirty, :default_attributes, :scopes - class WhereChain - def initialize(scope) - @scope = scope - @records = @scope.all - end - - def not(options) - return @scope if options.blank? - - # use index if searching by id - if options.key?(:id) || options.key?("id") - ids = @scope.pluck(:id) - Array.wrap(options.delete(:id) || options.delete("id")) - candidates = ids.map { |id| @scope.find_by_id(id) }.compact - end - - filtered_records = (candidates || @records || []).reject do |record| - options.present? && match_options?(record, options) - end - - ActiveHash::Relation.new(@scope.klass, filtered_records, {}) - end - - def match_options?(record, options) - options.all? do |col, match| - if match.kind_of?(Array) - match.any? { |v| normalize(v) == normalize(record[col]) } - else - normalize(record[col]) == normalize(match) - end - end - end - - private :match_options? - - def normalize(v) - v.respond_to?(:to_sym) ? v.to_sym : v - end - - private :normalize - end - if Object.const_defined?(:ActiveModel) extend ActiveModel::Naming include ActiveModel::Conversion @@ -205,7 +163,9 @@ def create!(attributes = {}) end def all(options = {}) - ActiveHash::Relation.new(self, @records || [], options[:conditions] || {}) + relation = ActiveHash::Relation.new(self, @records || []) + relation = relation.where!(options[:conditions]) if options[:conditions] + relation end delegate :where, :find, :find_by, :find_by!, :find_by_id, :count, :pluck, :ids, :pick, :first, :last, :order, to: :all diff --git a/lib/active_hash/condition.rb b/lib/active_hash/condition.rb new file mode 100644 index 00000000..5665621e --- /dev/null +++ b/lib/active_hash/condition.rb @@ -0,0 +1,44 @@ +class ActiveHash::Relation::Condition + attr_reader :constraints, :inverted + + def initialize(constraints) + @constraints = constraints + @inverted = false + end + + def invert! + @inverted = !inverted + + self + end + + def matches?(record) + match = begin + return true unless constraints + + expectation_method = inverted ? :any? : :all? + + constraints.send(expectation_method) do |attribute, expected| + value = record.public_send(attribute) + + matches_value?(value, expected) + end + end + + inverted ? !match : match + end + + private + + def matches_value?(value, comparison) + return comparison.any? { |v| matches_value?(value, v) } if comparison.is_a?(Array) + return comparison.include?(value) if comparison.is_a?(Range) + return comparison.match?(value) if comparison.is_a?(Regexp) + + normalize(value) == normalize(comparison) + end + + def normalize(value) + value.respond_to?(:to_s) ? value.to_s : value + end +end \ No newline at end of file diff --git a/lib/active_hash/conditions.rb b/lib/active_hash/conditions.rb new file mode 100644 index 00000000..063c06ae --- /dev/null +++ b/lib/active_hash/conditions.rb @@ -0,0 +1,21 @@ +class ActiveHash::Relation::Conditions + attr_reader :conditions + + delegate_missing_to :conditions + + def initialize(conditions = []) + @conditions = conditions + end + + def matches?(record) + conditions.all? do |condition| + condition.matches?(record) + end + end + + def self.wrap(conditions) + return conditions if conditions.is_a?(self) + + new(conditions) + end +end \ No newline at end of file diff --git a/lib/active_hash/relation.rb b/lib/active_hash/relation.rb index 90a7b50f..f32274c2 100644 --- a/lib/active_hash/relation.rb +++ b/lib/active_hash/relation.rb @@ -7,24 +7,80 @@ class Relation delegate :empty?, :length, :first, :second, :third, :last, to: :records delegate :sample, to: :records - def initialize(klass, all_records, query_hash = nil) + attr_reader :conditions, :order_values, :klass, :all_records + + def initialize(klass, all_records, conditions = nil, order_values = nil) self.klass = klass self.all_records = all_records - self.query_hash = query_hash - self.records_dirty = false + self.conditions = Conditions.wrap(conditions || []) + self.order_values = order_values || [] + end + + def where(conditions_hash = :chain) + return WhereChain.new(self) if conditions_hash == :chain + + spawn.where!(conditions_hash) + end + + class WhereChain + attr_reader :relation + + def initialize(relation) + @relation = relation + end + + def not(conditions_hash) + relation.conditions << Condition.new(conditions_hash).invert! + relation + end + end + + def order(*options) + spawn.order!(*options) + end + + def reorder(*options) + spawn.reorder!(*options) + end + + def where!(conditions_hash, inverted = false) + self.conditions << Condition.new(conditions_hash) self end - def where(query_hash = :chain) - return ActiveHash::Base::WhereChain.new(self) if query_hash == :chain + def spawn + self.class.new(klass, all_records, conditions, order_values) + end + + def order!(*options) + check_if_method_has_arguments!(:order, options) + self.order_values += preprocess_order_args(options) + self + end + + def reorder!(*options) + check_if_method_has_arguments!(:order, options) + + self.order_values = preprocess_order_args(options) + @records = apply_order_values(records, order_values) - self.records_dirty = true unless query_hash.nil? || query_hash.keys.empty? - self.query_hash.merge!(query_hash || {}) + self + end + + def records + @records ||= begin + filtered_records = apply_conditions(all_records, conditions) + ordered_records = apply_order_values(filtered_records, order_values) # rubocop:disable Lint/UselessAssignment + end + end + + def reload + @records = nil # Reset records self end def all(options = {}) - if options.has_key?(:conditions) + if options.key?(:conditions) where(options[:conditions]) else where({}) @@ -58,10 +114,11 @@ def find(id = nil, *args, &block) end def find_by_id(id) - return where(id: id).first if query_hash.present? - index = klass.send(:record_index)[id.to_s] # TODO: Make index in Base publicly readable instead of using send? - index and records[index] + return unless index + + record = all_records[index] + record if conditions.matches?(record) end def count @@ -84,86 +141,32 @@ def pick(*column_names) pluck(*column_names).first end - def reload - @records = filter_all_records_by_query_hash - end - - def order(*options) - check_if_method_has_arguments!(:order, options) - relation = where({}) - return relation if options.blank? - - processed_args = preprocess_order_args(options) - candidates = relation.dup - - order_by_args!(candidates, processed_args) - - candidates - end - def to_ary records.dup end def method_missing(method_name, *args) - return super unless self.klass.scopes.key?(method_name) + return super unless klass.scopes.key?(method_name) - instance_exec(*args, &self.klass.scopes[method_name]) + instance_exec(*args, &klass.scopes[method_name]) end - attr_reader :query_hash, :klass, :all_records, :records_dirty - - private - - attr_writer :query_hash, :klass, :all_records, :records_dirty - - def records - if !defined?(@records) || @records.nil? || records_dirty - reload - else - @records - end + def respond_to_missing?(method_name, include_private = false) + klass.scopes.key?(method_name) || super end - def filter_all_records_by_query_hash - self.records_dirty = false - return all_records if query_hash.blank? - - # use index if searching by id - if query_hash.key?(:id) || query_hash.key?("id") - ids = (query_hash.delete(:id) || query_hash.delete("id")) - ids = range_to_array(ids) if ids.is_a?(Range) - candidates = Array.wrap(ids).map { |id| klass.find_by_id(id) }.compact - end + private - return candidates if query_hash.blank? + attr_writer :conditions, :order_values, :klass, :all_records - (candidates || all_records || []).select do |record| - match_options?(record, query_hash) - end - end + def apply_conditions(records, conditions) + return records if conditions.blank? - def match_options?(record, options) - options.all? do |col, match| - if match.kind_of?(Array) - match.any? { |v| normalize(v) == normalize(record[col]) } - else - normalize(match) === normalize(record[col]) - end + records.select do |record| + conditions.matches?(record) end end - def normalize(v) - v.respond_to?(:to_sym) ? v.to_sym : v - end - - def range_to_array(range) - return range.to_a unless range.end.nil? - - e = records.last[:id] - (range.begin..e).to_a - end - def check_if_method_has_arguments!(method_name, args) return unless args.blank? @@ -179,7 +182,9 @@ def preprocess_order_args(order_args) ary.map! { |e| e.split(/\W+/) }.reverse! end - def order_by_args!(candidates, args) + def apply_order_values(records, args) + ordered_records = records.dup + args.each do |arg| field, dir = if arg.is_a?(Hash) arg.to_a.flatten.map(&:to_sym) @@ -189,7 +194,7 @@ def order_by_args!(candidates, args) arg.to_sym end - candidates.sort! do |a, b| + ordered_records.sort! do |a, b| if dir.present? && dir.to_sym.upcase.equal?(:DESC) b[field] <=> a[field] else @@ -197,6 +202,8 @@ def order_by_args!(candidates, args) end end end + + ordered_records end end end diff --git a/spec/active_hash/base_spec.rb b/spec/active_hash/base_spec.rb index 4c6b5063..575c3290 100644 --- a/spec/active_hash/base_spec.rb +++ b/spec/active_hash/base_spec.rb @@ -205,6 +205,29 @@ class Region < ActiveHash::Base end end + describe ".reload" do + before do + Country.field :name + Country.field :language + Country.data = [ + {:id => 1, :name => "US", :language => 'English'}, + {:id => 2, :name => "Canada", :language => 'English'}, + {:id => 3, :name => "Mexico", :language => 'Spanish'} + ] + end + + it "it reloads cached records" do + countries = Country.where(language: 'Spanish') + expect(countries.count).to eq(1) + + Country.create(id: 4, name: 'Spain', language: 'Spanish') + + expect(countries.count).to eq(1) + countries.reload + expect(countries.count).to eq(2) + end + end + describe ".where" do before do Country.field :name @@ -221,7 +244,7 @@ class Region < ActiveHash::Base end it "returns WhereChain class if no conditions are provided" do - expect(Country.where.class).to eq(ActiveHash::Base::WhereChain) + expect(Country.where.class).to eq(ActiveHash::Relation::WhereChain) end it "returns all records when passed nil" do @@ -988,10 +1011,43 @@ class Region < ActiveHash::Base it "populates the data correctly in the order provided" do countries = Country.where(language: 'English').order(id: :desc) + expect(countries.count).to eq 2 expect(countries.first).to eq Country.find_by(name: "Canada") expect(countries.second).to eq Country.find_by(name: "US") end + + it "can be chained" do + countries = Country.order(language: :asc) + expect(countries.first).to eq Country.find_by(name: "US") + + countries = countries.order(name: :asc) + expect(countries.first).to eq Country.find_by(name: "Canada") + end + + it "doesn't change the order of original records" do + countries = Country.order(id: :desc) + + expect(countries.first).to eq Country.find_by(name: "Mexico") + expect(countries.second).to eq Country.find_by(name: "Canada") + expect(countries.third).to eq Country.find_by(name: "US") + + expect(countries.find(1)).to eq Country.find_by(name: "US") + + expect(Country.all.first).to eq Country.find_by(name: "US") + expect(Country.all.second).to eq Country.find_by(name: "Canada") + expect(Country.all.third).to eq Country.find_by(name: "Mexico") + end + end + + describe ".reorder" do + it "re-orders records" do + countries = Country.order(language: :asc) + expect(countries.first).to eq Country.find_by(name: "US") + + countries = countries.reorder(id: :desc) + expect(countries.first).to eq Country.find_by(name: "Mexico") + end end describe ".exists?" do From b35b81bf5ee37b19bf7bdc2a6a4e93ee94506af1 Mon Sep 17 00:00:00 2001 From: Mattias Pfeiffer Date: Thu, 6 Oct 2022 11:32:10 +0200 Subject: [PATCH 2/3] Add `invert_where` method as in AR 7 to invert all conditions --- lib/active_hash/conditions.rb | 2 +- lib/active_hash/relation.rb | 9 +++++++++ spec/active_hash/base_spec.rb | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/active_hash/conditions.rb b/lib/active_hash/conditions.rb index 063c06ae..ebbd755c 100644 --- a/lib/active_hash/conditions.rb +++ b/lib/active_hash/conditions.rb @@ -1,7 +1,7 @@ class ActiveHash::Relation::Conditions attr_reader :conditions - delegate_missing_to :conditions + delegate :<<, :map, to: :conditions def initialize(conditions = []) @conditions = conditions diff --git a/lib/active_hash/relation.rb b/lib/active_hash/relation.rb index f32274c2..3ad90f72 100644 --- a/lib/active_hash/relation.rb +++ b/lib/active_hash/relation.rb @@ -48,6 +48,15 @@ def where!(conditions_hash, inverted = false) self end + def invert_where + spawn.invert_where! + end + + def invert_where! + conditions.map(&:invert!) + self + end + def spawn self.class.new(klass, all_records, conditions, order_values) end diff --git a/spec/active_hash/base_spec.rb b/spec/active_hash/base_spec.rb index 575c3290..0de658b9 100644 --- a/spec/active_hash/base_spec.rb +++ b/spec/active_hash/base_spec.rb @@ -341,6 +341,22 @@ class Region < ActiveHash::Base end end + describe ".invert_where" do + before do + Country.field :name + Country.field :language + Country.data = [ + {:id => 1, :name => "US", :language => 'English'}, + {:id => 2, :name => "Canada", :language => 'English'}, + {:id => 3, :name => "Mexico", :language => 'Spanish'} + ] + end + + it "inverts all conditions" do + expect(Country.where(id: 1).where.not(id: 3).invert_where.map(&:name)).to match_array(%w(Mexico)) + end + end + describe ".where.not" do before do Country.field :name From fbbc057b30d253e73dd353db134f31b4b600a83a Mon Sep 17 00:00:00 2001 From: Mattias Pfeiffer Date: Thu, 6 Oct 2022 12:11:23 +0200 Subject: [PATCH 3/3] Use `#cover?` instead of `#include?` for Range --- lib/active_hash/condition.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_hash/condition.rb b/lib/active_hash/condition.rb index 5665621e..27fa3276 100644 --- a/lib/active_hash/condition.rb +++ b/lib/active_hash/condition.rb @@ -32,7 +32,7 @@ def matches?(record) def matches_value?(value, comparison) return comparison.any? { |v| matches_value?(value, v) } if comparison.is_a?(Array) - return comparison.include?(value) if comparison.is_a?(Range) + return comparison.cover?(value) if comparison.is_a?(Range) return comparison.match?(value) if comparison.is_a?(Regexp) normalize(value) == normalize(comparison)