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

@@ -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))
 
 ## 8.0.0 / 2020-04-17
 

README.md

@@ -296,13 +296,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.

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
 

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

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

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?

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

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'

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