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

Rework HTTP handler configuration #3636

Merged
merged 20 commits into from
Apr 27, 2022
Merged

Rework HTTP handler configuration #3636

merged 20 commits into from
Apr 27, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Apr 22, 2022

The main goal is to use maps with defaults for the handler-specific options of all HTTP handlers in the listen section.

Because the rework is significant, it turned out that it makes sense to simplify the configuration of mongoose_client_api, which was way too complicated for the end user (and some options were giving the illusion of being configurable, while only the initial values worked, e.g. Swagger path). Now all handlers are enabled by default and the user can set a custom list (with handlers = [...]) or disable the Swagger docs (with docs = false). For simplicity and consistency, all handlers are also enabled by default for mongoose_api.

A new behaviour, mongoose_http_handler, is introduced. Its introduction improved the separation between the listener implementation (ejabberd_cowboy) and the API module for managing handlers. There are two optional callbacks:

  • config_spec/0 returning config specification for the handler, if it has any options
  • routes/1 returning Cowboy routes, if different from the default

Excluded from this PR:

  • mod_bosh does not implement the new behaviour because of a callback name clash for config_spec. If it requires any options or more complex routes, we could implement a new module with these functions. This would separate module-specific logic from handler-specific logic.
  • mongoose_api_client is very confusing as there is mongoose_client_api as well. The former is obsolete, undocumented, but apparently still offers some unique functionality (e.g. change_password, subscription). It needs to be fully replaced by the latter, but this is out of scope for now. I think we could sort this out soon as a part of our ongoing API rework.

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #3636 (eeda654) into master (6d1c24b) will increase coverage by 0.00%.
The diff coverage is 96.34%.

@@           Coverage Diff           @@
##           master    #3636   +/-   ##
=======================================
  Coverage   81.03%   81.04%           
=======================================
  Files         427      428    +1     
  Lines       31962    31939   -23     
=======================================
- Hits        25901    25884   -17     
+ Misses       6061     6055    -6     
Impacted Files Coverage Δ
src/config/mongoose_config_parser_toml.erl 99.20% <ø> (ø)
src/config/mongoose_config_validator.erl 97.05% <ø> (ø)
src/mod_bosh.erl 95.62% <ø> (ø)
src/ejabberd_cowboy.erl 85.07% <83.33%> (-3.30%) ⬇️
src/mongoose_client_api/mongoose_client_api.erl 69.84% <87.50%> (+2.56%) ⬆️
src/config/mongoose_config_spec.erl 99.06% <100.00%> (-0.16%) ⬇️
src/config/mongoose_config_utils.erl 61.53% <100.00%> (+61.53%) ⬆️
src/domain/mongoose_domain_handler.erl 98.78% <100.00%> (+0.06%) ⬆️
src/mod_websockets.erl 84.24% <100.00%> (-0.32%) ⬇️
src/mongoose_api.erl 74.32% <100.00%> (+0.99%) ⬆️
... and 17 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 6d1c24b...eeda654. Read the comment docs.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@chrzaszcz chrzaszcz force-pushed the http-handler-rework branch 2 times, most recently from dbbefc5 to f534527 Compare April 25, 2022 15:03
@mongoose-im

This comment was marked as outdated.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 25, 2022

small_tests_24 / small_tests / f534527
Reports root / small


small_tests_23 / small_tests / f534527
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / f534527
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / f534527
Reports root/ big
OK: 2843 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f534527
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / f534527
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / f534527
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / f534527
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / f534527
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / f534527
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / f534527
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / f534527
Reports root/ big
OK: 3229 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / f534527
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / f534527
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / f534527
Reports root/ big
OK: 1715 / Failed: 1 / User-skipped: 367 / Auto-skipped: 0

