diff --git a/lib/global_id/uri/gid.rb b/lib/global_id/uri/gid.rb index ae729bd..f15bec0 100644 --- a/lib/global_id/uri/gid.rb +++ b/lib/global_id/uri/gid.rb @@ -36,14 +36,15 @@ class InvalidModelIdError < URI::InvalidComponentError; end COMPOSITE_MODEL_ID_MAX_SIZE = 20 COMPOSITE_MODEL_ID_DELIMITER = "/" - URI_PARSER = URI::RFC2396_Parser.new # :nodoc: + URI_PARSER = URI::RFC3986_Parser.new # :nodoc: class << self - # Validates +app+'s as URI hostnames containing only alphanumeric characters - # and hyphens. An ArgumentError is raised if +app+ is invalid. + # Validates +app+'s as URI hostnames containing only alphanumeric characters, + # hyphens and dashes. An ArgumentError is raised if +app+ is invalid. # # URI::GID.validate_app('bcx') # => 'bcx' # URI::GID.validate_app('foo-bar') # => 'foo-bar' + # URI::GID.validate_app('foo_bar') # => 'foo_bar' # # URI::GID.validate_app(nil) # => ArgumentError # URI::GID.validate_app('foo/bar') # => ArgumentError @@ -51,7 +52,7 @@ def validate_app(app) parse("gid://#{app}/Model/1").app rescue URI::Error raise ArgumentError, 'Invalid app name. ' \ - 'App names must be valid URI hostnames: alphanumeric and hyphen characters only.' + 'App names must be valid URI hostnames: alphanumeric, hyphen and underscore characters only.' end # Create a new URI::GID by parsing a gid string with argument check. @@ -64,7 +65,7 @@ def validate_app(app) # URI.parse('gid://bcx') # => URI::GID instance # URI::GID.parse('gid://bcx/') # => raises URI::InvalidComponentError def parse(uri) - generic_components = URI.split(uri) << URI_PARSER << true # RFC2396 parser, true arg_check + generic_components = URI.split(uri) << URI_PARSER << true # RFC3986 parser, true arg_check new(*generic_components) end diff --git a/test/cases/global_id_test.rb b/test/cases/global_id_test.rb index baf7bc1..7767db7 100644 --- a/test/cases/global_id_test.rb +++ b/test/cases/global_id_test.rb @@ -11,7 +11,7 @@ class GlobalIDTest < ActiveSupport::TestCase end assert_raises ArgumentError do - GlobalID.app = 'blog_app' + GlobalID.app = 'blog:app' end assert_raises ArgumentError do diff --git a/test/cases/global_locator_test.rb b/test/cases/global_locator_test.rb index 6775d2d..ee10a0a 100644 --- a/test/cases/global_locator_test.rb +++ b/test/cases/global_locator_test.rb @@ -319,12 +319,6 @@ def locate_many(gids, options = {}); gids.map(&:model_id); end end end - test 'locator name cannot have underscore' do - assert_raises ArgumentError do - GlobalID::Locator.use('under_score') { |gid| 'will never be found' } - end - end - test "by valid purpose returns right model" do instance = Person.new login_sgid = instance.to_signed_global_id(for: 'login') diff --git a/test/cases/uri_gid_test.rb b/test/cases/uri_gid_test.rb index 3c3b3fa..3c19277 100644 --- a/test/cases/uri_gid_test.rb +++ b/test/cases/uri_gid_test.rb @@ -4,7 +4,7 @@ class URI::GIDTest < ActiveSupport::TestCase setup do @gid_string = 'gid://bcx/Person/5' @gid = URI::GID.parse(@gid_string) - @cpk_gid_string = 'gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value' + @cpk_gid_string = 'gid://bcx/CompositePrimaryKeyModel/tenant_key-value/id-value' @cpk_gid = URI::GID.parse(@cpk_gid_string) end @@ -12,7 +12,7 @@ class URI::GIDTest < ActiveSupport::TestCase assert_equal @gid.app, 'bcx' assert_equal @gid.model_name, 'Person' assert_equal @gid.model_id, '5' - assert_equal ["tenant-key-value", "id-value"], @cpk_gid.model_id + assert_equal ["tenant_key-value", "id-value"], @cpk_gid.model_id end test 'parsed for non existing model class' do @@ -20,8 +20,8 @@ class URI::GIDTest < ActiveSupport::TestCase assert_equal("1", flat_id_gid.model_id) assert_equal("NonExistingModel", flat_id_gid.model_name) - composite_id_gid = URI::GID.parse("gid://bcx/NonExistingModel/tenant-key-value/id-value") - assert_equal(["tenant-key-value", "id-value"], composite_id_gid.model_id) + composite_id_gid = URI::GID.parse("gid://bcx/NonExistingModel/tenant_key-value/id-value") + assert_equal(["tenant_key-value", "id-value"], composite_id_gid.model_id) assert_equal("NonExistingModel", composite_id_gid.model_name) end @@ -35,7 +35,7 @@ class URI::GIDTest < ActiveSupport::TestCase end test 'create from a composite primary key model' do - model = CompositePrimaryKeyModel.new(id: ["tenant-key-value", "id-value"]) + model = CompositePrimaryKeyModel.new(id: ["tenant_key-value", "id-value"]) assert_equal @cpk_gid_string, URI::GID.create('bcx', model).to_s end @@ -50,19 +50,19 @@ class URI::GIDTest < ActiveSupport::TestCase end test 'build with a composite primary key' do - array = URI::GID.build(['bcx', 'CompositePrimaryKeyModel', ["tenant-key-value", "id-value"], nil]) + array = URI::GID.build(['bcx', 'CompositePrimaryKeyModel', ["tenant_key-value", "id-value"], nil]) assert array hash = URI::GID.build( app: 'bcx', model_name: 'CompositePrimaryKeyModel', - model_id: ["tenant-key-value", "id-value"], + model_id: ["tenant_key-value", "id-value"], params: nil ) assert hash assert_equal array, hash - assert_equal("gid://bcx/CompositePrimaryKeyModel/tenant-key-value/id-value", array.to_s) + assert_equal("gid://bcx/CompositePrimaryKeyModel/tenant_key-value/id-value", array.to_s) end test 'build with wrong ordered array creates a wrong ordered gid' do @@ -157,13 +157,17 @@ class URI::GIDAppValidationTest < ActiveSupport::TestCase end test 'apps containing non alphanumeric characters are invalid' do - assert_invalid_app 'foo&bar' assert_invalid_app 'foo:bar' - assert_invalid_app 'foo_bar' end - test 'app with hyphen is allowed' do + test 'app with hyphen and/or underscore is allowed' do assert_equal 'foo-bar', URI::GID.validate_app('foo-bar') + assert_equal 'foo_bar', URI::GID.validate_app('foo_bar') + assert_equal 'foo-bar_baz', URI::GID.validate_app('foo-bar_baz') + + # RFC3986 allows ampersands in hostnames + # see: https://github.com/ruby/uri/issues/141#issuecomment-2541219003 + assert_equal 'foo&bar', URI::GID.validate_app('foo&bar') end private