Skip to content

Commit

Permalink
Allow for composite identifiers delimited by /
Browse files Browse the repository at this point in the history
This commit extends global id to allow representing models with composite
identifiers. The value will be joined by `/`. For example:

Given a `TravelRoute` model with `origin` and `destination` as the
compsoite primary key, the global id will be represented as:

```
TravelRoute.new(origin: "Ottawa", destination: "New York").to_global_id
=> gid://app/TravelRoute/Ottawa/New%20York
```

Co-authored-by: Adrianna Chang <[email protected]>
Co-authored-by: Nikita Vasilevsky <[email protected]>
  • Loading branch information
adrianna-chang-shopify and nvasilevski committed Aug 28, 2023
1 parent 5c68d15 commit 5ce154c
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 22 deletions.
9 changes: 5 additions & 4 deletions lib/global_id/global_id.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ def find(options = {})
end

def model_class
model = model_name.constantize
@model_class ||= begin
model = model_name.constantize

unless model <= GlobalID
if model <= GlobalID
raise ArgumentError, "GlobalID and SignedGlobalID cannot be used as model_class."
end
model
else
raise ArgumentError, "GlobalID and SignedGlobalID cannot be used as model_class."
end
end

Expand Down
38 changes: 31 additions & 7 deletions lib/global_id/locator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

class GlobalID
module Locator
class InvalidModelIdError < StandardError; end

class << self
# Takes either a GlobalID or a string that can be turned into a GlobalID
#
Expand Down Expand Up @@ -126,27 +128,49 @@ def normalize_app(app)

class BaseLocator
def locate(gid)
return unless model_id_is_valid?(gid)
gid.model_class.find gid.model_id
end

def locate_many(gids, options = {})
models_and_ids = gids.collect { |gid| [ gid.model_class, gid.model_id ] }
ids_by_model = models_and_ids.group_by(&:first)
loaded_by_model = Hash[ids_by_model.map { |model, ids|
[ model, find_records(model, ids.map(&:last), ignore_missing: options[:ignore_missing]).index_by { |record| record.id.to_s } ]
}]
ids_by_model = Hash.new { |hash, key| hash[key] = [] }

gids.each do |gid|
next unless model_id_is_valid?(gid)
ids_by_model[gid.model_class] << gid.model_id
end

records_by_model_name_and_id = {}
ids_by_model.each do |model, ids|

records = find_records(model, ids, ignore_missing: options[:ignore_missing])

records_by_id = records.index_by do |record|
record.id.is_a?(Array) ? record.id.map(&:to_s) : record.id.to_s
end

records_by_model_name_and_id[model.name] = records_by_id
end

models_and_ids.collect { |(model, id)| loaded_by_model[model][id] }.compact
gids.filter_map { |gid| records_by_model_name_and_id[gid.model_name][gid.model_id] }
end

private
def find_records(model_class, ids, options)
if options[:ignore_missing]
model_class.where(id: ids)
model_class.where(model_class.primary_key => ids)
else
model_class.find(ids)
end
end

private
def model_id_is_valid?(gid)
primary_key = Array(gid.model_class.primary_key)
primary_key_size = primary_key.size

Array(gid.model_id).size == primary_key_size
end
end

class UnscopedLocator < BaseLocator
Expand Down
37 changes: 30 additions & 7 deletions lib/global_id/uri/gid.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ class GID < Generic

# Raised when creating a Global ID for a model without an id
class MissingModelIdError < URI::InvalidComponentError; end
class InvalidModelIdError < URI::InvalidComponentError; end

# Maximum size of a model id segment
COMPOSITE_MODEL_ID_MAX_SIZE = 20
COMPOSITE_MODEL_ID_DELIMITER = "/"

