Re: [evolution-patches] Exchange: Crash during calendar sync

["Veerapuram Varadhan" <vvaradhan novell com>]

On Wed, 2005-11-23 at 11:58 +0000, "Veerapuram Varadhan"  wrote:
> On Wed, 2005-11-23 at 10:32 +0000, Sarfraaz Ahmed  wrote:
> > Hi,
> > 
> > In ... check_change_type, you could simply add another check to that
> > already existing if condition. It would be easy to understand then.
> Its more of an aesthetic change.  My patch avoids a "Big indented
> block". :-)
> 
> > However, we need to figure out how the instances would be matched
> > though. That FIXME you put there will also have to be fixed soon now
> :)
> > 
> Thats a FIXME, which eventually will be fixed soon.
> 
The time has come to get rid of that "Fixme".  Attached patch fixes that
new "FIXME" and other recurrence related things.

Let me know your comments.

V. Varahan.

? cal-recur-sync.diff
? depcomp
? fix-316315.diff
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution-exchange/ChangeLog,v
retrieving revision 1.416
diff -u -p -r1.416 ChangeLog
--- ChangeLog	28 Nov 2005 11:36:48 -0000	1.416
+++ ChangeLog	1 Dec 2005 13:41:44 -0000
@@ -1,3 +1,26 @@
+2005-11-23  Veerapuram Varadhan <vvaradhan novell com>
+
+	* calendar/e-cal-backend-exchange.c: (match_object_sexp)
+	(check_change_type): Calendar components that are just instances
+	and without master object, don't have corresponding
+	icalcomponent. Check for the existence of icalcomponent before
+	adding them to the change list.  Also, process the instances and
+	return them.
+	(match_recurrence_sexp): Local function added for handling 
+	recurrences during a query.
+	(get_changes): Use e_xmlhash_foreach_key_remove() instead of 
+	e_xmlhash_foreach_key ().
+	(e_cal_backend_exchange_compute_changes_foreach_key): Change
+	return type to gboolean and don't remove elements from hash as we
+	are in "_foreach_" callback.
+	(set_default_timezone): Just set the timezone without doing any
+	existence check.
+
+	* calendar/e-cal-backend-exchange-tasks.c (get_changed_tasks):
+	Check for VTIMEZONE component and add it to the main-component.
+	
+	** Fixes #316315 + many others.
+	
 2005-11-28  Sarfraaz Ahmed  <asarfraaz novell com>
 
 	* calendar/e-cal-backend-exchange-calendar.c (modify_object_with_href):
