[california/wip/725786-edit-recurring] Better solution to master/generated instance problem
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [california/wip/725786-edit-recurring] Better solution to master/generated instance problem
- Date: Wed, 16 Jul 2014 02:32:25 +0000 (UTC)
commit e463b92af715c4f78cc5ca38355d3567ad6bebfa
Author: Jim Nelson <jim yorba org>
Date: Tue Jul 15 16:27:49 2014 -0700
Better solution to master/generated instance problem
All masters are now loaded in the backing and associated with their
generated instances before being injected into the system...this
alleviates I/O and async issues in the UI, who always have access to
the masters by virtue of having access to their instances.
.../backing-calendar-source-subscription.vala | 101 ++++++++++++++++----
src/backing/backing-calendar-source.vala | 13 +--
.../backing-eds-calendar-source-subscription.vala | 102 +++++++++++---------
src/backing/eds/backing-eds-calendar-source.vala | 2 +-
src/component/component-instance.vala | 46 ++++++++-
src/host/host-create-update-event.vala | 2 +-
src/host/host-create-update-recurring.vala | 25 ++----
src/host/host-show-event.vala | 9 +-
vapi/libecal-1.2.vapi | 3 +-
vapi/libecal-1.2/libecal-1.2.metadata | 5 +-
10 files changed, 202 insertions(+), 106 deletions(-)
---
diff --git a/src/backing/backing-calendar-source-subscription.vala
b/src/backing/backing-calendar-source-subscription.vala
index 12a1342..bf4fbc4 100644
--- a/src/backing/backing-calendar-source-subscription.vala
+++ b/src/backing/backing-calendar-source-subscription.vala
@@ -9,15 +9,13 @@ namespace California.Backing {
/**
* A subscription to an active timespan of interest of a calendar.
*
- * CalendarSourceSubscription generates { link Component.Instance}s of the various events in a
- * window of time in a calendar. Note that for recurring events, there is no interface to directly
- * edit or remove the "original" iCalendar source. Rather, the caller should update or remove
- * generated instances of that recurring event with flags to indicate how the original is to be
- * altered to match these changes. See { link CalendarSource} for the interface to make these
- * alterations.
+ * The subscription notifies of calendar updates via its signals. It can list complete or partial
+ * collections of { link Component.Instances} it has reported via those signals.
*
- * The subscription can notify of calendar event updates and list a complete or partial collections
- * of the same.
+ * CalendarSourceSubscription generates { link Component.Instance}s of the various events in a
+ * window of time in a calendar. Both master instances and generated recurring instances are
+ * reported through the interface. Operations performed on instances should be done with the
+ * subscription's { link CalendarSource}.
*/
public abstract class CalendarSourceSubscription : BaseObject {
@@ -50,22 +48,63 @@ public abstract class CalendarSourceSubscription : BaseObject {
/**
* Fired as existing { link Component.Instance}s are discovered when starting a subscription.
*
+ * Like { link instance_added}, this is only fired for master Instances which do not
+ * generate recurring instances and generated Instances. See { link master_discovered} to
+ * be notified of all master Instances.
+ *
* This is fired while { link start} is working, either in the foreground or in the background.
* It won't fire until start() is invoked.
+ *
+ * @see master_discovered
*/
public signal void instance_discovered(Component.Instance instance);
/**
- * Indicates that an { link Instance} within the { link window} has been added to the calendar.
+ * Fired as existing master { link Component.Instance}s are discovered when starting a
+ * subscription.
+ *
+ * This is fired for all discovered master Instances whether or not they may generate
+ * Instances for this subscription.
+ *
+ * This is fired while { link start} is working, either in the foreground or in the background.
+ * It won't fire until start() is invoked.
+ *
+ * @see instance_discovered
+ */
+ public signal void master_discovered(Component.Instance instance);
+
+ /**
+ * Fired when a { link Component.Instance} within the { link window} has been added to the
+ * { link CalendarSource}.
+ *
+ * This is fired only for master Instances which do not generate recurring Instances and for
+ * generated Instances. See { link master_added} to be notified of all master Instances.
*
* The signal is fired for both local additions (added through this interface) and remote
* additions.
*
* This signal won't fire until { link start} is called.
+ *
+ * @see master_added
*/
public signal void instance_added(Component.Instance instance);
/**
+ * Fired as existing master { link Component.Instance}s are added to the { link CalendarSource}.
+ *
+ * This is fired for all discovered master Instances whether or not they may generate
+ * Instances for this subscription.
+ *
+ * The signal is fired for both local additions (added through this interface) and remote
+ * additions.
+ *
+ * This signal won't fire until { link start} is called.
+ *
+ * @see instance_added
+ */
+ public signal void master_added(Component.Instance instance);
+
+ /**
* Indicates that an { link Instance} within the { link date_window} has been removed from the
* calendar.
*
@@ -143,10 +182,17 @@ public abstract class CalendarSourceSubscription : BaseObject {
* @see instance_discovered
*/
protected virtual void notify_instance_discovered(Component.Instance instance) {
- if (add_instance(instance))
- instance_discovered(instance);
- else
+ if (!add_instance(instance)) {
debug("Cannot add discovered component %s to %s: already known", instance.to_string(),
to_string());
+
+ return;
+ }
+
+ if (instance.is_master_instance)
+ master_discovered(instance);
+
+ if (!instance.can_generate_instances)
+ instance_discovered(instance);
}
/**
@@ -156,10 +202,17 @@ public abstract class CalendarSourceSubscription : BaseObject {
* @see instance_added
*/
protected virtual void notify_instance_added(Component.Instance instance) {
- if (add_instance(instance))
- instance_added(instance);
- else
+ if (!add_instance(instance)) {
debug("Cannot add component %s to %s: already known", instance.to_string(), to_string());
+
+ return;
+ }
+
+ if (instance.is_master_instance)
+ master_added(instance);
+
+ if (!instance.can_generate_instances)
+ instance_added(instance);
}
/**
@@ -195,13 +248,13 @@ public abstract class CalendarSourceSubscription : BaseObject {
* Notify that the { link Component.Instance}s have been dropped due to the { link Source} going
* unavailable.
*/
- protected virtual void notify_instance_dropped(Component.Instance instance) {
+ protected virtual void notify_instance_dropped(Component.UID uid) {
Gee.Collection<Component.Instance> removed_instances;
- if (remove_instance(instance.uid, out removed_instances)) {
+ if (remove_instance(uid, out removed_instances)) {
foreach (Component.Instance removed_instance in removed_instances)
instance_dropped(removed_instance);
} else {
- debug("Cannot notify dropped component %s in %s: not known", instance.to_string(), to_string());
+ debug("Cannot notify dropped component %s in %s: not known", uid.to_string(), to_string());
}
}
@@ -264,7 +317,7 @@ public abstract class CalendarSourceSubscription : BaseObject {
// the multimap
debug("Dropping %d instances to %s: unavailable", instances.size, calendar.to_string());
foreach (Component.Instance instance in instances.get_values().to_array())
- notify_instance_dropped(instance);
+ notify_instance_dropped(instance.uid);
}
/**
@@ -285,6 +338,16 @@ public abstract class CalendarSourceSubscription : BaseObject {
}
/**
+ * Returns the master { link Component.Instance} for the { link Component.UID}.
+ *
+ * @returns null if the UID has not been sene.
+ */
+ public Component.Instance? master_for_uid(Component.UID uid) {
+ return traverse<Component.Instance>(instances.get(uid))
+ .first_matching(instance => instance.is_master_instance);
+ }
+
+ /**
* Returns the seen { link Component.Instance} matching the supplied (possibly partially
* filled-out) Instance.
*
diff --git a/src/backing/backing-calendar-source.vala b/src/backing/backing-calendar-source.vala
index 895f7cf..7b2fac0 100644
--- a/src/backing/backing-calendar-source.vala
+++ b/src/backing/backing-calendar-source.vala
@@ -95,17 +95,14 @@ public abstract class CalendarSource : Source {
AffectedInstances affected, Cancellable? cancellable = null) throws Error;
/**
- * Fetches the master component of an { link Component.Instance}.
+ * Fetches the master component of an { link Component.Instance} by its { link Component.UID}.
*
- * If an Instance is recurring, { link CalendarSourceSubscription} produces individual instances
- * of those recurrences (which therefore lack the rules and information that produce them).
- * This call allows for the original (or master) component to be retrieved.
- *
- * If the instance is not recurring, the returned Instance will be identical (or near-identical)
- * to the one produced by CalendarSourceSubscription.
+ * Unlike master instances generated by a { link CalendarSourceSubscription}, there instance
+ * is not tracked by the { link CalendarSource} and will not be updated as it is update
+ * elsewhere, either locally or remotely.
*
* @see Component.Instance.is_recurring_instance
- * @throws An Error if the { link Component.UID} is unknown to the { link CalendarSource},
+ * @throws An Error if the { link Component.UID} is unknown to the CalendarSource,
* as well as for the sundry I/O errors and such.
*/
public abstract async Component.Instance fetch_master_component_async(Component.UID uid,
diff --git a/src/backing/eds/backing-eds-calendar-source-subscription.vala
b/src/backing/eds/backing-eds-calendar-source-subscription.vala
index b3003a6..30a314f 100644
--- a/src/backing/eds/backing-eds-calendar-source-subscription.vala
+++ b/src/backing/eds/backing-eds-calendar-source-subscription.vala
@@ -11,17 +11,21 @@ namespace California.Backing {
*/
internal class EdsCalendarSourceSubscription : CalendarSourceSubscription {
+ private delegate void InstanceNotifier(Component.Instance instance);
+
private E.CalClientView view;
+ private string sexp;
// this is different than "active", which gets set when start completes
private bool started = false;
private Error? start_err = null;
// Called from EdsCalendarSource.subscribe_async(). The CalClientView should not be started
public EdsCalendarSourceSubscription(EdsCalendarSource eds_calendar, Calendar.ExactTimeSpan window,
- E.CalClientView view) {
+ E.CalClientView view, string sexp) {
base (eds_calendar, window);
this.view = view;
+ this.sexp = sexp;
}
~EdsCalendarSourceSubscription() {
@@ -82,36 +86,36 @@ internal class EdsCalendarSourceSubscription : CalendarSourceSubscription {
// next
view.start();
- // prime with the list of known events
- view.client.generate_instances(
- window.start_exact_time.to_time_t(),
- window.end_exact_time.to_time_t(),
- cancellable,
- on_instance_generated,
- on_generate_finished);
+ discovery_async.begin(cancellable);
}
- private bool on_instance_generated(E.CalComponent eds_component, time_t instance_start,
- time_t instance_end) {
+ private async void discovery_async(Cancellable? cancellable) {
+ SList<unowned iCal.icalcomponent> ical_components;
try {
- Component.Event? event = Component.Instance.convert(calendar, eds_component.get_icalcomponent())
- as Component.Event;
- if (event != null)
- notify_instance_discovered(event);
+ yield view.client.get_object_list(sexp, cancellable, out ical_components);
} catch (Error err) {
- debug("Unable to generate discovered event for %s: %s", to_string(), err.message);
+ start_err = err;
+
+ start_failed(err);
+
+ return;
}
- return true;
- }
-
- private void on_generate_finished() {
+ // process all known objects within the sexp range
+ on_objects_discovered_added(ical_components, notify_instance_discovered);
+
// only set when generation (start) is finished
active = true;
}
- private void on_objects_added(SList<weak iCal.icalcomponent> objects) {
- foreach (weak iCal.icalcomponent ical_component in objects) {
+ private void on_objects_added(SList<unowned iCal.icalcomponent> ical_components) {
+ // process all added objects
+ on_objects_discovered_added(ical_components, notify_instance_added);
+ }
+
+ private void on_objects_discovered_added(SList<unowned iCal.icalcomponent> ical_components,
+ InstanceNotifier notifier) {
+ foreach (unowned iCal.icalcomponent ical_component in ical_components) {
if (String.is_empty(ical_component.get_uid()))
continue;
@@ -121,47 +125,53 @@ internal class EdsCalendarSourceSubscription : CalendarSourceSubscription {
if (has_uid(uid))
notify_instance_removed(uid);
- // if no recurrences, add this alone
- if (!E.Util.component_has_recurrences(ical_component)) {
- add_instance(ical_component);
-
+ // add all instances, master and generated
+ Component.Instance? master = add_instance(null, ical_component, notifier);
+ if (master == null)
+ continue;
+
+ // if no recurrences, done
+ if (!E.Util.component_has_recurrences(ical_component))
continue;
- }
// generate recurring instances
- view.client.generate_instances_for_object(
+ view.client.generate_instances_for_object_sync(
ical_component,
window.start_exact_time.to_time_t(),
window.end_exact_time.to_time_t(),
- null,
- on_instance_added,
- null);
+ (eds_component, start, end) => {
+ add_instance(master, eds_component.get_icalcomponent(), notifier);
+
+ return true;
+ }
+ );
}
}
- private bool on_instance_added(E.CalComponent eds_component, time_t instance_start,
- time_t instance_end) {
- add_instance(eds_component.get_icalcomponent());
-
- return true;
- }
-
// Assumes all existing events with UID/RID have been removed already
- private void add_instance(iCal.icalcomponent ical_component) {
+ private Component.Instance? add_instance(Component.Instance? master, iCal.icalcomponent ical_component,
+ InstanceNotifier notifier) {
// convert the added component into a new Event
- Component.Event? added_event;
+ Component.Event? added_event = null;
try {
added_event = Component.Instance.convert(calendar, ical_component) as Component.Event;
- if (added_event != null)
- notify_instance_added(added_event);
+ if (added_event != null) {
+ // assign the master (if this isn't the master already)
+ added_event.master = master;
+
+ // notify of didscovery/addition
+ notifier(added_event);
+ }
} catch (Error err) {
debug("Unable to process added event: %s", err.message);
}
+
+ return added_event;
}
- private void on_objects_modified(SList<weak iCal.icalcomponent> objects) {
- SList<weak iCal.icalcomponent> add_list = new SList<weak iCal.icalcomponent>();
- foreach (weak iCal.icalcomponent ical_component in objects) {
+ private void on_objects_modified(SList<unowned iCal.icalcomponent> ical_components) {
+ SList<unowned iCal.icalcomponent> add_list = new SList<unowned iCal.icalcomponent>();
+ foreach (unowned iCal.icalcomponent ical_component in ical_components) {
// if not an instance and has recurring, treat as an add (which removes and adds generated
// instances)
if (!E.Util.component_is_instance(ical_component) &&
E.Util.component_has_recurrences(ical_component)) {
@@ -207,8 +217,8 @@ internal class EdsCalendarSourceSubscription : CalendarSourceSubscription {
on_objects_added(add_list);
}
- private void on_objects_removed(SList<weak E.CalComponentId?> ids) {
- foreach (weak E.CalComponentId id in ids)
+ private void on_objects_removed(SList<unowned E.CalComponentId?> ids) {
+ foreach (unowned E.CalComponentId id in ids)
notify_instance_removed(new Component.UID(id.uid));
}
}
diff --git a/src/backing/eds/backing-eds-calendar-source.vala
b/src/backing/eds/backing-eds-calendar-source.vala
index eb1e2a9..92a7b1c 100644
--- a/src/backing/eds/backing-eds-calendar-source.vala
+++ b/src/backing/eds/backing-eds-calendar-source.vala
@@ -135,7 +135,7 @@ internal class EdsCalendarSource : CalendarSource {
E.CalClientView view;
yield client.get_view(sexp, cancellable, out view);
- return new EdsCalendarSourceSubscription(this, window, view);
+ return new EdsCalendarSourceSubscription(this, window, view, sexp);
}
public override async Component.UID? create_component_async(Component.Instance instance,
diff --git a/src/component/component-instance.vala b/src/component/component-instance.vala
index 35e6581..4154669 100644
--- a/src/component/component-instance.vala
+++ b/src/component/component-instance.vala
@@ -32,6 +32,7 @@ public abstract class Instance : BaseObject, Gee.Hashable<Instance> {
public const string PROP_RRULE = "rrule";
public const string PROP_RID = "rid";
public const string PROP_SEQUENCE = "sequence";
+ public const string PROP_MASTER = "master";
protected const string PROP_IN_FULL_UPDATE = "in-full-update";
@@ -78,12 +79,49 @@ public abstract class Instance : BaseObject, Gee.Hashable<Instance> {
public Component.DateTime? rid { get; set; default = null; }
/**
- * Returns true if the { link Recurrable} is in fact a recurring instance.
+ * Returns true if the { link Instance} is a master instance.
+ *
+ * A master instance is one that has not been generated from another Instance's recurring
+ * rule (RRULE). In practice, this means the Instance does not have a RECURRENCE-ID.
+ *
+ * @see rid
+ * @see rrule
+ */
+ public bool is_master_instance { get { return rid == null; } }
+
+ /**
+ * Returns true if the { link Instance} is a generated recurring instance.
+ *
+ * A generated recurring instance is one that has been artificially constructed from another
+ * Instance's recurring rule (RRULE). In practice, this means the Instance has a
+ * RECURRENCE-ID.
+ *
+ * @see rid
+ * @see Backing.CalendarSource.fetch_master_component_async
+ */
+ public bool is_generated_instance { get { return rid != null; } }
+
+ /**
+ * Returns true if the master { link Instance} can generate recurring instances.
+ *
+ * This indicates the Instance is a master Instance and can generate recurring instances from
+ * its RRULE. In practice, this means the Instance has no RECURRENCE-ID but does have an
+ * RRULE. (Generated instances will have the RRULE that construct them as well as a
+ * RECURRENCE-ID.)
*
* @see rid
* @see Backing.CalendarSource.fetch_master_component_async
*/
- public bool is_recurring_instance { get { return rid != null; } }
+ public bool can_generate_instances { get { return rid == null && rrule != null; } }
+
+ /**
+ * If a generated { link Instance}, holds a reference to the master Instance that generated it.
+ *
+ * The { link Backing} should do everything it can to provide a master Instance for all
+ * generated Instances. However, if this is null it is not a guarantee this is a master
+ * Instance. Use { link is_master_instance}.
+ */
+ public Instance? master { get; internal set; default = null; }
/**
* The SEQUENCE of a VEVENT, VTODO, or VJOURNAL.
@@ -421,10 +459,10 @@ public abstract class Instance : BaseObject, Gee.Hashable<Instance> {
if (this == other)
return true;
- if (is_recurring_instance != other.is_recurring_instance)
+ if (is_generated_instance != other.is_generated_instance)
return false;
- if (is_recurring_instance && !rid.equal_to(other.rid))
+ if (is_generated_instance && !rid.equal_to(other.rid))
return false;
if (sequence != other.sequence)
diff --git a/src/host/host-create-update-event.vala b/src/host/host-create-update-event.vala
index 095e693..2f5d3be 100644
--- a/src/host/host-create-update-event.vala
+++ b/src/host/host-create-update-event.vala
@@ -283,7 +283,7 @@ public class CreateUpdateEvent : Gtk.Grid, Toolkit.Card {
return;
// if updating a recurring event, need to ask about update scope
- if (event.is_recurring_instance && is_update) {
+ if (event.is_generated_instance && is_update) {
rotating_button_box.family = FAMILY_RECURRING;
return;
diff --git a/src/host/host-create-update-recurring.vala b/src/host/host-create-update-recurring.vala
index 7c4ba87..9e42a74 100644
--- a/src/host/host-create-update-recurring.vala
+++ b/src/host/host-create-update-recurring.vala
@@ -160,24 +160,8 @@ public class CreateUpdateRecurring : Gtk.Grid, Toolkit.Card {
// *must* have an Event by this point, whether from before or due to this jump
assert(event != null);
- // need to load the master component in order to update the RRULE (which isn't stored in
- // the generated instances)
- load_master_async.begin();
- }
-
- private async void load_master_async() {
- try {
- Component.Instance master_instance = yield event.calendar_source.fetch_master_component_async(
- event.uid, null);
- master = master_instance as Component.Event;
- } catch (Error err) {
- debug("Unable to load master from %s: %s", event.calendar_source.to_string(),
- err.message);
-
- master = null;
- }
-
- if (master == null) {
+ // need to use the master component in order to update the master RRULE
+ if (!can_update_recurring(event)) {
jump_back();
return;
@@ -186,7 +170,12 @@ public class CreateUpdateRecurring : Gtk.Grid, Toolkit.Card {
update_controls();
}
+ public static bool can_update_recurring(Component.Event event) {
+ return event.is_master_instance || (event.master is Component.Event);
+ }
+
private void update_controls() {
+ master = (event.is_master_instance ? event : event.master) as Component.Event;
assert(master != null);
make_recurring_checkbutton.active = (master.rrule != null);
diff --git a/src/host/host-show-event.vala b/src/host/host-show-event.vala
index c647be4..b20bc33 100644
--- a/src/host/host-show-event.vala
+++ b/src/host/host-show-event.vala
@@ -86,13 +86,10 @@ public class ShowEvent : Gtk.Grid, Toolkit.Card {
// description
set_label(null, description_text, Markup.linkify(escape(event.description), linkify_delegate));
- // don't current support updating recurring events properly; see
- // https://bugzilla.gnome.org/show_bug.cgi?id=725786
bool read_only = event.calendar_source != null && event.calendar_source.read_only;
- bool updatable = !event.is_recurring_instance && !read_only;
- update_button.visible = updatable;
- update_button.no_show_all = updatable;
+ update_button.visible = !read_only;
+ update_button.no_show_all = !read_only;
remove_button.visible = !read_only;
remove_button.no_show_all = !read_only;
@@ -142,7 +139,7 @@ public class ShowEvent : Gtk.Grid, Toolkit.Card {
//
// TODO: Gtk.Stack would be a better widget for this animation, but it's unavailable in
// Glade as of GTK+ 3.12.
- if (event.is_recurring_instance) {
+ if (event.is_generated_instance) {
button_box_revealer.reveal_child = false;
remove_recurring_revealer.reveal_child = true;
diff --git a/vapi/libecal-1.2.vapi b/vapi/libecal-1.2.vapi
index 4d9a21a..d692d2e 100644
--- a/vapi/libecal-1.2.vapi
+++ b/vapi/libecal-1.2.vapi
@@ -53,7 +53,8 @@ namespace E {
public unowned string get_local_attachment_store ();
[CCode (finish_name = "e_cal_client_get_object_finish")]
public async void get_object (string uid, string? rid, GLib.Cancellable? cancellable, out
iCal.icalcomponent out_icalcomp) throws GLib.Error;
- public async bool get_object_list (string sexp, GLib.Cancellable? cancellable) throws
GLib.Error;
+ [CCode (finish_name = "e_cal_client_get_object_list_finish")]
+ public async bool get_object_list (string sexp, GLib.Cancellable? cancellable, out
GLib.SList<weak iCal.icalcomponent> out_icalcomps) throws GLib.Error;
public async bool get_object_list_as_comps (string sexp, GLib.Cancellable? cancellable)
throws GLib.Error;
public bool get_object_list_as_comps_sync (string sexp, GLib.SList out_ecalcomps,
GLib.Cancellable? cancellable) throws GLib.Error;
public bool get_object_list_sync (string sexp, GLib.SList out_icalcomps, GLib.Cancellable?
cancellable) throws GLib.Error;
diff --git a/vapi/libecal-1.2/libecal-1.2.metadata b/vapi/libecal-1.2/libecal-1.2.metadata
index e93a428..2577912 100644
--- a/vapi/libecal-1.2/libecal-1.2.metadata
+++ b/vapi/libecal-1.2/libecal-1.2.metadata
@@ -89,9 +89,10 @@ e_cal_client_get_object.cancellable nullable="1"
e_cal_client_get_object_finish type_name="void"
e_cal_client_get_object_finish.out_icalcomp is_out="1" transfer_ownership="1"
-e_cal_client_get_object_list async="1"
+e_cal_client_get_object_list async="1" finish_name="e_cal_client_get_object_list_finish"
e_cal_client_get_object_list.cancellable nullable="1"
-e_cal_client_get_object_list_finish.icalcomps is_out="1" value_owned="1" type_arguments="iCal.icalcomponent"
+
+e_cal_client_get_object_list_finish.out_icalcomps is_out="1" transfer_ownership="1" type_arguments="unowned
iCal.icalcomponent"
e_cal_client_get_object_list_as_comps async="1"
e_cal_client_get_object_list_as_comps.cancellable nullable="1"
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
