GNOME Bugzilla – Bug 640092
Folks should minimize use of GLib.List (including its public API)
Last modified: 2011-04-23 21:06:54 UTC
Vala's trouble handling memory managing GLib.List has bit us a countless number of times (eg, it's likely behind the issue stated in bug #639976), so we're probably best off removing it from the library as much as possible. We've already replaced some internal uses of GLib.List, but it's common in our public API. We should replace these instances if feasible (breaking API as necessary).
Eitan, could you look at the existing public API and come up with a list of places where we could replace GLib.List with LinkedHashSet (and propose a minimal number of new data types where it isn't sufficient)? This would make the API a little more awkward in C, which is my only significant concern. Off the top of my head, I think all or nearly all cases where we have a GLib.List in the public API we could use a LinkedHashSet (since the contents should be unique anyhow). There's some relevant discussion in bug #629078 and bug #639976
(In reply to comment #1) > Eitan, could you look at the existing public API and come up with a list of > places where we could replace GLib.List with LinkedHashSet (and propose a > minimal number of new data types where it isn't sufficient)? > > This would make the API a little more awkward in C, which is my only > significant concern. > > Off the top of my head, I think all or nearly all cases where we have a > GLib.List in the public API we could use a LinkedHashSet (since the contents > should be unique anyhow). > > There's some relevant discussion in bug #629078 and bug #639976 I'll take this on.
Here's the vapi which contains GLib.List and my suggested replacements. Note that it doesn't occur in Tpf. The main reason I've suggested GLib.HashTable in a few places is because the members aren't inherently ordered. We could use LinkedHashSet in these places as well, if only for the advantage that it implements Iterator (so we can use foreach() with it in Vala). Working with a LinkedHashSet or GHashTable from C seems like similar complexity (except that more people are familiar with GHashTable than LinkedHashSet or the Gee classes it implements). This would obviously pull in libgee as a requirement for clients, though we're already doing that with the current implementation of LinkedHashSet. ========================================================== public class BackendStore : GLib.Object { // TODO: make it a GLib.HashTable public GLib.List<Folks.Backend> enabled_backends { owned get; private set; } } public class Individual : GLib.Object, Folks.Aliasable, Folks.Favouritable, Folks.Groupable, Folks.HasAvatar, Folks.HasPresence, Folks.IMable { // TODO: LinkedHashSet public Individual (GLib.List<Folks.Persona>? personas); // TODO: LinkedHashSet public GLib.List<Folks.Persona> personas { get; set; } // TODO: GLib.HashTables public signal void personas_changed (GLib.List<Folks.Persona>? added, GLib.List<Folks.Persona>? removed); } public class IndividualAggregator : GLib.Object { // TODO: GLib.HashTables public signal void individuals_changed (GLib.List<Folks.Individual>? added, GLib.List<Folks.Individual>? removed, string? message, Folks.Persona? actor, Folks.Groupable.ChangeReason reason); } public abstract class PersonaStore : GLib.Object { // TODO: GLib.HashTables public signal void personas_changed (GLib.List<Folks.Persona>? added, GLib.List<Folks.Persona>? removed, string? message, Folks.Persona? actor, Folks.Groupable.ChangeReason reason); }
(In reply to comment #3) > The main reason I've suggested GLib.HashTable in a few places is because the > members aren't inherently ordered. We could use LinkedHashSet in these places > as well, if only for the advantage that it implements Iterator (so we can use > foreach() with it in Vala). I'm not sure whether the convenience of being able to use foreach{} over a GHashTableIter is worth the increase in memory usage of LinkedHashSet vs. HashTable. That said, if we're exposing libgee in the public API, it would be neater (in Vala at least; I don't know about C) and more consistent to use Vala types (HashMap and HashSet) instead. The only reason I can think of against using HashSet instead of LinkedHashSet (when the elements are unordered) is because (as I understand from a quick reading of the code) getting the next iteration when traversing a HashSet isn't O(1) (though it looks like traversal overall is O(n) in amortised time). The same is true for HashTable. > public class Individual : GLib.Object, Folks.Aliasable, > Folks.Favouritable, Folks.Groupable, Folks.HasAvatar, Folks.HasPresence, > Folks.IMable { > // TODO: LinkedHashSet > public Individual (GLib.List<Folks.Persona>? personas); > // TODO: LinkedHashSet > public GLib.List<Folks.Persona> personas { get; set; } For the above reason, I would suggest HashSets for these, as they're inherently unordered (as we discussed last year: a natural order over an individual's personas for one property is most likely not the same as the order over the personas for a different property). > // TODO: GLib.HashTables > public signal void personas_changed (GLib.List<Folks.Persona>? > added, GLib.List<Folks.Persona>? removed); > } Again, HashSets. (I'm presuming you're suggesting using the HashTable as a mapping from Persona → nothing?) > public class IndividualAggregator : GLib.Object { > // TODO: GLib.HashTables > public signal void individuals_changed > (GLib.List<Folks.Individual>? added, GLib.List<Folks.Individual>? removed, > string? message, Folks.Persona? actor, Folks.Groupable.ChangeReason reason); > } Same here. > public abstract class PersonaStore : GLib.Object { > // TODO: GLib.HashTables > public signal void personas_changed (GLib.List<Folks.Persona>? > added, GLib.List<Folks.Persona>? removed, string? message, Folks.Persona? > actor, Folks.Groupable.ChangeReason reason); > } Same here.
Add some more goal bugs for the 0.3.5 release.
(Actually setting the new milestone for these bugs)
I forgot to pre-emptively re-type the GLib.List portions of the new interfaces in bug #638279 before I merged them to master, so those will also need to be cleaned up. (At least they haven't already been released).
We should also add a "Do not use GLib.List anywhere." section to HACKING along with this branch.
And we should also factor in bug #629078 comment #23.
Philip, think you could check out the test failure in this branch (warning, there are a ton of added GLib.message calls, commented code, etc.)?: http://git.collabora.co.uk/?p=user/treitter/folks.git;a=shortlog;h=refs/heads/WIP-bgo640092-minimize-glib-list I can't figure out why it's happening, but we hit this with consecutive tests in aggregation.vala: ================================== telepathy-Message: Get Channels property failed: User requested disconnection telepathy-WARNING **: Failed to determine whether we can set aliases on Telepathy account 'Fake Account': User requested disconnection aborting... ================================== One other symptom in the failing cases is that the connection is ready as soon as we hit Tpf.PersonaStore._account_status_changed_cb() with new_status == CONNECTED (when the tests succeed, the connection is not yet ready for one or two iterations of that callback where it is CONNECTED). ================================== The first test case within Aggregation that runs succeeds (so if you comment out the first test, the second one will succeed; otherwise it will fail as above). The unmodified version of test_iid() succeeds without causing problems for subsequent cases; something about successfully linking contacts together (as I do in the modified version) causes the test failure above.
This looks just like the problems you're having here: https://bugzilla.gnome.org/show_bug.cgi?id=629537#c16 Unfortunately, I can't reproduce these test failures either. All the tests pass for me on WIP-bgo640092-minimize-glib-list. :-( How are you running the tests? I'm just running `make check` inside a JHBuild shell.
This is with: • Vala 0.11.5 • tp-glib (master, 58bf150c70bee50ca69c7b7bed632aed66e72eae, Feb 1) • libgee 0.6 release • dbus-glib 0.82 • GLib (master, 1e5916ffae7bfaf041df454677562aec4557e21c, Jan 23) And I get the same results after upgrading GLib to the latest version of master (9823ff1d203166f33302dce2a26e1dee86c4d569) and rebuilding all of these packages in jhbuild. And I tried again after rebuilding everything from Folks down via: jhbuild build -a -c -f folks with the same results. I'm running 'make check' via: jhbuild buildone -n -f --check folks I'll try again with a fresh jhbuild prefix, but I doubt that will matter.
As with bug #629537 comment 23, I've bisected my WIP branch (after squashing a few indivisible commits), and the change that causes the "User requested disconnection" problem with the tests is the one that changes the type of candidate_inds (as well as types within Individual).
Punting to 0.3.7.
Created attachment 184245 [details] [review] Squashed diff of the 640092-die-glib-list-die branch http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/640092-die-glib-list-die Here's the work I did on this yesterday (which I couldn't attach before due to Bugzilla being down). All the API changes should be listed in NEWS. With this branch applied, the only real uses of GLib.List left in folks.vapi are in presenting lists of FieldDetails instances, which I think is correct. This branch depends on my branches for bug #645684 and bug #645685.
Review of attachment 184245 [details] [review]: These generally seem to go in the right direction, but please update for the comments here: bug 645685 comment 7 We also need to rebase upon master and update the new backends and interfaces to use the appropriate types. Please make sure things are consistent in the cases where removing LinkedHashSet won't force us to update all the types to fix the build. (eg, ImDetails, WebServiceDetails, LocalIdDetails should all use the same type for their main member.)
(In reply to comment #16) > Review of attachment 184245 [details] [review]: > > These generally seem to go in the right direction, but please update for the > comments here: bug 645685 comment 7 > > We also need to rebase upon master and update the new backends and interfaces > to use the appropriate types. Please make sure things are consistent in the > cases where removing LinkedHashSet won't force us to update all the types to > fix the build. (eg, ImDetails, WebServiceDetails, LocalIdDetails should all use > the same type for their main member.) While you're on this cleaning rampage, you might as well replace all instances of HashTable with HashMap and instances of HashTable and HashMap with HashMultiMap (if appropriate - I'm not sure we have any cases where it makes sense, but it may save us from some compound types).
Created attachment 186496 [details] [review] Squashed diff of the 640092-die-glib-list-die branch (updated) http://cgit.collabora.co.uk/git/user/pwith/folks/log/?h=640092-die-glib-list-die Here it is. This collects together the fixes for bug #640092 and the stuff discussed in comment #7 on bug #645685, plus some other related API changes which occurred to me as I was working on it (such as changing Gee.Hash[Map|Set] to the more generic Gee.[Map|Set] on APIs like LocalIdDetails). All the test cases pass and folks-inspect seems to be happy. I haven't yet ported Empathy to use the updated API since I wanted to get some feedback first. I haven't made the changes mentioned in comment #1 on bug #644330 since the reasoning behind them isn't clear to me. We should discuss that and either get it committed with this load of changes, or throw it out.
Created attachment 186498 [details] folks.vapi as generated by the branch Here's the folks.vapi generated with the changes applied. This needs to be reviewed carefully, especially for consistency between interfaces like ImDetails, PhoneDetails, UrlDetails, etc.
*** Bug 639933 has been marked as a duplicate of this bug. ***
Review of attachment 186496 [details] [review]: Very nice in general - thanks for the thankless work involved. I've got some suggested fixes below (note that a few points are repeated throughout the commits). After these minor fixes, please commit (so we don't leave this hanging for too long). We obviously won't want to do a release until we've written the patches to update Empathy and QtFolks (which I'll do), to make sure all the changes are as reasonable as they look. ============================================================ Update the "Helps" and "Closes" lines in the commit messages (the newer commits don't include them) It looks like folks-inspect needs to be updated for changes to local-ids, notes, and roles: local-ids Can't convert from type 'GeeSet' to 'gchararray' notes Can't convert from type 'GeeSet' to 'gchararray' roles Can't convert from type 'GeeSet' to 'gchararray' backends/key-file/kf-persona.vala ================================= + HashMultiMap<string, string> im_addresses = + new HashMultiMap<string, string> (str_hash, str_equal, + str_hash, str_equal); I'm fairly sure that all the Gee types are smart about auto-setting the hash and equal functions based on the generic types. It's only GLib.HashTable where it's necessary. You repeated this pattern throughout this commit and others as well. See if the test suite passes if you cut out all the explicit hash and equal functions in this first commit. If so, please follow through on all the other commits as well. + var normalized_addresses = new HashSet<string> (); Might as well change this to en_GB while we're mangling the history. backends/telepathy/lib/tpf-persona.vala ======================================= + * A mapping of IM protocol to an unordered set of IM addresses. "unordered set" is redundant. You repeat this throughout this commit and other commits, so please update those as well. backends/tracker/lib/trf-persona-store.vala =========================================== /* FIXME: * - this conversion should go away once we've switched to use the * same data structure for each property that is a list of something. * See: https://bugzilla.gnome.org/show_bug.cgi?id=646079 */ Is this FIXME still relevant (especially in light of _set_attrib_set())? It seems like we should just cut it. + var ims = new HashSet <FieldDetails> (); Extra space between HashSet and <FieldDetails> backends/key-file/kf-backend.vala ================================= + public override Map<string, PersonaStore> persona_stores Good idea exposing this as the general Map instead HashMap. folks/individual.vala ===================== + * the Individual. The order of the personas in the two sets is undefined. Maybe clarify as "As the parameters are (unordered) sets, the orders of their elements are undefined"
(In reply to comment #19) > Created an attachment (id=186498) [details] > folks.vapi as generated by the branch > > Here's the folks.vapi generated with the changes applied. This needs to be > reviewed carefully, especially for consistency between interfaces like > ImDetails, PhoneDetails, UrlDetails, etc. Looks good to me.
(In reply to comment #21) > Review of attachment 186496 [details] [review]: > > Very nice in general - thanks for the thankless work involved. I've got some > suggested fixes below (note that a few points are repeated throughout the > commits). > > After these minor fixes, please commit (so we don't leave this hanging for too > long). We obviously won't want to do a release until we've written the patches > to update Empathy and QtFolks (which I'll do), to make sure all the changes are > as reasonable as they look. > > ============================================================ > > Update the "Helps" and "Closes" lines in the commit messages (the newer commits > don't include them) Done. > It looks like folks-inspect needs to be updated for changes to local-ids, > notes, and roles: > > local-ids Can't convert from type 'GeeSet' to 'gchararray' > notes Can't convert from type 'GeeSet' to 'gchararray' > roles Can't convert from type 'GeeSet' to 'gchararray' Done. > backends/key-file/kf-persona.vala > ================================= > + HashMultiMap<string, string> im_addresses = > + new HashMultiMap<string, string> (str_hash, str_equal, > + str_hash, str_equal); > > I'm fairly sure that all the Gee types are smart about auto-setting the hash > and equal functions based on the generic types. It's only GLib.HashTable where > it's necessary. You repeated this pattern throughout this commit and others as > well. See if the test suite passes if you cut out all the explicit hash and > equal functions in this first commit. If so, please follow through on all the > other commits as well. Good point. Done. > + var normalized_addresses = new HashSet<string> (); > > Might as well change this to en_GB while we're mangling the history. Done. :-D > backends/telepathy/lib/tpf-persona.vala > ======================================= > + * A mapping of IM protocol to an unordered set of IM addresses. > > "unordered set" is redundant. You repeat this throughout this commit and other > commits, so please update those as well. I wanted to make it explicit that folks doesn't impose an order over things like this (even though sets are implicitly unordered anyway). I've changed it to “(unordered) set”. > backends/tracker/lib/trf-persona-store.vala > =========================================== > > /* FIXME: > * - this conversion should go away once we've switched to use the > * same data structure for each property that is a list of something. > * See: https://bugzilla.gnome.org/show_bug.cgi?id=646079 */ > > Is this FIXME still relevant (especially in light of _set_attrib_set())? It > seems like we should just cut it. I think so, since ImDetails.im_addresses doesn't have type Set<FieldDetails>. I guess this can be handled in bug #646079 if necessary. > + var ims = new HashSet <FieldDetails> (); > > Extra space between HashSet and <FieldDetails> Fixed. > folks/individual.vala > ===================== > + * the Individual. The order of the personas in the two sets is undefined. > > Maybe clarify as "As the parameters are (unordered) sets, the orders of their > elements are undefined" Done.
Comment on attachment 186496 [details] [review] Squashed diff of the 640092-die-glib-list-die branch (updated) Updated and merged to master! commit 42992e45d4441de8378f2c15ae6de157ff9b0ab2 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 11:22:18 2011 +0100 Change PostalAddress.types to be a Set<string> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 4 +- backends/tracker/lib/trf-persona.vala | 4 +- folks/postal-address-details.vala | 25 +++++++++++-------- tests/tracker/add-persona.vala | 4 +- .../tracker/postal-address-details-interface.vala | 2 +- tests/tracker/set-postal-addresses.vala | 4 +- 7 files changed, 24 insertions(+), 20 deletions(-) commit df92a32fc1c0597df616c0bbe57c7d07772ecf93 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:41:57 2011 +0100 Change PotentialMatch.known_email_aliases to be of type Set<string> Helps: bgo#640092 NEWS | 1 + folks/potential-match.vala | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletions(-) commit a1aa6294a48a6cec8b92edd7f7cc73f6047c39cc Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:41:20 2011 +0100 Change RoleDetails.roles to be of type Set<Role> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 2 +- backends/tracker/lib/trf-persona.vala | 2 +- folks/individual.vala | 7 +++++-- folks/role-details.vala | 4 ++-- tests/tracker/add-persona.vala | 4 ++-- tools/inspect/utils.vala | 20 ++++++++++++++++++++ 7 files changed, 32 insertions(+), 8 deletions(-) commit 99d591f3b4888c6ad4f5ca3f89bc187f351fba72 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:40:50 2011 +0100 Change NoteDetails.notes to be of type Set<Note> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 2 +- backends/tracker/lib/trf-persona.vala | 2 +- folks/individual.vala | 7 +++++-- folks/note-details.vala | 4 ++-- tests/tracker/add-persona.vala | 4 ++-- tools/inspect/utils.vala | 20 ++++++++++++++++++++ 7 files changed, 32 insertions(+), 8 deletions(-) commit 99af9effc77f18bb83c369d48b16513a92cff920 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:40:10 2011 +0100 Change LocalIdDetails.local_ids to be of type Set<string> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 12 ++++++------ backends/tracker/lib/trf-persona.vala | 6 ++++-- folks/individual-aggregator.vala | 2 +- folks/individual.vala | 7 +++++-- folks/local-id-details.vala | 4 ++-- tools/inspect/utils.vala | 3 ++- 7 files changed, 21 insertions(+), 14 deletions(-) commit 37a01ab4fdc9e0ac85b850824212a1ed44e8d27e Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:31:54 2011 +0100 Change PersonaStore.personas to be a Map<string, Persona> This helps in our quest to get rid of HashTable. Helps: bgo#640092 NEWS | 1 + backends/key-file/kf-persona-store.vala | 20 +++++++++----------- backends/libsocialweb/lib/swf-persona-store.vala | 14 +++++++------- backends/telepathy/lib/tpf-persona-store.vala | 11 +++++------ backends/tracker/lib/trf-persona-store.vala | 20 ++++++++++---------- folks/persona-store.vala | 4 +++- tests/tracker/remove-persona.vala | 4 ++-- tools/inspect/utils.vala | 14 +++++++------- 8 files changed, 44 insertions(+), 44 deletions(-) commit 03bfb40e46b9c804700c57e0a6a6faf6aa503cda Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:14:05 2011 +0100 Change IndividualAggregator.individuals to be a Map<string, Individual> This helps in our quest to get rid of HashTable. Helps: bgo#640092 NEWS | 1 + folks/individual-aggregator.vala | 32 +++++++++++++++--------------- tests/tracker/add-contact.vala | 5 +-- tests/tracker/avatar-updates.vala | 2 +- tests/tracker/emails-updates.vala | 2 +- tests/tracker/match-all.vala | 2 +- tests/tracker/match-email-addresses.vala | 4 +- tests/tracker/match-im-addresses.vala | 4 +- tests/tracker/match-known-emails.vala | 4 +- tests/tracker/match-name.vala | 4 +- tests/tracker/match-phone-number.vala | 4 +- tools/inspect/command-individuals.vala | 10 ++++---- tools/inspect/command-personas.vala | 6 +--- tools/inspect/utils.vala | 20 +++++++++--------- 14 files changed, 49 insertions(+), 51 deletions(-) commit 290c04b913002f5630b3ce3a72620f5b9964cae8 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 01:02:54 2011 +0100 Change IndividualAggregator.get_[all_]potential_matches() to return a Map We don't want to expose the fact that we're using a *hash* map internally. Helps: bgo#640092 NEWS | 4 ++++ folks/individual-aggregator.vala | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) commit 2d66369e4196c71aa390405ee82c67dd4af980ca Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Apr 20 00:58:00 2011 +0100 Change IndividualAggregator.link_personas() to take a Set<Persona> Helps: bgo#640092 NEWS | 1 + folks/individual-aggregator.vala | 12 ++++-------- tests/libsocialweb/aggregation.vala | 8 ++++---- tests/tracker/link-personas-via-local-ids.vala | 10 +++++----- tests/tracker/link-personas.vala | 10 +++++----- 5 files changed, 19 insertions(+), 22 deletions(-) commit d27602ca291598b2de3b69b3b32c35784423d244 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 16:40:07 2011 +0000 Port IndividualAggregator.individuals_changed to use Set<Individual> Helps: bgo#640092 NEWS | 2 + folks/individual-aggregator.vala | 30 ++++++++++++-------- tests/key-file/individual-retrieval.vala | 11 ++++-- tests/libsocialweb/aggregation.vala | 18 +++++++---- tests/libsocialweb/dummy-lsw.vala | 12 ++++---- tests/telepathy/individual-properties.vala | 14 ++++---- tests/telepathy/individual-retrieval.vala | 4 +- tests/tracker/add-contact.vala | 8 ++-- tests/tracker/add-persona.vala | 8 ++-- tests/tracker/additional-names-updates.vala | 8 ++-- tests/tracker/avatar-details-interface.vala | 6 ++-- tests/tracker/avatar-updates.vala | 8 ++-- tests/tracker/birthday-details-interface.vala | 8 ++-- tests/tracker/birthday-updates.vala | 8 ++-- tests/tracker/default-contact.vala | 8 ++-- tests/tracker/duplicated-emails.vala | 8 ++-- tests/tracker/duplicated-phones.vala | 8 ++-- tests/tracker/email-details-interface.vala | 8 ++-- tests/tracker/emails-updates.vala | 8 ++-- tests/tracker/family-name-updates.vala | 9 +++-- tests/tracker/favourite-details-interface.vala | 8 ++-- tests/tracker/favourite-updates.vala | 8 ++-- tests/tracker/fullname-updates.vala | 8 ++-- tests/tracker/gender-details-interface.vala | 8 ++-- tests/tracker/given-name-updates.vala | 8 ++-- tests/tracker/im-details-interface.vala | 8 ++-- tests/tracker/imaddresses-updates.vala | 8 ++-- tests/tracker/individual-retrieval.vala | 8 ++-- tests/tracker/link-personas-via-local-ids.vala | 27 +++++++++++------ tests/tracker/link-personas.vala | 27 +++++++++++------ tests/tracker/match-all.vala | 8 ++-- tests/tracker/match-email-addresses.vala | 8 ++-- tests/tracker/match-im-addresses.vala | 8 ++-- tests/tracker/match-known-emails.vala | 8 ++-- tests/tracker/match-name.vala | 8 ++-- tests/tracker/match-phone-number.vala | 8 ++-- tests/tracker/name-details-interface.vala | 8 ++-- tests/tracker/nickname-updates.vala | 8 ++-- tests/tracker/note-details-interface.vala | 8 ++-- tests/tracker/phone-details-interface.vala | 8 ++-- tests/tracker/phones-updates.vala | 8 ++-- .../tracker/postal-address-details-interface.vala | 8 ++-- tests/tracker/prefix-name-updates.vala | 8 ++-- tests/tracker/remove-contact.vala | 8 ++-- tests/tracker/remove-persona.vala | 8 ++-- tests/tracker/role-details-interface.vala | 8 ++-- tests/tracker/set-alias.vala | 8 ++-- tests/tracker/set-avatar.vala | 8 ++-- tests/tracker/set-birthday.vala | 8 ++-- tests/tracker/set-duplicate-email.vala | 8 ++-- tests/tracker/set-emails.vala | 8 ++-- tests/tracker/set-favourite.vala | 8 ++-- tests/tracker/set-full-name.vala | 8 ++-- tests/tracker/set-gender.vala | 8 ++-- tests/tracker/set-im-addresses.vala | 8 ++-- tests/tracker/set-notes.vala | 8 ++-- tests/tracker/set-phones.vala | 8 ++-- tests/tracker/set-postal-addresses.vala | 8 ++-- tests/tracker/set-roles.vala | 8 ++-- tests/tracker/set-structured-name.vala | 8 ++-- tests/tracker/set-urls.vala | 8 ++-- tests/tracker/suffix-name-updates.vala | 8 ++-- tests/tracker/url-details-interface.vala | 8 ++-- tests/tracker/website-updates.vala | 8 ++-- 64 files changed, 307 insertions(+), 277 deletions(-) commit 853d7721b144153a3c1496c012b24e226999f440 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 16:11:09 2011 +0000 Fix a race condition in Tpf.PersonaStore The /IndividualRetrieval/aggregator:add test can execute multiple calls to Tpf.PersonaStore.add_persona_from_details() with the same contact ID simultaneously. Since the change to use sets, the fact that only one of the adds is succeeding is no longer hidden by our use of GLib.List. This commit fixes the error handling in Tpf.PersonaStore.add_persona_from_details() to be more resilient to the contact already existing. Helps: bgo#640092 backends/telepathy/lib/tpf-persona-store.vala | 47 ++++++++++++------------- 1 files changed, 23 insertions(+), 24 deletions(-) commit 2d2e1e1096b562c40c1d02a0ecc373287e51b83e Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 16:09:21 2011 +0000 Remove unused code from Tpf.PersonaStore backends/telepathy/lib/tpf-persona-store.vala | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-) commit dd378b72c4bc63567b0ed74d4b319008fbb1c6be Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 16:07:55 2011 +0000 Port PersonaStore.personas_changed to Set<Persona> Helps: bgo#640092 NEWS | 1 + backends/key-file/kf-persona-store.vala | 24 +++++--- backends/libsocialweb/lib/swf-persona-store.vala | 29 ++++++--- backends/telepathy/lib/tpf-persona-store.vala | 72 ++++++++++++++-------- backends/tracker/lib/trf-persona-store.vala | 42 ++++++++----- folks/individual-aggregator.vala | 23 +++---- folks/individual.vala | 10 +-- folks/persona-store.vala | 11 ++- 8 files changed, 126 insertions(+), 86 deletions(-) commit 4bb93d34cc916f98d6b0c8fa66e4ef5188ad551e Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 15:12:00 2011 +0000 Port Individual.personas_changed to use Set<Persona> Helps: bgo#640092 NEWS | 1 + folks/individual.vala | 33 +++++++++++++++++---------------- 2 files changed, 18 insertions(+), 16 deletions(-) commit aa315bb33900610dae1d4f4093c274488be8f4e4 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 11:40:45 2011 +0000 Port Individual.personas to be a Set<Persona> This is a major user of GLib.List, and porting it should make porting a lot of other stuff a lot easier. Helps: bgo#640092 NEWS | 1 + folks/individual-aggregator.vala | 63 ++++++++++--------------- folks/individual.vala | 69 ++++++++++++++++------------ tests/folks/aggregation.vala | 47 +++++++++++++------ tests/key-file/individual-retrieval.vala | 8 ++- tests/libsocialweb/aggregation.vala | 28 ++++++++---- tests/telepathy/individual-properties.vala | 14 +++++- tests/tracker/add-persona.vala | 11 +++-- tests/tracker/remove-contact.vala | 7 ++- tests/tracker/remove-persona.vala | 9 +++- tests/tracker/set-alias.vala | 35 ++++++++------ tests/tracker/set-avatar.vala | 6 ++- tests/tracker/set-birthday.vala | 6 ++- tests/tracker/set-duplicate-email.vala | 16 ++++--- tests/tracker/set-emails.vala | 7 ++- tests/tracker/set-favourite.vala | 28 +++++++---- tests/tracker/set-full-name.vala | 7 ++- tests/tracker/set-gender.vala | 6 ++- tests/tracker/set-im-addresses.vala | 6 ++- tests/tracker/set-notes.vala | 6 ++- tests/tracker/set-phones.vala | 7 ++- tests/tracker/set-postal-addresses.vala | 6 ++- tests/tracker/set-roles.vala | 6 ++- tests/tracker/set-structured-name.vala | 8 ++- tests/tracker/set-urls.vala | 6 ++- tools/inspect/utils.vala | 19 ++++---- 26 files changed, 262 insertions(+), 170 deletions(-) commit 1c7e4040ef1d916b93f1674efef5ebe1dcb3d0eb Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 11:00:27 2011 +0000 Convert the internal store of Personas in Individual to use HashSet This converts the private variables in Individual which store its set of Personas to use a single HashSet instead of a separate GLib.List and HashSet. This won't compile, but making all the necessary adjustments to Individual.personas to make it compile would've made the commit too complex to review. The next commit will update Individual.personas. Helps: bgo#640092 folks/individual.vala | 113 +++++++++++++++++++++++++------------------------ 1 files changed, 57 insertions(+), 56 deletions(-) commit eca4e5453097b76557533eadab8d21b7aca90c1a Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu Feb 3 11:41:38 2011 -0800 Remove use of GLib.List in public BackendStore API. Helps bgo#640092 - Folks should minimize use of GLib.List (including its public API) NEWS | 4 ++ folks/backend-store.vala | 16 ++------ tests/folks/backend-loading.vala | 44 ++++++++++-------------- tests/telepathy/individual-retrieval.vala | 2 +- tests/tracker/add-persona.vala | 2 +- tests/tracker/duplicated-emails.vala | 2 +- tests/tracker/duplicated-phones.vala | 2 +- tests/tracker/link-personas-via-local-ids.vala | 2 +- tests/tracker/link-personas.vala | 2 +- tests/tracker/match-all.vala | 2 +- tests/tracker/match-email-addresses.vala | 2 +- tests/tracker/match-im-addresses.vala | 2 +- tests/tracker/match-known-emails.vala | 2 +- tests/tracker/match-name.vala | 2 +- tests/tracker/match-phone-number.vala | 2 +- tests/tracker/remove-persona.vala | 2 +- tests/tracker/set-duplicate-email.vala | 2 +- 17 files changed, 40 insertions(+), 52 deletions(-) commit cc7b9ac3b058d0734f2df749abc50e71d5797149 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Mar 24 10:05:48 2011 +0000 Use just a HashSet in IndividualAggregator._add_personas() Remove use of GLib.List. Helps: bgo#640092 folks/individual-aggregator.vala | 35 ++++++++++++++--------------------- 1 files changed, 14 insertions(+), 21 deletions(-) commit f8baf37765580eaa497f675f102ef713a67083fe Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 22:27:27 2011 +0100 Change UrlDetails.urls to be a Set<FieldDetails> Helps: bgo#640092 NEWS | 1 + backends/libsocialweb/lib/swf-persona.vala | 18 ++-- backends/tracker/lib/trf-persona-store.vala | 115 ++++---------------------- backends/tracker/lib/trf-persona.vala | 22 ++--- folks/individual.vala | 31 +++---- folks/url-details.vala | 5 +- tests/tracker/add-persona.vala | 11 +-- tests/tracker/set-urls.vala | 12 ++-- tests/tracker/website-updates.vala | 4 +- tools/inspect/utils.vala | 23 +----- 10 files changed, 68 insertions(+), 174 deletions(-) commit 6510a5cee80afafeefd507387455c6ebcb66a880 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 22:13:01 2011 +0100 Change PhoneDetails.phone_numbers to be a Set<FieldDetails> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 104 +++------------------------ backends/tracker/lib/trf-persona.vala | 23 ++---- folks/individual.vala | 35 +++++----- folks/phone-details.vala | 5 +- folks/potential-match.vala | 10 +-- tests/tracker/add-persona.vala | 13 ++-- tests/tracker/duplicated-phones.vala | 18 ++--- tests/tracker/match-phone-number.vala | 16 ++-- tests/tracker/phones-updates.vala | 4 +- tests/tracker/set-phones.vala | 10 ++-- tools/inspect/utils.vala | 6 +- 12 files changed, 74 insertions(+), 171 deletions(-) commit 1d035bd4d12e61b7809b5db251993306f6c170ae Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 21:55:06 2011 +0100 Change EmailDetails.email_addresses to be a Set<FieldDetails> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 96 +++++++++++++++++++++++---- backends/tracker/lib/trf-persona.vala | 23 ++----- folks/email-details.vala | 5 +- folks/individual.vala | 32 ++++----- folks/potential-match.vala | 10 +-- tests/tracker/add-persona.vala | 13 ++-- tests/tracker/duplicated-emails.vala | 16 ++-- tests/tracker/emails-updates.vala | 6 +- tests/tracker/match-email-addresses.vala | 16 ++-- tests/tracker/match-known-emails.vala | 16 ++-- tests/tracker/remove-persona.vala | 11 ++-- tests/tracker/set-duplicate-email.vala | 14 ++-- tests/tracker/set-emails.vala | 10 ++-- tools/inspect/utils.vala | 23 ++++++- 15 files changed, 185 insertions(+), 107 deletions(-) commit f912d31414251e3a87616cdedb46d1d3015ccd53 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 21:41:20 2011 +0100 Change PostalAddressDetails.postal_addresses to be a Set<PostalAddress> Helps: bgo#640092 NEWS | 1 + backends/tracker/lib/trf-persona-store.vala | 66 +++++++++++++-------------- backends/tracker/lib/trf-persona.vala | 27 +++++------- folks/individual.vala | 17 +++---- folks/postal-address-details.vala | 13 +++-- tests/tracker/add-persona.vala | 9 ++-- tests/tracker/set-postal-addresses.vala | 7 +-- tools/inspect/utils.vala | 4 +- 8 files changed, 68 insertions(+), 76 deletions(-) commit 56077a1d890d079e5d0fe6291706a037395db158 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 20:59:18 2011 +0100 Change FieldDetails.parameters to be a MultiMap<string, string> Helps: bgo#640092 NEWS | 3 + backends/tracker/lib/trf-persona-store.vala | 18 +++----- backends/tracker/lib/trf-persona.vala | 23 +++-------- folks/field-details.vala | 59 ++++++++++++--------------- tests/folks/field-details.vala | 34 +++++++--------- tests/tracker/set-urls.vala | 26 +++++++----- 6 files changed, 73 insertions(+), 90 deletions(-) commit 754541d94a53004aa38dfb163f49f9e258930ca5 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 20:29:12 2011 +0100 Change GroupDetails.groups to be a Set<string> Helps: bgo#640092 NEWS | 1 + backends/telepathy/lib/tpf-persona.vala | 24 ++++++++---------- folks/group-details.vala | 5 +++- folks/individual.vala | 37 +++++++++++++--------------- tests/telepathy/individual-properties.vala | 6 ++-- tools/inspect/utils.vala | 19 +++++--------- 6 files changed, 43 insertions(+), 49 deletions(-) commit beef9692895520e335770e0035bbec85240a61b1 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 19:13:30 2011 +0100 Change Backend.persona_stores to be a Map<string, PersonaStore> Helps: bgo#640092 NEWS | 1 + backends/key-file/kf-backend.vala | 20 ++++++++++---------- backends/libsocialweb/sw-backend.vala | 20 ++++++++++---------- backends/telepathy/tp-backend.vala | 22 +++++++++++----------- backends/tracker/tr-backend.vala | 20 ++++++++++---------- folks/backend.vala | 5 ++++- folks/individual-aggregator.vala | 7 +++---- tests/telepathy/individual-retrieval.vala | 3 +-- tests/telepathy/persona-store-capabilities.vala | 2 +- tests/tracker/add-persona.vala | 2 +- tests/tracker/duplicated-emails.vala | 2 +- tests/tracker/duplicated-phones.vala | 2 +- tests/tracker/link-personas-via-local-ids.vala | 2 +- tests/tracker/link-personas.vala | 2 +- tests/tracker/match-all.vala | 2 +- tests/tracker/match-email-addresses.vala | 2 +- tests/tracker/match-im-addresses.vala | 2 +- tests/tracker/match-known-emails.vala | 2 +- tests/tracker/match-name.vala | 2 +- tests/tracker/match-phone-number.vala | 2 +- tests/tracker/remove-persona.vala | 2 +- tests/tracker/set-duplicate-email.vala | 2 +- tools/import.vala | 15 ++++++++++----- tools/inspect/command-backends.vala | 7 +++---- tools/inspect/command-persona-stores.vala | 12 ++++++------ tools/inspect/utils.vala | 9 ++++----- 26 files changed, 87 insertions(+), 82 deletions(-) commit f45fcbb3b1eb92feed741a1a513265d30624aaa8 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 18:57:46 2011 +0100 Remove LinkedHashSet in favour of Gee.HashSet Helps: bgo#640092 NEWS | 1 + folks/Makefile.am | 1 - folks/linked-hash-set.vala | 377 ----------------------------------- tests/folks/Makefile.am | 6 - tests/folks/linked-hash-set.vala | 404 -------------------------------------- 5 files changed, 1 insertions(+), 788 deletions(-) commit 4b28646ea714d9322f09c35095716887ac200115 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 18:43:18 2011 +0100 Change WebServiceDetails.…addresses to be a MultiMap<string, string> Helps: bgo#640092 NEWS | 2 + backends/key-file/kf-persona-store.vala | 4 +- backends/key-file/kf-persona.vala | 42 ++++++++++++-------------- backends/libsocialweb/lib/swf-persona.vala | 11 ++---- backends/tracker/lib/trf-persona-store.vala | 28 +++++++---------- backends/tracker/lib/trf-persona.vala | 17 ++++++----- folks/individual-aggregator.vala | 25 +++++++--------- folks/individual.vala | 27 +++++++--------- folks/web-service-details.vala | 11 ++---- tools/inspect/utils.vala | 12 +++---- 10 files changed, 80 insertions(+), 99 deletions(-) commit 6b5c6befa0a4c392a1ffb6516963a0cbbbb1e4bd Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Apr 19 18:12:15 2011 +0100 Change ImDetails.im_addresses to be a MultiMap<string, string> Helps: bgo#640092 NEWS | 1 + backends/key-file/kf-persona-store.vala | 6 +- backends/key-file/kf-persona.vala | 55 +++++-------- backends/libsocialweb/lib/swf-persona.vala | 12 +-- backends/telepathy/lib/tpf-persona.vala | 17 ++--- backends/tracker/lib/trf-persona-store.vala | 118 ++++++++++++++++++++++----- backends/tracker/lib/trf-persona.vala | 52 ++++--------- folks/im-details.vala | 13 ++-- folks/individual-aggregator.vala | 28 +++---- folks/individual.vala | 25 +++--- folks/potential-match.vala | 16 +--- tests/tracker/add-persona.vala | 17 ++--- tests/tracker/im-details-interface.vala | 2 +- tests/tracker/imaddresses-updates.vala | 14 ++-- tests/tracker/link-personas.vala | 24 ++---- tests/tracker/match-im-addresses.vala | 27 +++---- tests/tracker/set-im-addresses.vala | 24 ++---- tools/import-pidgin.vala | 20 +---- tools/inspect/utils.vala | 14 ++-- 19 files changed, 235 insertions(+), 250 deletions(-)