Fix cancel_all_subscriptions API

[abitmore]

For clients, it's not broken. However, the implementation need to be improved. It now calls set_subscribe_callback( std::function<void(const fc::variant&)>(), true). I think using true as the second parameter here meant to clear filters according to the API document, but the document was wrong. The set_subscribe_callback function will always clear filters, and its second parameter is for something else, setting it to true may trigger an exception when #752 is done.

Update:

The set_subscribe_callback documentation has been fixed in develop branch.

This ticket is to rewrite cancel_all_subscriptions to do real cleanups, and perhaps refactor
it and set_subscribe_callback to call same cleanup function rather than having same code in both functions.

[jmjatlanta]

The documentation states that the boolean is indeed intended to clear filters. The code however says that this boolean dictates if a notification is sent on object creation and removal.

If set to true, an FC_ASSERT can fail if the application option "enable subscribe to all" is not set (note: this assert is not in the master branch, but does exist in the develop branch).

For details see

bitshares-core/libraries/app/database_api.cpp

Line 332 in cdc2db3

void database_api_impl::set_subscribe_callback( std::function<void(const variant&)> cb, bool notify_remove_create )

Therefore, the fix would be to adjust the documentation. Correct?

[abitmore]

Therefore, the fix would be to adjust the documentation. Correct?

The set_subscribe_callback documentation has been fixed in develop branch.

This ticket is to rewrite cancel_all_subscriptions to do real cleanups, and perhaps refactor
it and set_subscribe_callback to call same cleanup function rather than having same code in both functions.

[jmjatlanta]

Questions:

1. Should database_api::set_subscribe_callback(func) clear the _subscribed_accounts collection? (It currently does.)
2. Should database_api::set_subscribe_callback(func) clear the _market_subscriptions collection? (It currently does not.)

I cannot imagine a real use case, but here is a contrived example:

• set_subscribe_callback(A) is called
• accounts and markets are subscribed to
• set_subscribe_callback(B) is called

Would the user expect that the previous subscriptions to (1) accounts and (2) markets be maintained?

[abitmore]

Related discussion: #726 #774 #777.

[abitmore]

Merged #1009. Closing.