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.

Sign up for GitHub

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

Jump to bottom

Unify the admin REST API #3753

Merged
merged 14 commits into from
Sep 15, 2022
Merged

Unify the admin REST API #3753

merged 14 commits into from
Sep 15, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Sep 6, 2022 •
edited
Loading

The main goal is to have the following structure for the Admin REST API:

  • mongoose_admin_api implements mongoose_http_handler
  • mongoose_admin_api_* implement the individual command categories.

This is the same as for the Client API, making the two unified.

The new handler modules are using exceptions for error handling, which makes it less error-prone, as it is easier to read the logic and check if all cases are covered. The previous de-facto convention of multiple nested case constructs was less readable.

The following future work is excluded from this PR to reduce its size:

  • Update config spec tests to check mongoose_admin_api instead of mongoose_api_admin
  • Remove mongoose_api_admin
  • Remove mongoose_commands
  • Change mongoose_domain_handler and mongoose_api (yet another admin API) to use mongoose_admin_api
  • Update docs and the migration guide

@chrzaszcz chrzaszcz changed the base branch from master to feature/unified-rest-api September 6, 2022 12:23
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@codecov
Copy link

codecov bot commented Sep 6, 2022 •
edited
Loading

Codecov Report

Base: 81.16% // Head: 82.42% // Increases project coverage by +1.26% 🎉

Coverage data is based on head (4583d13) compared to base (95bf2d5).
Patch coverage: 99.76% of modified lines in pull request are covered.

Additional details and impacted files 
@@                     Coverage Diff                      @@
##           feature/unified-rest-api    #3753      +/-   ##
============================================================
+ Coverage                     81.16%   82.42%   +1.26%     
============================================================
  Files                           528      537       +9     
  Lines                         33918    34342     +424     
============================================================
+ Hits                          27528    28308     +780     
+ Misses                         6390     6034     -356
Impacted Files Coverage Δ
src/inbox/mod_inbox_api.erl 100.00% <ø> (ø)
src/mod_muc_api.erl 99.45% <ø> (ø)
src/mod_roster_api.erl 94.73% <ø> (ø)
src/mongoose_http_handler.erl 100.00% <ø> (ø)
src/mongoose_stanza_api.erl 90.00% <ø> (-1.67%) ⬇️
src/muc_light/mod_muc_light_commands.erl 11.76% <ø> (-70.59%) ⬇️
src/mongoose_admin_api/mongoose_admin_api.erl 97.14% <97.14%> (ø)
...mongoose_admin_api/mongoose_admin_api_contacts.erl 100.00% <100.00%> (ø)
...rc/mongoose_admin_api/mongoose_admin_api_inbox.erl 100.00% <100.00%> (ø)
...mongoose_admin_api/mongoose_admin_api_messages.erl 100.00% <100.00%> (ø)
... and 37 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mongoose-im mongoose-im mentioned this pull request Sep 6, 2022
Closed
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@chrzaszcz chrzaszcz force-pushed the unified-admin-api branch 2 times, most recently from da093a3 to 57452bb Compare September 14, 2022 08:23
@mongoose-im

This comment was marked as outdated.

Sign in to view
Paweł Chrząszcz added 10 commits September 14, 2022 10:57
@chrzaszcz
Add mongoose_admin_api
8a52cfb 
This module will delegate Admin REST API calls to the handler modules,
just like mongoose_client_api is doing for client API requests.
@chrzaszcz
Add Admin API module for contacts (roster)
d754a60 
Also: fix types in mod_roster_api
@chrzaszcz
Add Admin API modules for stanzas and messages
5d471f8 
Also: fix types in mongoose_stanza_api

Marked missing error handling as TO DO - it will be done in a separate task.
@chrzaszcz
Add Admin API module for inbox
6c5ffb7 
Also: fix types and whitespace in mod_inbox_api
@chrzaszcz
Add Admin API module for MUC
2a49bc4 
Also: fix types in mod_muc_api
@chrzaszcz
Add Admin API module for MUC Light
b5c8307 
Also: fix types and typos (sic!) in MUC Light modules
@chrzaszcz
Add Admin API module for sessions
a04f373
@chrzaszcz
Add Admin API module for users (accounts)
34c72b3
@chrzaszcz
Enable mongoose_admin_api (new) instead of mongoose_api_admin (old)
a9fdeac
@chrzaszcz
Update REST tests for the new API
bffcc6c 
- Update expected messages
- Actually test message errors for MUC (invites were tested instead)
- Remove tests for listing commands and adding mongoose_commands
  Command listing will be possible only with GraphQL, and
  mongoose_commands will be removed in a follow-up PR.

Also: remove test repeat
@mongoose-im
Copy link
Collaborator

mongoose-im commented Sep 14, 2022 •
edited
Loading

small_tests_24 / small_tests / 9b05bf2
Reports root / small

small_tests_25 / small_tests / 9b05bf2
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 9b05bf2
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 9b05bf2
Reports root/ big
OK: 1957 / Failed: 0 / User-skipped: 530 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 9b05bf2
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / 9b05bf2
Reports root/ big
OK: 3556 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / 9b05bf2
Reports root/ big
OK: 1957 / Failed: 0 / User-skipped: 530 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / 9b05bf2
Reports root/ big
OK: 2078 / Failed: 0 / User-skipped: 409 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 9b05bf2
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 9b05bf2
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 9b05bf2
Reports root/ big
OK: 2418 / Failed: 0 / User-skipped: 404 / Auto-skipped: 0

