refactor request so routing_tokens are immutable and passed to constructor

as suggested by @bethesque at
https://github.com/seancribbs/webmachine-ruby/pull/223/files#r25999998

Author: madlep

lib/webmachine/adapters/rack.rb

@@ -97,7 +97,13 @@ def build_webmachine_request(rack_req, headers)
         Webmachine::Request.new(rack_req.request_method,
                                 rack_req.url,
                                 headers,
-                                RequestBody.new(rack_req))
+                                RequestBody.new(rack_req),
+                                routing_tokens(rack_req)
+                               )
+      end
+
+      def routing_tokens(rack_req)
+        nil # no-op for default, un-mapped rack adapter
       end
 
       class RackResponse
@@ -173,13 +179,10 @@ def each
     end # class Rack
 
     class RackMapped < Rack
-      def build_webmachine_request(rack_req, headers)
-        request = super
+      def routing_tokens(rack_req)
         routing_match = rack_req.path_info.match(Webmachine::Request::ROUTING_PATH_MATCH)
         routing_path = routing_match ? routing_match[1] : ""
-        routing_tokens = routing_path.split(SLASH)
-        request.routing_tokens = routing_tokens
-        request
+        routing_path.split(SLASH)
       end
     end
 

lib/webmachine/request.rb

@@ -11,19 +11,19 @@ class Request
 
     extend Forwardable
 
-    attr_reader :method, :uri, :headers, :body
+    attr_reader :method, :uri, :headers, :body, :routing_tokens
     attr_accessor :disp_path, :path_info, :path_tokens
-    attr_writer :routing_tokens
 
     # @param [String] method the HTTP request method
     # @param [URI] uri the requested URI, including host, scheme and
     #   port
     # @param [Headers] headers the HTTP request headers
     # @param [String,#to_s,#each,nil] body the entity included in the
     #   request, if present
-    def initialize(method, uri, headers, body)
+    def initialize(method, uri, headers, body, routing_tokens=nil)
       @method, @headers, @body = method, headers, body
       @uri = build_uri(uri, headers)
+      @routing_tokens = routing_tokens || @uri.path.match(ROUTING_PATH_MATCH)[1].split(SLASH)
     end
 
     def_delegators :headers, :[]
@@ -167,10 +167,6 @@ def options?
 
     ROUTING_PATH_MATCH = /^\/(.*)/.freeze
 
-    def routing_tokens
-      @routing_tokens ||= uri.path.match(ROUTING_PATH_MATCH)[1].split(SLASH)
-    end
-
     private
 
     IPV6_MATCH = /\A\[(?<address> .* )\]:(?<port> \d+ )\z/x.freeze  # string like "[::1]:80"

spec/webmachine/dispatcher/route_spec.rb

@@ -7,7 +7,8 @@ def warn(*msgs); end # silence warnings for tests
 describe Webmachine::Dispatcher::Route do
   let(:method) { "GET" }
   let(:uri) { URI.parse("http://localhost:8080/") }
-  let(:request){ Webmachine::Request.new(method, uri, Webmachine::Headers.new, "") }
+  let(:routing_tokens) { nil }
+  let(:request){ Webmachine::Request.new(method, uri, Webmachine::Headers.new, "", routing_tokens) }
   let(:resource){ Class.new(Webmachine::Resource) }
 
   describe '#apply' do
@@ -16,9 +17,7 @@ def warn(*msgs); end # silence warnings for tests
     }
 
     describe 'a path_info fragment' do
-      before do
-        uri.path = '/hello/planet%20earth%20++'
-      end
+      let(:uri) { URI.parse("http://localhost:8080/hello/planet%20earth%20++") }
 
       it 'should decode the value' do
         route.apply(request)
@@ -30,14 +29,10 @@ def warn(*msgs); end # silence warnings for tests
   matcher :match_route do |*expected|
     route = Webmachine::Dispatcher::Route.new(expected[0], Class.new(Webmachine::Resource), expected[1] || {})
     match do |actual|
-      case actual
-      when String
-        request.uri.path = actual if String === actual
-      when Array
-        request.uri.path = "/some/route/" + actual.join("/")
-        request.routing_tokens = actual
-      end
-      route.match?(request)
+      uri = URI.parse("http://localhost:8080")
+      uri.path = actual
+      req = Webmachine::Request.new("GET", uri, Webmachine::Headers.new, "", routing_tokens)
+      route.match?(req)
     end
 
     failure_message do |_|
@@ -132,7 +127,8 @@ def call(request)
     end
 
     context "with a request with explicitly specified routing tokens" do
-      subject { ["foo", "bar"] }
+      subject { "/some/route/foo/bar" }
+      let(:routing_tokens) { ["foo", "bar"] }
       it { is_expected.to match_route(["foo", "bar"]) }
       it { is_expected.to match_route(["foo", :id]) }
       it { is_expected.to match_route ['*'] }
@@ -187,7 +183,8 @@ def call(request)
 
     context "on a deep path" do
       subject { described_class.new(%w{foo bar baz}, resource) }
-      before { request.uri.path = "/foo/bar/baz"; subject.apply(request) }
+      let(:uri) { URI.parse("http://localhost:8080/foo/bar/baz") }
+      before { subject.apply(request) }
 
       it "should assign the dispatched path as the path past the initial slash" do
         expect(request.disp_path).to eq("foo/bar/baz")

spec/webmachine/request_spec.rb

@@ -3,11 +3,12 @@
 describe Webmachine::Request do
   subject { request }
 
-  let(:uri)         { URI.parse("http://localhost:8080/some/resource") }
-  let(:http_method) { "GET" }
-  let(:headers)     { Webmachine::Headers.new }
-  let(:body)        { "" }
-  let(:request)     { Webmachine::Request.new(http_method, uri, headers, body) }
+  let(:uri)             { URI.parse("http://localhost:8080/some/resource") }
+  let(:http_method)     { "GET" }
+  let(:headers)         { Webmachine::Headers.new }
+  let(:body)            { "" }
+  let(:routing_tokens)  { nil }
+  let(:request)         { Webmachine::Request.new(http_method, uri, headers, body, routing_tokens) }
 
   it "should provide access to the headers via brackets" do
     subject.headers['Accept'] = "*/*"
@@ -242,14 +243,15 @@ def body; block_given? ? yield(@body) : @body; end
   describe '#routing_tokens' do
     subject { request.routing_tokens }
 
-    context "haven't be explicitly set" do
+    context "haven't been explicitly set" do
+      let(:routing_tokens) { nil }
       it "extracts the routing tokens from the path portion of the uri" do
         expect(subject).to eq(["some", "resource"])
       end
     end
 
     context "have been explicitly set" do
-      before { request.routing_tokens = ["foo", "bar"] }
+      let(:routing_tokens) { ["foo", "bar"] }
 
       it "uses the specified routing_tokens" do
         expect(subject).to eq(["foo", "bar"])