From 6acb99d14c4fdb150d8cfba24cfd6b38dd691f33 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Sun, 26 Jan 2025 23:31:42 -0500 Subject: [PATCH] Introduce `ActiveResource::WhereClause` Closes [#408][] Introduce a simple chainable `WhereClause` class inspired by [Active Record][]. All methods (including those that integrate with [Enumerable][]) are delegated to `WhereClause#all`, which itself delegates to `Base.find`. By merging parameters through `.where`-chaining, this commit supports deferred fetching: ```ruby people = Person.where(id: 2).where(name: "david") # => GET /people.json?id=2&name=david people = Person.where(id: 2).all(params: { name: "david" }) # => GET /people.json?id=2&name=david people = Person.all(from: "/records.json").where(id: 2) # => GET /records.json?id=2 ``` [#408]: https://github.com/rails/activeresource/issues/408 [Active Record]: https://github.com/rails/rails/blob/main/activerecord/lib/active_record/relation/where_clause.rb [Enumerable]: https://ruby-doc.org/3.4.1/Enumerable.html --- lib/active_resource.rb | 1 + lib/active_resource/base.rb | 4 +- lib/active_resource/where_clause.rb | 48 +++++++++++++++++++++++ test/cases/finder_test.rb | 60 +++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 lib/active_resource/where_clause.rb diff --git a/lib/active_resource.rb b/lib/active_resource.rb index 8e4f34ecf4..b5a4003e9f 100644 --- a/lib/active_resource.rb +++ b/lib/active_resource.rb @@ -46,6 +46,7 @@ module ActiveResource autoload :InheritingHash autoload :Validations autoload :Collection + autoload :WhereClause if ActiveSupport::VERSION::STRING >= "7.1" def self.deprecator diff --git a/lib/active_resource/base.rb b/lib/active_resource/base.rb index 29376f081d..b0dca8f51b 100644 --- a/lib/active_resource/base.rb +++ b/lib/active_resource/base.rb @@ -1037,12 +1037,12 @@ def last(*args) # This is an alias for find(:all). You can pass in all the same # arguments to this method as you can to find(:all) def all(*args) - find(:all, *args) + WhereClause.new(self, *args) end def where(clauses = {}) raise ArgumentError, "expected a clauses Hash, got #{clauses.inspect}" unless clauses.is_a? Hash - find(:all, params: clauses) + all(params: clauses) end diff --git a/lib/active_resource/where_clause.rb b/lib/active_resource/where_clause.rb new file mode 100644 index 0000000000..6c61d2b535 --- /dev/null +++ b/lib/active_resource/where_clause.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module ActiveResource + class WhereClause < BasicObject + delegate :find, to: :@resource_class + delegate_missing_to :resources + + def initialize(resource_class, options = {}) + @resource_class = resource_class + @options = options + @resources = nil + @loaded = false + end + + def where(clauses = {}) + all(params: clauses) + end + + def all(options = {}) + WhereClause.new(@resource_class, @options.deep_merge(options)) + end + + def load + unless @loaded + @resources = find(:all, @options) + @loaded = true + end + + self + end + + def reload + reset + load + end + + private + def resources + load + @resources + end + + def reset + @resources = nil + @loaded = false + end + end +end diff --git a/test/cases/finder_test.rb b/test/cases/finder_test.rb index 5aea5496ab..275880917e 100644 --- a/test/cases/finder_test.rb +++ b/test/cases/finder_test.rb @@ -63,6 +63,66 @@ def test_where_with_clauses assert_kind_of StreetAddress, addresses.first end + def test_where_with_multiple_where_clauses + ActiveResource::HttpMock.respond_to.get "/people.json?id=2&name=david", {}, @people_david + + people = Person.where(id: 2).where(name: "david") + assert_equal 1, people.size + assert_kind_of Person, people.first + assert_equal 2, people.first.id + assert_equal "David", people.first.name + end + + def test_where_chained_from_all + ActiveResource::HttpMock.respond_to.get "/records.json?id=2", {}, @people_david + + people = Person.all(from: "/records.json").where(id: 2) + assert_equal 1, people.size + assert_kind_of Person, people.first + assert_equal 2, people.first.id + assert_equal "David", people.first.name + end + + def test_where_with_chained_into_all + ActiveResource::HttpMock.respond_to.get "/records.json?id=2&name=david", {}, @people_david + + people = Person.where(id: 2).all(from: "/records.json", params: { name: "david" }) + assert_equal 1, people.size + assert_kind_of Person, people.first + assert_equal 2, people.first.id + assert_equal "David", people.first.name + end + + def test_where_loading + ActiveResource::HttpMock.respond_to.get "/people.json?id=2", {}, @people_david + people = Person.where(id: 2) + + assert_changes -> { ActiveResource::HttpMock.requests.count }, from: 0, to: 1 do + people.load + end + assert_no_changes -> { ActiveResource::HttpMock.requests.count }, from: 1 do + 10.times { people.load } + end + end + + def test_where_reloading + ActiveResource::HttpMock.respond_to.get "/people.json?id=2", {}, @people_david + people = Person.where(id: 2) + + assert_changes -> { ActiveResource::HttpMock.requests.count }, from: 0, to: 1 do + assert_equal 1, people.size + end + assert_no_changes -> { ActiveResource::HttpMock.requests.count }, from: 1 do + assert_equal 1, people.size + end + assert_changes -> { ActiveResource::HttpMock.requests.count }, from: 1, to: 2 do + people.reload + end + assert_no_changes -> { ActiveResource::HttpMock.requests.count }, from: 2 do + assert_equal 1, people.size + end + end + def test_where_with_clause_in ActiveResource::HttpMock.respond_to { |m| m.get "/people.json?id%5B%5D=2", {}, @people_david } people = Person.where(id: [2])