Make RESTful attachment API concurrency aware and the code a little more concise (thanks Damien!)

git-svn-id: https://svn.apache.org/repos/asf/incubator/couchdb/trunk@675699 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 85 insertions, 128 deletions

diff --git a/share/www/script/couch_tests.js b/share/www/script/couch_tests.js
index a0e6b2ff..6a1e72c8 100644
--- a/share/www/script/couch_tests.js
+++ b/share/www/script/couch_tests.js
@@ -561,54 +561,65 @@ var tests = {
     
     // test RESTful doc API
     
-    // test without rev, should fail
-    var xhr = CouchDB.request("DELETE", "/test_suite_db/bin_doc/foo.txt");
-    T(xhr.status == 412);
-    
-    db.deleteDocAttachment({_id:"bin_doc"}, "foo.txt");
-    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc/foo.txt");
-    T(xhr.status == 404);
-    
-    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc/foo.txt", {
-      body: "This is not base64 encoded text",
-      headers:{"Content-Type":"text/plain;charset=utf-8"}
+    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc2/foo2.txt", {
+      body:"This is no base64 encoded text",
+      headers:{"Content-Type": "text/plain;charset=utf-8"}
     });
     T(xhr.status == 201);
-
-    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc/foo.txt");
-    T(xhr.responseText == "This is not base64 encoded text");
+    var rev = JSON.parse(xhr.responseText).rev;
+    
+    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc2/foo2.txt");
+    T(xhr.responseText == "This is no base64 encoded text");
     T(xhr.getResponseHeader("Content-Type") == "text/plain;charset=utf-8");
+    
+    // test without rev, should fail
+    var xhr = CouchDB.request("DELETE", "/test_suite_db/bin_doc2/foo2.txt");
+    T(xhr.status == 412);
 
+    // test with rev, should not fail
+    var xhr = CouchDB.request("DELETE", "/test_suite_db/bin_doc2/foo2.txt?rev=" + rev);
+    T(xhr.status == 200);
+    
+    
     // test binary data
-    var xhr = CouchDB.request("GET", "/favicon.ico");
-    var bin_data = xhr.responseText;
-    // bin_data = 123;
-    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc/favicon.ico", {
-      headers:{"Content-Type":"image/vnd.microsoft.icon"},
+    var bin_data = "JHAPDO*AU£PN ){(3u[d 93DQ9¡€])}    ææøo'∂ƒæ≤çæππ•¥∫¶®#†π¶®¥π€ª®˙π8np";
+    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc3/attachment.txt", {
+      headers:{"Content-Type":"text/plain;charset=utf-8"},
       body:bin_data
     });
     T(xhr.status == 201);
     var rev = JSON.parse(xhr.responseText).rev;
-
-    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc/favicon.ico");
+    
+    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc3/attachment.txt");
     T(xhr.responseText == bin_data);
-    T(xhr.getResponseHeader("Content-Type") == "image/vnd.microsoft.icon");
+    T(xhr.getResponseHeader("Content-Type") == "text/plain;charset=utf-8");
     
-     var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc/favicon.ico", {
-      headers:{"Content-Type":"image/vnd.microsoft.icon"},
+    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc3/attachment.txt", {
+      headers:{"Content-Type":"text/plain;charset=utf-8"},
       body:bin_data
     });
     T(xhr.status == 412);
 
-    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc/favicon.ico?rev="+rev, {
-      headers:{"Content-Type":"image/vnd.microsoft.icon"},
+    var xhr = CouchDB.request("PUT", "/test_suite_db/bin_doc3/attachment.txt?rev=" + rev, {
+      headers:{"Content-Type":"text/plain;charset=utf-8"},
       body:bin_data
     });
     T(xhr.status == 201);
     var rev = JSON.parse(xhr.responseText).rev;
 
-    var xhr = CouchDB.request("DELETE", "/test_suite_db/bin_doc/foo.txt?rev="+rev);
+    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc3/attachment.txt");
+    T(xhr.responseText == bin_data);
+    T(xhr.getResponseHeader("Content-Type") == "text/plain;charset=utf-8");
+
+    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc3/attachment.txt?rev=" + rev);
+    T(xhr.responseText == bin_data);
+    T(xhr.getResponseHeader("Content-Type") == "text/plain;charset=utf-8");
+
+    var xhr = CouchDB.request("DELETE", "/test_suite_db/bin_doc3/attachment.txt?rev=" + rev);
     T(xhr.status == 200);
