Skip to content

Commit

Permalink
[4.3] HELP-12897: properly paginate directory's users (#6268)
Browse files Browse the repository at this point in the history
* [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.
  • Loading branch information
jamesaimonetti authored and lazedo committed Jan 24, 2020
1 parent b79decc commit 3a8ff80
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 15 deletions.
4 changes: 2 additions & 2 deletions applications/crossbar/src/modules/cb_directories.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
Expand Down Expand Up @@ -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
].
2 changes: 1 addition & 1 deletion core/kazoo_apps/priv/couchdb/account/directories.json
Original file line number Diff line number Diff line change
Expand Up @@ -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]); }}"
}
}
}
27 changes: 15 additions & 12 deletions core/kazoo_apps/src/kapps_notify_publisher.erl
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,30 @@
-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">>
,<<"voicemail_save">>
,<<"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()}.

Expand Down Expand Up @@ -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(<<NotifyType/binary>>) ->
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'
Expand Down
142 changes: 142 additions & 0 deletions core/kazoo_proper/src/pqc_cb_directories.erl
Original file line number Diff line number Diff line change
@@ -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, [<<?MODULE_STRING>>]).

-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, <<?MODULE_STRING>>}]
,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)).
35 changes: 35 additions & 0 deletions core/kazoo_proper/src/pqc_cb_users.erl
Original file line number Diff line number Diff line change
@@ -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()
).
2 changes: 2 additions & 0 deletions core/kazoo_stdlib/src/kz_term.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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()].
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 3a8ff80

Please sign in to comment.