From 11fd696dd60d4bd2109da79f5d22263b8c4fe737 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Thu, 1 Dec 2011 02:34:58 +0000
Subject: [PATCH] PUT is no longer atomic, remove temporary file use

It's not necessary for a backend MogileFS storage layer and
just leaves us open to errors (like the bug we just fixed).
---
 lib/mogstored_rack.rb            | 63 ++++++++++++----------------------------
 test/test_unit_mogstored_rack.rb |  9 ++++--
 2 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/lib/mogstored_rack.rb b/lib/mogstored_rack.rb
index aedec4c..80f0a55 100644
--- a/lib/mogstored_rack.rb
+++ b/lib/mogstored_rack.rb
@@ -49,21 +49,7 @@ class MogstoredRack
     @mkcol_perms = opts[:mkcol_perms] || (~File.umask & 0777)
     @reread_verify = !! opts[:reread_verify] # unsupported
     @open_flags = opts[:open_flags] || 0
-    @open_flags |= IO::RDWR | IO::CREAT | IO::EXCL
-  end
-
-  def tmpfile(basename, dir) # :nodoc:
-    t = Time.now.utc.strftime("%Y%m%d%H%M%S")
-    seq = 0
-    begin
-      fp = File.open("#{dir}/#{basename}.#{t}.#{seq}.tmp", @open_flags, 0600)
-    rescue Errno::EEXIST
-      retry if (seq += 1) < 10
-      raise
-    end
-    fp.binmode
-    fp.sync = true
-    fp
+    @open_flags |= IO::RDWR | IO::CREAT
   end
 
   def call(env) # :nodoc:
@@ -118,40 +104,34 @@ class MogstoredRack
              return r(400,
                       "Bad range, Content-Range: #{range} does not match\n" \
                       "Content-Length: #{clen.inspect}", env)
-    File.open(path, @open_flags & ~IO::EXCL, 0600) do |fp|
-      fp.binmode
-      fp.sync = true
-      fp.seek(off_out)
-      received_md5 = put_loop(env["rack.input"], fp)
-      err = content_md5_fail?(env, received_md5) and return err
-      fp.chmod(@put_perms)
-      fsync(File.dirname(path), fp) if @fsync
+    File.open(path, @open_flags, 0600) { |fp| put_write(env, fp, off_out) }
+  end
+
+  def put_write(env, fp, offset = nil)
+    fp.binmode
+    fp.sync = true
+    fp.seek(offset) if offset
+    received_md5 = put_loop(env, fp)
+    err = content_md5_fail?(env, received_md5) and return err
+    fp.chmod(@put_perms)
+    if @fsync
+      fp.fsync
+      File.open(File.dirname(fp.path)) { |io| io.fsync }
     end
     r(201)
   end
 
   def put(env) # :nodoc:
     path = server_path(env) or return r(400)
-    dir = File.dirname(path)
-    File.directory?(dir) or return r(403)
     range = env["HTTP_CONTENT_RANGE"] and
       return put_range(range, env, path)
-
-    tmp = tmpfile(File.basename(path), dir)
-    received = put_loop(env["rack.input"], tmp)
-    err = content_md5_fail?(env, received) and return err
-    tmp.chmod(@put_perms)
-    renamed = File.rename(tmp.path, path)
-    fsync(dir, tmp) if @fsync
-    r(201)
-    ensure
-      if tmp
-        File.unlink(tmp.path) unless renamed
-        tmp.close
-      end
+    File.open(path, @open_flags|IO::TRUNC, 0600) { |fp| put_write(env, fp) }
+    rescue Errno::ENOENT
+      r(403)
   end
 
-  def put_loop(src, dst) # :nodoc:
+  def put_loop(env, dst) # :nodoc:
+    src = env["rack.input"]
     buf = ""
     md5 = Digest::MD5.new
     while src.read(@io_size, buf)
@@ -200,9 +180,4 @@ class MogstoredRack
            "received: #{received}", env)
   end
 
-  # fsync each and every directory component above us on the same device
-  def fsync(dir, tmp) # :nodoc:
-    tmp.fsync
-    File.open(dir) { |io| io.fsync }
-  end
 end
diff --git a/test/test_unit_mogstored_rack.rb b/test/test_unit_mogstored_rack.rb
index 26bd998..038f5f2 100644
--- a/test/test_unit_mogstored_rack.rb
+++ b/test/test_unit_mogstored_rack.rb
@@ -22,8 +22,14 @@ class TestUnitMogstoredRack < Test::Unit::TestCase
   end
 
   def all_methods(req)
-    assert_equal 200, req.get("/").status
+    assert_equal 200, req.get("/").status # needed to pass "mogadm check"
     assert ! File.directory?("#@docroot/dev666")
+
+    r = req.request("PUT", "/dev666/666.fid", :input => StringIO.new("AAAA"))
+    assert_equal 403, r.status
+
+    assert ! File.directory?("#@docroot/dev666")
+
     assert_equal 204, req.request("MKCOL", "/dev666").status
     assert File.directory?("#@docroot/dev666")
 
@@ -38,7 +44,6 @@ class TestUnitMogstoredRack < Test::Unit::TestCase
     opts = { :input => io, "HTTP_CONTENT_MD5" => md5 }
     r = req.request("PUT", "/dev666/666.fid", opts)
     assert_equal 400, r.status
-    assert_equal "HELLO", IO.read("#@docroot/dev666/666.fid")
 
     # valid MD5
     io = StringIO.new("VALID")
-- 
2.11.4.GIT