Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI #224

Merged
merged 15 commits into from Dec 28, 2012
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/faraday.rb
Expand Up @@ -181,7 +181,12 @@ def lookup_middleware(key)
end

def middleware_mutex(&block)
(@middleware_mutex ||= Mutex.new).synchronize(&block)
@middleware_mutex ||= Mutex.new
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a very dubious way to initialize a Mutex. If you have two threads that call middleware_mutex at the same time, they may get different mutexes the first time.

Also, instead of trying to use @middleware_mutex.locked? to create a poor mans recursive mutex, this should really just use Monitor instead. It acts just like a mutex but it's recursive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I have no idea what I'm doing here. Just trying to remove autoload.
It'd be awesome if someone that wanted autoload gone would help remove it
:)

It looks like I can essentially replace Mutex with Monitor, and it'll
magically work?

On Fri, Dec 28, 2012 at 1:54 AM, Eric Lindvall notifications@github.comwrote:

In lib/faraday.rb:

@@ -181,7 +181,12 @@ def lookup_middleware(key)
end

 def middleware_mutex(&block)
  •  (@middleware_mutex ||= Mutex.new).synchronize(&block)
    
  •  @middleware_mutex ||= Mutex.new
    

This seems like a very dubious way to initialize a Mutex. If you have two
threads that call middleware_mutex at the same time, they may get
different mutexes the first time.

Also, instead of trying to use @middleware_mutex.locked? to create a poor
mans recursive mutex, this should really just use Monitor instead. It
acts just like a mutex but it's recursive.


Reply to this email directly or view it on GitHubhttps://github.com/technoweenie/faraday/pull/224/files#r2514046.

Rick Olson
http://github.com/technoweenie

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. Monitor has the same interface.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rad, thanks.

if @middleware_mutex.locked?
block.call
else
@middleware_mutex.synchronize(&block)
end
end

def fetch_middleware(key)
Expand Down
1 change: 1 addition & 0 deletions lib/faraday/adapter/net_http.rb
Expand Up @@ -13,6 +13,7 @@ class NetHttp < Faraday::Adapter
EOFError,
Errno::ECONNABORTED,
Errno::ECONNREFUSED,
Errno::ENETUNREACH,
Errno::ECONNRESET,
Errno::EINVAL,
Net::HTTPBadResponse,
Expand Down
61 changes: 34 additions & 27 deletions lib/faraday/adapter/test.rb
Expand Up @@ -28,46 +28,46 @@ def empty?
@stack.empty?
end

def match(request_method, path, body)
def match(request_method, path, headers, body)
return false if !@stack.key?(request_method)
stack = @stack[request_method]
consumed = (@consumed[request_method] ||= [])
path = normalize_path(path)

if stub = matches?(stack, path, body)
if stub = matches?(stack, path, headers, body)
consumed << stack.delete(stub)
stub
else
matches?(consumed, path, body)
matches?(consumed, path, headers, body)
end
end

def get(path, &block)
new_stub(:get, path, &block)
def get(path, headers = {}, &block)
new_stub(:get, path, headers, &block)
end

def head(path, &block)
new_stub(:head, path, &block)
def head(path, headers = {}, &block)
new_stub(:head, path, headers, &block)
end

def post(path, body=nil, &block)
new_stub(:post, path, body, &block)
def post(path, body=nil, headers = {}, &block)
new_stub(:post, path, headers, body, &block)
end

def put(path, body=nil, &block)
new_stub(:put, path, body, &block)
def put(path, body=nil, headers = {}, &block)
new_stub(:put, path, headers, body, &block)
end

def patch(path, body=nil, &block)
new_stub(:patch, path, body, &block)
def patch(path, body=nil, headers = {}, &block)
new_stub(:patch, path, headers, body, &block)
end

def delete(path, &block)
new_stub(:delete, path, &block)
def delete(path, headers = {}, &block)
new_stub(:delete, path, headers, &block)
end

def options(path, &block)
new_stub(:options, path, &block)
def options(path, headers = {}, &block)
new_stub(:options, path, headers, &block)
end

# Raises an error if any of the stubbed calls have not been made.
Expand All @@ -85,12 +85,12 @@ def verify_stubbed_calls

protected

def new_stub(request_method, path, body=nil, &block)
(@stack[request_method] ||= []) << Stub.new(normalize_path(path), body, block)
def new_stub(request_method, path, headers = {}, body=nil, &block)
(@stack[request_method] ||= []) << Stub.new(normalize_path(path), headers, body, block)
end

def matches?(stack, path, body)
stack.detect { |stub| stub.matches?(path, body) }
def matches?(stack, path, headers, body)
stack.detect { |stub| stub.matches?(path, headers, body) }
end

# ensure leading + trailing slash
Expand All @@ -102,23 +102,24 @@ def normalize_path(path)
end
end

