IMAP Followup flag with BackendCombined
-
@Luke1982 said in IMAP Followup flag with BackendCombined:
Since I can’t open a github issue
Actually the leading ticket system is a Jira installation hosted at https://jira.z-hub.io/secure/Dashboard.jspa. The development flow is explained at https://wiki.z-hub.io/display/ZP/Development+guidelines.
I am sure the z-push developers will get to your question once they find the time.
-
@fbartels Cool, didn’t know that. Will do.
-
Hi @Luke1982,
did you create https://jira.z-hub.io/browse/ZP-1561?
The IMAP backend was a community contribution and I’m not very familiar with it. I’m also not very familiar with the IMAP flags.
As far as I can see setting the flags coming from the IMAP server happens here: https://github.com/Z-Hub/Z-Push/blob/master/src/backend/imap/imap.php#L1243. Check https://github.com/Z-Hub/Z-Push/blob/master/src/lib/syncobjects/syncmailflags.php which properties are available for the flags. Maybe some of them is missing and that’s why device doesn’t process it correctly.
Manfred
-
Hi @Manfred
Yes, that’s the issue I created. Thanks for the input, very helpful. I’m also not very familiar with IMAP so this is a learning experience for me as well but let me try some debugging and see what happens.
I tried logging WBXML, but since that’s binary logging it isn’t of much use, Any idea how to decode it? I’d like to see what the eventual output to the device is.
-
Hi @Luke1982,
@Luke1982 said in IMAP Followup flag with BackendCombined:
I tried logging WBXML, but since that’s binary logging it isn’t of much use, Any idea how to decode it? I’d like to see what the eventual output to the device is.
how so? The WBXML data is in clear text in z-push.log if you set LOGLEVEL to LOGLEVEL_WBXML in the main Z-Push config.php.
Manfred
-
@Manfred You’re right, didn’t realize that. Let me do some tests there.
-
@Manfred Well it seems like something is going wrong already here https://github.com/Z-Hub/Z-Push/blob/master/src/backend/imap/imap.php#L1248 where the flagstatus is set to ‘FollowUp’ while this (https://github.com/Z-Hub/Z-Push/blob/master/src/lib/syncobjects/syncmailflags.php#L32) line clearly states in the comment that the only options are ‘clear, complete and active’. But the idea that there are three options for a flag already does not comply with the RFC that states the flag should be binary: either it’s there or it isn’t. Let me follow that path and test.
-
@Manfred it seems ‘GetMessage’ is only called when first retrieving the e-mail, not when checking for changes. I logged the WBXML and the message I chose as testmessage only showed up at the timestamp of the initial download (which was: starting outlook). After that the changes to the ‘read’ status on the server (that sync works) did come accross but I couldn’t find the e-mail in the logs anymore.
So the changing of existing mails happens somewhere else. Any tips?
-
Hi @Luke1982,
@Luke1982 said in IMAP Followup flag with BackendCombined:
@Manfred it seems ‘GetMessage’ is only called when first retrieving the e-mail, not when checking for changes. I logged the WBXML and the message I chose as testmessage only showed up at the timestamp of the initial download (which was: starting outlook). After that the changes to the ‘read’ status on the server (that sync works) did come accross but I couldn’t find the e-mail in the logs anymore.
So the changing of existing mails happens somewhere else. Any tips?
If I remember correctly GetMessage is called for every item which is to be synced. If it’s not called for the email which flag you’ve changed, then probably the imap backend hasn’t detected (or discarded) this change. Take a look at GetMessageList() and ChangesSink() functions in imap.php and check if they behave as expected.
Manfred
-
I’ve adapted ChangesSink to detect changes to folders/mailboxes that have changes in the FollowUp flag on at least one of the messages. Now I was, as you suggested, looking at
GetMessageList
. Since the initial message download usesGetMessage
and sends the flag status correctly, something is going wrong there.What I suspect is that this piece isn’t working, since I can’t find any other piece of code that uses that array key. How should
GetMessageList
feed its data to the diff engine so that it picks up the flag status correctly?Since right now I believe I do make
ChangesSink
detect flag changes, but still the backend doesn’t send the new flag status to the PDA (even when you force a re-download of the message by changing the ‘read’ status on a message).If I remember correctly GetMessage is called for every item which is to be synced.
It seems to be called, but the output/result is not used in the same way as during the initial download it seems.
-
Hi @Luke1982,
@Luke1982 said in IMAP Followup flag with BackendCombined:
I’ve adapted ChangesSink to detect changes to folders/mailboxes that have changes in the FollowUp flag on at least one of the messages. Now I was, as you suggested, looking at
GetMessageList
. Since the initial message download usesGetMessage
and sends the flag status correctly, something is going wrong there.What I suspect is that this piece isn’t working, since I can’t find any other piece of code that uses that array key. How should
GetMessageList
feed its data to the diff engine so that it picks up the flag status correctly?I took a quick look at the code and I’m not sure if that’s the right place, but the messages from GetMessageList are compared to the saved state using getDiffTo in diffstate.php. Here I don’t see $message[“star”] comparison. Try adding that.
Since right now I believe I do make
ChangesSink
detect flag changes, but still the backend doesn’t send the new flag status to the PDA (even when you force a re-download of the message by changing the ‘read’ status on a message).If I remember correctly GetMessage is called for every item which is to be synced.
It seems to be called, but the output/result is not used in the same way as during the initial download it seems.
Well, add additional logging to check if
ChangesSink
detect flag changes or not. If yes, then follow the calls to check where the message is dropped. If not, then you have to figure out how to detect those changes.Manfred
-
@Manfred Exactly, I came to that conclusion as well. A couple of things bother me:
- I’d rather not commit to diffstate since that’s a stock Z-push file and basically a mail-backend should be able to deliver the information to the diff engine in a way that it can understand. I mean: the Kopano mail backend does that so why should the IMAP backend differ.
- I wonder to what the diff engine compares. Is that the
var/lib/z-push
folder?
Let me setup a github repo to show you my work so far. I’ve setup a first commit just to show how I had the detection of flags in mind. I don’t see that state being saved in ChangesSink so I have some doubts if this will actually work properly (since like this, won’t there always be a change?)
I then added a log output after this line to print what the new state is to make sure it triggers on flag updates. Doesn’t always work but I see there’s a 30 second sleeper in there so it seems like there’s some throttling going on as well to relieve server load. On a footnote, there seems to be some caching going on since the log keeps printing log messages I’ve already disabled. Tried turning opcache off but to no avail.
EDIT
So turning opcache off did have effect, just had to resave the file. Now I can confirm that my change makes ChangesSink pick up the flag change. -
-
Hi @Luke1982,
@Luke1982 said in IMAP Followup flag with BackendCombined:
@Manfred Exactly, I came to that conclusion as well. A couple of things bother me:
- I’d rather not commit to diffstate since that’s a stock Z-push file and basically a mail-backend should be able to deliver the information to the diff engine in a way that it can understand. I mean: the Kopano mail backend does that so why should the IMAP backend differ.
Kopano backend has its own diff engine and doesn’t use BackendDiff.
- I wonder to what the diff engine compares. Is that the
var/lib/z-push
folder?
Kind of. The states of the phones are saved in this folder when using file states and these states are loaded for the comparison.
Let me setup a github repo to show you my work so far. I’ve setup a first commit just to show how I had the detection of flags in mind. I don’t see that state being saved in ChangesSink so I have some doubts if this will actually work properly (since like this, won’t there always be a change?)
The check itself doesn’t seem very efficient to me. I’d try imap_headers() and count the number of flagged messages from it. I didn’t do a thorough research, maybe there’s even better way to get that number.
@Luke1982 said in IMAP Followup flag with BackendCombined:
@Manfred So this change together with the first one fixed it. I saw that the Kopano backend doesn’t implement
GetMessageList
at all so couldn’t get an example on how to do it on the backend side, so stuck to doing it on the diffengine side.Yes, this change is fine.
Manfred
-
Hi @Manfred
I noticed Kopano does not implement the interface when I tried to research how that backend sends its changes to the diff engine: it doesn’t. At least not at the same engine as you explained
The check itself doesn’t seem very efficient to me. I’d try imap_headers() and count the number of flagged messages from it. I didn’t do a thorough research, maybe there’s even better way to get that number.
Well yes my IMAP skills suck. I’ve never done any php-imap stuff before and only briefly touched the RFC. Your colleague Michael improved my
ChangesSink
commit since I also didn’t do the flaggedcount properly of efficiëntly. As said: my IMAP skills suck and Z-Push and its architecture are still not very clear to me.I tested his change and they work, so I guess we’re good to go.