diff --git a/coreapi/callbacks.c b/coreapi/callbacks.c index 9da57295ebe3536f244485a960049e524b76873d..6ca30ed8da3a749858356b08f94507bac4c99a90 100644 --- a/coreapi/callbacks.c +++ b/coreapi/callbacks.c @@ -571,7 +571,11 @@ static void register_failure(SalOp *op) { /* Do nothing. There will be an auth_requested() callback. If the callback doesn't provide an AuthInfo, then * the proxy config will transition to the failed state.*/ } else { - Account::toCpp(account)->setState(LinphoneRegistrationFailed, details); + if (Account::toCpp(account)->isUnregistering()) { + Account::toCpp(account)->setState(LinphoneRegistrationNone, details); + } else { + Account::toCpp(account)->setState(LinphoneRegistrationFailed, details); + } } if (Account::toCpp(account)->getPresencePublishEvent()) { /*prevent publish to be sent now until registration gets successful*/ diff --git a/coreapi/proxy.c b/coreapi/proxy.c index bbfbc62e319f98c92c28b0ce0a8447c306f645f0..bbb5af9368c75218daf5f11fa8489eb3df838b5d 100644 --- a/coreapi/proxy.c +++ b/coreapi/proxy.c @@ -890,14 +890,22 @@ void linphone_core_remove_account(LinphoneCore *core, LinphoneAccount *account) linphone_core_notify_account_removed(core, account); Account::toCpp(account)->setDeletionDate(ms_time(NULL)); - if (linphone_account_get_state(account) == LinphoneRegistrationOk) { - LinphoneAccountParams *params = linphone_account_params_clone(linphone_account_get_params(account)); - linphone_account_params_set_register_enabled(params, FALSE); - linphone_account_set_params(account, params); - linphone_account_params_unref(params); - Account::toCpp(account)->update(); - } else if (linphone_account_get_state(account) != LinphoneRegistrationNone) { - Account::toCpp(account)->setState(LinphoneRegistrationNone, "Registration disabled"); + auto state = linphone_account_get_state(account); + switch (state) { + case LinphoneRegistrationNone: + case LinphoneRegistrationOk: + case LinphoneRegistrationRefreshing: + case LinphoneRegistrationProgress: { + LinphoneAccountParams *params = linphone_account_params_clone(linphone_account_get_params(account)); + linphone_account_params_set_register_enabled(params, FALSE); + linphone_account_set_params(account, params); + linphone_account_params_unref(params); + // Account::toCpp(account)->update(); + } break; + case LinphoneRegistrationFailed: + case LinphoneRegistrationCleared: + Account::toCpp(account)->setState(LinphoneRegistrationNone, "Registration disabled"); + break; } linphone_proxy_config_write_all_to_config_file(core); diff --git a/src/account/account.cpp b/src/account/account.cpp index 9b6a781f2b46de9f572ecb3904649ad8e44c96cd..ea97e1423a140bc66d600116cb79b36d24d9cbd1 100644 --- a/src/account/account.cpp +++ b/src/account/account.cpp @@ -354,13 +354,28 @@ void Account::updateDependentAccount(LinphoneRegistrationState state, const std: } } +void Account::handleDeletion() { + switch (mState) { + case LinphoneRegistrationOk: + unregister(); + break; + case LinphoneRegistrationNone: + case LinphoneRegistrationCleared: + break; + default: + // In all other states, un-registration is aborted. + setState(LinphoneRegistrationNone, "Registration disabled"); + break; + } +} + void Account::setState(LinphoneRegistrationState state, const std::string &message) { auto core = getCCore(); if (mState != state || state == LinphoneRegistrationOk) { /*allow multiple notification of LinphoneRegistrationOk for refreshing*/ const auto identity = (mParams) ? mParams->getIdentityAddress()->toString() : std::string(); - if (!mParams) lWarning() << "AccountParams not set for Account [" << this->toC() << "]"; - lInfo() << "Account [" << this << "] for identity [" << identity << "] moving from state [" + if (!mParams) lWarning() << "AccountParams not set for " << *this; + lInfo() << *this << " for identity [" << identity << "] moving from state [" << linphone_registration_state_to_string(mState) << "] to [" << linphone_registration_state_to_string(state) << "] on core [" << core << "]"; mIsUnregistering = false; @@ -378,6 +393,10 @@ void Account::setState(LinphoneRegistrationState state, const std::string &messa LinphoneRegistrationState previousState = mState; mState = state; + if (mDeletionDate != 0) { + handleDeletion(); + } + if (!mDependency) { updateDependentAccount(state, message); } @@ -556,8 +575,8 @@ std::shared_ptr<Address> Account::guessContactForRegister() { if (core && core->push_notification_enabled) { if (!newParams->isPushNotificationAvailable()) { - lError() << "Couldn't compute automatic push notifications parameters on account [" << this->toC() - << "] because account params do not have available push notifications"; + lError() << "Couldn't compute automatic push notifications parameters on " << *this + << " because account params do not have available push notifications"; } else if (newParams->mPushNotificationAllowed || newParams->mRemotePushNotificationAllowed) { if (newParams->mPushNotificationConfig->getProvider().empty()) { bool tester_env = !!linphone_config_get_int(core->config, "tester", "test_env", FALSE); @@ -583,8 +602,8 @@ std::shared_ptr<Address> Account::guessContactForRegister() { if (!contactParamsWrapper->getUriParamValue(paramName).empty()) { contactParamsWrapper->removeUriParam(paramName); didRemoveParams = true; - lError() << "Removing '" << paramName << "' from account [" << this - << "] contact uri parameters because it will be generated automatically since core " + lError() << "Removing '" << paramName << "' from " << *this + << " contact uri parameters because it will be generated automatically since core " "has push notification enabled"; } } @@ -598,8 +617,8 @@ std::shared_ptr<Address> Account::guessContactForRegister() { } } - lWarning() << "Account [" << this << "] contact uri parameters changed from '" - << newParams->mContactUriParameters << "' to '" << newContactUriParams << "'"; + lWarning() << *this << " contact uri parameters changed from '" << newParams->mContactUriParameters + << "' to '" << newContactUriParams << "'"; newParams->mContactUriParameters = newContactUriParams; } } @@ -628,7 +647,7 @@ std::shared_ptr<Address> Account::guessContactForRegister() { } lInfo() << "Added push notification informations '" << newParams->getPushNotificationConfig()->asString(mParams->mRemotePushNotificationAllowed) - << "' added to account [" << this << "]"; + << "' added to " << *this; setAccountParams(newParams); } } @@ -660,15 +679,14 @@ std::list<SalAddress *> Account::getOtherContacts() { void Account::registerAccount() { if (mParams->mRegisterEnabled) { if (mParams->mProxyAddress == nullptr) { - lError() << "Can't register LinphoneAccount [" << this << "] without a proxy address"; + lError() << "Can't register " << *this << " without a proxy address"; return; } if (mParams->mIdentityAddress == nullptr) { - lError() << "Can't register LinphoneAccount [" << this << "] without an identity address"; + lError() << "Can't register " << *this << " without an identity address"; return; } - lInfo() << "LinphoneAccount [" << this - << "] about to register (LinphoneCore version: " << linphone_core_get_version() << ")"; + lInfo() << *this << " about to register (LinphoneCore version: " << linphone_core_get_version() << ")"; if (mOp) mOp->release(); mOp = new SalRegisterOp(getCCore()->sal.get()); @@ -697,15 +715,12 @@ void Account::registerAccount() { } else { /* unregister if registered*/ unregister(); - if (mState == LinphoneRegistrationProgress) { - setState(LinphoneRegistrationCleared, "Registration cleared"); - } } } void Account::refreshRegister() { if (!mParams) { - lWarning() << "refreshRegister is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "refreshRegister is called but no AccountParams is set on " << *this; return; } @@ -721,10 +736,12 @@ void Account::pauseRegister() { } void Account::unregister() { - if (mOp && - (mState == LinphoneRegistrationOk || (mState == LinphoneRegistrationProgress && mParams->mExpires != 0))) { - mOp->unregister(); - mIsUnregistering = true; + if (mOp) { + if (mState == LinphoneRegistrationOk || mState == LinphoneRegistrationFailed) { + lInfo() << *this << " unregistering."; + mOp->unregister(); + mIsUnregistering = true; + } } } @@ -837,7 +854,7 @@ LinphoneTransportType Account::getTransport() { bool Account::isAvpfEnabled() const { if (!mParams) { - lWarning() << "isAvpfEnabled is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "isAvpfEnabled is called but no AccountParams is set on " << *this; return false; } auto core = getCCore(); @@ -851,7 +868,7 @@ bool Account::isAvpfEnabled() const { const LinphoneAuthInfo *Account::findAuthInfo() const { if (!mParams) { - lWarning() << "findAuthInfo is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "findAuthInfo is called but no AccountParams is set on " << *this; return nullptr; } @@ -862,7 +879,7 @@ const LinphoneAuthInfo *Account::findAuthInfo() const { int Account::getUnreadChatMessageCount() const { if (!mParams) { - lWarning() << "getUnreadMessageCount is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "getUnreadMessageCount is called but no AccountParams is set on " << *this; return -1; } @@ -872,7 +889,7 @@ int Account::getUnreadChatMessageCount() const { list<shared_ptr<AbstractChatRoom>> Account::getChatRooms() const { list<shared_ptr<AbstractChatRoom>> results; if (!mParams) { - lWarning() << "getChatRooms is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "getChatRooms is called but no AccountParams is set on " << *this; return results; } @@ -900,7 +917,7 @@ void Account::setMissedCallsCount(int count) { list<shared_ptr<CallLog>> Account::getCallLogs() const { if (!mParams) { - lWarning() << "getCallLogs is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "getCallLogs is called but no AccountParams is set on " << *this; list<shared_ptr<CallLog>> callLogs; return callLogs; } @@ -912,7 +929,7 @@ list<shared_ptr<CallLog>> Account::getCallLogs() const { list<shared_ptr<CallLog>> Account::getCallLogsForAddress(const std::shared_ptr<Address> remoteAddress) const { if (!mParams) { - lWarning() << "getCallLogsForAddress is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "getCallLogsForAddress is called but no AccountParams is set on " << *this; list<shared_ptr<CallLog>> callLogs; return callLogs; } @@ -924,7 +941,7 @@ list<shared_ptr<CallLog>> Account::getCallLogsForAddress(const std::shared_ptr<A void Account::deleteCallLogs() const { if (!mParams) { - lWarning() << "deleteCallLogs is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "deleteCallLogs is called but no AccountParams is set on Account " << *this; return; } @@ -935,7 +952,7 @@ void Account::deleteCallLogs() const { list<shared_ptr<ConferenceInfo>> Account::getConferenceInfos() const { if (!mParams) { - lWarning() << "getConferenceInfos is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "getConferenceInfos is called but no AccountParams is set on " << *this; list<shared_ptr<ConferenceInfo>> conferences; return conferences; } @@ -947,7 +964,7 @@ list<shared_ptr<ConferenceInfo>> Account::getConferenceInfos() const { void Account::writeToConfigFile(int index) { if (!mParams) { - lWarning() << "writeToConfigFile is called but no AccountParams is set on Account [" << this->toC() << "]"; + lWarning() << "writeToConfigFile is called but no AccountParams is set on " << *this; return; } @@ -970,6 +987,11 @@ bool Account::canRegister() { if (mDependency) { return mDependency->getState() == LinphoneRegistrationOk; } + if (getState() == LinphoneRegistrationNone && mDeletionDate != 0) { + // Account is removed, but never registered before. + // This case happens when doing a removeAccount() while network is off. + return false; + } return true; } @@ -1001,19 +1023,19 @@ int Account::done() { mRegisterChanged = false; } - if (mNeedToRegister) { - pauseRegister(); - } + // if (mNeedToRegister) { + // pauseRegister(); + // } if (computePublishParamsHash()) { - lInfo() << "Publish params have changed on account [" << this->toC() << "]"; + lInfo() << "Publish params have changed on " << *this; if (mPresencePublishEvent) { /*publish is terminated*/ mPresencePublishEvent->terminate(); } if (mParams->mPublishEnabled) setSendPublish(true); } else { - lInfo() << "Publish params have not changed on account [" << this->toC() << "]"; + lInfo() << "Publish params have not changed " << *this; } if (getCCore()) { @@ -1090,7 +1112,7 @@ void Account::apply(LinphoneCore *lc) { shared_ptr<EventPublish> Account::createPublish(const std::string event, int expires) { if (!getCore()) { - lError() << "Cannot create publish from account [" << this->toC() << "] not attached to any core"; + lError() << "Cannot create publish from " << *this << " not attached to any core"; return nullptr; } return dynamic_pointer_cast<EventPublish>( @@ -1207,11 +1229,11 @@ int Account::sendPublish() { bool Account::check() { if (mParams->mProxy.empty()) { - lWarning() << "No proxy given for account " << this; + lWarning() << "No proxy given for " << *this; return false; } if (mParams->mIdentityAddress == NULL) { - lWarning() << "Identity address of account " << this << " has not been set"; + lWarning() << "Identity address of " << *this << " has not been set"; return false; } resolveDependencies(); diff --git a/src/account/account.h b/src/account/account.h index 92ba40083a9a243073ea8259fea338655af21c3a..f54afc546af5e791773b8e0c72c1a11dbf8d86eb 100644 --- a/src/account/account.h +++ b/src/account/account.h @@ -178,6 +178,7 @@ private: std::list<SalAddress *> getOtherContacts(); void triggerUpdate(); + void handleDeletion(); std::shared_ptr<AccountParams> mParams; @@ -189,7 +190,7 @@ private: bool mSendPublish = false; bool mIsUnregistering = false; - time_t mDeletionDate; + time_t mDeletionDate = 0; std::string mSipEtag; @@ -243,6 +244,11 @@ public: } }; +inline std::ostream &operator<<(std::ostream &ostr, const Account &account) { + ostr << "Account [" << (void *)&account << "]"; + return ostr; +} + LINPHONE_END_NAMESPACE #endif // ifndef _L_ACCOUNT_H_ diff --git a/tester/account_tester.c b/tester/account_tester.c index d96d3947a17bd41dba78ac062de2d15c071987de..1a67085ebf3d4b7b1df13908bb9d633586f9c5f7 100644 --- a/tester/account_tester.c +++ b/tester/account_tester.c @@ -89,7 +89,7 @@ static void simple_account_creation(void) { ms_free(local_rc); } -void simple_account_params_creation(void) { +static void simple_account_params_creation(void) { LinphoneCoreManager *marie = linphone_core_manager_new("marie_rc"); LinphoneAccountParams *params = linphone_core_create_account_params(marie->lc); @@ -102,9 +102,35 @@ void simple_account_params_creation(void) { linphone_core_manager_destroy(marie); } -void registration_state_changed_on_account(LinphoneAccount *account, - LinphoneRegistrationState state, - BCTBX_UNUSED(const char *message)) { +static void account_removal(void) { + LinphoneCoreManager *marie = linphone_core_manager_new("marie_rc"); + LinphoneAccount *account = linphone_core_get_default_account(marie->lc); + if (BC_ASSERT_PTR_NOT_NULL(account)) { + BC_ASSERT_TRUE(linphone_account_get_state(account) == LinphoneRegistrationOk); + linphone_core_remove_account(marie->lc, account); + BC_ASSERT_TRUE(wait_for(marie->lc, NULL, &marie->stat.number_of_LinphoneRegistrationCleared, 1)); + } + linphone_core_manager_destroy(marie); +} + +static void account_removal_2(void) { + LinphoneCoreManager *marie = linphone_core_manager_new("marie_rc"); + LinphoneAccount *account = linphone_core_get_default_account(marie->lc); + if (BC_ASSERT_PTR_NOT_NULL(account)) { + BC_ASSERT_TRUE(linphone_account_get_state(account) == LinphoneRegistrationOk); + linphone_account_refresh_register(account); + BC_ASSERT_TRUE(linphone_account_get_state(account) == LinphoneRegistrationRefreshing); + /* then immediately drop the account */ + linphone_core_remove_account(marie->lc, account); + /* It should unregister anyway */ + BC_ASSERT_TRUE(wait_for(marie->lc, NULL, &marie->stat.number_of_LinphoneRegistrationCleared, 1)); + } + linphone_core_manager_destroy(marie); +} + +static void registration_state_changed_on_account(LinphoneAccount *account, + LinphoneRegistrationState state, + BCTBX_UNUSED(const char *message)) { LinphoneCore *lc = linphone_account_get_core(account); stats *counters; ms_message("New registration state %s for user id [%s] at account [%s]\n", @@ -246,8 +272,10 @@ static void account_dependency_to_self(void) { linphone_core_manager_destroy(marie); } -test_t account_tests[] = { +static test_t account_tests[] = { TEST_NO_TAG("Simple account creation", simple_account_creation), + TEST_NO_TAG("Account removal", account_removal), + TEST_NO_TAG("Account removal while refreshing", account_removal_2), TEST_NO_TAG("Simple account params creation", simple_account_params_creation), TEST_NO_TAG("Account dependency to self", account_dependency_to_self), TEST_NO_TAG("Registration state changed callback on account", registration_state_changed_callback_on_account), diff --git a/tester/register_tester.c b/tester/register_tester.c index ed3ebc6fb33e3def22d45cc0094bb2631af7fb1e..41fbba082694be3f678c95461c46984aceb0afce 100644 --- a/tester/register_tester.c +++ b/tester/register_tester.c @@ -1034,9 +1034,7 @@ static void proxy_transport_change_with_wrong_port_givin_up(void) { LinphoneProxyConfig *proxy_config; LinphoneAuthInfo *info = linphone_auth_info_new(test_username, NULL, test_password, NULL, auth_domain, NULL); /*create authentication structure from identity*/ - char route[256]; LinphoneTransports *transport = linphone_factory_create_transports(linphone_factory_get()); - sprintf(route, "sip:%s", test_route); linphone_transports_set_udp_port(transport, LC_SIP_TRANSPORT_RANDOM); linphone_transports_set_tcp_port(transport, LC_SIP_TRANSPORT_RANDOM); linphone_transports_set_tls_port(transport, LC_SIP_TRANSPORT_RANDOM); @@ -1044,22 +1042,22 @@ static void proxy_transport_change_with_wrong_port_givin_up(void) { linphone_core_add_auth_info(lcm->lc, info); /*add authentication info to LinphoneCore*/ linphone_auth_info_unref(info); - register_with_refresh_base_3(lcm->lc, FALSE, auth_domain, "sip2.linphone.org:5987", 0, transport, + register_with_refresh_base_3(lcm->lc, FALSE, auth_domain, "sip2.linphone.org:5987;transport=tcp", 0, transport, LinphoneRegistrationProgress); linphone_transports_unref(transport); + BC_ASSERT(wait_for_until(lcm->lc, lcm->lc, &counters->number_of_LinphoneRegistrationFailed, 1, 33000)); + proxy_config = linphone_core_get_default_proxy_config(lcm->lc); linphone_proxy_config_edit(proxy_config); - - BC_ASSERT_FALSE(wait_for_until(lcm->lc, lcm->lc, &counters->number_of_LinphoneRegistrationCleared, 1, 3000)); linphone_proxy_config_enableregister(proxy_config, FALSE); linphone_proxy_config_done(proxy_config); - BC_ASSERT(wait_for(lcm->lc, lcm->lc, &counters->number_of_LinphoneRegistrationCleared, 1)); - BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationCleared, 1, int, "%d"); + BC_ASSERT(wait_for_until(lcm->lc, lcm->lc, &counters->number_of_LinphoneRegistrationNone, 1, 33000)); + BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationCleared, 0, int, "%d"); BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationOk, 0, int, "%d"); BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationProgress, 1, int, "%d"); - BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationFailed, 0, int, "%d"); + BC_ASSERT_EQUAL(counters->number_of_LinphoneRegistrationFailed, 1, int, "%d"); linphone_core_manager_destroy(lcm); }