Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQL in MongooseIM proof of concept #3354

Merged
merged 28 commits into from Dec 10, 2021
Merged

Conversation

Premwoik
Copy link
Contributor

@Premwoik Premwoik commented Oct 22, 2021

This PR proposes how MIM Rest API and CLI could be replaced by GraphQL in the future.

TODO:

  • Init and propose basic structure
  • Add listener with GraphiQL site
  • Add basic authentication
  • Support selected part of existing Rest API (domain management)
  • Allow to use GraphQL from command line (create CLT to execute GraphQL)
  • Better configuration / Fix tests
  • Add basic tests

Multi-schema adaptation

  • Use GraphQL from my fork
  • Split listeners
  • Separate admin and user in schemas
  • Rebuild permissions system
  • Code refactoring

To do in other PR:

  • Finish domain management API (custom error messages formatting, add tests)
  • Add more tests

@mongoose-im

This comment has been minimized.

rebar.config Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #3354 (a719db0) into feature/graphql (9b906c5) will decrease coverage by 0.00%.
The diff coverage is 78.10%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           feature/graphql    #3354      +/-   ##
===================================================
- Coverage            80.76%   80.75%   -0.01%     
===================================================
  Files                  414      423       +9     
  Lines                32335    32525     +190     
===================================================
+ Hits                 26116    26267     +151     
- Misses                6219     6258      +39     
Impacted Files Coverage Δ
..._graphql/admin/mongoose_graphql_admin_mutation.erl 0.00% <0.00%> (ø)
...ose_graphql/admin/mongoose_graphql_admin_query.erl 0.00% <0.00%> (ø)
src/mongoose_graphql/mongoose_graphql_domain.erl 0.00% <0.00%> (ø)
src/mongoose_graphql/mongoose_graphql_errors.erl 33.33% <33.33%> (ø)
src/config/mongoose_config_spec.erl 97.90% <80.00%> (-0.65%) ⬇️
src/mongoose_api_common.erl 84.67% <80.00%> (-0.37%) ⬇️
...goose_graphql/mongoose_graphql_cowboy_response.erl 81.81% <81.81%> (ø)
.../mongoose_graphql/mongoose_graphql_permissions.erl 85.00% <85.00%> (ø)
src/mongoose_graphql.erl 90.00% <90.00%> (ø)
...ngoose_graphql/mongoose_graphql_cowboy_handler.erl 90.41% <90.41%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b906c5...a719db0. Read the comment docs.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@Premwoik Premwoik marked this pull request as ready for review November 9, 2021 16:07
@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

Seems that the latest versions of GraphiQL don't work with the Erlang GraphQL library,
so I downgrade to the last working one.
- Fix descriptions
- Make tests paralell
- Add graphql_SUITE to default.spec
- Log schema loading error
- Simplify mongoose_graphql_cowboy_handler init
@Premwoik Premwoik changed the base branch from master to feature/graphql December 7, 2021 12:30
@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im

This comment has been minimized.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 8, 2021

small_tests_23 / small_tests / fba3a94
Reports root / small


small_tests_24 / small_tests / fba3a94
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / fba3a94
Reports root/ big
OK: 2730 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / fba3a94
Reports root/ big
OK: 2730 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / fba3a94
Reports root/ big
OK: 2741 / Failed: 1 / User-skipped: 203 / Auto-skipped: 0

sm_SUITE:parallel:messages_are_properly_flushed_during_resumption
{error,
  {{badmatch,
     {error,
       {connection_step_failed,
         {#Fun<sm_SUITE.11.68776247>,
          {client,
            <<"alicE_messages_are_properly_flushed_during_resumption_1821@domain.example.com">>,
            escalus_tcp,<0.30958.1>,undefined,
            [{username,
               <<"alicE_messages_are_properly_flushed_during_resumption_1821">>},
             {server,<<"domain.example.com">>},
             {host,<<"localhost">>},
             {password,<<"matygrysa">>},
             {stream_management,true},
             {stream_id,<<"cc96bd4c901386b7">>}]},
          [{compression,[<<"zlib">>]},
           {starttls,true},
           {stream_management,true},
           {advanced_message_processing,true},
           {client_state_indication,false},
           {sasl_mechanisms,[<<"SCRAM-SHA-256">>,<<"PLAIN">>]},
           {caps,undefined}]},
         {timeout,get_resumed}}}},
   [{sm_SUITE,'-messages_are_properly_flushed_during_resumption/1-fun-1-',
      3,
      [{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
       {line,1226}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1292}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1224}]}]}}

