GNOME Bugzilla – Bug 657635
Linking personas from different (e-d-s) stores is not working
Last modified: 2011-09-15 16:48:58 UTC
I've tried linking a persona from the system address book and one from a Google address book and even though the linking command in folks-inspect reports it successfully linked the personas, I can't find the new individual with the linked personas. As stated by Philip on irc , this is due to the fact that all other e-d-s Persona Store which aren't primary don't have trust_level = FULL.
Created attachment 195105 [details] [review] Use iids to link via local-ids This patch set (squashed) fixes the problem for me. There were two problems preventing linking of Personas from different PersonaStores: - the initial trust-level of Edsf.PersonaStore was NONE. I've changed to PARTIAL. - we were using the contact-id as a member of the local-ids property. Hence when the individual aggregator tried to map between the local-ids and the link_map (which is a mapping from iids to Individuals), no one was found. I've tested linking personas from the system address book and a Google address book and it works nicely. The un-squashed patch set is here: http://cgit.collabora.com/git/user/rgs/folks/log/?h=bgo-657635
I'd like to know exactly what we want the LocalIdDetails interface to mean before I can understand this patch. What is a local ID? What are they used for in Tracker? EDS? As I said on IRC, I think the correct fix (or something which will go a decent way to fixing the problem) is to: • Remove the restrictions in the IndividualAggregator which only allow the writeable store to have trust_level == FULL. • Modify Edsf.PersonaStore to give trust_level = FULL to both the system address book and user-owned Google Contacts address books. That way, the personas from the system and Google Contacts address books should be aggregated together by their linkable properties, and we won't have to rely on a hacky mess of ill-defined IIDs and “local IDs”.
Comment on attachment 195105 [details] [review] Use iids to link via local-ids (Marking as needs-work until the questions about local IDs are answered.)
(In reply to comment #2) > I'd like to know exactly what we want the LocalIdDetails interface to mean > before I can understand this patch. What is a local ID? What are they used for > in Tracker? EDS? > Excerpt from folks/local-id-details.vala: /** * This interface represents the list of IDs corresponding * to {@link Persona}s from backends with write support so * that they can be linked. * * @since 0.5.0 */
Sigh - I've accidentally hit commit, so re-posting. Excerpt from folks/local-id-details.vala: /** * This interface represents the list of IDs corresponding * to {@link Persona}s from backends with write support so * that they can be linked. * * @since 0.5.0 */ So the valid use-cases for such linking property that come to mind are: - linking two e-d-s contacts from the same or different e-d-s PersonaStores (that have no im-addresses whatsoever) - same for Tracker I do agree that we've failed in clearly defining what a Local ID is, hence the inconsistency in the code that the attached patch is trying to fix. That was partially due to my poor understanding of the internals of the linking machinery. So if we agree upon the presented use-cases, I'd say that the Local IDs should be a list of Persona IIDs that should be linked together. Though by definition IIDs are only unique within a backend and using UIDs to link Personas would probably be better we are tied to this because the linking code performs lookups via IIDs and not UIDs (though I wonder of what the correctness of that). See folks/individual-aggregator.vala: if (trust_level != PersonaStoreTrust.NONE) { var candidate_ind = this._link_map.lookup (persona.iid); ..... So a couple of questions: 1) do we agree on the need to provide a mechanism to link, say, two Personas from a writeable backend (i.e.: e-d-s, from the same or different PersonaStores)? 2) whats the best property to link them with? The iid is a function of their contact id and is what the IndividualAggregator picks up for linking (as is the case for Tpf.Personas, since their iids is based on its protocol / IM id).
(In reply to comment #5) *snip* > So a couple of questions: > > 1) do we agree on the need to provide a mechanism to link, say, two Personas > from a writeable backend (i.e.: e-d-s, from the same or different > PersonaStores)? Yes. > 2) whats the best property to link them with? The iid is a function of their > contact id and is what the IndividualAggregator picks up for linking (as is the > case for Tpf.Personas, since their iids is based on its protocol / IM id). IID seems reasonable, since it works without requiring modifications to the IndividualAggregator (which is definitely good). The only advantage that using the UID instead would bring is that it would allow linking of personas (which have no other properties) between backends; and would also mean that the Tracker backend doesn't need modifying, since it currently uses UIDs as local IDs. So if we're going with IIDs, your patch seems good to commit. Additionally, we'd need the changes mentioned in comment #2, and the Tracker backend would also need to be changed to use IIDs instead of UIDs as local IDs. Finally, the documentation for LocalIdDetails should be expanded to actually say something useful.
(In reply to comment #6) (...) > > So if we're going with IIDs, your patch seems good to commit. Additionally, > we'd need the changes mentioned in comment #2, Why? At least this patch doesn't need us to relax the current assertions/restrictions in the IndividualAggregator. As to setting the default trust_level of an Edsf.PersonaStore to FULL , again, the proposed solution doesn't need such change. And I prefer we leave it to PARTIAL and just promote the primary Store to trust_level = FULL.
(In reply to comment #7) > (In reply to comment #6) > > (...) > > > > > So if we're going with IIDs, your patch seems good to commit. Additionally, > > we'd need the changes mentioned in comment #2, > > Why? At least this patch doesn't need us to relax the current > assertions/restrictions in the IndividualAggregator. But the current restrictions are outdated and clearly wrong, so should be fixed regardless (even though it's not necessary for fixing this bug). > As to setting the default trust_level of an Edsf.PersonaStore to FULL , again, > the proposed solution doesn't need such change. And I prefer we leave it to > PARTIAL and just promote the primary Store to trust_level = FULL. You don't trust the Google Contacts address book which you own and administer? It should have trust_level == FULL, even though that's not necessary for fixing this bug. This is, for example, necessary to link Google Contacts personas to Telepathy personas by their im-address properties (Edsf.Persona.im_addresses ↔ Tp.Persona.iid).
Merged the first part (to allow linking between different stores - not the part related to trust of Stores in the IndividualAggregator): commit 58bc8efd338248eb2a954c5409381ca8ea6860a4 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 22:48:56 2011 +0100 interface: throw some light on LocalIdDetail's cryptic comments commit a1a8c976ac18ad18474a684e4a48b953289ed518 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 22:24:18 2011 +0100 tracker: update linking test to use iid instead of uid commit e32d9bac78db8a84a0ce3bb83e43e26ea3eb68d3 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 23:03:10 2011 +0100 tracker: use iid instead of uid when linking via local ids commit 304e00323f57a2d0836993f5a6bf37882f60ddcf Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 19:57:07 2011 +0100 e-d-s: add test to link personas from different stores This test checks what was fixed in: https://bugzilla.gnome.org/show_bug.cgi?id=657635 commit b5750b7b1e37a924984cce3e045b6a4f385feba8 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Wed Aug 31 17:01:11 2011 +0100 e-d-s: make address book uri a settable property in the test Backend This allows us to instantiate multiple Backends, each one associated to a it's own address book, which comes in handy for tests involving multiple e-d-s PersonaStores. commit 63b4ab7ab7be30e39936f7c0fd779481536a575e Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Mon Aug 29 18:27:48 2011 +0100 e-d-s: update link-personas test to use iid instead of contact_id commit 5b44d73ebd013d649034001496ad79939efab372 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Mon Aug 29 18:26:27 2011 +0100 e-d-s: use this.local_ids in linkable_property_to_links () Otherwise, this method can have a different behaviour depending on the state of the current Persona depending on whether this.local_ids was called before or not. commit 19a426de3734cba718bc4bfa7372b2d8656ff614 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Mon Aug 29 18:25:00 2011 +0100 e-d-s: we should use the Persona's iid as local-ids If we don't and use the contact_id as before, linking Personas from different PersonaStores won't work. Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=657635 commit 563830b0894682564e4d418353614a043809a2c0 Author: Raul Gutierrez Segales <rgs@collabora.co.uk> Date: Mon Aug 29 15:47:37 2011 +0100 e-d-s: default trust level for PersonaStores is PARTIAL Without this, we can't link personas from different (e-d-s) PersonaStores. Helps: https://bugzilla.gnome.org/show_bug.cgi?id=657635
*** Bug 657981 has been marked as a duplicate of this bug. ***
(Punting bugs to 0.6.3)
What's left to do here?
I guess we could close it, though this doesn't work in some cases because of: https://bugzilla.gnome.org/show_bug.cgi?id=659075 and https://bugzilla.gnome.org/show_bug.cgi?id=657141
As long as those two bugs cover everything, closing this should be fine.