[messaging] Fix synchronization issues 41/206641/1 accepted/tizen_5.0_unified accepted/tizen/5.0/unified/20190529.001828 submit/tizen_5.0/20190528.054019
authorPawel Wasowski <p.wasowski2@samsung.com>
Thu, 9 May 2019 11:31:49 +0000 (13:31 +0200)
committerPawel Wasowski <p.wasowski2@samsung.com>
Wed, 22 May 2019 10:36:28 +0000 (10:36 +0000)
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 <p.wasowski2@samsung.com>
(cherry picked from commit 1440977fb5202077c98d86da90ee2046bc213132)

src/messaging/email_manager.cc
src/messaging/message.cc
src/messaging/message.h

index 8c1207b..5b89dd6 100644 (file)
 //#include <JSWebAPIErrorFactory.h>
 //#include <JSWebAPIError.h>
 //#include <JSUtil.h>
+#include <chrono>
+#include <list>
 #include <memory>
 #include <sstream>
+#include <thread>
 #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<std::mutex> lock(m_mutex);
   std::vector<std::shared_ptr<Message>> messages = callback->getMessages();
+  std::list<std::shared_ptr<Message>> 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);
 }
 
index 305985d..89d0a05 100644 (file)
@@ -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;
 }
index c4c5c15..7562fc3 100644 (file)
@@ -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);