class << self
# Validates +app+'s as URI hostnames containing only alphanumeric characters
Expand Down Expand Up @@ -83,7 +88,8 @@ def create(app, model, params = nil)
def build(args)
parts = Util.make_components_hash(self, args)
parts[:host] = parts[:app]
parts[:path] = "/#{parts[:model_name]}/#{CGI.escape(parts[:model_id].to_s)}"
model_id_segment = Array(parts[:model_id]).map { |p| CGI.escape(p.to_s) }.join(COMPOSITE_MODEL_ID_DELIMITER)
parts[:path] = "/#{parts[:model_name]}/#{model_id_segment}"

if parts[:params] && !parts[:params].empty?
parts[:query] = URI.encode_www_form(parts[:params])
Expand Down Expand Up @@ -147,12 +153,22 @@ def check_scheme(scheme)

def set_model_components(path, validate = false)
_, model_name, model_id = path.split('/', 3)
validate_component(model_name) && validate_model_id(model_id, model_name) if validate

model_id = CGI.unescape(model_id) if model_id

validate_component(model_name) && validate_model_id_section(model_id, model_name) if validate
@model_name = model_name
@model_id = model_id

if model_id
model_id_parts = model_id
.split(COMPOSITE_MODEL_ID_DELIMITER, COMPOSITE_MODEL_ID_MAX_SIZE)
.reject(&:blank?)

model_id_parts.map! do |id|
validate_model_id(id)
CGI.unescape(id)
end

@model_id = model_id_parts.length == 1 ? model_id_parts.first : model_id_parts
end
end

def validate_component(component)
Expand All @@ -162,13 +178,20 @@ def validate_component(component)
"Expected a URI like gid://app/Person/1234: #{inspect}"
end

def validate_model_id(model_id, model_name)
return model_id unless model_id.blank? || model_id.include?('/')
def validate_model_id_section(model_id, model_name)
return model_id unless model_id.blank?

raise MissingModelIdError, "Unable to create a Global ID for " \
"#{model_name} without a model id."
end

def validate_model_id(model_id_part)
return unless model_id_part.include?('/')

raise InvalidModelIdError, "Unable to create a Global ID for " \
"#{model_name} with a malformed model id."
end

def parse_query_params(query)
Hash[URI.decode_www_form(query)].with_indifferent_access if query
end
Expand Down
18 changes: 17 additions & 1 deletion test/cases/global_id_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class GlobalIDTest < ActiveSupport::TestCase
test 'value equality' do
assert_equal GlobalID.new('gid://app/model/id'), GlobalID.new('gid://app/model/id')
assert_equal GlobalID.new('gid://app/Person/5'), GlobalID.new('gid://app/Person/5')
end

test 'invalid app name' do
Expand Down Expand Up @@ -44,26 +44,30 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
@person_uuid_gid = GlobalID.create(Person.new(@uuid))
@person_namespaced_gid = GlobalID.create(Person::Child.new(4))
@person_model_gid = GlobalID.create(PersonModel.new(id: 1))
@cpk_model_gid = GlobalID.create(CompositePrimaryKeyModel.new(id: ["tenant-key-value", "id-value"]))
end

test 'find' do
assert_equal Person.find(@person_gid.model_id), @person_gid.find
assert_equal Person.find(@person_uuid_gid.model_id), @person_uuid_gid.find
assert_equal Person::Child.find(@person_namespaced_gid.model_id), @person_namespaced_gid.find
assert_equal PersonModel.find(@person_model_gid.model_id), @person_model_gid.find
assert_equal CompositePrimaryKeyModel.find(@cpk_model_gid.model_id), @cpk_model_gid.find
end

test 'find with class' do
assert_equal Person.find(@person_gid.model_id), @person_gid.find(only: Person)
assert_equal Person.find(@person_uuid_gid.model_id), @person_uuid_gid.find(only: Person)
assert_equal PersonModel.find(@person_model_gid.model_id), @person_model_gid.find(only: PersonModel)
assert_equal CompositePrimaryKeyModel.find(@cpk_model_gid.model_id), @cpk_model_gid.find(only: CompositePrimaryKeyModel)
end