class Stub < Struct.new(:path, :params, :body, :block)
def initialize(full, body, block)
class Stub < Struct.new(:path, :params, :headers, :body, :block)
def initialize(full, headers, body, block)
path, query = full.split('?')
params = query ?
Faraday::Utils.parse_nested_query(query) :
{}
super path, params, body, block
super path, params, headers, body, block
end

def matches?(request_uri, request_body)
def matches?(request_uri, request_headers, request_body)
request_path, request_query = request_uri.split('?')
request_params = request_query ?
Faraday::Utils.parse_nested_query(request_query) :
{}
request_path == path &&
params_match?(request_params) &&
(body.to_s.size.zero? || request_body == body)
(body.to_s.size.zero? || request_body == body) &&
headers_match?(request_headers)
end

def params_match?(request_params)
Expand All @@ -127,6 +128,12 @@ def params_match?(request_params)
end
end

def headers_match?(request_headers)
headers.keys.all? do |key|
request_headers[key] == headers[key]
end
end

def to_s
"#{path} #{body}"
end
Expand All @@ -146,7 +153,7 @@ def call(env)
super
normalized_path = Faraday::Utils.normalize_path(env[:url])

if stub = stubs.match(env[:method], normalized_path, env[:body])
if stub = stubs.match(env[:method], normalized_path, env.request_headers, env[:body])
env[:params] = (query = env[:url].query) ?
Faraday::Utils.parse_nested_query(query) :
{}
Expand Down
2 changes: 1 addition & 1 deletion lib/faraday/request/basic_authentication.rb
@@ -1,7 +1,7 @@
require 'base64'

module Faraday
class Request::BasicAuthentication < Request::Authorization
class Request::BasicAuthentication < Request.load_middleware(:authorization)
# Public
def self.header(login, pass)
value = Base64.encode64([login, pass].join(':'))
Expand Down
2 changes: 1 addition & 1 deletion lib/faraday/request/token_authentication.rb
@@ -1,5 +1,5 @@
module Faraday
class Request::TokenAuthentication < Request::Authorization
class Request::TokenAuthentication < Request.load_middleware(:authorization)
# Public
def self.header(token, options = nil)
options ||= {}
Expand Down
9 changes: 9 additions & 0 deletions script/console
@@ -0,0 +1,9 @@
#!/usr/bin/env ruby
# Usage: script/console
# Starts an IRB console with this library loaded.

require 'rubygems'
require 'irb'
require File.expand_path("../ruby", __FILE__)
require File.expand_path("../../lib/#{MakeScript.lib_name}", __FILE__)
IRB.start
32 changes: 32 additions & 0 deletions script/gemspec
@@ -0,0 +1,32 @@
#!/usr/bin/env ruby
# Usage: script/gemspec
# Updates the gemspec's name, version, and file manifest.

require File.expand_path("../ruby", __FILE__)

