summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Vatamaniuc <vatamane@gmail.com>2022-06-05 00:04:47 -0400
committerNick Vatamaniuc <nickva@users.noreply.github.com>2022-06-07 11:37:46 -0400
commit14a5213470f9372b2d97af0bfc214f438d059132 (patch)
treeabbb028a1cf3e34a88b7337bfc44467932ad25cf
parent369ecc98115f60ee6cabb4dac246dcb6817c56a6 (diff)
downloadcouchdb-14a5213470f9372b2d97af0bfc214f438d059132.tar.gz
Remove duplicate _revs_diff implementation
The implementation was duplicated because fabric needed to return a response ([]) for existing revs. It doesn't seem worth it to keep a whole separate copy just to handle the [] case. Instead, make the primary couchdb implementation always return [] and just filter it out for the local httpd endpoint handlers. It seems we didn't have any _revs_diff tests, so add a few basic tests along the way. Also, it turns out we still support the _missing_revs API. That's an endpoint that's not used by the replicator since before 1.2 days. At some point it might be worth removing it, but for now ensure we also test it alongside _revs_diff.
-rw-r--r--src/chttpd/test/eunit/chttpd_revs_diff_tests.erl238
-rw-r--r--src/couch/src/couch_db.erl87
-rw-r--r--src/couch/src/couch_httpd_db.erl14
-rw-r--r--src/fabric/src/fabric_rpc.erl53
4 files changed, 290 insertions, 102 deletions
diff --git a/src/chttpd/test/eunit/chttpd_revs_diff_tests.erl b/src/chttpd/test/eunit/chttpd_revs_diff_tests.erl
new file mode 100644
index 000000000..9a9bd25b7
--- /dev/null
+++ b/src/chttpd/test/eunit/chttpd_revs_diff_tests.erl
@@ -0,0 +1,238 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_revs_diff_tests).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+
+-define(TDEF_FE(Name), fun(Arg) -> {atom_to_list(Name), ?_test(Name(Arg))} end).
+
+-define(USER, "chttpd_revs_diff_test_admin").
+-define(PASS, "pass").
+-define(AUTH, {basic_auth, {?USER, ?PASS}}).
+-define(JSON, {"Content-Type", "application/json"}).
+
+-define(DOC1, <<"doc1">>).
+-define(DOC2, <<"doc2">>).
+-define(REVA, <<"reva">>).
+-define(REVB, <<"revb">>).
+-define(REVC, <<"revc">>).
+-define(REVD, <<"revd">>).
+
+test_docs() ->
+ [
+ {?DOC1, [?REVB, ?REVA]},
+ {?DOC1, [?REVC, ?REVA]},
+ {?DOC2, [?REVD]}
+ ].
+
+setup() ->
+ Hashed = couch_passwords:hash_admin_password(?PASS),
+ ok = config:set("admins", ?USER, ?b2l(Hashed), _Persist = false),
+ Addr = config:get("chttpd", "bind_address", "127.0.0.1"),
+ Db = binary_to_list(?tempdb()),
+ Port = mochiweb_socket_server:get(chttpd, port),
+ Url = lists:concat(["http://", Addr, ":", Port, "/"]),
+ ok = create_db(Url, Db),
+ ok = create_docs(Url, Db, test_docs()),
+ {Url, Db}.
+
+teardown({Url, Db}) ->
+ delete_db(Url, Db),
+ ok = config:delete("admins", ?USER, _Persist = false).
+
+start_couch() ->
+ test_util:start_couch([chttpd]).
+
+stop_couch(Ctx) ->
+ test_util:stop_couch(Ctx).
+
+chttpd_revs_diff_test_() ->
+ {
+ "chttpd _revs_diff tests",
+ {
+ setup,
+ fun start_couch/0,
+ fun stop_couch/1,
+ {
+ foreach,
+ fun setup/0,
+ fun teardown/1,
+ [
+ ?TDEF_FE(t_empty_revs_diff),
+ ?TDEF_FE(t_revs_diff_no_revs),
+ ?TDEF_FE(t_revs_diff_non_existent_doc),
+ ?TDEF_FE(t_revs_diff_all_revs),
+ ?TDEF_FE(t_revs_diff_some_missing_some_not),
+ ?TDEF_FE(t_empty_missing_revs),
+ ?TDEF_FE(t_missing_revs_no_revs),
+ ?TDEF_FE(t_missing_revs_non_existent_doc),
+ ?TDEF_FE(t_missing_revs_all_revs),
+ ?TDEF_FE(t_missing_revs_some_missing_some_not)
+ ]
+ }
+ }
+ }.
+
+t_empty_revs_diff({Top, Db}) ->
+ {Code, Res} = req(post, Top ++ Db ++ "/_revs_diff", #{}),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{}, Res).
+
+t_revs_diff_no_revs({Top, Db}) ->
+ Body = #{?DOC1 => [], <<"non_existent_doc">> => []},
+ {Code, Res} = req(post, Top ++ Db ++ "/_revs_diff", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{}, Res).
+
+t_revs_diff_non_existent_doc({Top, Db}) ->
+ Body = #{<<"non_existent_doc">> => [<<"1-rev">>]},
+ {Code, Res} = req(post, Top ++ Db ++ "/_revs_diff", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(
+ #{
+ <<"non_existent_doc">> => #{
+ <<"missing">> => [<<"1-rev">>]
+ }
+ },
+ Res
+ ).
+
+t_revs_diff_all_revs({Top, Db}) ->
+ Body = #{
+ ?DOC1 => [<<"2-", ?REVB/binary>>, <<"2-", ?REVC/binary>>],
+ ?DOC2 => [<<"1-", ?REVD/binary>>]
+ },
+ {Code, Res} = req(post, Top ++ Db ++ "/_revs_diff", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{}, Res).
+
+t_revs_diff_some_missing_some_not({Top, Db}) ->
+ Body = #{
+ ?DOC1 => [<<"2-", ?REVB/binary>>, <<"1-xyz">>, <<"2-def">>, <<"3-klm">>],
+ ?DOC2 => [<<"1-pqr">>]
+ },
+ {Code, Res} = req(post, Top ++ Db ++ "/_revs_diff", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(
+ #{
+ ?DOC1 => #{
+ <<"missing">> => [<<"1-xyz">>, <<"2-def">>, <<"3-klm">>],
+ <<"possible_ancestors">> => [<<"2-revb">>, <<"2-revc">>]
+ },
+ ?DOC2 => #{
+ <<"missing">> => [<<"1-pqr">>]
+ }
+ },
+ Res
+ ).
+
+t_empty_missing_revs({Top, Db}) ->
+ {Code, Res} = req(post, Top ++ Db ++ "/_missing_revs", #{}),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{<<"missing_revs">> => #{}}, Res).
+
+t_missing_revs_no_revs({Top, Db}) ->
+ Body = #{?DOC1 => [], <<"non_existent_doc">> => []},
+ {Code, Res} = req(post, Top ++ Db ++ "/_missing_revs", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{<<"missing_revs">> => #{}}, Res).
+
+t_missing_revs_non_existent_doc({Top, Db}) ->
+ Body = #{<<"non_existent_doc">> => [<<"1-rev">>]},
+ {Code, Res} = req(post, Top ++ Db ++ "/_missing_revs", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(
+ #{
+ <<"missing_revs">> => #{
+ <<"non_existent_doc">> => [<<"1-rev">>]
+ }
+ },
+ Res
+ ).
+
+t_missing_revs_all_revs({Top, Db}) ->
+ Body = #{
+ ?DOC1 => [<<"2-", ?REVB/binary>>, <<"2-", ?REVC/binary>>],
+ ?DOC2 => [<<"1-", ?REVD/binary>>]
+ },
+ {Code, Res} = req(post, Top ++ Db ++ "/_missing_revs", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(#{<<"missing_revs">> => #{}}, Res).
+
+t_missing_revs_some_missing_some_not({Top, Db}) ->
+ Body = #{
+ ?DOC1 => [<<"2-", ?REVB/binary>>, <<"1-xyz">>, <<"2-def">>, <<"3-klm">>],
+ ?DOC2 => [<<"1-pqr">>]
+ },
+ {Code, Res} = req(post, Top ++ Db ++ "/_missing_revs", Body),
+ ?assertEqual(200, Code),
+ ?assertEqual(
+ #{
+ <<"missing_revs">> => #{
+ ?DOC1 => [<<"1-xyz">>, <<"2-def">>, <<"3-klm">>],
+ ?DOC2 => [<<"1-pqr">>]
+ }
+ },
+ Res
+ ).
+
+create_db(Top, Db) ->
+ case req(put, Top ++ Db) of
+ {201, #{}} ->
+ ok;
+ Error ->
+ error({failed_to_create_test_db, Db, Error})
+ end.
+
+delete_db(Top, Db) ->
+ case req(delete, Top ++ Db) of
+ {200, #{}} ->
+ ok;
+ Error ->
+ error({failed_to_delete_test_db, Db, Error})
+ end.
+
+create_docs(Top, Db, DocRevs) ->
+ Docs = lists:map(
+ fun({Id, Revs}) ->
+ #{
+ <<"_id">> => Id,
+ <<"_revisions">> => #{
+ <<"ids">> => Revs,
+ <<"start">> => length(Revs)
+ }
+ }
+ end,
+ DocRevs
+ ),
+ Body = #{
+ <<"docs">> => Docs,
+ <<"new_edits">> => false
+ },
+ {Code, Res} = req(post, Top ++ Db ++ "/_bulk_docs", Body),
+ ?assertEqual(201, Code),
+ ?assertEqual([], Res),
+ ok.
+
+req(Method, Url) ->
+ Headers = [?JSON, ?AUTH],
+ {ok, Code, _, Res} = test_request:request(Method, Url, Headers),
+ {Code, jiffy:decode(Res, [return_maps])}.
+
+req(Method, Url, #{} = Body) ->
+ req(Method, Url, jiffy:encode(Body));
+req(Method, Url, Body) ->
+ Headers = [?JSON, ?AUTH],
+ {ok, Code, _, Res} = test_request:request(Method, Url, Headers, Body),
+ {Code, jiffy:decode(Res, [return_maps])}.
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index 473b7853e..572746229 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -354,52 +354,21 @@ open_doc_revs(Db, Id, Revs, Options) ->
% Each returned result is a list of tuples:
% {Id, MissingRevs, PossibleAncestors}
-% if no revs are missing, it's omitted from the results.
+% if no revs are missing, MissingRevs is []
get_missing_revs(Db, IdRevsList) ->
- Results = get_full_doc_infos(Db, [Id1 || {Id1, _Revs} <- IdRevsList]),
- {ok, find_missing(IdRevsList, Results)}.
-
-find_missing([], []) ->
- [];
-find_missing([{Id, Revs} | RestIdRevs], [FullInfo | RestLookupInfo]) when
- is_record(FullInfo, full_doc_info)
-->
- case couch_key_tree:find_missing(FullInfo#full_doc_info.rev_tree, Revs) of
- [] ->
- find_missing(RestIdRevs, RestLookupInfo);
- MissingRevs ->
- #doc_info{revs = RevsInfo} = couch_doc:to_doc_info(FullInfo),
- LeafRevs = [Rev || #rev_info{rev = Rev} <- RevsInfo],
- % Find the revs that are possible parents of this rev
- PossibleAncestors =
- lists:foldl(
- fun({LeafPos, LeafRevId}, Acc) ->
- % this leaf is a "possible ancenstor" of the missing
- % revs if this LeafPos lessthan any of the missing revs
- case
- lists:any(
- fun({MissingPos, _}) ->
- LeafPos < MissingPos
- end,
- MissingRevs
- )
- of
- true ->
- [{LeafPos, LeafRevId} | Acc];
- false ->
- Acc
- end
- end,
- [],
- LeafRevs
- ),
- [
- {Id, MissingRevs, PossibleAncestors}
- | find_missing(RestIdRevs, RestLookupInfo)
- ]
- end;
-find_missing([{Id, Revs} | RestIdRevs], [not_found | RestLookupInfo]) ->
- [{Id, Revs, []} | find_missing(RestIdRevs, RestLookupInfo)].
+ FDIs = get_full_doc_infos(Db, [Id || {Id, _Revs} <- IdRevsList]),
+ Results = lists:zipwith(
+ fun
+ ({Id, Revs}, #full_doc_info{rev_tree = RevTree} = FDI) ->
+ MissingRevs = couch_key_tree:find_missing(RevTree, Revs),
+ {Id, MissingRevs, possible_ancestors(FDI, MissingRevs)};
+ ({Id, Revs}, not_found) ->
+ {Id, Revs, []}
+ end,
+ IdRevsList,
+ FDIs
+ ),
+ {ok, Results}.
get_doc_info(Db, Id) ->
case get_full_doc_info(Db, Id) of
@@ -2128,6 +2097,34 @@ set_design_doc_end_key(Options, rev) ->
lists:keystore(end_key_gt, 1, Options, {end_key_gt, Key2})
end.
+possible_ancestors(_FullInfo, []) ->
+ [];
+possible_ancestors(FullInfo, MissingRevs) ->
+ #doc_info{revs = RevsInfo} = couch_doc:to_doc_info(FullInfo),
+ LeafRevs = [Rev || #rev_info{rev = Rev} <- RevsInfo],
+ % Find the revs that are possible parents of this rev
+ lists:foldl(
+ fun({LeafPos, LeafRevId}, Acc) ->
+ % this leaf is a "possible ancenstor" of the missing
+ % revs if this LeafPos lessthan any of the missing revs
+ case
+ lists:any(
+ fun({MissingPos, _}) ->
+ LeafPos < MissingPos
+ end,
+ MissingRevs
+ )
+ of
+ true ->
+ [{LeafPos, LeafRevId} | Acc];
+ false ->
+ Acc
+ end
+ end,
+ [],
+ LeafRevs
+ ).
+
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
diff --git a/src/couch/src/couch_httpd_db.erl b/src/couch/src/couch_httpd_db.erl
index e82eea7f7..e33a52b36 100644
--- a/src/couch/src/couch_httpd_db.erl
+++ b/src/couch/src/couch_httpd_db.erl
@@ -451,11 +451,13 @@ db_req(#httpd{method = 'POST', path_parts = [_, <<"_missing_revs">>]} = Req, Db)
|| {Id, RevStrs} <- JsonDocIdRevs
],
{ok, Results} = couch_db:get_missing_revs(Db, JsonDocIdRevs2),
- Results2 = [{Id, couch_doc:revs_to_strs(Revs)} || {Id, Revs, _} <- Results],
+ % Skip docs with no missing revs, they have Revs = []
+ Results2 = [{Id, Revs} || {Id, Revs, _} <- Results, Revs =/= []],
+ Results3 = [{Id, couch_doc:revs_to_strs(Revs)} || {Id, Revs} <- Results2],
send_json(
Req,
{[
- {missing_revs, {Results2}}
+ {missing_revs, {Results3}}
]}
);
db_req(#httpd{path_parts = [_, <<"_missing_revs">>]} = Req, _Db) ->
@@ -466,7 +468,9 @@ db_req(#httpd{method = 'POST', path_parts = [_, <<"_revs_diff">>]} = Req, Db) ->
JsonDocIdRevs2 =
[{Id, couch_doc:parse_revs(RevStrs)} || {Id, RevStrs} <- JsonDocIdRevs],
{ok, Results} = couch_db:get_missing_revs(Db, JsonDocIdRevs2),
- Results2 =
+ % Skip docs with no missing revs, they have Revs = []
+ Results2 = [Res || {_Id, Revs, _PAs} = Res <- Results, Revs =/= []],
+ Results3 =
lists:map(
fun({Id, MissingRevs, PossibleAncestors}) ->
{Id, {
@@ -479,9 +483,9 @@ db_req(#httpd{method = 'POST', path_parts = [_, <<"_revs_diff">>]} = Req, Db) ->
end
}}
end,
- Results
+ Results2
),
- send_json(Req, {Results2});
+ send_json(Req, {Results3});
db_req(#httpd{path_parts = [_, <<"_revs_diff">>]} = Req, _Db) ->
send_method_not_allowed(Req, "POST");
db_req(#httpd{method = 'PUT', path_parts = [_, <<"_security">>]} = Req, Db) ->
diff --git a/src/fabric/src/fabric_rpc.erl b/src/fabric/src/fabric_rpc.erl
index cfda1a37e..8780737ef 100644
--- a/src/fabric/src/fabric_rpc.erl
+++ b/src/fabric/src/fabric_rpc.erl
@@ -283,30 +283,7 @@ get_missing_revs(DbName, IdRevsList) ->
get_missing_revs(DbName, IdRevsList, []).
get_missing_revs(DbName, IdRevsList, Options) ->
- % reimplement here so we get [] for Ids with no missing revs in response
- set_io_priority(DbName, Options),
- rexi:reply(
- case get_or_create_db(DbName, Options) of
- {ok, Db} ->
- Ids = [Id1 || {Id1, _Revs} <- IdRevsList],
- {ok,
- lists:zipwith(
- fun({Id, Revs}, FullDocInfoResult) ->
- case FullDocInfoResult of
- #full_doc_info{rev_tree = RevisionTree} = FullInfo ->
- MissingRevs = couch_key_tree:find_missing(RevisionTree, Revs),
- {Id, MissingRevs, possible_ancestors(FullInfo, MissingRevs)};
- not_found ->
- {Id, Revs, []}
- end
- end,
- IdRevsList,
- couch_db:get_full_doc_infos(Db, Ids)
- )};
- Error ->
- Error
- end
- ).
+ with_db(DbName, Options, {couch_db, get_missing_revs, [IdRevsList]}).
update_docs(DbName, Docs0, Options) ->
{Docs1, Type} =
@@ -601,34 +578,6 @@ maybe_filtered_json_doc(Doc, Opts, {selector, _Style, {_Selector, Fields}}) when
maybe_filtered_json_doc(Doc, Opts, _Filter) ->
couch_doc:to_json_obj(Doc, Opts).
-possible_ancestors(_FullInfo, []) ->
- [];
-possible_ancestors(FullInfo, MissingRevs) ->
- #doc_info{revs = RevsInfo} = couch_doc:to_doc_info(FullInfo),
- LeafRevs = [Rev || #rev_info{rev = Rev} <- RevsInfo],
- % Find the revs that are possible parents of this rev
- lists:foldl(
- fun({LeafPos, LeafRevId}, Acc) ->
- % this leaf is a "possible ancenstor" of the missing
- % revs if this LeafPos lessthan any of the missing revs
- case
- lists:any(
- fun({MissingPos, _}) ->
- LeafPos < MissingPos
- end,
- MissingRevs
- )
- of
- true ->
- [{LeafPos, LeafRevId} | Acc];
- false ->
- Acc
- end
- end,
- [],
- LeafRevs
- ).
-
make_att_readers([]) ->
[];
make_att_readers([#doc{atts = Atts0} = Doc | Rest]) ->