Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Upgrade sinatra to 1.4.2 [Fixes #46496455] #109

Merged
merged 1 commit into from

2 participants

@frodenas

Changes needed to upgrade to sinatra 1.4.2

  • Use Rack:File instead of Sinatra::Base::StaticFile
  • Redirects uses 303 http code for other verbs than GET
director/lib/director/api/api_helper.rb
((22 lines not shown))
def send_disposable_file(path, opts = {})
- stat = File.stat(path)
- last_modified(opts[:last_modified] || stat.mtime)
-
- if opts[:type] or not response["Content-Type"]
- content_type(opts[:type] || File.extname(path),
- :default => "application/octet-stream")
+ if opts[:type] or not response['Content-Type']

I know this isn't your fault, but can you change or not to || ! - the operator precedence of and, or and not is not the same as for &&, || and !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
director/lib/director/api/api_helper.rb
@@ -5,38 +5,37 @@ module Api
module ApiHelper
READ_CHUNK_SIZE = 16384
- class DisposableStaticFile < ::Sinatra::Base::StaticFile
+ class DisposableRackFile < ::Rack::File

just call it DisposableFile so if we have to change implementation again we won't have to rename all uses of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@frodenas

@pmenglund Fixed both comments.

director/lib/director/api/api_helper.rb
((31 lines not shown))
end
- if opts[:disposition] == "attachment" || opts[:filename]
- attachment opts[:filename] || path
- elsif opts[:disposition] == "inline"
- response["Content-Disposition"] = "inline"
- end
+ disposition = opts[:disposition]
+ filename = opts[:filename]
+ disposition = 'attachment' if disposition.nil? and filename

found one more and :)

@frodenas
frodenas added a note

Merge or die :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@frodenas frodenas Upgrade sinatra to 1.4.2
Changes needed to upgrade to sinatra 1.4.2

- Use Rack:File instead of Sinatra::Base::StaticFile
- Redirects uses 303 http code for other verbs than GET
ec67394
@frodenas

@pmenglund Be ready for your next PR!!! :)

@pmenglund

@frodenas haha - I'm sure it will be nit-picky :)

@pmenglund pmenglund merged commit b1b6d50 into master

1 check passed

Details default The Travis build passed
@frodenas frodenas deleted the upgrade_sinatra branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 3, 2013
  1. @frodenas

    Upgrade sinatra to 1.4.2

    frodenas authored
    Changes needed to upgrade to sinatra 1.4.2
    
    - Use Rack:File instead of Sinatra::Base::StaticFile
    - Redirects uses 303 http code for other verbs than GET
