search

ruby / uri

URI is a module providing classes to handle Uniform Resource Identifiers
https://ruby.github.io/uri/
Other
79 stars 42 forks source link

Add proper Ractor support to URI #26

Closed eregon closed 3 years ago

eregon commented 3 years ago

This avoids the drawbacks/hacks of both #22 and #15.

It uses a module to map scheme names to scheme classes, which also works with Ractor.

hsbt commented 3 years ago

Can you pick https://github.com/ruby/uri/pull/25 with this pull-request?

eregon commented 3 years ago

@hsbt Done!

eregon commented 3 years ago

@hsbt OK to merge this? CI is green and it fixes the bug found in #25 + the other advantages.

hsbt commented 3 years ago

👍 Thanks!

byroot commented 3 years ago

This seem to break support of schemes that aren't valid ruby constants, e.g:

Error:
ActiveRecord::ConnectionAdapters::MergeAndResolveDefaultUrlConfigTest#test_url_with_hyphenated_scheme:
NameError: wrong constant name IBM-DB

    if !uri_class && !const_name.empty? && Schemes.const_defined?(const_name, false)
                                                  ^^^^^^^^^^^^^^^
    /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `const_defined?'
    /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `for'
    /usr/local/lib/ruby/3.1.0/uri/rfc2396_parser.rb:210:in `parse'
    /rails/activerecord/lib/active_record/database_configurations/connection_url_resolver.rb:25:in `initialize'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `new'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `build_url_hash'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:38:in `initialize'
    /rails/activerecord/lib/active_record/database_configurations.rb:262:in `new'
    /rails/activerecord/lib/active_record/database_configurations.rb:262:in `environment_url_config'
    /rails/activerecord/lib/active_record/database_configurations.rb:253:in `block in merge_db_environment_variables'
    /rails/activerecord/lib/active_record/database_configurations.rb:250:in `map'
    /rails/activerecord/lib/active_record/database_configurations.rb:250:in `merge_db_environment_variables'
    /rails/activerecord/lib/active_record/database_configurations.rb:180:in `build_configs'
    /rails/activerecord/lib/active_record/database_configurations.rb:19:in `initialize'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `new'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `resolve_db_config'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:157:in `test_url_with_hyphenated_scheme'

rails test rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:154

https://buildkite.com/rails/rails/builds/79689#6a1b7cfc-fa17-4752-bc27-62158a228e79/1018-1029

byroot commented 3 years ago

-, . and + are symbols that can be found in valid registered schemes: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml