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



On Mon, Apr 18, 2011 at 10:18 PM, Matthew Barnes <mbarnes redhat com> wrote:
> On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote:
>> I just created a new branch 'eclient' in eds [1] where is added my work
>> on the new API which will deprecate EBook/ECal. It is following glib/gio
>> async pattern and, I believe, makes things more coherent.
>
> Thanks for posting this, Milan.  I want to emphasize how important these
> new APIs are and ask everyone to look it over and provide comments.
>
> I had a sneak peek at this over the weekend and jotted some notes.  So
> far I've only reviewed in detail the client-side headers because that's
> what I'm most concerned about getting right.  The rest of it can be
> tweaked and changed as needed -- even the backend APIs are never truly
> frozen.  But the client-side APIs *will* be frozen, since that's what
> other projects will be migrating to and attempting to use.
>
> The new client-side APIs are:
>
> EClient (abstract base class)
> http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-client.h?h=eclient
>
> ECalClient (replaces ECal)
> http://git.gnome.org/browse/evolution-data-server/tree/calendar/libecal/e-cal-client.h?h=eclient
I would like you to a incorporate some change to the free/busy api.
Some servers allow querying free/busy information
for multiple users at a time and the results appear in a iterative
fashion. The freebusy information of some
users are delivered first, while the server keeps fetching other
user's free/busy information. So if we could have he
FreeBusy api such as this, we could leverage those features,

ECalClientFreeBusy
e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t
start, time_t end, const GSList *users, GCancellable *cancellable,
GError **error);
(with a async counter-part)

Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar
*user, GSList *ecalcomps)

The clients could watch for the signal and update the freebusy
information as and when they receive from eds.

