From 3a8ff8086be771a6f3c3a8ca451398706c2d9eaa Mon Sep 17 00:00:00 2001 From: James Aimonetti Date: Fri, 24 Jan 2020 14:43:04 -0800 Subject: [PATCH] [4.3] HELP-12897: properly paginate directory's users (#6268) * [4.3] HELP-12897: properly paginate directory's users Prior, the view only indexed by the directory IDs on a user's doc. When `paginate=false` was included on a fetch of the directory, the directory/users_listing view did not interact nicely with the internal pagination protections added. The view arguments used `[{key, DirectoryId}, {limit, 50}]`; the first page would be returned and since the next `startkey` would be `DirectoryId` again, crossbar_doc gets stuck in a loop as there was no way to "increment" the startkey. Instead, the view now emits `[DirectoryId, UserId]` to allow differentiation between rows, and view options are now passed in as `[{startkey, [DirectoryId]}]`. This plays much nicer with pagination regardless of whether pagination has been disabled or not. --- .../crossbar/src/modules/cb_directories.erl | 4 +- .../priv/couchdb/account/directories.json | 2 +- .../kazoo_apps/src/kapps_notify_publisher.erl | 27 ++-- core/kazoo_proper/src/pqc_cb_directories.erl | 142 ++++++++++++++++++ core/kazoo_proper/src/pqc_cb_users.erl | 35 +++++ core/kazoo_stdlib/src/kz_term.erl | 2 + 6 files changed, 197 insertions(+), 15 deletions(-) create mode 100644 core/kazoo_proper/src/pqc_cb_directories.erl create mode 100644 core/kazoo_proper/src/pqc_cb_users.erl diff --git a/applications/crossbar/src/modules/cb_directories.erl b/applications/crossbar/src/modules/cb_directories.erl index b24a2523b5f..89665899c94 100644 --- a/applications/crossbar/src/modules/cb_directories.erl +++ b/applications/crossbar/src/modules/cb_directories.erl @@ -300,7 +300,7 @@ read(Id, Context) -> -spec load_directory_users(kz_term:ne_binary(), cb_context:context()) -> cb_context:context(). load_directory_users(Id, Context) -> Context1 = crossbar_doc:load_view(?CB_USERS_LIST - ,[{'key', Id}] + ,[{'startkey', [Id]}] ,Context ,fun normalize_users_results/2 ), @@ -363,7 +363,7 @@ normalize_view_results(JObj, Acc) -> -spec normalize_users_results(kz_json:object(), kz_json:objects()) -> kz_json:objects(). normalize_users_results(JObj, Acc) -> [kz_json:from_list([{<<"user_id">>, kz_doc:id(JObj)} - ,{<<"callflow_id">>, kz_json:get_value(<<"value">>, JObj)} + ,{<<"callflow_id">>, kz_json:get_ne_binary_value(<<"value">>, JObj)} ]) | Acc ]. diff --git a/core/kazoo_apps/priv/couchdb/account/directories.json b/core/kazoo_apps/priv/couchdb/account/directories.json index a5acf5e37d3..705637ffe38 100644 --- a/core/kazoo_apps/priv/couchdb/account/directories.json +++ b/core/kazoo_apps/priv/couchdb/account/directories.json @@ -13,7 +13,7 @@ "map": "function(doc) { if (doc.pvt_type != 'directory' || doc.pvt_deleted) return; emit(doc._id, {'id': doc._id, 'name': doc.name, 'flags': doc.flags || []}); }" }, "users_listing": { - "map": "function(doc) { if ( doc.pvt_deleted || typeof doc.directories !== 'object' ) return; for ( i in doc.directories ) { emit(i, doc.directories[i]); }}" + "map": "function(doc) { if ( doc.pvt_deleted || typeof doc.directories !== 'object' ) return; for ( directory_id in doc.directories ) { emit([directory_id, doc._id], doc.directories[directory_id]); }}" } } } diff --git a/core/kazoo_apps/src/kapps_notify_publisher.erl b/core/kazoo_apps/src/kapps_notify_publisher.erl index c3460c5d11e..45f7961741f 100644 --- a/core/kazoo_apps/src/kapps_notify_publisher.erl +++ b/core/kazoo_apps/src/kapps_notify_publisher.erl @@ -19,11 +19,12 @@ -define(DEFAULT_TIMEOUT, 30 * ?MILLISECONDS_IN_SECOND). -define(TIMEOUT, kapps_config:get_pos_integer(?NOTIFY_CAT, <<"notify_publisher_timeout_ms">>, ?DEFAULT_TIMEOUT)). --define(DEFAULT_PUBLISHER_ENABLED, - kapps_config:get_is_true(?NOTIFY_CAT, <<"notify_persist_enabled">>, true) +-define(DEFAULT_PUBLISHER_ENABLED + ,kapps_config:get_is_true(?NOTIFY_CAT, <<"notify_persist_enabled">>, 'true') ). --define(ACCOUNT_SHOULD_PERSIST(AccountId), - kapps_account_config:get_global(AccountId, ?NOTIFY_CAT, <<"should_persist_for_retry">>, true) + +-define(ACCOUNT_SHOULD_PERSIST(AccountId) + ,kapps_account_config:get_global(AccountId, ?NOTIFY_CAT, <<"should_persist_for_retry">>, 'true') ). -define(DEFAULT_TYPE_EXCEPTION, [<<"system_alert">> @@ -31,15 +32,17 @@ ,<<"register">> ,<<"webhook">> ]). --define(GLOBAL_FORCE_NOTIFY_TYPE_EXCEPTION, - kapps_config:get_ne_binaries(?NOTIFY_CAT, <<"notify_persist_temporary_force_exceptions">>, []) +-define(GLOBAL_FORCE_NOTIFY_TYPE_EXCEPTION + ,kapps_config:get_ne_binaries(?NOTIFY_CAT, <<"notify_persist_temporary_force_exceptions">>, []) ). --define(NOTIFY_TYPE_EXCEPTION(AccountId), - kapps_account_config:get_global(AccountId, ?NOTIFY_CAT, <<"notify_persist_exceptions">>, ?DEFAULT_TYPE_EXCEPTION) + +-define(NOTIFY_TYPE_EXCEPTION(AccountId) + ,kapps_account_config:get_global(AccountId, ?NOTIFY_CAT, <<"notify_persist_exceptions">>, ?DEFAULT_TYPE_EXCEPTION) ). --define(DEFAULT_RETRY_PERIOD, - kapps_config:get_integer(<<"tasks.notify_resend">>, <<"retry_after_fudge_s">>, 10 * ?SECONDS_IN_MINUTE)). +-define(DEFAULT_RETRY_PERIOD + ,kapps_config:get_integer(<<"tasks.notify_resend">>, <<"retry_after_fudge_s">>, 10 * ?SECONDS_IN_MINUTE) + ). -type failure_reason() :: {kz_term:ne_binary(), kz_term:api_object()}. @@ -315,12 +318,12 @@ cast_to_binary(Error) -> -spec notify_type(kz_amqp_worker:publish_fun() | kz_term:ne_binary()) -> kz_term:api_ne_binary(). notify_type(<<"publish_", NotifyType/binary>>) -> NotifyType; -notify_type(NotifyType) when is_binary(NotifyType) -> +notify_type(<>) -> lager:error("unknown notification publish function ~s", [NotifyType]), 'undefined'; notify_type(PublishFun) -> case catch erlang:fun_info_mfa(PublishFun) of - {kapi_notifications, Fun, 1} -> notify_type(cast_to_binary(Fun)); + {'kapi_notifications', Fun, 1} -> notify_type(cast_to_binary(Fun)); _Other -> lager:error("unknown notification publish function: ~p", [_Other]), 'undefined' diff --git a/core/kazoo_proper/src/pqc_cb_directories.erl b/core/kazoo_proper/src/pqc_cb_directories.erl new file mode 100644 index 00000000000..7adcd5a7b21 --- /dev/null +++ b/core/kazoo_proper/src/pqc_cb_directories.erl @@ -0,0 +1,142 @@ +%%%----------------------------------------------------------------------------- +%%% @copyright (C) 2011-2020, 2600Hz +%%% @doc Test the directories API endpoint +%%% @end +%%%----------------------------------------------------------------------------- +-module(pqc_cb_directories). + +-export([create/3 + ,fetch/3, fetch/4 + ]). + +-export([seq/0 + ,cleanup/0 + ]). + +-include("kazoo_proper.hrl"). + +-define(ACCOUNT_NAMES, [<>]). + +-spec create(pqc_cb_api:state(), kz_term:ne_binary(), kzd_directories:doc()) -> pqc_cb_api:response(). +create(API, AccountId, Directory) -> + URL = directories_url(AccountId), + RequestHeaders = pqc_cb_api:request_headers(API), + RequestEnvelope = pqc_cb_api:create_envelope(Directory), + + pqc_cb_api:make_request([#{'response_codes' => [201]}] + ,fun kz_http:put/3 + ,URL + ,RequestHeaders + ,kz_json:encode(RequestEnvelope) + ). + +-spec fetch(pqc_cb_api:state(), kz_term:ne_binary(), kz_term:ne_binary()) -> pqc_cb_api:response(). +fetch(API, AccountId, DirectoryId) -> + fetch(API, AccountId, DirectoryId, 'undefined'). + +-spec fetch(pqc_cb_api:state(), kz_term:ne_binary(), kz_term:ne_binary(), kz_term:api_proplist()) -> pqc_cb_api:response(). +fetch(API, AccountId, DirectoryId, QueryString) -> + URL = directory_url(AccountId, DirectoryId) ++ querystring(QueryString), + RequestHeaders = pqc_cb_api:request_headers(API), + + pqc_cb_api:make_request([#{'response_codes' => [200]}] + ,fun kz_http:get/2 + ,URL + ,RequestHeaders + ). + +directories_url(AccountId) -> + string:join([pqc_cb_accounts:account_url(AccountId), "directories"], "/"). + +directory_url(AccountId, DirectoryId) -> + string:join([directories_url(AccountId), kz_term:to_list(DirectoryId)], "/"). + +querystring('undefined') -> ""; +querystring([]) -> ""; +querystring(QS) -> ["?", kz_http_util:props_to_querystring(QS)]. + +-spec seq() -> 'ok'. +seq() -> + Model = initial_state(), + API = pqc_kazoo_model:api(Model), + + AccountResp = pqc_cb_accounts:create_account(API, hd(?ACCOUNT_NAMES)), + lager:info("created account: ~s", [AccountResp]), + + AccountId = kz_json:get_value([<<"data">>, <<"id">>], kz_json:decode(AccountResp)), + + CreateResp = create(API, AccountId, directories_doc()), + lager:info("created directory: ~s", [CreateResp]), + Directory = kz_json:get_json_value(<<"data">>, kz_json:decode(CreateResp)), + DirectoryId = kz_doc:id(Directory), + + Users = create_users(API, AccountId, DirectoryId), + UserIds = [kz_doc:id(User) || User <- Users], + + FetchResp = fetch(API, AccountId, DirectoryId, [{<<"paginate">>, 'false'}]), + lager:info("fetched directory: ~s", [FetchResp]), + FetchedUsers = kzd_directories:users(kz_json:get_json_value(<<"data">>, kz_json:decode(FetchResp))), + lager:info("fetched users: ~p", [FetchedUsers]), + lager:info("user ids: ~p", [UserIds]), + + 'true' = length(UserIds) =:= length(FetchedUsers), + 'true' = lists:all(fun(FetchedUser) -> + lists:member(kz_json:get_ne_binary_value(<<"user_id">>, FetchedUser) + ,UserIds + ) + end + ,FetchedUsers + ), + + cleanup(API), + lager:info("FINISHED SEQ"). + +-spec initial_state() -> pqc_kazoo_model:model(). +initial_state() -> + _ = init_system(), + API = pqc_cb_api:authenticate(), + pqc_kazoo_model:new(API). + +init_system() -> + TestId = kz_binary:rand_hex(5), + kz_util:put_callid(TestId), + + _ = kz_data_tracing:clear_all_traces(), + _ = [kapps_controller:start_app(App) || + App <- ['crossbar'] + ], + _ = [crossbar_maintenance:start_module(Mod) || + Mod <- ['cb_directories', 'cb_users_v2'] + ], + lager:info("INIT FINISHED"). + +-spec cleanup() -> 'ok'. +cleanup() -> + _ = pqc_cb_accounts:cleanup_accounts(?ACCOUNT_NAMES), + cleanup_system(). + +cleanup(API) -> + lager:info("CLEANUP TIME, EVERYBODY HELPS"), + _ = pqc_cb_accounts:cleanup_accounts(API, ?ACCOUNT_NAMES), + _ = pqc_cb_api:cleanup(API), + cleanup_system(). + +cleanup_system() -> 'ok'. + +directories_doc() -> + kz_json:exec_first( + [{fun kzd_directories:set_name/2, <>}] + ,kzd_directories:new() + ). + +-spec create_users(pqc_cb_api:state(), kz_term:ne_binary(), kz_term:ne_binary()) -> [kzd_users:doc()]. +create_users(API, AccountId, DirectoryId) -> + [create_user(API, AccountId, DirectoryId, N) || N <- lists:seq(1, 100)]. + +-spec create_user(pqc_cb_api:state(), kz_term:ne_binary(), kz_term:ne_binary(), 1..100) -> kzd_users:doc(). +create_user(API, AccountId, DirectoryId, NthUser) -> + Directories = kz_json:from_list([{DirectoryId, kz_binary:rand_hex(16)}]), + UserDoc = kzd_users:set_directories(pqc_cb_users:user_doc(), Directories), + + Results = pqc_cb_users:create(API, AccountId, kz_json:set_value(<<"nth">>, NthUser, UserDoc)), + kz_json:get_json_value(<<"data">>, kz_json:decode(Results)). diff --git a/core/kazoo_proper/src/pqc_cb_users.erl b/core/kazoo_proper/src/pqc_cb_users.erl new file mode 100644 index 00000000000..ddcae87dc81 --- /dev/null +++ b/core/kazoo_proper/src/pqc_cb_users.erl @@ -0,0 +1,35 @@ +%%%----------------------------------------------------------------------------- +%%% @copyright (C) 2011-2020, 2600Hz +%%% @doc Test the directories API endpoint +%%% @end +%%%----------------------------------------------------------------------------- +-module(pqc_cb_users). + +-export([create/3]). + +-export([user_doc/0]). + +-spec create(pqc_cb_api:state(), kz_term:ne_binary(), kzd_users:doc()) -> pqc_cb_api:response(). +create(API, AccountId, User) -> + URL = users_url(AccountId), + RequestHeaders = pqc_cb_api:request_headers(API), + RequestEnvelope = pqc_cb_api:create_envelope(User), + + pqc_cb_api:make_request([#{'response_codes' => [201]}] + ,fun kz_http:put/3 + ,URL + ,RequestHeaders + ,kz_json:encode(RequestEnvelope) + ). + +users_url(AccountId) -> + string:join([pqc_cb_accounts:account_url(AccountId), "users"], "/"). + +-spec user_doc() -> kzd_users:doc(). +user_doc() -> + kz_json:exec_first( + [{fun kzd_users:set_first_name/2, kz_binary:rand_hex(5)} + ,{fun kzd_users:set_last_name/2, kz_binary:rand_hex(5)} + ] + ,kzd_users:new() + ). diff --git a/core/kazoo_stdlib/src/kz_term.erl b/core/kazoo_stdlib/src/kz_term.erl index 1d4b12355f3..9e5eba35c61 100644 --- a/core/kazoo_stdlib/src/kz_term.erl +++ b/core/kazoo_stdlib/src/kz_term.erl @@ -66,6 +66,7 @@ %% Denotes definition of each key-value in a proplist. -type proplist() :: [proplist_property()]. +-type api_proplist() :: proplist() | 'undefined'. %% A key-value form of data, `[{Key, Value}|atom]'. -type proplists() :: [proplist()]. @@ -180,6 +181,7 @@ ,api_pid_ref/0 ,api_pid_refs/0 ,api_pos_integer/0 + ,api_proplist/0 ,api_reference/0 ,api_string/0 ,api_terms/0