Make stem_interactive_updates option work again

After the aebdbc452573f70f4e50d88af5814d0fbe936333 stemming is done separately
from merge so stem interactive option didn't take effect. That is mostly ok as
speed improvements should reduce the need for that option, but it still might
be nice to keep the option (just in case).

Also, a nice side effect is it removes an extra external function from
couch_key_tree module and simplifies the tests a bit.

Related PR: #958

4 files changed, 87 insertions, 119 deletions

diff --git a/src/couch/src/couch_db_updater.erl b/src/couch/src/couch_db_updater.erl
index fba99a773..acb9ec1c9 100644
--- a/src/couch/src/couch_db_updater.erl
+++ b/src/couch/src/couch_db_updater.erl
@@ -506,7 +506,7 @@ merge_rev_trees(Limit, MergeConflicts, [NewDocs|RestDocsList],
     NewDocInfo0 = lists:foldl(fun({Client, NewDoc}, OldInfoAcc) ->
         merge_rev_tree(OldInfoAcc, NewDoc, Client, MergeConflicts)
     end, OldDocInfo, NewDocs),
-    NewDocInfo1 = stem_full_doc_info(NewDocInfo0, Limit),
+    NewDocInfo1 = maybe_stem_full_doc_info(NewDocInfo0, Limit),
     % When MergeConflicts is false, we updated #full_doc_info.deleted on every
     % iteration of merge_rev_tree. However, merge_rev_tree does not update
     % #full_doc_info.deleted when MergeConflicts is true, since we don't need
@@ -624,9 +624,14 @@ merge_rev_tree(OldInfo, NewDoc, _Client, true) ->
     {NewTree, _} = couch_key_tree:merge(OldTree, NewTree0),
     OldInfo#full_doc_info{rev_tree = NewTree}.
 
-stem_full_doc_info(#full_doc_info{rev_tree = Tree} = Info, Limit) ->
-    Stemmed = couch_key_tree:stem(Tree, Limit),
-    Info#full_doc_info{rev_tree = Stemmed}.
+maybe_stem_full_doc_info(#full_doc_info{rev_tree = Tree} = Info, Limit) ->
+    case config:get_boolean("couchdb", "stem_interactive_updates", true) of
+        true ->
+            Stemmed = couch_key_tree:stem(Tree, Limit),
+            Info#full_doc_info{rev_tree = Stemmed};
+        false ->
+            Info
+    end.
 
 update_docs_int(Db, DocsList, LocalDocs, MergeConflicts, FullCommit) ->
     UpdateSeq = couch_db_engine:get_update_seq(Db),
diff --git a/src/couch/src/couch_key_tree.erl b/src/couch/src/couch_key_tree.erl
index 5d5361507..94150418e 100644
--- a/src/couch/src/couch_key_tree.erl
+++ b/src/couch/src/couch_key_tree.erl
@@ -60,7 +60,6 @@ map/2,
 map_leafs/2,
 mapfold/3,
 multi_merge/2,
-merge/3,
 merge/2,
 remove_leafs/2,
 stem/2
@@ -81,18 +80,6 @@ multi_merge(RevTree, Trees) ->
     end, RevTree, lists:sort(Trees)).
 
 
-%% @doc Merge a path into the given tree and then stem the result.
-%% Although Tree is of type tree(), it must not contain any branches.
--spec merge(revtree(), tree() | path(), pos_integer()) ->
-                {revtree(), new_leaf | new_branch | internal_node}.
-merge(RevTree, Tree, StemDepth) ->
-    {Merged, Result} = merge(RevTree, Tree),
-    case config:get("couchdb", "stem_interactive_updates", "true") of
-        "true" -> {stem(Merged, StemDepth), Result};
-        _ -> {Merged, Result}
-    end.
-
-
 %% @doc Merge a path into a tree.
 -spec merge(revtree(), tree() | path()) ->
                 {revtree(), new_leaf | new_branch | internal_node}.
diff --git a/src/couch/src/test_engine_util.erl b/src/couch/src/test_engine_util.erl
index 89997538d..fef9e9f92 100644
--- a/src/couch/src/test_engine_util.erl
+++ b/src/couch/src/test_engine_util.erl
@@ -309,7 +309,8 @@ gen_write(Engine, St, {Action, {DocId, Body, Atts0}}, UpdateSeq) ->
         conflict -> new_branch;
         _ -> new_leaf
     end,
-    {NewTree, NodeType} = couch_key_tree:merge(PrevRevTree, Path, RevsLimit),
+    {MergedTree, NodeType} = couch_key_tree:merge(PrevRevTree, Path),
+    NewTree = couch_key_tree:stem(MergedTree, RevsLimit),
 
     NewFDI = PrevFDI#full_doc_info{
         deleted = couch_doc:is_deleted(NewTree),
diff --git a/src/couch/test/couch_key_tree_tests.erl b/src/couch/test/couch_key_tree_tests.erl
index 2b7d5fe62..5d9cc8372 100644
--- a/src/couch/test/couch_key_tree_tests.erl
+++ b/src/couch/test/couch_key_tree_tests.erl
@@ -16,138 +16,108 @@
 
 -define(DEPTH, 10).
 
-setup() ->
-    meck:new(config),
-    meck:expect(config, get, fun(_, _, Default) -> Default end).
-
-teardown(_) ->
-    meck:unload(config).
 
 key_tree_merge_test_()->
     {
         "Key tree merge",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_merge_with_empty_tree(),
-                should_merge_reflexive(),
-                should_merge_prefix_of_a_tree_with_tree(),
-                should_produce_conflict_on_merge_with_unrelated_branch(),
-                should_merge_reflexive_for_child_nodes(),
-                should_merge_tree_to_itself(),
-                should_merge_tree_of_odd_length(),
-                should_merge_tree_with_stem(),
-                should_merge_with_stem_at_deeper_level(),
-                should_merge_with_stem_at_deeper_level_with_deeper_paths(),
-                should_merge_single_tree_with_deeper_stem(),
-                should_merge_tree_with_large_stem(),
-                should_merge_stems(),
-                should_create_conflicts_on_merge(),
-                should_create_no_conflicts_on_merge(),
-                should_ignore_conflicting_branch()
-            ]
-        }
+        [
+            should_merge_with_empty_tree(),
+            should_merge_reflexive(),
+            should_merge_prefix_of_a_tree_with_tree(),
+            should_produce_conflict_on_merge_with_unrelated_branch(),
+            should_merge_reflexive_for_child_nodes(),
+            should_merge_tree_to_itself(),
+            should_merge_tree_of_odd_length(),
+            should_merge_tree_with_stem(),
+            should_merge_with_stem_at_deeper_level(),
+            should_merge_with_stem_at_deeper_level_with_deeper_paths(),
+            should_merge_single_tree_with_deeper_stem(),
+            should_merge_tree_with_large_stem(),
+            should_merge_stems(),
+            should_create_conflicts_on_merge(),
+            should_create_no_conflicts_on_merge(),
+            should_ignore_conflicting_branch()
+        ]
     }.
 
 key_tree_missing_leaves_test_()->
     {
-        "Missing tree leaves",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_not_find_missing_leaves(),
-                should_find_missing_leaves()
-            ]
-        }
+         "Missing tree leaves",
+         [
+             should_not_find_missing_leaves(),
+             should_find_missing_leaves()
+         ]
     }.
 
 key_tree_remove_leaves_test_()->
     {
         "Remove tree leaves",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_effect_on_removing_no_leaves(),
-                should_have_no_effect_on_removing_non_existant_branch(),
-                should_remove_leaf(),
-                should_produce_empty_tree_on_removing_all_leaves(),
-                should_have_no_effect_on_removing_non_existant_node(),
-                should_produce_empty_tree_on_removing_last_leaf()
-            ]
-        }
+        [
+            should_have_no_effect_on_removing_no_leaves(),
+            should_have_no_effect_on_removing_non_existant_branch(),
+            should_remove_leaf(),
+            should_produce_empty_tree_on_removing_all_leaves(),
+            should_have_no_effect_on_removing_non_existant_node(),
+            should_produce_empty_tree_on_removing_last_leaf()
+        ]
     }.
 
 key_tree_get_leaves_test_()->
     {
         "Leaves retrieving",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_extract_subtree(),
-                should_extract_subsubtree(),
-                should_gather_non_existant_leaf(),
-                should_gather_leaf(),
-                shoul_gather_multiple_leaves(),
-                should_gather_single_leaf_for_multiple_revs(),
-                should_gather_multiple_for_multiple_revs(),
-                should_retrieve_full_key_path(),
-                should_retrieve_full_key_path_for_node(),
-                should_retrieve_leaves_with_parent_node(),
-                should_retrieve_all_leaves()
-            ]
-        }
+        [
+            should_extract_subtree(),
+            should_extract_subsubtree(),
+            should_gather_non_existant_leaf(),
+            should_gather_leaf(),
+            shoul_gather_multiple_leaves(),
+            should_gather_single_leaf_for_multiple_revs(),
+            should_gather_multiple_for_multiple_revs(),
+            should_retrieve_full_key_path(),
+            should_retrieve_full_key_path_for_node(),
+            should_retrieve_leaves_with_parent_node(),
+            should_retrieve_all_leaves()
+        ]
     }.
 
 key_tree_leaf_counting_test_()->
     {
         "Leaf counting",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_leaves_for_empty_tree(),
-                should_have_single_leaf_for_tree_with_single_node(),
-                should_have_two_leaves_for_tree_with_chindler_siblings(),
-                should_not_affect_on_leaf_counting_for_stemmed_tree()
-            ]
-        }
+        [
+            should_have_no_leaves_for_empty_tree(),
+            should_have_single_leaf_for_tree_with_single_node(),
+            should_have_two_leaves_for_tree_with_chindler_siblings(),
+            should_not_affect_on_leaf_counting_for_stemmed_tree()
+        ]
     }.
 
 key_tree_stemming_test_()->
     {
         "Stemming",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_effect_for_stemming_more_levels_than_exists(),
-                should_return_one_deepest_node(),
-                should_return_two_deepest_nodes()
-            ]
-        }
+        [
+            should_have_no_effect_for_stemming_more_levels_than_exists(),
+            should_return_one_deepest_node(),
+            should_return_two_deepest_nodes()
+        ]
     }.
 
 
 should_merge_with_empty_tree()->
     One = {1, {"1","foo",[]}},
     ?_assertEqual({[One], new_leaf},
-                  couch_key_tree:merge([], One, ?DEPTH)).
+                  merge_and_stem([], One)).
 
 should_merge_reflexive()->
     One = {1, {"1","foo",[]}},
     ?_assertEqual({[One], internal_node},
-                  couch_key_tree:merge([One], One, ?DEPTH)).
+                  merge_and_stem([One], One)).
 
 should_merge_prefix_of_a_tree_with_tree()->
     One = {1, {"1","foo",[]}},
     TwoSibs = [{1, {"1","foo",[]}},
                {1, {"2","foo",[]}}],
     ?_assertEqual({TwoSibs, internal_node},
-                  couch_key_tree:merge(TwoSibs, One, ?DEPTH)).
+                  merge_and_stem(TwoSibs, One)).
 
 should_produce_conflict_on_merge_with_unrelated_branch()->
     TwoSibs = [{1, {"1","foo",[]}},
@@ -157,12 +127,12 @@ should_produce_conflict_on_merge_with_unrelated_branch()->
                  {1, {"2","foo",[]}},
                  {1, {"3","foo",[]}}],
     ?_assertEqual({ThreeSibs, new_branch},
-                  couch_key_tree:merge(TwoSibs, Three, ?DEPTH)).
+                  merge_and_stem(TwoSibs, Three)).
 
 should_merge_reflexive_for_child_nodes()->
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], TwoChild, ?DEPTH)).
+                  merge_and_stem([TwoChild], TwoChild)).
 
 should_merge_tree_to_itself()->
     TwoChildSibs = {1, {"1","foo", [{"1a", "bar", []},
@@ -170,7 +140,7 @@ should_merge_tree_to_itself()->
     Leafs = couch_key_tree:get_all_leafs([TwoChildSibs]),
     Paths = lists:map(fun leaf_to_path/1, Leafs),
     FinalTree = lists:foldl(fun(Path, TreeAcc) ->
-        {NewTree, internal_node} = couch_key_tree:merge(TreeAcc, Path),
+        {NewTree, internal_node} = merge_and_stem(TreeAcc, Path),
         NewTree
     end, [TwoChildSibs], Paths),
     ?_assertEqual([TwoChildSibs], FinalTree).
@@ -192,7 +162,7 @@ should_merge_tree_of_odd_length()->
     TwoChildPlusSibs = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]},
                                         {"1b", "bar", []}]}},
     ?_assertEqual({[TwoChildPlusSibs], new_leaf},
-                  couch_key_tree:merge([TwoChildSibs], TwoChild, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], TwoChild)).
 
 should_merge_tree_with_stem()->
     Stemmed = {2, {"1a", "bar", []}},
