From c8a2194c5e0121d5876e7917990e3983961b9681 Mon Sep 17 00:00:00 2001 From: Tony Sun Date: Wed, 3 Jun 2015 13:44:29 -0400 Subject: [PATCH 1/2] Revert "Enable bulk deletes" This reverts commit 6490184bf36c6eb04587d5fb3bd86923e62ec2b5. COUCHDB-2651 --- src/mango_error.erl | 7 ---- src/mango_httpd.erl | 74 +++++++++++++------------------------- src/mango_idx.erl | 25 +------------ src/mango_opts.erl | 27 +------------- test/01-index-crud-test.py | 30 ---------------- test/mango.py | 9 ----- 6 files changed, 27 insertions(+), 145 deletions(-) diff --git a/src/mango_error.erl b/src/mango_error.erl index 6dcf7c8..babf564 100644 --- a/src/mango_error.erl +++ b/src/mango_error.erl @@ -110,13 +110,6 @@ info(mango_idx_view, {index_not_found, BadIdx}) -> fmt("JSON index ~s not found in this design doc.", [BadIdx]) }; -info(mango_opts, {invalid_bulk_docs, Val}) -> - { - 400, - <<"invalid_bulk_docs">>, - fmt("Bulk Delete requires an array of non-null docids. Docids: ~w", - [Val]) - }; info(mango_opts, {invalid_ejson, Val}) -> { 400, diff --git a/src/mango_httpd.erl b/src/mango_httpd.erl index 671ffd2..28a1578 100644 --- a/src/mango_httpd.erl +++ b/src/mango_httpd.erl @@ -65,7 +65,7 @@ handle_index_req(#httpd{method='POST', path_parts=[_, _]}=Req, Db) -> {ok, DDoc} -> <<"exists">>; {ok, NewDDoc} -> - CreateOpts = get_idx_w_opts(Opts), + CreateOpts = get_idx_create_opts(Opts), case mango_crud:insert(Db, NewDDoc, CreateOpts) of {ok, [{RespProps}]} -> case lists:keyfind(error, 1, RespProps) of @@ -80,28 +80,6 @@ handle_index_req(#httpd{method='POST', path_parts=[_, _]}=Req, Db) -> end, chttpd:send_json(Req, {[{result, Status}, {id, Id}, {name, Name}]}); -%% Essentially we just iterate through the list of ddoc ids passed in and -%% delete one by one. If an error occurs, all previous documents will be -%% deleted, but an error will be thrown for the current ddoc id. -handle_index_req(#httpd{method='POST', path_parts=[_, <<"_index">>, - <<"_bulk_delete">>]}=Req, Db) -> - {ok, Opts} = mango_opts:validate_bulk_delete(chttpd:json_body_obj(Req)), - Idxs = mango_idx:list(Db), - DDocs = get_bulk_delete_ddocs(Opts), - DelOpts = get_idx_w_opts(Opts), - {Success, Fail} = lists:foldl(fun(DDocId0, {Success0, Fail0}) -> - DDocId = convert_to_design_id(DDocId0), - Filt = fun(Idx) -> mango_idx:ddoc(Idx) == DDocId end, - Id = {<<"id">>, DDocId}, - case mango_idx:delete(Filt, Db, Idxs, DelOpts) of - {ok, true} -> - {[{[Id, {<<"ok">>, true}]} | Success0], Fail0}; - {error, Error} -> - {Success0, [{[Id, {<<"error">>, Error}]} | Fail0]} - end - end, {[], []}, DDocs), - chttpd:send_json(Req, {[{<<"success">>, Success}, {<<"fail">>, Fail}]}); - handle_index_req(#httpd{method='DELETE', path_parts=[A, B, <<"_design">>, DDocId0, Type, Name]}=Req, Db) -> PathParts = [A, B, <<"_design/", DDocId0/binary>>, Type, Name], @@ -109,22 +87,36 @@ handle_index_req(#httpd{method='DELETE', handle_index_req(#httpd{method='DELETE', path_parts=[_, _, DDocId0, Type, Name]}=Req, Db) -> + DDocId = case DDocId0 of + <<"_design/", _/binary>> -> DDocId0; + _ -> <<"_design/", DDocId0/binary>> + end, Idxs = mango_idx:list(Db), - DDocId = convert_to_design_id(DDocId0), - DelOpts = get_idx_del_opts(Req), Filt = fun(Idx) -> IsDDoc = mango_idx:ddoc(Idx) == DDocId, IsType = mango_idx:type(Idx) == Type, IsName = mango_idx:name(Idx) == Name, IsDDoc andalso IsType andalso IsName end, - case mango_idx:delete(Filt, Db, Idxs, DelOpts) of - {ok, true} -> - chttpd:send_json(Req, {[{ok, true}]}); - {error, not_found} -> - throw({not_found, missing}); - {error, Error} -> - ?MANGO_ERROR({error_saving_ddoc, Error}) + case lists:filter(Filt, Idxs) of + [Idx] -> + {ok, DDoc} = mango_util:load_ddoc(Db, mango_idx:ddoc(Idx)), + {ok, NewDDoc} = mango_idx:remove(DDoc, Idx), + FinalDDoc = case NewDDoc#doc.body of + {[{<<"language">>, <<"query">>}]} -> + NewDDoc#doc{deleted = true, body = {[]}}; + _ -> + NewDDoc + end, + DelOpts = get_idx_del_opts(Req), + case mango_crud:insert(Db, FinalDDoc, DelOpts) of + {ok, _} -> + chttpd:send_json(Req, {[{ok, true}]}); + _ -> + ?MANGO_ERROR(error_saving_ddoc) + end; + [] -> + throw({not_found, missing}) end; handle_index_req(Req, _Db) -> @@ -156,7 +148,7 @@ set_user_ctx(#httpd{user_ctx=Ctx}, Db) -> Db#db{user_ctx=Ctx}. -get_idx_w_opts(Opts) -> +get_idx_create_opts(Opts) -> case lists:keyfind(w, 1, Opts) of {w, N} when is_integer(N), N > 0 -> [{w, integer_to_list(N)}]; @@ -165,15 +157,6 @@ get_idx_w_opts(Opts) -> end. -get_bulk_delete_ddocs(Opts) -> - case lists:keyfind(docids, 1, Opts) of - {docids, DDocs} when is_list(DDocs) -> - DDocs; - _ -> - [] - end. - - get_idx_del_opts(Req) -> try WStr = chttpd:qs_value(Req, "w", "2"), @@ -184,13 +167,6 @@ get_idx_del_opts(Req) -> end. -convert_to_design_id(DDocId) -> - case DDocId of - <<"_design/", _/binary>> -> DDocId; - _ -> <<"_design/", DDocId/binary>> - end. - - start_find_resp(Req) -> chttpd:start_delayed_json_response(Req, 200, [], "{\"docs\":["). diff --git a/src/mango_idx.erl b/src/mango_idx.erl index bf81679..1c15894 100644 --- a/src/mango_idx.erl +++ b/src/mango_idx.erl @@ -41,8 +41,7 @@ end_key/2, cursor_mod/1, idx_mod/1, - to_json/1, - delete/4 + to_json/1 ]). @@ -135,28 +134,6 @@ remove(DDoc, Idx) -> {ok, NewDDoc#doc{body = Body}}. -delete(Filt, Db, Indexes, DelOpts) -> - case lists:filter(Filt, Indexes) of - [Idx] -> - {ok, DDoc} = mango_util:load_ddoc(Db, mango_idx:ddoc(Idx)), - {ok, NewDDoc} = mango_idx:remove(DDoc, Idx), - FinalDDoc = case NewDDoc#doc.body of - {[{<<"language">>, <<"query">>}]} -> - NewDDoc#doc{deleted = true, body = {[]}}; - _ -> - NewDDoc - end, - case mango_crud:insert(Db, FinalDDoc, DelOpts) of - {ok, _} -> - {ok, true}; - Error -> - {error, Error} - end; - [] -> - {error, not_found} - end. - - from_ddoc(Db, {Props}) -> DbName = db_to_name(Db), DDoc = proplists:get_value(<<"_id">>, Props), diff --git a/src/mango_opts.erl b/src/mango_opts.erl index 0c66a21..f7874a6 100644 --- a/src/mango_opts.erl +++ b/src/mango_opts.erl @@ -31,8 +31,7 @@ validate_use_index/1, validate_bookmark/1, validate_sort/1, - validate_fields/1, - validate_bulk_delete/1 + validate_fields/1 ]). @@ -130,22 +129,6 @@ validate_find({Props}) -> validate(Props, Opts). -validate_bulk_delete({Props}) -> - Opts = [ - {<<"docids">>, [ - {tag, docids}, - {validator, fun validate_bulk_docs/1} - ]}, - {<<"w">>, [ - {tag, w}, - {optional, true}, - {default, 2}, - {validator, fun is_pos_integer/1} - ]} - ], - validate(Props, Opts). - - validate(Props, Opts) -> case mango_util:assert_ejson({Props}) of true -> @@ -209,14 +192,6 @@ validate_selector(Else) -> ?MANGO_ERROR({invalid_selector_json, Else}). -%% We re-use validate_use_index to make sure the index names are valid -validate_bulk_docs(Docs) when is_list(Docs) -> - lists:foreach(fun validate_use_index/1, Docs), - {ok, Docs}; -validate_bulk_docs(Else) -> - ?MANGO_ERROR({invalid_bulk_docs, Else}). - - validate_use_index(IndexName) when is_binary(IndexName) -> case binary:split(IndexName, <<"/">>) of [DesignId] -> diff --git a/test/01-index-crud-test.py b/test/01-index-crud-test.py index a44376c..459566b 100644 --- a/test/01-index-crud-test.py +++ b/test/01-index-crud-test.py @@ -150,36 +150,6 @@ def test_delete_idx_no_design(self): post_indexes = self.db.list_indexes() assert pre_indexes == post_indexes - def test_bulk_delete(self): - fields = ["field1"] - ret = self.db.create_index(fields, name="idx_01") - assert ret is True - - fields = ["field2"] - ret = self.db.create_index(fields, name="idx_02") - assert ret is True - - fields = ["field3"] - ret = self.db.create_index(fields, name="idx_03") - assert ret is True - - docids = [] - - for idx in self.db.list_indexes(): - if idx["ddoc"] is not None: - docids.append(idx["ddoc"]) - - docids.append("_design/this_is_not_an_index_name") - - ret = self.db.bulk_delete(docids) - - assert ret["fail"][0]["id"] == "_design/this_is_not_an_index_name" - assert len(ret["success"]) == 3 - - for idx in self.db.list_indexes(): - assert idx["type"] != "json" - assert idx["type"] != "text" - def test_recreate_index(self): pre_indexes = self.db.list_indexes() for i in range(5): diff --git a/test/mango.py b/test/mango.py index b39c916..57dfffb 100644 --- a/test/mango.py +++ b/test/mango.py @@ -135,15 +135,6 @@ def delete_index(self, ddocid, name, idx_type="json"): r = self.sess.delete(self.path(path), params={"w":"3"}) r.raise_for_status() - def bulk_delete(self, docs): - body = { - "docids" : docs, - "w": 3 - } - body = json.dumps(body) - r = self.sess.post(self.path("_index/_bulk_delete"), data=body) - return r.json() - def find(self, selector, limit=25, skip=0, sort=None, fields=None, r=1, conflicts=False, use_index=None, explain=False, bookmark=None, return_raw=False): From 3a8efbee5b954a38fb221a9653eb1682987aa2a0 Mon Sep 17 00:00:00 2001 From: Tony Sun Date: Wed, 3 Jun 2015 15:53:21 -0400 Subject: [PATCH 2/2] Enable bulk delete with fabric update_docs Rather than delete each doc one by one, we use fabric:update_docs to bulk delete. Fixes:COUCHDB-2651 --- src/mango_crud.erl | 3 +- src/mango_httpd.erl | 17 +++++++++++ src/mango_idx.erl | 62 ++++++++++++++++++++++++++++++++++++++ src/mango_opts.erl | 26 +++++++++++++++- src/mango_util.erl | 3 ++ test/01-index-crud-test.py | 30 ++++++++++++++++++ test/mango.py | 9 ++++++ 7 files changed, 148 insertions(+), 2 deletions(-) diff --git a/src/mango_crud.erl b/src/mango_crud.erl index 68c9d6c..c9eb845 100644 --- a/src/mango_crud.erl +++ b/src/mango_crud.erl @@ -21,7 +21,8 @@ ]). -export([ - collect_cb/2 + collect_cb/2, + maybe_add_user_ctx/2 ]). diff --git a/src/mango_httpd.erl b/src/mango_httpd.erl index 28a1578..2fc2535 100644 --- a/src/mango_httpd.erl +++ b/src/mango_httpd.erl @@ -80,6 +80,14 @@ handle_index_req(#httpd{method='POST', path_parts=[_, _]}=Req, Db) -> end, chttpd:send_json(Req, {[{result, Status}, {id, Id}, {name, Name}]}); +handle_index_req(#httpd{method='POST', path_parts=[_, <<"_index">>, + <<"_bulk_delete">>]}=Req, Db) -> + {ok, Opts} = mango_opts:validate_bulk_delete(chttpd:json_body_obj(Req)), + DDocIds = get_bulk_delete_ddocs_ids(Opts), + DelOpts = get_idx_create_opts(Opts), + {Success, Error} = mango_idx:bulk_delete(Db, DDocIds, DelOpts), + chttpd:send_json(Req, {[{<<"success">>, Success}, {<<"error">>, Error}]}); + handle_index_req(#httpd{method='DELETE', path_parts=[A, B, <<"_design">>, DDocId0, Type, Name]}=Req, Db) -> PathParts = [A, B, <<"_design/", DDocId0/binary>>, Type, Name], @@ -157,6 +165,15 @@ get_idx_create_opts(Opts) -> end. +get_bulk_delete_ddocs_ids(Opts) -> + case lists:keyfind(docids, 1, Opts) of + {docids, DDocs} when is_list(DDocs) -> + DDocs; + _ -> + [] + end. + + get_idx_del_opts(Req) -> try WStr = chttpd:qs_value(Req, "w", "2"), diff --git a/src/mango_idx.erl b/src/mango_idx.erl index 1c15894..31d5e7d 100644 --- a/src/mango_idx.erl +++ b/src/mango_idx.erl @@ -26,6 +26,7 @@ validate/1, add/2, remove/2, + bulk_delete/3, from_ddoc/2, special/1, @@ -134,6 +135,67 @@ remove(DDoc, Idx) -> {ok, NewDDoc#doc{body = Body}}. +bulk_delete(Db, DDocIds, DelOpts0) -> + DelOpts = mango_crud:maybe_add_user_ctx(Db, DelOpts0), + {DeleteDocs, Errors} = lists:foldl(fun(DDocId0, {D, E}) -> + Id = {<<"id">>, DDocId0}, + case get_bulk_delete_ddoc(Db, DDocId0) of + not_found -> + {D, [{[Id, {<<"error">>, <<"does not exist">>}]} | E]}; + invalid_ddoc_lang -> + {D, [{[Id, {<<"error">>, <<"not a query doc">>}]} | E]}; + error_loading_doc -> + {D, [{[Id, {<<"error">>, <<"loading doc">>}]} | E]}; + DDoc -> + {[DDoc#doc{deleted = true, body = {[]}} | D], E } + end + end, {[], []}, DDocIds), + case fabric:update_docs(Db, DeleteDocs, DelOpts) of + {ok, Results} -> + bulk_delete_results(lists:zip(DeleteDocs, Results), Errors); + {accepted, Results} -> + bulk_delete_results(lists:zip(DeleteDocs, Results), Errors); + {aborted, Abort} -> + bulk_delete_results(lists:zip(DeleteDocs, Abort), Errors) + end. + + +bulk_delete_results(DeleteResults, LoadErrors) -> + {Success, Errors} = lists:foldl(fun({#doc{id=DDocId}, Result}, {S, E}) -> + Id = {<<"id">>, DDocId}, + case Result of + {_, {_Pos, _}} -> + {[{[Id, {<<"ok">>, true}]} | S], E}; + {{_Id, _Rev}, Error} -> + {_Code, ErrorStr, _Reason} = chttpd:error_info(Error), + {S, [{[Id, {<<"error">>, ErrorStr}]} | E]}; + Error -> + {_Code, ErrorStr, _Reason} = chttpd:error_info(Error), + {S, [{[Id, {<<"error">>, ErrorStr}]} | E]} + end + end, {[], []}, DeleteResults), + {Success, Errors ++ LoadErrors}. + + +get_bulk_delete_ddoc(Db, Id0) -> + Id = case Id0 of + <<"_design/", _/binary>> -> Id0; + _ -> <<"_design/", Id0/binary>> + end, + try mango_util:open_doc(Db, Id, [deleted, ejson_body]) of + {ok, #doc{deleted = false, body=Body} = Doc} -> + mango_util:check_lang(Doc), + Doc; + not_found -> + not_found + catch + {{mango_error, mango_util, {invalid_ddoc_lang, _}}} -> + invalid_ddoc_lang; + {{mango_error, mango_util, {error_loading_doc, _}}} -> + error_loading_doc + end. + + from_ddoc(Db, {Props}) -> DbName = db_to_name(Db), DDoc = proplists:get_value(<<"_id">>, Props), diff --git a/src/mango_opts.erl b/src/mango_opts.erl index f7874a6..d7af601 100644 --- a/src/mango_opts.erl +++ b/src/mango_opts.erl @@ -14,7 +14,8 @@ -export([ validate_idx_create/1, - validate_find/1 + validate_find/1, + validate_bulk_delete/1 ]). -export([ @@ -129,6 +130,22 @@ validate_find({Props}) -> validate(Props, Opts). +validate_bulk_delete({Props}) -> + Opts = [ + {<<"docids">>, [ + {tag, docids}, + {validator, fun validate_bulk_docs/1} + ]}, + {<<"w">>, [ + {tag, w}, + {optional, true}, + {default, 2}, + {validator, fun is_pos_integer/1} + ]} + ], + validate(Props, Opts). + + validate(Props, Opts) -> case mango_util:assert_ejson({Props}) of true -> @@ -192,6 +209,13 @@ validate_selector(Else) -> ?MANGO_ERROR({invalid_selector_json, Else}). +validate_bulk_docs(Docs) when is_list(Docs) -> + lists:foreach(fun ?MODULE:is_string/1, Docs), + {ok, Docs}; +validate_bulk_docs(Else) -> + ?MANGO_ERROR({invalid_bulk_docs, Else}). + + validate_use_index(IndexName) when is_binary(IndexName) -> case binary:split(IndexName, <<"/">>) of [DesignId] -> diff --git a/src/mango_util.erl b/src/mango_util.erl index 99e15d5..a48a7fb 100644 --- a/src/mango_util.erl +++ b/src/mango_util.erl @@ -15,6 +15,7 @@ -export([ open_doc/2, + open_doc/3, open_ddocs/1, load_ddoc/2, @@ -37,6 +38,8 @@ has_suffix/2, + check_lang/1, + join/2 ]). diff --git a/test/01-index-crud-test.py b/test/01-index-crud-test.py index 459566b..0a7f595 100644 --- a/test/01-index-crud-test.py +++ b/test/01-index-crud-test.py @@ -150,6 +150,36 @@ def test_delete_idx_no_design(self): post_indexes = self.db.list_indexes() assert pre_indexes == post_indexes + def test_bulk_delete(self): + fields = ["field1"] + ret = self.db.create_index(fields, name="idx_01") + assert ret is True + + fields = ["field2"] + ret = self.db.create_index(fields, name="idx_02") + assert ret is True + + fields = ["field3"] + ret = self.db.create_index(fields, name="idx_03") + assert ret is True + + docids = [] + + for idx in self.db.list_indexes(): + if idx["ddoc"] is not None: + docids.append(idx["ddoc"]) + + docids.append("_design/this_is_not_an_index_name") + + ret = self.db.bulk_delete(docids) + print ret + assert ret["error"][0]["id"] == "_design/this_is_not_an_index_name" + assert len(ret["success"]) == 3 + + for idx in self.db.list_indexes(): + assert idx["type"] != "json" + assert idx["type"] != "text" + def test_recreate_index(self): pre_indexes = self.db.list_indexes() for i in range(5): diff --git a/test/mango.py b/test/mango.py index 57dfffb..b39c916 100644 --- a/test/mango.py +++ b/test/mango.py @@ -135,6 +135,15 @@ def delete_index(self, ddocid, name, idx_type="json"): r = self.sess.delete(self.path(path), params={"w":"3"}) r.raise_for_status() + def bulk_delete(self, docs): + body = { + "docids" : docs, + "w": 3 + } + body = json.dumps(body) + r = self.sess.post(self.path("_index/_bulk_delete"), data=body) + return r.json() + def find(self, selector, limit=25, skip=0, sort=None, fields=None, r=1, conflicts=False, use_index=None, explain=False, bookmark=None, return_raw=False):