diff options
author | Filipe David Borba Manana <fdmanana@apache.org> | 2012-01-06 12:22:29 +0000 |
---|---|---|
committer | Filipe David Borba Manana <fdmanana@apache.org> | 2012-01-07 12:46:04 +0000 |
commit | 6d1d23b7d0eba4de3c4097907adc37b09191196e (patch) | |
tree | ba2c4751017013c832de77db388c836dee851266 | |
parent | b64643a3ee4a3148f9d70fa38ea5a8b0e55650a1 (diff) | |
download | couchdb-6d1d23b7d0eba4de3c4097907adc37b09191196e.tar.gz |
Fix document deletion followed by creation
If the updater collects 2 updates for the same document
where the first deletes it and the second creates it, it
would send a conflict error to the client of the create
request. This wouldn't happen if the 2 requests were
collected in 2 different batches by the updater.
Closes COUCHDB-188
-rw-r--r-- | src/couchdb/couch_db_updater.erl | 22 | ||||
-rwxr-xr-x | test/etap/074-doc-update-conflicts.t | 62 |
2 files changed, 72 insertions, 12 deletions
diff --git a/src/couchdb/couch_db_updater.erl b/src/couchdb/couch_db_updater.erl index e57f05bcc..0b890fe2c 100644 --- a/src/couchdb/couch_db_updater.erl +++ b/src/couchdb/couch_db_updater.erl @@ -576,16 +576,16 @@ merge_rev_trees(_Limit, _Merge, [], [], AccNewInfos, AccRemoveSeqs, AccSeq) -> {ok, lists:reverse(AccNewInfos), AccRemoveSeqs, AccSeq}; merge_rev_trees(Limit, MergeConflicts, [NewDocs|RestDocsList], [OldDocInfo|RestOldInfo], AccNewInfos, AccRemoveSeqs, AccSeq) -> - #full_doc_info{id=Id,rev_tree=OldTree,deleted=OldDeleted,update_seq=OldSeq} + #full_doc_info{id=Id,rev_tree=OldTree,deleted=OldDeleted0,update_seq=OldSeq} = OldDocInfo, - NewRevTree = lists:foldl( - fun({Client, {#doc{revs={Pos,[_Rev|PrevRevs]}}=NewDoc, Ref}}, AccTree) -> + {NewRevTree, _} = lists:foldl( + fun({Client, {#doc{revs={Pos,[_Rev|PrevRevs]}}=NewDoc, Ref}}, {AccTree, OldDeleted}) -> if not MergeConflicts -> case couch_key_tree:merge(AccTree, couch_doc:to_path(NewDoc), Limit) of {_NewTree, conflicts} when (not OldDeleted) -> send_result(Client, Ref, conflict), - AccTree; + {AccTree, OldDeleted}; {NewTree, conflicts} when PrevRevs /= [] -> % Check to be sure if prev revision was specified, it's % a leaf node in the tree @@ -594,10 +594,10 @@ merge_rev_trees(Limit, MergeConflicts, [NewDocs|RestDocsList], {LeafPos, LeafRevId} == {Pos-1, hd(PrevRevs)} end, Leafs), if IsPrevLeaf -> - NewTree; + {NewTree, OldDeleted}; true -> send_result(Client, Ref, conflict), - AccTree + {NewTree, OldDeleted} end; {NewTree, no_conflicts} when AccTree == NewTree -> % the tree didn't change at all @@ -616,21 +616,21 @@ merge_rev_trees(Limit, MergeConflicts, [NewDocs|RestDocsList], couch_doc:to_path(NewDoc2), Limit), % we changed the rev id, this tells the caller we did send_result(Client, Ref, {ok, {OldPos + 1, NewRevId}}), - NewTree2; + {NewTree2, OldDeleted}; true -> send_result(Client, Ref, conflict), - AccTree + {NewTree, OldDeleted} end; {NewTree, _} -> - NewTree + {NewTree, NewDoc#doc.deleted} end; true -> {NewTree, _} = couch_key_tree:merge(AccTree, couch_doc:to_path(NewDoc), Limit), - NewTree + {NewTree, OldDeleted} end end, - OldTree, NewDocs), + {OldTree, OldDeleted0}, NewDocs), if NewRevTree == OldTree -> % nothing changed merge_rev_trees(Limit, MergeConflicts, RestDocsList, RestOldInfo, diff --git a/test/etap/074-doc-update-conflicts.t b/test/etap/074-doc-update-conflicts.t index 7c2f80d82..09d063313 100755 --- a/test/etap/074-doc-update-conflicts.t +++ b/test/etap/074-doc-update-conflicts.t @@ -26,7 +26,7 @@ test_db_name() -> <<"couch_test_update_conflicts">>. main(_) -> test_util:init_code_path(), - etap:plan(25), + etap:plan(35), case (catch test()) of ok -> etap:end_tests(); @@ -45,6 +45,8 @@ test() -> fun(NumClients) -> test_concurrent_doc_update(NumClients) end, [100, 500, 1000, 2000, 5000]), + test_bulk_delete_create(), + couch_server_sup:stop(), ok. @@ -132,6 +134,64 @@ test_concurrent_doc_update(NumClients) -> delete_db(Db3). +% COUCHDB-188 +test_bulk_delete_create() -> + {ok, Db} = create_db(test_db_name()), + Doc = couch_doc:from_json_obj({[ + {<<"_id">>, <<"foobar">>}, + {<<"value">>, 0} + ]}), + {ok, Rev} = couch_db:update_doc(Db, Doc, []), + + DeletedDoc = couch_doc:from_json_obj({[ + {<<"_id">>, <<"foobar">>}, + {<<"_rev">>, couch_doc:rev_to_str(Rev)}, + {<<"_deleted">>, true} + ]}), + NewDoc = couch_doc:from_json_obj({[ + {<<"_id">>, <<"foobar">>}, + {<<"value">>, 666} + ]}), + + {ok, Results} = couch_db:update_docs(Db, [DeletedDoc, NewDoc], []), + ok = couch_db:close(Db), + + etap:is(length([ok || {ok, _} <- Results]), 2, + "Deleted and non-deleted versions got an ok reply"), + + [{ok, Rev1}, {ok, Rev2}] = Results, + {ok, Db2} = couch_db:open_int(test_db_name(), []), + + {ok, [{ok, Doc1}]} = couch_db:open_doc_revs( + Db2, <<"foobar">>, [Rev1], [conflicts, deleted_conflicts]), + {ok, [{ok, Doc2}]} = couch_db:open_doc_revs( + Db2, <<"foobar">>, [Rev2], [conflicts, deleted_conflicts]), + ok = couch_db:close(Db2), + + {Doc1Props} = couch_doc:to_json_obj(Doc1, []), + {Doc2Props} = couch_doc:to_json_obj(Doc2, []), + + etap:is(couch_util:get_value(<<"_deleted">>, Doc1Props), true, + "Document was deleted"), + etap:is(couch_util:get_value(<<"_deleted">>, Doc2Props), undefined, + "New document not flagged as deleted"), + etap:is(couch_util:get_value(<<"value">>, Doc2Props), 666, + "New leaf revision has the right value"), + etap:is(couch_util:get_value(<<"_conflicts">>, Doc1Props), undefined, + "Deleted document has no conflicts"), + etap:is(couch_util:get_value(<<"_deleted_conflicts">>, Doc1Props), undefined, + "Deleted document has no deleted conflicts"), + etap:is(couch_util:get_value(<<"_conflicts">>, Doc2Props), undefined, + "New leaf revision doesn't have conflicts"), + etap:is(couch_util:get_value(<<"_deleted_conflicts">>, Doc2Props), undefined, + "New leaf revision doesn't have deleted conflicts"), + + etap:is(element(1, Rev1), 2, "Deleted revision has position 2"), + etap:is(element(1, Rev2), 1, "New leaf revision has position 1"), + + delete_db(Db2). + + spawn_client(Doc) -> spawn(fun() -> {ok, Db} = couch_db:open_int(test_db_name(), []), |