You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2019/09/17 15:37:29 UTC
[couchdb] branch prototype/fdb-layer updated: Make get_security and
get_revs_limit calls consistent
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/prototype/fdb-layer by this push:
new 9c6b5cb Make get_security and get_revs_limit calls consistent
9c6b5cb is described below
commit 9c6b5cbd3fdebb45162df57c59ed9291ab5138fd
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Mon Sep 16 17:39:32 2019 -0400
Make get_security and get_revs_limit calls consistent
There are two fixes:
1) In `fabric2_fdb:get_config/1`, Db was matched before and after
`ensure_current/1`. Only the db prefix path was used, which doesn't normally
change, but it's worth fixing it anyway.
2) We used a cached version of the security document outside the transaction.
Now we force it go through a transaction to call `fabric2_fdb:get_config/1`
which call `ensure_current/1`. When done, we also update the cached Db handle.
Do the same thing for revs_limit even thought it is not as critical as
security.
---
src/fabric/src/fabric2_db.erl | 14 +++++++---
src/fabric/src/fabric2_fdb.erl | 16 ++++++++++-
src/fabric/test/fabric2_db_security_tests.erl | 38 ++++++++++++++-------------
3 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index 853b502..8927ce3 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -346,12 +346,18 @@ get_pid(#{}) ->
nil.
-get_revs_limit(#{revs_limit := RevsLimit}) ->
- RevsLimit.
+get_revs_limit(#{} = Db) ->
+ RevsLimitBin = fabric2_fdb:transactional(Db, fun(TxDb) ->
+ fabric2_fdb:get_config(TxDb, <<"revs_limit">>)
+ end),
+ ?bin2uint(RevsLimitBin).
-get_security(#{security_doc := SecurityDoc}) ->
- SecurityDoc.
+get_security(#{} = Db) ->
+ SecBin = fabric2_fdb:transactional(Db, fun(TxDb) ->
+ fabric2_fdb:get_config(TxDb, <<"security_doc">>)
+ end),
+ ?JSON_DECODE(SecBin).
get_update_seq(#{} = Db) ->
diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl
index 391122e..ccfeb3c 100644
--- a/src/fabric/src/fabric2_fdb.erl
+++ b/src/fabric/src/fabric2_fdb.erl
@@ -28,6 +28,7 @@
get_info/1,
get_config/1,
+ get_config/2,
set_config/3,
get_stat/2,
@@ -338,7 +339,7 @@ get_config(#{} = Db) ->
#{
tx := Tx,
db_prefix := DbPrefix
- } = Db = ensure_current(Db),
+ } = ensure_current(Db),
{Start, End} = erlfdb_tuple:range({?DB_CONFIG}, DbPrefix),
Future = erlfdb:get_range(Tx, Start, End),
@@ -349,6 +350,19 @@ get_config(#{} = Db) ->
end, erlfdb:wait(Future)).
+get_config(#{} = Db, ConfigKey) ->
+ #{
+ tx := Tx,
+ db_prefix := DbPrefix
+ } = ensure_current(Db),
+
+ Key = erlfdb_tuple:pack({?DB_CONFIG, ConfigKey}, DbPrefix),
+ case erlfdb:wait(erlfdb:get(Tx, Key)) of
+ % config values are expected to be set so we blow if not_found
+ Val when Val =/= not_found -> Val
+ end.
+
+
set_config(#{} = Db, ConfigKey, ConfigVal) ->
#{
tx := Tx,
diff --git a/src/fabric/test/fabric2_db_security_tests.erl b/src/fabric/test/fabric2_db_security_tests.erl
index 9796011..b4df3b4 100644
--- a/src/fabric/test/fabric2_db_security_tests.erl
+++ b/src/fabric/test/fabric2_db_security_tests.erl
@@ -47,6 +47,7 @@ security_test_() ->
setup() ->
Ctx = test_util:start_couch([fabric]),
DbName = ?tempdb(),
+ PubDbName = ?tempdb(),
{ok, Db1} = fabric2_db:create(DbName, [{user_ctx, ?ADMIN_USER}]),
SecProps = {[
{<<"admins">>, {[
@@ -60,40 +61,42 @@ setup() ->
]},
ok = fabric2_db:set_security(Db1, SecProps),
{ok, Db2} = fabric2_db:open(DbName, []),
- {Db2, Ctx}.
+ {ok, PubDb} = fabric2_db:create(PubDbName, []),
+ {Db2, PubDb, Ctx}.
-cleanup({Db, Ctx}) ->
+cleanup({Db, PubDb, Ctx}) ->
ok = fabric2_db:delete(fabric2_db:name(Db), []),
+ ok = fabric2_db:delete(fabric2_db:name(PubDb), []),
test_util:stop_couch(Ctx).
-is_admin_name({Db, _}) ->
+is_admin_name({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"admin_name1">>},
?assertEqual(true, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
-is_not_admin_name({Db, _}) ->
+is_not_admin_name({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"member1">>},
?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
-is_admin_role({Db, _}) ->
+is_admin_role({Db, _, _}) ->
UserCtx = #user_ctx{roles = [<<"admin_role1">>]},
?assertEqual(true, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
-is_not_admin_role({Db, _}) ->
+is_not_admin_role({Db, _, _}) ->
UserCtx = #user_ctx{roles = [<<"member_role1">>]},
?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
-check_is_admin({Db, _}) ->
+check_is_admin({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"admin_name1">>},
?assertEqual(ok, fabric2_db:check_is_admin(Db#{user_ctx := UserCtx})).
-check_is_not_admin({Db, _}) ->
+check_is_not_admin({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"member_name1">>},
?assertThrow(
{unauthorized, <<"You are not a db or server admin.">>},
@@ -105,12 +108,12 @@ check_is_not_admin({Db, _}) ->
).
-check_is_member_name({Db, _}) ->
+check_is_member_name({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"member_name1">>},
?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
-check_is_not_member_name({Db, _}) ->
+check_is_not_member_name({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>},
?assertThrow(
{unauthorized, <<"You are not authorized", _/binary>>},
@@ -122,12 +125,12 @@ check_is_not_member_name({Db, _}) ->
).
-check_is_member_role({Db, _}) ->
+check_is_member_role({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"member_role1">>]},
?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
-check_is_not_member_role({Db, _}) ->
+check_is_not_member_role({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
?assertThrow(
{forbidden, <<"You are not allowed to access", _/binary>>},
@@ -135,25 +138,24 @@ check_is_not_member_role({Db, _}) ->
).
-check_admin_is_member({Db, _}) ->
+check_admin_is_member({Db, _, _}) ->
UserCtx = #user_ctx{name = <<"admin_name1">>},
?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
-check_is_member_of_public_db({Db, _}) ->
- PublicDb = Db#{security_doc := {[]}},
+check_is_member_of_public_db({_, PubDb, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
?assertEqual(
ok,
- fabric2_db:check_is_member(PublicDb#{user_ctx := #user_ctx{}})
+ fabric2_db:check_is_member(PubDb#{user_ctx := #user_ctx{}})
),
?assertEqual(
ok,
- fabric2_db:check_is_member(PublicDb#{user_ctx := UserCtx})
+ fabric2_db:check_is_member(PubDb#{user_ctx := UserCtx})
).
-check_set_user_ctx({Db0, _}) ->
+check_set_user_ctx({Db0, _, _}) ->
DbName = fabric2_db:name(Db0),
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
{ok, Db1} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),