pubsub_SUITE:dag+basic:publish_test
{error,{{badmatch,false},
    [{pubsub_tools,check_response,2,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,491}]},
     {pubsub_tools,receive_response,3,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,481}]},
     {pubsub_tools,receive_and_check_response,4,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,471}]},
     {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

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 25, 2022

small_tests_24 / small_tests / f7f6e3c
Reports root / small


small_tests_23 / small_tests / f7f6e3c
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / f7f6e3c
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / f7f6e3c
Reports root/ big
OK: 2843 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f7f6e3c
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / f7f6e3c
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / f7f6e3c
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / f7f6e3c
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / f7f6e3c
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / f7f6e3c
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / f7f6e3c
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / f7f6e3c
Reports root/ big
OK: 3229 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / f7f6e3c
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / f7f6e3c
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / f7f6e3c
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@esl esl deleted a comment from mongoose-im Apr 25, 2022
API:
- config_spec/0 returns the config section spec for all possible HTTP handlers
- get_routes/1 returns Cowboy routes for a list of configured handlers

There are two optional callbacks:
- config_spec/0 returning config specification for the handler,
  if it has any options
- routes/1 returning Cowboy routes, if different from the default

All handlers which have options should be listed
  in configurable_handler_modules/0
Use maps with defaults in the config spec
Use maps with defaults, include all handlers by default
- Use maps with defaults
- Include all handlers by default

New options:
- 'handlers' - to choose specific low-level handlers,
- 'docs' - to disable docs.
It will be placed in mongoose_http_handler

Also: refactor 'store_trails'
- Use maps with defaults
- Add new options
- Rework test layout after reworking mongoose_client_api config
- Remove the custom logic for comparing handler tuples
- Simplify examples to match the defaults in mongooseim.toml
- Document the new options: `handlers` and `docs`
- Update the list of available handlers
@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 26, 2022

small_tests_24 / small_tests / cf739ba
Reports root / small


small_tests_23 / small_tests / cf739ba
Reports root / small


dynamic_domains_mysql_redis_24 / mysql_redis / cf739ba
Reports root/ big
OK: 2843 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / cf739ba
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / cf739ba
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / cf739ba
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / cf739ba
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / cf739ba
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / cf739ba
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / cf739ba
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / cf739ba
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / cf739ba
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / cf739ba
Reports root/ big
OK: 3229 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / cf739ba
Reports root/ big
OK: 3244 / Failed: 2 / User-skipped: 142 / Auto-skipped: 0

pep_SUITE:pep_tests:delayed_receive
{error,{{badmatch,[]},
    [{pep_SUITE,'-delayed_receive/1-fun-0-',3,
          [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
           {line,276}]},
     {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

pep_SUITE:pep_tests:send_caps_after_login_test
{error,{{assertion_failed,assert_many,false,[is_roster_set,is_presence],[],[]},
    [{escalus_new_assert,assert_true,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
                {line,84}]},
     {escalus_story,'-make_all_clients_friends/1-fun-0-',2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,111}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
     {escalus_utils,distinct_pairs,2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,60}]},
     {escalus_story,make_all_clients_friends,1,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,106}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / cf739ba
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 26, 2022

small_tests_24 / small_tests / 8367450
Reports root / small


small_tests_23 / small_tests / 8367450
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 8367450
Reports root/ big
OK: 2859 / Failed: 1 / User-skipped: 133 / Auto-skipped: 0

bosh_SUITE:essential_https:accept_higher_hold_value
{error,
  {{assertEqual,
     [{module,bosh_SUITE},
      {line,251},
      {expression,"get_bosh_sessions ( )"},
      {expected,[]},
      {value,
        [{bosh_session,<<"3da36d68bc677101cd293eaa35be988999384039">>,
           <8633.6002.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,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_pgsql_mnesia_23 / pgsql_mnesia / 8367450
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 8367450
Reports root/ big
OK: 2843 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 8367450
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 8367450
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 8367450
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 8367450
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 8367450
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 8367450
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 8367450
Reports root/ big
OK: 3229 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 8367450
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 8367450
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 8367450
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review April 26, 2022 07:53
Copy link
Contributor

@Premwoik Premwoik left a comment

Choose a reason for hiding this comment

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

All looks good to me :) I like the changes, the code and config look clearer now. I added minor comments about formatting.

src/mongoose_api_admin.erl Show resolved Hide resolved
src/mongoose_client_api/mongoose_client_api.erl Outdated Show resolved Hide resolved
src/mongoose_client_api/mongoose_client_api.erl Outdated Show resolved Hide resolved
rel/files/mongooseim.toml Show resolved Hide resolved
@mongoose-im
Copy link
Collaborator

mongoose-im commented Apr 27, 2022

small_tests_24 / small_tests / eeda654
Reports root / small


small_tests_23 / small_tests / eeda654
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / eeda654
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / eeda654
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / eeda654
Reports root/ big
OK: 2843 / Failed: 0 / User-skipped: 150 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / eeda654
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / eeda654
Reports root/ big
OK: 1506 / Failed: 0 / User-skipped: 401 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / eeda654
Reports root/ big
OK: 2860 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / eeda654
Reports root/ big
OK: 1547 / Failed: 0 / User-skipped: 360 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / eeda654
Reports root/ big
OK: 3229 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / eeda654
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / eeda654
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / eeda654
Reports root/ big
OK: 1854 / Failed: 0 / User-skipped: 368 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / eeda654
Reports root/ big
OK: 3234 / Failed: 0 / User-skipped: 142 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / eeda654
Reports root/ big
OK: 1697 / Failed: 0 / User-skipped: 367 / Auto-skipped: 0

@Premwoik Premwoik merged commit 0d956f2 into master Apr 27, 2022
@Premwoik Premwoik deleted the http-handler-rework branch April 27, 2022 09:09
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants