--- Begin Message ---
- From: Patrick Ohly <patrick ohly gmx de>
- To: Evolution Hackers <evolution-hackers gnome org>
- Subject: Re: [Evolution-hackers] e_cal_remove_object_with_mod() + empty rid: semantic?
- Date: Tue, 17 May 2011 10:58:12 +0200
On Mo, 2011-05-16 at 18:06 +0200, Patrick Ohly wrote:
> I'll do it as <uid>[\n<rid>] so that entries without an rid continue to
> look exactly like the current ones.
Looks good. I ran my KCal-EDS test program which adds, modifies and
removes events, including parent and child (= detached recurrence) in
various orders.
I am now seeing all ECalView signals that I expect and need.
I also ran Evolution in parallel to the test and saw it react to all
signals, but not quite as I would have liked.
Evolution seems to interpret uid + empty rid as "all recurrences
removed", which IMHO is wrong. It should mean "parent removed", because
otherwise there is no way of expressing that change.
For "child removed, parent not updated (= no EXDATE added)" I would
expect Evolution to show the original, unmodified recurrence again.
Instead it only removes the recurrence (as if EXDATE had been added).
I consider this an Evolution bug which can and should be fixed
separately. Please understand that it is currently lower priority for
me, my main goal has to be to get EDS working as intended in EKCal-EDS,
without Evolution.
Please have a look at the attached patch series. They were tested with
the gnome-2-32 branch. Except for the very last one, all apply to
"master". Does this look okay? May I submit to "master" (after fixing
the last patch), gnome-3-0 and gnome-2-32?
--
Bye, Patrick Ohly
--
Patrick Ohly gmx de
http://www.estamos.de/
>From 2e128c63a4a9dd64a3a327262498deb869a119f7 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Wed, 11 May 2011 16:59:51 +0200
Subject: [PATCH 1/8] calendar file backend: support removing parent event with CALOBJ_MOD_THIS
It was possible to create a meeting series with just a detached event
(RECURRENCE-ID set) by importing a meeting invitation for that single
recurrence. It was not possible to arrive at that same state after
adding the parent event (the one with the RRULE) because
e_cal_remove_object_with_mod() removed all instances for
CALOBJ_MOD_THIS and empty rid.
This contradicts the intended semantic of e_cal_remove_object_with_mod():
"By using a combination of the @uid, @rid and @mod
arguments, you can remove specific instances. If what you want
is to remove all instances, use e_cal_remove_object instead."
This patch implements the desired semantic:
- e_cal_backend_file_remove_object(CALOBJ_MOD_THIS) now always
calls remove_instance().
- remove_instance() was extended to also work for the parent
event.
- That call removes the complete object if nothing is left
after removing the instance. This case must be handled by
the caller. The return value is the original object (if
it still exists) and NULL if not.
- Because the uid pointer into the object may become invalid
as part of the removal, a more permanent pointer has to
be provided by the caller.
---
calendar/backends/file/e-cal-backend-file.c | 134 ++++++++++++++++++---------
1 files changed, 88 insertions(+), 46 deletions(-)
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index 33bab50..7a8c450 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -2629,47 +2629,94 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const
g_static_rec_mutex_unlock (&priv->idle_save_rmutex);
}
-static void
-remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *rid)
+/**
+ * Remove one and only one instance. The object may be empty
+ * afterwards, in which case it will be removed completely.
+ *
+ * @uid pointer to UID which must remain valid even if the object gets
+ * removed
+ * @rid NULL, "", or non-empty string when manipulating a specific recurrence;
+ * also must remain valid
+ * @return modified object or NULL if it got removed
+ */
+static ECalBackendFileObject *
+remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *uid, const gchar *rid)
{
gchar *hash_rid;
ECalComponent *comp;
struct icaltimetype current;
- if (!rid || !*rid)
- return;
+ /* only check for non-NULL below, empty string is detected here */
+ if (rid && !*rid)
+ rid = NULL;
- if (g_hash_table_lookup_extended (obj_data->recurrences, rid, (gpointer *)&hash_rid, (gpointer *)&comp)) {
- /* remove the component from our data */
+ if (rid) {
+ /* remove recurrence */
+ if (g_hash_table_lookup_extended (obj_data->recurrences, rid,
+ (gpointer *)&hash_rid, (gpointer *)&comp)) {
+ /* remove the component from our data */
+ icalcomponent_remove_component (cbfile->priv->icalcomp,
+ e_cal_component_get_icalcomponent (comp));
+ cbfile->priv->comp = g_list_remove (cbfile->priv->comp, comp);
+ obj_data->recurrences_list = g_list_remove (obj_data->recurrences_list, comp);
+ g_hash_table_remove (obj_data->recurrences, rid);
+ } else {
+ /* not an error, only add EXDATE */
+ }
+ /* component empty? */
+ if (!obj_data->full_object) {
+ if (!obj_data->recurrences_list) {
+ /* empty now, remove it */
+ remove_component (cbfile, uid, obj_data);
+ return NULL;
+ } else {
+ return obj_data;
+ }
+ }
+ /* remove the main component from our data before modifying it */
icalcomponent_remove_component (cbfile->priv->icalcomp,
- e_cal_component_get_icalcomponent (comp));
- cbfile->priv->comp = g_list_remove (cbfile->priv->comp, comp);
- obj_data->recurrences_list = g_list_remove (obj_data->recurrences_list, comp);
- g_hash_table_remove (obj_data->recurrences, rid);
- }
+ e_cal_component_get_icalcomponent (obj_data->full_object));
+ cbfile->priv->comp = g_list_remove (cbfile->priv->comp, obj_data->full_object);
- if (!obj_data->full_object)
- return;
+ /* add EXDATE or EXRULE to parent */
+ e_cal_util_remove_instances (e_cal_component_get_icalcomponent (obj_data->full_object),
+ icaltime_from_string (rid), CALOBJ_MOD_THIS);
- /* remove the component from our data, temporarily */
- icalcomponent_remove_component (cbfile->priv->icalcomp,
- e_cal_component_get_icalcomponent (obj_data->full_object));
- cbfile->priv->comp = g_list_remove (cbfile->priv->comp, obj_data->full_object);
+ /* Since we are only removing one instance of recurrence
+ event, update the last modified time on the component */
+ current = icaltime_current_time_with_zone (icaltimezone_get_utc_timezone ());
+ e_cal_component_set_last_modified (obj_data->full_object, ¤t);
+
+ /* add the modified object to the beginning of the list,
+ so that it's always before any detached instance we
+ might have */
+ icalcomponent_add_component (cbfile->priv->icalcomp,
+ e_cal_component_get_icalcomponent (obj_data->full_object));
+ cbfile->priv->comp = g_list_prepend (cbfile->priv->comp, obj_data->full_object);
+ } else {
+ /* remove the main component from our data before deleting it */
+ if (!remove_component_from_intervaltree (cbfile, obj_data->full_object)) {
+ /* return without changing anything */
+ g_message (G_STRLOC " Could not remove component from interval tree!");
+ return obj_data;
+ }
+ icalcomponent_remove_component (cbfile->priv->icalcomp,
+ e_cal_component_get_icalcomponent (obj_data->full_object));
+ cbfile->priv->comp = g_list_remove (cbfile->priv->comp, obj_data->full_object);
- e_cal_util_remove_instances (e_cal_component_get_icalcomponent (obj_data->full_object),
- icaltime_from_string (rid), CALOBJ_MOD_THIS);
+ /* remove parent */
+ g_object_unref (obj_data->full_object);
+ obj_data->full_object = NULL;
- /* Since we are only removing one instance of recurrence
- event, update the last modified time on the component */
- current = icaltime_current_time_with_zone (icaltimezone_get_utc_timezone ());
- e_cal_component_set_last_modified (obj_data->full_object, ¤t);
+ /* component may be empty now, check that */
+ if (!obj_data->recurrences_list) {
+ remove_component (cbfile, uid, obj_data);
+ return NULL;
+ }
+ }
- /* add the modified object to the beginning of the list,
- so that it's always before any detached instance we
- might have */
- icalcomponent_add_component (cbfile->priv->icalcomp,
- e_cal_component_get_icalcomponent (obj_data->full_object));
- cbfile->priv->comp = g_list_prepend (cbfile->priv->comp, obj_data->full_object);
+ /* component still exists in a modified form */
+ return obj_data;
}
static gchar *
@@ -2730,8 +2777,6 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
if (rid && *rid)
recur_id = rid;
- comp = obj_data->full_object;
-
switch (mod) {
case CALOBJ_MOD_ALL :
*old_object = get_object_string_from_fileobject (obj_data, recur_id);
@@ -2740,20 +2785,16 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
*object = NULL;
break;
case CALOBJ_MOD_THIS :
- if (!recur_id) {
- *old_object = get_object_string_from_fileobject (obj_data, recur_id);
- remove_component (cbfile, uid, obj_data);
- *object = NULL;
- } else {
- *old_object = get_object_string_from_fileobject (obj_data, recur_id);
+ *old_object = get_object_string_from_fileobject (obj_data, recur_id);
- remove_instance (cbfile, obj_data, recur_id);
- if (comp)
- *object = e_cal_component_get_as_string (comp);
- }
+ obj_data = remove_instance (cbfile, obj_data, uid, recur_id);
+ if (obj_data && obj_data->full_object)
+ *object = e_cal_component_get_as_string (obj_data->full_object);
break;
case CALOBJ_MOD_THISANDPRIOR :
case CALOBJ_MOD_THISANDFUTURE :
+ comp = obj_data->full_object;
+
if (!recur_id || !*recur_id) {
g_static_rec_mutex_unlock (&priv->idle_save_rmutex);
g_propagate_error (error, EDC_ERROR (ObjectNotFound));
@@ -2802,6 +2843,7 @@ cancel_received_object (ECalBackendFile *cbfile, icalcomponent *icalcomp, gchar
ECalBackendFilePrivate *priv;
gchar *rid;
ECalComponent *comp;
+ const gchar *uid = icalcomponent_get_uid (icalcomp);
priv = cbfile->priv;
@@ -2809,7 +2851,7 @@ cancel_received_object (ECalBackendFile *cbfile, icalcomponent *icalcomp, gchar
*new_object = NULL;
/* Find the old version of the component. */
- obj_data = g_hash_table_lookup (priv->comp_uid_hash, icalcomponent_get_uid (icalcomp));
+ obj_data = g_hash_table_lookup (priv->comp_uid_hash, uid);
if (!obj_data)
return FALSE;
@@ -2826,11 +2868,11 @@ cancel_received_object (ECalBackendFile *cbfile, icalcomponent *icalcomp, gchar
/* new_object is kept NULL if not removing the instance */
rid = e_cal_component_get_recurid_as_string (comp);
if (rid && *rid) {
- remove_instance (cbfile, obj_data, rid);
- if (obj_data->full_object)
+ obj_data = remove_instance (cbfile, obj_data, uid, rid);
+ if (obj_data && obj_data->full_object)
*new_object = e_cal_component_get_as_string (obj_data->full_object);
} else
- remove_component (cbfile, icalcomponent_get_uid (icalcomp), obj_data);
+ remove_component (cbfile, uid, obj_data);
g_free (rid);
@@ -3071,7 +3113,7 @@ e_cal_backend_file_receive_objects (ECalBackendSync *backend, EDataCal *cal, con
if (obj_data->full_object)
old_object = e_cal_component_get_as_string (obj_data->full_object);
if (rid)
- remove_instance (cbfile, obj_data, rid);
+ remove_instance (cbfile, obj_data, uid, rid);
else
remove_component (cbfile, uid, obj_data);
--
1.7.2.5
>From 39c941bd54a706757f96276d95ca34795c6ce158 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 09:29:16 +0200
Subject: [PATCH 2/8] libecal: added CALOBJ_MOD_ONLY_THIS
The goal is to have an orthogonal API where each operation also
has an inverse operation. Adding a detached recurrence was
possible with e_cal_modify_object(), but removing it again
wasn't without modifying the parent appointment.
CALOBJ_MOD_ONLY_THIS in e_cal_remove_object_with_mod() provides
that inverse operation by avoiding the modifications to the
parent.
The semantic in e_cal_modify_object(), the other call taking a
CalObjModType, is unchanged. CALOBJ_MOD_ONLY_THIS is not valid there.
Because not all backends reject CALOBJ_MOD_ONLY_THIS when they don't
support it, a static capability CAL_STATIC_CAPABILITY_REMOVE_ONLY_THIS
is added that must be checked first before using CALOBJ_MOD_ONLY_THIS.
---
calendar/libecal/e-cal-util.h | 2 +
calendar/libecal/e-cal.c | 42 +++++++++++++++++++++++----
calendar/libedata-cal/e-cal-backend-sync.c | 2 +-
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/calendar/libecal/e-cal-util.h b/calendar/libecal/e-cal-util.h
index 02e0660..539e7b7 100644
--- a/calendar/libecal/e-cal-util.h
+++ b/calendar/libecal/e-cal-util.h
@@ -47,6 +47,7 @@ typedef enum {
CALOBJ_MOD_THIS = 1 << 0,
CALOBJ_MOD_THISANDPRIOR = 1 << 1,
CALOBJ_MOD_THISANDFUTURE = 1 << 2,
+ CALOBJ_MOD_ONLY_THIS = 1 << 3,
CALOBJ_MOD_ALL = 0x07
} CalObjModType;
@@ -111,6 +112,7 @@ gboolean e_cal_util_event_dates_match (icalcomponent *icalcomp1, icalcomponent *
#define CAL_STATIC_CAPABILITY_NO_THISANDFUTURE "no-thisandfuture"
#define CAL_STATIC_CAPABILITY_NO_THISANDPRIOR "no-thisandprior"
#define CAL_STATIC_CAPABILITY_NO_TRANSPARENCY "no-transparency"
+#define CAL_STATIC_CAPABILITY_REMOVE_ONLY_THIS "remove-only-this"
#define CAL_STATIC_CAPABILITY_ONE_ALARM_ONLY "one-alarm-only"
#define CAL_STATIC_CAPABILITY_ORGANIZER_MUST_ATTEND "organizer-must-attend"
#define CAL_STATIC_CAPABILITY_ORGANIZER_NOT_EMAIL_ADDRESS "organizer-not-email-address"
diff --git a/calendar/libecal/e-cal.c b/calendar/libecal/e-cal.c
index da18f59..f2cab92 100644
--- a/calendar/libecal/e-cal.c
+++ b/calendar/libecal/e-cal.c
@@ -3642,6 +3642,8 @@ e_cal_create_object (ECal *ecal, icalcomponent *icalcomp, gchar **uid, GError **
* or a specific set of instances (CALOBJ_MOD_THISNADPRIOR and
* CALOBJ_MOD_THISANDFUTURE).
*
+ * CALOBJ_MOD_ONLY_THIS is not supported in this call.
+ *
* Returns: TRUE if the operation was successful, FALSE otherwise.
*/
gboolean
@@ -3674,19 +3676,45 @@ e_cal_modify_object (ECal *ecal, icalcomponent *icalcomp, CalObjModType mod, GEr
/**
* e_cal_remove_object_with_mod:
* @ecal: A calendar client.
- * @uid: UID og the object to remove.
+ * @uid: UID of the object to remove.
* @rid: Recurrence ID of the specific recurrence to remove.
* @mod: Type of removal.
* @error: Placeholder for error information.
*
* This function allows the removal of instances of a recurrent
- * appointment. By using a combination of the @uid, @rid and @mod
- * arguments, you can remove specific instances. If what you want
- * is to remove all instances, use e_cal_remove_object instead.
+ * appointment. If what you want is to remove all instances, use
+ * e_cal_remove_object instead.
+ *
+ * By using a combination of the @uid, @rid and @mod arguments, you
+ * can remove specific instances. @uid is mandatory. Empty or NULL
+ * @rid selects the parent appointment (the one with the recurrence
+ * rule). A non-empty @rid selects the recurrence at the time specified
+ * in @rid, using the same time zone as the parent appointment's start
+ * time.
*
- * If not all instances are removed, the client will get a "obj_modified"
- * signal, while it will get a "obj_removed" signal when all instances
- * are removed.
+ * The exact semantic then depends on @mod. CALOBJ_MOD_THIS,
+ * CALOBJ_MOD_THISANDPRIOR, CALOBJ_MOD_THISANDFUTURE and
+ * CALOBJ_MOD_ALL ensure that the event does not recur at the selected
+ * instance(s). This is done by removing any detached recurrence
+ * matching the selection criteria and modifying the parent
+ * appointment (adding EXDATE, adjusting recurrence rules, etc.). It
+ * is not an error if @uid+@rid do not match an existing instance.
+ *
+ * If not all instances are removed, the client will get a
+ * "obj_modified" signal for the parent appointment, while it will get
+ * an "obj_removed" signal when all instances are removed.
+ *
+ * CALOBJ_MOD_ONLY_THIS changes the semantic of CALOBJ_MOD_THIS: @uid
+ * and @rid must select an existing instance. That instance is
+ * removed without modifying the parent appointment. In other words,
+ * e_cal_remove_object_with_mod(CALOBJ_MOD_ONLY_THIS) is the inverse
+ * operation for adding a detached recurrence. The client is
+ * always sent an "obj_removed" signal.
+ *
+ * Note that not all backends support CALOBJ_MOD_ONLY_THIS. Check for
+ * the CAL_STATIC_CAPABILITY_REMOVE_ONLY_THIS capability before using
+ * it. Previous releases did not check consistently for unknown
+ * @mod values, using it with them may have had unexpected results.
*
* Returns: TRUE if the operation was successful, FALSE otherwise.
*/
diff --git a/calendar/libedata-cal/e-cal-backend-sync.c b/calendar/libedata-cal/e-cal-backend-sync.c
index 8264719..514c99d 100644
--- a/calendar/libedata-cal/e-cal-backend-sync.c
+++ b/calendar/libedata-cal/e-cal-backend-sync.c
@@ -670,7 +670,7 @@ _e_cal_backend_remove_object (ECalBackend *backend, EDataCal *cal, EServerMethod
ECalComponentId *id = g_new0 (ECalComponentId, 1);
id->uid = g_strdup (uid);
- if (mod == CALOBJ_MOD_THIS)
+ if (mod == CALOBJ_MOD_THIS || mod == CALOBJ_MOD_ONLY_THIS)
id->rid = g_strdup (rid);
if (!object)
--
1.7.2.5
>From 472fbe158b83867abd624bda30c1ff04afb2260f Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 09:36:59 +0200
Subject: [PATCH 3/8] libecal: catch invalid CalObjModType values
This protects backends without their own parameter checking
from being invoked with invalid CalObjModType values. Note
that this only excludes values that haven't been defined.
Backends still need to check whether they support the
selected mode.
---
calendar/libecal/e-cal.c | 21 +++++++++++++++++++--
1 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/calendar/libecal/e-cal.c b/calendar/libecal/e-cal.c
index f2cab92..3ba8c4e 100644
--- a/calendar/libecal/e-cal.c
+++ b/calendar/libecal/e-cal.c
@@ -3655,7 +3655,15 @@ e_cal_modify_object (ECal *ecal, icalcomponent *icalcomp, CalObjModType mod, GEr
e_return_error_if_fail (E_IS_CAL (ecal), E_CALENDAR_STATUS_INVALID_ARG);
e_return_error_if_fail (icalcomp, E_CALENDAR_STATUS_INVALID_ARG);
e_return_error_if_fail (icalcomponent_is_valid (icalcomp), E_CALENDAR_STATUS_INVALID_ARG);
- e_return_error_if_fail (mod & CALOBJ_MOD_ALL, E_CALENDAR_STATUS_INVALID_ARG);
+ switch (mod) {
+ case CALOBJ_MOD_THIS:
+ case CALOBJ_MOD_THISANDPRIOR:
+ case CALOBJ_MOD_THISANDFUTURE:
+ case CALOBJ_MOD_ALL:
+ break;
+ default:
+ e_return_error_if_fail ("valid CalObjModType" && FALSE, E_CALENDAR_STATUS_INVALID_ARG);
+ }
priv = ecal->priv;
e_return_error_if_fail (priv->gdbus_cal, E_CALENDAR_STATUS_REPOSITORY_OFFLINE);
@@ -3726,7 +3734,16 @@ e_cal_remove_object_with_mod (ECal *ecal, const gchar *uid,
e_return_error_if_fail (E_IS_CAL (ecal), E_CALENDAR_STATUS_INVALID_ARG);
e_return_error_if_fail (uid, E_CALENDAR_STATUS_INVALID_ARG);
- e_return_error_if_fail (mod & CALOBJ_MOD_ALL, E_CALENDAR_STATUS_INVALID_ARG);
+ switch (mod) {
+ case CALOBJ_MOD_THIS:
+ case CALOBJ_MOD_THISANDPRIOR:
+ case CALOBJ_MOD_THISANDFUTURE:
+ case CALOBJ_MOD_ONLY_THIS:
+ case CALOBJ_MOD_ALL:
+ break;
+ default:
+ e_return_error_if_fail ("valid CalObjModType" && FALSE, E_CALENDAR_STATUS_INVALID_ARG);
+ }
priv = ecal->priv;
e_return_error_if_fail (priv->gdbus_cal, E_CALENDAR_STATUS_REPOSITORY_OFFLINE);
--
1.7.2.5
>From 1da8ab59739911ef7704b2bec1e4a3985977e36d Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 09:48:37 +0200
Subject: [PATCH 4/8] calendar file backend: white list check for supported CalObjModType
Explicitly check that the CalObjModType is supported before
starting to work on the appointment. Relies in libecal to reject
completely bogus modes with an "invalid parameter" error.
---
calendar/backends/file/e-cal-backend-file.c | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index 7a8c450..c0059d6 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -2420,6 +2420,16 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const
e_return_data_cal_error_if_fail (priv->icalcomp != NULL, NoSuchCal);
e_return_data_cal_error_if_fail (calobj != NULL, ObjectNotFound);
+ switch (mod) {
+ case CALOBJ_MOD_THIS:
+ case CALOBJ_MOD_THISANDPRIOR:
+ case CALOBJ_MOD_THISANDFUTURE:
+ case CALOBJ_MOD_ALL:
+ break;
+ default:
+ g_propagate_error (error, EDC_ERROR (NotSupported));
+ return;
+ }
/* Parse the icalendar text */
icalcomp = icalparser_parse_string ((gchar *) calobj);
@@ -2618,6 +2628,9 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const
g_list_free (detached);
}
break;
+ case CALOBJ_MOD_ONLY_THIS:
+ // not reached, keep compiler happy
+ break;
}
save (cbfile);
@@ -2762,6 +2775,16 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
e_return_data_cal_error_if_fail (priv->icalcomp != NULL, NoSuchCal);
e_return_data_cal_error_if_fail (uid != NULL, ObjectNotFound);
+ switch (mod) {
+ case CALOBJ_MOD_THIS:
+ case CALOBJ_MOD_THISANDPRIOR:
+ case CALOBJ_MOD_THISANDFUTURE:
+ case CALOBJ_MOD_ALL:
+ break;
+ default:
+ g_propagate_error (error, EDC_ERROR (NotSupported));
+ return;
+ }
*old_object = *object = NULL;
@@ -2784,6 +2807,9 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
*object = NULL;
break;
+ case CALOBJ_MOD_ONLY_THIS:
+ /* not reached, keep compiler happy */
+ break;
case CALOBJ_MOD_THIS :
*old_object = get_object_string_from_fileobject (obj_data, recur_id);
--
1.7.2.5
>From 45fa1bae8fe75e6ab6092789439c348091af70ee Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 11:05:59 +0200
Subject: [PATCH 5/8] calendar file backend: removal notification for detached recurrence, part 1
If e_cal_remove_object_with_mod() was called for an appointment where
only a detached recurrence existed, no "objects-removed" signal was
triggered although it should have been.
Apparently Evolution avoids the problem by calling
e_cal_remove_component() instead in this case. Fixing the problem
makes writing clients easier (no special cases).
With this patch, remove_instance() itself decides what it reports back
to the caller. Note that it cannot report back both a modification and
a removal at the moment.
---
calendar/backends/file/e-cal-backend-file.c | 56 ++++++++++++++++++---------
1 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index c0059d6..30bf3a0 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -2646,14 +2646,22 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const
* Remove one and only one instance. The object may be empty
* afterwards, in which case it will be removed completely.
*
+ * @mod CALOBJ_MOD_THIS or CAL_OBJ_MOD_ONLY_THIS: the later only removes
+ * the instance, the former also adds an EXDATE if rid is set
+ * TODO: CAL_OBJ_MOD_ONLY_THIS
* @uid pointer to UID which must remain valid even if the object gets
* removed
* @rid NULL, "", or non-empty string when manipulating a specific recurrence;
* also must remain valid
+ * @error may be NULL if caller is not interested in errors
* @return modified object or NULL if it got removed
*/
static ECalBackendFileObject *
-remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *uid, const gchar *rid)
+remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
+ const gchar *uid, const gchar *rid,
+ CalObjModType mod,
+ gchar **old_object, gchar **object,
+ GError **error)
{
gchar *hash_rid;
ECalComponent *comp;
@@ -2667,6 +2675,10 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const
/* remove recurrence */
if (g_hash_table_lookup_extended (obj_data->recurrences, rid,
(gpointer *)&hash_rid, (gpointer *)&comp)) {
+ /* Removing without parent? Report as removal. */
+ if (old_object && !obj_data->full_object)
+ *old_object = e_cal_component_get_as_string (comp);
+
/* remove the component from our data */
icalcomponent_remove_component (cbfile->priv->icalcomp,
e_cal_component_get_icalcomponent (comp));
@@ -2691,7 +2703,9 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const
e_cal_component_get_icalcomponent (obj_data->full_object));
cbfile->priv->comp = g_list_remove (cbfile->priv->comp, obj_data->full_object);
- /* add EXDATE or EXRULE to parent */
+ /* add EXDATE or EXRULE to parent, report as update */
+ if (old_object)
+ *old_object = e_cal_component_get_as_string (obj_data->full_object);
e_cal_util_remove_instances (e_cal_component_get_icalcomponent (obj_data->full_object),
icaltime_from_string (rid), CALOBJ_MOD_THIS);
@@ -2700,6 +2714,10 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const
current = icaltime_current_time_with_zone (icaltimezone_get_utc_timezone ());
e_cal_component_set_last_modified (obj_data->full_object, ¤t);
+ /* report update */
+ if (object)
+ *object = e_cal_component_get_as_string (obj_data->full_object);
+
/* add the modified object to the beginning of the list,
so that it's always before any detached instance we
might have */
@@ -2717,7 +2735,9 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const
e_cal_component_get_icalcomponent (obj_data->full_object));
cbfile->priv->comp = g_list_remove (cbfile->priv->comp, obj_data->full_object);
- /* remove parent */
+ /* remove parent, report as removal */
+ if (old_object)
+ *old_object = e_cal_component_get_as_string (obj_data->full_object);
g_object_unref (obj_data->full_object);
obj_data->full_object = NULL;
@@ -2811,11 +2831,7 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
/* not reached, keep compiler happy */
break;
case CALOBJ_MOD_THIS :
- *old_object = get_object_string_from_fileobject (obj_data, recur_id);
-
- obj_data = remove_instance (cbfile, obj_data, uid, recur_id);
- if (obj_data && obj_data->full_object)
- *object = e_cal_component_get_as_string (obj_data->full_object);
+ obj_data = remove_instance (cbfile, obj_data, uid, recur_id, mod, old_object, object, error);
break;
case CALOBJ_MOD_THISANDPRIOR :
case CALOBJ_MOD_THISANDFUTURE :
@@ -2888,17 +2904,17 @@ cancel_received_object (ECalBackendFile *cbfile, icalcomponent *icalcomp, gchar
return FALSE;
}
- if (obj_data->full_object)
- *old_object = e_cal_component_get_as_string (obj_data->full_object);
-
- /* new_object is kept NULL if not removing the instance */
rid = e_cal_component_get_recurid_as_string (comp);
if (rid && *rid) {
- obj_data = remove_instance (cbfile, obj_data, uid, rid);
+ obj_data = remove_instance (cbfile, obj_data, uid, rid, CALOBJ_MOD_THIS, old_object, new_object, NULL);
if (obj_data && obj_data->full_object)
*new_object = e_cal_component_get_as_string (obj_data->full_object);
- } else
+ } else {
+ /* report as removal by keeping *new_object NULL */
+ if (obj_data->full_object)
+ *old_object = e_cal_component_get_as_string (obj_data->full_object);
remove_component (cbfile, uid, obj_data);
+ }
g_free (rid);
@@ -3136,12 +3152,14 @@ e_cal_backend_file_receive_objects (ECalBackendSync *backend, EDataCal *cal, con
fetch_attachments (backend, comp);
obj_data = g_hash_table_lookup (priv->comp_uid_hash, uid);
if (obj_data) {
- if (obj_data->full_object)
- old_object = e_cal_component_get_as_string (obj_data->full_object);
- if (rid)
- remove_instance (cbfile, obj_data, uid, rid);
- else
+ if (rid) {
+ remove_instance (cbfile, obj_data, uid, rid, CALOBJ_MOD_THIS,
+ &old_object, &new_object, NULL);
+ } else {
+ if (obj_data->full_object)
+ old_object = e_cal_component_get_as_string (obj_data->full_object);
remove_component (cbfile, uid, obj_data);
+ }
if (!is_declined)
add_component (cbfile, comp, FALSE);
--
1.7.2.5
>From 55d171be74d6d18e566b22bdbf8a585d58fb9bcb Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 13:30:06 +0200
Subject: [PATCH 6/8] calendar file backend: removal notification for detached recurrence, part 2
e_cal_remove_object_with_mod() can only return one pair of old/new
object pointers to the caller. When the function modifies the parent
and removes a detached recurrence, the removal of the detached
recurrence had to be deduced by clients from the modification of the
parent.
Now clients are explicitly informed about removal of the detached
recurrence in addition to the modification of the parent.
---
calendar/backends/file/e-cal-backend-file.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index 30bf3a0..54fff57 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -2679,6 +2679,17 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
if (old_object && !obj_data->full_object)
*old_object = e_cal_component_get_as_string (comp);
+ /* Removing with parent? Report directly instead of going via caller. */
+ if (obj_data->full_object) {
+ /* old object string not provided,
+ instead rely on the view detecting
+ whether it contains the id */
+ ECalComponentId id;
+ id.uid = (gchar *)uid;
+ id.rid = (gchar *)rid;
+ e_cal_backend_notify_object_removed (E_CAL_BACKEND (cbfile), &id, NULL, NULL);
+ }
+
/* remove the component from our data */
icalcomponent_remove_component (cbfile->priv->icalcomp,
e_cal_component_get_icalcomponent (comp));
--
1.7.2.5
>From ccaa11856590745aea28532cce67a66af06daa18 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Thu, 12 May 2011 14:04:37 +0200
Subject: [PATCH 7/8] calendar file backend: support remove with CALOBJ_MOD_ONLY_THIS
Support for this capability is easy:
- report removal of the detached recurrence
- report error when not found
- avoid modifying the parent (= full_object)
---
calendar/backends/file/e-cal-backend-file.c | 34 ++++++++++++++++++++++----
1 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c
index 54fff57..a9facd5 100644
--- a/calendar/backends/file/e-cal-backend-file.c
+++ b/calendar/backends/file/e-cal-backend-file.c
@@ -446,6 +446,7 @@ e_cal_backend_file_get_static_capabilities (ECalBackendSync *backend, EDataCal *
*capabilities = g_strdup (CAL_STATIC_CAPABILITY_NO_EMAIL_ALARMS ","
CAL_STATIC_CAPABILITY_NO_THISANDFUTURE ","
CAL_STATIC_CAPABILITY_DELEGATE_SUPPORTED ","
+ CAL_STATIC_CAPABILITY_REMOVE_ONLY_THIS ","
CAL_STATIC_CAPABILITY_NO_THISANDPRIOR);
}
@@ -2675,12 +2676,16 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
/* remove recurrence */
if (g_hash_table_lookup_extended (obj_data->recurrences, rid,
(gpointer *)&hash_rid, (gpointer *)&comp)) {
- /* Removing without parent? Report as removal. */
- if (old_object && !obj_data->full_object)
+ /* Removing without parent or not modifying parent?
+ Report removal to caller. */
+ if (old_object &&
+ (!obj_data->full_object || mod == CALOBJ_MOD_ONLY_THIS))
*old_object = e_cal_component_get_as_string (comp);
- /* Removing with parent? Report directly instead of going via caller. */
- if (obj_data->full_object) {
+ /* Reporting parent modification to caller?
+ Report directly instead of going via caller. */
+ if (obj_data->full_object &&
+ mod != CALOBJ_MOD_ONLY_THIS) {
/* old object string not provided,
instead rely on the view detecting
whether it contains the id */
@@ -2696,6 +2701,10 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
cbfile->priv->comp = g_list_remove (cbfile->priv->comp, comp);
obj_data->recurrences_list = g_list_remove (obj_data->recurrences_list, comp);
g_hash_table_remove (obj_data->recurrences, rid);
+ } else if (mod == CALOBJ_MOD_ONLY_THIS) {
+ if (error)
+ g_propagate_error (error, EDC_ERROR (ObjectNotFound));
+ return obj_data;
} else {
/* not an error, only add EXDATE */
}
@@ -2709,6 +2718,11 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
return obj_data;
}
}
+
+ /* avoid modifying parent? */
+ if (mod == CALOBJ_MOD_ONLY_THIS)
+ return obj_data;
+
/* remove the main component from our data before modifying it */
icalcomponent_remove_component (cbfile->priv->icalcomp,
e_cal_component_get_icalcomponent (obj_data->full_object));
@@ -2736,6 +2750,15 @@ remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data,
e_cal_component_get_icalcomponent (obj_data->full_object));
cbfile->priv->comp = g_list_prepend (cbfile->priv->comp, obj_data->full_object);
} else {
+ if (!obj_data->full_object) {
+ /* Nothing to do, parent doesn't exist. Tell
+ caller about this? Not an error with
+ CALOBJ_MOD_THIS. */
+ if (mod == CALOBJ_MOD_ONLY_THIS && error)
+ g_propagate_error (error, EDC_ERROR (ObjectNotFound));
+ return obj_data;
+ }
+
/* remove the main component from our data before deleting it */
if (!remove_component_from_intervaltree (cbfile, obj_data->full_object)) {
/* return without changing anything */
@@ -2810,6 +2833,7 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
case CALOBJ_MOD_THIS:
case CALOBJ_MOD_THISANDPRIOR:
case CALOBJ_MOD_THISANDFUTURE:
+ case CALOBJ_MOD_ONLY_THIS:
case CALOBJ_MOD_ALL:
break;
default:
@@ -2839,8 +2863,6 @@ e_cal_backend_file_remove_object (ECalBackendSync *backend, EDataCal *cal,
*object = NULL;
break;
case CALOBJ_MOD_ONLY_THIS:
- /* not reached, keep compiler happy */
- break;
case CALOBJ_MOD_THIS :
obj_data = remove_instance (cbfile, obj_data, uid, recur_id, mod, old_object, object, error);
break;
--
1.7.2.5
>From a391ae860a24cdf11551ea8f6ccccbd73835034e Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick ohly intel com>
Date: Tue, 17 May 2011 09:45:24 +0200
Subject: [PATCH 8/8] calendar: include rid in "objects-removed" ECalView signal
Since migration to D-Bus for libecal<->EDS communication, the
RECURRENCE-ID (rid) has not been sent in the "objects-removed" signal.
As a result, a backend could not communicate the removal of specific
recurrences.
This patch adds the rid after a newline to the string stored
internally and transferred via D-Bus. Because the newline is only
added when needed, traditional uid-only removals look the same as
before and continue to work with older versions of libecal. A uid+rid
combination will look like an unknown uid to an older libecal which
does not know how to split them. Therefore the D-Bus API is considered
unchanged and the interface number is not increased.
Whether clients really interpret "objects-removed" with empty rid (=
parent removed) or valid rid (= child removed) correctly is outside
the scope of this patch.
---
calendar/libecal/e-cal-view.c | 17 +++++++++++++----
calendar/libedata-cal/e-data-cal-view.c | 19 ++++++++++++++-----
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/calendar/libecal/e-cal-view.c b/calendar/libecal/e-cal-view.c
index af132ca..6319c58 100644
--- a/calendar/libecal/e-cal-view.c
+++ b/calendar/libecal/e-cal-view.c
@@ -92,9 +92,18 @@ build_id_list (const gchar * const *seq)
list = NULL;
for (i = 0; seq[i]; i++) {
ECalComponentId *id;
+ const gchar * eol;
+
id = g_new (ECalComponentId, 1);
- id->uid = g_strdup (seq[i]);
- id->rid = NULL; /* TODO */
+ /* match encoding as in notify_remove() in e-data-cal-view.c: <uid>[\n<rid>] */
+ eol = strchr (seq[i], '\n');
+ if (eol) {
+ id->uid = g_strndup (seq[i], eol - seq[i]);
+ id->rid = g_strdup (eol + 1);
+ } else {
+ id->uid = g_strdup (seq[i]);
+ id->rid = NULL;
+ }
list = g_list_prepend (list, id);
}
@@ -138,14 +147,14 @@ objects_modified_cb (EGdbusCalView *gdbus_calview, const gchar * const *objects,
}
static void
-objects_removed_cb (EGdbusCalView *gdbus_calview, const gchar * const *uids, ECalView *view)
+objects_removed_cb (EGdbusCalView *gdbus_calview, const gchar * const *seq, ECalView *view)
{
GList *list;
g_return_if_fail (E_IS_CAL_VIEW (view));
g_object_ref (view);
- list = build_id_list (uids);
+ list = build_id_list (seq);
g_signal_emit (G_OBJECT (view), signals[OBJECTS_REMOVED], 0, list);
diff --git a/calendar/libedata-cal/e-data-cal-view.c b/calendar/libedata-cal/e-data-cal-view.c
index 394acf0..7fc4a4d 100644
--- a/calendar/libedata-cal/e-data-cal-view.c
+++ b/calendar/libedata-cal/e-data-cal-view.c
@@ -189,7 +189,7 @@ send_pending_removes (EDataCalView *view)
if (priv->removes->len == 0)
return;
- /* TODO: send ECalComponentIds as a list of pairs */
+ /* send ECalComponentIds as <uid>[\n<rid>], as encoded in notify_remove() */
e_gdbus_cal_view_emit_objects_removed (view->priv->gdbus_object, (const gchar * const *) priv->removes->data);
reset_array (priv->removes);
}
@@ -268,7 +268,8 @@ static void
notify_remove (EDataCalView *view, ECalComponentId *id)
{
EDataCalViewPrivate *priv = view->priv;
- gchar *uid;
+ gchar *ids;
+ size_t uid_len, rid_len;
send_pending_adds (view);
send_pending_changes (view);
@@ -277,9 +278,17 @@ notify_remove (EDataCalView *view, ECalComponentId *id)
send_pending_removes (view);
}
- /* TODO: store ECalComponentId instead of just uid*/
- uid = g_strdup (id->uid);
- g_array_append_val (priv->removes, uid);
+ /* store ECalComponentId as <uid>[\n<rid>] (matches D-Bus API) */
+ uid_len = id->uid ? strlen (id->uid) : 0;
+ rid_len = id->rid ? strlen (id->rid) : 0;
+ ids = g_malloc (uid_len + rid_len + (rid_len ? 2 : 1));
+ if (uid_len)
+ strcpy (ids, id->uid);
+ if (rid_len) {
+ ids[uid_len] = '\n';
+ strcpy (ids + uid_len + 1, id->rid);
+ }
+ g_array_append_val (priv->removes, ids);
g_hash_table_remove (priv->ids, id);
--
1.7.2.5
_______________________________________________
evolution-hackers mailing list
evolution-hackers gnome org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers
--- End Message ---