Report log


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / fba3a94
Reports root/ big
OK: 2730 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / fba3a94
Reports root/ big
OK: 1526 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / fba3a94
Reports root/ big
OK: 1526 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / fba3a94
Reports root/ big
OK: 1611 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / fba3a94
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / fba3a94
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / fba3a94
Reports root/ big
OK: 1904 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / fba3a94
Reports root/ big
OK: 3132 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / fba3a94
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / fba3a94
Reports root/ big
OK: 1750 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general, I have some comments.

@@ -0,0 +1,205 @@
-module(graphql_SUITE).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add it to dynamic_domains.spec as well to be tested also in the dynamic domains presets?

big_tests/tests/graphql_SUITE.erl Outdated Show resolved Hide resolved
priv/graphql/wsite/index.html Show resolved Hide resolved
priv/graphql/wsite/index.html Outdated Show resolved Hide resolved
src/ejabberd_ctl.erl Outdated Show resolved Hide resolved
src/mongoose_graphql/mongoose_graphql_permissions.erl Outdated Show resolved Hide resolved
src/mongoose_graphql/mongoose_graphql_permissions.erl Outdated Show resolved Hide resolved
src/mongoose_graphql/mongoose_graphql_permissions.erl Outdated Show resolved Hide resolved
test/mongoose_graphql_default_resolver.erl Outdated Show resolved Hide resolved
test/mongoose_graphql_SUITE.erl Outdated Show resolved Hide resolved
@mongoose-im

This comment has been minimized.

- Use jiffy instead of jsx
- Fix descriptions
- Other small fixes
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 9, 2021

small_tests_24 / small_tests / a719db0
Reports root / small


small_tests_23 / small_tests / a719db0
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / a719db0
Reports root/ big
OK: 2750 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / a719db0
Reports root/ big
OK: 2733 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / a719db0
Reports root/ big
OK: 2749 / Failed: 1 / User-skipped: 186 / Auto-skipped: 0

mam_SUITE:rdbms_async_pool_muc_all:muc06:muc_show_x_user_to_moderators_in_anon_rooms
{error,{test_case_failed,"Respond size is 0, 1 is expected."}}

Report log


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / a719db0
Reports root/ big
OK: 2750 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / a719db0
Reports root/ big
OK: 1526 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / a719db0
Reports root/ big
OK: 1526 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / a719db0
Reports root/ big
OK: 1611 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / a719db0
Reports root/ big
OK: 1904 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / a719db0
Reports root/ big
OK: 3132 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / a719db0
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / a719db0
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / a719db0
Reports root/ big
OK: 3137 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / a719db0
Reports root/ big
OK: 1750 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / a719db0
Reports root/ big
OK: 2750 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link
Collaborator

@DenysGonchar DenysGonchar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, but I have a few comments that must be addressed in the following PRs:

  • I think we can get rid of separate schemas for big_tests, more over that would be nice to test exactly what is config file. I some specific case requires a dedicated schema - we may want to add it in /priv/graphql/test_schema directory instead of keeping copies for every SUITE, however I would prefer to test such cases as unit tests.
  • please don't forget to change git reference for graphql in rebar.config file before merging feature branch to master. it can be either original repo (if they accept your PR) or we should make ESL's fork.

@DenysGonchar DenysGonchar merged commit d35b68b into feature/graphql Dec 10, 2021
@DenysGonchar DenysGonchar deleted the mim-graphql-poc branch December 10, 2021 13:01
@Premwoik Premwoik modified the milestone: 5.1.0 May 25, 2022
@chrzaszcz chrzaszcz added this to the 6.0.0 milestone Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants