UrlHelpers uses a consistent type for base_url

There are subtle differences in behavior depending on whether it is a
String or a URI. This patch forces it to be a URI and uses Prefix to
construct absolute URLs.

Fixes hanami#249

lib/hanami/router.rb

@@ -147,7 +147,7 @@ def call(env)
     #   end
     #
     #   router.path(:root) # => "/"
-    #   router.url(:root)  # => "https://hanamirb.org"
+    #   router.url(:root)  # => #<URI::HTTPS https://hanamirb.org>
     def root(to: nil, &blk)
       get(ROOT_PATH, to: to, as: :root, &blk)
     end
@@ -191,7 +191,7 @@ def root(to: nil, &blk)
     #   end
     #
     #   router.path(:welcome) # => "/"
-    #   router.url(:welcome)  # => "http://localhost/"
+    #   router.url(:welcome)  # => #<URI::HTTP http://localhost/>
     #
     # @example Constraints
     #   require "hanami/router"
@@ -466,7 +466,7 @@ def path(name, variables = {})
     #
     # @param name [Symbol] the route name
     #
-    # @return [String]
+    # @return [URI::HTTP, URI::HTTPS]
     #
     # @raise [Hanami::Router::MissingRouteError] when the router fails to
     #   recognize a route, because of the given arguments.
@@ -483,9 +483,9 @@ def path(name, variables = {})
     #     get "/:name", to: ->(*) { ... }, as: :framework
     #   end
     #
-    #   router.url(:login)                          # => "https://hanamirb.org/login"
-    #   router.url(:login, return_to: "/dashboard") # => "https://hanamirb.org/login?return_to=%2Fdashboard"
-    #   router.url(:framework, name: "router")      # => "https://hanamirb.org/router"
+    #   router.url(:login)                          # => #<URI::HTTPS https://hanamirb.org/login>
+    #   router.url(:login, return_to: "/dashboard") # => #<URI::HTTPS https://hanamirb.org/login?return_to=%2Fdashboard>
+    #   router.url(:framework, name: "router")      # => #<URI::HTTPS https://hanamirb.org/router>
     def url(name, variables = {})
       url_helpers.url(name, variables)
     end

lib/hanami/router/prefix.rb

@@ -10,6 +10,7 @@ class Prefix
       # @since 2.0.0
       # @api private
       def initialize(prefix)
+        prefix = DEFAULT_SEPARATOR if prefix.empty?
         @prefix = prefix
       end
 

lib/hanami/router/url_helpers.rb

@@ -2,6 +2,7 @@
 
 require "hanami/router/errors"
 require "mustermann/error"
+require_relative "./prefix"
 
 module Hanami
   class Router
@@ -11,7 +12,8 @@ class UrlHelpers
       # @since 2.0.0
       # @api private
       def initialize(base_url)
-        @base_url = base_url
+        @base_url = URI(base_url)
+        @prefix = Prefix.new(@base_url.path)
         @named = {}
       end
 
@@ -34,7 +36,7 @@ def path(name, variables = {})
       # @since 2.0.0
       # @api private
       def url(name, variables = {})
-        @base_url + path(name, variables)
+        @base_url + @prefix.join(path(name, variables)).to_s
       end
     end
   end

spec/integration/hanami/router/prefix_option_spec.rb

@@ -50,17 +50,17 @@
     end
 
     it "generates absolute URLs with prefix" do
-      expect(subject.url(:root)).to eq("https://hanami.test/admin")
+      expect(subject.url(:root)).to eq(URI("https://hanami.test/admin"))
 
-      expect(subject.url(:get_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:post_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:put_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:patch_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:delete_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:trace_home)).to eq("https://hanami.test/admin/home")
-      expect(subject.url(:options_home)).to eq("https://hanami.test/admin/home")
+      expect(subject.url(:get_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:post_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:put_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:patch_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:delete_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:trace_home)).to eq(URI("https://hanami.test/admin/home"))
+      expect(subject.url(:options_home)).to eq(URI("https://hanami.test/admin/home"))
 
-      expect(subject.url(:dashboard_home)).to eq("https://hanami.test/admin/dashboard/home")
+      expect(subject.url(:dashboard_home)).to eq(URI("https://hanami.test/admin/dashboard/home"))
     end
 
     it "recognizes requests to root" do

spec/integration/hanami/router/routing_spec.rb

@@ -98,7 +98,7 @@
 
             it "recognizes by the given symbol" do
               expect(router.path(:"#{verb}_named_route")).to eq("/named_route")
-              expect(router.url(:"#{verb}_named_route")).to  eq("http://localhost/named_route")
+              expect(router.url(:"#{verb}_named_route")).to  eq(URI("http://localhost/named_route"))
             end
           end
 
