[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    [Kde-pim] [PATCH] dimap status handling improvements
From:       Till Adam <adam () kde ! org>
Date:       2004-07-14 9:40:05
Message-ID: 200407141140.12740.adam () kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]

[Attachment #4 (multipart/mixed)]


Hey folks,

as discussed in the "[Kde-pim] Kmail and MS-Exchange via IMAP" thread, there 
are situations in which message status is not handled as gracefully as it 
could be with disconnected imap. Namely:

1) messages with the deleted flag set are downloaded, which makes no sense

2) each client first uploads its flags, thereby overriding what is on the 
    server, and then syncs the flags from the server (which it has just
    written). That causes problems when syncing from mutliple clients.

The atached patch remedies 1) by checking for the deleted flag and avoiding 
download of those messages. This is based on the patch David posted to 
kde-pim. It also lessens the impact of 2) by introducing a per folder bool 
mStatusChangedLocally which is set to false after a complete sync and set to 
true after the first setStatus() was performed on the folder. If it is not 
set, the sync will skip uploading of flags, since the local store did not 
change, status wise, and it can be safely assumed, that the status on the 
server is more accurate, should it differ from the local store. As soon as 
one mail changes status locally, in a folder, the folder is "dirty" and will 
upload its flags to the server, following a "last to write" strategy.

Please review and test. 

Note that at least my cyrus only commits flag changes to messages after a 
folder is de-selected, which means if you want to see the change you do to 
status flags with online imap, you'll have to switch folders, with online 
imap, otherwise the server will not commit the status changes, and a sync of 
that folder will get the old flags.

Till

P.S.: I'm moving this to kmail-devel, where it is more appropriate, I guess.

["KMail-dimap-status-improvements.diff" (text/x-diff)]

Index: kmfoldercachedimap.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmfoldercachedimap.cpp,v
retrieving revision 1.128
diff -u -3 -p -b -r1.128 kmfoldercachedimap.cpp
--- kmfoldercachedimap.cpp	30 Jun 2004 12:29:25 -0000	1.128
+++ kmfoldercachedimap.cpp	14 Jul 2004 09:28:58 -0000
@@ -124,7 +124,7 @@ KMFolderCachedImap::KMFolderCachedImap( 
     mLastUid( 0 ), uidWriteTimer( -1 ), mUserRights( 0 ),
     mFolderRemoved( false ), mResync( false ),
     /*mHoldSyncs( false ),*/ mRecurse( true ),
-    mContentsTypeChanged( false )
+    mContentsTypeChanged( false ), mStatusChangedLocally( false )
 {
   setUidValidity("");
   mLastUid=0;
@@ -138,10 +138,11 @@ KMFolderCachedImap::~KMFolderCachedImap(
   if( !mFolderRemoved ) {
     // Only write configuration when the folder haven't been deleted
     KConfig* config = KMKernel::config();
-    KConfigGroupSaver saver(config, "Folder-" + folder()->idString());
-    config->writeEntry("ImapPath", mImapPath);
-    config->writeEntry("NoContent", mNoContent);
-    config->writeEntry("ReadOnly", mReadOnly);
+    KConfigGroupSaver saver( config, "Folder-" + folder()->idString() );
+    config->writeEntry( "ImapPath", mImapPath );
+    config->writeEntry( "NoContent", mNoContent );
+    config->writeEntry( "ReadOnly", mReadOnly );
+    config->writeEntry( "StatusChangedLocally", mStatusChangedLocally );
 
     writeUidCache();
   }
@@ -174,6 +175,8 @@ void KMFolderCachedImap::readConfig()
 
   KMFolderMaildir::readConfig();
   mContentsTypeChanged = false;
+  mStatusChangedLocally =
+    config->readBoolEntry( "StatusChangedLocally", false );
 }
 
 void KMFolderCachedImap::remove()
@@ -629,10 +632,15 @@ void KMFolderCachedImap::serverSyncInter
        // We haven't downloaded messages yet, so we need to build the map.
        if( uidMapDirty )
          reloadUidMap();
-       // Upload flags, unless we know from the ACL that we're not allowed to do \
that +       // Upload flags, unless we know from the ACL that we're not allowed
+       // to do that or they did not change locally
        if ( mUserRights <= 0 || ( mUserRights & KMail::ACLJobs::WriteFlags ) ) {
+         if ( mStatusChangedLocally ) {
          uploadFlags();
          break;
+         } else {
+           kdDebug(5006) << "Skipping flags upload, folder unchanged: " << label() \
<< endl; +         }
        }
     }
     // Else carry on
@@ -964,6 +972,13 @@ void KMFolderCachedImap::slotImapStatusC
 }
 
 
