[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              | Assigned
  Priority           | 2. Medium
  Milestone          |
  Patch              | 1
  Owners             | Michael Rubinsky
------------------------------------------------------------------------------


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

> Turns out $this->_collections was more or less ok.
>
> I just did the following:
> - Re-create the account on the PIM
> - Subscribed (=ping) a folder I didn't sync in yet.

If the client is issuing a PING for a collection that has not yet been  
SYNCed, then this is a client bug/issue. You can't PING a collection  
until it's been SYNCed.

>    The code path that handles a "<Ping>" with a folder list
>    first adds the folder via addCollection() so we have a sync key for sure.
>
>    Later in that code path we call updatePingableFlag().
>    In fact that's the only place we call updatePingableFlag() at all.
>
>    -> the newly subscribed folder was flagged as NOT PINGABLE.

DId the folder previously have a SYNC performed on it, before the PING?

> Since collections::addCollection() does not update the collection in  
> the SyncCache,
> updatePingable() flag relied on the side effect of using
> the $this->_collections array directly while working on the cache. Nasty.

It's not a nasty side effect, it's the intended behavior - see my  
earlier reply.

> There's more: If a collection gets reset because the sync key does  
> not match anymore
> and the PIM sends a <Ping> without a folder list, we will not set  
> the PINGABLE flag again.

If the synckey doesn't match, what *should* happen is that an  
appropriate error code is sent back to the client indicating this  
(don't have the codes handy at the moment to know exactly which one).  
There is a specific code for telling the client that we can't accept  
an empty PING request. Once the client gets this code it should NOT  
issue another empty PING request, but (depending on the code we send)  
either a full SYNC, a FOLDERSYNC, or a full PING.

> -> IMHO we should run collections->updatePingable() on every PING request
> right before the "// Start waiting for changes" line. Also updatePingable()
> should modify the cache _and_ the current object.

I'll have to look at this...