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

List:       kde-pim
Subject:    [Kde-pim] [PATCH] fix IMAP filtering (Bug 95064)
From:       Thomas Neumann <tneumann () users ! sourceforge ! net>
Date:       2008-10-26 19:19:39
Message-ID: 20081026201939.50c65405 () emer
[Download RAW message or body]

Hi,

the attached patch fixes filtering for online IMAP accounts (Bug 95064).
What it basically does is avoiding duplicate filtering (see below for a
discussion). There might be a simpler way to achieve the same goal, but
I could not come up with an alternative strategy that is as robust
regarding IMAP server behavior.

The basic problem is as follows: A new message arrives, KMail filters
it, and updates the filtered result back on the IMAP server. Due to the
asynchronous execution if IMAP accesses, it can happen that KMail sees
its own update (which is a delete+insert) as new mail, triggering a new
filter run (and creating a duplicate in some cases, otherwise just
filtering and re-filtering all over).

The way I avoided this problem was to add a "filtered" flag in
MessageStatus, explicitly keeping track of already filtered messages
and refusing to filter them again. I don't think this can be avoided,
as all operations are asynchronous (e.g., new messages might arrive
in-between) and there does not seem to be a foolproof way to detect
that the "new" mail is really caused by an update.

Comments/suggestions welcome, if you think this is the right approach
please commit it, as I do not have SVN write access.

Thomas

["imapfiltering.patch" (text/x-patch)]

Index: kmail/kmacctimap.cpp
===================================================================
--- kmail/kmacctimap.cpp	(revision 875822)
+++ kmail/kmacctimap.cpp	(working copy)
@@ -599,6 +599,10 @@ int KMAcctImap::slotFilterMsg( KMMessage
   if ( serNum )
     mFilterSerNumsToSave.remove( QString( "%1" ).arg( serNum ) );
 
+  // refuse to filter a mail that was already filtered before
+  if ( msg->messageStatus().wasFiltered() )
+    return -1;
+
   int filterResult = kmkernel->filterMgr()->process(msg,
                                                     KMFilterMgr::Inbound,
                                                     true,
@@ -609,6 +613,9 @@ int KMAcctImap::slotFilterMsg( KMMessage
       i18nc( "@info:status", "Unable to process messages: " ) + \
QString::fromLocal8Bit(strerror(errno)));  return 2;
   }
+  // mark as filtered
+  msg->status().setWasFiltered();
+
   if (msg->parent()) { // unGet this msg
     int idx = -1;
     KMFolder * p = 0;
Index: libkdepim/messagestatus.h
===================================================================
--- libkdepim/messagestatus.h	(revision 875423)
+++ libkdepim/messagestatus.h	(working copy)
@@ -185,6 +185,11 @@ class KDEPIM_EXPORT MessageStatus
     */
     bool hasAttachment() const;
 
+    /** Check for Filtered status.
+        @return true if status indicates the message as filtered.
+    */
+    bool wasFiltered() const;
+
     /* ----- setters ----------------------------------------------------- */
 
     /** Set the status to new. */
@@ -259,6 +264,11 @@ class KDEPIM_EXPORT MessageStatus
     */
     void setHasAttachment( bool withAttachment = true );
 
+    /** Set the status for filering.
+        @param wasFiltered Set (true) or unset (false) this status flag.
+    */
+    void setWasFiltered( bool wasFiltered = true );
+
     /* ----- state representation  --------------------------------------- */
 
     /** Get the status as a whole e.g. for storage in an index.
@@ -398,6 +408,12 @@ class KDEPIM_EXPORT MessageStatus
     */
     static MessageStatus statusHasAttachment();
 
+    /** Return a predefined status initialized as Filtered as is useful
+        e.g. when providing a state for comparison.
+        @return A reference to a status instance initialized as Filtered.
+    */
+    static MessageStatus statusWasFiltered();
+
   private:
     qint32 mStatus;
 };
Index: libkdepim/messagestatus.cpp
===================================================================
--- libkdepim/messagestatus.cpp	(revision 875423)
+++ libkdepim/messagestatus.cpp	(working copy)
@@ -56,7 +56,8 @@ enum MsgStatus {
     KMMsgStatusToAct =             0x00001000,
     KMMsgStatusSpam =              0x00002000,
     KMMsgStatusHam =               0x00004000,
-    KMMsgStatusHasAttach =         0x00008000
+    KMMsgStatusHasAttach =         0x00008000,
+    KMMsgStatusFiltered =          0x00010000
 };
 
 MessageStatus::MessageStatus()
@@ -145,6 +146,9 @@ void MessageStatus::set( const MessageSt
   if ( other.hasAttachment() ) {
     setHasAttachment();
   }
+  if ( other.wasFiltered() ) {
+    setWasFiltered();
+  }
 }
 
 void MessageStatus::toggle( const MessageStatus &other )
@@ -186,6 +190,9 @@ void MessageStatus::toggle( const Messag
   if ( other.hasAttachment() ) {
     setHasAttachment( !( mStatus & KMMsgStatusHasAttach ) );
   }
+  if ( other.wasFiltered() ) {
+    setWasFiltered( !( mStatus & KMMsgStatusFiltered ) );
+  }
 }
 
 bool MessageStatus::isOfUnknownStatus() const
@@ -273,6 +280,11 @@ bool MessageStatus::hasAttachment() cons
   return ( mStatus & KMMsgStatusHasAttach );
 }
 
+bool MessageStatus::wasFiltered() const
+{
+  return ( mStatus & KMMsgStatusFiltered );
+}
+
 void MessageStatus::setNew()
 {
   // new overrides old and read
@@ -425,6 +437,15 @@ void MessageStatus::setHasAttachment( bo
   }
 }
 
+void MessageStatus::setWasFiltered( bool wasFiltered )
+{
+  if ( wasFiltered ) {
+    mStatus |= KMMsgStatusFiltered;
+  } else {
+    mStatus &= ~KMMsgStatusFiltered;
+  }
+}
+
 qint32 MessageStatus::toQInt32() const
 {
   return mStatus;
@@ -486,6 +507,9 @@ QString MessageStatus::getStatusStr() co
   if ( mStatus & KMMsgStatusHasAttach ) {
     sstr += 'T';
   }
+  if ( mStatus & KMMsgStatusFiltered ) {
+    sstr += '+';
+  }
 
   return sstr;
 }
@@ -545,11 +569,14 @@ void MessageStatus::setStatusFromStr( QS
   if ( aStr.contains( 'C' ) ) {
     setHasAttachment( false );
   }
+  if ( aStr.contains( '+' ) ) {
+    setWasFiltered();
+  }
 }
 
 QString MessageStatus::getSortRank() const
 {
-  QString sstr = "bcbbbbbbbb";
+  QString sstr = "bcbbbbbbbbb";
 
   // put watched ones first, then normal ones, ignored ones last
   if ( mStatus & KMMsgStatusWatched ) {
@@ -601,6 +628,9 @@ QString MessageStatus::getSortRank() con
   if ( mStatus & KMMsgStatusToAct ) {
     sstr[9] = 'a';
   }
+  if ( mStatus & KMMsgStatusFiltered ) {
+    sstr[10] = 'a';
+  }
 
   return sstr;
 }
@@ -725,3 +755,10 @@ MessageStatus MessageStatus::statusHasAt
   st.setHasAttachment();
   return st;
 }
+
+MessageStatus MessageStatus::statusWasFiltered()
+{
+  MessageStatus st;
+  st.setWasFiltered();
+  return st;
+}



_______________________________________________
KDE PIM mailing list kde-pim@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]