[Tickets #12605] Re: AS: Fix loss of PINGABLE flag

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12605
------------------------------------------------------------------------------
  Ticket             | 12605
  Updated By         | Michael Rubinsky <mrubinsk at horde.org>
  Summary            | AS: Fix loss of PINGABLE flag
  Queue              | Synchronization
  Version            | Git master
  Type               | Bug
  State              | Feedback
  Priority           | 2. Medium
  Milestone          |
  Patch              | 1
  Owners             | Michael Rubinsky
------------------------------------------------------------------------------


Michael Rubinsky <mrubinsk at horde.org> (2013-08-23 16:12) wrote:

> We don't have to ping the folder if we don't have a synckey yet
> (as you mentioned we can't send changes anyway), we just skip it  
> until we have a synckey.
>
> We could modify the "<Ping>" handling to mark all wanted folders
> as pingable unconditionally and remove the "pingable" flag
> from any folder that is not in the list.
>
> If the PIM sends "<Ping>" without a folder list, we check
> which pingable folders have a sync key and skip those without one.
> I don't think the new strategy costs any extra performance than the  
> current one.

My concern is, that since this is an undefined condition, if we don't  
send *something* back to the client about the unsync'd collection then  
it will not issue a full sync on it. I guess only testing will answer  
this, but I really hate working around broken client behavior with  
server responses that are essentially undefined for the context.


> The broken client in question is iOS 4, I'll have to retest how iOS  
> 6 behaves.
> Though it's not that uncommon to configure an account, may be open the
> INBOX to see if it's initially working and then go back to the settings menu
> and select a bunch of folders than should be pushed through.

...and that's perfectly fine. What the client should do in this case  
is issue a SYNC request (which can even occur while any previous PING  
is still running) for the newly pushable folders. This will invalidate  
the cache that the PING command uses, so the PING will eventually  
terminate with the "Detected changes by another process" log notice,  
and when the client issues the next PING, it should be a full request  
that includes the newly pingable folders.


>
> Looking through the code how updatePingable() works, I think
> it sets the 'pingable' flag for any folder in the collection with a  
> 'synckey' field?!

Only with your patch :)

The existing behavior is to only flag the collections that were  
included in the current PING requests FOLDERS element ($_collections),  
not all known collections ($collections in the foreach loop).

> Isn't that NOT the desired behavior?
>
> That would explain why I see getServerChanges() calls
> for any folder I've accessed on the PIM, even though
> I just set the push mode for three of them...
>