Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-20 Thread Milan Crha
On Tue, 2011-05-10 at 12:19 +0200, Milan Crha wrote:
> The e_client_utils_open_new/_finish is still waiting its addition.

Hi all,
so I added that function, it was slightly complicated, as indicated in
the previous email, but it's finally done. Both e_book_backend_open()
and e_cal_backend_open() contain information about "opening phase", how
it works and what should the descendant structure do.

I also added "refresh" method to address books, thus I was able to
define this in EClient itself, as common function.

I've nothing more left here, so I will move the eclient branch to master
on Monday, if there will not be any objection, and then I'll adapt
evolution-exchange, evolution-mapi, evolution-groupwise and will help
with few others to have them compilable against it. Feel free to ping me
if you'll have any question.
Thanks and bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-10 Thread Milan Crha
On Mon, 2011-05-09 at 12:04 -0400, Matthew Barnes wrote:
> On Mon, 2011-05-09 at 17:49 +0200, Milan Crha wrote: 
> > It's just because of (so called) consistency. With merging common error
> > codes into E_CLIENT_ERROR_ "namespace" I realized that checking for
> > particular errors will not be that easy as is now, because one might
> > have two switches, one for domain E_CLIENT_ERROR and the other for
> > E_CAL/BOOK_ERROR. Not a big deal, but still.
> 
> If you're using a switch statement to check for specific GErrors then
> you're doing it wrong.  You should be using g_error_matches() for each
> case, so it doesn't matter if all the error codes are in the same domain
> or they're all in different domains.
> 
> I'd like to get rid of the redundancy.

OK, done on both server and client sides. Those common properties has
macro prefix "CLIENT_", those specific are prefixed with "BOOK_"/"CAL_".
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-10 Thread Milan Crha
On Mon, 2011-05-09 at 17:00 +0200, Milan Crha wrote:
> On Wed, 2011-05-04 at 14:37 +0200, Milan Crha wrote:
> > So here left basically three things,
> >a) merging some API in utils, 

Hi,
I committed a change, introducing libedataserverui/e-client-utils.h/.c

It adds new
   typedef enum {
E_CLIENT_SOURCE_TYPE_CONTACTS,
E_CLIENT_SOURCE_TYPE_EVENTS,
E_CLIENT_SOURCE_TYPE_MEMOS,
E_CLIENT_SOURCE_TYPE_TASKS
   } EClientSourceType;

(I chose this wording, in a sense: EClientSourceType tells you what is
stored behind the ESource. I harmonized this wording with
ECalClientSourceType too.)

The newly added functions are:
e_client_utils_new
e_client_utils_new_from_uri
e_client_utils_new_system
e_client_utils_new_default
e_client_utils_set_default
e_client_utils_set_default_source
e_client_utils_get_sources



The e_client_utils_open_new/_finish is still waiting its addition. Main
reason is that there is no mechanism, at the moment, to distinguish
current backend open mode/state. Right now it can be online and loaded.
It's not much documented, but from its usage I see that:
   online - TRUE, if the backend storage/server is reachable; use to be
set in 'open' backend's method or unset on failed
authentication or similar occasions
   loaded - TRUE, if the 'open' backend's method was called

What I would need is a signal from the backend, "fully-opened", which
may cover authentication too. (The "fully" prefix is rather redundant,
but self explanatory.)

When you look how the async open method for an addressbook is written,
that located in libedataserverui/e-book-auth-util.c, then after usual
"open" call on the EBook it checks whether the associated ESource has
"auth" set, and if so, then it *forces* the "authenticate_user" method
call. I believe it's not correct, it may wait and respond on backend's
authentication request, but because it's not clear in what state the
backend is, with respect of opening and authenticating, then I
understand there was not many options.

This is the interstate issue Matthew mentioned earlier, in another
thread, (I understand it that way at least), where is hard to
differentiate between open-called, authenticating, fully-opened/ready
for queries.

