Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #174 from whitepages/flock

Added index update serializeation via lockfile.  Addresses multiple bug report.
  • Loading branch information...
commit 46c5a1df162c58dd1317275c6cfb8e6da6c7747e 2 parents 2612b33 + 8e28e9a
@cwninja cwninja authored
View
7 lib/geminabox.rb
@@ -11,6 +11,9 @@
module Geminabox
+ class Error < StandardError ; end
+ class AlreadyLocked < Error ; end
+
require_relative 'geminabox/version'
require_relative 'geminabox/proxy'
require_relative 'geminabox/http_adapter'
@@ -43,6 +46,8 @@ class << self
:allow_delete,
:rubygems_proxy,
:http_adapter,
+ :lockfile,
+ :retry_interval,
:allow_remote_failure
)
@@ -73,6 +78,8 @@ def call(env)
rubygems_proxy: (ENV['RUBYGEMS_PROXY'] == 'true'),
allow_delete: true,
http_adapter: HttpClientAdapter.new,
+ lockfile: '/tmp/geminabox.lockfile',
+ retry_interval: 60,
allow_remote_failure: false
)
View
47 lib/geminabox/server.rb
@@ -16,6 +16,8 @@ def self.delegate_to_geminabox(*delegate_methods)
:allow_replace,
:gem_permissions,
:allow_delete,
+ :lockfile,
+ :retry_interval,
:rubygems_proxy
)
@@ -100,8 +102,10 @@ def dependency_cache
end
get '/reindex' do
- self.class.reindex(:force_rebuild)
- redirect url("/")
+ serialize_update do
+ self.class.reindex(:force_rebuild)
+ redirect url("/")
+ end
end
get '/gems/:gemname' do
@@ -115,22 +119,31 @@ def dependency_cache
unless self.class.allow_delete?
error_response(403, 'Gem deletion is disabled - see https://github.com/cwninja/geminabox/issues/115')
end
- File.delete file_path if File.exists? file_path
- self.class.reindex(:force_rebuild)
- redirect url("/")
+
+ serialize_update do
+ File.delete file_path if File.exists? file_path
+ self.class.reindex(:force_rebuild)
+ redirect url("/")
+ end
+
end
post '/upload' do
- unless params[:file] && params[:file][:filename] && (tmpfile = params[:file][:tempfile])
+ if params[:file] && params[:file][:filename] && (tmpfile = params[:file][:tempfile])
+ serialize_update do
+ handle_incoming_gem(Geminabox::IncomingGem.new(tmpfile))
+ end
+ else
@error = "No file selected"
halt [400, erb(:upload)]
end
- handle_incoming_gem(Geminabox::IncomingGem.new(tmpfile))
end
post '/api/v1/gems' do
begin
- handle_incoming_gem(Geminabox::IncomingGem.new(request.body))
+ serialize_update do
+ handle_incoming_gem(Geminabox::IncomingGem.new(request.body))
+ end
rescue Object => o
File.open "/tmp/debug.txt", "a" do |io|
io.puts o, o.backtrace
@@ -140,6 +153,24 @@ def dependency_cache
private
+ def serialize_update(&block)
+ with_lock(&block)
+ rescue AlreadyLocked
+ halt 503, { 'Retry-After' => settings.retry_interval }, 'Repository lock is held by another process'
+ end
+
+ def with_lock
+ file_class.open(settings.lockfile, File::RDWR | File::CREAT) do |f|
+ raise AlreadyLocked unless f.flock(File::LOCK_EX | File::LOCK_NB)
+ yield
+ end
+ end
+
+ # This method provides a test hook, as stubbing File is painful...
+ def file_class
+ File
+ end
+
def handle_incoming_gem(gem)
begin
GemStore.create(gem, params[:overwrite])
View
115 test/units/geminabox/server_test.rb
@@ -0,0 +1,115 @@
+require 'test_helper'
+
+module Geminabox
+ class ServerTest < Minitest::Test
+ class FileOpenNoYield
+ attr_reader :name, :mode, :block, :mock_file
+ def initialize(dummy) ; end
+
+ def open(name, mode, &block)
+ @name, @mode, @block = name, mode, block
+ end
+ end
+
+ class FileOpenYields
+ attr_reader :mock_file
+
+ def initialize(flock_return)
+ @mock_file = Minitest::Mock.new
+ mock_file.expect(:flock, flock_return, [File::LOCK_EX | File::LOCK_NB])
+ end
+
+ def open(name, mode)
+ yield mock_file
+ end
+ end
+
+ module PrepServer
+ attr_reader :server, :fake_file_class, :mock_file
+
+ def prep_server(klass, flock_return = nil)
+ @server = Geminabox::Server.new.instance_variable_get(:@instance)
+ # ivar for the tests. Local variable for the define_method. <sigh>
+ fake_file_class = @fake_file_class = klass.new(flock_return)
+ server.singleton_class.send(:define_method, :file_class){ fake_file_class }
+ @mock_file = fake_file_class.mock_file
+ end
+ end
+
+ def teardown
+ Geminabox.http_adapter = HttpClientAdapter.new
+ Geminabox.allow_remote_failure = false
+ end
+
+ describe "#with_lock" do
+ include PrepServer
+
+ def do_call
+ server.send(:with_lock) { @called = true }
+ end
+
+ def test_it_yields
+ prep_server(FileOpenYields, true)
+ do_call
+ assert_equal true, @called
+ end
+
+ def test_it_calls_flock_on_the_yielded_value_exclusive_nonblocking
+ prep_server(FileOpenYields, true)
+ do_call
+ assert mock_file.verify
+ end
+
+ def test_it_blows_up_if_it_cannot_obtain_lock
+ prep_server(FileOpenYields, false)
+ assert_raises(AlreadyLocked) { do_call }
+ end
+
+ def test_it_calls_File_open_with_correct_args
+ prep_server(FileOpenNoYield)
+ do_call
+ assert_equal Geminabox.settings.lockfile, fake_file_class.name
+ assert_equal File::RDWR | File::CREAT, fake_file_class.mode
+ end
+ end
+
+ describe "#serialize_update" do
+ include PrepServer
+
+ before do
+ @server = Geminabox::Server.new.instance_variable_get(:@instance)
+ def @server.args
+ @args
+ end
+ end
+
+ def test_block_passed_to_with_lock
+ def @server.with_lock(&block)
+ @args = block
+ end
+
+ blk = Proc.new { @called = true }
+ @server.send(:serialize_update, &blk)
+ assert_equal blk, @server.args
+ end
+
+ def test_AlreadyLocked_in_with_lock_invokes_halt
+ def @server.with_lock(&block)
+ raise AlreadyLocked
+ end
+
+ def @server.halt(code, headers, message)
+ @args = [code, headers, message]
+ end
+
+ @server.send(:serialize_update){}
+ expected_args = [
+ 503,
+ {'Retry-After' => Geminabox.settings.retry_interval},
+ 'Repository lock is held by another process'
+ ]
+ assert_equal expected_args, @server.args
+ end
+ end
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.