New blist saving API

[Jan Kaluza]

Hi again,
thanks for your responses. I've changed the patch little bit (newest
version is here: http://hanzz.jabbim.cz/abstract_blist_save.diff).
I've added delete_node op and It uses default saving functions if UI
doesn't provide any. I'm still not decided, if it should be plugin or
if it should be part of libpurple. Is there any reason why it is
better to have it as plugin instead of couple of functions in ops?

On Fri, Jun 12, 2009 at 1:31 AM, Paul Aurich<paul at darkrain42.org> wrote:
> On Jun 6, 2009, at 07:30, Sadrul Habib Chowdhury wrote:
>>
>> * Jan Kaluza had this to say on [06 Jun 2009, 13:30:09 +0200]:
>>>
>>> Hi,
>>> some days ago, I've written patch (attached to this email) which
>>> allows applications, which use libpurple, to choose how they want to
>>> save/load blist data (currently stored in blist.xml). The goal is to
>>> make it easy to write for example mysql backend for blist storing
>>> without any change in libpurple code.
>>
>> Cool!
>
> +1
>
>>> About the patch:
>>>
>>> This patch moves all blist.xml read/write functions from blist.c to
>>> special file blistsaving.c for backward compatibility and it also
>>> moves purple_blist_load() and purple_blist_schedule_save() from
>>> blist.h/blist.c to blistsaving.h/blistsaving.c. It adds new
>>> PurpleBlistUiOps function called save_node(PurpleBuddyList *blist,
>>> PurpleBlistNode *node) which is called every time the node is changed
>>> (it replaces all purple_schedule_save() in blist.c) and it's up to UI
>>> to save it how it wants. For backward compatibility, there is
>>> purple_blist_save_callback(PurpleBuddyList *blist, PurpleBlistNode
>>> *node) defined in blistsaving.h, which can be used for save_node op
>>> and simply calls purple_schedule_save().
>
> Instead of relying on the UI op remove indicating the saving code should
> delete the node from the storage backend, I'd prefer an explicit
> "delete_node" op.
>
> In purple_blist_add_buddy (for example), the buddy is "removed" and then
> moved to a new group immediately, which should only require one save
> operation.

I've added delete_node op to the latest version of patch.

>>
>> I wouldn't tie this with the UiOps. A completely independant plugin
>> could, for example, handle the loading of the blist/accounts/prefs etc.
>>
>> Let's have something like this instead:
>>
>> store.c:
>> PurpleStore
>> {
>>        gboolean (*store)(...);
>>        gboolean (*load)(...);
>> };
>>
>> store/blist.c:
>> PurpleStoreBlist
>> {
>>        PurpleStore;
>>        gboolean (*save_node)(PurpleBlistNode *);
>> };
>>
>> store/account.c:
>> PurpleStoreAccount
>> {
>>        PurpleStore;
>>        gboolean (*save_account)(PurpleAccount *);
>> };
>>
>> store/prefs.c:
>> PurpleStorePrefs
>> {
>>        PurpleStore;
>>        gboolean (*save_int)(const char *, int);
>>        ...
>> };
>>
>> .. and so on and so forth for Status, SavedStatus etc. You get the idea.
>
> With this (and any implementation that allows UIs to overload the saving
> operations), how do we support adding new information to the structure being
> saved (imagine if this were implemented and we wanted to add the
> current_error data to PurpleAccount)?
>
> (I've thought about this and don't have a good solution. All I can come up
> with is "require the UI to save and pass in an opaque blob (e.g. the current
> XML under <account/>)", but storing that sort of data in a database makes me
> die a little inside (granted: this would still alleviate Jan's performance
> issues).

Hm, that could be problem :). I think it should be up to UI to choose
what to save and what not to save, because UI is responsible for
loading the data.

>>> This patch also contains changes in Pidgin and Finch to work well with
>>> my new API.
>>>
>>
>> Ideally, I think, it would be better if the change is completely
>> transparent to the client. So there will be no change *necessary* in
>> Pidgin or Finch, although some change for enhanced performance should be
>> OK.
>
> I agree. If the UI doesn't provide the operations, default to the code in
> (here) blistsaving.c.

If the UI doesn't provide save_node op or delete_node op, default
functions will be used.

>>
>> Sadrul
>>
>
> ~Paul
>

Jan "HanzZ" Kaluza