Allow interactive requests to reopen a re-created db instance

Previously, if a database was re-created on another node, a request with that
database might have found the previous db instance in the cache. In that case
it would have correctly reopened the db while in a transaction, but, because
the old db instance was deleted it would throw a database_does_not_exist which
was not the correct behavior.

To prevent that from happening, introduce an interactive = true|false option
when opening a database. User requests may specify that option and then when
the db is re-opened, it will allow it to automatically upgrade to the new db
instance instead returning an error.

Background processes will still get a database_doest_not_exist error if they
keep a db open which has now been re-created.

The interactive option may also be used in the future to set other transaction
parameters like timeouts and retries that might be different for interactive
requests vs background tasks.

6 files changed, 99 insertions, 22 deletions

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index fdaf4af8c..8acccb461 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -395,7 +395,8 @@ delete_db_req(#httpd{user_ctx=Ctx}=Req, DbName) ->
     end.
 
 do_db_req(#httpd{path_parts=[DbName|_], user_ctx=Ctx}=Req, Fun) ->
-    {ok, Db} = fabric2_db:open(DbName, [{user_ctx, Ctx}]),
+    Options = [{user_ctx, Ctx}, {interactive, true}],
+    {ok, Db} = fabric2_db:open(DbName, Options),
     Fun(Req, Db).
 
 db_req(#httpd{method='GET',path_parts=[_DbName]}=Req, Db) ->
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index 667cf35c6..4ac105589 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -201,7 +201,8 @@ open(DbName, Options) ->
     case fabric2_server:fetch(DbName, UUID) of
         #{} = Db ->
             Db1 = maybe_set_user_ctx(Db, Options),
-            {ok, require_member_check(Db1)};
+            Db2 = maybe_set_interactive(Db1, Options),
+            {ok, require_member_check(Db2)};
         undefined ->
             Result = fabric2_fdb:transactional(DbName, Options, fun(TxDb) ->
                 fabric2_fdb:open(TxDb, Options)
@@ -1426,6 +1427,11 @@ get_all_docs_meta(TxDb, Options) ->
     end ++ [{total, DocCount}, {offset, null}].
 
 
+maybe_set_interactive(#{} = Db, Options) ->
+    Interactive = fabric2_util:get_value(interactive, Options, false),
+    Db#{interactive := Interactive}.
+
+
 maybe_set_user_ctx(Db, Options) ->
     case fabric2_util:get_value(user_ctx, Options) of
         #user_ctx{} = UserCtx ->
diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl
index f721ca4ab..52303cef1 100644
--- a/src/fabric/src/fabric2_fdb.erl
+++ b/src/fabric/src/fabric2_fdb.erl
@@ -243,7 +243,8 @@ create(#{} = Db0, Options) ->
         after_doc_read => undefined,
         % All other db things as we add features,
 
-        db_options => Options1
+        db_options => Options1,
+        interactive => false
     },
     aegis:init_db(Db2, Options).
 
@@ -270,6 +271,9 @@ open(#{} = Db0, Options) ->
     UUID = fabric2_util:get_value(uuid, Options1),
     Options2 = lists:keydelete(uuid, 1, Options1),
 
+    Interactive = fabric2_util:get_value(interactive, Options2, false),
+    Options3 = lists:keydelete(interactive, 1, Options2),
+
     Db2 = Db1#{
         db_prefix => DbPrefix,
         db_version => DbVersion,
@@ -287,7 +291,8 @@ open(#{} = Db0, Options) ->
         before_doc_update => undefined,
         after_doc_read => undefined,
 
-        db_options => Options2
+        db_options => Options3,
+        interactive => Interactive
     },
 
     Db3 = load_config(Db2),
@@ -318,7 +323,8 @@ refresh(#{tx := undefined, name := DbName} = Db) ->
         #{md_version := Ver} = Db1 when Ver > OldVer ->
             Db1#{
                 user_ctx := maps:get(user_ctx, Db),
-                security_fun := maps:get(security_fun, Db)
+                security_fun := maps:get(security_fun, Db),
+                interactive := maps:get(interactive, Db)
             };
         _ ->
             Db
@@ -337,18 +343,20 @@ reopen(#{} = OldDb) ->
         uuid := UUID,
         db_options := Options,
         user_ctx := UserCtx,
-        security_fun := SecurityFun
+        security_fun := SecurityFun,
+        interactive := Interactive
     } = OldDb,
     Options1 = lists:keystore(user_ctx, 1, Options, {user_ctx, UserCtx}),
     NewDb = open(init_db(Tx, DbName, Options1), Options1),
 
     % Check if database was re-created
-    case maps:get(uuid, NewDb) of
-        UUID -> ok;
-        _OtherUUID -> error(database_does_not_exist)
+    case {Interactive, maps:get(uuid, NewDb)} of
+        {true, _} -> ok;
+        {false, UUID} -> ok;
+        {false, _OtherUUID} -> error(database_does_not_exist)
     end,
 
-    NewDb#{security_fun := SecurityFun}.
+    NewDb#{security_fun := SecurityFun, interactive := Interactive}.
 
 
 delete(#{} = Db) ->
diff --git a/src/fabric/src/fabric2_server.erl b/src/fabric/src/fabric2_server.erl
index b557da8c7..be674b10e 100644
--- a/src/fabric/src/fabric2_server.erl
+++ b/src/fabric/src/fabric2_server.erl
@@ -271,5 +271,6 @@ sanitize(#{} = Db) ->
     Db#{
         tx := undefined,
         user_ctx := #user_ctx{},
-        security_fun := undefined
+        security_fun := undefined,
+        interactive := false
     }.
diff --git a/src/fabric/test/fabric2_db_crud_tests.erl b/src/fabric/test/fabric2_db_crud_tests.erl
index 000f3709c..3d90c65b5 100644
--- a/src/fabric/test/fabric2_db_crud_tests.erl
+++ b/src/fabric/test/fabric2_db_crud_tests.erl
@@ -38,6 +38,8 @@ crud_test_() ->
                     ?TDEF_FE(open_db),
                     ?TDEF_FE(delete_db),
                     ?TDEF_FE(recreate_db),
+                    ?TDEF_FE(recreate_db_interactive),
+                    ?TDEF_FE(recreate_db_non_interactive),
                     ?TDEF_FE(undelete_db),
                     ?TDEF_FE(remove_deleted_db),
                     ?TDEF_FE(scheduled_remove_deleted_db, 15),
@@ -179,6 +181,32 @@ recreate_db(_) ->
     ?assertError(database_does_not_exist, fabric2_db:open(DbName, BadOpts)).
 
 
+recreate_db_interactive(_) ->
+    DbName = ?tempdb(),
+    ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+    {ok, Db1} = fabric2_db:open(DbName, [{interactive, true}]),
+
+    ?assertEqual(ok, fabric2_db:delete(DbName, [])),
+    ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+    ?assertMatch({ok, _}, fabric2_db:get_db_info(Db1)).
+
+
+recreate_db_non_interactive(_) ->
+    % This is also the default case, but we check that parsing the `false` open
+    % value works correctly.
+    DbName = ?tempdb(),
+    ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+    {ok, Db1} = fabric2_db:open(DbName, [{interactive, false}]),
+
+    ?assertEqual(ok, fabric2_db:delete(DbName, [])),
+    ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+    ?assertError(database_does_not_exist, fabric2_db:get_db_info(Db1)).
+
+
 undelete_db(_) ->
     DbName = ?tempdb(),
     ?assertError(database_does_not_exist, fabric2_db:delete(DbName, [])),
diff --git a/src/fabric/test/fabric2_db_security_tests.erl b/src/fabric/test/fabric2_db_security_tests.erl
index 063979a3f..3d7167a00 100644
--- a/src/fabric/test/fabric2_db_security_tests.erl
+++ b/src/fabric/test/fabric2_db_security_tests.erl
@@ -40,7 +40,8 @@ security_test_() ->
                 ?TDEF(check_set_user_ctx),
                 ?TDEF(check_forbidden),
                 ?TDEF(check_fail_no_opts),
-                ?TDEF(check_fail_name_null)
+                ?TDEF(check_fail_name_null),
+                ?TDEF(check_forbidden_with_interactive_reopen)
             ])
         }
     }.
@@ -51,6 +52,18 @@ setup() ->
     DbName = ?tempdb(),
     PubDbName = ?tempdb(),
     {ok, Db1} = fabric2_db:create(DbName, [?ADMIN_CTX]),
+    ok = set_test_security(Db1),
+    {ok, _} = fabric2_db:create(PubDbName, [?ADMIN_CTX]),
+    {DbName, PubDbName, Ctx}.
+
+
+cleanup({DbName, PubDbName, Ctx}) ->
+    ok = fabric2_db:delete(DbName, []),
+    ok = fabric2_db:delete(PubDbName, []),
+    test_util:stop_couch(Ctx).
+
+
+set_test_security(Db) ->
     SecProps = {[
         {<<"admins">>, {[
             {<<"names">>, [<<"admin_name1">>, <<"admin_name2">>]},
@@ -61,16 +74,7 @@ setup() ->
             {<<"roles">>, [<<"member_role1">>, <<"member_role2">>]}
         ]}}
     ]},
-    ok = fabric2_db:set_security(Db1, SecProps),
-    {ok, _} = fabric2_db:create(PubDbName, [?ADMIN_CTX]),
-    {DbName, PubDbName, Ctx}.
-
-
-cleanup({DbName, PubDbName, Ctx}) ->
-    ok = fabric2_db:delete(DbName, []),
-    ok = fabric2_db:delete(PubDbName, []),
-    test_util:stop_couch(Ctx).
-
+    ok = fabric2_db:set_security(Db, SecProps).
 
 
 check_is_admin({DbName, _, _}) ->
@@ -184,3 +188,32 @@ check_fail_name_null({DbName, _, _}) ->
     UserCtx = #user_ctx{name = null},
     {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
     ?assertThrow({unauthorized, _}, fabric2_db:get_db_info(Db)).
+
+
+check_forbidden_with_interactive_reopen({DbName, _, _}) ->
+    UserCtx = #user_ctx{name = <<"foo">>},
+    Options = [{user_ctx, UserCtx}, {interactive, true}],
+
+    {ok, Db1} = fabric2_db:open(DbName, Options),
+
+    % Verify foo is forbidden by default
+    ?assertThrow({forbidden, _}, fabric2_db:get_db_info(Db1)),
+
+    % Allow foo
+    {ok, Db2} = fabric2_db:open(DbName, [?ADMIN_CTX]),
+    AllowFoo = {[
+        {<<"members">>, {[
+           {<<"names">>, [<<"foo">>]}
+        ]}}
+    ]},
+    ok = fabric2_db:set_security(Db2, AllowFoo),
+
+    ?assertMatch({ok, _}, fabric2_db:get_db_info(Db1)),
+
+    % Recreate test db instance with the default security
+    ok = fabric2_db:delete(DbName, [?ADMIN_CTX]),
+    {ok, Db3} = fabric2_db:create(DbName, [?ADMIN_CTX]),
+    ok = set_test_security(Db3),
+
+    % Original handle is forbidden to again
+    ?assertThrow({forbidden, _}, fabric2_db:get_db_info(Db1)).