Re: [Evolution-hackers] Contacts database modifications history

[Tristan Van Berkom <tristan upstairslabs com>]

On Tue, 2014-04-22 at 14:13 +0200, Milan Crha wrote:
> On Tue, 2014-04-22 at 07:28 +0000, Potrola, MateuszX wrote:
> > I have one additional question about updating internal table by this
> > new module, should we add some new external API for EBookSqlite to
> > create/retrieve/modify  backlog entries or the module can operate
> > directly on the database using  instance of sqlite3 struct from
> > EBookSqlite ?
>
>       Hi,
> there is still required to change the EBookSqlite API, to have an access
> to the internal sqlite pointer (thus I'd prefer that all the backlogging
> does your module on the sqlite3 database provided by the EBookSqlite).
> I do not mind how you will do it (can be either a new function, or an
> extra parameter to some signal), I only do not want to expose the
> sqlite3 dependency in the public header, thus the pointer would be used
> as 'void *', rather than 'sqlite3 *'.

Sorry to interject, I don't have time to write such a lengthly email,
but I would like to warn against exposing the database pointer in the
EBookSqlite API to the world.

Handing out access to the internal DB handle seems to be an open door to
a world of trouble (I can envision bug reports going INVALID because
said user has some kind of non-compliant plugin that is messing directly
with the tables *owned* by EBookSqlite, or creating tables that
EBookSqlite might try to create in a future EDS release)... in other
words, EBookSqlite can only ensure the stability/reliability of it's DB
if it can at least insist to be the only API that is ever used to access
the said DB. 

Rather, if you need to record history of changes then perhaps making it
a feature of the EBookSqlite would be nice, if on the other hand you are
only interested in having change notifications deliver the "before" and
"after" state of a contact which changed, then we have all of that
already in the default backend EBookBackendFile.

Note that EBookBackendFile *always* has a copy of the contact before it
was modified when modifying (or even deleting) a contact, this is
because the EBookBackendFile needs to keep track of contact avatars
saved on disk. It would seem that this problem space is somewhat related
and could be coupled together with improved change notifications if the
photo caching on disk code were wrapped up with this change notification
machinery into a module that could be reusable in multiple EDS backends.

I would write more but I have to run, just throwing my 2 cents into the
discussion.

Cheers,
    -Tristan


> My rough idea would be to add two new signals on EBookSqlite:
>
>    gboolean before_add_contacts (EBookSqlite *ebsql,
>                                const GSList *contacts,
>                                const GSList *extras,
>                                gboolean replace,
>                                [void *sqlitedb,]
>                                GCancellable *cancellable,
>                                GError **error);
>
>    gboolean before_remove_contacts (EBookSqlite *ebsql,
>                                   const GSList *uids,
>                                   [void *sqlitedb,]
>                                   GCancellable *cancellable,
>                                   GError **error);
>
> with the default implementation returning TRUE, like when everything
> worked fine. The callbacks should be called in the respective functions.
> There are also more things to be changed, like adding the ESource
> property for EBookSqlite and consolidate transactions appropriately.
>
> This is only a rough idea, you may come with something better, I
> believe.
>       Bye,
>       Milan
>
> _______________________________________________
> evolution-hackers mailing list
> evolution-hackers gnome org
> To change your list options or unsubscribe, visit ...
> https://mail.gnome.org/mailman/listinfo/evolution-hackers