(Setting target bugs for Folks 0.6.1. Search for this phrase to deal with bug mail spam.)

Didn't make this release; punting to 0.6.2.

I can't implement linking or unlinking in gnome contacts without this.

I just pushed link support in gnome contacts. It currently requires you to restart contacts to see the result of the operation due to this bug, but it will let you test any patches for this bug.

Created attachment 195828 [details] [review]
Add support for re-linking on linkable properties being changed

https://www.gitorious.org/folks/folks/commits/656689-watch-linkable-properties

This works for me, though doubtless there will be corner cases that other people find. I think the approach is reasonable, and it fixes a few other bugs on the way — notably the problem Alex was having in https://bugzilla.gnome.org/show_bug.cgi?id=657282#c10.

Somehow, as luck would have it, the tests pass with this branch…apart from the tests which are broken by bug# 658411. (The tests assume a specific sequence of individuals_changed_detailed signal emissions, which is broken by excess emissions caused by extraneous property change notifications of linkable properties.)

Why does folks always start doing re-linking immediately on a change event (be it an added persona now, or a changed persona with the patch. Its almost always the case that several changes will happen in a row, and batching these up so that new computation is done only in and idle and any inbetween personas are never seen in public.

Also, such idles help with protection against reentry of signals.
For instance, if i set a persona property like this

 var ind = get_individual();
 var persona = ind.personas.to_array()[0];
 persona.nickname = foo;
 // Here we might get an individuals_changed signal that makes ind point
 // to some old uninteresting dead gobject. It may also call into your 
 // signal handlers that modify your state
 use_individual (ind);

I've been testing this and it looks solid, I am half-way through with bug #658411 though.

Review of attachment 195828 [details] [review]:

This seems to work for me (at least, not obviously break tests) and Raul approved the code itself (I haven't read it), so let's merge this.

Merged:

commit cd1fd7f6cdd9ba89b248eb6096d43f9d8a660af0
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 22:22:52 2011 +0100

    Bug 656689 — Re-link personas on linkable properties being changed
    
    Listen for notifications of changes to the linkable properties of every
    persona in the IndividualAggregator. When we receive such a notification,
    relink that persona by artificially removing them from the aggregator and
    immediately re-adding them.
    
    Like many things in the IndividualAggregator, this is ugly — but it appears
    to work, and I can get my head around it.
    
    Closes: bgo#656689

commit 7b74199aeed817fb4b08ee45e427c161fa7d9088
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 22:20:12 2011 +0100

    core: Fix individuals_changed_detailed mappings for unlinked individuals
    
    It turns out that we can do a post-processing phase on the mappings to be
    emitted by IndividualAggregator.individuals_changed_detailed and ensure that
    individuals who've been unlinked are represented by a mapping from themselves
    to the set of individuals replacing them, rather than a mapping from
    themselves to null, and null to the set of individuals replacing them.
    
    This probably isn't the most efficient way to fix the problem, but it works
    and I can just about understand it.
    
    Helps: bgo#656689, bgo#657282

commit 55abd8380a639d7e475483fcd0121d7b6552d51c
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 22:16:44 2011 +0100

    core: Be more thorough when removing individuals from the link map
    
    Since the values of personas' linkable properties could've changed since we
    added them to the link map, we have to be more thorough when removing
    individuals from the map. We therefore now remove them by examining all the
    values in the map, and removing every mapping to the individual we're
    interested in.
    
    Helps: bgo#656689

commit fa1fb9f4ab151268fd2eee16a1afe7c2f87e5f42
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 22:10:20 2011 +0100

    core: Add more debug output and validation to the IndividualAggregator
    
    Everyone loves debug output, and validating that we aren't leaving stale
    entries in the link map which will later cause whoever's looking into
    aggregator bugs to hide in a corner and cry.
    
    Helps: bgo#656689

commit 6ed7d51617cc9b2aa141497d7437c8b7247cbf40
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 22:04:20 2011 +0100

    core: Clarify that Persona.linkable_properties doesn't change at runtime
    
    Helps: bgo#656689

commit 150b7579f1069f561f2069c9efad864b9c87e2e9
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Sep 6 21:57:42 2011 +0100

    core: Ensure we always notify of new Individuals
    
    In the following situation, it was possible for
    IndividualAggregator.individuals_changed_detailed to not emit a notification
    that an individual was added:
    
    If two personas were added by a given store in the same emission of
    PersonaStore.personas_changed, the IA would create an Individual, i1, from
    the first and add a mapping (null → i1) to the change set. It would then
    process the second, destroying the first individual and creating a new
    Individual, i2, (correctly) containing both personas. In doing so, it would
    remove the mapping (null → i1) from the change set, but would incorrectly
    not add a mapping (null → i2) in its place.
    
    This situation can be extended to others where a single new Individual is
    formed from multiple new personas coming from a single emission of
    PersonaStore.personas_changed.
    
    Closes: bgo#657282 (again)