Re: [Evolution-hackers] (summarize ][) New 'eclient' branch in eds
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
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
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
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
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
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
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
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
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