I propose following changes in the backend:
a) add 'opening' boolean flag, which will be managed automatically. It
will be set during 'open' call, and unset only if the storage/server
will be fully connected, thus if the server will require authentication,
then the backend asks for credentials and keeps this flag to TRUE. The
flag will be automatically unset also for cases when the 'open' method
call finishes with an error. During the opening phase every call to the
backend will deny with Busy error, except of close / cancel /
authenticate_user and similar calls.
b) the backend descendants will notify client about "opened" either
within 'open' call itself, for cases where notification is not needed,
or when it successfully connects to the storage/server (including
authentication), by a call to function like
e_book/cal_backend_notify_opened (backend, const GError *error);
The NULL error is meant for 'success'. This will invoke signal on the
client side with a result. The internal 'opening' flag will be updated
accordingly on the e_..._backend_notify_opened() call, which is supposed
to be called every time.
c) it may not matter if the e_..._backend_notify_opened() will be called
before or after the notification about 'open' method call.
d) a new backend property "opening" will be introduced.
e) the "loaded" property might be replaced with "opened" and will be set
to TRUE only after the backend is successfully opened and authenticated
against its storage/server (basically on e_..._backend_notify_opened()
call with error set to NULL).
f) setting 'online' to FALSE will not result in an opened=FALSE if it
was previously opened, though the backend descendant will be able to
unset the 'opened' flag, by calling the e_..._backend_notify_opened()
with the GError set to a detailed reason why it is not opened.
g) On the client side the EClient::open-changed signal will be
introduced, which will have one argument, the GError from the
e_..._backend_notify_opened(), which will be called on each invocation
from the backend itself.

It'll be easy to do an e_client_utils_open_new() properly with above
changes done, but I would like to know opinions from others whether such
change makes sense and if you'll not see any issue with this approach.
If not, then I'll be more than happy to do this change in the EClient
branch.
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-09 Thread Matthew Barnes
On Mon, 2011-05-09 at 17:49 +0200, Milan Crha wrote: 
> It's just because of (so called) consistency. With merging common error
> codes into E_CLIENT_ERROR_ "namespace" I realized that checking for
> particular errors will not be that easy as is now, because one might
> have two switches, one for domain E_CLIENT_ERROR and the other for
> E_CAL/BOOK_ERROR. Not a big deal, but still.

If you're using a switch statement to check for specific GErrors then
you're doing it wrong.  You should be using g_error_matches() for each
case, so it doesn't matter if all the error codes are in the same domain
or they're all in different domains.

I'd like to get rid of the redundancy.

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-09 Thread Milan Crha
On Mon, 2011-05-09 at 17:41 +0200, Patrick Ohly wrote:
> Why duplicate the LOADED/ONLINE/READONLY/CACHE_DIR/CAPABILITIES
> properties? They could be defined as common E_CLIENT_BACKEND_
> properties.

It's just because of (so called) consistency. With merging common error
codes into E_CLIENT_ERROR_ "namespace" I realized that checking for
particular errors will not be that easy as is now, because one might
have two switches, one for domain E_CLIENT_ERROR and the other for
E_CAL/BOOK_ERROR. Not a big deal, but still.

And even it is not the same thing, I didn't want to have some properties
in CLIENT_BACKEND_ "namespace" and some in each respective, having to
hunt in documentation/header files for a complete list of properties.

Maybe doesn't matter at all? It's easy to change it.
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-09 Thread Patrick Ohly
On Mo, 2011-05-09 at 17:00 +0200, Milan Crha wrote:
> On Wed, 2011-05-04 at 14:37 +0200, Milan Crha wrote:
> > So here left basically three things,
> >a) merging some API in utils,
> >b) getting well-known properties,
> >c) setting well-known properties 
> 
>   Hi,
> I just did a commit into the eclient branch with a fix for b) and c).
> There is a little change, I named those functions:
>e_client_get_backend_property_sync
>e_client_set_backend_property_sync
> with their async versions too.
> 
> Well-known properties are:
>   * for a book:
>   BOOK_BACKEND_PROPERTY_LOADED
>   BOOK_BACKEND_PROPERTY_ONLINE
>   BOOK_BACKEND_PROPERTY_READONLY
>   BOOK_BACKEND_PROPERTY_CACHE_DIR
>   BOOK_BACKEND_PROPERTY_CAPABILITIES
>   BOOK_BACKEND_PROPERTY_REQUIRED_FIELDS
>   BOOK_BACKEND_PROPERTY_SUPPORTED_FIELDS
>   BOOK_BACKEND_PROPERTY_SUPPORTED_AUTH_METHODS
> 
>* for a calendar:
>   CAL_BACKEND_PROPERTY_LOADED
>   CAL_BACKEND_PROPERTY_ONLINE
>   CAL_BACKEND_PROPERTY_READONLY
>   CAL_BACKEND_PROPERTY_CACHE_DIR
>   CAL_BACKEND_PROPERTY_CAPABILITIES
>   CAL_BACKEND_PROPERTY_CAL_EMAIL_ADDRESS
>   CAL_BACKEND_PROPERTY_ALARM_EMAIL_ADDRESS
>   CAL_BACKEND_PROPERTY_DEFAULT_OBJECT

