Fix couch_views updater_running info resultimprove-updater-status-in-couch-jobs

Previously we always returned `false` because the result from
`couch_jobs:get_job_state` was expected to be just `Status`, but it is `{ok,
Status}`. That part is now explicit so we account for every possible job state
and would fail on a clause match if we get something else there.

Moved `job_state/2` function to `couch_view_jobs` to avoid duplicating the
logic on how to calculate job_id and keep it all in one module.

Tests were updated to explicitly check for each state job state.

3 files changed, 56 insertions, 34 deletions

diff --git a/src/couch_views/src/couch_views.erl b/src/couch_views/src/couch_views.erl
index 3ea4d54be..d9ba0c16b 100644
--- a/src/couch_views/src/couch_views.erl
+++ b/src/couch_views/src/couch_views.erl
@@ -99,21 +99,17 @@ get_info(Db, DDoc) ->
     DbName = fabric2_db:name(Db),
     {ok, Mrst} = couch_views_util:ddoc_to_mrst(DbName, DDoc),
     Sig = fabric2_util:to_hex(Mrst#mrst.sig),
-    JobId = <<DbName/binary, "-", Sig/binary>>,
-    {UpdateSeq, DataSize, Status0} = fabric2_fdb:transactional(Db, fun(TxDb) ->
-        #{
-            tx := Tx
-        } = TxDb,
+    {UpdateSeq, DataSize, Status} = fabric2_fdb:transactional(Db, fun(TxDb) ->
         Seq = couch_views_fdb:get_update_seq(TxDb, Mrst),
         DataSize = get_total_view_size(TxDb, Mrst),
-        Status = couch_jobs:get_job_state(Tx, ?INDEX_JOB_TYPE, JobId),
-        {Seq, DataSize, Status}
+        JobStatus = case couch_views_jobs:job_state(TxDb, Mrst) of
+            {ok, pending} -> true;
+            {ok, running} -> true;
+            {ok, finished} -> false;
+            {error, not_found} -> false
+        end,
+        {Seq, DataSize, JobStatus}
     end),
-    Status1 = case Status0 of
-        pending -> true;
-        running -> true;
-        _ -> false
-    end,
     UpdateOptions = get_update_options(Mrst),
     {ok, [
         {language, Mrst#mrst.language},
@@ -122,7 +118,7 @@ get_info(Db, DDoc) ->
             {active, DataSize}
         ]}},
         {update_seq, UpdateSeq},
-        {updater_running, Status1},
+        {updater_running, Status},
         {update_options, UpdateOptions}
     ]}.
 
diff --git a/src/couch_views/src/couch_views_jobs.erl b/src/couch_views/src/couch_views_jobs.erl
index d0de44ea8..909e9234f 100644
--- a/src/couch_views/src/couch_views_jobs.erl
+++ b/src/couch_views/src/couch_views_jobs.erl
@@ -16,7 +16,8 @@
     set_timeout/0,
     build_view/3,
     build_view_async/2,
-    remove/2
+    remove/2,
+    job_state/2
 ]).
 
 -ifdef(TEST).
@@ -67,6 +68,11 @@ remove(TxDb, Sig) ->
     couch_jobs:remove(TxDb, ?INDEX_JOB_TYPE, JobId).
 
 
