From 5378826c6a4d453d259a3788cb0460df605559af Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Fri, 4 Mar 2016 21:49:04 +0100 Subject: [PATCH 1/4] Also use content checksum for ETag calculation Timestamp is not enough, as it might be the same for two consequent PUTs. --- lib/remote_storage/swift.rb | 10 +++++++--- spec/swift/app_spec.rb | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index 00bda35..1cf8760 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -156,7 +156,7 @@ module RemoteStorage if update_metadata_object(user, directory, key, metadata) if metadata_changed?(existing_metadata, metadata) - update_dir_objects(user, directory, timestamp) + update_dir_objects(user, directory, timestamp, checksum_for(data)) end server.headers["ETag"] = %Q("#{res.headers[:etag]}") @@ -166,6 +166,10 @@ module RemoteStorage end end + def checksum_for(data) + Digest::MD5.hexdigest(data) + end + def delete_data(user, directory, key) url = url_for_key(user, directory, key) @@ -316,9 +320,9 @@ module RemoteStorage true end - def update_dir_objects(user, directory, timestamp) + def update_dir_objects(user, directory, timestamp, checksum) parent_directories_for(directory).each do |dir| - etag = etag_for(dir, timestamp) + etag = etag_for(dir, timestamp, checksum) key = "rs:m:#{user}:#{dir}/" metadata = {e: etag, m: timestamp} diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 6e8a7a7..3df89e0 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -301,11 +301,11 @@ describe "App" do get "/phil/food/" last_response.status.must_equal 200 - last_response.headers["ETag"].must_equal "\"a693babe4b4027de2340b4f1c362d2c8\"" + last_response.headers["ETag"].must_equal "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" end it "responds with 304 when IF_NONE_MATCH header contains the ETag" do - header "If-None-Match", "\"a693babe4b4027de2340b4f1c362d2c8\"" + header "If-None-Match", "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" get "/phil/food/" last_response.status.must_equal 304 @@ -328,7 +328,7 @@ describe "App" do content["items"]["camaron"]["Content-Length"].must_equal 5 content["items"]["camaron"]["ETag"].must_equal "bla" content["items"]["desunyos/"].wont_be_nil - content["items"]["desunyos/"]["ETag"].must_equal "5e17228c28f15521416812ecac6f718e" + content["items"]["desunyos/"]["ETag"].must_equal "926233ce4a147a5b679e0ddf665517dd" end it "contains all items in the root directory" do @@ -339,7 +339,7 @@ describe "App" do content = JSON.parse(last_response.body) content["items"]["food/"].wont_be_nil - content["items"]["food/"]["ETag"].must_equal "a693babe4b4027de2340b4f1c362d2c8" + content["items"]["food/"]["ETag"].must_equal "f9f85fbf5aa1fa378fd79ac8aa0a457d" end end From 1d39445ba9713a81032796fbac7db121b95e6cc0 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Fri, 4 Mar 2016 22:16:35 +0100 Subject: [PATCH 2/4] Fix deleting metadata for empty subdirectories --- lib/remote_storage/swift.rb | 4 +-- spec/swift/app_spec.rb | 65 ++++++++++--------------------------- 2 files changed, 20 insertions(+), 49 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index 1cf8760..66f269a 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -341,8 +341,8 @@ module RemoteStorage parent_directories_for(directory).each do |dir| if dir_empty?(user, dir) - redis.del "rs:m:#{user}:#{directory}/" - redis.srem "rs:m:#{user}:#{parent_directory_for(dir)}:items", "#{dir}/" + redis.del "rs:m:#{user}:#{dir}/" + redis.srem "rs:m:#{user}:#{parent_directory_for(dir)}:items", "#{top_directory(dir)}/" else etag = etag_for(dir, timestamp) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 3df89e0..af0968f 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -186,23 +186,14 @@ describe "App" do RestClient.stub :put, put_stub do put "/phil/food/aguacate", "si" put "/phil/food/camaron", "yummi" + put "/phil/food/desunyos/bolon", "wow" end end it "deletes the metadata object in redis" do - put_stub = OpenStruct.new(headers: { - etag: "bla", - last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" - }) - get_stub = OpenStruct.new(body: "rootbody") - - RestClient.stub :put, put_stub do - RestClient.stub :delete, "" do - RestClient.stub :get, get_stub do - RemoteStorage::Swift.stub_any_instance :etag_for, "rootetag" do - delete "/phil/food/aguacate" - end - end + RestClient.stub :delete, "" do + RemoteStorage::Swift.stub_any_instance :etag_for, "rootetag" do + delete "/phil/food/aguacate" end end @@ -213,19 +204,9 @@ describe "App" do it "deletes the directory objects metadata in redis" do old_metadata = redis.hgetall "rs:m:phil:food/" - put_stub = OpenStruct.new(headers: { - etag: "bla", - last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" - }) - get_stub = OpenStruct.new(body: "rootbody") - - RestClient.stub :put, put_stub do - RestClient.stub :delete, "" do - RestClient.stub :get, get_stub do - RemoteStorage::Swift.stub_any_instance :etag_for, "newetag" do - delete "/phil/food/aguacate" - end - end + RestClient.stub :delete, "" do + RemoteStorage::Swift.stub_any_instance :etag_for, "newetag" do + delete "/phil/food/aguacate" end end @@ -235,38 +216,28 @@ describe "App" do metadata["m"].wont_equal old_metadata["m"] food_items = redis.smembers "rs:m:phil:food/:items" - food_items.must_equal ["camaron"] + food_items.must_equal ["desunyos/", "camaron"] root_items = redis.smembers "rs:m:phil:/:items" root_items.must_equal ["food/"] end it "deletes the parent directory objects metadata when deleting all items" do - put_stub = OpenStruct.new(headers: { - etag: "bla", - last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" - }) - get_stub = OpenStruct.new(body: "rootbody") - - RestClient.stub :put, put_stub do - RestClient.stub :delete, "" do - RestClient.stub :get, get_stub do - RemoteStorage::Swift.stub_any_instance :etag_for, "rootetag" do - delete "/phil/food/aguacate" - delete "/phil/food/camaron" - end - end + RestClient.stub :delete, "" do + RemoteStorage::Swift.stub_any_instance :etag_for, "rootetag" do + delete "/phil/food/aguacate" + delete "/phil/food/camaron" + delete "/phil/food/desunyos/bolon" end end - metadata = redis.hgetall "rs:m:phil:food/" - metadata.must_be_empty + redis.smembers("rs:m:phil:food/desunyos:items").must_be_empty + redis.hgetall("rs:m:phil:food/desunyos/").must_be_empty - food_items = redis.smembers "rs:m:phil:food/:items" - food_items.must_be_empty + redis.smembers("rs:m:phil:food/:items").must_be_empty + redis.hgetall("rs:m:phil:food/").must_be_empty - root_items = redis.smembers "rs:m:phil:/:items" - root_items.must_be_empty + redis.smembers("rs:m:phil:/:items").must_be_empty end end end From 648f86e0f3a17bff0dc0feab7962df189fd57d6d Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Fri, 4 Mar 2016 22:40:04 +0100 Subject: [PATCH 3/4] Fix typo in specs --- spec/swift/app_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index af0968f..f132b75 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -186,7 +186,7 @@ describe "App" do RestClient.stub :put, put_stub do put "/phil/food/aguacate", "si" put "/phil/food/camaron", "yummi" - put "/phil/food/desunyos/bolon", "wow" + put "/phil/food/desayunos/bolon", "wow" end end @@ -216,7 +216,7 @@ describe "App" do metadata["m"].wont_equal old_metadata["m"] food_items = redis.smembers "rs:m:phil:food/:items" - food_items.must_equal ["desunyos/", "camaron"] + food_items.must_equal ["desayunos/", "camaron"] root_items = redis.smembers "rs:m:phil:/:items" root_items.must_equal ["food/"] @@ -227,12 +227,12 @@ describe "App" do RemoteStorage::Swift.stub_any_instance :etag_for, "rootetag" do delete "/phil/food/aguacate" delete "/phil/food/camaron" - delete "/phil/food/desunyos/bolon" + delete "/phil/food/desayunos/bolon" end end - redis.smembers("rs:m:phil:food/desunyos:items").must_be_empty - redis.hgetall("rs:m:phil:food/desunyos/").must_be_empty + redis.smembers("rs:m:phil:food/desayunos:items").must_be_empty + redis.hgetall("rs:m:phil:food/desayunos/").must_be_empty redis.smembers("rs:m:phil:food/:items").must_be_empty redis.hgetall("rs:m:phil:food/").must_be_empty @@ -262,7 +262,7 @@ describe "App" do RestClient.stub :put, put_stub do put "/phil/food/aguacate", "si" put "/phil/food/camaron", "yummi" - put "/phil/food/desunyos/bolon", "wow" + put "/phil/food/desayunos/bolon", "wow" end end @@ -298,8 +298,8 @@ describe "App" do content["items"]["camaron"]["Content-Type"].must_equal "text/plain; charset=utf-8" content["items"]["camaron"]["Content-Length"].must_equal 5 content["items"]["camaron"]["ETag"].must_equal "bla" - content["items"]["desunyos/"].wont_be_nil - content["items"]["desunyos/"]["ETag"].must_equal "926233ce4a147a5b679e0ddf665517dd" + content["items"]["desayunos/"].wont_be_nil + content["items"]["desayunos/"]["ETag"].must_equal "dd36e3cfe52b5f33421150b289a7d48d" end it "contains all items in the root directory" do From df87deaf0b405f1905c136f983e096aac670e5d5 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Fri, 4 Mar 2016 22:48:50 +0100 Subject: [PATCH 4/4] Don't fail when order of array is different --- spec/swift/app_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index f132b75..29e5e4f 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -216,7 +216,7 @@ describe "App" do metadata["m"].wont_equal old_metadata["m"] food_items = redis.smembers "rs:m:phil:food/:items" - food_items.must_equal ["desayunos/", "camaron"] + food_items.sort.must_equal ["camaron", "desayunos/"] root_items = redis.smembers "rs:m:phil:/:items" root_items.must_equal ["food/"]