+void KMFolderCachedImap::setStatus(QValueList<int>& ids, KMMsgStatus status, bool \
toggle) +{
+  KMFolderMaildir::setStatus(ids, status, toggle);
+  // This is not perfect, what if the status didn't really change? Oh well ...
+  mStatusChangedLocally = true;
+}
+
 /* Upload new folders to server */
 void KMFolderCachedImap::createNewFolders()
 {
@@ -1145,7 +1160,9 @@ void KMFolderCachedImap::slotGetMessages
     KMMessage msg;
     msg.fromString((*it).cdata.mid(16, pos - 16));
     flags = msg.headerField("X-Flags").toInt();
+    bool deleted = ( flags & 8 );
     ulong uid = msg.UID();
+    if ( !deleted ) {
     if( uid != 0 ) {
       if ( uidsOnServer.count() == uidsOnServer.size() ) {
         uidsOnServer.resize( KMail::nextPrime( uidsOnServer.size() * 2 ) );
@@ -1153,7 +1170,7 @@ void KMFolderCachedImap::slotGetMessages
       }
       uidsOnServer.insert( uid, &v );
     }
-    if ( /*flags & 8 ||*/ uid <= lastUid()) {
+      if (  uid <= lastUid()) {
       /*
        * If this message UID is not present locally, then it must
        * have been deleted by the user, so we delete it on the
@@ -1163,15 +1180,19 @@ void KMFolderCachedImap::slotGetMessages
        */
       // kdDebug(5006) << "KMFolderCachedImap::slotGetMessagesData() : folder \
"<<label()<<" already has msg="<<msg->headerField("Subject") << ", UID="<<uid << ", \
lastUid = " << mLastUid << endl;  KMMsgBase *existingMessage = findByUID(uid);
+          // if this is a read only folder, ignore status updates from the server
+          // since we can't write our status back our local version is what has to
+          // be considered correct.
       if( !existingMessage ) {
          // kdDebug(5006) << "message with uid " << uid << " is gone from local \
cache. Must be deleted on server!!!" << endl;  uidsForDeletionOnServer << uid;
       } else {
+          if (!mReadOnly) {
         /* The message is OK, update flags */
-        if (!mReadOnly)
           KMFolderImap::flagsToStatus( existingMessage, flags );
-         // kdDebug(5006) << "message with uid " << uid << " found in the local \
cache. " << endl;  }
+        }
+        // kdDebug(5006) << "message with uid " << uid << " found in the local \
cache. " << endl;  } else {
       ulong size = msg.headerField("X-Length").toULong();
       mMsgsForDownload << KMail::CachedImapJob::MsgForDownload(uid, flags, size);
@@ -1179,6 +1200,7 @@ void KMFolderCachedImap::slotGetMessages
          mUidsForDownload << uid;
 
     }
+    }
     (*it).cdata.remove(0, pos);
     (*it).done++;
     pos = (*it).cdata.find("\r\n--IMAPDIGEST", 1);
@@ -1193,6 +1215,7 @@ void KMFolderCachedImap::getMessagesResu
   } else {
     if( lastSet ) { // always true here (this comes from online-imap...)
       mContentState = imapFinished;
+      mStatusChangedLocally = false; // we are up to date again
     }
   }
   serverSyncInternal();
Index: kmfoldercachedimap.h
===================================================================
RCS file: /home/kde/kdepim/kmail/kmfoldercachedimap.h,v
retrieving revision 1.54
diff -u -3 -p -b -r1.54 kmfoldercachedimap.h
--- kmfoldercachedimap.h	21 Jun 2004 12:10:52 -0000	1.54
+++ kmfoldercachedimap.h	14 Jul 2004 09:28:58 -0000
@@ -214,6 +214,9 @@ public:
   /// Reimplemented from FolderStorage
   void setContentsType( KMail::FolderContentsType type );
 
+  // Reimplemented so the mStatusChangedLocally bool can be set
+  virtual void setStatus(QValueList<int>& ids, KMMsgStatus status, bool toggle);
+
 protected slots:
   /**
    * Connected to ListJob::receivedFolders
@@ -369,6 +372,11 @@ private:
   bool mRecurse;
   bool mCreateInbox;
   bool mContentsTypeChanged;
+  /** Set to true by setStatus. Indicates that the client has changed
+      the status of at least one mail. The mail flags will therefor be
+      uploaded to the server, overwriting the server's notion of the status
+      of the mails in this folder. */
+  bool mStatusChangedLocally;
 };
 
 #endif /*kmfoldercachedimap_h*/


[Attachment #8 (application/pgp-signature)]

_______________________________________________
kde-pim mailing list
kde-pim@mail.kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic