diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2020-05-08 12:09:33 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2020-05-09 12:06:24 -0400 |
commit | 968def848f1251bc9e8d1d5c0d388d803d8837b2 (patch) | |
tree | 7134fb001789a9bc9071dee5e809432307288c0b | |
parent | d66e95afef1b25d13bad21e5597ef5a17209628a (diff) | |
download | couchdb-968def848f1251bc9e8d1d5c0d388d803d8837b2.tar.gz |
Fix couch_views updater_running info result
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.
-rw-r--r-- | src/couch_views/src/couch_views.erl | 22 | ||||
-rw-r--r-- | src/couch_views/src/couch_views_jobs.erl | 8 | ||||
-rw-r--r-- | src/couch_views/test/couch_views_info_test.erl | 60 |
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]))). |