- Chenthill.
>
> EBookClient (replaces EBook)
> http://git.gnome.org/browse/evolution-data-server/tree/addressbook/libebook/e-book-client.h?h=eclient
>
> ECredentials (authenication)
> http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-credentials.h?h=eclient
>
>
> I'll split my comments into two posts so this doesn't get too
> overwhelming.  Simple, hopefully non-controversial stuff here and
> meatier topics in a separate post.  Overall I'm pretty happy with
> the APIs.
>
>
> * There's some overlap between the new EClient API and the new
>  ESource API that I'm working on.  Some functions will need to be
>  dropped once the new ESource API is in place, so I don't know if
>  you want to do this now or wait.
>
>  ESourceList is being removed so obviously any functions involving
>  ESourceList will be dropped:
>
>    e_client_util_get_system_source()
>    e_client_util_set_default()
>    e_client_util_get_source_for_uri()
>    e_cal_client_get_sources()
>    e_book_client_get_sources()
>
>  We will no longer refer ESources using URIs.  We will only refer
>  to ESources by their unique ID string.  So the following functions
>  will be dropped:
>
>    e_client_get_uri()
>    e_cal_client_new_from_uri()
>    e_book_client_new_from_uri()
>
>  Default sources will be tracked using the new ESourceRegistry API,
>  so the following functions will be dropped:
>
>    e_cal_client_set_default()
>    e_cal_client_set_default_source()
>    e_book_client_set_default()
>    e_book_client_set_default_source()
>
>  There's a few functions that I think are too trivial to keep in light
>  of the lookup capabilities of ESourceRegistry.  I'd like to keep the
>  EClient APIs as slim as possible initially and drop these functions:
>
>    e_cal_client_new_system()
>
>      Instead do:
>
>      source = e_source_registry_lookup_by_uid (registry, "system");
>      client = e_cal_client_new (source, source_type, error);
>
>    e_cal_client_new_default()
>
>      Instead do:
>
>      source = e_source_registry_get_default_calendar (registry);
>      client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error);
>
>      (similar functions exist for tasks and memos)
>
>    e_book_client_new_system_addressbook()
>
>      Instead do:
>
>      source = e_source_registry_lookup_by_uid (registry, "system");
>      client = e_book_client_new (source, error);
>
>    e_book_client_new_default_addressbook()
>
>      Instead do:
>
>      source = e_source_registry_get_default_address_book (registry);
>      client = e_book_client_new (source, error);
>
>
> * We should use GIO error codes whenever possible, and I see quite a bit
>  of overlap here.  I think following error codes should be dropped:
>
>    E_CAL_CLIENT_ERROR_SUCCESS
>    E_BOOK_CLIENT_ERROR_SUCCESS
>
>      There's no need for an error code for successful operations.
>
>    E_CAL_CLIENT_ERROR_INVALID_ARG
>    E_BOOK_CLIENT_ERROR_INVALID_ARG
>
>      Use G_IO_ERROR_INVALID_ARGUMENT.
>
>    E_CAL_CLIENT_ERROR_BUSY
>    E_BOOK_CLIENT_ERROR_BUSY
>
>      Use G_IO_ERROR_BUSY.
>
>    E_CAL_CLIENT_ERROR_PERMISSION_DENIED
>    E_BOOK_CLIENT_ERROR_PERMISSION_DENIED
>
>      Use G_IO_ERROR_PERMISSION_DENIED.
>
>    E_CAL_CLIENT_ERROR_NOT_SUPPORTED
>    E_CAL_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED
>    E_BOOK_CLIENT_ERROR_NOT_SUPPORTED
>    E_BOOK_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED
>
>      Use G_IO_ERROR_NOT_SUPPORTED.
>
>    E_CAL_CLIENT_ERROR_CANCELLED
>    E_BOOK_CLIENT_ERROR_CANCELLED
>
>      Use G_IO_ERROR_CANCELLED.
>
>    E_CAL_CLIENT_ERROR_INVALID_SERVER_VERSION
>    E_BOOK_CLIENT_ERROR_INVALID_SERVER_VERSION
>
>      I don't think these are necessary anymore since we started
>      using versioned bus names for the addressbook and calendar
>      services.  If there's a client/server version mismatch,
>      the client will get a D-Bus error about it.
>
>    E_CAL_CLIENT_ERROR_DBUS_ERROR
>    E_BOOK_CLIENT_ERROR_DBUS_ERROR
>
>      I expect GDBus will handle D-Bus errors and hand us back a
>      G_IO_ERROR_DBUS_ERROR.
>
>    E_BOOK_CLIENT_ERROR_NO_SPACE
>
>      Use G_IO_ERROR_NO_SPACE.
>
>    E_CAL_CLIENT_ERROR_OTHER_ERROR
>    E_BOOK_CLIENT_ERROR_OTHER_ERROR
>
>      Use G_IO_ERROR_FAILED.
>
>
> * Of the remaining error codes, a number of them in ECalClient and
>  EBookClient can be combined and moved to EClient.  Especially ones
>  related to offline and authentication, since that's what EClient
>  handles.
>
>
> * I would really prefer we use GList instead of GSList throughout the
>  API.  GSList is silly and really should never have been added to GLib,
>  in my opinion.  Most modern GNOME APIs that I've seen prefer GList,
>  and it's a pain in the butt having to convert between the two.
>
>
> * I thought backends could send their own custom error messages now, so
>  are e_cal_client_error_to_string() and e_book_client_error_to_string()
>  still necessary?
>
>
> * In EBookClient, drop the 'const' qualifier from EContact arguments.
>  Trying to use 'const' with GObjects is misguided and pointless.  I've
>  cursed at EIterator many times for this.
>
>
> * Why do we have "get_capabilities" functions in EClient, ECalClient and
>  EBookClient?  Are they different sets of capabilities?  And if getting
>  capabilities involves a D-Bus call then doesn't it need to be async
>  everywhere?
>
>
> That's all I have for now.  I'll save the heavier stuff for a separate
> post.
>
> _______________________________________________
> evolution-hackers mailing list
> evolution-hackers gnome org
> To change your list options or unsubscribe, visit ...
> http://mail.gnome.org/mailman/listinfo/evolution-hackers
>


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