Commit 2247cdd5 authored by Philip Withnall's avatar Philip Withnall

Bug 665692 — Use constructors correctly

In order to allow libfolks to be used from introspected languages (such as
Python) properly, we need to correctly use the GObject construction process,
rather than generating code which does all object initialisation inside
a *_new() function. This involves moving lots of code into construct{} blocks.

There are some complications; mostly the need for various private variables to
now be exposed as construct-only properties. Most of them should've been
anyway.

Other complications arose from the fact that moving code to a construct{}
block can subtly change the execution order of the code if the Object() call
lists properties which are non-construct properties (e.g. the “alias” property
of a Persona). The setters for these properties will now be called _after_ the
construct{} code, whereas previously they would've been called beforehand.
This rears its head in Tpf.Persona, but hopefully nowhere else.

Closes: bgo#665692
parent 04b30abb
......@@ -8,6 +8,24 @@ Bugs fixed:
* Bug 665039 — Crash in folks_backends_sw_backend_add_service
* Bug 665728 — TpfPersonaStore: prepare() isn't mutually exclusive inside a
single thread
* Bug 665692 — Use constructors correctly
API changes:
* Add Edsf.PersonaStore.source
* Make Edsf.Persona.contact writeable on construct (previously private setter)
* Make Edsf.Persona.contact_id writeable on construct (previously private
setter)
* Add Swf.PersonaStore.service
* Make Swf.Persona.lsw_contact writeable on construct (previously private
setter)
* Add Trf.Persona.tracker_id
* Add Trf.Persona.cursor
* Make AbstractFieldDetails.value writeable on construct (previously just a
normal setter)
* Make AbstractFieldDetails.parameters writeable on construct (previously just a
normal setter)
* Add ObjectCache.type_id
* Add ObjectCache.id
Overview of changes from libfolks 0.6.4.1 to libfolks 0.6.5
=============================================================
......
......@@ -60,6 +60,11 @@ public class Folks.Backends.Eds.Backend : Folks.Backend
* {@inheritDoc}
*/
public Backend ()
{
Object ();
}
construct
{
this._persona_stores = new HashMap<string, PersonaStore> ();
this._persona_stores_ro = this._persona_stores.read_only_view;
......
......@@ -42,7 +42,6 @@ public class Edsf.PersonaStore : Folks.PersonaStore
private E.BookClient _addressbook;
private E.BookClientView _ebookview;
private E.SourceList? _source_list = null;
private E.Source _source;
private string _query_str;
/* The timeout after which we consider a property change to have failed if we
......@@ -191,6 +190,16 @@ public class Edsf.PersonaStore : Folks.PersonaStore
get { return this._personas_ro; }
}
/**
* The EDS {@link E.Source} associated with this persona store.
*
* @since UNRELEASED
*/
public E.Source source
{
get; construct;
}
/**
* Create a new PersonaStore.
*
......@@ -203,12 +212,17 @@ public class Edsf.PersonaStore : Folks.PersonaStore
public PersonaStore (E.Source s)
{
string eds_uid = s.peek_uid ();
Object (id: eds_uid, display_name: eds_uid);
this._source = s;
Object (id: eds_uid,
display_name: eds_uid,
source: s);
}
construct
{
this._personas = new HashMap<string, Persona> ();
this._personas_ro = this._personas.read_only_view;
this._query_str = "(contains \"x-evolution-any-field\" \"\")";
this._source.changed.connect (this._source_changed_cb);
this.source.changed.connect (this._source_changed_cb);
this._notify_if_default ();
}
......@@ -550,7 +564,7 @@ public class Edsf.PersonaStore : Folks.PersonaStore
this._source_list.changed.connect (this._source_list_changed_cb);
/* Connect to the address book. */
this._addressbook = new E.BookClient (this._source);
this._addressbook = new E.BookClient (this.source);
this._addressbook.notify["readonly"].connect (
this._address_book_notify_read_only_cb);
......@@ -1989,7 +2003,7 @@ public class Edsf.PersonaStore : Folks.PersonaStore
*/
private void _update_trust_level ()
{
unowned SourceGroup? group = (SourceGroup?) this._source.peek_group ();
unowned SourceGroup? group = (SourceGroup?) this.source.peek_group ();
if (group != null)
{
var base_uri = group.peek_base_uri ();
......@@ -2025,7 +2039,7 @@ public class Edsf.PersonaStore : Folks.PersonaStore
E.BookClient.get_sources (out sources);
var default_source = sources.peek_default_source ();
if (default_source != null &&
this._source.peek_uid () == default_source.peek_uid ())
this.source.peek_uid () == default_source.peek_uid ())
{
is_default = true;
}
......
......@@ -140,13 +140,15 @@ public class Edsf.Persona : Folks.Persona,
private bool _is_favourite;
private E.Contact _contact; /* should be set on construct */
/**
* The e-d-s contact represented by this Persona
*/
public E.Contact contact
{
get;
private set;
get { return this._contact; }
construct { this._contact = value; }
}
/**
......@@ -371,7 +373,7 @@ public class Edsf.Persona : Folks.Persona,
*
* @since 0.6.0
*/
public string contact_id { get; private set; }
public string contact_id { get; construct; }
private string _full_name = "";
/**
......@@ -703,16 +705,20 @@ public class Edsf.Persona : Folks.Persona,
if (full_name == null)
full_name = "";
debug ("Creating new Edsf.Persona with IID '%s'", iid);
Object (display_id: full_name,
uid: uid,
iid: iid,
store: store,
is_user: is_user);
is_user: is_user,
contact_id: contact_id,
contact: contact);
}
construct
{
debug ("Creating new Edsf.Persona with IID '%s'", this.iid);
this._gender = Gender.UNSPECIFIED;
this.contact_id = contact_id;
this._phone_numbers = new HashSet<PhoneFieldDetails> (
(GLib.HashFunc) PhoneFieldDetails.hash,
(GLib.EqualFunc) PhoneFieldDetails.equal);
......@@ -747,7 +753,7 @@ public class Edsf.Persona : Folks.Persona,
(GLib.EqualFunc) RoleFieldDetails.equal);
this._roles_ro = this._roles.read_only_view;
this._update (contact);
this._update (this._contact);
}
/**
......@@ -805,12 +811,15 @@ public class Edsf.Persona : Folks.Persona,
/**
* Update attribs of the persona.
*/
internal void _update (E.Contact contact)
internal void _update (E.Contact updated_contact)
{
this.contact = contact;
this.freeze_notify ();
/* We get a new E.Contact instance from EDS containing all the updates,
* so replace our existing contact with it. */
this._contact = updated_contact;
this.notify_property ("contact");
this._update_names ();
this._update_avatar ();
this._update_urls ();
......
......@@ -41,6 +41,10 @@ internal class Edsf.MemoryIcon : Object, Icon, LoadableIcon
*/
public MemoryIcon (string? image_type, uint8[] image_data)
{
/* Note: To be correct, these should both be properties of the object
* and this constructor should just call Object(…). However, this is an
* internal class, so we can skip all the pain of making a uint8[] object
* for this class only. */
this._image_data = image_data;
this._image_type = image_type;
}
......
......@@ -81,6 +81,11 @@ public class Folks.Backends.Kf.Backend : Folks.Backend
* {@inheritDoc}
*/
public Backend ()
{
Object ();
}
construct
{
this._persona_stores = new HashMap<string, PersonaStore> ();
this._persona_stores_ro = this._persona_stores.read_only_view;
......
......@@ -34,7 +34,6 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
{
private HashMap<string, Persona> _personas;
private Map<string, Persona> _personas_ro;
private File _file;
private GLib.KeyFile _key_file;
private unowned Cancellable _save_key_file_cancellable = null;
private bool _is_prepared = false;
......@@ -143,6 +142,15 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
get { return this._personas_ro; }
}
/**
* File containing the persona store data.
*
* This must be in GLib key file format.
*
* @since UNRELEASED
*/
public File file { get; construct; }
/**
* Create a new PersonaStore.
*
......@@ -154,10 +162,13 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
var id = key_file.get_basename ();
Object (id: id,
display_name: id);
display_name: id,
file: key_file);
}
construct
{
this.trust_level = PersonaStoreTrust.FULL;
this._file = key_file;
this._personas = new HashMap<string, Persona> ();
this._personas_ro = this._personas.read_only_view;
}
......@@ -178,7 +189,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
{
this._prepare_pending = true;
var filename = this._file.get_path ();
var filename = this.file.get_path ();
this._key_file = new GLib.KeyFile ();
/* Load or create the file */
......@@ -191,7 +202,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
{
uint8 *contents = null;
yield this._file.load_contents_async (null, out contents,
yield this.file.load_contents_async (null, out contents,
null);
var contents_s = (string) contents;
......@@ -218,7 +229,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
}
/* Ensure the parent directory tree exists for the new file */
File parent_dir = this._file.get_parent ();
File parent_dir = this.file.get_parent ();
try
{
......@@ -245,7 +256,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
try
{
/* Create the file */
FileOutputStream stream = yield this._file.create_async (
FileOutputStream stream = yield this.file.create_async (
FileCreateFlags.PRIVATE, Priority.DEFAULT);
yield stream.close_async (Priority.DEFAULT);
}
......@@ -271,8 +282,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
var added_personas = new HashSet<Persona> ();
foreach (var persona_id in groups)
{
Persona persona = new Kf.Persona (this._key_file, persona_id,
this);
Persona persona = new Kf.Persona (persona_id, this);
this._personas.set (persona.iid, persona);
added_personas.add (persona);
}
......@@ -379,7 +389,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
/* Create a new persona and set its addresses property to update the
* key file */
Persona persona = new Kf.Persona (this._key_file, persona_id, this);
Persona persona = new Kf.Persona (persona_id, this);
this._personas.set (persona.iid, persona);
try
......@@ -409,12 +419,17 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
return persona;
}
internal unowned KeyFile get_key_file ()
{
return this._key_file;
}
internal async void save_key_file ()
{
var key_file_data = this._key_file.to_data ();
var cancellable = new Cancellable ();
debug ("Saving key file '%s'.", this._file.get_path ());
debug ("Saving key file '%s'.", this.file.get_path ());
/* There's no point in having two competing file write operations.
* We can ensure that only one is running by just checking if a
......@@ -435,7 +450,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
* Vala <= 0.10, it returned the character length). FIXME: We need to
* take this into account until we depend explicitly on
* Vala >= 0.11. */
yield this._file.replace_contents_async (key_file_data,
yield this.file.replace_contents_async (key_file_data,
key_file_data.length, null, false, FileCreateFlags.PRIVATE,
cancellable);
}
......@@ -446,7 +461,7 @@ public class Folks.Backends.Kf.PersonaStore : Folks.PersonaStore
/* Translators: the first parameter is a filename, the second is
* an error message. */
warning (_("Could not write updated key file '%s': %s"),
this._file.get_path (), e.message);
this.file.get_path (), e.message);
}
}
......
......@@ -33,7 +33,6 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
ImDetails,
WebServiceDetails
{
private unowned GLib.KeyFile _key_file;
private HashMultiMap<string, ImFieldDetails> _im_addresses;
private HashMultiMap<string, WebServiceFieldDetails> _web_service_addresses;
private string _alias = ""; /* must not be null */
......@@ -99,7 +98,8 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
debug ("Setting alias of Kf.Persona '%s' to '%s'.", this.uid, alias);
this._key_file.set_string (this.display_id, "__alias", alias);
unowned KeyFile key_file = ((Kf.PersonaStore) this.store).get_key_file ();
key_file.set_string (this.display_id, "__alias", alias);
yield ((Kf.PersonaStore) this.store).save_key_file ();
this._alias = alias;
......@@ -124,12 +124,14 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
public async void change_im_addresses (
MultiMap<string, ImFieldDetails> im_addresses) throws PropertyError
{
unowned KeyFile key_file = ((Kf.PersonaStore) this.store).get_key_file ();
/* Remove the current IM addresses from the key file */
foreach (var protocol1 in this._im_addresses.get_keys ())
{
try
{
this._key_file.remove_key (this.display_id, protocol1);
key_file.remove_key (this.display_id, protocol1);
}
catch (KeyFileError e1)
{
......@@ -178,7 +180,7 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
string[] addrs = (string[]) normalised_addresses.to_array ();
addrs.length = normalised_addresses.size;
this._key_file.set_string_list (this.display_id, protocol2, addrs);
key_file.set_string_list (this.display_id, protocol2, addrs);
}
/* Get the PersonaStore to save the key file */
......@@ -207,12 +209,14 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
MultiMap<string, WebServiceFieldDetails> web_service_addresses)
throws PropertyError
{
unowned KeyFile key_file = ((Kf.PersonaStore) this.store).get_key_file ();
/* Remove the current web service addresses from the key file */
foreach (var web_service1 in this._web_service_addresses.get_keys ())
{
try
{
this._key_file.remove_key (this.display_id,
key_file.remove_key (this.display_id,
"web-service." + web_service1);
}
catch (KeyFileError e)
......@@ -238,7 +242,7 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
foreach (var ws_fd1 in ws_fds)
addrs += ws_fd1.value;
this._key_file.set_string_list (this.display_id,
key_file.set_string_list (this.display_id,
"web-service." + web_service2, addrs);
foreach (var ws_fd2 in ws_fds)
......@@ -258,7 +262,7 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
* Create a new persona for the {@link PersonaStore} `store`, representing
* the Persona given by the group `uid` in the key file `key_file`.
*/
public Persona (KeyFile key_file, string id, Folks.PersonaStore store)
public Persona (string id, Folks.PersonaStore store)
{
var iid = store.id + ":" + id;
var uid = this.build_uid ("key-file", store.id, id);
......@@ -268,11 +272,13 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
uid: uid,
store: store,
is_user: false);
}
debug ("Adding key-file Persona '%s' (IID '%s', group '%s')", uid, iid,
id);
construct
{
debug ("Adding key-file Persona '%s' (IID '%s', group '%s')", this.uid,
this.iid, this.display_id);
this._key_file = key_file;
this._im_addresses = new HashMultiMap<string, ImFieldDetails> (
null, null, ImFieldDetails.hash, (EqualFunc) ImFieldDetails.equal);
this._web_service_addresses =
......@@ -282,16 +288,17 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
(GLib.EqualFunc) WebServiceFieldDetails.equal);
/* Load the IM addresses from the key file */
unowned KeyFile key_file = ((Kf.PersonaStore) this.store).get_key_file ();
try
{
var keys = this._key_file.get_keys (this.display_id);
var keys = key_file.get_keys (this.display_id);
foreach (unowned string key in keys)
{
/* Alias */
if (key == "__alias")
{
this._alias = this._key_file.get_string (this.display_id,
key);
this._alias = key_file.get_string (this.display_id, key);
if (this._alias == null)
{
......@@ -308,7 +315,7 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
decomposed_key[0] == "web-service")
{
unowned string web_service = decomposed_key[1];
var web_service_addresses = this._key_file.get_string_list (
var web_service_addresses = key_file.get_string_list (
this.display_id, web_service);
foreach (var web_service_address in web_service_addresses)
......@@ -322,7 +329,7 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
/* IM addresses */
unowned string protocol = key;
var im_addresses = this._key_file.get_string_list (
var im_addresses = key_file.get_string_list (
this.display_id, protocol);
foreach (var im_address in im_addresses)
......
......@@ -39,7 +39,6 @@ public class Swf.PersonaStore : Folks.PersonaStore
private bool _is_prepared = false;
private bool _prepare_pending = false;
private bool _is_quiescent = false;
private ClientService _service;
private ClientContactView _contact_view;
/* No writeable properties
......@@ -149,6 +148,14 @@ public class Swf.PersonaStore : Folks.PersonaStore
get { return this._personas_ro; }
}
/**
* The libsocialweb {@link SocialWebClient.ClientService} associated with the
* persona store.
*
* @since UNRELEASED
*/
public ClientService service { get; construct; }
/**
* Create a new PersonaStore.
*
......@@ -157,11 +164,14 @@ public class Swf.PersonaStore : Folks.PersonaStore
*/
public PersonaStore (ClientService service)
{
Object (display_name: service.get_display_name(),
id: service.get_name ());
Object (display_name: service.get_display_name (),
id: service.get_name (),
service: service);
}
construct
{
this.trust_level = PersonaStoreTrust.PARTIAL;
this._service = service;
this._personas = new HashMap<string, Persona> ();
this._personas_ro = this._personas.read_only_view;
}
......@@ -219,7 +229,7 @@ public class Swf.PersonaStore : Folks.PersonaStore
* async call to return. See: bgo#665039. */
this.ref ();
this._service.get_static_capabilities (
this.service.get_static_capabilities (
(service, caps, error) =>
{
if (caps == null)
......@@ -244,7 +254,7 @@ public class Swf.PersonaStore : Folks.PersonaStore
/* Take another ref for this async call. */
this.ref ();
this._service.contacts_query_open_view
this.service.contacts_query_open_view
("people", parameters, (query, contact_view) =>
{
/* The D-Bus call could return an error. In this
......@@ -325,7 +335,7 @@ public class Swf.PersonaStore : Folks.PersonaStore
{
foreach (var contact in contacts)
{
if (this._service.get_name () != contact.service)
if (this.service.get_name () != contact.service)
{
continue;
}
......@@ -341,7 +351,7 @@ public class Swf.PersonaStore : Folks.PersonaStore
var removed_personas = new HashSet<Persona> ();
foreach (var contact in contacts)
{
if (this._service.get_name () != contact.service)
if (this.service.get_name () != contact.service)
{
continue;
}
......
......@@ -185,7 +185,7 @@ public class Swf.Persona : Folks.Persona,
public Contact lsw_contact
{
get { return this._lsw_contact; }
private set
construct
{
if (_lsw_contact != null && _lsw_contact != value)
{
......@@ -253,7 +253,6 @@ public class Swf.Persona : Folks.Persona,
public Persona (PersonaStore store, Contact contact)
{
var id = get_contact_id (contact);
var service = contact.service.dup();
var uid = this.build_uid (BACKEND_NAME, store.id, id);
var iid = this._build_iid (store.id, id);
......@@ -261,13 +260,17 @@ public class Swf.Persona : Folks.Persona,
uid: uid,
iid: iid,
store: store,
is_user: false);
this.lsw_contact = contact;
is_user: false,
lsw_contact: contact);
}
construct
{
debug ("Creating new Sw.Persona '%s' for %s UID '%s': %p",
uid, store.display_name, id, this);
this.uid, this.store.display_name, this.display_id, this);
var facebook_jid = this._build_facebook_jid (store.id, id);
var facebook_jid =
this._build_facebook_jid (this.store.id, this.display_id);
if (facebook_jid != null)
{
try
......@@ -286,10 +289,11 @@ public class Swf.Persona : Folks.Persona,
}
}
var service = this.lsw_contact.service.dup ();
this._web_service_addresses.set (service,
new WebServiceFieldDetails (id));
new WebServiceFieldDetails (this.display_id));
update (contact);
this.update (this.lsw_contact);
}
~Persona ()
......
......@@ -58,6 +58,11 @@ public class Folks.Backends.Sw.Backend : Folks.Backend
* {@inheritDoc}
*/
public Backend ()
{
Object ();
}
construct
{
this._persona_stores = new HashMap<string, PersonaStore> ();
this._persona_stores_ro = this._persona_stores.read_only_view;
......
......@@ -46,16 +46,23 @@ private interface LoggerIface : Object
internal class Logger : GLib.Object
{
private static LoggerIface _logger;
private string _account_path;
private uint _logger_watch_id;
public signal void invalidated ();
public signal void favourite_contacts_changed (string[] added,
string[] removed);
/**
* D-Bus object path of the {@link TelepathyGLib.Account} to watch for
* favourite contacts.
*
* @since UNRELEASED
*/
public string account_path { get; construct; }
public Logger (string account_path)
{
this._account_path = account_path;
Object (account_path: account_path);
}
~Logger ()
...