From 95208fc015d3daf284adb677e0d6af1ef4f2472c Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Sat, 1 Jul 2017 18:24:52 +0200 Subject: [PATCH] Avoid duplicate routes when using host_locales The purpose of this refactor is to avoid duplicate routes. Previously, route_translator allowed duplicate routes `/cars` and `/en/cars` under the `en` domain when `host_locales` was set. Fix: #87 --- CHANGELOG.md | 1 + README.md | 6 +++--- lib/route_translator.rb | 3 +-- lib/route_translator/host.rb | 12 +---------- lib/route_translator/locale_sanitizer.rb | 11 ---------- lib/route_translator/translator.rb | 5 +---- lib/route_translator/translator/path.rb | 3 +-- .../translator/path/segment.rb | 3 +-- test/integration/host_locales_test.rb | 12 +++++++++++ test/routing_test.rb | 21 +++++++------------ 10 files changed, 29 insertions(+), 48 deletions(-) delete mode 100644 lib/route_translator/locale_sanitizer.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index dbe7a33e..3dcee551 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * [BUGFIX] Verify host path consistency by default ([#91](https://github.com/enriclluelles/route_translator/issues/91), [#171](https://github.com/enriclluelles/route_translator/issues/171)) * [FEATURE] Remove the option to verify host path consistency +* [ENHANCEMENT] Avoid duplicate routes when using host_locales (#87) * [ENHANCEMENT] Update development dependencies ## 5.5.1 / 2017-11-14 diff --git a/README.md b/README.md index 2d6bef78..ac5bb008 100644 --- a/README.md +++ b/README.md @@ -314,13 +314,13 @@ RouteTranslator.config.host_locales = { 'russia.*' => :ru, '*.com' => :en } # RouteTranslator.config.host_locales = { '*.com' => :en, 'russia.*' => :ru } # 'russia.com' will have locale :en ``` -If `host_locales` option is set, the following options will be forced (even if you set to true): +If `host_locales` option is set, the following options will be forced: ```ruby +@config.hide_locale = true +@config.force_locale = false @config.generate_unlocalized_routes = false @config.generate_unnamed_unlocalized_routes = false -@config.force_locale = false -@config.hide_locale = false ``` This is to avoid odd behaviour brought about by route conflicts and because `host_locales` forces and hides the host-locale dynamically. diff --git a/lib/route_translator.rb b/lib/route_translator.rb index 4112aa51..733215aa 100644 --- a/lib/route_translator.rb +++ b/lib/route_translator.rb @@ -5,7 +5,6 @@ require 'route_translator/extensions' require 'route_translator/translator' require 'route_translator/host' -require 'route_translator/locale_sanitizer' module RouteTranslator extend RouteTranslator::Host @@ -22,7 +21,7 @@ class << self def resolve_host_locale_config_conflicts @config.force_locale = false - @config.hide_locale = false + @config.hide_locale = true @config.generate_unlocalized_routes = false @config.generate_unnamed_unlocalized_routes = false end diff --git a/lib/route_translator/host.rb b/lib/route_translator/host.rb index 81051c86..b85a6986 100644 --- a/lib/route_translator/host.rb +++ b/lib/route_translator/host.rb @@ -15,14 +15,6 @@ def regex_for(host_string) end end - def native_locale?(locale) - locale.to_s.match(/native_/).present? - end - - def native_locales - config.host_locales.values.map { |locale| :"native_#{locale}" } - end - module_function def locale_from_host(host) @@ -34,9 +26,7 @@ def locale_from_host(host) end def lambdas_for_locale(locale) - sanitized_locale = RouteTranslator::LocaleSanitizer.sanitize(locale) - - lambdas[sanitized_locale] ||= ->(req) { sanitized_locale == RouteTranslator::Host.locale_from_host(req.host).to_s } + lambdas[locale] ||= ->(req) { locale == RouteTranslator::Host.locale_from_host(req.host) } end end end diff --git a/lib/route_translator/locale_sanitizer.rb b/lib/route_translator/locale_sanitizer.rb deleted file mode 100644 index 92122690..00000000 --- a/lib/route_translator/locale_sanitizer.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -module RouteTranslator - module LocaleSanitizer - module_function - - def sanitize(locale) - locale.to_s.gsub('native_', '') - end - end -end diff --git a/lib/route_translator/translator.rb b/lib/route_translator/translator.rb index 193edb73..8f19049e 100644 --- a/lib/route_translator/translator.rb +++ b/lib/route_translator/translator.rb @@ -25,7 +25,7 @@ def translate_options(options, locale) translated_options = options.dup if translated_options.exclude?(RouteTranslator.locale_param_key) - translated_options[RouteTranslator.locale_param_key] = RouteTranslator::LocaleSanitizer.sanitize(locale) + translated_options[RouteTranslator.locale_param_key] = locale.to_s end translated_options @@ -52,7 +52,6 @@ def translate_path(path, locale, scope) def available_locales locales = RouteTranslator.available_locales - locales.concat(RouteTranslator.native_locales) if RouteTranslator.native_locales.present? # Make sure the default locale is translated in last place to avoid # problems with wildcards when default locale is omitted in paths. The # default routes will catch all paths like wildcard if it is translated first. @@ -81,8 +80,6 @@ def route_name_for(args, old_name, suffix, kaller) locale = if args_locale args_locale.to_s.underscore - elsif kaller.respond_to?("#{old_name}_native_#{current_locale_name}_#{suffix}") - "native_#{current_locale_name}" elsif kaller.respond_to?("#{old_name}_#{current_locale_name}_#{suffix}") current_locale_name else diff --git a/lib/route_translator/translator/path.rb b/lib/route_translator/translator/path.rb index 11a94e51..7c3778d6 100644 --- a/lib/route_translator/translator/path.rb +++ b/lib/route_translator/translator/path.rb @@ -9,8 +9,7 @@ class << self private def display_locale?(locale) - !RouteTranslator.config.hide_locale && !RouteTranslator.native_locale?(locale) && - (!default_locale?(locale) || config_requires_locale?) + !RouteTranslator.config.hide_locale && (!default_locale?(locale) || config_requires_locale?) end def config_requires_locale? diff --git a/lib/route_translator/translator/path/segment.rb b/lib/route_translator/translator/path/segment.rb index 9c9b3eec..d989126d 100644 --- a/lib/route_translator/translator/path/segment.rb +++ b/lib/route_translator/translator/path/segment.rb @@ -32,8 +32,7 @@ def translate_resource(str, locale, scope) end def translate_string(str, locale, scope) - sanitized_locale = RouteTranslator::LocaleSanitizer.sanitize(locale) - translated_resource = translate_resource(str, sanitized_locale, scope) + translated_resource = translate_resource(str, locale.to_s, scope) CGI.escape translated_resource end diff --git a/test/integration/host_locales_test.rb b/test/integration/host_locales_test.rb index 382b64d5..aec04174 100644 --- a/test/integration/host_locales_test.rb +++ b/test/integration/host_locales_test.rb @@ -64,6 +64,18 @@ def test_preserve_i18n_locale assert_equal :en, I18n.locale end + def test_prefixed_path + # prefixed es route on es com + host! 'www.testapp.es' + get '/es/native' + assert_response :not_found + + # prefixed ru route on ru com + host! 'ru.testapp.com' + get '/ru/native' + assert_response :not_found + end + def test_non_native_path # ru route on es com host! 'www.testapp.es' diff --git a/test/routing_test.rb b/test/routing_test.rb index 5b740c81..72d537cf 100644 --- a/test/routing_test.rb +++ b/test/routing_test.rb @@ -498,23 +498,20 @@ def test_blank_localized_routes end end - def test_path_helper_arguments + def test_path_helper_arguments_with_host_locales config_default_locale_settings 'es' - I18n.with_locale :es do - config_host_locales '*.es' => 'es', '*.com' => 'en' + config_host_locales '*.es' => 'es', '*.com' => 'en' - draw_routes do - localized do - resources :products - end + draw_routes do + localized do + resources :products end + end + I18n.with_locale :es do assert_equal '/productos', @routes.url_helpers.products_path assert_equal '/productos/some_product', @routes.url_helpers.product_path('some_product') assert_equal '/productos/some_product?some=param', @routes.url_helpers.product_path('some_product', some: 'param') - assert_equal '/en/products', @routes.url_helpers.products_path(locale: 'en') - assert_equal '/en/products/some_product', @routes.url_helpers.product_path('some_product', locale: 'en') - assert_equal '/en/products/some_product?some=param', @routes.url_helpers.product_path('some_product', locale: 'en', some: 'param') end end @@ -558,7 +555,6 @@ def test_host_locales end assert_recognizes({ controller: 'people', action: 'index', locale: 'es' }, path: 'http://testapp.es/gente', method: :get) - assert_recognizes({ controller: 'people', action: 'index', locale: 'es' }, path: 'http://testapp.es/es/gente', method: :get) assert_recognizes({ controller: 'people', action: 'index' }, path: 'http://testapp.es/', method: :get) assert_recognizes({ controller: 'people', action: 'index', locale: 'en' }, path: 'http://testapp.com/people', method: :get) @@ -652,7 +648,6 @@ def setup @routes = ActionDispatch::Routing::RouteSet.new config_default_locale_settings 'es' - config_host_locales es: 'es' draw_routes do localized do @@ -669,7 +664,7 @@ def teardown def test_url_helpers_are_included controller = ProductsController.new - %w[product_path product_url product_es_path product_es_url product_native_es_path product_native_es_url].each do |method| + %w[product_path product_url product_es_path product_es_url].each do |method| assert controller.respond_to? method end end