Merge pull request #139 from joserafa11/add-eager-load-feature

Issue-138 Add Eager Load Option

Gemfile.lock

@@ -39,6 +39,8 @@ GEM
     method_source (1.0.0)
     mini_portile2 (2.8.4)
     minitest (5.19.0)
+    nokogiri (1.14.0-aarch64-linux)
+      racc (~> 1.4)
     nokogiri (1.15.4)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
@@ -67,6 +69,7 @@ GEM
     zeitwerk (2.6.11)
 
 PLATFORMS
+  aarch64-linux
   ruby
 
 DEPENDENCIES

README.md

@@ -161,6 +161,25 @@ GlobalID::Locator.locate_many gids
 
 Note the order is maintained in the returned results.
 
+### Options
+
+Either `GlobalID::Locator.locate` or `GlobalID::Locator.locate_many` supports a hash of options as second parameter. The supported options are:
+
+* :includes - A Symbol, Array, Hash or combination of them
+  The same structure you would pass into a `includes` method of Active Record.
+  See [Active Record eager loading associations](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations)
+  If present, `locate` or `locate_many` will eager load all the relationships specified here.
+  Note: It only works if all the gids models have that relationships.
+* :only - A class, module or Array of classes and/or modules that are
+  allowed to be located.  Passing one or more classes limits instances of returned
+  classes to those classes or their subclasses.  Passing one or more modules in limits
+  instances of returned classes to those including that module.  If no classes or
+  modules match, +nil+ is returned.
+* :ignore_missing (Only for `locate_many`) - By default, `locate_many` will call `#find` on the model to locate the
+  ids extracted from the GIDs. In Active Record (and other data stores following the same pattern),
+  `#find` will raise an exception if a named ID can't be found. When you set this option to true,
+  we will use `#where(id: ids)` instead, which does not raise on missing records.
+
 ### Custom App Locator
 
 A custom locator can be set for an app by calling `GlobalID::Locator.use` and providing an app locator to use for that app.
@@ -172,7 +191,7 @@ A custom locator can either be a block or a class.
 Using a block:
 
 ```ruby
-GlobalID::Locator.use :foo do |gid|
+GlobalID::Locator.use :foo do |gid, options|
   FooRemote.const_get(gid.model_name).find(gid.model_id)
 end
 ```
