chore: address various rerview notes by @rnewson
diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index ac93e02..890d819 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -404,7 +404,7 @@
; Per document access settings
[per_doc_access]
-;enabled = false
+;enable = false
; CSP (Content Security Policy) Support
[csp]
diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 148ca98..6911b5e 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -961,14 +961,14 @@
% fetch the old doc revision, so we can compare access control
% in send_update_doc() later.
Doc0 = couch_doc_open(Db, DocId, nil, [{user_ctx, Req#httpd.user_ctx}]),
- Revs = chttpd:qs_value(Req, "rev"),
- case Revs of
+ Rev = chttpd:qs_value(Req, "rev"),
+ case Rev of
undefined ->
Body = {[{<<"_deleted">>, true}]};
Rev ->
Body = {[{<<"_rev">>, ?l2b(Rev)}, {<<"_deleted">>, true}]}
end,
- Doc = #doc{revs = Revs, body = Body, deleted = true, access = Doc0#doc.access},
+ Doc = #doc{revs = Rev, body = Body, deleted = true, access = Doc0#doc.access},
send_updated_doc(Req, Db, DocId, couch_doc_from_req(Req, Db, DocId, Doc));
db_doc_req(#httpd{method = 'GET', mochi_req = MochiReq} = Req, Db, DocId) ->
#doc_query_args{
@@ -1940,10 +1940,11 @@
lists:foldl(fun parse_doc_query/2, #doc_query_args{}, chttpd:qs(Req)).
parse_shards_opt(Req) ->
+ AccessValue = list_to_existing_atom(chttpd:qs_value(Req, "access", "false")),
[
{n, parse_shards_opt("n", Req, config:get_integer("cluster", "n", 3))},
{q, parse_shards_opt("q", Req, config:get_integer("cluster", "q", 2))},
- {access, parse_shards_opt("access", Req, chttpd:qs_value(Req, "access", false))},
+ {access, parse_shards_opt("access", Req, AccessValue)},
{placement,
parse_shards_opt(
"placement", Req, config:get("cluster", "placement")
@@ -1972,27 +1973,26 @@
throw({bad_request, Err})
end
end;
-parse_shards_opt("access", Req, Value) when is_list(Value) ->
- parse_shards_opt("access", Req, list_to_existing_atom(Value));
-parse_shards_opt("access", _Req, Value) when Value =:= true ->
- case config:get_boolean("per_doc_access", "enabled", false) of
+parse_shards_opt("access", _Req, true) ->
+ case config:get_boolean("per_doc_access", "enable", false) of
true ->
true;
false ->
- Err = ?l2b(["The `access` option is not available on this CouchDB installation."]),
+ Err = <<"The `access` option is not available on this CouchDB installation.">>,
throw({bad_request, Err})
end;
-parse_shards_opt("access", _Req, Value) when Value =:= false ->
+parse_shards_opt("access", _Req, false) ->
false;
parse_shards_opt("access", _Req, _Value) ->
- Err = ?l2b(["The `access` value should be a boolean."]),
+ Err = <<"The `access` value should be a boolean.">>,
throw({bad_request, Err});
parse_shards_opt(Param, Req, Default) ->
Val = chttpd:qs_value(Req, Param, Default),
- Err = ?l2b(["The `", Param, "` value should be a positive integer."]),
case couch_util:validate_positive_int(Val) of
true -> Val;
- false -> throw({bad_request, Err})
+ false ->
+ Err = ?l2b(["The `", Param, "` value should be a positive integer."]),
+ throw({bad_request, Err})
end.
parse_engine_opt(Req) ->
diff --git a/src/couch/src/couch_access_native_proc.erl b/src/couch/src/couch_access_native_proc.erl
index 8c82cfc..494221a 100644
--- a/src/couch/src/couch_access_native_proc.erl
+++ b/src/couch/src/couch_access_native_proc.erl
@@ -132,8 +132,6 @@
Access
),
ById ++ BySeq;
- Else ->
- % TODO: no comprende: should not be needed once we implement
- % _access field validation
+ _Else ->
[[], []]
end.
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index c1e9da0..5b60307 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -140,7 +140,6 @@
]).
-include_lib("couch/include/couch_db.hrl").
-
-include("couch_db_int.hrl").
-define(DBNAME_REGEX,
@@ -821,20 +820,11 @@
} = Db#db.user_ctx,
case Access of
[] ->
- % if doc has no _access, userCtX must be admin
+ % if doc has no _access, userCtx must be admin
is_admin(Db);
Access ->
% if doc has _access, userCtx must be admin OR matching user or role
- case is_admin(Db) of
- true ->
- true;
- _ ->
- case {check_name(UserName, Access), check_roles(UserRoles, Access)} of
- {true, _} -> true;
- {_, true} -> true;
- _ -> false
- end
- end
+ is_admin(Db) or (check_name(UserName, Access) or check_roles(UserRoles, Access))
end.
check_name(null, _Access) -> false;
@@ -989,7 +979,7 @@
case couch_doc:has_access(Doc) of
true ->
validate_ddoc(Db, Doc);
- _Else ->
+ false ->
case catch check_is_admin(Db) of
ok -> validate_ddoc(Db, Doc);
Error -> Error
@@ -1421,13 +1411,13 @@
false ->
% we’re done here
{ok, DocErrors};
- _ ->
- AccessViolations = lists:filter(fun({_Ref, Tag}) -> Tag =:= access end, Results),
+ true ->
+ AccessViolations = lists:filter(fun({_Ref, Tag}) -> Tag == access end, Results),
case length(AccessViolations) of
0 ->
% we’re done here
{ok, DocErrors};
- _ ->
+ N when N > 0 ->
% dig out FDIs from Docs matching our tags/refs
DocsDict = lists:foldl(
fun(Doc, Dict) ->
@@ -1472,6 +1462,7 @@
{ok, DocBuckets, LocalDocs, DocErrors} =
before_docs_update(Db, Docs, PrepValidateFun, ?INTERACTIVE_EDIT),
+
if
(AllOrNothing) and (DocErrors /= []) ->
RefErrorDict = dict:from_list([{doc_tag(Doc), Doc} || Doc <- Docs]),
diff --git a/src/couch/src/couch_db_updater.erl b/src/couch/src/couch_db_updater.erl
index f78a043..21ae6e9 100644
--- a/src/couch/src/couch_db_updater.erl
+++ b/src/couch/src/couch_db_updater.erl
@@ -266,7 +266,6 @@
% duplicate documents if the incoming groups are not sorted, so as a sanity
% check we sort them again here. See COUCHDB-2735.
Cmp = fun([#doc{id = A} | _], [#doc{id = B} | _]) -> A < B end,
- % couch_log:notice("~n s_a_t_g_d: GroupedDocs: ~p, UserCtx: ~p ~n", [GroupedDocs, UserCtx]),
lists:map(
fun(DocGroup) ->
[{Client, maybe_tag_doc(D), UserCtx} || D <- DocGroup]
@@ -740,7 +739,6 @@
{DocsListValidated, OldDocInfosValidated} = validate_docs_access(
Db, DocsList, OldDocInfos
),
- % couch_log:notice("~n~n u_d_i: DocsList: ~p~n, OldDocInfos: ~p~n, DocsListValidated: ~p~n, OldDocInfosValidated: ~p~n~n~n", [DocsList, OldDocInfos, DocsListValidated, OldDocInfosValidated]),
{ok, AccOut} = merge_rev_trees(DocsListValidated, OldDocInfosValidated, AccIn),
#merge_acc{
add_infos = NewFullDocInfos,
@@ -783,15 +781,13 @@
validate_docs_access(Db, DocsList, OldDocInfos) ->
case couch_db:has_access_enabled(Db) of
true -> validate_docs_access_int(Db, DocsList, OldDocInfos);
- _Else -> {DocsList, OldDocInfos}
+ false -> {DocsList, OldDocInfos}
end.
validate_docs_access_int(Db, DocsList, OldDocInfos) ->
validate_docs_access(Db, DocsList, OldDocInfos, [], []).
validate_docs_access(_Db, [], [], DocsListValidated, OldDocInfosValidated) ->
- % TODO: check if need to reverse this? maybe this is the cause of the test reverse issue?
- % {lists:reverse(DocsListValidated), lists:reverse(OldDocInfosValidated)};
{lists:reverse(DocsListValidated), lists:reverse(OldDocInfosValidated)};
validate_docs_access(
Db, [Docs | DocRest], [OldInfo | OldInfoRest], DocsListValidated, OldDocInfosValidated
@@ -833,7 +829,7 @@
% we sent out all docs as invalid access, drop the old doc info associated with it
0 ->
{DocsListValidated, OldDocInfosValidated};
- _ ->
+ N when N > 0 ->
{[NewDocs | DocsListValidated], [OldInfo | OldDocInfosValidated]}
end,
validate_docs_access(
diff --git a/src/couch/test/eunit/couchdb_access_tests.erl b/src/couch/test/eunit/couchdb_access_tests.erl
index bce0cfd..bd19c9a 100644
--- a/src/couch/test/eunit/couchdb_access_tests.erl
+++ b/src/couch/test/eunit/couchdb_access_tests.erl
@@ -50,7 +50,7 @@
ok = config:set("admins", "a", binary_to_list(Hashed), false),
ok = config:set("couchdb", "uuid", "21ac467c1bc05e9d9e9d2d850bb1108f", false),
ok = config:set("log", "level", "debug", false),
- ok = config:set("per_doc_access", "enabled", "true", false),
+ ok = config:set("per_doc_access", "enable", "true", false),
% cleanup and setup
{ok, _, _, _} = test_request:delete(url() ++ "/db", ?ADMIN_REQ_HEADERS),
@@ -172,9 +172,9 @@
%
should_not_let_create_access_db_if_disabled(_PortType, Url) ->
- ok = config:set("per_doc_access", "enabled", "false", false),
+ ok = config:set("per_doc_access", "enable", "false", false),
{ok, Code, _, _} = test_request:put(url() ++ "/db?q=1&n=1&access=true", ?ADMIN_REQ_HEADERS, ""),
- ok = config:set("per_doc_access", "enabled", "true", false),
+ ok = config:set("per_doc_access", "enable", "true", false),
?_assertEqual(400, Code).
should_not_let_anonymous_user_create_doc(_PortType, Url) ->