@@ -107,7 +107,7 @@
 
             it "recognizes" do
               expect(router.path(:"#{verb}_named_route_var", var: "route")).to eq("/named_route")
-              expect(router.url(:"#{verb}_named_route_var", var: "route")).to  eq("http://localhost/named_route")
+              expect(router.url(:"#{verb}_named_route_var", var: "route")).to  eq(URI("http://localhost/named_route"))
             end
           end
 
@@ -120,7 +120,7 @@
                 __send__ verb, "/custom_named_route", to: ->(_) { r }, as: :"#{verb}_custom_named_route"
               end
 
-              expect(router.url(:"#{verb}_custom_named_route")).to eq("https://hanamirb.org/custom_named_route")
+              expect(router.url(:"#{verb}_custom_named_route")).to eq(URI("https://hanamirb.org/custom_named_route"))
             end
           end
         end
@@ -198,7 +198,7 @@
 
           it "recognizes by :root" do
             expect(router.path(:root)).to eq("/")
-            expect(router.url(:root)).to  eq("http://localhost/")
+            expect(router.url(:root)).to  eq(URI("http://localhost/"))
           end
         end
 

spec/support/generation_test_case.rb

@@ -67,6 +67,6 @@ def _expected(type, expected)
   end
 
   def _absolute(expected)
-    "http://localhost#{expected}"
+    URI("http://localhost#{expected}")
   end
 end

spec/unit/hanami/router/new_spec.rb

@@ -48,7 +48,7 @@
         root to: ->(*) {}
       end
 
-      expect(router.url(:root)).to match("https")
+      expect(router.url(:root)).to eq(URI("https://hanami.test"))
     end
 
     # FIXME: verify if Hanami::Router#defined? is still needed

spec/unit/hanami/router/url_spec.rb

@@ -17,39 +17,47 @@
 
   describe "#url" do
     it "recognizes fixed string" do
-      expect(router.url(:fixed)).to eq("#{base_url}/hanami")
+      expect(router.url(:fixed)).to eq(URI("#{base_url}/hanami"))
     end
 
     it "recognizes string with variables" do
-      expect(router.url(:variables, id: "hanami")).to eq("#{base_url}/flowers/hanami")
+      expect(router.url(:variables, id: "hanami")).to eq(URI("#{base_url}/flowers/hanami"))
     end
 
     it "raises error when variables aren't satisfied" do
       expect { router.url(:variables) }.to raise_error(Hanami::Router::InvalidRouteExpansionError, "No route could be generated for `:variables': cannot expand with keys [], possible expansions: [:id]")
     end
 
     it "recognizes string with variables and constraints" do
-      expect(router.url(:constraints, id: 23)).to eq("#{base_url}/books/23")
+      expect(router.url(:constraints, id: 23)).to eq(URI("#{base_url}/books/23"))
     end
 
     it "recognizes optional variables" do
-      expect(router.url(:optional)).to eq("#{base_url}/articles")
-      expect(router.url(:optional, page: "1")).to eq("#{base_url}/articles?page=1")
-      expect(router.url(:optional, format: "rss")).to eq("#{base_url}/articles.rss")
-      expect(router.url(:optional, format: "rss", page: "1")).to eq("#{base_url}/articles.rss?page=1")
+      expect(router.url(:optional)).to eq(URI("#{base_url}/articles"))
+      expect(router.url(:optional, page: "1")).to eq(URI("#{base_url}/articles?page=1"))
+      expect(router.url(:optional, format: "rss")).to eq(URI("#{base_url}/articles.rss"))
+      expect(router.url(:optional, format: "rss", page: "1")).to eq(URI("#{base_url}/articles.rss?page=1"))
     end
 
     it "recognizes glob string" do
-      expect(router.url(:glob)).to eq("#{base_url}/files/")
+      expect(router.url(:glob)).to eq(URI("#{base_url}/files/"))
     end
 
     it "escapes additional params in query string" do
-      expect(router.url(:fixed, return_to: "/dashboard")).to eq("#{base_url}/hanami?return_to=%2Fdashboard")
+      expect(router.url(:fixed, return_to: "/dashboard")).to eq(URI("#{base_url}/hanami?return_to=%2Fdashboard"))
     end
 
     # FIXME: should preserve this behavior?
     xit "raises error when insufficient params are passed" do
       expect { router.url(nil) }.to raise_error(Hanami::Router::InvalidRouteExpansionError, "No route could be generated for nil - please check given arguments")
     end
+
+    context "base_url that contains a path" do
+      let(:base_url) { "https://hanami.test/example" }
+
+      it "doesn't clobber the base_url prefix" do
+        expect(router.url(:fixed)).to eq(URI("https://hanami.test/example/hanami"))
+      end
+    end
   end
 end