Subject: [sup-devel] [PATCH] Delete messages from index if there is no location left

From: Sascha Silbe <[email-masked]>
Date: Fri, 11 Feb 2011 15:30:15 +0100

If messages don't have any location, we cannot access their body and are also
unable to restore them from a message state dump (written by sup-dump). So
it's more consistent to the user if we remove them completely. This is
most likely what the user expects anyway: why else would they delete the
message from all locations?

The drawback is that messages that were moved between sources need to be
either scanned in a single sup-sync run or the new location must be scanned
first. But that isn't a regression from behaviour before the maildir branch
was merged.

Signed-off-by: Sascha Silbe <[email-masked]>
---

I've had this in my branch for quite some time now, but didn't have a
a chance yet to thoroughly test it after rebasing on top of next (which
made changes in the same area of the code, causing conflicts). Feel
free to push to next and/or master once you verified it doesn't cause
you to loose mails.

 bin/sup-sync     |    9 +++++++++
 lib/sup/index.rb |    9 +++++++++
 lib/sup/poll.rb  |    9 +++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/bin/sup-sync b/bin/sup-sync
index b4d5cba..c54359d 100755
--- a/bin/sup-sync
+++ b/bin/sup-sync
@@ -115,6 +115,7 @@ def time
     end
   end

+  deleted_message_ids = Set.new
   sources.each do |source|
     puts "Scanning #{source}..."
     num_added = num_updated = num_deleted = num_scanned = num_restored = 0
@@ -124,9 +125,11 @@ def time
       num_scanned += 1
       if action == :delete
         num_deleted += 1
+        deleted_message_ids.add m.id if m.locations.empty?
         puts "Deleting #{m.id}" if opts[:verbose]
       elsif action == :add
         seen[m.id] = true
+        deleted_message_ids.delete(m.id)

         ## tweak source labels according to commandline arguments if necessary
         m.labels.delete :inbox if opts[:archive]
@@ -190,6 +193,12 @@ def time
     puts "Restored state on #{num_restored} (#{100.0 * num_restored / num_scanned}%) messages." if num_restored > 0
   end

+  puts "Deleting #{deleted_message_ids.size} messages from index."
+  deleted_message_ids.each do |id|
+    debug "Removing #{id} from index"
+    index.remove_message_by_id id
+  end
+
   index.save

   if opts[:optimize]
diff --git a/lib/sup/index.rb b/lib/sup/index.rb
index 4ad91e7..7dcc7f9 100644
--- a/lib/sup/index.rb
+++ b/lib/sup/index.rb
@@ -129,6 +129,7 @@ def load_index
   def add_message m; sync_message m, true end
   def update_message m; sync_message m, true end
   def update_message_state m; sync_message m, false end
+  def remove_message_by_id id; delete_message id end

   def save_index
     info "Flushing Xapian updates to disk. This may take a while..."
@@ -627,6 +628,14 @@ def build_xapian_query opts
     end
   end

+  def delete_message id
+    doc = synchronize { find_doc(id) }
+    fail unless doc
+    doc.clear_terms
+    doc.clear_values
+    synchronize { @xapian.delete_document doc.docid }
+  end
+
   def sync_message m, overwrite
     doc = synchronize { find_doc(m.id) }
     existed = doc != nil
diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
index 7e05292..46ad441 100644
--- a/lib/sup/poll.rb
+++ b/lib/sup/poll.rb
@@ -98,6 +98,7 @@ def do_poll
     from_and_subj = []
     from_and_subj_inbox = []
     loaded_labels = Set.new
+    deleted_message_ids = Set.new

     @mutex.synchronize do
       @poll_sources.each do |source|
@@ -112,9 +113,11 @@ def do_poll
         numi = 0
         poll_from source do |action,m,old_m,progress|
           if action == :delete
+            deleted_message_ids.add m.id if m.locations.empty?
             yield "Deleting #{m.id}"
           elsif action == :add
             if old_m
+              deleted_message_ids.delete(m.id)
               new_locations = (m.locations - old_m.locations)
               if not new_locations.empty?
                 yield "Message at #{new_locations[0].info} is an update of an old message. Updating labels from #{old_m.labels.to_a * ','} => #{m.labels.to_a * ','}"
@@ -139,6 +142,12 @@ def do_poll
         total_numi += numi
       end

+      yield "Deleting #{deleted_message_ids.size} messages from index."
+      deleted_message_ids.each do |id|
+        debug "Removing #{id} from index"
+        Index.remove_message_by_id id
+      end
+
       loaded_labels = loaded_labels - LabelManager::HIDDEN_RESERVED_LABELS - [:inbox, :killed]
       yield "Done polling; loaded #{total_num} new messages total"
       @last_poll = Time.now
--
1.7.2.3

_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: Romain Dessort <[email-masked]>
Date: Fri, 11 Feb 2011 19:18:31 +0100

> message from all locations?

Yes, I agree with you, this should be the normal behaviour.

> [...]

Thanks for this patch !
I going to try to merge it with my current Sup version.
-- 
Romain Dessort
Jabber ID : [email-masked]
GnuPG : 3072D/724BC532
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel