Re: [evolution-patches] Patch for suppressing multiple password prompts in Calendar



On Tue, 2004-06-15 at 09:06 +0530, Harish Krishnaswamy wrote:
I cannot agree more that it does look ugly but this cannot put into the
GW specific module because it would not have been loaded yet :)

The code for tracking if I had set up an idle function already - would
have to be added at this very abstraction again.
So?

You have all the info you need to identify the key at that point, so you're not breaking any abstraction layers by using it to optimise the password request.  Particularly since it appears that e-cal is doing the password requesting, not the backend (which seems wrong to me for a few reasons).
And since the open calendar calls are asynchronous, the password dialogs
for both the calendar and todo sources (but both for creating the same
calendar page in the GUI)  would pop out at the same time. And since
both are executed by the same thread - (gtk_main_loop) - i cannot (or i
do not see how) i can make one wait for the other to complete..
(the main loop would pop out the first dialog and while it waits for
input, call the next dialog also)
Well for starters you odn't block both the gui thread and the callee thread waiting for a synchronous reply.  Its actually a pretty risky way to do things, you're asking for deadlocks or other nasty behaviour.  Only the callee thread needs to be blocked for a synchronous call.  But to do it that way the request-auth callback would itself have to return the result via an asynchronous callback. But if the password requester was async, then it gives you a lot more control.
Any clues would be welcome.. 

GW is merely an example of a service provider that supports multiple
services - calendar, todo , addressbook requiring a single
authentication ( Exchange is another - but it plugs into the evolution
system differently with its own views at the front end). The right
approach may be to create separate abstractions for services and
accounts - so a backend provider can declare the services it provides
and let the higher abstractions figure out which e-sources are related
to one another and share a common authentication. As of today, the only
common link between them is the uri.
So i don't see how this patch helps this in any way, since the patch only affects the calendar?

The common link is still enough to identify the password though isn't it?  If you've already got a password requester up for a given service and you get another request, you could simply wait till you got one reply back and send it to both callees.

  I am welcome to suggestions if this could be done better, if you can
help me see how.

Well I guess JP should really be responding to this, he's probably got a better idea of how that code is supposed to be working.

The mailer does things a bit differently, the backend requests the authentication information directly, when it needs it - which has come in handy since you may need to request auth info from other places or in different ways for a given service.  It doesn't need to check for duplicates, but if it was a problem it could pretty easily.

I was just trying to help, i'll leave it to JP and HPJ now.
harish


On Tue, 2004-06-15 at 06:51, Not Zed wrote:
> You can't do this, you can't hardcode a uri prefix into another
> unrelated abstraction layer.  If we did that all over the place we'd
> have an unmaintaintable monolithic mess of spaghetti.
> 
> Can't you just keep track of whether you've already setup an idle
> function for it and wait in that case?
> 
> 
> On Mon, 2004-06-14 at 22:04 +0530, Harish Krishnaswamy wrote: 
> > The patch below suppresses multiple password prompts for GroupWise
> > accounts in the Calendar component and is a fix for the bug #59386.
> > 
> > The Calendar source and the task source of a GroupWise account share the
> > same authentication tokens. The calendar component initiates the
> > asynchronous creation of a cal source as well as a task source (Task
> > summary in the Calendar view) in the same thread. (The gtk_main_loop
> > thread invokes the authentication callback function). 
> > This patch sets up the async_gw_auth_idle_cb function that delays the
> > authentication call for the second source till the first has had the
> > chance to obtain the password.
> > (This function can be used by any backend which provides both calendar
> > and task services as part of a single account).
> > 
> > harish
> > 
> > Index: ChangeLog
> > ===================================================================
> > RCS file: /cvs/gnome/evolution-data-server/calendar/ChangeLog,v
> > retrieving revision 1.290
> > diff -u -p -r1.290 ChangeLog
> > --- ChangeLog	14 Jun 2004 05:53:23 -0000	1.290
> > +++ ChangeLog	14 Jun 2004 16:19:04 -0000
> > @@ -1,4 +1,10 @@
> >  2004-06-14  Harish Krishnaswamy  <kharish novell com>
> > +	
> > +	* libecal/e-cal.c: (async_gw_auth_idle_cb), (async_auth_func_cb):
> > +	Fixes #59386. The calendar component for GW does not prompt for
> > +	password twice (for calendar and todo items).
> > +
> > +2004-06-14  Harish Krishnaswamy  <kharish novell com>
> >  
> > 
> >  	* backends/groupwise/e-cal-backend-groupwise-utils.c:
> > Index: libecal/e-cal.c
> > ===================================================================
> > RCS file: /cvs/gnome/evolution-data-server/calendar/libecal/e-cal.c,v
> > retrieving revision 1.68
> > diff -u -p -r1.68 e-cal.c
> > --- libecal/e-cal.c	2 Jun 2004 16:15:00 -0000	1.68
> > +++ libecal/e-cal.c	14 Jun 2004 16:19:05 -0000
> > @@ -1589,6 +1589,34 @@ typedef struct {
> >  } ECalAsyncData;
> >  
> >  static gboolean
> > +async_gw_auth_idle_cb (gpointer data)
> > +{
> > +	ECalAsyncData *ccad = data;
> > +	static gboolean authenticating = FALSE;
> > +	static gboolean cancelled = FALSE;
> > +
> > +	/* Check if Calendar authentication is already in progress */
> > +	if (!authenticating) {
> > +		/* If calendar authentication had been cancelled, 
> > +		 * todo authentication is considered cancelled as well. */
> > +		if (cancelled)
> > +			return FALSE;
> > +		authenticating = TRUE;
> > +		g_mutex_lock (ccad->mutex);
> > +		ccad->password = ccad->real_auth_func (ccad->ecal, ccad->auth_prompt,
> > ccad->auth_key, ccad->real_auth_user_data);
> > +		if (!ccad->password)
> > +			cancelled = TRUE;
> > +		g_cond_signal (ccad->cond);
> > +		g_mutex_unlock (ccad->mutex);
> > +		authenticating = FALSE;
> > +		return FALSE;
> > +	}
> > +	else 
> > +		return TRUE; /* this would cause the idle thread to be called again
> > and provide
> > +			      a chance for calendar authentication to be completed. */
> > +}
> > +
> > +static gboolean
> >  async_auth_idle_cb (gpointer data)
> >  {
> >  	ECalAsyncData *ccad = data;
> > @@ -1629,7 +1657,10 @@ async_auth_func_cb (ECal *ecal, const ch
> >  	ccad->auth_prompt = prompt;
> >  	ccad->auth_key = key;
> >  
> > -	g_idle_add ((GSourceFunc) async_auth_idle_cb, ccad);
> > +	if (!strncmp (ecal->priv->uri, "groupwise://", strlen
> > ("groupwise://")))
> > +		g_idle_add ((GSourceFunc) async_gw_auth_idle_cb, ccad);
> > +	else
> > +		g_idle_add ((GSourceFunc) async_auth_idle_cb, ccad);
> >  		
> >  	g_mutex_lock (ccad->mutex);
> >  	g_cond_wait (ccad->cond, ccad->mutex);
> > 
> > _______________________________________________
> > evolution-patches mailing list
> > evolution-patches lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-patches
> -- 
> 
> Michael Zucchi <notzed ximian com>
> 
> Ximian Evolution and Free Software
> Developer
_______________________________________________
evolution-patches mailing list
evolution-patches lists ximian com
http://lists.ximian.com/mailman/listinfo/evolution-patches
--
Michael Zucchi <notzed ximian com>

Ximian Evolution and Free Software Developer


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]