From 0aec9e4a8f285869d51b04075e3fcd0009666b98 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. Fixes #87 and fixes #171 --- CHANGELOG.md | 1 + README.md | 4 ++-- 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 +-- lib/route_translator/translator/path/segment.rb | 3 +-- test/integration/host_locales_test.rb | 12 ++++++++++++ test/routing_test.rb | 8 ++------ 10 files changed, 22 insertions(+), 40 deletions(-) delete mode 100644 lib/route_translator/locale_sanitizer.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index cf23d7bb..e9127865 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](https://github.com/enriclluelles/route_translator/issues/87), [#171](https://github.com/enriclluelles/route_translator/issues/171)) ## 9.0.0 / 2020-11-07 diff --git a/README.md b/README.md index e6cb4059..1ea19dec 100644 --- a/README.md +++ b/README.md @@ -312,13 +312,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.force_locale = false @config.generate_unlocalized_routes = false @config.generate_unnamed_unlocalized_routes = false -@config.hide_locale = false +@config.hide_locale = true ``` 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 cc9c7dbd..7fd008d4 100644 --- a/lib/route_translator.rb +++ b/lib/route_translator.rb @@ -6,7 +6,6 @@ require 'route_translator/extensions' require 'route_translator/translator' require 'route_translator/host' -require 'route_translator/locale_sanitizer' module RouteTranslator extend RouteTranslator::Host @@ -34,7 +33,7 @@ def resolve_host_locale_config_conflicts @config.force_locale = false @config.generate_unlocalized_routes = false @config.generate_unnamed_unlocalized_routes = false - @config.hide_locale = false + @config.hide_locale = true end end diff --git a/lib/route_translator/host.rb b/lib/route_translator/host.rb index 2393991f..a029f8df 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.include?('native_') - 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 fccbb6b0..e6820e9d 100644 --- a/lib/route_translator/translator.rb +++ b/lib/route_translator/translator.rb @@ -26,7 +26,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 @@ -53,7 +53,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. @@ -82,8 +81,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 33304cb8..c9b5df55 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) Addressable::URI.normalize_component translated_resource end diff --git a/test/integration/host_locales_test.rb b/test/integration/host_locales_test.rb index e81347b7..6ee41139 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 d1812d14..90fdeeb6 100644 --- a/test/routing_test.rb +++ b/test/routing_test.rb @@ -503,7 +503,7 @@ def test_blank_localized_routes end end - def test_path_helper_arguments + def test_path_helper_arguments_with_host_locales I18n.default_locale = :es config_host_locales '*.es' => 'es', '*.com' => 'en' @@ -517,9 +517,6 @@ def test_path_helper_arguments 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 @@ -579,7 +576,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) @@ -703,7 +699,7 @@ def teardown def test_url_helpers_are_included controller = ProductsController.new - %i[product_path product_url product_es_path product_es_url product_native_es_path product_native_es_url].each do |method_name| + %i[product_path product_url product_es_path product_es_url].each do |method_name| assert_respond_to controller, method_name end end