def replace_header(head, header_name)
head.sub!(/(\.#{header_name}\s*= ').*'/) { "#{$1}#{MakeScript.send(header_name)}'"}
end

# read spec file and split out manifest section
spec = File.read(MakeScript.gemspec_file)
head, manifest, tail = spec.split(" # = MANIFEST =\n")

# replace name version and date
replace_header(head, :lib_name)
replace_header(head, :version)

files = `git ls-files`.
split("\n").
sort.
reject { |file| file =~ /^\./ }.
reject { |file| file =~ /^(rdoc|pkg)/ }.
map { |file| " #{file}" }.
join("\n")

# piece file back together and write
manifest = " s.files = %w[\n#{files}\n ]\n"
spec = [head, manifest, tail].join(" # = MANIFEST =\n")
File.open(MakeScript.gemspec_file, 'w') { |io| io.write(spec) }
puts "Updated #{MakeScript.gemspec_file}"

29 changes: 29 additions & 0 deletions script/generate_certs
@@ -0,0 +1,29 @@
#!/usr/bin/env ruby
# Usage: generate_certs
# Generate test certs for testing Faraday with SSL

require File.expand_path("../ruby", __FILE__)
require 'openssl'
require 'fileutils'

# Adapted from WEBrick::Utils. Skips cert extensions so it
# can be used as a CA bundle
def create_self_signed_cert(bits, cn, comment)
rsa = OpenSSL::PKey::RSA.new(bits)
cert = OpenSSL::X509::Certificate.new
cert.version = 2
cert.serial = 1
name = OpenSSL::X509::Name.new(cn)
cert.subject = name
cert.issuer = name
cert.not_before = Time.now
cert.not_after = Time.now + (365*24*60*60)
cert.public_key = rsa.public_key
cert.sign(rsa, OpenSSL::Digest::SHA1.new)
return [cert, rsa]
end

cert, key = create_self_signed_cert(1024, [['CN', 'localhost']], 'Faraday Test CA')
FileUtils.mkdir_p 'tmp'
File.open('tmp/faraday-cert.key', 'w') {|f| f.puts(key) }
File.open('tmp/faraday-cert.crt', 'w') {|f| f.puts(cert.to_s) }
12 changes: 12 additions & 0 deletions script/package
@@ -0,0 +1,12 @@
#!/usr/bin/env ruby
# Usage: script/gem
# Updates the gemspec and builds a new gem in the pkg directory.

require File.expand_path("../ruby", __FILE__)
require 'fileutils'

FileUtils.mkdir_p File.expand_path("../../pkg", __FILE__)
system "script/gemspec"
system "gem build #{MakeScript.gemspec_file}"
system "mv #{MakeScript.gem_file} pkg"

14 changes: 14 additions & 0 deletions script/release
@@ -0,0 +1,14 @@
#!/usr/bin/env ruby
# Usage: script/release
# Build the package, tag a commit, push it to origin, and then release the
# package publicly.

require File.expand_path("../ruby", __FILE__)

system "script/package"
system "git commit --allow-empty -a -m 'Release #{MakeScript.version}'"
system "git tag v#{MakeScript.version}"
system "git push origin"
system "git push origin v#{MakeScript.version}"
system "gem push pkg/#{MakeScript.gem_file}"

36 changes: 36 additions & 0 deletions script/ruby.rb
@@ -0,0 +1,36 @@
# Opinionated module that picks out a ruby library's name and version based on
# some conventions:
#
# * #{lib_name}.gemspec
# * A VERSION constant defined in lib/#{lib_name}.rb
#
# Example:
#
# # foo.gemspec
# # lib/foo.rb
# module Foo
# VERSION = "0.1"
# end

module MakeScript
extend self

def lib_name
@lib_name ||= Dir['*.gemspec'].first.split('.').first
end

def version
@version ||= begin
line = File.read("lib/#{lib_name}.rb")[/^\s*VERSION\s*=\s*.*/]
line.match(/.*VERSION\s*=\s*['"](.*)['"]/)[1]
end
end

def gemspec_file
@gemspec_file ||= "#{lib_name}.gemspec"
end

def gem_file
@gem_file ||= "#{lib_name}-#{version}.gem"
end
end
7 changes: 5 additions & 2 deletions script/test
@@ -1,15 +1,18 @@
#!/usr/bin/env ruby
# Usage: script/test [file] [adapter]... -- [test/unit options]
# Runs the test suite against a local server spawned automatically in a
# thread. After tests are done, the server is shut down.
#
# If filename arguments are given, only those files are run. If arguments given
# are not filenames, they are taken as words that filter the list of files to run.
#
# Examples
# Examples:
#
# $ script/test
# $ script/test test/env_test.rb
# $ script/test excon typhoeus
#
# # Run only tests matching /ssl/ for the net_http adapter, with SSL enabled.
# $ SSL=yes script/test net_http -- -n /ssl/

require 'rubygems'
Expand All @@ -33,7 +36,7 @@ if ssl_mode = ENV['SSL'] == 'yes'
unless ENV['SSL_KEY'] and ENV['SSL_FILE']
key_file = ENV['SSL_KEY'] = 'tmp/faraday-cert.key'
cert_file = ENV['SSL_FILE'] = 'tmp/faraday-cert.crt'
system 'rake', key_file, cert_file
system 'script/generate_certs'
abort unless $?.success?
end
end
Expand Down
19 changes: 17 additions & 2 deletions test/adapters/test_middleware_test.rb
Expand Up @@ -2,8 +2,9 @@

module Adapters
class TestMiddleware < Faraday::TestCase
Stubs = Faraday::Adapter.lookup_middleware(:test)::Stubs
def setup
@stubs = Faraday::Adapter::Test::Stubs.new
@stubs = Stubs.new
@conn = Faraday.new do |builder|
builder.adapter :test, @stubs
end
Expand Down Expand Up @@ -41,6 +42,13 @@ def test_middleware_ignores_unspecified_get_params
end
end

def test_middleware_with_http_headers
@stubs.get('/yo', { 'X-HELLO' => 'hello' }) { [200, {}, 'a'] }
@stubs.get('/yo') { [200, {}, 'b'] }
assert_equal 'a', @conn.get('/yo') { |env| env.headers['X-HELLO'] = 'hello' }.body
assert_equal 'b', @conn.get('/yo').body
end

def test_middleware_allow_different_outcomes_for_the_same_request
@stubs.get('/hello') { [200, {'Content-Type' => 'text/html'}, 'hello'] }
@stubs.get('/hello') { [200, {'Content-Type' => 'text/html'}, 'world'] }
Expand All @@ -62,9 +70,16 @@ def test_yields_env_to_stubs
end

def test_raises_an_error_if_no_stub_is_found_for_request
assert_raises Faraday::Adapter::Test::Stubs::NotFound do
assert_raises Stubs::NotFound do
@conn.get('/invalid'){ [200, {}, []] }
end
end

def test_raises_an_error_if_no_stub_is_found_for_request_without_this_header
@stubs.get('/yo', { 'X-HELLO' => 'hello' }) { [200, {}, 'a'] }
assert_raises Faraday::Adapter::Test::Stubs::NotFound do
@conn.get('/yo')
end
end
end
end