Why duplicate the LOADED/ONLINE/READONLY/CACHE_DIR/CAPABILITIES
properties? They could be defined as common E_CLIENT_BACKEND_
properties.

-- 
Bye, Patrick Ohly
--  
patrick.o...@gmx.de
http://www.estamos.de/


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-09 Thread Milan Crha
On Wed, 2011-05-04 at 14:37 +0200, Milan Crha wrote:
> So here left basically three things,
>a) merging some API in utils,
>b) getting well-known properties,
>c) setting well-known properties 

Hi,
I just did a commit into the eclient branch with a fix for b) and c).
There is a little change, I named those functions:
   e_client_get_backend_property_sync
   e_client_set_backend_property_sync
with their async versions too.

Well-known properties are:
  * for a book:
BOOK_BACKEND_PROPERTY_LOADED
BOOK_BACKEND_PROPERTY_ONLINE
BOOK_BACKEND_PROPERTY_READONLY
BOOK_BACKEND_PROPERTY_CACHE_DIR
BOOK_BACKEND_PROPERTY_CAPABILITIES
BOOK_BACKEND_PROPERTY_REQUIRED_FIELDS
BOOK_BACKEND_PROPERTY_SUPPORTED_FIELDS
BOOK_BACKEND_PROPERTY_SUPPORTED_AUTH_METHODS

   * for a calendar:
CAL_BACKEND_PROPERTY_LOADED
CAL_BACKEND_PROPERTY_ONLINE
CAL_BACKEND_PROPERTY_READONLY
CAL_BACKEND_PROPERTY_CACHE_DIR
CAL_BACKEND_PROPERTY_CAPABILITIES
CAL_BACKEND_PROPERTY_CAL_EMAIL_ADDRESS
CAL_BACKEND_PROPERTY_ALARM_EMAIL_ADDRESS
CAL_BACKEND_PROPERTY_DEFAULT_OBJECT

I dropped API functions for those properties, except of
CAL_BACKEND_PROPERTY_DEFAULT_OBJECT, which left there because the API
returned icalcomponent, not a string.

As a side-effect the the get_capabilities EBookClient/ECalClient
functions have been removed.

Default implementation for getter on EBook/CalBackend takes care of
common properties and returns a Not Supported error when it is asked for
a property it doesn't know anything about. The setter's default
implementation returns the Not Supported error always.

I hope it is something close to that what you expected.
Bye,
Milan

P.S.: merging to utils is not done yet

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-04 Thread Patrick Ohly
On Mi, 2011-05-04 at 14:37 +0200, Milan Crha wrote:
>   Hi,
> I'm getting slightly lost what is left and what is under discussion,
> thus please let me summarize things here:
> 
> On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote:
> > First of all, +1 for rethinking the API. I'd like to suggest that
> > besides modernizing the API we also take this opportunity to move more
> > of EBook/ECal into a common core.
> 
> It seems strange to me to have EClientSourceType defined in e-client.h
> and its main usage in other file (remember of compiler dependency
> issue), thus I suggest to keep e-client/e-book-client/e-cal-client as
> they are and rename e-client-authentication.h/.c (which resides in
> libedataserverui) to e-client-utils.h/.c and do common "interfaces"
> here. This is including also e_client_open_new() being defined here.
> This is for client-side code merging.