@@ -182,7 +201,7 @@ Using a class:
 ```ruby
 GlobalID::Locator.use :bar, BarLocator.new
 class BarLocator
-  def locate(gid)
+  def locate(gid, options = {})
     @search_client.search name: gid.model_name, id: gid.model_id
   end
 end

lib/global_id.rb

@@ -16,4 +16,8 @@ def self.eager_load!
     super
     require 'global_id/signed_global_id'
   end
+
+  def self.deprecator # :nodoc:
+    @deprecator ||= ActiveSupport::Deprecation.new("2.1", "GlobalID")
+  end
 end

lib/global_id/locator.rb

@@ -8,14 +8,27 @@ class << self
       # Takes either a GlobalID or a string that can be turned into a GlobalID
       #
       # Options:
+      # * <tt>:includes</tt> - A Symbol, Array, Hash or combination of them.
+      #   The same structure you would pass into a +includes+ method of Active Record.
+      #   If present, locate will load all the relationships specified here.
+      #   See https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations.
       # * <tt>:only</tt> - A class, module or Array of classes and/or modules that are
       #   allowed to be located.  Passing one or more classes limits instances of returned
       #   classes to those classes or their subclasses.  Passing one or more modules in limits
       #   instances of returned classes to those including that module.  If no classes or
       #   modules match, +nil+ is returned.
       def locate(gid, options = {})
-        if gid = GlobalID.parse(gid)
-          locator_for(gid).locate gid if find_allowed?(gid.model_class, options[:only])
+        gid = GlobalID.parse(gid)
+
+        return unless gid && find_allowed?(gid.model_class, options[:only])
+
+        locator = locator_for(gid)
+
+        if locator.method(:locate).arity == 1
+          GlobalID.deprecator.warn "It seems your locator is defining the `locate` method only with one argument. Please make sure your locator is receiving the options argument as well, like `locate(gid, options = {})`."
+          locator.locate(gid)
+        else
+          locator.locate(gid, options.except(:only))
         end
       end
 
@@ -30,6 +43,11 @@ def locate(gid, options = {})
       # per model class, but still interpolate the results to match the order in which the gids were passed.
       #
       # Options:
+      # * <tt>:includes</tt> - A Symbol, Array, Hash or combination of them
+      #   The same structure you would pass into a includes method of Active Record.
+      #   @see https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations
+      #   If present, locate_many will load all the relationships specified here.
+      #   Note: It only works if all the gids models have that relationships.
       # * <tt>:only</tt> - A class, module or Array of classes and/or modules that are
       #   allowed to be located.  Passing one or more classes limits instances of returned
       #   classes to those classes or their subclasses.  Passing one or more modules in limits
@@ -51,6 +69,10 @@ def locate_many(gids, options = {})
       # Takes either a SignedGlobalID or a string that can be turned into a SignedGlobalID
       #
       # Options:
+      # * <tt>:includes</tt> - A Symbol, Array, Hash or combination of them
+      #   The same structure you would pass into a includes method of Active Record.
+      #   @see https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations
+      #   If present, locate_signed will load all the relationships specified here.
       # * <tt>:only</tt> - A class, module or Array of classes and/or modules that are
       #   allowed to be located.  Passing one or more classes limits instances of returned
       #   classes to those classes or their subclasses.  Passing one or more modules in limits
@@ -68,6 +90,11 @@ def locate_signed(sgid, options = {})
       # the results to match the order in which the gids were passed.
       #
       # Options:
+      # * <tt>:includes</tt> - A Symbol, Array, Hash or combination of them
+      #   The same structure you would pass into a includes method of Active Record.
+      #   @see https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations
+      #   If present, locate_many_signed will load all the relationships specified here.
+      #   Note: It only works if all the gids models have that relationships.
       # * <tt>:only</tt> - A class, module or Array of classes and/or modules that are
       #   allowed to be located.  Passing one or more classes limits instances of returned
       #   classes to those classes or their subclasses.  Passing one or more modules in limits
@@ -84,7 +111,7 @@ def locate_many_signed(sgids, options = {})
       #
       # Using a block:
       #
-      #   GlobalID::Locator.use :foo do |gid|
+      #   GlobalID::Locator.use :foo do |gid, options|
       #     FooRemote.const_get(gid.model_name).find(gid.model_id)
       #   end
       #
@@ -93,7 +120,7 @@ def locate_many_signed(sgids, options = {})
       #   GlobalID::Locator.use :bar, BarLocator.new
       #
       #   class BarLocator
-      #     def locate(gid)
+      #     def locate(gid, options = {})
       #       @search_client.search name: gid.model_name, id: gid.model_id
       #     end
       #   end
@@ -127,9 +154,12 @@ def normalize_app(app)
       @locators = {}
 
       class BaseLocator
-        def locate(gid)
+        def locate(gid, options = {})
           return unless model_id_is_valid?(gid)
-          gid.model_class.find gid.model_id
+          model_class = gid.model_class
+          model_class = model_class.includes(options[:includes]) if options[:includes]
+
+          model_class.find gid.model_id
         end
 
         def locate_many(gids, options = {})
@@ -143,7 +173,7 @@ def locate_many(gids, options = {})
           records_by_model_name_and_id = {}
 
           ids_by_model.each do |model, ids|
-            records = find_records(model, ids, ignore_missing: options[:ignore_missing])
+            records = find_records(model, ids, ignore_missing: options[:ignore_missing], includes: options[:includes])
 
             records_by_id = records.index_by do |record|
               record.id.is_a?(Array) ? record.id.map(&:to_s) : record.id.to_s
@@ -157,6 +187,8 @@ def locate_many(gids, options = {})
 
         private
           def find_records(model_class, ids, options)
+            model_class = model_class.includes(options[:includes]) if options[:includes]
+
             if options[:ignore_missing]
               model_class.where(model_class.primary_key => ids)
             else
@@ -170,7 +202,7 @@ def model_id_is_valid?(gid)
       end
 
       class UnscopedLocator < BaseLocator
-        def locate(gid)
+        def locate(gid, options = {})
           unscoped(gid.model_class) { super }
         end
 
@@ -194,12 +226,12 @@ def initialize(block)
           @locator = block
         end
 
-        def locate(gid)
-          @locator.call(gid)
+        def locate(gid, options = {})
+          @locator.call(gid, options)
         end
 
         def locate_many(gids, options = {})
-          gids.map { |gid| locate(gid) }
+          gids.map { |gid| locate(gid, options) }
         end
       end
   end

lib/global_id/railtie.rb

@@ -42,6 +42,10 @@ class Railtie < Rails::Railtie # :nodoc:
         send :extend, GlobalID::FixtureSet
       end
     end
+
+    initializer "web_console.deprecator" do |app|
+      app.deprecators[:global_id] = GlobalID.deprecator if app.respond_to?(:deprecators)
+    end
   end
 end
 

test/cases/global_locator_test.rb

@@ -64,6 +64,23 @@ class GlobalLocatorTest < ActiveSupport::TestCase
     assert_equal @gid.model_id, found.id
   end
 
+  test 'by GID with eager loading' do
+    assert_equal Person::Child.new('1', Person.new('1')),
+      GlobalID::Locator.locate(
+        Person::Child.new('1', Person.new('1')).to_gid,
+        includes: :parent
+      )
+  end
+
+  test 'by GID trying to eager load an unexisting relationship' do
+    assert_raises StandardError do
+      GlobalID::Locator.locate(
+        Person::Child.new('1', Person.new('1')).to_gid,
+        includes: :some_non_existent_relationship
+      )
+    end
+  end
+
   test 'by many GIDs of one class' do
     assert_equal [ Person.new('1'), Person.new('2') ],
       GlobalID::Locator.locate_many([ Person.new('1').to_gid, Person.new('2').to_gid ])
@@ -91,6 +108,22 @@ class GlobalLocatorTest < ActiveSupport::TestCase
       GlobalID::Locator.locate_many([ Person.new('1').to_gid, Person::Child.new('1').to_gid, Person.new('2').to_gid ], only: Person::Child)
   end
 
+  test 'by many GIDs with eager loading' do
+    assert_equal [ Person::Child.new('1', Person.new('1')), Person::Child.new('2', Person.new('2')) ],
+      GlobalID::Locator.locate_many(
+        [ Person::Child.new('1', Person.new('1')).to_gid, Person::Child.new('2', Person.new('2')).to_gid ],
+        includes: :parent
+      )
+  end
+
+  test 'by many GIDs trying to eager load an unexisting relationship' do
+    assert_raises StandardError do
+      GlobalID::Locator.locate_many(
+        [ Person::Child.new('1', Person.new('1')).to_gid, Person::Child.new('2', Person.new('2')).to_gid ],
+        includes: :some_non_existent_relationship
+      )
+    end
+  end
 
   test 'by SGID' do
     found = GlobalID::Locator.locate_signed(@sgid)
@@ -236,7 +269,7 @@ class GlobalLocatorTest < ActiveSupport::TestCase
 
   test 'use locator with class' do
     class BarLocator
-      def locate(gid); :bar; end
+      def locate(gid, options = {}); :bar; end
       def locate_many(gids, options = {}); gids.map(&:model_id); end
     end
 
@@ -248,6 +281,22 @@ def locate_many(gids, options = {}); gids.map(&:model_id); end
     end
   end
 
+  test 'use locator with class and single argument' do
+    class DeprecatedBarLocator
+      def locate(gid); :deprecated; end
+      def locate_many(gids, options = {}); gids.map(&:model_id); end
+    end
+
+    GlobalID::Locator.use :deprecated, DeprecatedBarLocator.new
+
+    with_app 'deprecated' do
+      assert_deprecated(nil, GlobalID.deprecator) do
+        assert_equal :deprecated, GlobalID::Locator.locate('gid://deprecated/Person/1')
+      end
+      assert_equal ['1', '2'], GlobalID::Locator.locate_many(['gid://deprecated/Person/1', 'gid://deprecated/Person/2'])
+    end
+  end
+
   test 'app locator is case insensitive' do
     GlobalID::Locator.use :insensitive do |gid|
       :insensitive

test/models/person.rb

@@ -53,4 +53,45 @@ def self.find(*)
   end
 end
 
-class Person::Child < Person; end
+class Person::ChildWithParent < Person
+  def self.find(id_or_ids)
+    if id_or_ids.is_a? Array
+      ids = id_or_ids
+      ids.collect { |id| find(id) }
+    else
+      id = id_or_ids
+
+      if id == HARDCODED_ID_FOR_MISSING_PERSON
+        raise 'Person missing'
+      else
+        Person::Child.new(id, Person.new(id))
+      end
+    end
+  end
+
+  def self.where(conditions)
+    (conditions[:id] - [HARDCODED_ID_FOR_MISSING_PERSON]).collect do |id|
+      Person::Child.new(id, Person.new(id))
+    end
+  end
+end
+
+class Person::Child < Person
+  attr_accessor :parent
+
+  def initialize(id = 1, parent = nil)
+    @id = id
+    @parent = parent
+  end
+
+  def self.includes(relationships)
+    return Person::ChildWithParent if relationships == :parent
+    return self if relationships.nil?
+
+    raise StandardError, 'Relationship does not exist'
+  end
+
+  def ==(other)
+    other.is_a?(self.class) && id == other.try(:id) && parent == other.parent
+  end
+end