662056 - Implement move later functionality to prevent conflicts with filter actions needing the body (race condition)

Status: NEW

(MailNews Core :: Filters, defect)

Description

[Kent James (:rkent)]

The "move" filter action is both the most common action, as well as the most disruptive, as it deletes the underlying message. Unfortunately, if you have other filter actions on the same message that require the message body (such as printing, attachment detach, saving the message as a file, etc.) then you enter a race condition, so either the move fails or the other action fails.

In FiltaQuilla I have experimented with a workaround for this in an action I call "move later". With this, I set a property on the nsIMsgDBHdr for the object called "moveLaterCount". This count is incremented by the filter action that needs the body, and decremented when the async action is completed. When the count reaches zero, the move later is initiated.

For this to be more transparent to the user, that same functionality should be implemented in the core "Move" filter action.

Comment 1

[David :Bienvenu]

This is dealt with in the core code by making sure the move action is the last action performed, after all the other actions have finished. But, if you introduce async actions, but apply the actions synchronously, this breaks down.

Comment 2

[Kent James (:rkent)]

> But, if you introduce async actions, but apply the actions synchronously, this breaks down.

Right. The moveLater functionality deals with this breakdown by callbacks from the async actions that then trigger the move, if the moveLaterCount is zero.

Comment 3

[Kent James (:rkent)]

Actually perhaps a better idea, and the one that I can make work more reliably, is to do the move first, and apply the other filters after the move.

I've experimented with this concept with a detach attachments action, which is really tricky because it changes the message URI. Very difficult with moveLater, works really well with applying filter after the move.

In FiltaQuilla, I have a new "folder name" search term, and in IMAP you have to enable incoming filters on the destination folder, and setup the complex filter action (like detachAttachments) to only apply if the folder name matches. This is way beyond the capabilities of the average user, while applying the filters after the move in core would be pretty straightforward to implement.

Comment 4

[:aceman]

Hi, any news on this enhancement?

Comment 5

[Kent James (:rkent)]

(In reply to :aceman from comment #4)
> Hi, any news on this enhancement?

I have not done any further work on this recently, though the issue remains.

Comment 6

[Wayne Mery (:wsmwk)]

It will be great if someone could pick this up.

Are these bugs involved?  bug 1071628, bug 1096096, bug 589092, bug 971401

Comment 7

[Thilo [:ThiloteE]]

It has been demonstrated at https://bugzilla.mozilla.org/show_bug.cgi?id=1781792#c23 that filter actions 1) copy to Imap sub-folder and 2) move to local folder within the same filter leads to dataloss

Problem (Filters with dataloss):

Filter (A) fails with

1. Copy the matching message to Imap Sub-Folder
2. Move the matching message to Local Folder

--> All mails are moved to Local Folder and the body is fine, but only ONE (the first) mail was copied to imap Sub-Folder

Filter (B) fails with

1. Copy the matching message to Local Folder
2. Move the matching message to imap Sub-Folder

--> All mails are copied to Local Folder, but all mails lack body, except first mail. All mails are moved correctly to imap Sub-Folder.

Filter (C) fails with

1. Move the matching message to Local Folder
2. Copy the matching message to imap Sub-Folder

--> All mails are moved to Local Folder and the body is fine, but only ONE (the first) mail was copied to imap Sub-Folder

Solution (Filter without dataloss):

Details are at https://bugzilla.mozilla.org/show_bug.cgi?id=1781792#c25. Use following filter actions in given order:

1. "Move later" the matching message to Imap Sub-Folder
2. Copy the matching message to Local Folder

This solution depends on the Filtaquilla Addon.

Comment 8

[gene smith]

From Thilo's comment 7:

This solution depends on the Filtaquilla Addon.

Yes, installing "filaquilla" and enabling "Move later" action seems to fix the problem described by Thilo above, at least on 128.12esr that I tested. This in spite of the fact that filaquilla help page says:

Filter actions that require the message body do not work correctly when a move is also requested
 for the message. Move Later is one solution to this problem, though it currently only works reliable
 with the Save Message as File action. So if you use that action combined with a move, try Move Later
 instead of the normal move, and the combination should work reliably. 

This implies that it might not work reliably if "copy" is the first rule, but it works OK for me and apparently also for commenter Thilo.

I can see the code to do the "move later" in filaquilla's source code file filtaquilla.js. But I don't know how to port that functionality into the main TB filter code. Any advice would be appreciated.

Kent James wrote in comment #3):

Actually perhaps a better idea, and the one that I can make work more
reliably, is to do the move first, and apply the other filters after the
move.

I've experimented with this concept with a detach attachments action, which
is really tricky because it changes the message URI. Very difficult with
moveLater, works really well with applying filter after the move.

In FiltaQuilla, I have a new "folder name" search term, and in IMAP you have
to enable incoming filters on the destination folder, and setup the complex
filter action (like detachAttachments) to only apply if the folder name
matches. This is way beyond the capabilities of the average user, while
applying the filters after the move in core would be pretty straightforward
to implement.

I'm not sure if this is a better way instead of the "move later". FWIW, it appears that filter rule setup forces "move" rule to be last in an attempt to avoid the problem, but, by itself, doesn't fix the problem (messages will still be deleted or partially deleted while they get copied). So I'm not seeing how forcing moves to be first would help at all -- at least in combination with copy.

Comment 9

[Wayne Mery (:wsmwk)]

ISTR Ben or Geoff commenting recently in a refactoring bug about the need for a better process.

Comment 10

[gene smith]

(In reply to Wayne Mery (:wsmwk) from comment #9)

ISTR Ben or Geoff commenting recently in a refactoring bug about the need for a better process.

Maybe you are referring to this bug 1781792, not sure. (It's the first bug in the "Blocks" list above.)