Skip to content

Commit

Permalink
Merge pull request #1513 from manikey123/782_test_p1
Browse files Browse the repository at this point in the history
Fix #782: Change hard-coded limitations in API's to configurable
  • Loading branch information
pmconrad authored Mar 20, 2019
2 parents de31b56 + 78cb2c7 commit ae78c2d
Show file tree
Hide file tree
Showing 11 changed files with 403 additions and 18 deletions.
30 changes: 16 additions & 14 deletions libraries/app/api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ namespace graphene { namespace app {
{
FC_ASSERT( _app.chain_database() );
const auto& db = *_app.chain_database();
FC_ASSERT( limit <= 100 );
uint64_t api_limit_get_account_history=_app.get_options().api_limit_get_account_history;
FC_ASSERT( limit <= api_limit_get_account_history );
vector<operation_history_object> result;
account_id_type account;
try {
Expand Down Expand Up @@ -353,7 +354,8 @@ namespace graphene { namespace app {
{
FC_ASSERT( _app.chain_database() );
const auto& db = *_app.chain_database();
FC_ASSERT( limit <= 100 );
uint64_t api_limit_get_account_history_operations=_app.get_options().api_limit_get_account_history_operations;
FC_ASSERT(limit <= api_limit_get_account_history_operations);
vector<operation_history_object> result;
account_id_type account;
try {
Expand Down Expand Up @@ -392,7 +394,8 @@ namespace graphene { namespace app {
{
FC_ASSERT( _app.chain_database() );
const auto& db = *_app.chain_database();
FC_ASSERT(limit <= 100);
uint64_t api_limit_get_relative_account_history=_app.get_options().api_limit_get_relative_account_history;
FC_ASSERT(limit <= api_limit_get_relative_account_history);
vector<operation_history_object> result;
account_id_type account;
try {
Expand Down Expand Up @@ -431,7 +434,8 @@ namespace graphene { namespace app {

history_operation_detail history_api::get_account_history_by_operations(const std::string account_id_or_name, vector<uint16_t> operation_types, uint32_t start, unsigned limit)
{
FC_ASSERT(limit <= 100);
uint64_t api_limit_get_account_history_by_operations=_app.get_options().api_limit_get_account_history_by_operations;
FC_ASSERT(limit <= api_limit_get_account_history_by_operations);
history_operation_detail result;
vector<operation_history_object> objs = get_relative_account_history(account_id_or_name, start, limit, limit + start - 1);
std::for_each(objs.begin(), objs.end(), [&](const operation_history_object &o) {
Expand Down Expand Up @@ -530,16 +534,16 @@ namespace graphene { namespace app {

// asset_api
asset_api::asset_api(graphene::app::application& app) :
_db( *app.chain_database()),
database_api( std::ref(*app.chain_database()), &(app.get_options())
_app(app),
_db( *app.chain_database()),
database_api( std::ref(*app.chain_database()), &(app.get_options())
) { }
asset_api::~asset_api() { }

vector<account_asset_balance> asset_api::get_asset_holders( std::string asset, uint32_t start, uint32_t limit ) const {
FC_ASSERT(limit <= 100);

uint64_t api_limit_get_asset_holders=_app.get_options().api_limit_get_asset_holders;
FC_ASSERT(limit <= api_limit_get_asset_holders);
asset_id_type asset_id = database_api.get_asset_id_from_string( asset );

const auto& bal_idx = _db.get_index_type< account_balance_index >().indices().get< by_asset_balance >();
auto range = bal_idx.equal_range( boost::make_tuple( asset_id ) );

Expand Down Expand Up @@ -571,7 +575,6 @@ namespace graphene { namespace app {
}
// get number of asset holders.
int asset_api::get_asset_holders_count( std::string asset ) const {

const auto& bal_idx = _db.get_index_type< account_balance_index >().indices().get< by_asset_balance >();
asset_id_type asset_id = database_api.get_asset_id_from_string( asset );
auto range = bal_idx.equal_range( boost::make_tuple( asset_id ) );
Expand All @@ -582,9 +585,7 @@ namespace graphene { namespace app {
}
// function to get vector of system assets with holders count.
vector<asset_holders> asset_api::get_all_asset_holders() const {

vector<asset_holders> result;

vector<asset_id_type> total_assets;
for( const asset_object& asset_obj : _db.get_index_type<asset_index>().indices() )
{
Expand Down Expand Up @@ -622,8 +623,9 @@ namespace graphene { namespace app {
optional<price> start,
uint32_t limit )const
{
FC_ASSERT( limit <= 101 );
auto plugin = _app.get_plugin<grouped_orders_plugin>( "grouped_orders" );
uint64_t api_limit_get_grouped_limit_orders=_app.get_options().api_limit_get_grouped_limit_orders;
FC_ASSERT( limit <= api_limit_get_grouped_limit_orders );
auto plugin = _app.get_plugin<graphene::grouped_orders::grouped_orders_plugin>( "grouped_orders" );
FC_ASSERT( plugin );
const auto& limit_groups = plugin->limit_order_groups();
vector< limit_order_group > result;
Expand Down
54 changes: 54 additions & 0 deletions libraries/app/application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,32 @@ void application_impl::set_dbg_init_key( graphene::chain::genesis_state_type& ge
genesis.initial_witness_candidates[i].block_signing_key = init_pubkey;
}



void application_impl::set_api_limit() {
if (_options->count("api-limit-get-account-history-operations")) {
_app_options.api_limit_get_account_history_operations = _options->at("api-limit-get-account-history-operations").as<uint64_t>();
}
if(_options->count("api-limit-get-account-history")){
_app_options.api_limit_get_account_history = _options->at("api-limit-get-account-history").as<uint64_t>();
}
if(_options->count("api-limit-get-grouped-limit-orders")){
_app_options.api_limit_get_grouped_limit_orders = _options->at("api-limit-get-grouped-limit-orders").as<uint64_t>();
}
if(_options->count("api-limit-get-relative-account-history")){
_app_options.api_limit_get_relative_account_history = _options->at("api-limit-get-relative-account-history").as<uint64_t>();
}
if(_options->count("api-limit-get-account-history-by-operations")){
_app_options.api_limit_get_account_history_by_operations = _options->at("api-limit-get-account-history-by-operations").as<uint64_t>();
}
if(_options->count("api-limit-get-asset-holders")){
_app_options.api_limit_get_asset_holders = _options->at("api-limit-get-asset-holders").as<uint64_t>();
}
if(_options->count("api-limit-get-key-references")){
_app_options.api_limit_get_key_references = _options->at("api-limit-get-key-references").as<uint64_t>();
}
}

void application_impl::startup()
{ try {
fc::create_directories(_data_dir / "blockchain");
Expand Down Expand Up @@ -437,6 +463,8 @@ void application_impl::startup()
if ( _options->count("enable-subscribe-to-all") )
_app_options.enable_subscribe_to_all = _options->at( "enable-subscribe-to-all" ).as<bool>();

set_api_limit();

if( _active_plugins.find( "market_history" ) != _active_plugins.end() )
_app_options.has_market_history_plugin = true;

Expand Down Expand Up @@ -975,6 +1003,20 @@ void application::set_program_options(boost::program_options::options_descriptio
("enable-standby-votes-tracking", bpo::value<bool>()->implicit_value(true),
"Whether to enable tracking of votes of standby witnesses and committee members. "
"Set it to true to provide accurate data to API clients, set to false for slightly better performance.")
("api-limit-get-account-history-operations",boost::program_options::value<uint64_t>()->default_value(100),
"For history_api::get_account_history_operations to set its default limit value as 100")
("api-limit-get-account-history",boost::program_options::value<uint64_t>()->default_value(100),
"For history_api::get_account_history to set its default limit value as 100")
("api-limit-get-grouped-limit-orders",boost::program_options::value<uint64_t>()->default_value(101),
"For orders_api::get_grouped_limit_orders to set its default limit value as 101")
("api-limit-get-relative-account-history",boost::program_options::value<uint64_t>()->default_value(100),
"For history_api::get_relative_account_history to set its default limit value as 100")
("api-limit-get-account-history-by-operations",boost::program_options::value<uint64_t>()->default_value(100),
"For history_api::get_account_history_by_operations to set its default limit value as 100")
("api-limit-get-asset-holders",boost::program_options::value<uint64_t>()->default_value(100),
"For asset_api::get_asset_holders to set its default limit value as 100")
("api-limit-get-key-references",boost::program_options::value<uint64_t>()->default_value(100),
"For database_api_impl::get_key_references to set its default limit value as 100")
;
command_line_options.add(configuration_file_options);
command_line_options.add_options()
Expand Down Expand Up @@ -1014,6 +1056,18 @@ void application::startup()
}
}

void application::set_api_limit()
{
try {
my->set_api_limit();
} catch ( const fc::exception& e ) {
elog( "${e}", ("e",e.to_detail_string()) );
throw;
} catch ( ... ) {
elog( "unexpected exception" );
throw;
}
}
std::shared_ptr<abstract_plugin> application::get_plugin(const string& name) const
{
return my->_active_plugins[name];
Expand Down
1 change: 1 addition & 0 deletions libraries/app/application_impl.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class application_impl : public net::node_delegate
}

void set_dbg_init_key( graphene::chain::genesis_state_type& genesis, const std::string& init_key );
void set_api_limit();

void startup();

Expand Down
3 changes: 2 additions & 1 deletion libraries/app/database_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,6 @@ dynamic_global_property_object database_api_impl::get_dynamic_global_properties(

vector<vector<account_id_type>> database_api::get_key_references( vector<public_key_type> key )const
{
FC_ASSERT(key.size() <= 100, "Number of keys must be 100 or less");
return my->get_key_references( key );
}

Expand All @@ -676,6 +675,8 @@ vector<vector<account_id_type>> database_api::get_key_references( vector<public_
*/
vector<vector<account_id_type>> database_api_impl::get_key_references( vector<public_key_type> keys )const
{
uint64_t api_limit_get_key_references=_app_options->api_limit_get_key_references;
FC_ASSERT(keys.size() <= api_limit_get_key_references);
const auto& idx = _db.get_index_type<account_index>();
const auto& aidx = dynamic_cast<const base_primary_index&>(idx);
const auto& refs = aidx.get_secondary_index<graphene::chain::account_member_index>();
Expand Down
1 change: 1 addition & 0 deletions libraries/app/include/graphene/app/api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ namespace graphene { namespace app {
vector<asset_holders> get_all_asset_holders() const;

private:
graphene::app::application& _app;
graphene::chain::database& _db;
graphene::app::database_api database_api;
};
Expand Down
9 changes: 8 additions & 1 deletion libraries/app/include/graphene/app/application.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ namespace graphene { namespace app {
public:
bool enable_subscribe_to_all = false;
bool has_market_history_plugin = false;
uint64_t api_limit_get_account_history_operations = 100;
uint64_t api_limit_get_account_history = 100;
uint64_t api_limit_get_grouped_limit_orders = 101;
uint64_t api_limit_get_relative_account_history = 100;
uint64_t api_limit_get_account_history_by_operations = 100;
uint64_t api_limit_get_asset_holders = 100;
uint64_t api_limit_get_key_references = 100;
};

class application
Expand Down Expand Up @@ -97,7 +104,7 @@ namespace graphene { namespace app {

net::node_ptr p2p_node();
std::shared_ptr<chain::database> chain_database()const;

void set_api_limit();
void set_block_production(bool producing_blocks);
fc::optional< api_access_info > get_api_access_info( const string& username )const;
void set_api_access_info(const string& username, api_access_info&& permissions);
Expand Down
47 changes: 47 additions & 0 deletions tests/common/database_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,45 @@ database_fixture::database_fixture()
{
options.insert(std::make_pair("max-ops-per-account", boost::program_options::variable_value((uint64_t)75, false)));
}
if (current_test_name == "api_limit_get_account_history_operations")
{
options.insert(std::make_pair("max-ops-per-account", boost::program_options::variable_value((uint64_t)125, false)));
options.insert(std::make_pair("api-limit-get-account-history-operations", boost::program_options::variable_value((uint64_t)300, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
if(current_test_name =="api_limit_get_account_history")
{
options.insert(std::make_pair("max-ops-per-account", boost::program_options::variable_value((uint64_t)125, false)));
options.insert(std::make_pair("api-limit-get-account-history", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
if(current_test_name =="api_limit_get_grouped_limit_orders")
{
options.insert(std::make_pair("api-limit-get-grouped-limit-orders", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("grouped_orders"), false)));
}
if(current_test_name =="api_limit_get_relative_account_history")
{
options.insert(std::make_pair("max-ops-per-account", boost::program_options::variable_value((uint64_t)125, false)));
options.insert(std::make_pair("api-limit-get-relative-account-history", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
if(current_test_name =="api_limit_get_account_history_by_operations")
{
options.insert(std::make_pair("api-limit-get-account-history-by-operations", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("api-limit-get-relative-account-history", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
if(current_test_name =="api_limit_get_asset_holders")
{
options.insert(std::make_pair("api-limit-get-asset-holders", boost::program_options::variable_value((uint64_t)250, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
if(current_test_name =="api_limit_get_key_references")
{
options.insert(std::make_pair("api-limit-get-key-references", boost::program_options::variable_value((uint64_t)200, false)));
options.insert(std::make_pair("plugins", boost::program_options::variable_value(string("account_history"), false)));
}
// add account tracking for ahplugin for special test case with track-account enabled
if( !options.count("track-account") && current_test_name == "track_account") {
std::vector<std::string> track_account;
Expand Down Expand Up @@ -160,6 +199,14 @@ database_fixture::database_fixture()
ahplugin->plugin_set_app(&app);
ahplugin->plugin_initialize(options);
ahplugin->plugin_startup();
if (current_test_name == "api_limit_get_account_history_operations" || current_test_name == "api_limit_get_account_history"
|| current_test_name == "api_limit_get_grouped_limit_orders" || current_test_name == "api_limit_get_relative_account_history"
|| current_test_name == "api_limit_get_account_history_by_operations" || current_test_name =="api_limit_get_asset_holders"
|| current_test_name =="api_limit_get_key_references")
{
app.initialize(graphene::utilities::temp_directory_path(), options);
app.set_api_limit();
}
}

if(current_test_name == "elasticsearch_objects" || current_test_name == "elasticsearch_suite") {
Expand Down
20 changes: 20 additions & 0 deletions tests/tests/asset_api_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,25 @@ BOOST_AUTO_TEST_CASE( asset_holders )
BOOST_CHECK(holders[2].name == "alice");
BOOST_CHECK(holders[3].name == "dan");
}
BOOST_AUTO_TEST_CASE( api_limit_get_asset_holders )
{
graphene::app::asset_api asset_api(app);

// create an asset and some accounts
create_bitasset("USD", account_id_type());
auto dan = create_account("dan");
auto bob = create_account("bob");
auto alice = create_account("alice");

// send them some bts
transfer(account_id_type()(db), dan, asset(100));
transfer(account_id_type()(db), alice, asset(200));
transfer(account_id_type()(db), bob, asset(300));

// make call
GRAPHENE_CHECK_THROW(asset_api.get_asset_holders(std::string( static_cast<object_id_type>(asset_id_type())), 0, 260), fc::exception);
vector<account_asset_balance> holders = asset_api.get_asset_holders(std::string( static_cast<object_id_type>(asset_id_type())), 0, 210);
BOOST_REQUIRE_EQUAL( holders.size(), 4u );
}

BOOST_AUTO_TEST_SUITE_END()
32 changes: 31 additions & 1 deletion tests/tests/database_api_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -940,5 +940,35 @@ BOOST_AUTO_TEST_CASE( verify_authority_multiple_accounts )
throw;
}
}

BOOST_AUTO_TEST_CASE( api_limit_get_key_references ){
try{
const int num_keys = 210;
const int num_keys1 = 2;
vector< private_key_type > numbered_private_keys;
vector< public_key_type > numbered_key_id;
numbered_private_keys.reserve( num_keys );
graphene::app::database_api db_api( db, &( app.get_options() ));
for( int i=0; i<num_keys1; i++ )
{
private_key_type privkey = generate_private_key(std::string("key_") + std::to_string(i));
public_key_type pubkey = privkey.get_public_key();
numbered_private_keys.push_back( privkey );
numbered_key_id.push_back( pubkey );
}
vector< vector<account_id_type> > final_result=db_api.get_key_references(numbered_key_id);
BOOST_REQUIRE_EQUAL( final_result.size(), 2u );
numbered_private_keys.reserve( num_keys );
for( int i=num_keys1; i<num_keys; i++ )
{
private_key_type privkey = generate_private_key(std::string("key_") + std::to_string(i));
public_key_type pubkey = privkey.get_public_key();
numbered_private_keys.push_back( privkey );
numbered_key_id.push_back( pubkey );
}
GRAPHENE_CHECK_THROW(db_api.get_key_references(numbered_key_id), fc::exception);
}catch (fc::exception& e) {
edump((e.to_detail_string()));
throw;
}
}
BOOST_AUTO_TEST_SUITE_END()
Loading

0 comments on commit ae78c2d

Please sign in to comment.