Otherwise caches will break new folder detection in Ping commands,
since everything works against singletons that live for the duration of
the command.
Details
- Reviewers
machniak - Group Reviewers
Syncroton Developers - Commits
- rS5dbe9ecc84fe: Clear caches so we can detect new folders
Diff Detail
- Repository
- rS syncroton
- Lint
Lint Skipped - Unit
No Test Coverage - Build Status
Buildable 53109 Build 18840: arc lint + arc unit
Event Timeline
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | This will make it never use cache. I don't think this should be needed if we do rcube::get_instance()->get_storage()->clear_cache('mailboxes', true); in hasHierarchyChanges(). | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I figured it's not too bad because we have the in-memory cache. But I suppose this disables the database cache? | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I've tested removing this, but it's required for the detection to work (I tested by modifying the subscription state in this case, not by creating new folders). | |
Added some tests
I haven't yet found a way to test beyond the first iteration, which is what we would require to test the in-memory caching issues.
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I'm not sure why this seems required. When testing with outlook it clearly makes a difference (maybe I'm doing something wrong while testing...), but I failed to reproduce the same behavior in a testcase so far. I tried testing it like this (couldn't really find a clean way to inject a change in an ongoing ping...): https://git.kolab.org/D5196 I'll have to re-test with Outlook. | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | This definitely makes the difference when we just change metadata with Outlook, don't know yet why. | |
This now properly tests the metadata cache issue.
I still need the magic ping value because I'm not aware of a way to change the config so it's picked up by the request,
and adding new xml properties is not really an option (we'd have to extend wbxml).
I suppose something via a header could be achieved.
This change will mean that every loop of ping will request all metadata. I think we could improve this by using QRESYNC modsequence,
but it would still require a select per folder...
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | The tests pass for me without this change. The 4th argument makes this GETMETADATA command to never write/read to/from the cache. As this is only one command, and we keep the result in-memory, maybe it's not a big deal. The cache would be used by other HTTP requests, until that call to clear_cache(), i.e. until Ping resets it again. This will get moot when we eventually merge D4686: Subscriptions engine | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I suppose for some reason rcube::get_instance()->get_storage()->clear_cache('mailboxes', true);works for you but doesn't for me. If it works the above should not be necessary after all. | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I'm using $config['imap_cache'] = 'db'; here, maybe you use redis/memcache? | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I use redis indeed, and it seems cache clearing is broken there (At least the request for the "*" folder always hits). | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | I think the problem could be that a "*" is a wildcard match in redis. | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | nope | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | The place to look at is rcube_cache::remove_record() and rcube_cache::read_record(), it's not that trivial. | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | This makes it work: https://git.kolab.org/D5211 But I don't follow this "add to remove" approach at all, so I'll leave it to you to translate it into something sensible. | |
| lib/kolab_sync_storage.php | ||
|---|---|---|
| 269 | (unless you also don't know the code, then I can look into it). | |