mysql_redis_25 / mysql_redis / 9b05bf2
Reports root/ big
OK: 3942 / Failed: 0 / User-skipped: 102 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 9b05bf2
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 9b05bf2
Reports root/ big
OK: 2258 / Failed: 0 / User-skipped: 396 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 9b05bf2
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Sep 14, 2022 •
edited
Loading

small_tests_24 / small_tests / 4583d13
Reports root / small

small_tests_25 / small_tests / 4583d13
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4583d13
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 4583d13
Reports root/ big
OK: 1957 / Failed: 0 / User-skipped: 530 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4583d13
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / 4583d13
Reports root/ big
OK: 1957 / Failed: 0 / User-skipped: 530 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / 4583d13
Reports root/ big
OK: 3556 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 4583d13
Reports root/ big
OK: 3572 / Failed: 1 / User-skipped: 88 / Auto-skipped: 0

muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive 
{error,{{assertion_failed,assert,is_groupchat_message,
              [<<"Restorable message">>],
              undefined,"undefined"},
    [{escalus_new_assert,assert_true,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
                {line,84}]},
     {muc_SUITE,wait_for_mam_result,3,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4394}]},
     {muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4130}]},
     {escalus_story,story,4,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,72}]},
     {muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4126}]},
     {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}]}]}}

Report log

internal_mnesia_25 / internal_mnesia / 4583d13
Reports root/ big
OK: 2078 / Failed: 0 / User-skipped: 409 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 4583d13
Reports root/ big
OK: 2258 / Failed: 0 / User-skipped: 396 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 4583d13
Reports root/ big
OK: 2418 / Failed: 0 / User-skipped: 404 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 4583d13
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

mysql_redis_25 / mysql_redis / 4583d13
Reports root/ big
OK: 3942 / Failed: 0 / User-skipped: 102 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 4583d13
Reports root/ big
OK: 3946 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive 
{error,{{assertion_failed,assert,is_groupchat_message,
              [<<"Restorable message">>],
              undefined,"undefined"},
    [{escalus_new_assert,assert_true,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
                {line,84}]},
     {muc_SUITE,wait_for_mam_result,3,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4394}]},
     {muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4130}]},
     {escalus_story,story,4,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,72}]},
     {muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
          [{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
           {line,4126}]},
     {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}]}]}}

Report log

pgsql_mnesia_24 / pgsql_mnesia / 4583d13
Reports root/ big
OK: 3946 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

service_mongoose_system_metrics_SUITE:system_metrics_are_reported_to_configurable_google_analytics 
{error,
  {{assertEqual,
     [{module,service_mongoose_system_metrics_SUITE},
      {line,470},
      {expression,"ActualTrackingIds"},
      {expected,[<<"UA-151671255-1">>,<<"UA-EXTRA-TRACKING-ID">>]},
      {value,[<<"UA-151671255-1">>]}]},
   [{service_mongoose_system_metrics_SUITE,
      events_are_reported_to_tracking_ids,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
       {line,470}]},
    {service_mongoose_system_metrics_SUITE,
      system_metrics_are_reported_to_configurable_google_analytics,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
       {line,204}]},
    {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_25 / odbc_mssql_mnesia / 4583d13
Reports root/ big
OK: 3572 / Failed: 1 / User-skipped: 88 / Auto-skipped: 0

inbox_extensions_SUITE:async_pools:one_to_one:mute_muted_entry_gets_unmuted 
{error,
  {{assert,
     [{module,inbox_extensions_SUITE},
      {line,798},
      {expression,"escalus_pred : is_message ( Message )"},
      {expected,true},
      {value,false}]},
   [{inbox_extensions_SUITE,check_message_with_properties,4,
      [{file,
         "/home/circleci/project/big_tests/tests/inbox_extensions_SUITE.erl"},
       {line,798}]},
    {inbox_extensions_SUITE,set_inbox_properties,4,
      [{file,
         "/home/circleci/project/big_tests/tests/inbox_extensions_SUITE.erl"},
       {line,792}]},
    {inbox_extensions_SUITE,'-mute_muted_entry_gets_unmuted/1-fun-2-',2,
      [{file,
         "/home/circleci/project/big_tests/tests/inbox_extensions_SUITE.erl"},
       {line,568}]},
    {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,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}]}]}}

Report log

pgsql_mnesia_24 / pgsql_mnesia / 4583d13
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 4583d13
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 4583d13
Reports root/ big
OK: 3573 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review September 14, 2022 09:38
arcusfelis
arcusfelis approved these changes Sep 14, 2022
View reviewed changes
Copy link
Contributor

@arcusfelis arcusfelis left a comment

Choose a reason for hiding this comment

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

looks cool

@arcusfelis arcusfelis merged commit 0c74a8f into feature/unified-rest-api Sep 15, 2022
@arcusfelis arcusfelis deleted the unified-admin-api branch September 15, 2022 08:21
@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
Reviewers

@arcusfelis arcusfelis arcusfelis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@chrzaszcz @mongoose-im @arcusfelis