From d028a5ddc48cf2a342b51f61deb08fdf7e4153e3 Mon Sep 17 00:00:00 2001 From: Pawel Wasowski Date: Thu, 9 May 2019 13:31:49 +0200 Subject: [PATCH] [messaging] Fix synchronization issues Due to the delay between updating email with native C API functions and updates of the corresponding message/thread records in the mail database, race conditions occured in the previous implementation, resulting in inconsitencies between thread_ids in the DB and those of messages in JS layer. Active waits have been added to mitigate this problems. [Verification] tct-tizen-messaging-email pass rate: 100% Change-Id: Ie0771ffc40e6cdee696756a8e831ae56bbe8f84d Signed-off-by: Pawel Wasowski (cherry picked from commit 1440977fb5202077c98d86da90ee2046bc213132) --- src/messaging/email_manager.cc | 155 +++++++++++++++++++++++++++++++++++++++-- src/messaging/message.cc | 5 ++ src/messaging/message.h | 1 + 3 files changed, 155 insertions(+), 6 deletions(-) diff --git a/src/messaging/email_manager.cc b/src/messaging/email_manager.cc index 8c1207b..5b89dd6 100644 --- a/src/messaging/email_manager.cc +++ b/src/messaging/email_manager.cc @@ -17,8 +17,11 @@ //#include //#include //#include +#include +#include #include #include +#include #include "common/logger.h" #include "common/platform_exception.h" #include "common/scope_exit.h" @@ -62,6 +65,7 @@ using namespace common; using namespace extension::tizen; +using namespace std::chrono_literals; namespace extension { namespace messaging { @@ -69,6 +73,16 @@ namespace messaging { namespace { const int ACCOUNT_ID_NOT_INITIALIZED = -1; const std::string FIND_FOLDERS_ATTRIBUTE_ACCOUNTID_NAME = "serviceId"; + +bool isFirstInThread(const Message* message) { + ScopeLogger(); + return message->getId() == message->getConversationId(); +} + +bool isFirstInThread(const email_mail_data_t* mail_data) { + ScopeLogger(); + return mail_data->mail_id == mail_data->thread_id; +} } // anonymous namespace EmailManager::EmailManager() : m_slot_size(-1), m_is_initialized(false) { @@ -251,11 +265,68 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< } } - err = email_get_mail_data(message->getId(), &mail_data_final); - if (EMAIL_ERROR_NONE != err) { - return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Couldn't retrieve added mail data", - ("email_get_mail_data error: %d (%s)", err, get_error_message(err))); + /* + * If the message is first in its thread (its mail_id == thread_id) + * and it has an attachment, there is some delay between addEmailAttachments + * return and the corresponding update of message's record in the internal + * email DB. + * + * The internal DB is queried for mail_data_final below, but due to + * the mentioned delay, it may return mail_data_final, that is not + * up-to-date, i.e with mail_id != thread_id). + * If such situation occurs, up to MAX_RETRIES retries of calling + * email_get_mail_data are performed. + * If returned mail_data_final is up-to-date before reaching retries limit, + * the returned value is used to set mail properties. + * If returned mail_data_final is still not up-to-date, thread_id of the message + * is set manually to mail_id. + */ + + if (isFirstInThread(mail_data)) { + int retry = 0; + const int MAX_RETRIES = 5; + for (; retry < MAX_RETRIES; ++retry) { + err = email_get_mail_data(message->getId(), &mail_data_final); + + if (EMAIL_ERROR_NONE != err) { + /* + * TODO: in the case of email_get_mail_data failure, + * the message should be removed from the databse + */ + return LogAndCreateResult( + ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox", + ("email_get_mail_data error: %d (%s)", err, get_error_message(err))); + } + + if (isFirstInThread(mail_data) && isFirstInThread(mail_data_final)) { + LoggerD("Message adding process finished after %d retries. mail_id == thread_id", retry); + break; + } + + int free_error = email_free_mail_data(&mail_data_final, 1); + if (EMAIL_ERROR_NONE != free_error) { + LoggerW("email_free_mail_data error: %d, %s", free_error, get_error_message(free_error)); + } + LoggerD("Retry number %d failed", retry); + std::this_thread::sleep_for(100ms); + } + + if (MAX_RETRIES == retry) { + LoggerD( + "Message adding process not finished after %d retries. Setting proper conversationId " + "manually", + retry); + mail_data_final->thread_id = mail_data_final->mail_id; + } + } else { + err = email_get_mail_data(message->getId(), &mail_data_final); + if (EMAIL_ERROR_NONE != err) { + return LogAndCreateResult( + ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox", + ("email_get_mail_data error: %d (%s)", err, get_error_message(err))); + } } + ret = message->updateEmailMessage(*mail_data_final); if (ret.IsError()) { return ret; @@ -915,6 +986,7 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca std::lock_guard lock(m_mutex); std::vector> messages = callback->getMessages(); + std::list> firstInThreadAndHasAttachment; MessageType type = callback->getMessageServiceType(); for (auto it = messages.begin(); it != messages.end(); ++it) { if ((*it)->getType() != type) { @@ -927,7 +999,11 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca if (ret.IsError()) return ret; if ((*it)->getHasAttachment()) { - LoggerD("Message has attachments. Workaround need to be used."); + if (isFirstInThread(it->get())) { + // Messages with attachments, that are first in their threads are added + // to the container, to be processed later in this function + firstInThreadAndHasAttachment.push_back(*it); + } // Update of mail on server using function email_update_mail() is not possible. // Attachment is updated only locally (can't be later loaded from server), // so use of workaround is needed: @@ -944,8 +1020,8 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca // storing old message id (*it)->setOldId(mail->mail_id); // deleting old mail - LoggerD("mail deleted = [%d]\n", mail->mail_id); error = email_delete_mail(mail->mailbox_id, &mail->mail_id, 1, 1); + LoggerD("mail deleted = [%d]\n", mail->mail_id); if (EMAIL_ERROR_NONE != error) { return LogAndCreateResult( ErrorCode::UNKNOWN_ERR, "Error while deleting old mail on update", @@ -961,6 +1037,73 @@ PlatformResult EmailManager::UpdateMessagesPlatform(MessagesCallbackUserData* ca } } } + + if (firstInThreadAndHasAttachment.size() == 0) { + return PlatformResult(ErrorCode::NO_ERROR); + } + + /* + * If a message with attachment, that is first in its thread was updated, + * the update process consists of adding a new message - updated version + * of the original and removing the original. + * + * After the original mail is deleted, the thread_id of the updated version + * changes - it is now identical to the mail_id of the new message. + * The change should be reflected in the updated message, sent to the JS + * layer. The code below sets proper thread_id for such messages and unsets + * inResponseTo fields. + * + * A few retries with sleeps between them are performed, to ensure, that after + * this function returns, messages returned to the JS layer and + * corresponding records in the mail DB have the same thread_ids. Due to a + * delay between calling email_delete_mail function and the update of records + * in the database, some time is needed. + */ + email_mail_data_t* mail_data{nullptr}; + int retry = 0; + const int MAX_RETRIES = 5; + for (; retry < MAX_RETRIES; ++retry) { + std::this_thread::sleep_for(100ms); + auto it = firstInThreadAndHasAttachment.begin(); + while (it != firstInThreadAndHasAttachment.end()) { + error = email_get_mail_data((*it)->getId(), &mail_data); + if (EMAIL_ERROR_NONE != error) { + return LogAndCreateResult( + ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox", + ("email_get_mail_data error: %d (%s)", error, get_error_message(error))); + } + + if (isFirstInThread(mail_data)) { + (*it)->setConversationId(mail_data->thread_id); + (*it)->unsetInResponseTo(); + it = firstInThreadAndHasAttachment.erase(it); + } else { + ++it; + } + + int free_error = email_free_mail_data(&mail_data, 1); + if (EMAIL_ERROR_NONE != free_error) { + LoggerW("email_free_mail_data error: %d, %s", free_error, get_error_message(free_error)); + } + } + + if (firstInThreadAndHasAttachment.size() == 0) { + LoggerD("Message updating process finished after %d retries", retry); + break; + } + + LoggerD("Message update retry number %d finished", retry); + } + + if (MAX_RETRIES == retry) { + LoggerW("Message updating process not finished after %d retries.", retry); + for (auto& message : firstInThreadAndHasAttachment) { + LoggerD("Setting proper conversationId manually, message id: %d", message->getId()); + message->setConversationId(message->getId()); + message->unsetInResponseTo(); + } + } + return PlatformResult(ErrorCode::NO_ERROR); } diff --git a/src/messaging/message.cc b/src/messaging/message.cc index 305985d..89d0a05 100644 --- a/src/messaging/message.cc +++ b/src/messaging/message.cc @@ -279,6 +279,11 @@ void Message::setInResponseTo(int inresp) { m_in_response_set = true; } +void Message::unsetInResponseTo() { + m_in_response = -1; + m_in_response_set = false; +} + void Message::setMessageStatus(MessageStatus status) { m_status = status; } diff --git a/src/messaging/message.h b/src/messaging/message.h index c4c5c15..7562fc3 100644 --- a/src/messaging/message.h +++ b/src/messaging/message.h @@ -98,6 +98,7 @@ class Message : public FilterableObject { virtual void setIsHighPriority(bool highpriority); virtual void setSubject(std::string subject); virtual void setInResponseTo(int inresp); + virtual void unsetInResponseTo(); virtual void setMessageStatus(MessageStatus status); virtual void setMessageAttachments(AttachmentPtrVector& attachments); virtual void setServiceId(int service_id); -- 2.7.4