diff --git a/Gemfile.lock b/Gemfile.lock index cb2cfbb..e1cd4e5 100644 --- a/Gemfile.lock +++ b/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 diff --git a/README.md b/README.md index 550a6d6..27b6074 100644 --- a/README.md +++ b/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 diff --git a/lib/global_id.rb b/lib/global_id.rb index eeb013f..7692b90 100644 --- a/lib/global_id.rb +++ b/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 diff --git a/lib/global_id/locator.rb b/lib/global_id/locator.rb index f4dec07..ca9dfe6 100644 --- a/lib/global_id/locator.rb +++ b/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: + # * :includes - 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. # * :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. 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: + # * :includes - 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. # * :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 @@ -51,6 +69,10 @@ def locate_many(gids, options = {}) # Takes either a SignedGlobalID or a string that can be turned into a SignedGlobalID # # Options: + # * :includes - 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. # * :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 @@ -68,6 +90,11 @@ def locate_signed(sgid, options = {}) # the results to match the order in which the gids were passed. # # Options: + # * :includes - 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. # * :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 @@ -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 diff --git a/lib/global_id/railtie.rb b/lib/global_id/railtie.rb index f02e1fd..49e0dd3 100644 --- a/lib/global_id/railtie.rb +++ b/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 diff --git a/test/cases/global_locator_test.rb b/test/cases/global_locator_test.rb index ac3b8ea..955a57c 100644 --- a/test/cases/global_locator_test.rb +++ b/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 diff --git a/test/models/person.rb b/test/models/person.rb index ebf3d5e..1a4d546 100644 --- a/test/models/person.rb +++ b/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