+    
+    var xhr = CouchDB.request("GET", "/test_suite_db/bin_doc3/attachment.txt?rev=" + rev);
+    T(xhr.status == 404);
   },
 
   content_negotiation: function(debug) {
diff --git a/src/couchdb/couch_httpd.erl b/src/couchdb/couch_httpd.erl
index 5615ac3e..27d2d902 100644
--- a/src/couchdb/couch_httpd.erl
+++ b/src/couchdb/couch_httpd.erl
@@ -633,115 +633,61 @@ handle_attachment_request(Req, 'GET', _DbName, Db, DocId, FileName) ->
         throw(Error)
     end;
 
-handle_attachment_request(Req, 'DELETE', _DbName, Db, DocId, FileName) ->
-    QueryRev = proplists:get_value("rev", Req:parse_qs()),
-    Etag = case Req:get_header_value("If-Match") of
-        undefined ->
-            undefined;
-        Tag ->
-            string:strip(Tag, both, $")
+handle_attachment_request(Req, Method, _DbName, Db, DocId, FileName)
+    when (Method == 'PUT') or (Method == 'DELETE') ->
+
+    Rev = extract_header_rev(Req),
+    
+    NewAttachment = case Method of
+        'DELETE' ->
+            [];
+        _ ->
+            [{FileName, {
+                Req:get_header_value("Content-Type"),
+                Req:recv_body()
+            }}]
     end,
-    case {QueryRev, Etag} of
-    {undefined, undefined} ->
-        throw({missing_rev, "Document rev/etag must be specified to delete"});
-    {_, undefined} ->
-        QueryRev;
-    {undefined, _} ->
-        Etag;
-    _ when QueryRev == Etag ->
-        Etag;
+
+    Doc =
+    case Rev of
+    missing_rev -> % make the new doc
+        #doc{id=DocId};
     _ ->
-        throw({bad_request, "Document rev and etag have different values"})
+        case couch_db:open_doc(Db, DocId, []) of
+        {ok, Doc0}   -> Doc0#doc{revs=[Rev]};
+        Error       -> throw(Error)
+        end
     end,
     
-    case couch_db:open_doc(Db, DocId, []) of
-    {ok, Doc} ->
-        #doc{attachments=Attachments,revs=Revs} = Doc,
-        case proplists:get_value(FileName, Attachments) of
-        undefined ->
-            throw({not_found, missing});
-        {_Type, _Bin} ->
-
-            NewAttachmentList = proplists:delete(FileName, Attachments),
-
-            {ok, NewRev} = couch_db:update_doc(Db, Doc#doc{
-                id=DocId,
-                revs=Revs,
-                attachments=NewAttachmentList
-            }, []),
-            
-            send_json(Req, 200, {obj, [
-                {ok, true},
-                {id, DocId},
-                {rev, NewRev}
-            ]})
-        end;
-    Error ->
-        throw(Error)
-    end;   
-    
-handle_attachment_request(Req, 'PUT', _DbName, Db, DocId, FileName) ->
-    case couch_db:open_doc(Db, DocId, []) of
-    {ok, Doc} ->
-        #doc{attachments=Attachments,revs=OldRevs} = Doc,
-
-        NewAttachments = 
-        case proplists:get_value(FileName, Attachments) of
-            undefined -> % new attachment, just append to list
-                Revs = OldRevs,
-                lists:append(Attachments, [{FileName, {
-                    Req:get_header_value("Content-Type"),
-                    Req:recv_body()
-                }}]);
-
-            {_Type, _Bin} -> % update of an existing attachment, delete and re-add
-                QueryRev = proplists:get_value("rev", Req:parse_qs()),
-                Etag = case Req:get_header_value("If-Match") of
-                    undefined ->
-                        undefined;
-                    Tag ->
-                        string:strip(Tag, both, $")
-                end,
-                Revs = case {QueryRev, Etag} of
-                {undefined, undefined} ->
-                    throw({missing_rev, "Document rev/etag must be specified to delete"});
-                {_, undefined} ->
-                    [QueryRev];
-                {undefined, _} ->
-                    [Etag];
-                _ when QueryRev == Etag ->
-                    [Etag];
-                _ ->
-                    throw({bad_request, "Document rev and etag have different values"})
-                end,
-
-                lists:append(
-                    proplists:delete(FileName, Attachments),
-                    [{FileName, {
-                        Req:get_header_value("Content-Type"),
-                        Req:recv_body()
-                    }
-                }])
-        end,
-
-        {ok, NewRev} = couch_db:update_doc(Db, Doc#doc{
-            id=DocId,
-            revs=Revs,
-            attachments=NewAttachments
-        }, []),
-
-        send_json(Req, 201, {obj, [
-            {ok, true},
-            {id, DocId},
-            {rev, NewRev}
-        ]});
-    Error ->
-        throw(Error)
-    end;   
+    #doc{attachments=Attachments} = Doc,
+    DocEdited = Doc#doc{
+        attachments = NewAttachment ++ proplists:delete(FileName, Attachments)},
+    {ok, UpdatedRev} = couch_db:update_doc(Db, DocEdited, []),
+    send_json(Req, case Method of 'DELETE' -> 200; _ -> 201 end, {obj, [
+        {ok, true},
+        {id, DocId},
+        {rev, UpdatedRev}
+    ]});
 
 handle_attachment_request(_Req, _Method, _DbName, _Db, _DocId, _FileName) ->
     throw({method_not_allowed, "GET,HEAD,DELETE,PUT"}).
 
+
+extract_header_rev(Req) ->
+    QueryRev = proplists:get_value("rev", Req:parse_qs()),
+    Etag = case Req:get_header_value("If-Match") of
+        undefined -> undefined;
+        Tag -> string:strip(Tag, both, $")
+    end,
+    case {QueryRev, Etag} of
+    {undefined, undefined} -> missing_rev;
+    {_, undefined} -> QueryRev;
+    {undefined, _} -> Etag;
+    _ when QueryRev == Etag -> Etag;
+    _ ->
+        throw({bad_request, "Document rev and etag have different values"})
+    end.
+
 % View request handling internals
 
 reverse_key_default(nil) -> <<>>;