Status rest api

[DenysGonchar]

This PR includes the following changes:

• Amoc status REST API implementation and tests.
• error reporting for /execution/* REST APIs
• overall REST API code clean up

here are some comments to the changes in this PR:

• doc/http-api.md, rebar.config, rebar.lock - update of amoc_rest version, removal of jiffy dependency for amoc tests.
• src/amoc_config/amoc_config_env.erl - tests and rest api uses formatting functionality, so it's moved in just one (the most proper) place.
• src/amoc_controller.erl - removed scenario list reporting for the idle state (a kind of hack used by the previous scenario status rest api). allow settings updates only during scenario execution.
• src/amoc_distribution/amoc_dist.erl - added tracking of the cluster status (on a master node). Ensuring that if settings updated on all the nodes, then the new node in the cluster will be started with them as well. Allow running stop/0 only on the master, as all other execution interfaces (required to track cluster state correctly).
• src/rest_api/amoc_api_helpers_execution.erl - main interfaces triggered by execution REST API, technically wrappers around amoc_dist interfaces. This module is also responsible for JSON data parsing and extra errors reporting.
• src/rest_api/amoc_api_helpers_scenario_info.erl - renamed src/rest_api/amoc_api_scenario_status.erl file, the following things changed:
  • test_status/1 interface replaced with is_loaded/1 since it was mostly used for this purpose, and we don't have scenario status concept in our REST API any more.
  • scenario_settings/1 and scenario_params/1 interfaces remained semantically the same but now they return JSON object as maps, this significantly increases readability.
  • get_edoc/1 interfaces remained unchanged.
• src/rest_api/amoc_api_helpers_scenario_upload.erl - just renaming to use one and the same naming format for rest api helper modules.
• src/rest_api/amoc_api_helpers_status.erl - helper module, contains function required to compose amoc node status JSON object.
• src/rest_api/amoc_api_logic_handler.erl - the list of changes:
  • the list of changes related to the amoc_rest version update:
    • added StatusNodeGet and changed StatusGet API implementations
    • ScenariosIdGet interface replaced with ScenariosDefaultsIdGet
  • updated implementation of the Execution* interfaces, now they use cluster state verification and src/rest_api/amoc_api_helpers_execution.erl helper module
  • changed JSON objects format from lists & tuples to maps
• test/amoc_api_execution_handler_SUITE.erl - updated old and added new test cases to verify improved errors reporting. minor adoptions to the changes in amoc_api_helper:patch/2 implementation (jiffy replacement and switch to maps representation of the JSON objects)
• test/amoc_api_helper.erl - jiffy replacement, maps usage for JSON objects decoding, encoding JSON data internaly in amoc_api_helper:patch/2 interface.
• test/amoc_api_node_handler_SUITE.erl - had only one testcase, merged into test/amoc_api_status_handler_SUITE.erl
• test/amoc_api_scenarios_handler_SUITE.erl - tests for the new ScenariosDefaultsIdGet REST API, switch to maps for JSON objects validation.
• test/amoc_api_status_handler_SUITE.erl - test cases for the new StatusNodeGet and StatusGet REST APIs
• test/amoc_config_helper.erl - switch to amoc_config_env:format/2 implementation.

[NelsonVides]

For the tests in test/amoc_api_status_handler_SUITE.erl, what about doing something like

returns_up_when_amoc_up_and_idle(_Config) ->
    returns_up_when_amoc_up_and(idle, #{}).

returns_up_when_amoc_up_and_disabled(_Config) ->
    returns_up_when_amoc_up_and(disabled, #{}).

returns_up_when_amoc_up_and_finished(_Config) ->
    SettingsMap = given_amoc_config_scenario_is_mocked(),
    returns_up_when_amoc_up_and(finished, #{<<"settings">> => SettingsMap}).

returns_up_when_amoc_up_and(Status, ExtraControllerStatus) ->
    %% given
    EnvMap = given_amoc_envs_are_set(),
    ControllerMap = given_amoc_controller_is_mocked(Status),
    %% when
    {CodeHttp, Body} = amoc_api_helper:get(?PATH),
    %% then
    ControllerStatus = maps:merge(ControllerMap, ExtraControllerStatus),
    ExpectedBody = #{<<"amoc_status">> => <<"up">>,
                     <<"controller">> => ControllerStatus,
                     <<"env">> => EnvMap},
    ?assertEqual(200, CodeHttp),
    ?assertEqual(ExpectedBody, Body).

Just to minimize the copy&pasting.

[DenysGonchar]

For the tests in test/amoc_api_status_handler_SUITE.erl, what about doing something like

returns_up_when_amoc_up_and_idle(_Config) ->
    returns_up_when_amoc_up_and(idle, #{}).

returns_up_when_amoc_up_and_disabled(_Config) ->
    returns_up_when_amoc_up_and(disabled, #{}).

returns_up_when_amoc_up_and_finished(_Config) ->
    SettingsMap = given_amoc_config_scenario_is_mocked(),
    returns_up_when_amoc_up_and(finished, #{<<"settings">> => SettingsMap}).

returns_up_when_amoc_up_and(Status, ExtraControllerStatus) ->
    %% given
    EnvMap = given_amoc_envs_are_set(),
    ControllerMap = given_amoc_controller_is_mocked(Status),
    %% when
    {CodeHttp, Body} = amoc_api_helper:get(?PATH),
    %% then
    ControllerStatus = maps:merge(ControllerMap, ExtraControllerStatus),
    ExpectedBody = #{<<"amoc_status">> => <<"up">>,
                     <<"controller">> => ControllerStatus,
                     <<"env">> => EnvMap},
    ?assertEqual(200, CodeHttp),
    ?assertEqual(ExpectedBody, Body).

Just to minimize the copy&pasting.

Being honest, I would rather keep readable copy-pasting.
I prefer to have the whole logic of the test case on just one screen w/o scrolling.

[aleklisi]

I have 2 generic questions:
regarding src/rest_api/amoc_api_logic_handler.erl
do you mind incuding errors descriptions to the specific errors

% replace
{404, #{}, #{}}.
% with something like:
{404, #{}, #{comment => "Scenario not found"}}.

regarding src/rest_api/amoc_api_helpers_execution.erl

% replace
{error, invalid_body}.
% with something like:
{error, {invalid_body, "Request body is missing ___ field"}}.

I must say I admire the number of tests you included to test the functionality.

Also as a side note and only a joke "Do not use master and slave terminology" https://www.vice.com/en_us/article/k7qbyv/github-to-remove-masterslave-terminology-from-its-platform

[DenysGonchar]

I have 2 generic questions:
regarding src/rest_api/amoc_api_logic_handler.erl
do you mind incuding errors descriptions to the specific errors

% replace
{404, #{}, #{}}.
% with something like:
{404, #{}, #{comment => "Scenario not found"}}.

regarding src/rest_api/amoc_api_helpers_execution.erl

% replace
{error, invalid_body}.
% with something like:
{error, {invalid_body, "Request body is missing ___ field"}}.

I must say I admire the number of tests you included to test the functionality.

Also as a side note and only a joke "Do not use master and slave terminology" https://www.vice.com/en_us/article/k7qbyv/github-to-remove-masterslave-terminology-from-its-platform

The meaning of the code is described, please see swagger ui

[aleklisi]

@DenysGonchar thanks for all the answers and clarification.

[NelsonVides]

PR seems fine, just some whitespaces around. Also, please add the EOL at EOF.

I just have one concern about testing suites. I think that needed to mock so so much of amoc_dist, in such a granular detail, is a code smell. While I don't have any suggestion on how to improve it, I'd like to raise concern about this, it very very tightly couples testing from implementation, and in that sense, tests are not a good abstraction. They just work for the moment.

[DenysGonchar]

PR seems fine, just some whitespaces around. Also, please add the EOL at EOF.

I just have one concern about testing suites. I think that needed to mock so so much of amoc_dist, in such a granular detail, is a code smell. While I don't have any suggestion on how to improve it, I'd like to raise concern about this, it very very tightly couples testing from implementation, and in that sense, tests are not a good abstraction. They just work for the moment.

we have integration tests for this. however it must be extended to test all the new REST APIs