Refactor Mango warning generation

Refactor the code generating Mango warnings to (hopefully!)
improve readability.

This also moves the metric counter for unindexed queries such that it
gets incremented only when no index is used. Previously, it would
increment if *any* warning was generated (e.g. when Mango warned that
the user-specified index was not found/valid but fell back to a
different index).

The warning string returned now also contains all warnings generated
for the query, delimited via newlines - not just the first one.

3 files changed, 53 insertions, 42 deletions

diff --git a/src/mango/src/mango_cursor.erl b/src/mango/src/mango_cursor.erl
index d9eefbdf2..f50824545 100644
--- a/src/mango/src/mango_cursor.erl
+++ b/src/mango/src/mango_cursor.erl
@@ -115,7 +115,7 @@ filter_indexes(Indexes0, DesignId, ViewName) ->
 
 
 remove_indexes_with_partial_filter_selector(Indexes) ->
-    FiltFun = fun(Idx) -> 
+    FiltFun = fun(Idx) ->
         case mango_idx:get_partial_filter_selector(Idx) of
             undefined -> true;
             _ -> false
@@ -147,46 +147,57 @@ group_indexes_by_type(Indexes) ->
     end, ?CURSOR_MODULES).
 
 
-maybe_add_warning(UserFun, #cursor{index = Index, opts = Opts}, UserAcc) ->
-    NoIndexWarning = case Index#idx.type of
-        <<"special">> ->
-            <<"no matching index found, create an index to optimize query time">>;
-        _ ->
-            ok
-    end,
+% warn if the _all_docs index was used to fulfil a query
+no_index_warning(#idx{type = Type}) when Type =:= <<"special">> ->
+    couch_stats:increment_counter([mango, unindexed_queries]),
+    [<<"No matching index found, create an index to optimize query time.">>];
+no_index_warning(_) ->
+    [].
 
-    UseIndexInvalidWarning = case lists:keyfind(use_index, 1, Opts) of
-        {use_index, []} ->
-            NoIndexWarning;
-        {use_index, [DesignId]} ->
-            case filter_indexes([Index], DesignId) of
-                [] ->
-                    fmt("_design/~s was not used because it does not contain a valid index for this query.", 
-                        [ddoc_name(DesignId)]);
-                _ ->
-                    NoIndexWarning
-            end;
-        {use_index, [DesignId, ViewName]} ->
-            case filter_indexes([Index], DesignId, ViewName) of
-                [] ->
-                    fmt("_design/~s, ~s was not used because it is not a valid index for this query.", 
-                        [ddoc_name(DesignId), ViewName]);
-                _ ->
-                    NoIndexWarning
-            end
-    end,
 
-    maybe_add_warning_int(UseIndexInvalidWarning, UserFun, UserAcc).
+% warn if user specified an index which doesn't exist or isn't valid
+% for the selector.
+% In this scenario, Mango will ignore the index hint and auto-select an index.
+invalid_index_warning(Index, Opts) ->
+    UseIndex = lists:keyfind(use_index, 1, Opts),
+    invalid_index_warning_int(Index, UseIndex).
 
 
-maybe_add_warning_int(ok, _, UserAcc) ->
-   UserAcc;
+invalid_index_warning_int(Index, {use_index, [DesignId]}) ->
+    case filter_indexes([Index], DesignId) of
+        [] ->
+            Reason = fmt("_design/~s was not used because it does not contain a valid index for this query.",
+                [ddoc_name(DesignId)]),
+            [Reason];
+        _ ->
+            []
+    end;
+invalid_index_warning_int(Index, {use_index, [DesignId, ViewName]}) ->
+    case filter_indexes([Index], DesignId, ViewName) of
+        [] ->
+            Reason = fmt("_design/~s, ~s was not used because it is not a valid index for this query.",
+                [ddoc_name(DesignId), ViewName]),
+            [Reason];
+        _ ->
+            []
+    end;
+invalid_index_warning_int(_, _) ->
+    [].
+
 
-maybe_add_warning_int(Warning, UserFun, UserAcc) ->
-    couch_stats:increment_counter([mango, unindexed_queries]),
-    Arg = {add_key, warning, Warning},
-    {_Go, UserAcc0} = UserFun(Arg, UserAcc),
-    UserAcc0.
+maybe_add_warning(UserFun, #cursor{index = Index, opts = Opts}, UserAcc) ->
+    W0 = invalid_index_warning(Index, Opts),
+    W1 = no_index_warning(Index),
+    Warnings = lists:append([W0, W1]),
+    case Warnings of
+        [] ->
+            UserAcc;
+        _ ->
+            WarningStr = lists:join(<<"\n">>, Warnings),
+            Arg = {add_key, warning, WarningStr},
+            {_Go, UserAcc1} = UserFun(Arg, UserAcc),
+            UserAcc1
+    end.
 
 % When there is an empty array for certain operators, we don't actually
 % want to execute the query so we deny it by making the range [empty].
diff --git a/src/mango/test/05-index-selection-test.py b/src/mango/test/05-index-selection-test.py
index 3f7fb9f21..271e36176 100644
--- a/src/mango/test/05-index-selection-test.py
+++ b/src/mango/test/05-index-selection-test.py
@@ -84,7 +84,7 @@ class IndexSelectionTests:
         ddocid = "_design/age"
         r = self.db.find({}, use_index=ddocid, return_raw=True)
         self.assertEqual(
-            r["warning"],
+            r["warning"][0].lower(),
             "{0} was not used because it does not contain a valid index for this query.".format(
                 ddocid
             ),
@@ -107,7 +107,7 @@ class IndexSelectionTests:
         selector = {"company": "Pharmex"}
         r = self.db.find(selector, use_index=ddocid, return_raw=True)
         self.assertEqual(
-            r["warning"],
+            r["warning"][0].lower(),
             "{0} was not used because it does not contain a valid index for this query.".format(
                 ddocid
             ),
@@ -124,7 +124,7 @@ class IndexSelectionTests:
 
         resp = self.db.find(selector, use_index=[ddocid, name], return_raw=True)
         self.assertEqual(
-            resp["warning"],
+            resp["warning"][0].lower(),
             "{0}, {1} was not used because it is not a valid index for this query.".format(
                 ddocid, name
             ),
@@ -162,7 +162,7 @@ class IndexSelectionTests:
             selector, sort=["foo", "bar"], use_index=ddocid_invalid, return_raw=True
         )
         self.assertEqual(
-            resp["warning"],
+            resp["warning"][0].lower(),
             "{0} was not used because it does not contain a valid index for this query.".format(
                 ddocid_invalid
             ),
diff --git a/src/mango/test/12-use-correct-index-test.py b/src/mango/test/12-use-correct-index-test.py
index 2de88a21a..3a2f60af8 100644
--- a/src/mango/test/12-use-correct-index-test.py
+++ b/src/mango/test/12-use-correct-index-test.py
@@ -93,8 +93,8 @@ class ChooseCorrectIndexForDocs(mango.DbPerClass):
         self.assertEqual(explain_resp["index"]["type"], "special")
         resp = self.db.find(selector, return_raw=True)
         self.assertEqual(
-            resp["warning"],
-            "no matching index found, create an index to optimize query time",
+            resp["warning"][0].lower(),
+            "no matching index found, create an index to optimize query time.",
         )
 
     def test_chooses_idxA(self):