@@ -200,52 +170,52 @@ should_merge_tree_with_stem()->
                                     {"1b", "bar", []}]}},
 
     ?_assertEqual({[TwoChildSibs], internal_node},
-                  couch_key_tree:merge([TwoChildSibs], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], Stemmed)).
 
 should_merge_with_stem_at_deeper_level()->
     Stemmed = {3, {"1bb", "boo", []}},
     TwoChildSibs = {1, {"1","foo", [{"1a", "bar", []},
                                     {"1b", "bar", [{"1bb", "boo", []}]}]}},
     ?_assertEqual({[TwoChildSibs], internal_node},
-                  couch_key_tree:merge([TwoChildSibs], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], Stemmed)).
 
 should_merge_with_stem_at_deeper_level_with_deeper_paths()->
     Stemmed = {3, {"1bb", "boo", []}},
     StemmedTwoChildSibs = [{2,{"1a", "bar", []}},
                            {2,{"1b", "bar", [{"1bb", "boo", []}]}}],
     ?_assertEqual({StemmedTwoChildSibs, internal_node},
-                  couch_key_tree:merge(StemmedTwoChildSibs, Stemmed, ?DEPTH)).
+                  merge_and_stem(StemmedTwoChildSibs, Stemmed)).
 
 should_merge_single_tree_with_deeper_stem()->
     Stemmed = {3, {"1aa", "bar", []}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChild], Stemmed)).
 
 should_merge_tree_with_large_stem()->
     Stemmed = {2, {"1a", "bar", [{"1aa", "bar", []}]}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChild], Stemmed)).
 
 should_merge_stems()->
     StemmedA = {2, {"1a", "bar", [{"1aa", "bar", []}]}},
     StemmedB = {3, {"1aa", "bar", []}},
     ?_assertEqual({[StemmedA], internal_node},
-                  couch_key_tree:merge([StemmedA], StemmedB, ?DEPTH)).
+                  merge_and_stem([StemmedA], StemmedB)).
 
 should_create_conflicts_on_merge()->
     OneChild = {1, {"1","foo",[{"1a", "bar", []}]}},
     Stemmed = {3, {"1aa", "bar", []}},
     ?_assertEqual({[OneChild, Stemmed], new_branch},
-                  couch_key_tree:merge([OneChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([OneChild], Stemmed)).
 
 should_create_no_conflicts_on_merge()->
     OneChild = {1, {"1","foo",[{"1a", "bar", []}]}},
     Stemmed = {3, {"1aa", "bar", []}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], new_leaf},
-                  couch_key_tree:merge([OneChild, Stemmed], TwoChild, ?DEPTH)).
+                  merge_and_stem([OneChild, Stemmed], TwoChild)).
 
 should_ignore_conflicting_branch()->
     %% this test is based on couch-902-test-case2.py
@@ -274,7 +244,7 @@ should_ignore_conflicting_branch()->
     {
         "COUCHDB-902",
         ?_assertEqual({[FooBar], new_leaf},
-                      couch_key_tree:merge([Foo], Bar, ?DEPTH))
+                      merge_and_stem([Foo], Bar))
     }.
 
 should_not_find_missing_leaves()->
@@ -436,3 +406,8 @@ should_return_two_deepest_nodes()->
     TwoChild = [{0, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}}],
     Stemmed = [{1, {"1a", "bar", [{"1aa", "bar", []}]}}],
     ?_assertEqual(Stemmed, couch_key_tree:stem(TwoChild, 2)).
+
+
+merge_and_stem(RevTree, Tree) ->
+    {Merged, Result} = couch_key_tree:merge(RevTree, Tree),
+    {couch_key_tree:stem(Merged, ?DEPTH), Result}.