fix remaining access tests

apache · Jul 28, 2023 · b1e1899 · b1e1899
1 parent 5ec411a
commit b1e1899
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 49 deletions.
diff --git a/src/couch/src/couch_db_updater.erl b/src/couch/src/couch_db_updater.erl
@@ -266,6 +266,7 @@ sort_and_tag_grouped_docs(Client, GroupedDocs, UserCtx) ->
     % 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]
@@ -739,7 +740,7 @@ update_docs_int(Db, DocsList, LocalDocs, ReplicatedChanges) ->
     {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,
@@ -791,7 +792,7 @@ validate_docs_access_int(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)};
-    {DocsListValidated, OldDocInfosValidated};
+    {lists:reverse(DocsListValidated), lists:reverse(OldDocInfosValidated)};
 validate_docs_access(
     Db, [Docs | DocRest], [OldInfo | OldInfoRest], DocsListValidated, OldDocInfosValidated
 ) ->
@@ -831,7 +832,7 @@ validate_docs_access(
         case length(NewDocs) of
             % we sent out all docs as invalid access, drop the old doc info associated with it
             0 ->
-                {[NewDocs | DocsListValidated], OldDocInfosValidated};
+                {DocsListValidated, OldDocInfosValidated};
             _ ->
                 {[NewDocs | DocsListValidated], [OldInfo | OldDocInfosValidated]}
         end,

diff --git a/src/couch/test/eunit/couchdb_access_tests.erl b/src/couch/test/eunit/couchdb_access_tests.erl
@@ -87,7 +87,7 @@ access_test_() ->
         fun should_let_user_create_doc_for_themselves/2,
         fun should_not_let_user_create_doc_for_someone_else/2,
         fun should_let_user_create_access_ddoc/2,
-        % fun access_ddoc_should_have_no_effects/2,
+        fun access_ddoc_should_have_no_effects/2,
 
         % Doc updates
         fun users_with_access_can_update_doc/2,
@@ -100,8 +100,6 @@ access_test_() ->
         fun user_with_access_can_read_doc/2,
         fun user_without_access_can_not_read_doc/2,
         fun user_can_not_read_doc_without_access/2,
-        fun admin_with_access_can_read_conflicted_doc/2,
-        % fun user_with_access_can_not_read_conflicted_doc/2,
 
         % Doc deletes
         fun should_let_admin_delete_doc_with_access/2,
@@ -130,10 +128,8 @@ access_test_() ->
 
         fun should_allow_user_to_replicate_from_access_to_access/2,
         fun should_allow_user_to_replicate_from_access_to_no_access/2,
-        % TODO: find out why this is flakey
-        % fun should_allow_user_to_replicate_from_no_access_to_access/2,
-
-        % fun should_allow_user_to_replicate_from_no_access_to_no_access/2,
+        fun should_allow_user_to_replicate_from_no_access_to_access/2,
+        fun should_allow_user_to_replicate_from_no_access_to_no_access/2,
 
         % _revs_diff for docs you don’t have access to
         fun should_not_allow_user_to_revs_diff_other_docs/2
@@ -373,45 +369,6 @@ user_with_access_can_read_doc(_PortType, Url) ->
     ),
     ?_assertEqual(200, Code).
 
-% TODO: induce conflict with two different _access users per rev
-% could be comiing from a split-brain scenario
-% whoever ends up winner can read the doc, but not the leaf
-% that doesn’t belong to them
-% whoever loses can only request their leaf
-% user_with_access_can_not_read_conflicted_doc(_PortType, Url) ->
-%     {ok, 201, _, _} = test_request:put(
-%         Url ++ "/db/a",
-%         ?ADMIN_REQ_HEADERS,
-%         "{\"_id\":\"f1\",\"a\":1,\"_access\":[\"x\"]}"
-%     ),
-%     {ok, 201, _, _} = test_request:put(
-%         Url ++ "/db/a?new_edits=false",
-%         ?ADMIN_REQ_HEADERS,
-%         "{\"_id\":\"f1\",\"_rev\":\"7-XYZ\",\"a\":1,\"_access\":[\"x\"]}"
-%     ),
-%     {ok, Code, _, _} = test_request:get(
-%         Url ++ "/db/a",
-%         ?USERX_REQ_HEADERS
-%     ),
-%     ?_assertEqual(403, Code).
-
-admin_with_access_can_read_conflicted_doc(_PortType, Url) ->
-    {ok, 201, _, _} = test_request:put(
-        Url ++ "/db/a",
-        ?ADMIN_REQ_HEADERS,
-        "{\"_id\":\"a\",\"a\":1,\"_access\":[\"x\"]}"
-    ),
-    {ok, 201, _, _} = test_request:put(
-        Url ++ "/db/a?new_edits=false",
-        ?ADMIN_REQ_HEADERS,
-        "{\"_id\":\"a\",\"_rev\":\"7-XYZ\",\"a\":1,\"_access\":[\"x\"]}"
-    ),
-    {ok, Code, _, _} = test_request:get(
-        Url ++ "/db/a",
-        ?ADMIN_REQ_HEADERS
-    ),
-    ?_assertEqual(200, Code).
-
 user_without_access_can_not_read_doc(_PortType, Url) ->
     {ok, 201, _, _} = test_request:put(
         Url ++ "/db/a",