Apply review

- Fix descriptions
- Make tests paralell
- Add graphql_SUITE to default.spec
- Log schema loading error
- Simplify mongoose_graphql_cowboy_handler init

big_tests/default.spec

@@ -25,6 +25,7 @@
 {suites, "tests", disco_and_caps_SUITE}.
 {suites, "tests", extdisco_SUITE}.
 {suites, "tests", gdpr_SUITE}.
+{suites, "tests", graphql_SUITE}.
 {suites, "tests", inbox_SUITE}.
 {suites, "tests", inbox_extensions_SUITE}.
 {suites, "tests", jingle_SUITE}.

big_tests/tests/graphql_SUITE.erl

@@ -15,16 +15,16 @@ all() ->
      {group, user_handler}].
 
 groups() ->
-    [{cowboy_handler, [], [can_connect_to_admin,
+    [{cowboy_handler, [parallel], [can_connect_to_admin,
                            can_connect_to_user
                           ]},
-     {user_handler, [], [wrong_creds_cannot_access_protected_types,
+     {user_handler, [parallel], [wrong_creds_cannot_access_protected_types,
                          unauth_cannot_access_protected_types,
                          unauth_can_access_unprotected_types,
                          can_execute_query_with_variables,
                          auth_user_can_access_protected_types
                         ]},
-     {admin_handler, [], [wrong_creds_cannot_access_protected_types,
+     {admin_handler, [parallel], [wrong_creds_cannot_access_protected_types,
                           unauth_cannot_access_protected_types,
                           unauth_can_access_unprotected_types,
                           can_execute_query_with_variables,
@@ -48,7 +48,7 @@ init_per_group(admin_handler, Config) ->
         true ->
             [{schema_endpoint, Endpoint} | Config];
         false ->
-            {skipped, <<"Admin credentials not defined in config">>}
+            {skipped, <<"Admin credentials are not defined in config">>}
     end;
 init_per_group(user_handler, Config) ->
     Config1 = escalus:create_users(Config, escalus:get_users([alice])),
@@ -92,7 +92,7 @@ wrong_creds_cannot_access_protected_types(Config) ->
     assert_no_permissions(Status, Data).
 
 auth_user_can_access_protected_types(Config) ->
-    escalus:story(
+    escalus:fresh_story(
         Config, [{alice, 1}],
         fun(Alice) ->
             Password = user_password(alice),
@@ -121,7 +121,7 @@ can_execute_query_with_variables(Config) ->
     {Status, Data} = execute(Ep, Body, undefined),
     ?assertEqual({<<"200">>,<<"OK">>}, Status),
     % operation M1 was executed, because id is in path
-    % access was granted, error was returned because valid resolver was not defined
+    % access was granted, an error was returned because valid resolver was not defined
     ?assertMatch(#{<<"data">> := #{<<"id">> := null},
                    <<"errors">> :=
                        [#{<<"extensions">> := #{<<"code">> := <<"resolver_crash">>},
@@ -136,7 +136,7 @@ assert_no_permissions(Status, Data) ->
 
 assert_access_granted(Status, Data) ->
     ?assertEqual({<<"200">>,<<"OK">>}, Status),
-    % access was granted, error was returned because valid resolver was not defined
+    % access was granted, an error was returned because valid resolver was not defined
     ?assertMatch(#{<<"errors">> :=
                    [#{<<"extensions">> :=
                      #{<<"code">> := <<"resolver_crash">>}}]}, Data).

priv/graphql/schemas/admin/admin_schema.gql

@@ -7,7 +7,7 @@ directive @protected on FIELD_DEFINITION | OBJECT
 
 """
 Contains all admin available queries.
-Only authenticated admin can execute this queries.
+Only an authenticated admin can execute these queries.
 """
 type AdminQuery @protected{
   "Get all enabled domains by hostType"
@@ -18,7 +18,7 @@ type AdminQuery @protected{
 
 """
 Contains all admin available mutations
-Only authenticated admin can execute this mutations.
+Only an authenticated admin can execute these mutations.
 """
 type AdminMutation @protected{
   "Add new domain"
@@ -33,7 +33,7 @@ type AdminMutation @protected{
 
 """
 A dynamic domain representation.
-Some operation could return not complete object i.e. some fields can be null.
+Some operation could return incomplete object i.e. some fields can be null.
 """
 type Domain{
   "Domain name"

src/mongoose_graphql.erl

@@ -3,6 +3,8 @@
 %% @end
 -module(mongoose_graphql).
 
+-include_lib("kernel/include/logger.hrl").
+
 %API
 -export([init/0,
          get_endpoint/1,
@@ -122,10 +124,16 @@ load_multiple_file_schema(Pattern) ->
         SchemaData = [read_schema_file(P) || P <- Paths],
         {ok, lists:flatten(SchemaData)}
     catch
-        _:_ ->
+        throw:{error, Reason, Path} ->
+            ?LOG_ERROR(#{what => graphql_cannot_load_schema,
+                         reason => Reason, path => Path}),
             {error, cannot_load}
     end.
 
 read_schema_file(Path) ->
-    {ok, Data} = file:read_file(Path),
-    binary_to_list(Data).
+    case file:read_file(Path) of
+         {ok, Data} ->
+            binary_to_list(Data);
+         {error, Reason} ->
+            throw({error, Reason, Path})
+    end.

src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl

@@ -8,9 +8,6 @@
 
 -behavior(cowboy_rest).
 
-%% ejabberd_cowboy callbacks
--export([cowboy_router_paths/2]).
-
 %% Cowboy Handler Interface
 -export([init/2]).
 
@@ -33,16 +30,14 @@
 
 -ignore_xref([cowboy_router_paths/2, from_json/2, to_html/2, to_json/2]).
 
-%% -- API ---------------------------------------------------
-
-cowboy_router_paths(BasePath, Opts) ->
-    [{[BasePath, "/"], ?MODULE, [{priv_file, mongooseim, "graphql/wsite/index.html"} | Opts]}].
+%% API
 
-init(Req, [{priv_file, _, _} = PrivFile | Opts]) ->
+init(Req, Opts) ->
+    IndexLocation = {priv_file, mongooseim, "graphql/wsite/index.html"},
     OptsMap = maps:from_list(Opts),
     {cowboy_rest,
      Req,
-     OptsMap#{index_location => PrivFile}
+     OptsMap#{index_location => IndexLocation}
     }.
 
 allowed_methods(Req, State) ->

src/mongoose_graphql/mongoose_graphql_permissions.erl

@@ -2,9 +2,10 @@
 %% permissions.
 %%
 %% GraphQL has directives that allow attaching additional information to schema,
-%% objects, fields, and more. The custom directive `@protected' is created to mark
-%% which objects or fields could be accessed only by authorized request. This module
-%% analyzes the AST and tries to find if there is at least one protected resource.
+%% to objects, to fields, and more. The custom directive `@protected' is created
+%% to mark which objects or fields could be accessed only by authorized request.
+%% This module analyzes the AST and tries to find if there is at least one protected
+%% resource.
 %%
 %% If unauthorized request want to execute a query that contains protected resources,
 %% an error is thrown.

test/mongoose_graphql_SUITE.erl

@@ -9,24 +9,24 @@
 all() ->
     [can_create_endpoint, 
      can_load_splitted_schema,
-     {group, unprotected_graphql}, 
+     {group, unprotected_graphql},
      {group, protected_graphql},
      {group, errors_handling}].
 
 groups() ->
-    [{protected_graphql, [], 
+    [{protected_graphql, [parallel],
      [auth_can_execute_protected_query, 
       auth_can_execute_protected_mutation,
       unauth_cannot_execute_protected_query,
       unauth_cannot_execute_protected_mutation,
       unauth_can_access_introspection]},
-     {unprotected_graphql, [], 
+     {unprotected_graphql, [parallel],
       [can_execute_query_with_vars,
        auth_can_execute_query,
        auth_can_execute_mutation,
        unauth_can_execute_query,
        unauth_can_execute_mutation]},
-     {errors_handling, [], 
+     {errors_handling, [parallel],
       [should_catch_parsing_errors,
        should_catch_type_check_params_errors,
        should_catch_type_check_errors