GraphQL - Check loaded modules #3719
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report
@@ Coverage Diff @@
## master #3719 +/- ##
==========================================
+ Coverage 82.24% 82.25% +0.01%
==========================================
Files 523 526 +3
Lines 33801 33853 +52
==========================================
+ Hits 27799 27847 +48
- Misses 6002 6006 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql-check-loaded-modules
branch
from
8950a71
to
c8c4fee
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql-check-loaded-modules
branch
from
c8c4fee
to
570d40b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql-check-loaded-modules
branch
from
570d40b
to
240bfab
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql-check-loaded-modules
branch
from
240bfab
to
2263e4f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This is an example that shows how to use this directive.
Premwoik
force-pushed
the
graphql-check-loaded-modules
branch
from
2263e4f
to
f38d746
Compare
|
small_tests_25 / small_tests / f38d746 small_tests_24 / small_tests / f38d746 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f38d746 ldap_mnesia_24 / ldap_mnesia / f38d746 dynamic_domains_mysql_redis_25 / mysql_redis / f38d746 pgsql_mnesia_24 / pgsql_mnesia / f38d746 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / f38d746 ldap_mnesia_25 / ldap_mnesia / f38d746 internal_mnesia_25 / internal_mnesia / f38d746 pgsql_mnesia_25 / pgsql_mnesia / f38d746 riak_mnesia_24 / riak_mnesia / f38d746 mysql_redis_25 / mysql_redis / f38d746 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / f38d746 mssql_mnesia_25 / odbc_mssql_mnesia / f38d746 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / f38d746 graphql_roster_SUITE:user_roster:user_invite_accept_and_cancel_subscription{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bob_user_invite_accept_and_cancel_subscription_912@localhost/res1">>,
escalus_tcp,<0.18909.0>,
[{event_manager,<0.18907.0>},
{server,<<"localhost">>},
{username,
<<"bOb_user_invite_accept_and_cancel_subscription_912">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.18907.0>},
{server,<<"localhost">>},
{username,
<<"bOb_user_invite_accept_and_cancel_subscription_912">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"bob_user_invite_accept_and_cancel_subscription_912">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"bOb_user_invite_accept_and_cancel_subscription_912">>},
{server,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"cd6522193143271e">>}]},
1],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{graphql_roster_SUITE,
user_invite_accept_and_cancel_subscription_story,3,
[{file,
"/home/circleci/project/big_tests/tests/graphql_roster_SUITE...elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / f38d746 |
| Field#schema_field{resolve = Fun} | ||
| end; | ||
| {error, not_found} -> | ||
| Field |
There was a problem hiding this comment.
Wouldn't it be better to return something like "Invalid domain or host type"? For me it looks counter-intuitive that we skip the check as if everything was fine.
There was a problem hiding this comment.
I was thinking about it and decided not to return an error here because we check domain/host type again in the API modules. The error returned from API modules is more precise because we know there if the domain or host type is nonexistent.
If we return an error here, then we stop testing the error returned from API modules because we override it. If this is not a problem, then I can return an error here. Tests will require small adjustments because the error message will change.
There was a problem hiding this comment.
We have to create a follow-up ticket to add the use directive to other categories. I think we can add returning an error there if we decide to do it. What do you think?
This comment was marked as outdated.
This comment was marked as outdated.
Turns out that introspection fields' schema is not a schema_field() type.
|
small_tests_24 / small_tests / 4f80f7f small_tests_25 / small_tests / 4f80f7f dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4f80f7f ldap_mnesia_24 / ldap_mnesia / 4f80f7f dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4f80f7f dynamic_domains_mysql_redis_25 / mysql_redis / 4f80f7f ldap_mnesia_25 / ldap_mnesia / 4f80f7f internal_mnesia_25 / internal_mnesia / 4f80f7f pgsql_mnesia_24 / pgsql_mnesia / 4f80f7f elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 4f80f7f pgsql_mnesia_25 / pgsql_mnesia / 4f80f7f mysql_redis_25 / mysql_redis / 4f80f7f bosh_SUITE:essential:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"080aee48127221702109b868f21d1eea8edc25f8">>,
<9154.6124.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}mssql_mnesia_25 / odbc_mssql_mnesia / 4f80f7f riak_mnesia_24 / riak_mnesia / 4f80f7f mysql_redis_25 / mysql_redis / 4f80f7f |
This PR addresses MIM-1671 and adds support for custom directives. It introduces the
mongoose_graphql_directivemodule that processes the schema directives. It traverses the AST, and delegates processing directives to the suitable handlers that implement the behavior. The directives have the ability to modify the annotated note. Currently, we have two handlers:@protecteddirective,@usedirective.The mongoose_graphql_directive behavior consists of two callbacks:
handle_directive- handles field directives,handle_object_directives- handles object directives.Use directive
The loaded modules are checked per command so all commands have to be annotated with the directive. This is because we need to obtain the domain from the argument (an admin case). A user endpoint can obtain a domain from an authorized user.
However, categories also can be annotated and args will be aggregated and checked per command. Take a look at an example taken from the user schema:
I annotated a
mod_lastandmod_inboxwith theusedirective for an example. We still need to add it to other categories.In addition, as we have already checked if the domain or host type exists, we do not need to do it again in the resolvers. But currently, when the domain is nonexistent, we skip checking loaded modules and use delegate handling this case to the original field resolver.
Removed
Previously only the
@protectedwas supported. We use it for checking permissions, it was handled in themongoose_graphql_permissionsmodule but now the logic is moved to themongoose_graphql_directive_protectedmodule. Themongoose_graphql_permissionshas been removed.I also removed the condition that allowed running introspection on the protected root query type because we haven't had this type protected anymore.
TODO
usedirective in other categories.