+job_state(#{} = TxDb, #mrst{} = Mrst) ->
+    JobId = job_id(TxDb, Mrst),
+    couch_jobs:get_job_state(TxDb, ?INDEX_JOB_TYPE, JobId).
+
+
 ensure_correct_tx(#{tx := undefined} = TxDb) ->
     TxDb;
 
diff --git a/src/couch_views/test/couch_views_info_test.erl b/src/couch_views/test/couch_views_info_test.erl
index 777cdb3dc..993801a0d 100644
--- a/src/couch_views/test/couch_views_info_test.erl
+++ b/src/couch_views/test/couch_views_info_test.erl
@@ -45,8 +45,7 @@ foreach_setup() ->
     {ok, _} = fabric2_db:update_doc(Db, Doc1, []),
 
     run_query(Db, DDoc, ?MAP_FUN1),
-    {ok, Info} = couch_views:get_info(Db, DDoc),
-    {Db, Info}.
+    {Db, DDoc}.
 
 
 foreach_teardown({Db, _}) ->
@@ -66,41 +65,62 @@ views_info_test_() ->
                 fun foreach_setup/0,
                 fun foreach_teardown/1,
                 [
-                    fun sig_is_binary/1,
-                    fun language_is_js/1,
-                    fun update_seq_is_binary/1,
-                    fun updater_running_is_boolean/1,
-                    fun active_size_is_non_neg_int/1,
-                    fun update_opts_is_bin_list/1
+                    ?TDEF_FE(sig_is_binary),
+                    ?TDEF_FE(language_is_js),
+                    ?TDEF_FE(update_seq_is_binary),
+                    ?TDEF_FE(updater_running_is_boolean),
+                    ?TDEF_FE(active_size_is_non_neg_int),
+                    ?TDEF_FE(update_opts_is_bin_list)
                 ]
             }
         }
     }.
 
 
-sig_is_binary({_, Info}) ->
-    ?_assert(is_binary(prop(signature, Info))).
+sig_is_binary({Db, DDoc}) ->
+    {ok, Info} = couch_views:get_info(Db, DDoc),
+    ?assert(is_binary(prop(signature, Info))).
+
+
+language_is_js({Db, DDoc}) ->
+    {ok, Info} = couch_views:get_info(Db, DDoc),
+    ?assertEqual(<<"javascript">>, prop(language, Info)).
+
 
+active_size_is_non_neg_int({Db, DDoc}) ->
+    {ok, Info} = couch_views:get_info(Db, DDoc),
+    ?assert(check_non_neg_int([sizes, active], Info)).
 
-language_is_js({_, Info}) ->
-    ?_assertEqual(<<"javascript">>, prop(language, Info)).
 
+updater_running_is_boolean({Db, DDoc}) ->
+    meck:new(couch_jobs, [passthrough]),
 
-active_size_is_non_neg_int({_, Info}) ->
-    ?_assert(check_non_neg_int([sizes, active], Info)).
+    meck:expect(couch_jobs, get_job_state, 3, meck:val({ok, running})),
+    {ok, Info1} = couch_views:get_info(Db, DDoc),
+    ?assert(prop(updater_running, Info1)),
 
+    meck:expect(couch_jobs, get_job_state, 3, meck:val({ok, pending})),
+    {ok, Info2} = couch_views:get_info(Db, DDoc),
+    ?assert(prop(updater_running, Info2)),
 
-updater_running_is_boolean({_, Info}) ->
-    ?_assert(is_boolean(prop(updater_running, Info))).
+    meck:expect(couch_jobs, get_job_state, 3, meck:val({ok, finished})),
+    {ok, Info3} = couch_views:get_info(Db, DDoc),
+    ?assert(not prop(updater_running, Info3)),
 
+    meck:expect(couch_jobs, get_job_state, 3, meck:val({error, not_found})),
+    {ok, Info4} = couch_views:get_info(Db, DDoc),
+    ?assert(not prop(updater_running, Info4)).
 
-update_seq_is_binary({_, Info}) ->
-    ?_assert(is_binary(prop(update_seq, Info))).
 
+update_seq_is_binary({Db, DDoc}) ->
+    {ok, Info} = couch_views:get_info(Db, DDoc),
+    ?assert(is_binary(prop(update_seq, Info))).
 
-update_opts_is_bin_list({_, Info}) ->
+
+update_opts_is_bin_list({Db, DDoc}) ->
+    {ok, Info} = couch_views:get_info(Db, DDoc),
     Opts = prop(update_options, Info),
-    ?_assert(is_list(Opts) andalso
+    ?assert(is_list(Opts) andalso
             (Opts == [] orelse lists:all([is_binary(B) || B <- Opts]))).