Index: calendar/e-cal-backend-exchange-calendar.c
===================================================================
RCS file: /cvs/gnome/evolution-exchange/calendar/e-cal-backend-exchange-calendar.c,v
retrieving revision 1.48
diff -u -p -r1.48 e-cal-backend-exchange-calendar.c
--- calendar/e-cal-backend-exchange-calendar.c	28 Nov 2005 11:36:48 -0000	1.48
+++ calendar/e-cal-backend-exchange-calendar.c	1 Dec 2005 13:41:45 -0000
@@ -1348,6 +1348,8 @@ receive_objects (ECalBackendSync *backen
 	icalcomponent *subcomp;
 	ECalBackendSyncStatus status = GNOME_Evolution_Calendar_Success;	
 
+	d(printf ("receive_objects(%p, %p, %s)\n", backend, cal, calobj));
+
 	cbexc =	E_CAL_BACKEND_EXCHANGE_CALENDAR (backend);
 
 	g_return_val_if_fail (E_IS_CAL_BACKEND_EXCHANGE_CALENDAR (cbexc), GNOME_Evolution_Calendar_InvalidObject);
Index: calendar/e-cal-backend-exchange-tasks.c
===================================================================
RCS file: /cvs/gnome/evolution-exchange/calendar/e-cal-backend-exchange-tasks.c,v
retrieving revision 1.20
diff -u -p -r1.20 e-cal-backend-exchange-tasks.c
--- calendar/e-cal-backend-exchange-tasks.c	23 Aug 2005 08:28:24 -0000	1.20
+++ calendar/e-cal-backend-exchange-tasks.c	1 Dec 2005 13:41:45 -0000
@@ -566,7 +566,7 @@ get_changed_tasks (ECalBackendExchange *
 	E2kContext *ctx;
 	const char *modtime, *str, *prop;
 	char *uid, *body;
-	char *tzid;
+	char *tzid = NULL;
 	int status, i, priority, percent;
 	float f_percent;
 	ECalComponent *ecal, *ecomp;
@@ -574,6 +574,7 @@ get_changed_tasks (ECalBackendExchange *
 	const icaltimezone *itzone;
 	ECalComponentDateTime ecdatetime;
 	icalcomponent *icalcomp;
+	icalcomponent *tz_comp = NULL;
 
         g_return_val_if_fail (E_IS_CAL_BACKEND_EXCHANGE (cbex), SOUP_STATUS_CANCELLED);
 
@@ -630,8 +631,21 @@ get_changed_tasks (ECalBackendExchange *
 			g_hash_table_insert (modtimes, g_strdup (result->href),
 					     g_strdup (modtime));
 		}
-
-		e_cal_backend_exchange_add_timezone (cbex, icalcomp);		
+		tzid = e2k_properties_get_prop (result->props,
+						E2K_PR_EXCHANGE_TIMEZONE);
+		d(printf ("ecbet_get_changed_tasks: tzid = %s\n", tzid));
+		if (tzid)
+			tz_comp = icalparser_parse_string (tzid);
+		if (tz_comp && 
+		    icalcomponent_isa (tz_comp) == ICAL_VTIMEZONE_COMPONENT) {
+			icaltimezone *zone;
+			d(printf ("ecbet_get_changed_tasks: adding timezone comp\n"));
+			zone = icaltimezone_new ();
+			icaltimezone_set_component (zone, tz_comp);
+			icalcomponent_add_component (icalcomp, tz_comp);
+			icaltimezone_free (zone, 1);
+		}
+		e_cal_backend_exchange_add_timezone (cbex, icalcomp); 
 
 		itt = icaltime_from_timet (e2k_parse_timestamp (modtime), 0);
 		if (!icaltime_is_null_time (itt)) 
Index: calendar/e-cal-backend-exchange.c
===================================================================
RCS file: /cvs/gnome/evolution-exchange/calendar/e-cal-backend-exchange.c,v
retrieving revision 1.46
diff -u -p -r1.46 e-cal-backend-exchange.c
--- calendar/e-cal-backend-exchange.c	28 Nov 2005 11:36:48 -0000	1.46
+++ calendar/e-cal-backend-exchange.c	1 Dec 2005 13:41:45 -0000
@@ -378,7 +378,7 @@ open_calendar (ECalBackendSync *backend,
 	load_cache (cbex, euri);
 
 	g_mutex_unlock (cbex->priv->open_lock);
-
+	
 	return GNOME_Evolution_Calendar_Success;
 }
 
@@ -502,6 +502,8 @@ e_cal_backend_exchange_add_object (ECalB
 
 	is_instance = (icalcomponent_get_first_property (comp, ICAL_RECURRENCEID_PROPERTY) != NULL);
 
+	d(printf("ecbe_add_object: is_instance: %d\n", is_instance));	
+
 	if (ecomp && ecomp->icomp && !is_instance)
 		return FALSE;
 
@@ -865,6 +867,30 @@ typedef struct {
 	ECalBackend *backend;
 	icaltimezone *default_zone;
 } MatchObjectData;
+static void
+match_recurrence_sexp (gpointer data, gpointer user_data)
+{
+	icalcomponent *icomp = data;
+	MatchObjectData *match_data = user_data;
+	ECalComponent *comp = NULL;
+	char* comp_as_string = NULL;
+	
+	d(printf("ecbe_match_recurrence_sexp(%p, %p)\n", icomp, match_data));
+
+	if (!icomp || !match_data)
+		return;
+
+	comp = e_cal_component_new ();
+	e_cal_component_set_icalcomponent (comp, icalcomponent_new_clone (icomp));
+
+	if ((!match_data->search_needed) ||
+	    (e_cal_backend_sexp_match_comp (match_data->obj_sexp, comp, match_data->backend))) {
+		comp_as_string = e_cal_component_get_as_string (comp);
+		match_data->obj_list = g_list_append (match_data->obj_list, comp_as_string);
+		d(printf ("ecbe_match_recurrence_sexp: match found, adding \n%s\n", comp_as_string));
+	}
+	g_object_unref (comp);
+}
 
 static void
 match_object_sexp (gpointer key, gpointer value, gpointer data)
@@ -872,23 +898,31 @@ match_object_sexp (gpointer key, gpointe
 	ECalBackendExchangeComponent *ecomp = value;
 	MatchObjectData *match_data = data;
 	ECalComponent *comp;
-	
-	comp = e_cal_component_new ();
-	e_cal_component_set_icalcomponent (comp, icalcomponent_new_clone (ecomp->icomp));
 
-	if ((!match_data->search_needed) ||
-	    (e_cal_backend_sexp_match_comp (match_data->obj_sexp, comp, match_data->backend))) {
-		match_data->obj_list = g_list_append (match_data->obj_list,
-						      e_cal_component_get_as_string (comp));
+	/* 
+	   In case of detached instances with no master object,
+	   ecomp->icomp will be NULL and hence should be skipped
+	   from matching.
+	*/
+	if (!ecomp || !match_data)
+		return;
 
-		#if 0
-		/* match also recurrences */
-		g_hash_table_foreach (obj_data->recurrences,
-				      (GHFunc) match_recurrence_sexp,
-				      match_data);
-		#endif
+	if (ecomp->icomp) {
+		comp = e_cal_component_new ();
+		e_cal_component_set_icalcomponent (comp, icalcomponent_new_clone (ecomp->icomp));
+		
+		if ((!match_data->search_needed) ||
+		    (e_cal_backend_sexp_match_comp (match_data->obj_sexp, comp, match_data->backend))) {
+			match_data->obj_list = g_list_append (match_data->obj_list,
+						      e_cal_component_get_as_string (comp));
+		}
+		g_object_unref (comp);
 	}
-	g_object_unref (comp);
+
+	/* match also recurrences */
+	g_list_foreach (ecomp->instances,
+			(GFunc) match_recurrence_sexp,
+			match_data);
 }
 
 static ECalBackendSyncStatus
@@ -971,6 +1005,7 @@ e_cal_backend_exchange_add_timezone (ECa
 	if (!prop) 
 		return GNOME_Evolution_Calendar_InvalidObject;
 	tzid = icalproperty_get_tzid (prop);
+	d(printf("ecbe_add_timezone: tzid = %s\n", tzid));
 	if (g_hash_table_lookup (cbex->priv->timezones, tzid))
 		return GNOME_Evolution_Calendar_ObjectIdAlreadyExists;
 
@@ -992,6 +1027,9 @@ add_timezone (ECalBackendSync *backend, 
 	GNOME_Evolution_Calendar_CallStatus status;
 	icalcomponent *vtzcomp;
 
+	if (!tzobj)
+		return GNOME_Evolution_Calendar_InvalidObject;
+
 	vtzcomp = icalcomponent_new_from_string ((char *)tzobj);
 	if (!vtzcomp)
 		return GNOME_Evolution_Calendar_InvalidObject;
@@ -1017,16 +1055,16 @@ set_default_timezone (ECalBackendSync *b
 		      const char *tzid)
 {
 	ECalBackendExchange *cbex = E_CAL_BACKEND_EXCHANGE (backend);
-	icaltimezone *zone;
 
 	d(printf("ecbe_set_default_timezone(%p, %p, %s)\n", backend, cal, tzid));
-
-	zone = g_hash_table_lookup (cbex->priv->timezones, tzid);
-	if (zone) {
-		cbex->priv->default_timezone = zone;
-		return GNOME_Evolution_Calendar_Success;
-	} else
-		return GNOME_Evolution_Calendar_ObjectNotFound;
+	/* 
+	   We call this function before calling e_cal_open in client and
+	   hence we set the timezone directly.  In the implementation of
+	   e_cal_open, we set this timezone to every-objects that we create.
+	*/
+	   
+	cbex->priv->default_timezone = icaltimezone_get_builtin_timezone_from_tzid (tzid);
+	return GNOME_Evolution_Calendar_Success;
 }
 
 struct search_data {
@@ -1333,13 +1371,30 @@ check_change_type (gpointer key, gpointe
 	ECalBackendExchangeComponent *ecomp = value;
 	struct ChangeData *change_data = data;
 	char *calobj;
-	ECalComponent *comp;
+	ECalComponent *comp = NULL;
 	char *uid = key;
+	GList *l = NULL;
+	icalcomponent *icomp = NULL;
 	
-	if (ecomp != NULL) {
+	/* 
+	   In case of detached instances with no master object,
+	   ecomp->icomp will be NULL and hence should be skipped
+	   from matching.
+	*/
+	if (!ecomp)
+		return;
+	icomp = ecomp->icomp;
+	l = ecomp->instances;
+	for (icomp = ecomp->icomp; l; icomp = l->data, l = l->next) {
+		if (!icomp)
+			continue;
 
 		comp = e_cal_component_new ();
-		e_cal_component_set_icalcomponent (comp, icalcomponent_new_clone (ecomp->icomp));
+		/*
+		  e_cal_component_set_icalcomponent does a icalcomponent_free of 
+		  previous icalcomponent before setting the new one. 
+		 */
+		e_cal_component_set_icalcomponent (comp, icalcomponent_new_clone (icomp));	
 
 		calobj = e_cal_component_get_as_string (comp);
 		switch (e_xmlhash_compare (change_data->ehash, uid, calobj)){
@@ -1355,9 +1410,8 @@ check_change_type (gpointer key, gpointe
 		}
 		
 		g_free (calobj);
-		g_object_unref (comp);
 	}
-		
+	g_object_unref (comp);		
 }
 
 struct cbe_data {
@@ -1367,7 +1421,7 @@ struct cbe_data {
 	EXmlHash *ehash;
 };
 
-static void
+static gboolean
 e_cal_backend_exchange_compute_changes_foreach_key (const char *key, const char *value, gpointer data)
 {
 	struct cbe_data *cbedata = data;
@@ -1377,16 +1431,19 @@ e_cal_backend_exchange_compute_changes_f
 	if (!ecomp) {
 		ECalComponent *comp;
 		comp = e_cal_component_new ();
+		if (ecomp->icomp)
+			e_cal_component_set_icalcomponent (comp, 
+							   icalcomponent_new_clone (ecomp->icomp));
 		if (cbedata->kind == ICAL_VTODO_COMPONENT)
 			e_cal_component_set_new_vtype (comp, E_CAL_COMPONENT_TODO);
 		else
 			e_cal_component_set_new_vtype (comp, E_CAL_COMPONENT_EVENT);
 		e_cal_component_set_uid (comp, key);
 		cbedata->deletes = g_list_prepend (cbedata->deletes, e_cal_component_get_as_string (comp));
-		
-		e_xmlhash_remove (cbedata->ehash, key);
 		g_object_unref (comp);	
+		return TRUE;
 	}
+	return FALSE;
 }
 
 /* Attachments */
@@ -1650,7 +1707,7 @@ get_changes (ECalBackendSync *backend, E
 	cbedata.kind = e_cal_backend_get_kind (E_CAL_BACKEND (cbex));
 	cbedata.deletes = NULL;
 	cbedata.ehash = ehash;
-	e_xmlhash_foreach_key (ehash, (EXmlHashFunc)e_cal_backend_exchange_compute_changes_foreach_key, &cbedata);
+	e_xmlhash_foreach_key_remove (ehash, (EXmlHashRemoveFunc)e_cal_backend_exchange_compute_changes_foreach_key, &cbedata);
 	
 	*deletes = cbedata.deletes;