Skip to content

Commit

Permalink
Added support for Locator.locate_many and Locator.locate_many_signed …
Browse files Browse the repository at this point in the history
…to efficiently fetch many models
  • Loading branch information
dhh committed Feb 5, 2015
1 parent 5671ed8 commit ab5f975
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
@@ -0,0 +1,3 @@
* Added support for Locator.locate_many and Locator.locate_many_signed to efficiently fetch many models.

*Tom Ward, DHH*

This comment has been minimized.

Copy link
@kaspth

kaspth Feb 8, 2015

Contributor

I thought we were switching to the releases feature on GitHub? @rafaelfranca @dhh

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 8, 2015

Author Member

This comment has been minimized.

Copy link
@kaspth

kaspth via email Feb 8, 2015

Contributor

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 8, 2015

Author Member

This comment has been minimized.

Copy link
@kaspth

kaspth Feb 8, 2015

Contributor

Cool beans. I'll update it tomorrow and remove the changelog file.

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 8, 2015

Author Member

This comment has been minimized.

Copy link
@kaspth

kaspth via email Feb 8, 2015

Contributor

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 8, 2015

Author Member

This comment has been minimized.

Copy link
@cristianbica

cristianbica via email Feb 8, 2015

Member

This comment has been minimized.

Copy link
@kaspth

kaspth Feb 8, 2015

Contributor

Yep, Christian that's what we will be doing 👍

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 8, 2015

Author Member
49 changes: 49 additions & 0 deletions lib/global_id/locator.rb
@@ -1,3 +1,6 @@
require 'active_support'
require 'active_support/core_ext/enumerable' # For Enumerable#index_by

class GlobalID
module Locator
class << self
Expand All @@ -15,6 +18,31 @@ def locate(gid, options = {})
end
end

# Takes an array of GlobalIDs or strings that can be turned into a GlobalIDs.
# The GlobalIDs are located using Model.find(:all, ids), so the models must respond to
# that finder signature.
#
# This approach will efficiently call only one #find per model class, but still interpolate
# the results to match the order in which the gids were passed.
#
# Options:
# * <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_many(gids, options = {})
if (allowed_gids = parse_allowed(gids, options[:only])).any?
models_and_ids = allowed_gids.collect { |gid| [ gid.model_name.constantize, gid.model_id ] }
ids_by_model = models_and_ids.group_by(&:first)
loaded_by_model = Hash[ids_by_model.map { |model, ids| [ model, model.find(:all, ids.map(&:last)).index_by(&:id) ] }]

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Feb 5, 2015

Member

find(:all) is not valid in Rails 4. It was deprecated in Rails 4.0 and removed in Rails 4.1. Use all.

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Feb 5, 2015

Member

Also, why not use order instead of index_by?

This comment has been minimized.

Copy link
@dhh

dhh Feb 5, 2015

Author Member

Gracias. Fixed to #all in the next commit.

I'm trying to rely as little on the full AR query interface to make it easier for non-AR stores to be GlobalID compliant.

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Feb 5, 2015

Member

Yeah, make sense.

This comment has been minimized.

Copy link
@dhh

dhh Feb 5, 2015

Author Member

Doh. #all doesn't take parameters, dude! :)

This comment has been minimized.

Copy link
@sgrif

sgrif Feb 5, 2015

find can take an array.

This comment has been minimized.

Copy link
@sgrif

sgrif Feb 5, 2015

Model.find([1, 2, 3])

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Feb 5, 2015

Member

lol. Yeah. I think just find with the array of ids would work. http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find

I think we don't know how to use Rails.

This comment has been minimized.

Copy link
@dhh

dhh via email Feb 5, 2015

Author Member

models_and_ids.collect { |(model, id)| loaded_by_model[model][id] }.compact
else
[]
end
end

# Takes either a SignedGlobalID or a string that can be turned into a SignedGlobalID
#
# Options:
Expand All @@ -27,6 +55,23 @@ def locate_signed(sgid, options = {})
SignedGlobalID.find sgid, options
end

# Takes an array of SignedGlobalIDs or strings that can be turned into a SignedGlobalIDs.
# The SignedGlobalIDs are located using Model.find(:all, ids), so the models must respond to
# that finder signature.
#
# This approach will efficiently call only one #find per model class, but still interpolate
# the results to match the order in which the gids were passed.
#
# Options:
# * <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_many_signed(sgids, options = {})
locate_many sgids.collect { |sgid| SignedGlobalID.parse(sgid) }, options
end

# Tie a locator to an app.
# Useful when different apps collaborate and reference each others' Global IDs.
#
Expand Down Expand Up @@ -64,6 +109,10 @@ def find_allowed?(model_class, only = nil)
only ? Array(only).any? { |c| model_class <= c } : true
end

def parse_allowed(gids, only = nil)
gids.collect { |gid| GlobalID.parse(gid) }.compact.select { |gid| find_allowed?(gid.model_class, only) }
end

def normalize_app(app)
app.to_s.downcase
end
Expand Down
32 changes: 32 additions & 0 deletions test/cases/global_locator_test.rb
Expand Up @@ -55,6 +55,22 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_equal @gid.model_id, found.id
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 ])
end

test 'by many GIDs of mixed classes' do
assert_equal [ Person.new('1'), Person::Child.new('1'), Person.new('2') ],
GlobalID::Locator.locate_many([ Person.new('1').to_gid, Person::Child.new('1').to_gid, Person.new('2').to_gid ])
end

test 'by many GIDs with only: restriction to match subclass' do
assert_equal [ Person::Child.new('1') ],
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 SGID' do
found = GlobalID::Locator.locate_signed(@sgid)
assert_kind_of @sgid.model_class, found
Expand Down Expand Up @@ -103,6 +119,22 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_equal @sgid.model_id, found.id
end

test 'by many SGIDs of one class' do
assert_equal [ Person.new('1'), Person.new('2') ],
GlobalID::Locator.locate_many_signed([ Person.new('1').to_sgid, Person.new('2').to_sgid ])
end

test 'by many SGIDs of mixed classes' do
assert_equal [ Person.new('1'), Person::Child.new('1'), Person.new('2') ],
GlobalID::Locator.locate_many_signed([ Person.new('1').to_sgid, Person::Child.new('1').to_sgid, Person.new('2').to_sgid ])
end

test 'by many SGIDs with only: restriction to match subclass' do
assert_equal [ Person::Child.new('1') ],
GlobalID::Locator.locate_many_signed([ Person.new('1').to_sgid, Person::Child.new('1').to_sgid, Person.new('2').to_sgid ], only: Person::Child)
end


test 'by GID string' do
found = GlobalID::Locator.locate(@gid.to_s)
assert_kind_of @gid.model_class, found
Expand Down
10 changes: 7 additions & 3 deletions test/models/person.rb
Expand Up @@ -3,16 +3,20 @@ class Person

attr_reader :id

def self.find(id)
new(id)
def self.find(*args)
if args.first == :all
args.from(1).flatten.collect { |id| new(id) }
else
new(args.first)
end
end

def initialize(id = 1)
@id = id
end

def ==(other)
id == other.try(:id)
other.is_a?(self.class) && id == other.try(:id)
end
end

Expand Down

2 comments on commit ab5f975

@fgermanojr
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged pending deploy

@kaspth
Copy link
Contributor

@kaspth kaspth commented on ab5f975 Feb 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're saying it's unreleased?

Please sign in to comment.