I still think that "list and open databases" are common operations and
that having different libraries and code for contacts and calendar is a
historic artifact. But let's not get hung up on that and move on without
changing it.

> > Talking of capabilities, I found their usefulness a bit limited
> > because they a) cannot change during the life time of a client and
> > b) they only report "exists/doesn't exit".
> 
> Here was suggested a common e_client_get_property_sync (EClient *client,
> const gchar *in_name, gchar **out_value, ...); function (with its async
> equivalent) (the e_client_get_backend_property_sync is better
> descriptive, less confusing with GObject properties, I guess, though
> it's quite long name). I'm fine with that, I can add it.

Thanks.

> > Setting values should also be allowed. That gives us a way how
> > a client can configure the storage to behave in certain ways.
> > This can be per-database (tweak the actual on-disk storage)
> > or per EClient (modify behavior as part of current session).
> 
> This breaks abstraction, from my point of view. The client may not
> know/expect anything about backends,

That's not quite true in all cases. On an embedded system, a client
might know exactly that it is going to be installed together with a
specific backend and may want to influence that backend without having
to maintain a non-upstream API for it.

> so any property setting from client
> side is not needed.

Another situation would be a writable property that is supported by one
or more backends. Writing in a backend which doesn't support should
trigger an error, but in others it may make sense.

For example, setting the color of a calendar is possible in CalDAV. A
read/write property would make sense to expose that to clients.

>  You can ask for "change" tag, or "last modified"
> tag, but you should not be able to change it from client side in other
> than _add/_modify/_remove way.

"change" and "last modified" would be read-only. But other properties
may be writable. Even if there are none predefined now, adding the API
now is necessary to make it future-proof.

-- 
Bye, Patrick Ohly
--  
patrick.o...@gmx.de
http://www.estamos.de/


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds

2011-05-04 Thread Milan Crha
Hi,
I'm getting slightly lost what is left and what is under discussion,
thus please let me summarize things here:

On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote:
> First of all, +1 for rethinking the API. I'd like to suggest that
> besides modernizing the API we also take this opportunity to move more
> of EBook/ECal into a common core.

It seems strange to me to have EClientSourceType defined in e-client.h
and its main usage in other file (remember of compiler dependency
issue), thus I suggest to keep e-client/e-book-client/e-cal-client as
they are and rename e-client-authentication.h/.c (which resides in
libedataserverui) to e-client-utils.h/.c and do common "interfaces"
here. This is including also e_client_open_new() being defined here.
This is for client-side code merging.

We opened also a question of server-side merging (mainly merge factory
and views into one base object and change only minimal parts (like dbus
service names and backend types) in descendants. I can either postpone
this, or do it before merging to master, though this is another big
step.

> Talking of capabilities, I found their usefulness a bit limited
> because they a) cannot change during the life time of a client and
> b) they only report "exists/doesn't exit".

Here was suggested a common e_client_get_property_sync (EClient *client,
const gchar *in_name, gchar **out_value, ...); function (with its async
equivalent) (the e_client_get_backend_property_sync is better
descriptive, less confusing with GObject properties, I guess, though
it's quite long name). I'm fine with that, I can add it.

> Setting values should also be allowed. That gives us a way how
> a client can configure the storage to behave in certain ways.
> This can be per-database (tweak the actual on-disk storage)
> or per EClient (modify behavior as part of current session).

This breaks abstraction, from my point of view. The client may not
know/expect anything about backends, so any property setting from client
side is not needed. You can ask for "change" tag, or "last modified"
tag, but you should not be able to change it from client side in other
than _add/_modify/_remove way.



So here left basically three things,
   a) merging some API in utils,
   b) getting well-known properties,
   c) setting well-known properties
and all are sorted out, I guess. Thinking of well-known properties, I
would include other already used there too, like the "cache-dir",
"cal-email-address", "alarm-email-address", as these are just
properties, only each has its own API, which is unnecessary.

Is everyone fine with the above? (Note it's just my feeling about result
of the discussions, which can be particularly wrong in any way.)
Thanks and bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers