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
enriclluelles · Feb 5, 2018 · a7b8c1d · a7b8c1d
1 parent 764944d
commit a7b8c1d
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 48 deletions.
diff --git 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))
 
 ## 5.6.0 / 2018-02-05
 

diff --git 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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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)
 
             URI.escape translated_resource
           end

diff --git 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
@@ -508,23 +508,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
 
@@ -568,7 +565,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)
@@ -662,7 +658,6 @@ def setup
     @routes = ActionDispatch::Routing::RouteSet.new
 
     config_default_locale_settings 'es'
-    config_host_locales es: 'es'
 
     draw_routes do
       localized do
@@ -679,7 +674,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