test 'find with class no match' do
assert_nil @person_gid.find(only: Hash)
assert_nil @person_uuid_gid.find(only: Array)
assert_nil @person_namespaced_gid.find(only: String)
assert_nil @person_model_gid.find(only: Float)
assert_nil @cpk_model_gid.find(only: Hash)
end

test 'find with subclass' do
Expand Down Expand Up @@ -135,6 +139,7 @@ class GlobalIDCreationTest < ActiveSupport::TestCase
assert_equal "gid://bcx/Person/#{@uuid}", @person_uuid_gid.to_s
assert_equal 'gid://bcx/Person::Child/4', @person_namespaced_gid.to_s
assert_equal 'gid://bcx/PersonModel/1', @person_model_gid.to_s
assert_equal 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value', @cpk_model_gid.to_s
end

test 'as param' do
Expand All @@ -149,13 +154,18 @@ class GlobalIDCreationTest < ActiveSupport::TestCase

assert_equal 'Z2lkOi8vYmN4L1BlcnNvbk1vZGVsLzE', @person_model_gid.to_param
assert_equal @person_model_gid, GlobalID.parse('Z2lkOi8vYmN4L1BlcnNvbk1vZGVsLzE')

expected_encoded = 'Z2lkOi8vYmN4L0NvbXBvc2l0ZVByaW1hcnlLZXlNb2RlbC90ZW5hbnQta2V5LXZhbHVlL2lkLXZhbHVl'
assert_equal expected_encoded, @cpk_model_gid.to_param
assert_equal @cpk_model_gid, GlobalID.parse(expected_encoded)
end

test 'as URI' do
assert_equal URI('gid://bcx/Person/5'), @person_gid.uri
assert_equal URI("gid://bcx/Person/#{@uuid}"), @person_uuid_gid.uri
assert_equal URI('gid://bcx/Person::Child/4'), @person_namespaced_gid.uri
assert_equal URI('gid://bcx/PersonModel/1'), @person_model_gid.uri
assert_equal URI('gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value'), @cpk_model_gid.uri
end

test 'as JSON' do
Expand All @@ -170,27 +180,33 @@ class GlobalIDCreationTest < ActiveSupport::TestCase

assert_equal 'gid://bcx/PersonModel/1', @person_model_gid.as_json
assert_equal '"gid://bcx/PersonModel/1"', @person_model_gid.to_json

assert_equal 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value', @cpk_model_gid.as_json
assert_equal '"gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value"', @cpk_model_gid.to_json
end

test 'model id' do
assert_equal '5', @person_gid.model_id
assert_equal @uuid, @person_uuid_gid.model_id
assert_equal '4', @person_namespaced_gid.model_id
assert_equal '1', @person_model_gid.model_id
assert_equal ['tenant-key-value', 'id-value'], @cpk_model_gid.model_id
end

test 'model name' do
assert_equal 'Person', @person_gid.model_name
assert_equal 'Person', @person_uuid_gid.model_name
assert_equal 'Person::Child', @person_namespaced_gid.model_name
assert_equal 'PersonModel', @person_model_gid.model_name
assert_equal 'CompositePrimaryKeyModel', @cpk_model_gid.model_name
end

test 'model class' do
assert_equal Person, @person_gid.model_class
assert_equal Person, @person_uuid_gid.model_class
assert_equal Person::Child, @person_namespaced_gid.model_class
assert_equal PersonModel, @person_model_gid.model_class
assert_equal CompositePrimaryKeyModel, @cpk_model_gid.model_class
assert_raise ArgumentError do
GlobalID.find 'gid://bcx/SignedGlobalID/5'
end
Expand Down
52 changes: 52 additions & 0 deletions test/cases/global_locator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ class GlobalLocatorTest < ActiveSupport::TestCase
model = Person.new('id')
@gid = model.to_gid
@sgid = model.to_sgid
@cpk_model = CompositePrimaryKeyModel.new(id: ["tenant-key-value", "id-value"])
@cpk_gid = @cpk_model.to_gid
@cpk_sgid = @cpk_model.to_sgid
end

