Merge pull request #106 from 5apps/bugfix/handle_wrong_redis_metadata

Handle out of sync metadata in Redis on PUTs

lib/remote_storage/swift.rb

@@ -52,7 +52,9 @@ module RemoteStorage
 
       set_response_headers(res)
 
-      none_match = (server.env["HTTP_IF_NONE_MATCH"] || "").gsub(/^"?W\//, "").split(",").map(&:strip)
+      none_match = (server.env["HTTP_IF_NONE_MATCH"] || "").split(",")
+                                                           .map(&:strip)
+                                                           .map { |s| s.gsub(/^"?W\//, "") }
       server.halt 304 if none_match.include? %Q("#{res.headers[:etag]}")
 
       return res.body
@@ -71,7 +73,9 @@ module RemoteStorage
 
       server.headers["Content-Type"] = "application/ld+json"
 
-      none_match = (server.env["HTTP_IF_NONE_MATCH"] || "").gsub(/^"?W\//, "").split(",").map(&:strip)
+      none_match = (server.env["HTTP_IF_NONE_MATCH"] || "").split(",")
+                                                           .map(&:strip)
+                                                           .map { |s| s.gsub(/^"?W\//, "") }
 
       if etag
         server.halt 304 if none_match.include? %Q("#{etag}")
@@ -139,9 +143,24 @@ module RemoteStorage
       url = url_for_key(user, directory, key)
 
       if required_match = server.env["HTTP_IF_MATCH"]
-        unless required_match.gsub(/^"?W\//, "") == %Q("#{existing_metadata["e"]}")
+        required_match = required_match.gsub(/^"?W\//, "")
+        unless required_match == %Q("#{existing_metadata["e"]}")
+
+          # get actual metadata and compare in case redis metadata became out of sync
+          begin
+            head_res = do_head_request(url)
+          # The file doesn't exist in Orbit, return 412
+          rescue RestClient::ResourceNotFound
             server.halt 412, "Precondition Failed"
           end
+
+          if required_match == %Q("#{head_res.headers[:etag]}")
+            # log previous size difference that was missed ealier because of redis failure
+            log_size_difference(user, existing_metadata["s"], head_res.headers[:content_length])
+          else
+            server.halt 412, "Precondition Failed"
+          end
+        end
       end
       if server.env["HTTP_IF_NONE_MATCH"] == "*"
         server.halt 412, "Precondition Failed" unless existing_metadata.empty?

spec/swift/app_spec.rb

@@ -284,11 +284,44 @@ describe "App" do
         it "fails the request if the header does not match the current ETag" do
           header "If-Match", "someotheretag"
 
+          head_stub = OpenStruct.new(headers: {
+            etag: "oldetag",
+            last_modified: "Fri, 04 Mar 2016 12:20:18 GMT",
+            content_type: "text/plain",
+            content_length: 23
+          })
+
+          RestClient.stub :head, head_stub do
             put "/phil/food/aguacate", "aye"
+          end
 
           last_response.status.must_equal 412
           last_response.body.must_equal "Precondition Failed"
         end
+
+        it "allows the request if redis metadata became out of sync" do
+          header "If-Match", "\"existingetag\""
+
+          head_stub = OpenStruct.new(headers: {
+            etag: "existingetag",
+            last_modified: "Fri, 04 Mar 2016 12:20:18 GMT",
+            content_type: "text/plain",
+            content_length: 23
+          })
+
+          put_stub = OpenStruct.new(headers: {
+            etag: "newetag",
+            last_modified: "Fri, 04 Mar 2016 12:20:18 GMT"
+          })
+
+          RestClient.stub :head, head_stub do
+            RestClient.stub :put, put_stub do
+              put "/phil/food/aguacate", "aye"
+            end
+          end
+
+          last_response.status.must_equal 200
+        end
       end
 
       describe "If-None-Match header set to '*'" do
@@ -307,7 +340,7 @@ describe "App" do
           last_response.status.must_equal 201
         end
 
-        it "fails the request if the document already exsits" do
+        it "fails the request if the document already exists" do
           put_stub = OpenStruct.new(headers: {
             etag: "someetag",
             last_modified: "Fri, 04 Mar 2016 12:20:18 GMT"