This page is out of date. Refresh to see the latest.
View
25 Gemfile.lock
@@ -31,7 +31,7 @@ PATH
retryable (~> 1.3.2)
ruby-atmos-pure (~> 1.0.5)
sigar (~> 0.7.2)
- sinatra (~> 1.2.8)
+ sinatra (~> 1.4.2)
sys-filesystem (~> 1.1.0)
thin (~> 1.5.0)
uuidtools (~> 2.1.2)
@@ -121,7 +121,7 @@ PATH
aws-sdk (= 1.8.5)
fog (~> 1.10.0)
sequel (~> 3.43.0)
- sinatra (~> 1.2.8)
+ sinatra (~> 1.4.2)
thin (~> 1.5.0)
yajl-ruby (~> 1.1.0)
@@ -167,7 +167,7 @@ PATH
redis (~> 3.0.2)
resque (~> 1.23.0)
sequel (~> 3.43.0)
- sinatra (~> 1.2.8)
+ sinatra (~> 1.4.2)
thin (~> 1.5.0)
yajl-ruby (~> 1.1.0)
@@ -180,7 +180,7 @@ PATH
eventmachine (~> 0.12.10)
logging (~> 1.5.0)
nats (~> 0.4.28)
- sinatra (~> 1.2.8)
+ sinatra (~> 1.4.2)
thin (~> 1.5.0)
yajl-ruby (~> 1.1.0)
@@ -221,7 +221,7 @@ PATH
remote: simple_blobstore_server
specs:
simple_blobstore_server (1.5.0.pre.3)
- sinatra (~> 1.2.8)
+ sinatra (~> 1.4.2)
thin (~> 1.5.0)
GEM
@@ -233,7 +233,6 @@ GEM
json (~> 1.4)
nokogiri (>= 1.4.4)
uuidtools (~> 2.1)
- backports (3.1.1)
bcrypt-ruby (3.0.1)
builder (3.1.4)
ci_reporter (1.8.4)
@@ -259,7 +258,7 @@ GEM
ffi (1.6.0)
fog (1.10.0)
builder
- excon (~> 0.14)
+ excon (~> 0.20)
formatador (~> 0.2.0)
mime-types
multi_json (~> 1.0)
@@ -318,7 +317,9 @@ GEM
coderay (~> 1.0.5)
method_source (~> 0.8)
slop (~> 3.4)
- rack (1.4.5)
+ rack (1.5.2)
+ rack-protection (1.5.0)
+ rack
rack-test (0.6.2)
rack (>= 1.0)
rake (10.0.4)
@@ -354,10 +355,10 @@ GEM
simplecov-html (0.7.1)
simplecov-rcov (0.2.3)
simplecov (>= 0.4.1)
- sinatra (1.2.9)
- backports
- rack (~> 1.1, < 1.5)
- tilt (>= 1.2.2, < 2.0)
+ sinatra (1.4.2)
+ rack (~> 1.5, >= 1.5.2)
+ rack-protection (~> 1.4)
+ tilt (~> 1.3, >= 1.3.4)
slop (3.4.4)
sqlite3 (1.3.7)
sys-filesystem (1.1.0)
View
2  bosh_agent/bosh_agent.gemspec
@@ -21,7 +21,7 @@ Gem::Specification.new do |s|
s.add_dependency 'ruby-atmos-pure', '~>1.0.5'
s.add_dependency 'thin', '~>1.5.0'
s.add_dependency 'yajl-ruby', '~>1.1.0'
- s.add_dependency 'sinatra', '~>1.2.8'
+ s.add_dependency 'sinatra', '~>1.4.2'
s.add_dependency 'nats', '~>0.4.28'
s.add_dependency 'sigar', '~>0.7.2'
s.add_dependency 'httpclient', '=2.2.4'
View
2  bosh_cli/lib/cli/director.rb
@@ -456,7 +456,7 @@ def request_and_track(method, uri, options = {})
http_status, _, headers = request(method, uri, content_type, payload)
location = headers[:location]
- redirected = http_status == 302
+ redirected = [302, 303].include? http_status
task_id = nil
if redirected
View
25 bosh_cli/spec/unit/director_spec.rb
@@ -271,7 +271,7 @@
end
describe "tracking request" do
- it "starts polling task if request responded with a redirect to task URL" do
+ it "starts polling task if request responded with a redirect (302) to task URL" do
options = { :arg1 => 1, :arg2 => 2 }
@director.should_receive(:request).
@@ -292,6 +292,27 @@
should == ["polling result", "502"]
end
+ it "starts polling task if request responded with a redirect (303) to task URL" do
+ options = { :arg1 => 1, :arg2 => 2 }
+
+ @director.should_receive(:request).
+ with(:get, "/stuff", "text/plain", "abc").
+ and_return([303, "body", { :location => "/tasks/502" }])
+
+ tracker = mock("tracker", :track => "polling result", :output => "foo")
+
+ Bosh::Cli::TaskTracker.should_receive(:new).
+ with(@director, "502", options).
+ and_return(tracker)
+
+ @director.request_and_track(:get, "/stuff",
+ {:content_type => "text/plain",
+ :payload => "abc",
+ :arg1 => 1, :arg2 => 2
+ }).
+ should == ["polling result", "502"]
+ end
+
describe "not tracking trackable requests" do
it "returns without tracking/polling task if request responded with a redirect to task URL" do
options = { :arg1 => 1, :arg2 => 2 }
@@ -319,7 +340,7 @@
end
end
- it "considers all responses but 302 a failure" do
+ it "considers all responses but 302 and 303 a failure" do
[200, 404, 403].each do |code|
@director.should_receive(:request).
with(:get, "/stuff", "text/plain", "abc").
View
2  bosh_registry/bosh_registry.gemspec
@@ -20,7 +20,7 @@ Gem::Specification.new do |s|
s.executables = %w(bosh_registry migrate)
s.add_dependency "sequel", "~>3.43.0"
- s.add_dependency "sinatra", "~>1.2.8"
+ s.add_dependency "sinatra", "~>1.4.2"
s.add_dependency "thin", "~>1.5.0"
s.add_dependency "yajl-ruby", "~>1.1.0"
s.add_dependency "fog", "~>1.10.0"
View
2  director/director.gemspec
@@ -38,7 +38,7 @@ Gem::Specification.new do |s|
s.add_dependency "redis", "~>3.0.2"
s.add_dependency "resque", "~>1.23.0"
s.add_dependency "sequel", "~>3.43.0"
- s.add_dependency "sinatra", "~>1.2.8"
+ s.add_dependency "sinatra", "~>1.4.2"
s.add_dependency "thin", "~>1.5.0"
s.add_dependency "yajl-ruby", "~>1.1.0"
View
41 director/lib/director/api/api_helper.rb
@@ -5,38 +5,37 @@ module Api
module ApiHelper
READ_CHUNK_SIZE = 16384
- class DisposableStaticFile < ::Sinatra::Base::StaticFile
+ class DisposableFile < ::Rack::File
def close
- super
FileUtils.rm_rf(self.path) if File.exists?(self.path)
end
end
- # Adapted from Sinatra::Base#send_file. There are two differences:
- # it doesn't support range queries
- # it uses DisposableStaticFile instead of Sinatra::Base::StaticFile.
- # DisposableStaticFile gets removed on "close" call. This is primarily
+ # Adapted from Sinatra::Base#send_file. There is one difference:
+ # it uses DisposableFile instead of Rack::File.
+ # DisposableFile gets removed on "close" call. This is primarily
# meant to serve temporary files fetched from the blobstore.
+ # We CANNOT use a Sinatra after filter, as the filter is called before
+ # the contents of the file is sent to the client.
def send_disposable_file(path, opts = {})
- stat = File.stat(path)
- last_modified(opts[:last_modified] || stat.mtime)
-
- if opts[:type] or not response["Content-Type"]
- content_type(opts[:type] || File.extname(path),
- :default => "application/octet-stream")
+ if opts[:type] || !response['Content-Type']
+ content_type opts[:type] || File.extname(path), :default => 'application/octet-stream'
end
- if opts[:disposition] == "attachment" || opts[:filename]
- attachment opts[:filename] || path
- elsif opts[:disposition] == "inline"
- response["Content-Disposition"] = "inline"
- end
+ disposition = opts[:disposition]
+ filename = opts[:filename]
+ disposition = 'attachment' if disposition.nil? && filename
+ filename = path if filename.nil?
+ attachment(filename, disposition) if disposition
- file_length = opts[:length] || stat.size
- sf = DisposableStaticFile.open(path, "rb")
+ last_modified opts[:last_modified] if opts[:last_modified]
- response["Content-Length"] ||= file_length.to_s
- halt sf
+ file = DisposableFile.new nil
+ file.path = path
+ result = file.serving env
+ result[1].each { |k,v| headers[k] ||= v }
+ headers['Content-Length'] = result[1]['Content-Length']
+ halt opts[:status] || result[0], result[2]
rescue Errno::ENOENT
not_found
end
View
2  health_monitor/health_monitor.gemspec
@@ -23,7 +23,7 @@ Gem::Specification.new do |s|
s.add_dependency "nats", "~>0.4.28"
s.add_dependency "yajl-ruby", "~>1.1.0"
s.add_dependency "thin", "~>1.5.0"
- s.add_dependency "sinatra", "~>1.2.8"
+ s.add_dependency "sinatra", "~>1.4.2"
s.add_dependency "aws-sdk", "1.8.5"
s.bindir = 'bin'
View
2  simple_blobstore_server/simple_blobstore_server.gemspec
@@ -18,7 +18,7 @@ Gem::Specification.new do |s|
s.require_path = "lib"
s.add_dependency "thin", "~>1.5.0"
- s.add_dependency "sinatra", "~> 1.2.8"
+ s.add_dependency "sinatra", "~> 1.4.2"
s.bindir = 'bin'
s.executables << 'simple_blobstore_server'
View
BIN  vendor/cache/backports-3.1.1.gem
Binary file not shown
View
BIN  vendor/cache/rack-1.4.5.gem
Binary file not shown
View
BIN  vendor/cache/rack-1.5.2.gem
Binary file not shown
View
BIN  vendor/cache/rack-protection-1.5.0.gem
Binary file not shown
View
BIN  vendor/cache/sinatra-1.2.9.gem
Binary file not shown
View
BIN  vendor/cache/sinatra-1.4.2.gem
Binary file not shown
Something went wrong with that request. Please try again.