Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactor redirects

  • Loading branch information...
commit bb29bd1f1f5c7c3eb7883591d206a7f2aadcec2e 1 parent a3e536c
Paul Sadauskas authored
View
1  .autotest
@@ -1,4 +1,3 @@
#!/usr/bin/env ruby
require 'autotest/redgreen'
-require 'autotest/timestamp'
View
2  lib/resourceful.rb
@@ -1,5 +1,7 @@
require 'resourceful/http_accessor'
+require 'resourceful/util'
+
# Resourceful is a library that provides a high level HTTP interface.
module Resourceful
# A Hash of named URIs. Many methods in Resourceful that need a URI
View
22 lib/resourceful/request.rb
@@ -5,19 +5,31 @@ module Resourceful
class Request
+ REDIRECTABLE_METHODS = [:get, :head]
+
attr_accessor :method, :resource, :body, :header
def initialize(http_method, resource, body = nil, header = nil)
@method, @resource, @body, @header = http_method, resource, body, header
+ @response = nil
end
- def make
-
- http_resp = NetHttpAdapter.make_request(@method, @resource.uri, @body, @header)
- response = Resourceful::Response.new(*http_resp)
+ def response
+ if @response.nil?
+ http_resp = NetHttpAdapter.make_request(@method, @resource.uri, @body, @header)
+ @response = Resourceful::Response.new(*http_resp)
+ end
- response
+ @response
+ end
+ def should_be_redirected?
+ if resource.on_redirect.nil?
+ return true if method.in? REDIRECTABLE_METHODS
+ false
+ else
+ resource.on_redirect.call(self, response)
+ end
end
end
View
71 lib/resourceful/resource.rb
@@ -24,29 +24,26 @@ def on_redirect(&block)
end
def get
- request = Resourceful::Request.new(:get, self)
- response = request.make
-
- if response.code == 301
- if @on_redirect.nil? or @on_redirect.call(request, response)
- @uris.unshift response.header['Location'].first
- request = Resourceful::Request.new(:get, self)
- response = request.make
- end
- end
-
- response
+ do_read_request(:get)
end
def post(data = "")
request = Resourceful::Request.new(:post, self, data)
- response = request.make
+ response = request.response
if response.code == 301
if @on_redirect and @on_redirect.call(request, response)
@uris.unshift response.header['Location'].first
request = Resourceful::Request.new(:post, self)
- response = request.make
+ response = request.response
+ end
+ end
+ if response.code == 302
+ if @on_redirect and @on_redirect.call(request, response)
+ @uris.unshift response.header['Location'].first
+ request = Resourceful::Request.new(:get, self)
+ response = request.response
+ @uris.shift # don't remember this new location
end
end
@@ -55,13 +52,21 @@ def post(data = "")
def put(data = "")
request = Resourceful::Request.new(:put, self, data)
- response = request.make
+ response = request.response
if response.code == 301
if @on_redirect and @on_redirect.call(request, response)
@uris.unshift response.header['Location'].first
request = Resourceful::Request.new(:put, self)
- response = request.make
+ response = request.response
+ end
+ end
+ if response.code == 302
+ if @on_redirect and @on_redirect.call(request, response)
+ @uris.unshift response.header['Location'].first
+ request = Resourceful::Request.new(:get, self)
+ response = request.response
+ @uris.shift # don't remember this new location
end
end
@@ -70,19 +75,49 @@ def put(data = "")
def delete
request = Resourceful::Request.new(:delete, self)
- response = request.make
+ response = request.response
if response.code == 301
if @on_redirect and @on_redirect.call(request, response)
@uris.unshift response.header['Location'].first
request = Resourceful::Request.new(:delete, self)
- response = request.make
+ response = request.response
+ end
+ end
+ if response.code == 302
+ if @on_redirect and @on_redirect.call(request, response)
+ @uris.unshift response.header['Location'].first
+ request = Resourceful::Request.new(:get, self)
+ response = request.response
+ @uris.shift # don't remember this new location
end
end
response
end
+ protected
+
+ def do_read_request(method)
+ request = Resourceful::Request.new(:get, self)
+ response = request.response
+
+ if response.is_redirect? and request.should_be_redirected?
+ previous_response = response
+ @uris.unshift response.header['Location'].first
+ request = Resourceful::Request.new(:delete, self)
+ response = request.response
+ @uris.shift unless previous_response.code == 301
+ end
+
+ response
+
+ end
+
+ def do_write_request(method, data)
+
+ end
+
end
end
View
7 lib/resourceful/response.rb
@@ -3,6 +3,8 @@
module Resourceful
class Response
+ REDIRECT_RESPONSE_CODES = [301,302,303,307]
+
attr_reader :code, :header, :body
alias headers header
@@ -10,6 +12,11 @@ def initialize(code, header, body)
@code, @header, @body = code, header, body
end
+ def is_redirect?
+ @code.in? REDIRECT_RESPONSE_CODES
+ end
+ alias was_redirect? is_redirect?
+
end
end
View
6 lib/resourceful/util.rb
@@ -0,0 +1,6 @@
+
+class Object
+ def in?(arr)
+ arr.include?(self)
+ end
+end
View
36 spec/acceptance_shared_specs.rb
@@ -0,0 +1,36 @@
+describe 'redirect', :shared => true do
+ before do
+ @callback = mock('callback')
+ @callback.stub!(:call).and_return(true)
+ end
+
+ it 'should be followed by default on GET' do
+ resp = @resource.get
+ resp.should be_instance_of(Resourceful::Response)
+ resp.code.should == 200
+ resp.header['Content-Type'].should == ['text/plain']
+ end
+
+ %w{PUT POST DELETE}.each do |method|
+ it "should not be followed by default on #{method}" do
+ resp = @resource.send(method.downcase.intern)
+ resp.should be_instance_of(Resourceful::Response)
+ resp.code.should == @redirect_code
+ end
+
+ it "should redirect on #{method} if the redirection callback returns true" do
+ @resource.on_redirect { @callback.call }
+ resp = @resource.send(method.downcase.intern)
+ resp.code.should == 200
+ end
+
+ it "should not redirect on #{method} if the redirection callback returns false" do
+ @callback.stub!(:call).and_return(false)
+ @resource.on_redirect { @callback.call }
+ resp = @resource.send(method.downcase.intern)
+ resp.code.should == @redirect_code
+ end
+ end
+
+end
+
View
64 spec/acceptance_spec.rb
@@ -1,5 +1,6 @@
require 'pathname'
require Pathname(__FILE__).dirname + 'spec_helper'
+require Pathname(__FILE__).dirname + 'acceptance_shared_specs'
require 'resourceful'
@@ -51,7 +52,7 @@
resp.header['Content-Type'].should == ['text/plain']
end
- describe 'redirects' do
+ describe 'redirecting' do
describe 'registering callback' do
before do
@@ -81,39 +82,11 @@
describe '301 Moved Permanently' do
before do
+ @redirect_code = 301
@resource = @accessor.resource('http://localhost:3000/redirect/301?http://localhost:3000/get')
-
- @callback = mock('callback')
- @callback.stub!(:call).and_return(true)
end
- it 'should be followed by default on GET' do
- resp = @resource.get
- resp.should be_instance_of(Resourceful::Response)
- resp.code.should == 200
- resp.header['Content-Type'].should == ['text/plain']
- end
-
- %w{PUT POST DELETE}.each do |method|
- it "should not be followed by default on #{method}" do
- resp = @resource.send(method.downcase.intern)
- resp.should be_instance_of(Resourceful::Response)
- resp.code.should == 301
- end
-
- it "should redirect on #{method} if the redirection callback returns true" do
- @resource.on_redirect { @callback.call }
- resp = @resource.send(method.downcase.intern)
- resp.code.should == 200
- end
-
- it "should not redirect on #{method} if the redirection callback returns false" do
- @callback.stub!(:call).and_return(false)
- @resource.on_redirect { @callback.call }
- resp = @resource.send(method.downcase.intern)
- resp.code.should == 301
- end
- end
+ it_should_behave_like 'redirect'
it 'should change the effective uri of the resource' do
@resource.get
@@ -123,9 +96,26 @@
end
describe '302 Found' do
+ before do
+ @redirect_code = 302
+ @resource = @accessor.resource('http://localhost:3000/redirect/302?http://localhost:3000/get')
+ end
+
+ it_should_behave_like 'redirect'
+
+ it 'should not change the effective uri of the resource'
+
+ end
+
+ describe '303 Other' do
+
+ end
+
+ describe '307 Temporary Redirect' do
end
+
end
describe 'caching' do
@@ -144,6 +134,18 @@
it 'should raise InvalidResponse when response code is invalid'
+ describe 'client errors' do
+
+ it 'should raise when there is one'
+
+ end
+
+ describe 'server errors' do
+
+ it 'should raise when there is one'
+
+ end
+
end
end
View
62 spec/resourceful/request_spec.rb
@@ -38,7 +38,7 @@
end
- describe 'make' do
+ describe 'response' do
before do
@net_http_adapter_response = mock('net_http_adapter_response')
Resourceful::NetHttpAdapter.stub!(:make_request).and_return(@net_http_adapter_response)
@@ -48,18 +48,22 @@
end
it 'should be a method' do
- @request.should respond_to(:make)
+ @request.should respond_to(:response)
end
it 'should look in the cache'
it 'should create a Resourceful::Response object from the NetHttpAdapter response' do
Resourceful::Response.should_receive(:new).with(@net_http_adapter_response).and_return(@response)
- @request.make
+ @request.response
+ end
+
+ it 'should set the response to #response' do
+ @request.response.should == @response
end
it 'should return the Response object' do
- @request.make.should == @response
+ @request.response.should == @response
end
describe 'GET' do
@@ -70,7 +74,7 @@
it 'should #get the uri from the NetHttpAdapter' do
Resourceful::NetHttpAdapter.should_receive(:make_request).
with(:get, @uri, nil, nil).and_return(@net_http_adapter_response)
- @request.make
+ @request.response
end
end
@@ -83,10 +87,56 @@
it 'should #get the uri from the NetHttpAdapter' do
Resourceful::NetHttpAdapter.should_receive(:make_request).with(:post, @uri, @post_data, nil).and_return(@net_http_adapter_response)
- @request.make
+ @request.response
+ end
+
+ end
+
+ end
+
+ describe '#should_be_redirected?' do
+ before do
+ @net_http_adapter_response = mock('net_http_adapter_response')
+ Resourceful::NetHttpAdapter.stub!(:make_request).and_return(@net_http_adapter_response)
+
+ @response = mock('response')
+ Resourceful::Response.stub!(:new).and_return(@response)
+ end
+
+ describe 'with no callback set' do
+ before do
+ @callback = nil
+ @resource.stub!(:on_redirect).and_return(@callback)
+ end
+
+ it 'should be true for GET' do
+ request = Resourceful::Request.new(:get, @resource, @post_data)
+
+ request.should_be_redirected?.should be_true
+ end
+
+ it 'should be false for POST, etc' do
+ request = Resourceful::Request.new(:post, @resource, @post_data)
+
+ request.should_be_redirected?.should be_false
end
+ end
+
+ it 'should be true when callback returns true' do
+ @callback = lambda { true }
+ @resource.stub!(:on_redirect).and_return(@callback)
+ request = Resourceful::Request.new(:get, @resource, @post_data)
+
+ request.should_be_redirected?.should be_true
+ end
+
+ it 'should be false when callback returns false' do
+ @callback = lambda { false }
+ @resource.stub!(:on_redirect).and_return(@callback)
+ request = Resourceful::Request.new(:get, @resource, @post_data)
+ request.should_be_redirected?.should be_false
end
end
View
91 spec/resourceful/resource_spec.rb
@@ -9,9 +9,9 @@
@uri = 'http://www.example.com/'
@resource = Resourceful::Resource.new(@accessor, @uri)
- @response = mock('response', :code => 200)
+ @response = mock('response', :code => 200, :is_redirect? => false)
- @request = mock('request', :make => @response)
+ @request = mock('request', :response => @response, :should_be_redirected? => true)
Resourceful::Request.stub!(:new).and_return(@request)
end
@@ -53,7 +53,7 @@
end
it 'should make the request' do
- @request.should_receive(:make).and_return(@response)
+ @request.should_receive(:response).and_return(@response)
@resource.get
end
@@ -75,7 +75,7 @@
end
it 'should make the request' do
- @request.should_receive(:make).and_return(@response)
+ @request.should_receive(:response).and_return(@response)
@resource.post('Hello from post!')
end
@@ -97,7 +97,7 @@
end
it 'should make the request' do
- @request.should_receive(:make).and_return(@response)
+ @request.should_receive(:response).and_return(@response)
@resource.put('Hello from put!')
end
@@ -119,7 +119,7 @@
end
it 'should make the request' do
- @request.should_receive(:make).and_return(@response)
+ @request.should_receive(:response).and_return(@response)
@resource.delete
end
@@ -137,8 +137,8 @@
@resource = Resourceful::Resource.new(@accessor, @uri)
@redirected_uri = 'http://www.example.com/get'
- @redirect_response = mock('redirect_response', :code => 301, :header => {'Location' => [@redirected_uri]})
- @request.stub!(:make).and_return(@redirect_response, @response)
+ @redirect_response = mock('redirect_response', :code => 301, :header => {'Location' => [@redirected_uri]}, :is_redirect? => true)
+ @request.stub!(:response).and_return(@redirect_response, @response)
@callback = mock('callback')
@callback.stub!(:call).and_return(true)
@@ -158,23 +158,6 @@
@resource.on_redirect.should be_kind_of(Proc)
end
- it 'should perform the callback when response is a redirect' do
- @callback.should_receive(:call).and_return(true)
- @resource.get
- end
-
- it 'should not perform the callback when not redirected' do
- @request.stub!(:make).and_return(@response)
- @callback.should_not_receive(:call)
- @resource.get
- end
-
- it 'should not perform the redirect if the callback returns false' do
- @callback.should_receive(:call).and_return(false)
- resp = @resource.get
- resp.should == @redirect_response
- end
-
it 'should yield the request,response to the callback' do
@resource.on_redirect { |req,resp|
req.should == @request
@@ -192,15 +175,15 @@
@resource = Resourceful::Resource.new(@accessor, @uri)
@redirected_uri = 'http://www.example.com/get'
- @redirect_response = mock('redirect_response', :code => 301, :header => {'Location' => [@redirected_uri]})
- @request.stub!(:make).and_return(@redirect_response, @response)
+ @redirect_response = mock('redirect_response', :code => 301, :header => {'Location' => [@redirected_uri]}, :is_redirect? => true)
+ @request.stub!(:response).and_return(@redirect_response, @response)
@callback = mock('callback')
@callback.stub!(:call).and_return(true)
end
it 'should be followed automatically on GET' do
- @request.should_receive(:make).and_return(@redirect_response, @response)
+ @request.should_receive(:response).and_return(@redirect_response, @response)
@resource.get.should == @response
end
@@ -212,12 +195,12 @@
it 'should have the redirected to uri as the effective uri' do
@resource.get
- @resource.effective_uri.should == 'http://www.example.com/get'
+ @resource.effective_uri.should == @redirected_uri
end
%w{PUT POST DELETE}.each do |method|
it "should not redirect automatically on #{method}" do
- @request.should_receive(:make).once.and_return(@redirect_response)
+ @request.should_receive(:response).once.and_return(@redirect_response)
@resource.send(method.downcase.intern).should == @redirect_response
end
@@ -229,7 +212,7 @@
it "should not redirect on #{method} if the redirection callback returns false" do
@callback.stub!(:call).and_return(false)
@resource.on_redirect { @callback.call }
- @request.stub!(:make).once.and_return(@redirect_response)
+ @request.stub!(:response).once.and_return(@redirect_response)
@resource.send(method.downcase.intern).should == @redirect_response
end
end
@@ -237,6 +220,52 @@
end
describe '302 Found' do
+ before do
+ @uri = 'http://www.example.com/redirect/302?http://www.example.com/get'
+ @resource = Resourceful::Resource.new(@accessor, @uri)
+
+ @redirected_uri = 'http://www.example.com/get'
+ @redirect_response = mock('redirect_response', :code => 302, :header => {'Location' => [@redirected_uri]}, :is_redirect? => true)
+ @request.stub!(:response).and_return(@redirect_response, @response)
+
+ @callback = mock('callback')
+ @callback.stub!(:call).and_return(true)
+ end
+
+ it 'should be followed automatically on GET' do
+ @request.should_receive(:response).and_return(@redirect_response, @response)
+ @resource.get.should == @response
+ end
+
+ it 'should not add the redirected to uri to the beginning of the uri list' do
+ @resource.instance_variable_get(:@uris).should == [@uri]
+ @resource.get
+ @resource.instance_variable_get(:@uris).should == [@uri]
+ end
+
+ it 'should have the original request uri as the effective uri' do
+ @resource.get
+ @resource.effective_uri.should == @uri
+ end
+
+ %w{PUT POST DELETE}.each do |method|
+ it "should not redirect automatically on #{method}" do
+ @request.should_receive(:response).once.and_return(@redirect_response)
+ @resource.send(method.downcase.intern).should == @redirect_response
+ end
+
+ it "should redirect on #{method} if the redirection callback returns true" do
+ @resource.on_redirect { @callback.call }
+ @resource.send(method.downcase.intern).should == @response
+ end
+
+ it "should not redirect on #{method} if the redirection callback returns false" do
+ @callback.stub!(:call).and_return(false)
+ @resource.on_redirect { @callback.call }
+ @request.stub!(:response).once.and_return(@redirect_response)
+ @resource.send(method.downcase.intern).should == @redirect_response
+ end
+ end
end
View
11 spec/resourceful/response_spec.rb
@@ -43,5 +43,16 @@
@response.should respond_to(:body)
end
+ it 'should know if it is a redirect' do
+ Resourceful::Response.new(301, {}, "").is_redirect?.should == true
+ Resourceful::Response.new(302, {}, "").is_redirect?.should == true
+ Resourceful::Response.new(303, {}, "").is_redirect?.should == true
+ Resourceful::Response.new(307, {}, "").is_redirect?.should == true
+
+ #aliased as was_redirect?
+ Resourceful::Response.new(301, {}, "").was_redirect?.should == true
+ end
+
+
end
View
1  spec/spec_helper.rb
@@ -4,6 +4,7 @@
require 'pp'
$LOAD_PATH << Pathname(__FILE__).dirname + "../lib"
+require 'resourceful/util'
class Object
def tap
Please sign in to comment.
Something went wrong with that request. Please try again.