Merge pull request #178 from nsommer/make-where-chainable

Make #where chainable

active_hash.gemspec

@@ -45,5 +45,6 @@ Gem::Specification.new do |s|
   ].flatten
   s.test_files = s.files.grep(%r{^(test|spec|features)/})
   s.add_runtime_dependency('activesupport', '>= 5.0.0')
+  s.add_development_dependency "pry"
   s.required_ruby_version = '>= 2.4.0'
 end

lib/active_hash.rb

@@ -12,6 +12,7 @@
 end
 
 require 'active_hash/base'
+require 'active_hash/relation'
 require 'active_file/multiple_files'
 require 'active_file/hash_and_array_files'
 require 'active_file/base'

lib/active_hash/base.rb

@@ -23,7 +23,7 @@ def initialize(scope)
       end
 
       def not(options)
-        return @records if options.blank?
+        return @scope if options.blank?
 
         # use index if searching by id
         if options.key?(:id) || options.key?("id")
@@ -32,9 +32,11 @@ def not(options)
         end
         return candidates if options.blank?
 
-        (candidates || @records || []).reject do |record|
+        filtered_records = (candidates || @records || []).reject do |record|
           match_options?(record, options)
         end
+
+        ActiveHash::Relation.new(@scope.klass, filtered_records, {})
       end
 
       def match_options?(record, options)
@@ -182,76 +184,11 @@ def create!(attributes = {})
         record
       end
 
-      def all(options={})
-        if options.has_key?(:conditions)
-          where(options[:conditions])
-        else
-          @records ||= []
-        end
-      end
-
-      def where(options = :chain)
-        if options == :chain
-          return WhereChain.new(self)
-        elsif options.blank?
-          return @records
-        end
-
-        # use index if searching by id
-        if options.key?(:id) || options.key?("id")
-          ids = (options.delete(:id) || options.delete("id"))
-          ids = range_to_array(ids) if ids.is_a?(Range)
-          candidates = Array.wrap(ids).map { |id| find_by_id(id) }.compact
-        end
-        return candidates if options.blank?
-
-        (candidates || @records || []).select do |record|
-          match_options?(record, options)
-        end
-      end
-
-      def find_by(options)
-        where(options).first
+      def all(options = {})
+        ActiveHash::Relation.new(self, @records || [], options[:conditions] || {})
       end
 
-      def find_by!(options)
-        find_by(options) || (raise RecordNotFound.new("Couldn't find #{name}"))
-      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
-
-      def range_to_array(range)
-        return range.to_a unless range.end.nil?
-
-        e = data.last[:id]
-        (range.begin..e).to_a
-      end
-
-      private :range_to_array
-
-      def count
-        all.length
-      end
-
-      def pluck(*column_names)
-        column_names.map { |column_name| all.map(&column_name.to_sym) }.inject(&:zip)
-      end
+      delegate :where, :find, :find_by, :find_by!, :find_by_id, :count, :pluck, :first, :last, to: :all
 
       def transaction
         yield
@@ -269,30 +206,6 @@ def delete_all
         @records = []
       end
 
-      def find(id, * args)
-        case id
-          when :all
-            all
-          when :first
-            all(*args).first
-          when Array
-            id.map { |i| find(i) }
-          when nil
-            raise RecordNotFound.new("Couldn't find #{name} without an ID")
-          else
-            find_by_id(id) || begin
-              raise RecordNotFound.new("Couldn't find #{name} with ID=#{id}")
-            end
-        end
-      end
-
-      def find_by_id(id)
-        index = record_index[id.to_s]
-        index and @records[index]
-      end
-
-      delegate :first, :last, :to => :all
-
       def fields(*args)
         options = args.extract_options!
         args.each do |field|

lib/active_hash/relation.rb

@@ -0,0 +1,128 @@
+module ActiveHash
+  class Relation
+    include Enumerable
+
+    delegate :each, to: :records # Make Enumerable work
+    delegate :equal?, :==, :===, :eql?, to: :records
+    delegate :empty?, :length, :first, :second, :third, :last, to: :records
+
+    def initialize(klass, all_records, query_hash = nil)
+      self.klass = klass
+      self.all_records = all_records
+      self.query_hash = query_hash
+      self.records_dirty = false
+      self
+    end
+
+    def where(query_hash = :chain)
+      return ActiveHash::Base::WhereChain.new(self) if query_hash == :chain
+
+      self.records_dirty = true unless query_hash.nil? || query_hash.keys.empty?
+      self.query_hash.merge!(query_hash || {})
+      self
+    end
+
+    def all(options = {})
+      if options.has_key?(:conditions)
+        where(options[:conditions])
+      else
+        where({})
+      end
+    end
+
+    def find_by(options)
+      where(options).first
+    end
+
+    def find_by!(options)
+      find_by(options) || (raise RecordNotFound.new("Couldn't find #{klass.name}"))
+    end
+
+    def find(id, *args)
+      case id
+        when :all
+          all
+        when :first
+          all(*args).first
+        when Array
+          id.map { |i| find(i) }
+        when nil
+          raise RecordNotFound.new("Couldn't find #{klass.name} without an ID")
+        else
+          find_by_id(id) || begin
+            raise RecordNotFound.new("Couldn't find #{klass.name} with ID=#{id}")
+          end
+      end
+    end
+
+    def find_by_id(id)
+      index = klass.send(:record_index)[id.to_s] # TODO: Make index in Base publicly readable instead of using send?
+      index and records[index]
+    end
+
+    def count
+      length
+    end
+
+    def pluck(*column_names)
+      column_names.map { |column_name| all.map(&column_name.to_sym) }.inject(&:zip)
+    end
+
+    def reload
+      @records = filter_all_records_by_query_hash
+    end
+
+    attr_reader :query_hash, :klass, :all_records, :records_dirty
+
+    private
+
+    attr_writer :query_hash, :klass, :all_records, :records_dirty
+
+    def records
+      if @records.nil? || records_dirty
+        reload
+      else
+        @records
+      end
+    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
+
+      return candidates if query_hash.blank?
+
+      (candidates || all_records || []).select do |record|
+        match_options?(record, query_hash)
+      end
+    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
+
+    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
+  end
+end

