From fd84663742b0c7484087c03651c2ca3b80332181 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:50:08 +0100 Subject: [PATCH 01/13] Update mongoose_domain_api - Return two-element tuples as it is the current convention - Fold over step functions because there are multiple common steps - Return errors on duplicate deletion/insertion. This prevents the user from interpreting the 'ok' result as successful operation if the supplied domain name was wrong. - Don't check for enabled service - it is done once for all operations now. --- src/domain/mongoose_domain_api.erl | 240 +++++++++++++--------- src/domain/mongoose_domain_db_cleaner.erl | 2 +- src/domain/mongoose_domain_sql.erl | 21 +- 3 files changed, 147 insertions(+), 116 deletions(-) diff --git a/src/domain/mongoose_domain_api.erl b/src/domain/mongoose_domain_api.erl index e0267bec9d..2901739425 100644 --- a/src/domain/mongoose_domain_api.erl +++ b/src/domain/mongoose_domain_api.erl @@ -8,20 +8,26 @@ stop/0, get_host_type/1]). -%% domain API +%% external domain API for GraphQL or REST handlers -export([insert_domain/2, delete_domain/2, request_delete_domain/2, disable_domain/1, enable_domain/1, - get_domain_host_type/1, + get_domain_details/1, + check_host_type_and_get_domains/1]). + +%% external domain admin API for GraphQL or REST handlers +-export([set_domain_password/2, + delete_domain_password/1]). + +%% domain API +-export([get_domain_host_type/1, get_all_static/0, get_domains_by_host_type/1]). %% domain admin API --export([check_domain_password/2, - set_domain_password/2, - delete_domain_password/1]). +-export([check_domain_password/2]). %% subdomain API -export([register_subdomain/3, @@ -32,7 +38,7 @@ %% Helper for remove_domain -export([remove_domain_wrapper/3, - do_delete_domain_in_progress/3]). + do_delete_domain_in_progress/2]). %% For testing -export([get_all_dynamic/0]). @@ -47,6 +53,17 @@ -type subdomain_pattern() :: mongoose_subdomain_utils:subdomain_pattern(). -type remove_domain_acc() :: #{failed := [module()]}. +-type domain_info() :: #{domain := domain(), host_type => host_type(), status => status()}. + +-type insert_result() :: {ok, domain_info()} | + {static | unknown_host_type | duplicate, iodata()}. +-type delete_result() :: {ok, domain_info()} | + {static | unknown_host_type | not_found | wrong_host_type, iodata()}. +-type set_status_result() :: {ok, domain_info()} | + {static | unknown_host_type | not_found | deleted, iodata()}. +-type get_domains_result() :: {ok, [domain()]} | {unknown_host_type, iodata()}. +-type get_domain_details_result() :: {ok, domain_info()} | {static | not_found, iodata()}. + -export_type([status/0, remove_domain_acc/0]). -spec init() -> ok | {error, term()}. @@ -64,98 +81,125 @@ stop() -> catch mongoose_lazy_routing:stop(), ok. -%% Domain should be nameprepped using `jid:nameprep'. --spec insert_domain(domain(), host_type()) -> - ok | {error, duplicate} | {error, static} | {error, {db_error, term()}} - | {error, service_disabled} | {error, unknown_host_type}. +-spec insert_domain(domain(), host_type()) -> insert_result(). insert_domain(Domain, HostType) -> - case check_domain(Domain, HostType) of - ok -> - check_db(mongoose_domain_sql:insert_domain(Domain, HostType)); - Other -> - Other - end. - --type delete_domain_return() :: - ok | {error, static} | {error, unknown_host_type} | {error, service_disabled} - | {error, {db_error, term()}} | {error, wrong_host_type} | {error, {modules_failed, [module()]}}. + M = #{domain => Domain, host_type => HostType}, + fold(M, [fun check_domain/1, fun check_host_type/1, + fun do_insert_domain/1, fun force_check/1, fun return_domain/1]). -%% Returns ok, if domain not found. -%% Domain should be nameprepped using `jid:nameprep'. --spec delete_domain(domain(), host_type()) -> delete_domain_return(). +-spec delete_domain(domain(), host_type()) -> delete_result(). delete_domain(Domain, HostType) -> - do_delete_domain(Domain, HostType, sync). + delete_domain(Domain, HostType, sync). --spec request_delete_domain(domain(), host_type()) -> delete_domain_return(). +-spec request_delete_domain(domain(), host_type()) -> delete_result(). request_delete_domain(Domain, HostType) -> - do_delete_domain(Domain, HostType, async). + delete_domain(Domain, HostType, async). -do_delete_domain(Domain, HostType, RequestType) -> - case check_domain(Domain, HostType) of - ok -> - Res0 = check_db(mongoose_domain_sql:set_domain_for_deletion(Domain, HostType)), - case Res0 of - ok -> - delete_domain_password(Domain), - do_delete_domain_in_progress(Domain, HostType, RequestType); - Other -> - Other - end; - Other -> - Other - end. +-spec delete_domain(domain(), host_type(), sync | async) -> delete_result(). +delete_domain(Domain, HostType, RequestType) -> + M = #{domain => Domain, host_type => HostType, request_type => RequestType}, + fold(M, [fun check_domain/1, fun check_host_type/1, fun set_domain_for_deletion/1, + fun force_check/1, fun do_delete_domain/1, fun return_domain/1]). -%% This is ran only in the context of `do_delete_domain', -%% so it can already skip some checks --spec do_delete_domain_in_progress(domain(), host_type(), sync | async) -> delete_domain_return(). -do_delete_domain_in_progress(Domain, HostType, async) -> - mongoose_domain_db_cleaner:request_delete_domain(Domain, HostType); -do_delete_domain_in_progress(Domain, HostType, sync) -> - case mongoose_hooks:remove_domain(HostType, Domain) of - #{failed := []} -> - check_db(mongoose_domain_sql:delete_domain(Domain, HostType)); - #{failed := Failed} -> - {error, {modules_failed, Failed}} - end. - --spec disable_domain(domain()) -> - ok | {error, not_found} | {error, static} | {error, service_disabled} - | {error, {db_error, term()}}. +-spec disable_domain(domain()) -> set_status_result(). disable_domain(Domain) -> + M = #{domain => Domain, status => disabled}, + fold(M, [fun check_domain/1, fun set_status/1, fun force_check/1, fun return_domain/1]). + +-spec enable_domain(domain()) -> set_status_result(). +enable_domain(Domain) -> + M = #{domain => Domain, status => enabled}, + fold(M, [fun check_domain/1, fun set_status/1, fun force_check/1, fun return_domain/1]). + +-spec check_host_type_and_get_domains(host_type()) -> get_domains_result(). +check_host_type_and_get_domains(HostType) -> + M = #{host_type => HostType}, + fold(M, [fun check_host_type/1, fun get_domains/1]). + +-spec get_domain_details(domain()) -> get_domain_details_result(). +get_domain_details(Domain) -> + M = #{domain => Domain}, + fold(M, [fun check_domain/1, fun select_domain/1, fun return_domain/1]). + +check_domain(M = #{domain := Domain}) -> case mongoose_domain_core:is_static(Domain) of true -> - {error, static}; + {static, <<"Domain is static">>}; false -> - case service_domain_db:enabled() of - true -> - check_db(mongoose_domain_sql:disable_domain(Domain)); - false -> - {error, service_disabled} - end + M end. --spec enable_domain(domain()) -> - ok | {error, not_found} | {error, static} | {error, service_disabled} - | {error, {db_error, term()}}. -enable_domain(Domain) -> - case mongoose_domain_core:is_static(Domain) of +check_host_type(M = #{host_type := HostType}) -> + case mongoose_domain_core:is_host_type_allowed(HostType) of true -> - {error, static}; + M; false -> - case service_domain_db:enabled() of - true -> - check_db(mongoose_domain_sql:enable_domain(Domain)); - false -> - {error, service_disabled} - end + {unknown_host_type, <<"Unknown host type">>} end. -check_db(ok) -> - %% Speedup the next check. %% It's async. +select_domain(M = #{domain := Domain}) -> + case mongoose_domain_sql:select_domain(Domain) of + {ok, DomainDetails} -> + maps:merge(M, DomainDetails); + {error, not_found} -> + {not_found, <<"Given domain does not exist">>} + end. + +do_insert_domain(M = #{domain := Domain, host_type := HostType}) -> + case mongoose_domain_sql:insert_domain(Domain, HostType) of + ok -> + M; + {error, duplicate} -> + {duplicate, <<"Domain already exists">>} + end. + +set_domain_for_deletion(M = #{domain := Domain, host_type := HostType}) -> + case mongoose_domain_sql:set_domain_for_deletion(Domain, HostType) of + ok -> + M; + {error, wrong_host_type} -> + {wrong_host_type, <<"Wrong host type was provided">>}; + {error, not_found} -> + {not_found, <<"Given domain does not exist">>} + end. + +force_check(M) -> service_domain_db:force_check_for_updates(), - ok; -check_db(Result) -> - Result. + M. + +do_delete_domain(M = #{domain := Domain, host_type := HostType, request_type := RequestType}) -> + mongoose_domain_sql:delete_domain_admin(Domain), + case RequestType of + sync -> + do_delete_domain_in_progress(Domain, HostType), + M#{status => deleted}; + async -> + mongoose_domain_db_cleaner:request_delete_domain(Domain, HostType), + M#{status => deleting} + end. + +set_status(M = #{domain := Domain, status := Status}) -> + case mongoose_domain_sql:set_status(Domain, Status) of + {error, unknown_host_type} -> + {unknown_host_type, <<"Unknown host type">>}; + {error, domain_deleted} -> + {deleted, <<"Domain has been deleted">>}; + {error, not_found} -> + {not_found, <<"Given domain does not exist">>}; + ok -> + M + end. + +return_domain(M) -> + {ok, maps:with([domain, host_type, status], M)}. + +get_domains(#{host_type := HostType}) -> + {ok, get_domains_by_host_type(HostType)}. + +-spec do_delete_domain_in_progress(domain(), host_type()) -> ok. +do_delete_domain_in_progress(Domain, HostType) -> + #{failed := []} = mongoose_hooks:remove_domain(HostType, Domain), + ok = mongoose_domain_sql:delete_domain(Domain, HostType). %% Domain should be nameprepped using `jid:nameprep' -spec get_host_type(domain()) -> @@ -202,21 +246,6 @@ get_all_dynamic() -> get_domains_by_host_type(HostType) -> mongoose_domain_core:get_domains_by_host_type(HostType). -check_domain(Domain, HostType) -> - Static = mongoose_domain_core:is_static(Domain), - Allowed = mongoose_domain_core:is_host_type_allowed(HostType), - HasDb = service_domain_db:enabled(), - if - Static -> - {error, static}; - not Allowed -> - {error, unknown_host_type}; - not HasDb -> - {error, service_disabled}; - true -> - ok - end. - -type password() :: binary(). -spec check_domain_password(domain(), password()) -> ok | {error, wrong_password | not_found}. @@ -241,18 +270,24 @@ do_check_domain_password(Password, PassDetails) -> false end. --spec set_domain_password(domain(), password()) -> ok | {error, not_found}. +-spec set_domain_password(domain(), password()) -> {ok | not_found, iodata()}. set_domain_password(Domain, Password) -> case get_host_type(Domain) of {ok, _} -> - mongoose_domain_sql:set_domain_admin(Domain, Password); + ok = mongoose_domain_sql:set_domain_admin(Domain, Password), + {ok, <<"Domain password set successfully">>}; {error, not_found} -> - {error, not_found} + {not_found, <<"Given domain does not exist or is disabled">>} end. --spec delete_domain_password(domain()) -> ok. +-spec delete_domain_password(domain()) -> {ok, iodata()}. delete_domain_password(Domain) -> - mongoose_domain_sql:delete_domain_admin(Domain). + case mongoose_domain_sql:delete_domain_admin(Domain) of + ok -> + {ok, <<"Domain password deleted successfully">>}; + {error, not_found} -> + {not_found, <<"Domain password does not exist">>} + end. -spec register_subdomain(host_type(), subdomain_pattern(), mongoose_packet_handler:t()) -> @@ -282,3 +317,8 @@ remove_domain_wrapper(Acc, F, Module) -> class => C, reason => R, stacktrace => S}), {stop, Acc#{failed := [Module | maps:get(failed, Acc)]}} end. + +fold({_, _} = Result, _) -> + Result; +fold(M, [Step | Rest]) when is_map(M) -> + fold(Step(M), Rest). diff --git a/src/domain/mongoose_domain_db_cleaner.erl b/src/domain/mongoose_domain_db_cleaner.erl index ff2bc8e3e9..a3e191f5b1 100644 --- a/src/domain/mongoose_domain_db_cleaner.erl +++ b/src/domain/mongoose_domain_db_cleaner.erl @@ -99,7 +99,7 @@ handle_timeout(_TimerRef, {do_removal, LastEventId}, State) -> handle_delete_domain(Domain, HostType, State) -> try - mongoose_domain_api:do_delete_domain_in_progress(Domain, HostType, sync) + mongoose_domain_api:do_delete_domain_in_progress(Domain, HostType) catch Class:Reason:Stacktrace -> ?LOG_ERROR(#{what => domain_deletion_failed, domain => Domain, host_type => HostType, diff --git a/src/domain/mongoose_domain_sql.erl b/src/domain/mongoose_domain_sql.erl index b474cfff51..2bcc8e1732 100644 --- a/src/domain/mongoose_domain_sql.erl +++ b/src/domain/mongoose_domain_sql.erl @@ -5,8 +5,7 @@ -export([insert_domain/2, delete_domain/2, set_domain_for_deletion/2, - disable_domain/1, - enable_domain/1]). + set_status/2]). -export([select_domain_admin/1, set_domain_admin/2, @@ -121,8 +120,6 @@ prepare_test_queries() -> insert_domain(Domain, HostType) -> transaction(fun(Pool) -> case select_domain(Domain) of - {ok, #{host_type := HT}} when HT =:= HostType -> - ok; %% ignore second call {error, not_found} -> insert_domain_settings(Pool, Domain, HostType), insert_domain_event(Pool, Domain), @@ -138,7 +135,7 @@ select_domain(Domain) -> {selected, []} -> {error, not_found}; {selected, [Row]} -> - {ok, row_to_map(Row)} + {ok, row_to_map(Row)} end. delete_domain(Domain, HostType) -> @@ -165,23 +162,17 @@ set_domain_for_deletion(Domain, HostType) -> {ok, _} -> {error, wrong_host_type}; {error, not_found} -> - ok + {error, not_found} end end). -disable_domain(Domain) -> - set_status(Domain, disabled). - -enable_domain(Domain) -> - set_status(Domain, enabled). - select_domain_admin(Domain) -> Pool = get_db_pool(), case execute_successfully(Pool, domain_select_admin, [Domain]) of {selected, []} -> {error, not_found}; {selected, [Row]} -> - {ok, Row} + {ok, Row} end. set_domain_admin(Domain, Password) -> @@ -206,7 +197,7 @@ delete_domain_admin(Domain) -> {updated, 1} = delete_domain_admin(Pool, Domain), ok; {error, not_found} -> - ok + {error, not_found} end end). @@ -330,7 +321,7 @@ set_domain_for_deletion_settings(Pool, Domain) -> ExtStatus = status_to_int(deleting), execute_successfully(Pool, domain_update_settings_status, [ExtStatus, Domain]). --spec set_status(domain(), mongoose_domain_api:status()) -> ok | {error, term()}. +-spec set_status(domain(), enabled | disabled) -> ok | {error, term()}. set_status(Domain, Status) -> transaction(fun(Pool) -> case select_domain(Domain) of From e5e337d4cf001dad5e5df771d00734e250063f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:48:21 +0100 Subject: [PATCH 02/13] Update GraphQL schema definitions for domains - Check if service_domain_db is enabled - Use stringprepping for domain names - Don't nest deleted domain - use a separate status value 'deleted' instead - Require non-empty passwords --- priv/graphql/schemas/admin/domain.gql | 46 ++++++++------------ priv/graphql/schemas/global/domain.gql | 4 +- priv/graphql/schemas/global/scalar_types.gql | 2 + 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/priv/graphql/schemas/admin/domain.gql b/priv/graphql/schemas/admin/domain.gql index d65aa790b9..56f8b435cf 100644 --- a/priv/graphql/schemas/admin/domain.gql +++ b/priv/graphql/schemas/admin/domain.gql @@ -1,40 +1,32 @@ -type DomainAdminQuery @protected{ +type DomainAdminQuery @use(services: ["service_domain_db"]) @protected{ "Get all enabled domains by hostType. Only for global admin" domainsByHostType(hostType: String!): [String!] - @protected(type: GLOBAL) + @protected(type: GLOBAL) @use "Get information about the domain" - domainDetails(domain: String!): Domain - @protected(type: DOMAIN, args: ["domain"]) + domainDetails(domain: DomainName!): Domain + @protected(type: DOMAIN, args: ["domain"]) @use } -type DomainAdminMutation @protected{ +type DomainAdminMutation @use(services: ["service_domain_db"]) @protected{ "Add new domain. Only for global admin" - addDomain(domain: String!, hostType: String!): Domain - @protected(type: GLOBAL) + addDomain(domain: DomainName!, hostType: String!): Domain + @protected(type: GLOBAL) @use "Remove domain. Only for global admin" - removeDomain(domain: String!, hostType: String!): RemoveDomainPayload - @protected(type: GLOBAL) + removeDomain(domain: DomainName!, hostType: String!): Domain + @protected(type: GLOBAL) @use "Remove domain asynchronously. Only for global admin" - requestRemoveDomain(domain: String!, hostType: String!): RemoveDomainPayload - @protected(type: GLOBAL) + requestRemoveDomain(domain: DomainName!, hostType: String!): Domain + @protected(type: GLOBAL) @use "Enable domain. Only for global admin" - enableDomain(domain: String!): Domain - @protected(type: GLOBAL) + enableDomain(domain: DomainName!): Domain + @protected(type: GLOBAL) @use "Disable domain. Only for global admin" - disableDomain(domain: String!): Domain - @protected(type: GLOBAL) + disableDomain(domain: DomainName!): Domain + @protected(type: GLOBAL) @use "Create or update domain admin password" - setDomainPassword(domain: String!, password: String!): String - @protected(type: DOMAIN, args: ["domain"]) + setDomainPassword(domain: DomainName!, password: NonEmptyString!): String + @protected(type: DOMAIN, args: ["domain"]) @use "Delete domain admin password. Only for global admin" - deleteDomainPassword(domain: String!): String - @protected(type: GLOBAL) -} - -"A result of domain removal" -type RemoveDomainPayload{ - "Success message" - msg: String - "Removed domain data" - domain: Domain + deleteDomainPassword(domain: DomainName!): String + @protected(type: GLOBAL) @use } diff --git a/priv/graphql/schemas/global/domain.gql b/priv/graphql/schemas/global/domain.gql index 19443fcd2e..4fc46dd69c 100644 --- a/priv/graphql/schemas/global/domain.gql +++ b/priv/graphql/schemas/global/domain.gql @@ -4,7 +4,7 @@ Some operation could return incomplete object i.e. some fields can be null. """ type Domain{ "Domain name" - domain: String + domain: DomainName "Domain hostType" hostType: String "Is domain enabled?" @@ -18,4 +18,6 @@ enum DomainStatus { DISABLED "Domain has been marked for deletion and is disabled until all data is removed" DELETING + "Domain is deleted and does not exist anymore" + DELETED } diff --git a/priv/graphql/schemas/global/scalar_types.gql b/priv/graphql/schemas/global/scalar_types.gql index 0c06fdfc9d..66f95b575a 100644 --- a/priv/graphql/schemas/global/scalar_types.gql +++ b/priv/graphql/schemas/global/scalar_types.gql @@ -6,6 +6,8 @@ scalar Stanza @spectaql(options: [{ key: "example", value: "Hi!" }]) scalar JID @spectaql(options: [{ key: "example", value: "alice@localhost" }]) "The JID with resource" scalar FullJID @spectaql(options: [{ key: "example", value: "alice@localhost/res1" }]) +"XMPP Domain name (domain part of a JID)" +scalar DomainName @spectaql(options: [{ key: "example", value: "localhost" }]) "String that contains at least one character" scalar NonEmptyString @spectaql(options: [{ key: "example", value: "xyz789" }]) "Integer that has a value above zero" From 5a974e667df95113e4828e61a2dea0ca98692e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:56:04 +0100 Subject: [PATCH 03/13] Check loaded services even if no host type is supplied Services have no host type, so no argument is needed in the directive. --- .../mongoose_graphql_directive_use.erl | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/graphql/directive/mongoose_graphql_directive_use.erl b/src/graphql/directive/mongoose_graphql_directive_use.erl index d6217bd5e7..a32cfb2188 100644 --- a/src/graphql/directive/mongoose_graphql_directive_use.erl +++ b/src/graphql/directive/mongoose_graphql_directive_use.erl @@ -50,21 +50,14 @@ %% are not loaded. handle_directive(#directive{id = <<"use">>, args = Args}, #schema_field{} = Field, Ctx) -> #{modules := Modules, services := Services} = UseCtx = aggregate_use_ctx(Args, Ctx), - ArgValue = get_arg_value(UseCtx, Ctx), - % Assume that loaded modules can be checked only when host type can be obtained - case host_type_from_arg(ArgValue) of - {ok, HostType} -> - UnloadedModules = filter_unloaded_modules(HostType, Modules), - UnloadedServices = filter_unloaded_services(Services), - case {UnloadedModules, UnloadedServices} of - {[], []} -> - Field; - {_, _} -> - Fun = resolve_not_loaded_fun(UnloadedModules, UnloadedServices), - Field#schema_field{resolve = Fun} - end; - {error, not_found} -> - Field + UnloadedServices = filter_unloaded_services(Services), + UnloadedModules = filter_unloaded_modules(UseCtx, Ctx, Modules), + case {UnloadedModules, UnloadedServices} of + {[], []} -> + Field; + {_, _} -> + Fun = resolve_not_loaded_fun(UnloadedModules, UnloadedServices), + Field#schema_field{resolve = Fun} end. %% @doc Collect the used modules and services to be checked for each field separately. @@ -74,8 +67,7 @@ handle_object_directive(#directive{id = <<"use">>, args = Args}, Object, Ctx) -> %% Internal --spec get_arg_value(use_ctx(), ctx()) -> - jid:jid() | jid:lserver() | mongooseim:host_type(). +-spec get_arg_value(use_ctx(), ctx()) -> jid:jid() | jid:lserver() | mongooseim:host_type(). get_arg_value(#{arg := DomainArg}, #{field_args := FieldArgs}) -> get_arg(DomainArg, FieldArgs); get_arg_value(_UseCtx, #{user := #jid{lserver = Domain}}) -> @@ -113,6 +105,19 @@ host_type_from_arg(ArgValue) -> end end. +-spec filter_unloaded_modules(use_ctx(), ctx(), [binary()]) -> [binary()]. +filter_unloaded_modules(_UseCtx, _Ctx, []) -> + []; +filter_unloaded_modules(UseCtx, Ctx, Modules) -> + ArgValue = get_arg_value(UseCtx, Ctx), + % Assume that loaded modules can be checked only when host type can be obtained + case host_type_from_arg(ArgValue) of + {ok, HostType} -> + filter_unloaded_modules(HostType, Modules); + {error, not_found} -> + [] + end. + -spec filter_unloaded_modules(host_type(), [binary()]) -> [binary()]. filter_unloaded_modules(HostType, Modules) -> lists:filter(fun(M) -> not gen_mod:is_loaded(HostType, binary_to_existing_atom(M)) end, From c7bf7f1e697f86e348e6acc8a9b781e003748c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:57:30 +0100 Subject: [PATCH 04/13] Update the resolver for the Domain type It is a bit easier (and more consistent with the remaining API) to use maps. --- src/graphql/mongoose_graphql_domain.erl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/graphql/mongoose_graphql_domain.erl b/src/graphql/mongoose_graphql_domain.erl index fd4e89690f..df5ff28c0b 100644 --- a/src/graphql/mongoose_graphql_domain.erl +++ b/src/graphql/mongoose_graphql_domain.erl @@ -4,11 +4,11 @@ -ignore_xref([execute/4]). --include("mongoose_graphql_types.hrl"). - -execute(_Ctx, #domain{host_type = HostType}, <<"hostType">>, _Args) -> +execute(_Ctx, #{host_type := HostType}, <<"hostType">>, _Args) -> {ok, HostType}; -execute(_Ctx, #domain{status = Status}, <<"status">>, _Args) -> +execute(_Ctx, #{status := Status}, <<"status">>, _Args) -> {ok, Status}; -execute(_Ctx, #domain{domain = Name}, <<"domain">>, _Args) -> - {ok, Name}. +execute(_Ctx, #{domain := Name}, <<"domain">>, _Args) -> + {ok, Name}; +execute(_Ctx, #{}, _, _Args) -> + {ok, null}. From c882634b4357dd1f44a77d6c1d20930866b86ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:58:58 +0100 Subject: [PATCH 05/13] Add the resolver for the DomainName type Don't accept empty strings. --- src/graphql/mongoose_graphql_scalar.erl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/graphql/mongoose_graphql_scalar.erl b/src/graphql/mongoose_graphql_scalar.erl index ada6113aef..305e07ddc0 100644 --- a/src/graphql/mongoose_graphql_scalar.erl +++ b/src/graphql/mongoose_graphql_scalar.erl @@ -13,6 +13,7 @@ input(<<"DateTime">>, DT) -> binary_to_microseconds(DT); input(<<"Stanza">>, Value) -> exml:parse(Value); input(<<"JID">>, Jid) -> jid_from_binary(Jid); +input(<<"DomainName">>, Domain) -> domain_from_binary(Domain); input(<<"FullJID">>, Jid) -> full_jid_from_binary(Jid); input(<<"NonEmptyString">>, Value) -> non_empty_string_to_binary(Value); input(<<"PosInt">>, Value) -> validate_pos_integer(Value); @@ -29,6 +30,7 @@ input(Ty, V) -> output(<<"DateTime">>, DT) -> {ok, microseconds_to_binary(DT)}; output(<<"Stanza">>, Elem) -> {ok, exml:to_binary(Elem)}; output(<<"JID">>, Jid) -> {ok, jid:to_binary(Jid)}; +output(<<"DomainName">>, Domain) -> {ok, Domain}; output(<<"NonEmptyString">>, Value) -> binary_to_non_empty_string(Value); output(<<"PosInt">>, Value) -> validate_pos_integer(Value); output(Ty, V) -> @@ -43,6 +45,16 @@ jid_from_binary(Value) -> {ok, Jid} end. +domain_from_binary(<<>>) -> + {error, empty_domain_name}; +domain_from_binary(Value) -> + case jid:nameprep(Value) of + error -> + {error, failed_to_parse_domain_name}; + Domain -> + {ok, Domain} + end. + full_jid_from_binary(Value) -> case jid_from_binary(Value) of {ok, #jid{lresource = <<>>}} -> From eb143f4e0b59b732a9b47dd1463bb372767b269d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:55:11 +0100 Subject: [PATCH 06/13] Update the GraphQL domain API resolvers - Use the new API, which returns error tuples --- ...mongoose_graphql_domain_admin_mutation.erl | 73 ++++--------------- .../mongoose_graphql_domain_admin_query.erl | 19 ++--- 2 files changed, 24 insertions(+), 68 deletions(-) diff --git a/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl index 6e44de5f72..a2aacaa97d 100644 --- a/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl +++ b/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl @@ -5,71 +5,26 @@ -ignore_xref([execute/4]). +-import(mongoose_graphql_helper, [format_result/2]). + -include("../mongoose_graphql_types.hrl"). execute(_Ctx, admin, <<"addDomain">>, #{<<"domain">> := Domain, <<"hostType">> := HostType}) -> - case mongoose_domain_api:insert_domain(Domain, HostType) of - ok -> - {ok, #domain{domain = Domain, host_type = HostType}}; - {error, Error} -> - error_handler(Error, Domain, HostType) - end; + format_result(mongoose_domain_api:insert_domain(Domain, HostType), + #{domain => Domain, hostType => HostType}); execute(_Ctx, admin, <<"removeDomain">>, #{<<"domain">> := Domain, <<"hostType">> := HostType}) -> - case mongoose_domain_api:delete_domain(Domain, HostType) of - ok -> - DomainObj = #domain{domain = Domain, host_type = HostType}, - {ok, #{<<"domain">> => DomainObj, <<"msg">> => <<"Domain removed!">>}}; - {error, Error} -> - error_handler(Error, Domain, HostType) - end; -execute(_Ctx, admin, <<"requestRemoveDomain">>, #{<<"domain">> := Domain, <<"hostType">> := HostType}) -> - case mongoose_domain_api:request_delete_domain(Domain, HostType) of - ok -> - DomainObj = #domain{domain = Domain, host_type = HostType, status = deleting}, - {ok, #{<<"domain">> => DomainObj, <<"msg">> => <<"Domain disabled and enqueued for deletion">>}}; - {error, Error} -> - error_handler(Error, Domain, HostType) - end; + format_result(mongoose_domain_api:delete_domain(Domain, HostType), + #{domain => Domain, hostType => HostType}); +execute(_Ctx, admin, <<"requestRemoveDomain">>, #{<<"domain">> := Domain, + <<"hostType">> := HostType}) -> + format_result(mongoose_domain_api:request_delete_domain(Domain, HostType), + #{domain => Domain, hostType => HostType}); execute(_Ctx, admin, <<"enableDomain">>, #{<<"domain">> := Domain}) -> - case mongoose_domain_api:enable_domain(Domain) of - ok -> - {ok, #domain{status = enabled, domain = Domain}}; - {error, Error} -> - error_handler(Error, Domain, <<>>) - end; + format_result(mongoose_domain_api:enable_domain(Domain), #{domain => Domain}); execute(_Ctx, admin, <<"disableDomain">>, #{<<"domain">> := Domain}) -> - case mongoose_domain_api:disable_domain(Domain) of - ok -> - {ok, #domain{status = disabled, domain = Domain}}; - {error, Error} -> - error_handler(Error, Domain, <<>>) - end; + format_result(mongoose_domain_api:disable_domain(Domain), #{domain => Domain}); execute(_Ctx, admin, <<"setDomainPassword">>, #{<<"domain">> := Domain, <<"password">> := Password}) -> - case mongoose_domain_api:set_domain_password(Domain, Password) of - ok -> - {ok, <<"Domain password set successfully">>}; - {error, Error} -> - error_handler(Error, Domain, <<>>) - end; + format_result(mongoose_domain_api:set_domain_password(Domain, Password), #{domain => Domain}); execute(_Ctx, admin, <<"deleteDomainPassword">>, #{<<"domain">> := Domain}) -> - ok = mongoose_domain_api:delete_domain_password(Domain), - {ok, <<"Domain admin deleted successfully">>}. - -error_handler(Error, Domain, HostType) -> - case {error, Error} of - {error, service_disabled} -> - {error, service_disabled}; - {error, duplicate} -> - {error, #{what => domain_duplicate, domain => Domain}}; - {error, not_found} -> - {error, #{what => domain_not_found, domain => Domain}}; - {error, static} -> - {error, #{what => domain_static, domain => Domain}}; - {error, wrong_host_type} -> - {error, #{what => wrong_host_type, host_type => HostType}}; - {error, unknown_host_type} -> - {error, #{what => unknown_host_type, host_type => HostType}}; - {error, {db_error, Term}} -> - {error, #{what => db_error, term => Term}} - end. + format_result(mongoose_domain_api:delete_domain_password(Domain), #{domain => Domain}). diff --git a/src/graphql/admin/mongoose_graphql_domain_admin_query.erl b/src/graphql/admin/mongoose_graphql_domain_admin_query.erl index 1fca40edfa..ac4448da4e 100644 --- a/src/graphql/admin/mongoose_graphql_domain_admin_query.erl +++ b/src/graphql/admin/mongoose_graphql_domain_admin_query.erl @@ -5,16 +5,17 @@ -ignore_xref([execute/4]). +-import(mongoose_graphql_helper, [format_result/2]). + -include("../mongoose_graphql_types.hrl"). execute(_Ctx, admin, <<"domainsByHostType">>, #{<<"hostType">> := HostType}) -> - Domains = mongoose_domain_api:get_domains_by_host_type(HostType), - Domains2 = lists:map(fun(D) -> {ok, D} end, Domains), - {ok, Domains2}; + case mongoose_domain_api:check_host_type_and_get_domains(HostType) of + {ok, Domains} -> + Domains2 = lists:map(fun(D) -> {ok, D} end, Domains), + {ok, Domains2}; + Error -> + format_result(Error, #{hostType => HostType}) + end; execute(_Ctx, admin, <<"domainDetails">>, #{<<"domain">> := Domain}) -> - case mongoose_domain_sql:select_domain(Domain) of - {ok, #{host_type := HostType, status := Status}} -> - {ok, #domain{host_type = HostType, domain = Domain, status = Status}}; - {error, not_found} -> - {error, #{what => domain_not_found, domain => Domain}} - end. + format_result(mongoose_domain_api:get_domain_details(Domain), #{domain => Domain}). From ffe3da6f6105bee4a91963531ee0bf9f00dbc564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:59:49 +0100 Subject: [PATCH 07/13] Update the REST API for domains - Check if the service is enabled - Use the error messages from the 2-element tuples --- .../mongoose_admin_api_domain.erl | 77 ++++++++++--------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/src/mongoose_admin_api/mongoose_admin_api_domain.erl b/src/mongoose_admin_api/mongoose_admin_api_domain.erl index f14ec62119..9566ea3ddc 100644 --- a/src/mongoose_admin_api/mongoose_admin_api_domain.erl +++ b/src/mongoose_admin_api/mongoose_admin_api_domain.erl @@ -16,7 +16,7 @@ -ignore_xref([to_json/2, from_json/2]). --import(mongoose_admin_api, [parse_body/1, try_handle_request/3, throw_error/2, resource_created/4]). +-import(mongoose_admin_api, [parse_body/1, throw_error/2, resource_created/4]). -type req() :: cowboy_req:req(). -type state() :: mongoose_admin_api:state(). @@ -78,14 +78,25 @@ delete_completed(Req, State) -> %% Internal functions +try_handle_request(Req, State, Handler) -> + F = fun(ReqIn, StateIn) -> + case service_domain_db:enabled() of + true -> Handler(ReqIn, StateIn); + false -> throw_error(denied, <<"Dynamic domains service is disabled">>) + end + end, + mongoose_admin_api:try_handle_request(Req, State, F). + handle_get(Req, State) -> Bindings = cowboy_req:bindings(Req), Domain = get_domain(Bindings), - case mongoose_domain_sql:select_domain(Domain) of + case mongoose_domain_api:get_domain_details(Domain) of {ok, Props} -> - {jiffy:encode(Props), Req, State}; - {error, not_found} -> - throw_error(not_found, <<"Domain not found">>) + {jiffy:encode(maps:with([host_type, status], Props)), Req, State}; + {not_found, Msg} -> + throw_error(not_found, Msg); + {_, Msg} -> + throw_error(denied, Msg) end. handle_put(Req, State) -> @@ -94,18 +105,12 @@ handle_put(Req, State) -> Args = parse_body(Req), HostType = get_host_type(Args), case mongoose_domain_api:insert_domain(Domain, HostType) of - ok -> + {ok, _} -> {true, Req, State}; - {error, duplicate} -> - throw_error(duplicate, <<"Duplicate domain">>); - {error, static} -> - throw_error(denied, <<"Domain is static">>); - {error, {db_error, _}} -> - throw_error(internal, <<"Database error">>); - {error, service_disabled} -> - throw_error(denied, <<"Service disabled">>); - {error, unknown_host_type} -> - throw_error(denied, <<"Unknown host type">>) + {duplicate, Msg} -> + throw_error(duplicate, Msg); + {_, Msg} -> + throw_error(denied, Msg) end. handle_patch(Req, State) -> @@ -119,16 +124,12 @@ handle_patch(Req, State) -> mongoose_domain_api:disable_domain(Domain) end, case Result of - ok -> + {ok, _} -> {true, Req, State}; - {error, not_found} -> - throw_error(not_found, <<"Domain not found">>); - {error, static} -> - throw_error(denied, <<"Domain is static">>); - {error, service_disabled} -> - throw_error(denied, <<"Service disabled">>); - {error, {db_error, _}} -> - throw_error(internal, <<"Database error">>) + {not_found, Msg} -> + throw_error(not_found, Msg); + {_, Msg} -> + throw_error(denied, Msg) end. handle_delete(Req, State) -> @@ -145,23 +146,23 @@ handle_delete(_Req, _State, _Domain, #{}) -> throw_error(bad_request, <<"'host_type' field is missing">>). async_delete(Req, State, Domain, HostType) -> - mongoose_domain_api:request_delete_domain(Domain, HostType), - {true, Req, State#{deletion => in_process}}. + case mongoose_domain_api:request_delete_domain(Domain, HostType) of + {ok, _} -> + {true, Req, State#{deletion => in_process}}; + {not_found, Msg} -> + throw_error(not_found, Msg); + {_Reason, Msg} -> + throw_error(denied, Msg) + end. sync_delete(Req, State, Domain, HostType) -> case mongoose_domain_api:delete_domain(Domain, HostType) of - ok -> + {ok, _} -> {true, Req, State}; - {error, {db_error, _}} -> - throw_error(internal, <<"Database error">>); - {error, static} -> - throw_error(denied, <<"Domain is static">>); - {error, service_disabled} -> - throw_error(denied, <<"Service disabled">>); - {error, wrong_host_type} -> - throw_error(denied, <<"Wrong host type">>); - {error, unknown_host_type} -> - throw_error(denied, <<"Unknown host type">>) + {not_found, Msg} -> + throw_error(not_found, Msg); + {_Reason, Msg} -> + throw_error(denied, Msg) end. get_domain(#{domain := Domain}) -> From 74d2aa863f1b937750ba47eb492c86b1ec3bec99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:53:57 +0100 Subject: [PATCH 08/13] Update the legacy domain CLI - Check if the service is running - Use the error messages --- src/domain/service_admin_extra_domain.erl | 57 ++++++----------------- 1 file changed, 13 insertions(+), 44 deletions(-) diff --git a/src/domain/service_admin_extra_domain.erl b/src/domain/service_admin_extra_domain.erl index b06ecb3693..97640d2085 100644 --- a/src/domain/service_admin_extra_domain.erl +++ b/src/domain/service_admin_extra_domain.erl @@ -10,9 +10,7 @@ commands/0, insert_domain/2, delete_domain/2, enable_domain/1, disable_domain/1 ]). --include("mongoose.hrl"). -include("ejabberd_commands.hrl"). --include("jlib.hrl"). -type cmd_result() :: {ok, string()} | {error, string()}. @@ -44,59 +42,30 @@ commands() -> -spec insert_domain(binary(), binary()) -> cmd_result(). insert_domain(Domain, HostType) -> SDomain = jid:nameprep(Domain), - case mongoose_domain_api:insert_domain(SDomain, HostType) of - ok -> {ok, "Added"}; - {error, duplicate} -> - {error, "Domain already exists"}; - {error, static} -> - {error, "Domain is static"}; - {error, {db_error, _}} -> - {error, "Database error"}; - {error, service_disabled} -> - {error, "Service disabled"}; - {error, unknown_host_type} -> - {error, "Unknown host type"} - end. + call_api(fun() -> mongoose_domain_api:insert_domain(SDomain, HostType) end, "Added"). -spec delete_domain(binary(), binary()) -> cmd_result(). delete_domain(Domain, HostType) -> SDomain = jid:nameprep(Domain), - case mongoose_domain_api:delete_domain(SDomain, HostType) of - ok -> {ok, "Deleted"}; - {error, {db_error, _}} -> - {error, "Database error"}; - {error, static} -> - {error, "Domain is static"}; - {error, service_disabled} -> - {error, "Service disabled"}; - {error, wrong_host_type} -> - {error, "Wrong host type"}; - {error, unknown_host_type} -> - {error, "Unknown host type"} - end. + call_api(fun() -> mongoose_domain_api:delete_domain(SDomain, HostType) end, "Deleted"). -spec enable_domain(binary()) -> cmd_result(). enable_domain(Domain) -> SDomain = jid:nameprep(Domain), - Res = mongoose_domain_api:enable_domain(SDomain), - handle_enabled_result(Res, "enabled"). + call_api(fun() -> mongoose_domain_api:enable_domain(SDomain) end, "Enabled"). -spec disable_domain(binary()) -> cmd_result(). disable_domain(Domain) -> SDomain = jid:nameprep(Domain), - Res = mongoose_domain_api:disable_domain(SDomain), - handle_enabled_result(Res, "disabled"). + call_api(fun() -> mongoose_domain_api:disable_domain(SDomain) end, "Disabled"). -handle_enabled_result(Res, OkText) -> - case Res of - ok -> - {ok, OkText}; - {error, not_found} -> - {error, "Domain not found"}; - {error, static} -> - {error, "Domain is static"}; - {error, service_disabled} -> - {error, "Service disabled"}; - {error, {db_error, _}} -> - {error, "Database error"} +call_api(F, OkMsg) -> + case service_domain_db:enabled() of + true -> + case F() of + {ok, _} -> {ok, OkMsg}; + {_Reason, ErrMsg} -> {error, ErrMsg} + end; + false -> + {error, <<"Dynamic domains service is disabled">>} end. From 95be8aec62afaddc520ec34334b1d53804750586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:58:24 +0100 Subject: [PATCH 09/13] Remove types and error handlers that are not used anymore --- src/graphql/mongoose_graphql_enum.erl | 2 -- src/graphql/mongoose_graphql_errors.erl | 20 -------------------- 2 files changed, 22 deletions(-) diff --git a/src/graphql/mongoose_graphql_enum.erl b/src/graphql/mongoose_graphql_enum.erl index 27d4095e42..8251207928 100644 --- a/src/graphql/mongoose_graphql_enum.erl +++ b/src/graphql/mongoose_graphql_enum.erl @@ -4,8 +4,6 @@ -ignore_xref([input/2, output/2]). -input(<<"DomainStatus">>, Type) -> - {ok, list_to_binary(string:to_lower(binary_to_list(Type)))}; input(<<"PresenceShow">>, Show) -> {ok, list_to_binary(string:to_lower(binary_to_list(Show)))}; input(<<"PresenceType">>, Type) -> diff --git a/src/graphql/mongoose_graphql_errors.erl b/src/graphql/mongoose_graphql_errors.erl index 6a67f74b4e..3181a99e59 100644 --- a/src/graphql/mongoose_graphql_errors.erl +++ b/src/graphql/mongoose_graphql_errors.erl @@ -15,26 +15,6 @@ %% callback invoked when resolver returns error tuple -spec err(map(), term()) -> err_msg(). -err(_Ctx, #{jid := Jid, what := unknown_user}) when is_binary(Jid) -> - #{message => <<"Given user does not exist">>, extensions => #{code => unknown_user, jid => Jid}}; -err(_Ctx, #{domain := Domain, what := unknown_domain}) when is_binary(Domain) -> - #{message => <<"Given domain does not exist">>, extensions => #{code => unknown_domain, domain => Domain}}; -err(_Ctx, #{what := bad_from_jid}) -> - #{message => <<"Sending from this JID is not allowed">>, extensions => #{code => bad_from_jid}}; -err(_Ctx, #{domain := Domain, what := domain_not_found}) -> - #{message => <<"Given domain does not exist">>, extensions => #{code => domain_not_found, domain => Domain}}; -err(_Ctx, #{domain := Domain, what := domain_duplicate}) when is_binary(Domain) -> - #{message => <<"Domain already exists">>, extensions => #{code => domain_duplicate, domain => Domain}}; -err(_Ctx, #{domain := Domain, what := domain_static}) when is_binary(Domain) -> - #{message => <<"Domain static">>, extensions => #{code => domain_static, domain => Domain}}; -err(_Ctx, #{host_type := HostType, what := unknown_host_type}) when is_binary(HostType) -> - #{message => <<"Unknown host type">>, extensions => #{code => unknown_host_type, hostType => HostType}}; -err(_Ctx, #{host_type := HostType, what := wrong_host_type}) when is_binary(HostType) -> - #{message => <<"Wrong host type">>, extensions => #{code => wrong_host_type, hostType => HostType}}; -err(_Ctx, #{term := Term, what := db_error}) -> - #{message => <<"Database error">>, extensions => #{code => db_error, term => Term}}; -err(_Ctx, #{host_type := HostType, what := service_disabled}) when is_binary(HostType) -> - #{message => <<"Service disabled">>, extensions => #{code => service_disabled, hostType => HostType}}; err(_Ctx, #resolver_error{reason = Code, msg = Msg, context = Ext}) -> #{message => iolist_to_binary(Msg), extensions => Ext#{code => Code}}; err(_Ctx, ErrorTerm) -> From ca7e5a2909cb04ecdf5ac923582e1df52894e987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:42:25 +0100 Subject: [PATCH 10/13] Use updated domain API in domain_helper --- big_tests/dynamic_domains.config | 3 ++- big_tests/tests/domain_helper.erl | 15 ++++++++------- big_tests/tests/graphql_helper.erl | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/big_tests/dynamic_domains.config b/big_tests/dynamic_domains.config index 695970a5d6..12af68cc46 100644 --- a/big_tests/dynamic_domains.config +++ b/big_tests/dynamic_domains.config @@ -30,7 +30,7 @@ {mim2, [{node, mongooseim2@localhost}, {domain, <<"domain.example.com">>}, {host_type, <<"test type">>}, - {dynamic_domains, [{<<"test type">>, [<<"domain.example.com">>]}]}, + {dynamic_domains, []}, % domains already inserted on mim {vars, "mim2"}, {cluster, mim}, {c2s_port, 5232}, @@ -41,6 +41,7 @@ {mim3, [{node, mongooseim3@localhost}, {domain, <<"domain.example.com">>}, {host_type, <<"test type">>}, + {dynamic_domains, []}, % domains already inserted on mim {vars, "mim3"}, {c2s_tls_port, 5263}, {cluster, mim}]}, diff --git a/big_tests/tests/domain_helper.erl b/big_tests/tests/domain_helper.erl index ffae07f3dc..c3865d5f32 100644 --- a/big_tests/tests/domain_helper.erl +++ b/big_tests/tests/domain_helper.erl @@ -51,28 +51,29 @@ delete_domain(Node, Domain) -> ok = rpc(Node, mongoose_domain_core, delete, [Domain]). insert_persistent_domain(Node, Domain, HostType) -> - ok = rpc(Node, mongoose_domain_api, insert_domain, [Domain, HostType]). + {ok, _} = rpc(Node, mongoose_domain_api, insert_domain, [Domain, HostType]). delete_persistent_domain(Node, Domain, HostType) -> - ok = rpc(Node, mongoose_domain_api, delete_domain, [Domain, HostType]). + {ok, _} = rpc(Node, mongoose_domain_api, delete_domain, [Domain, HostType]). set_domain_password(Node, Domain, Password) -> - ok = rpc(Node, mongoose_domain_api, set_domain_password, [Domain, Password]). + {ok, _} = rpc(Node, mongoose_domain_api, set_domain_password, [Domain, Password]). delete_domain_password(Node, Domain) -> - ok = rpc(Node, mongoose_domain_api, delete_domain_password, [Domain]). + {ok, _} = rpc(Node, mongoose_domain_api, delete_domain_password, [Domain]). for_each_configured_domain(F) -> [for_each_configured_domain(F, Opts) || {_, Opts} <- ct:get_config(hosts)], ok. for_each_configured_domain(F, Opts) -> - case proplists:get_value(dynamic_domains, Opts, []) of - [] -> - ok; + case proplists:get_value(dynamic_domains, Opts) of + undefined -> + ok; % no dynamic domains required DomainsByHostType -> Node = #{node => proplists:get_value(node, Opts)}, [F(Node, Domain, HostType) || {HostType, Domains} <- DomainsByHostType, Domain <- Domains], + rpc(Node, service_domain_db, force_check_for_updates, []), rpc(Node, service_domain_db, sync_local, []) end. diff --git a/big_tests/tests/graphql_helper.erl b/big_tests/tests/graphql_helper.erl index 791a49c339..54567722bf 100644 --- a/big_tests/tests/graphql_helper.erl +++ b/big_tests/tests/graphql_helper.erl @@ -170,7 +170,7 @@ init_domain_admin_handler(Config, Domain) -> true -> Password = base16:encode(crypto:strong_rand_bytes(8)), Creds = {<<"admin@", Domain/binary>>, Password}, - ok = domain_helper:set_domain_password(mim(), Domain, Password), + domain_helper:set_domain_password(mim(), Domain, Password), add_specs([{protocol, http}, {domain_admin, Creds}, {schema_endpoint, domain_admin} | Config]); false -> {skip, require_rdbms} From 1d8aab82415971ffce1d646692ab199cc3876721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:43:25 +0100 Subject: [PATCH 11/13] Update big tests for GraphQL domain API Update expected results: - Errors on duplicate operations - New error messages - Deleted domain is not nested --- big_tests/tests/graphql_domain_SUITE.erl | 39 ++++++++++++------------ 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/big_tests/tests/graphql_domain_SUITE.erl b/big_tests/tests/graphql_domain_SUITE.erl index 7b5c626d87..d0bf7c6d4f 100644 --- a/big_tests/tests/graphql_domain_SUITE.erl +++ b/big_tests/tests/graphql_domain_SUITE.erl @@ -103,8 +103,8 @@ create_domain(DomainName, Config) -> Result = add_domain(DomainName, ?HOST_TYPE, Config), ParsedResult = get_ok_value([data, domain, addDomain], Result), ?assertEqual(#{<<"domain">> => DomainName, - <<"hostType">> => ?HOST_TYPE, - <<"status">> => null}, ParsedResult). + <<"hostType">> => ?HOST_TYPE, + <<"status">> => null}, ParsedResult). unknown_host_type_error_formatting(Config) -> DomainName = ?EXAMPLE_DOMAIN, @@ -115,12 +115,12 @@ unknown_host_type_error_formatting(Config) -> static_domain_error_formatting(Config) -> DomainName = <<"localhost">>, Result = add_domain(DomainName, ?HOST_TYPE, Config), - ?assertEqual(<<"Domain static">>, get_err_msg(Result)). + ?assertEqual(<<"Domain is static">>, get_err_msg(Result)). domain_duplicate_error_formatting(Config) -> DomainName = ?EXAMPLE_DOMAIN, Result = add_domain(DomainName, ?SECOND_HOST_TYPE, Config), - ?assertEqual(<<"Domain already exists">>, get_err_msg(Result)). + ?assertMatch(<<"Domain already exists">>, get_err_msg(Result)). domain_not_found_error_formatting_after_mutation_enable_domain(Config) -> DomainName = <<"NonExistingDomain">>, @@ -139,7 +139,7 @@ domain_not_found_error_formatting_after_query(Config) -> wrong_host_type_error_formatting(Config) -> Result = remove_domain(?EXAMPLE_DOMAIN, ?SECOND_HOST_TYPE, Config), - ?assertEqual(<<"Wrong host type">>, get_err_msg(Result)). + ?assertEqual(<<"Wrong host type was provided">>, get_err_msg(Result)). disable_domain(Config) -> Result = disable_domain(?EXAMPLE_DOMAIN, Config), @@ -169,28 +169,27 @@ get_domain_details(Config) -> delete_domain(Config) -> Result1 = remove_domain(?EXAMPLE_DOMAIN, ?HOST_TYPE, Config), ParsedResult1 = get_ok_value([data, domain, removeDomain], Result1), - ?assertMatch(#{<<"msg">> := <<"Domain removed!">>, - <<"domain">> := #{<<"domain">> := ?EXAMPLE_DOMAIN}}, + ?assertEqual(#{<<"domain">> => ?EXAMPLE_DOMAIN, + <<"hostType">> => ?HOST_TYPE, + <<"status">> => <<"DELETED">>}, ParsedResult1), - Result2 = remove_domain(?SECOND_EXAMPLE_DOMAIN, ?HOST_TYPE, Config), - ParsedResult2 = get_ok_value([data, domain, removeDomain], Result2), - ?assertMatch(#{<<"msg">> := <<"Domain removed!">>, - <<"domain">> := #{<<"domain">> := ?SECOND_EXAMPLE_DOMAIN}}, - ParsedResult2). + Result2 = remove_domain(?EXAMPLE_DOMAIN, ?HOST_TYPE, Config), + domain_not_found_error_formatting(Result2). request_delete_domain(Config) -> - Domain = <<"exampleDomain">>, - Result1 = request_remove_domain(Domain, ?HOST_TYPE, Config), + Result1 = request_remove_domain(?SECOND_EXAMPLE_DOMAIN, ?HOST_TYPE, Config), ParsedResult1 = get_ok_value([data, domain, requestRemoveDomain], Result1), - ?assertMatch(#{<<"msg">> := <<"Domain disabled and enqueued for deletion">>, - <<"domain">> := #{<<"domain">> := Domain, - <<"status">> := <<"DELETING">>}}, + ?assertEqual(#{<<"domain">> => ?SECOND_EXAMPLE_DOMAIN, + <<"hostType">> => ?HOST_TYPE, + <<"status">> => <<"DELETING">>}, ParsedResult1), F = fun() -> - Result = get_domain_details(Domain, Config), + Result = get_domain_details(?EXAMPLE_DOMAIN, Config), domain_not_found_error_formatting(Result) end, - mongoose_helper:wait_until(F, ok, #{time_left => timer:seconds(5)}). + mongoose_helper:wait_until(F, ok, #{time_left => timer:seconds(5)}), + Result2 = request_remove_domain(?EXAMPLE_DOMAIN, ?HOST_TYPE, Config), + domain_not_found_error_formatting(Result2). get_domains_after_deletion(Config) -> Result = get_domains_by_host_type(?HOST_TYPE, Config), @@ -297,4 +296,4 @@ delete_domain_password(Domain, Config) -> %% Helpers domain_not_found_error_formatting(Result) -> - ?assertEqual(<<"Given domain does not exist">>, get_err_msg(Result)). + ?assertMatch(<<"Given domain does not exist", _/binary>>, get_err_msg(Result)). From 3a00cce124134f339cc717e8c35cabb26a69374c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 10 Nov 2022 15:46:10 +0100 Subject: [PATCH 12/13] Update big tests for domain service Update expected results: - Two-element tuples are returned from the external API - Database errors don't have special handling - Errors on duplicate operations - New error messages --- big_tests/tests/service_domain_db_SUITE.erl | 242 ++++++++++---------- 1 file changed, 123 insertions(+), 119 deletions(-) diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index ae3c004e05..0132e753ea 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -1,6 +1,7 @@ -module(service_domain_db_SUITE). -include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). -compile([export_all, nowarn_export_all]). -import(distributed_helper, [mim/0, mim2/0, mim3/0, require_rpc_nodes/1, rpc/4]). @@ -56,7 +57,7 @@ db_cases() -> [ db_disabled_domain_not_in_core, db_reenabled_domain_is_in_db, db_reenabled_domain_is_in_core, - db_can_insert_domain_twice_with_the_same_host_type, + db_cannot_insert_domain_twice_with_the_same_host_type, db_cannot_insert_domain_twice_with_another_host_type, db_cannot_insert_domain_with_unknown_host_type, db_cannot_delete_domain_with_unknown_host_type, @@ -137,7 +138,7 @@ rest_cases() -> rest_can_delete_domain, rest_request_can_delete_domain, rest_cannot_delete_domain_without_correct_type, - rest_delete_missing_domain, + rest_cannot_delete_missing_domain, rest_cannot_insert_domain_twice_with_another_host_type, rest_cannot_insert_domain_with_unknown_host_type, rest_cannot_delete_domain_with_unknown_host_type, @@ -327,13 +328,13 @@ api_lookup_not_found(_) -> {error, not_found} = get_host_type(mim(), <<"example.missing">>). api_cannot_insert_static(_) -> - {error, static} = insert_domain(mim(), <<"example.cfg">>, <<"type1">>). + {static, <<"Domain is static">>} = insert_domain(mim(), <<"example.cfg">>, <<"type1">>). api_cannot_disable_static(_) -> - {error, static} = disable_domain(mim(), <<"example.cfg">>). + {static, <<"Domain is static">>} = disable_domain(mim(), <<"example.cfg">>). api_cannot_enable_static(_) -> - {error, static} = enable_domain(mim(), <<"example.cfg">>). + {static, <<"Domain is static">>} = enable_domain(mim(), <<"example.cfg">>). %% See also db_get_all_static api_get_all_static(_) -> @@ -351,7 +352,7 @@ api_get_domains_by_host_type(_) -> %% Similar to as api_get_all_static, just with DB service enabled db_get_all_static(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), sync(), %% Could be in any order [{<<"erlang-solutions.com">>, <<"type2">>}, @@ -360,141 +361,141 @@ db_get_all_static(_) -> lists:sort(get_all_static(mim())). db_get_all_dynamic(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = insert_domain(mim(), <<"example2.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example2.db">>, <<"type1">>), sync(), [{<<"example.db">>, <<"type1">>}, {<<"example2.db">>, <<"type1">>}] = lists:sort(get_all_dynamic(mim())). db_inserted_domain_is_in_db(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), {ok, #{host_type := <<"type1">>, status := enabled}} = select_domain(mim(), <<"example.db">>). db_inserted_domain_is_in_core(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), sync(), {ok, <<"type1">>} = get_host_type(mim(), <<"example.db">>). rest_cannot_enable_deleting(Config) -> HostType = ?config(host_type, Config), Domain = <<"example.db">>, - ok = insert_domain(mim(), Domain, HostType), + {ok, _} = insert_domain(mim(), Domain, HostType), {ok, #{status := enabled}} = select_domain(mim(), Domain), - ok = request_delete_domain(mim(), Domain, HostType), + {ok, _} = request_delete_domain(mim(), Domain, HostType), {ok, #{status := deleting}} = select_domain(mim(), Domain), - {error, domain_deleted} = enable_domain(mim(), Domain), + {deleted, _} = enable_domain(mim(), Domain), Server = ?config(server, Config), Server ! continue, F = fun () -> select_domain(mim(), <<"example.db">>) end, mongoose_helper:wait_until(F, {error, not_found}, #{time_left => timer:seconds(15)}). db_deleted_domain_from_db(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = delete_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = delete_domain(mim(), <<"example.db">>, <<"type1">>), {error, not_found} = select_domain(mim(), <<"example.db">>). db_deleted_domain_fails_with_wrong_host_type(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - {error, wrong_host_type} = + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {wrong_host_type, _} = delete_domain(mim(), <<"example.db">>, <<"type2">>), {ok, #{host_type := <<"type1">>, status := enabled}} = select_domain(mim(), <<"example.db">>). db_deleted_domain_from_core(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), sync(), - ok = delete_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = delete_domain(mim(), <<"example.db">>, <<"type1">>), sync(), {error, not_found} = get_host_type(mim(), <<"example.db">>). db_disabled_domain_is_in_db(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = disable_domain(mim(), <<"example.db">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = disable_domain(mim(), <<"example.db">>), {ok, #{host_type := <<"type1">>, status := disabled}} = select_domain(mim(), <<"example.db">>). db_disabled_domain_not_in_core(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = disable_domain(mim(), <<"example.db">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = disable_domain(mim(), <<"example.db">>), sync(), {error, not_found} = get_host_type(mim(), <<"example.db">>). db_reenabled_domain_is_in_db(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = disable_domain(mim(), <<"example.db">>), - ok = enable_domain(mim(), <<"example.db">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = disable_domain(mim(), <<"example.db">>), + {ok, _} = enable_domain(mim(), <<"example.db">>), {ok, #{host_type := <<"type1">>, status := enabled}} = select_domain(mim(), <<"example.db">>). db_reenabled_domain_is_in_core(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = disable_domain(mim(), <<"example.db">>), - ok = enable_domain(mim(), <<"example.db">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = disable_domain(mim(), <<"example.db">>), + {ok, _} = enable_domain(mim(), <<"example.db">>), sync(), {ok, <<"type1">>} = get_host_type(mim(), <<"example.db">>). -db_can_insert_domain_twice_with_the_same_host_type(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>). +db_cannot_insert_domain_twice_with_the_same_host_type(_) -> + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {duplicate, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>). db_cannot_insert_domain_twice_with_another_host_type(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), - {error, duplicate} = insert_domain(mim(), <<"example.db">>, <<"type2">>). + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {duplicate, _} = insert_domain(mim(), <<"example.db">>, <<"type2">>). db_cannot_insert_domain_with_unknown_host_type(_) -> - {error, unknown_host_type} = insert_domain(mim(), <<"example.db">>, <<"type6">>). + {unknown_host_type, _} = insert_domain(mim(), <<"example.db">>, <<"type6">>). db_cannot_delete_domain_with_unknown_host_type(_) -> - ok = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), + {ok, _} = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), sync(), - {error, unknown_host_type} = delete_domain(mim(), <<"example.db">>, <<"mim2only">>). + {unknown_host_type, _} = delete_domain(mim(), <<"example.db">>, <<"mim2only">>). db_cannot_enable_domain_with_unknown_host_type(_) -> - ok = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), - ok = disable_domain(mim2(), <<"example.db">>), + {ok, _} = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), + {ok, _} = disable_domain(mim2(), <<"example.db">>), sync(), - {error, unknown_host_type} = enable_domain(mim(), <<"example.db">>). + {unknown_host_type, _} = enable_domain(mim(), <<"example.db">>). db_cannot_disable_domain_with_unknown_host_type(_) -> - ok = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), + {ok, _} = insert_domain(mim2(), <<"example.db">>, <<"mim2only">>), sync(), - {error, unknown_host_type} = disable_domain(mim(), <<"example.db">>). + {unknown_host_type, _} = disable_domain(mim(), <<"example.db">>). db_domains_with_unknown_host_type_are_ignored_by_core(_) -> - ok = insert_domain(mim2(), <<"example.com">>, <<"mim2only">>), - ok = insert_domain(mim2(), <<"example.org">>, <<"type1">>), + {ok, _} = insert_domain(mim2(), <<"example.com">>, <<"mim2only">>), + {ok, _} = insert_domain(mim2(), <<"example.org">>, <<"type1">>), sync(), {ok, <<"type1">>} = get_host_type(mim(), <<"example.org">>), %% Counter-case {error, not_found} = get_host_type(mim(), <<"example.com">>). db_can_insert_update_delete_dynamic_domain_password(_) -> Domain = <<"password-example.com">>, - ok = insert_domain(mim(), Domain, <<"type1">>), + {ok, _} = insert_domain(mim(), Domain, <<"type1">>), sync(), - ok = set_domain_password(mim(), Domain, <<"rocky1">>), + {ok, _} = set_domain_password(mim(), Domain, <<"rocky1">>), ok = check_domain_password(mim(), Domain, <<"rocky1">>), - ok = set_domain_password(mim(), Domain, <<"rocky2">>), + {ok, _} = set_domain_password(mim(), Domain, <<"rocky2">>), ok = check_domain_password(mim(), Domain, <<"rocky2">>), - ok = delete_domain_password(mim(), Domain), + {ok, _} = delete_domain_password(mim(), Domain), {error, not_found} = select_domain_admin(mim(), Domain). db_can_insert_update_delete_static_domain_password(_) -> StaticDomain = <<"example.cfg">>, - ok = set_domain_password(mim(), StaticDomain, <<"rocky1">>), + {ok, _} = set_domain_password(mim(), StaticDomain, <<"rocky1">>), ok = check_domain_password(mim(), StaticDomain, <<"rocky1">>), - ok = set_domain_password(mim(), StaticDomain, <<"rocky2">>), + {ok, _} = set_domain_password(mim(), StaticDomain, <<"rocky2">>), ok = check_domain_password(mim(), StaticDomain, <<"rocky2">>), - ok = delete_domain_password(mim(), StaticDomain), + {ok, _} = delete_domain_password(mim(), StaticDomain), {error, not_found} = select_domain_admin(mim(), StaticDomain). db_cannot_set_password_for_unknown_domain(_) -> - {error, not_found} = set_domain_password(mim(), <<"unknown_domain">>, <<>>). + {not_found, _} = set_domain_password(mim(), <<"unknown_domain">>, <<>>). db_can_check_domain_password(_) -> StaticDomain = <<"example.cfg">>, - ok = set_domain_password(mim(), StaticDomain, <<"myrock">>), + {ok, _} = set_domain_password(mim(), StaticDomain, <<"myrock">>), ok = check_domain_password(mim(), StaticDomain, <<"myrock">>), {error, wrong_password} = check_domain_password(mim(), StaticDomain, <<"wrongrock">>). @@ -503,14 +504,14 @@ db_cannot_check_password_for_unknown_domain(_) -> db_deleting_domain_deletes_domain_admin(_) -> Domain = <<"password-del-example.db">>, - ok = insert_domain(mim(), Domain, <<"type1">>), + {ok, _} = insert_domain(mim(), Domain, <<"type1">>), sync(), - ok = set_domain_password(mim(), Domain, <<"deleteme">>), - ok = delete_domain(mim(), Domain, <<"type1">>), + {ok, _} = set_domain_password(mim(), Domain, <<"deleteme">>), + {ok, _} = delete_domain(mim(), Domain, <<"type1">>), {error, not_found} = select_domain_admin(mim(), Domain). sql_select_from(_) -> - ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), [{_, <<"example.db">>, <<"type1">>}] = rpc(mim(), mongoose_domain_sql, select_from, [0, 100]). @@ -533,7 +534,7 @@ find_gaps_between(From, To) -> rpc(mim(), mongoose_domain_loader, find_gaps_between, [From, To]). db_records_are_restored_on_mim_restart(_) -> - ok = insert_domain(mim(), <<"example.com">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"type1">>), %% Simulate MIM restart service_disabled(mim()), restart_domain_core(mim(), [], [<<"type1">>]), @@ -546,8 +547,8 @@ db_records_are_restored_on_mim_restart(_) -> {ok, <<"type1">>} = get_host_type(mim(), <<"example.com">>). db_record_is_ignored_if_domain_static(_) -> - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), - ok = insert_domain(mim(), <<"example.net">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.net">>, <<"dbgroup">>), %% Simulate MIM restart service_disabled(mim()), %% Only one domain is static @@ -564,10 +565,10 @@ db_record_is_ignored_if_domain_static(_) -> db_events_table_gets_truncated(_) -> %% Configure service with a very short interval - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), - ok = insert_domain(mim(), <<"example.net">>, <<"dbgroup">>), - ok = insert_domain(mim(), <<"example.org">>, <<"dbgroup">>), - ok = insert_domain(mim(), <<"example.beta">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.net">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.org">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.beta">>, <<"dbgroup">>), Max = get_max_event_id(mim()), true = is_integer(Max), true = Max > 0, @@ -576,18 +577,18 @@ db_events_table_gets_truncated(_) -> mongoose_helper:wait_until(F, Max, #{time_left => timer:seconds(15)}). db_could_sync_between_nodes(_) -> - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), sync(), {ok, <<"dbgroup">>} = get_host_type(mim2(), <<"example.com">>). db_deleted_from_one_node_while_service_disabled_on_another(_) -> - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), sync(), {ok, <<"dbgroup">>} = get_host_type(mim2(), <<"example.com">>), %% Service is disable on the second node service_disabled(mim2()), %% Removed from the first node - ok = delete_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = delete_domain(mim(), <<"example.com">>, <<"dbgroup">>), sync_local(mim()), {error, not_found} = get_host_type(mim(), <<"example.com">>), {ok, <<"dbgroup">>} = get_host_type(mim2(), <<"example.com">>), @@ -598,7 +599,7 @@ db_deleted_from_one_node_while_service_disabled_on_another(_) -> db_inserted_from_one_node_while_service_disabled_on_another(_) -> %% Service is disable on the second node service_disabled(mim2()), - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), %% Sync is working again service_enabled(mim2()), {ok, <<"dbgroup">>} = get_host_type(mim2(), <<"example.com">>). @@ -606,15 +607,15 @@ db_inserted_from_one_node_while_service_disabled_on_another(_) -> db_reinserted_from_one_node_while_service_disabled_on_another(_) -> %% This test shows the behaviour when someone %% reinserts a domain with a different host type. - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup">>), sync(), {ok, <<"dbgroup">>} = get_host_type(mim2(), <<"example.com">>), %% Service is disable on the second node service_disabled(mim2()), %% Removed from the first node - ok = delete_domain(mim(), <<"example.com">>, <<"dbgroup">>), + {ok, _} = delete_domain(mim(), <<"example.com">>, <<"dbgroup">>), sync_local(mim()), - ok = insert_domain(mim(), <<"example.com">>, <<"dbgroup2">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"dbgroup2">>), sync_local(mim()), %% Sync is working again service_enabled(mim2()), @@ -624,7 +625,7 @@ db_reinserted_from_one_node_while_service_disabled_on_another(_) -> {ok, <<"dbgroup2">>} = get_host_type(mim(), <<"example.com">>), {ok, <<"dbgroup2">>} = get_host_type(mim2(), <<"example.com">>), %% check deleting - ok = delete_domain(mim2(), <<"example.com">>, <<"dbgroup2">>), + {ok, _} = delete_domain(mim2(), <<"example.com">>, <<"dbgroup2">>), sync(), {error, not_found} = get_host_type(mim(), <<"example.com">>), {error, not_found} = get_host_type(mim2(), <<"example.com">>). @@ -636,13 +637,13 @@ db_crash_on_initial_load_restarts_service(_) -> ok. db_out_of_sync_restarts_service(_) -> - ok = insert_domain(mim2(), <<"example1.com">>, <<"type1">>), - ok = insert_domain(mim2(), <<"example2.com">>, <<"type1">>), + {ok, _} = insert_domain(mim2(), <<"example1.com">>, <<"type1">>), + {ok, _} = insert_domain(mim2(), <<"example2.com">>, <<"type1">>), sync(), %% Pause processing events on one node suspend_service(mim()), - ok = insert_domain(mim2(), <<"example3.com">>, <<"type1">>), - ok = insert_domain(mim2(), <<"example4.com">>, <<"type1">>), + {ok, _} = insert_domain(mim2(), <<"example3.com">>, <<"type1">>), + {ok, _} = insert_domain(mim2(), <<"example4.com">>, <<"type1">>), sync_local(mim2()), %% Truncate events table, keep only one event MaxId = get_max_event_id(mim2()), @@ -673,8 +674,8 @@ db_keeps_syncing_after_cluster_join(Config) -> %% (and mongooseim application gets restarted on mim1) leave_cluster(Config), service_enabled(mim()), - ok = insert_domain(mim(), <<"example1.com">>, HostType), - ok = insert_domain(mim2(), <<"example2.com">>, HostType), + {ok, _} = insert_domain(mim(), <<"example1.com">>, HostType), + {ok, _} = insert_domain(mim2(), <<"example2.com">>, HostType), sync(), %% Nodes don't have to be clustered to sync the domains. assert_domains_are_equal(HostType), @@ -683,13 +684,13 @@ db_keeps_syncing_after_cluster_join(Config) -> join_cluster(Config), service_enabled(mim()), %% THEN Sync is successful - ok = insert_domain(mim(), <<"example3.com">>, HostType), - ok = insert_domain(mim2(), <<"example4.com">>, HostType), + {ok, _} = insert_domain(mim(), <<"example3.com">>, HostType), + {ok, _} = insert_domain(mim2(), <<"example4.com">>, HostType), sync(), assert_domains_are_equal(HostType). db_gaps_are_getting_filled_automatically(_Config) -> - ok = insert_domain(mim(), <<"example.com">>, <<"type1">>), + {ok, _} = insert_domain(mim(), <<"example.com">>, <<"type1">>), sync(), Max = get_max_event_id(mim()), %% Create a gap in events by manually adding an event @@ -755,7 +756,7 @@ cli_can_delete_domain(Config) -> cli_cannot_delete_domain_without_correct_type(Config) -> {"Added\n", 0} = mongooseimctl("insert_domain", [<<"example.db">>, <<"type1">>], Config), - {"Error: \"Wrong host type\"\n", 1} = + {"Error: \"Wrong host type was provided\"\n", 1} = mongooseimctl("delete_domain", [<<"example.db">>, <<"type2">>], Config), {ok, _} = select_domain(mim(), <<"example.db">>). @@ -774,11 +775,11 @@ cli_cannot_delete_domain_with_unknown_host_type(Config) -> mongooseimctl("delete_domain", [<<"example.db">>, <<"type6">>], Config). cli_cannot_enable_missing_domain(Config) -> - {"Error: \"Domain not found\"\n", 1} = + {"Error: \"Given domain does not exist\"\n", 1} = mongooseimctl("enable_domain", [<<"example.db">>], Config). cli_cannot_disable_missing_domain(Config) -> - {"Error: \"Domain not found\"\n", 1} = + {"Error: \"Given domain does not exist\"\n", 1} = mongooseimctl("disable_domain", [<<"example.db">>], Config). cli_cannot_insert_domain_when_it_is_static(Config) -> @@ -798,39 +799,40 @@ cli_cannot_disable_domain_when_it_is_static(Config) -> mongooseimctl("disable_domain", [<<"example.cfg">>], Config). cli_insert_domain_fails_if_db_fails(Config) -> - {"Error: \"Database error\"\n", 1} = - mongooseimctl("insert_domain", [<<"example.db">>, <<"type1">>], Config). + Res = mongooseimctl("insert_domain", [<<"example.db">>, <<"type1">>], Config), + assert_cli_db_error(Res). cli_insert_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {"Error: \"Service disabled\"\n", 1} = + {"Error: \"Dynamic domains service is disabled\"\n", 1} = mongooseimctl("insert_domain", [<<"example.db">>, <<"type1">>], Config). cli_delete_domain_fails_if_db_fails(Config) -> - {"Error: \"Database error\"\n", 1} = - mongooseimctl("delete_domain", [<<"example.db">>, <<"type1">>], Config). + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + Res = mongooseimctl("delete_domain", [<<"example.db">>, <<"type1">>], Config), + assert_cli_db_error(Res). cli_delete_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {"Error: \"Service disabled\"\n", 1} = + {"Error: \"Dynamic domains service is disabled\"\n", 1} = mongooseimctl("delete_domain", [<<"example.db">>, <<"type1">>], Config). cli_enable_domain_fails_if_db_fails(Config) -> - {"Error: \"Database error\"\n", 1} = - mongooseimctl("enable_domain", [<<"example.db">>], Config). + Res = mongooseimctl("enable_domain", [<<"example.db">>], Config), + assert_cli_db_error(Res). cli_enable_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {"Error: \"Service disabled\"\n", 1} = + {"Error: \"Dynamic domains service is disabled\"\n", 1} = mongooseimctl("enable_domain", [<<"example.db">>], Config). cli_disable_domain_fails_if_db_fails(Config) -> - {"Error: \"Database error\"\n", 1} = - mongooseimctl("disable_domain", [<<"example.db">>], Config). + Res = mongooseimctl("disable_domain", [<<"example.db">>], Config), + assert_cli_db_error(Res). cli_disable_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {"Error: \"Service disabled\"\n", 1} = + {"Error: \"Dynamic domains service is disabled\"\n", 1} = mongooseimctl("disable_domain", [<<"example.db">>], Config). rest_can_insert_domain(Config) -> @@ -851,7 +853,7 @@ rest_request_can_delete_domain(Config) -> %% Request delete domain {{<<"202">>, _}, _} = domain_rest_helper:request_delete_domain(Config, <<"example.db">>, <<"type1">>), %% Wait until it is not found anymore - Return = {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>}, + Return = {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>}, F1 = fun() -> rest_select_domain(Config, <<"example.db">>) end, mongoose_helper:wait_until(F1, Return, #{time_left => timer:seconds(15)}), %% Double-check @@ -866,21 +868,21 @@ rest_can_delete_domain(Config) -> rest_cannot_delete_domain_without_correct_type(Config) -> rest_put_domain(Config, <<"example.db">>, <<"type1">>), - {{<<"403">>, <<"Forbidden">>}, <<"Wrong host type">>} = + {{<<"403">>, <<"Forbidden">>}, <<"Wrong host type was provided">>} = rest_delete_domain(Config, <<"example.db">>, <<"type2">>), {ok, _} = select_domain(mim(), <<"example.db">>). -rest_delete_missing_domain(Config) -> - {{<<"204">>, _}, _} = +rest_cannot_delete_missing_domain(Config) -> + {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_delete_domain(Config, <<"example.db">>, <<"type1">>). rest_cannot_enable_missing_domain(Config) -> - {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = + {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_patch_enabled(Config, <<"example.db">>, true). rest_cannot_insert_domain_twice_with_another_host_type(Config) -> rest_put_domain(Config, <<"example.db">>, <<"type1">>), - {{<<"409">>, <<"Conflict">>}, <<"Duplicate domain">>} = + {{<<"409">>, <<"Conflict">>}, <<"Domain already exists">>} = rest_put_domain(Config, <<"example.db">>, <<"type2">>). rest_cannot_insert_domain_with_unknown_host_type(Config) -> @@ -956,7 +958,7 @@ rest_cannot_select_domain_without_auth(Config) -> rest_cannot_disable_missing_domain(Config) -> - {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = + {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_patch_enabled(Config, <<"example.db">>, false). rest_can_enable_domain(Config) -> @@ -973,7 +975,7 @@ rest_can_select_domain(Config) -> rest_select_domain(Config, <<"example.db">>). rest_cannot_select_domain_if_domain_not_found(Config) -> - {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = + {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_select_domain(Config, <<"example.db">>). rest_cannot_put_domain_without_host_type(Config) -> @@ -1026,32 +1028,30 @@ rest_cannot_patch_domain_with_invalid_json(Config) -> %% SQL query is mocked to fail rest_insert_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = - rest_put_domain(Config, <<"example.db">>, <<"type1">>). + assert_rest_db_error(rest_put_domain(Config, <<"example.db">>, <<"type1">>)). rest_insert_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = + {{<<"403">>, <<"Forbidden">>}, <<"Dynamic domains service is disabled">>} = rest_put_domain(Config, <<"example.db">>, <<"type1">>). %% SQL query is mocked to fail rest_delete_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = - rest_delete_domain(Config, <<"example.db">>, <<"type1">>). + {ok, _} = insert_domain(mim(), <<"example.db">>, <<"type1">>), + assert_rest_db_error(rest_delete_domain(Config, <<"example.db">>, <<"type1">>)). rest_delete_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = + {{<<"403">>, <<"Forbidden">>}, <<"Dynamic domains service is disabled">>} = rest_delete_domain(Config, <<"example.db">>, <<"type1">>). %% SQL query is mocked to fail rest_enable_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = - rest_patch_enabled(Config, <<"example.db">>, true). + assert_rest_db_error(rest_patch_enabled(Config, <<"example.db">>, true)). rest_enable_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = + {{<<"403">>, <<"Forbidden">>}, <<"Dynamic domains service is disabled">>} = rest_patch_enabled(Config, <<"example.db">>, true). rest_cannot_enable_domain_when_it_is_static(Config) -> @@ -1257,13 +1257,10 @@ maybe_setup_meck(TC) when TC =:= rest_delete_domain_fails_if_db_fails; ok = rpc(mim(), meck, expect, [mongoose_domain_sql, delete_domain, 2, {error, {db_error, simulated_db_error}}]); maybe_setup_meck(TC) when TC =:= rest_enable_domain_fails_if_db_fails; - TC =:= cli_enable_domain_fails_if_db_fails -> - ok = rpc(mim(), meck, new, [mongoose_domain_sql, [passthrough, no_link]]), - ok = rpc(mim(), meck, expect, [mongoose_domain_sql, enable_domain, 1, - {error, {db_error, simulated_db_error}}]); -maybe_setup_meck(cli_disable_domain_fails_if_db_fails) -> + TC =:= cli_enable_domain_fails_if_db_fails; + TC =:= cli_disable_domain_fails_if_db_fails -> ok = rpc(mim(), meck, new, [mongoose_domain_sql, [passthrough, no_link]]), - ok = rpc(mim(), meck, expect, [mongoose_domain_sql, disable_domain, 1, + ok = rpc(mim(), meck, expect, [mongoose_domain_sql, set_status, 2, {error, {db_error, simulated_db_error}}]); maybe_setup_meck(db_crash_on_initial_load_restarts_service) -> ok = rpc(mim(), meck, new, [mongoose_domain_sql, [passthrough, no_link]]), @@ -1301,5 +1298,12 @@ assert_domains_are_equal(HostType) -> false -> ct:fail({Domains1, Domains2}) end. +assert_cli_db_error({Msg, 1}) -> + ?assertMatch([_|_], string:find(Msg, "simulated_db_error")). + +assert_rest_db_error({Result, Msg}) -> + ?assertEqual({<<"500">>, <<"Internal Server Error">>}, Result), + ?assertEqual(<<>>, Msg). % shouldn't leak out the DB error, it's in the logs anyway + dummy_auth_host_type() -> <<"dummy auth">>. %% specified in the TOML config file From ce0c96178348f41e6f676ae7642f01a87179a8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Mon, 14 Nov 2022 14:55:28 +0100 Subject: [PATCH 13/13] Add more tests to cover error cases --- big_tests/tests/graphql_domain_SUITE.erl | 30 ++++++++++++++++++--- big_tests/tests/service_domain_db_SUITE.erl | 14 +++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/big_tests/tests/graphql_domain_SUITE.erl b/big_tests/tests/graphql_domain_SUITE.erl index d0bf7c6d4f..23fa77d458 100644 --- a/big_tests/tests/graphql_domain_SUITE.erl +++ b/big_tests/tests/graphql_domain_SUITE.erl @@ -6,7 +6,7 @@ -import(distributed_helper, [mim/0, require_rpc_nodes/1, rpc/4]). -import(graphql_helper, [execute_command/4, get_ok_value/2, get_err_msg/1, skip_null_fields/1, - execute_domain_admin_command/4, get_unauthorized/1]). + execute_domain_admin_command/4, get_unauthorized/1, get_coercion_err_msg/1]). -define(HOST_TYPE, <<"dummy auth">>). -define(SECOND_HOST_TYPE, <<"test type">>). @@ -36,6 +36,7 @@ domain_tests() -> domain_not_found_error_formatting_after_mutation_enable_domain, domain_not_found_error_formatting_after_query, wrong_host_type_error_formatting, + invalid_domain_name_error, disable_domain, enable_domain, get_domains_by_host_type, @@ -45,7 +46,8 @@ domain_tests() -> get_domains_after_deletion, set_domain_password, set_nonexistent_domain_password, - delete_domain_password + delete_domain_password, + delete_nonexistent_domain_password ]. domain_admin_tests() -> @@ -110,7 +112,9 @@ unknown_host_type_error_formatting(Config) -> DomainName = ?EXAMPLE_DOMAIN, HostType = <<"NonExistingHostType">>, Result = add_domain(DomainName, HostType, Config), - ?assertEqual(<<"Unknown host type">>, get_err_msg(Result)). + ?assertEqual(<<"Unknown host type">>, get_err_msg(Result)), + Result2 = get_domains_by_host_type(HostType, Config), + ?assertEqual(<<"Unknown host type">>, get_err_msg(Result2)). static_domain_error_formatting(Config) -> DomainName = <<"localhost">>, @@ -141,6 +145,14 @@ wrong_host_type_error_formatting(Config) -> Result = remove_domain(?EXAMPLE_DOMAIN, ?SECOND_HOST_TYPE, Config), ?assertEqual(<<"Wrong host type was provided">>, get_err_msg(Result)). +invalid_domain_name_error(Config) -> + %% One operation tested, because they all use the DomainName type + Result1 = add_domain(<<>>, ?HOST_TYPE, Config), + get_coercion_err_msg(Result1), + TooLong = binary:copy(<<$a>>, 1024), + Result2 = add_domain(TooLong, ?HOST_TYPE, Config), + get_coercion_err_msg(Result2). + disable_domain(Config) -> Result = disable_domain(?EXAMPLE_DOMAIN, Config), ParsedResult = get_ok_value([data, domain, disableDomain], Result), @@ -209,7 +221,14 @@ set_nonexistent_domain_password(Config) -> delete_domain_password(Config) -> Result = delete_domain_password(domain_helper:domain(), Config), ParsedResult = get_ok_value([data, domain, deleteDomainPassword], Result), - ?assertNotEqual(nomatch, binary:match(ParsedResult, <<"successfully">>)). + ?assertNotEqual(nomatch, binary:match(ParsedResult, <<"successfully">>)), + Result2 = delete_domain_password(domain_helper:domain(), Config), + domain_password_not_found_error_formatting(Result2). + +delete_nonexistent_domain_password(Config) -> + Domain = <<"unknown-domain.com">>, + Result = delete_domain_password(Domain, Config), + domain_password_not_found_error_formatting(Result). domain_admin_get_domain_details(Config) -> Result = get_domain_details(?DOMAIN_ADMIN_EXAMPLE_DOMAIN, Config), @@ -297,3 +316,6 @@ delete_domain_password(Domain, Config) -> domain_not_found_error_formatting(Result) -> ?assertMatch(<<"Given domain does not exist", _/binary>>, get_err_msg(Result)). + +domain_password_not_found_error_formatting(Result) -> + ?assertEqual(<<"Domain password does not exist">>, get_err_msg(Result)). diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 0132e753ea..334123bd12 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -147,6 +147,7 @@ rest_cases() -> rest_can_enable_domain, rest_can_select_domain, rest_cannot_select_domain_if_domain_not_found, + rest_cannot_select_domain_when_it_is_static, rest_cannot_put_domain_without_host_type, rest_cannot_put_domain_without_body, rest_cannot_put_domain_with_invalid_json, @@ -874,7 +875,9 @@ rest_cannot_delete_domain_without_correct_type(Config) -> rest_cannot_delete_missing_domain(Config) -> {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = - rest_delete_domain(Config, <<"example.db">>, <<"type1">>). + rest_delete_domain(Config, <<"example.db">>, <<"type1">>), + {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = + domain_rest_helper:request_delete_domain(Config, <<"example.db">>, <<"type1">>). rest_cannot_enable_missing_domain(Config) -> {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = @@ -956,7 +959,6 @@ rest_cannot_select_domain_without_auth(Config) -> {{<<"401">>, <<"Unauthorized">>}, _} = rest_select_domain(set_no_creds(Config), <<"example.db">>). - rest_cannot_disable_missing_domain(Config) -> {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_patch_enabled(Config, <<"example.db">>, false). @@ -978,6 +980,10 @@ rest_cannot_select_domain_if_domain_not_found(Config) -> {{<<"404">>, <<"Not Found">>}, <<"Given domain does not exist">>} = rest_select_domain(Config, <<"example.db">>). +rest_cannot_select_domain_when_it_is_static(Config) -> + {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = + rest_select_domain(Config, <<"example.cfg">>). + rest_cannot_put_domain_without_host_type(Config) -> {{<<"400">>, <<"Bad Request">>}, <<"'host_type' field is missing">>} = putt_domain_with_custom_body(Config, #{}). @@ -1012,7 +1018,9 @@ rest_cannot_delete_domain_with_invalid_json(Config) -> rest_cannot_delete_domain_when_it_is_static(Config) -> {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = - rest_delete_domain(Config, <<"example.cfg">>, <<"type1">>). + rest_delete_domain(Config, <<"example.cfg">>, <<"type1">>), + {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = + domain_rest_helper:request_delete_domain(Config, <<"example.cfg">>, <<"type1">>). rest_cannot_patch_domain_without_enabled_field(Config) -> {{<<"400">>, <<"Bad Request">>}, <<"'enabled' field is missing">>} =