test 'by GID' do
Expand All @@ -13,6 +16,12 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_equal @gid.model_id, found.id
end

test 'composite primary key model by GID' do
found = GlobalID::Locator.locate(@cpk_gid)
assert_kind_of @cpk_gid.model_class, found
assert_equal ["tenant-key-value", "id-value"], found.id
end

test 'by GID with only: restriction with match' do
found = GlobalID::Locator.locate(@gid, only: Person)
assert_kind_of @gid.model_class, found
Expand Down Expand Up @@ -60,6 +69,18 @@ class GlobalLocatorTest < ActiveSupport::TestCase
GlobalID::Locator.locate_many([ Person.new('1').to_gid, Person.new('2').to_gid ])
end

test '#locate_many by composite primary key GIDs of the same class' do
records = [ @cpk_model, CompositePrimaryKeyModel.new(id: ["tenant-key-value2", "id-value2"]) ]
located = GlobalID::Locator.locate_many(records.map(&:to_gid))
assert_equal records, located
end

test '#locate_many by composite primary key GIDs of different classes' do
records = [ @cpk_model, Person.new('1') ]
located = GlobalID::Locator.locate_many(records.map(&:to_gid))
assert_equal records, located
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 ])
Expand All @@ -77,6 +98,12 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_equal @sgid.model_id, found.id
end

test 'by SGID of a composite primary key model' do
found = GlobalID::Locator.locate_signed(@cpk_sgid)
assert_kind_of @cpk_sgid.model_class, found
assert_equal @cpk_sgid.model_id, found.id
end

test 'by SGID with only: restriction with match' do
found = GlobalID::Locator.locate_signed(@sgid, only: Person)
assert_kind_of @sgid.model_class, found
Expand Down Expand Up @@ -124,11 +151,23 @@ class GlobalLocatorTest < ActiveSupport::TestCase
GlobalID::Locator.locate_many_signed([ Person.new('1').to_sgid, Person.new('2').to_sgid ])
end

test 'by many SGIDs of the same composite primary key class' do
records = [ @cpk_model, CompositePrimaryKeyModel.new(id: ["tenant-key-value2", "id-value2"]) ]
located = GlobalID::Locator.locate_many_signed(records.map(&:to_sgid))
assert_equal records, located
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 of composite primary key model mixed with other models' do
records = [ @cpk_model, Person.new('1') ]
located = GlobalID::Locator.locate_many_signed(records.map(&:to_sgid))
assert_equal records, located
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)
Expand Down Expand Up @@ -157,6 +196,12 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_equal @gid.model_id, found.id
end

test 'by to_param encoding for a composite primary key model' do
found = GlobalID::Locator.locate(@cpk_gid.to_param)
assert_kind_of @cpk_gid.model_class, found
assert_equal @cpk_gid.model_id, found.id
end

test 'by non-GID returns nil' do
assert_nil GlobalID::Locator.locate 'This is not a GID'
end
Expand All @@ -172,6 +217,13 @@ class GlobalLocatorTest < ActiveSupport::TestCase
assert_nil GlobalID::Locator.locate 'gid://app/Person/1/2'
end

test 'locating by a GID URI with a mismatching model_id returns nil' do
assert_nil GlobalID::Locator.locate 'gid://app/Person/1/2'
assert_nil GlobalID::Locator.locate 'gid://app/CompositePrimaryKeyModel/tenant-key-value/id-value/something_else'
assert_nil GlobalID::Locator.locate 'gid://app/CompositePrimaryKeyModel/tenant-key-value/'
assert_nil GlobalID::Locator.locate 'gid://app/CompositePrimaryKeyModel/tenant-key-value'
end

test 'use locator with block' do
GlobalID::Locator.use :foo do |gid|
:foo
Expand Down
Loading

0 comments on commit 5ce154c

Please sign in to comment.