GNOME Bugzilla – Bug 741643
Add PersonaStore synchronisation
Last modified: 2015-01-20 10:54:50 UTC
Patch coming with an implementation of synchronisation of specific PersonaStores with EDS address books, done by Rodrigo Moya and Xavier Claessens. I am not yet sure whether this should be committed, but it seems better to attach the patch here and discuss it, rather than let it languish on someone’s HDD forever. Pros: • Seems like an oft-requested feature (though to what end, I am not sure). • Simple implementation. Cons: • Somewhat against the entire architecture of folks as an aggregation agent. • Need to understand the underlying need for synchronisation (e.g. if it’s a performance or offline-access problem, those can be solved by caching within specific backends).
Created attachment 292893 [details] [review] FolksPersonaStore: add sync mode
Branch here: http://cgit.collabora.com/git/user/pwith/folks.git/log/?h=sync-mode
Xavier, Rodrigo, what’s the motivation and use-case for this API?
Review of attachment 292893 [details] [review]: Here’s a review, but only for posterity. I’m going to close this as WONTFIX, but please re-open it if you have some good justifications. :-) Basically: • This goes against the core idea of folks as an aggregator. • It has race conditions if used in multiple processes: if two processes have a store set to SYNCHRONIZE_ONLY, they will race to copy the personas to EDS. - This implementation is open-loop, with no feedback for implementing duplicate elimination or removal of personas deleted in EDS. - Because of this, I think the only reasonable approach for synchronisation is to have a single daemon process doing it. That doesn’t belong in libfolks, and already exists in the form of syncEvolution (https://syncevolution.org/) which tackles all the usual problems of synchronisation which are not tackled here. • It’s hard-coded to synchronising with EDS. ::: folks/backend.vala @@ +227,3 @@ + { + GLib.warning ("Not implemented"); + } These APIs should be split out into a separate patch. Instead of warning() on unimplemented APIs, they should throw an error about being unsupported. ::: folks/persona-store-sync.vala @@ +57,3 @@ + } + + private void _add_personas (Set<Persona> added) Needs to be async. @@ +147,3 @@ + v11.set_boolean (o10.is_favourite); + details.insert (Folks.PersonaStore.detail_key (PersonaDetail.IS_FAVOURITE), (owned) v11); + } We really need to factor this lot out and eliminate the need for repeating GValue code everywhere in folks. :-( @@ +153,3 @@ + } + + private void _remove_personas (Set<Persona> removed) Needs to be async. ::: folks/persona-store.vala @@ +522,3 @@ + this._sync_mode = value; + this.notify_property ("sync-mode"); + } This property is not saved or restored anywhere, so: • does not persist between uses of libfolks in the same process, and • can differ between uses of libfolks in different processes. Both of these are bad.