spec/active_hash/base_spec.rb

@@ -215,17 +215,21 @@ class Region < ActiveHash::Base
         {:id => 3, :name => "Mexico", :language => 'Spanish'}
       ]
     end
+
+    it 'returns a Relation class if conditions are provided' do
+      Country.where(language: 'English').class.should == ActiveHash::Relation
+    end
 
     it "returns WhereChain class if no conditions are provided" do
       Country.where.class.should == ActiveHash::Base::WhereChain
     end
 
     it "returns all records when passed nil" do
-      Country.where(nil).should == Country.all
+      Country.where(nil).to_a.should == Country.all.to_a
     end
 
     it "returns all records when an empty hash" do
-      Country.where({}).should == Country.all
+      Country.where({}).to_a.should == Country.all.to_a
     end
 
     it "returns all data as inflated objects" do
@@ -298,6 +302,16 @@ class Region < ActiveHash::Base
     it "filters records for multiple symbol values" do
       expect(Country.where(:name => [:US, :Canada]).map(&:name)).to match_array(%w(US Canada))
     end
+
+    it 'is chainable' do
+      where_relation = Country.where(language: 'English')
+
+      expect(where_relation.length).to eq 2
+      expect(where_relation.map(&:id)).to eq([1, 2])
+      chained_where_relation = where_relation.where(name: 'US')
+      expect(chained_where_relation.length).to eq 1
+      expect(chained_where_relation.map(&:id)).to eq([1])
+    end
   end
 
   describe ".where.not" do
@@ -316,13 +330,17 @@ class Region < ActiveHash::Base
         Country.where.not
       }.should raise_error(ArgumentError)
     end
+
+    it 'returns a chainable Relation when conditions are passed' do
+      Country.where.not(language: 'Spanish').class.should == ActiveHash::Relation
+    end
 
     it "returns all records when passed nil" do
-      Country.where.not(nil).should == Country.all
+      Country.where.not(nil).to_a.should == Country.all.to_a
     end
 
     it "returns all records when an empty hash" do
-      Country.where.not({}).should == Country.all
+      Country.where.not({}).to_a.should == Country.all.to_a
     end
 
     it "returns all records as inflated objects" do
@@ -366,7 +384,7 @@ class Region < ActiveHash::Base
     end
 
     it "returns all records when id is nil" do
-      expect(Country.where.not(:id => nil)).to eq Country.all
+      expect(Country.where.not(:id => nil).to_a).to eq Country.all.to_a
     end
 
     it "filters records for multiple ids" do
@@ -382,7 +400,7 @@ class Region < ActiveHash::Base
     end
 
     it "filters records for multiple conditions" do
-      expect(Country.where.not(:id => 1, :name => 'Mexico')).to match_array([Country.find(2)])
+      expect(Country.where.not(:id => 1, :name => 'Mexico').to_a).to match_array([Country.find(2)])
     end
   end
 
@@ -1357,7 +1375,7 @@ class Book < ActiveRecord::Base
       end
 
       it 'should return the query used to define the scope' do
-        expect(Country.english_language).to eq Country.where(language: 'English')
+        expect(Country.english_language.to_a).to eq Country.where(language: 'English').to_a
       end
 
       it 'should behave like the query used to define the scope' do
@@ -1384,7 +1402,7 @@ class Book < ActiveRecord::Base
       end
 
       it 'should return the query used to define the scope' do
-        expect(Country.with_language('English')).to eq Country.where(language: 'English')
+        expect(Country.with_language('English').to_a).to eq Country.where(language: 'English').to_a
       end
 
       it 'should behave like the query used to define the scope' do

spec/spec_helper.rb

@@ -1,3 +1,5 @@
+require "bundler/setup"
+require "pry"
 require 'rspec'
 require 'rspec/autorun'
 require 'yaml'