After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 652179 - libebook: Efficient access to change-tracking meta-data
libebook: Efficient access to change-tracking meta-data
Status: RESOLVED DUPLICATE of bug 652172
Product: evolution-data-server
Classification: Platform
Component: Contacts
unspecified
Other Linux
: Normal normal
: ---
Assigned To: evolution-addressbook-maintainers
Evolution QA team
meego
Depends on:
Blocks:
 
 
Reported: 2011-06-09 10:18 UTC by Murray Cumming
Modified: 2011-08-01 12:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch that adds e_book_get_revisioned_contact_uid_list() (37.01 KB, patch)
2011-07-21 02:55 UTC, Tristan Van Berkom
rejected Details | Review

Description Murray Cumming 2011-06-09 10:18:54 UTC
Data synchronization (such as SyncEvolution) only needs to know which items exist (list of local IDs) and which revision of each item is currently stored. Revisions can be identified with a string which changes for each modification. 

If libebook could provide revision data then SyncEvolution would not need to guess at it badly and slowly by comparing all data, keeping a journal or looking at timestamps.

This could probably be made possible by extending the EDS query language with a flag that limits the delivered data to just UID and REV for libebook (REV is in VCard 3.0). We could implement these queries for the local contact backend, rejecting the query with an error for other backends.
Comment 1 Tristan Van Berkom 2011-07-21 02:54:09 UTC
Have a patch for this on 'openismus-work' branch, tried a different
approach than extending the query language, also started some discussion
here:

http://mail.gnome.org/archives/evolution-hackers/2011-July/msg00042.html

The patch and test case commits are as follows:

commit 5089248e732827a84029e1d6cce7d93c08dbc800
Author: Tristan Van Berkom <tristan.van.berkom@gmail.com>
Date:   Wed Jul 20 22:46:23 2011 -0400

    This commit adds e_book_get_revisioned_contact_uid_list().
    
    Like e_book_get_contact_uid_list() except it also pulls a list
    of all the revisions of each coresponding uid.
    
    Patch adds getRevisionedContactUidList gdbus api and adds
    ->get_revisioned_uid_list() on the EBookBackend and
    EBookBackendSync classes.
    
    EBookBackendSync provides a default implementation which uses
    e_book_backend_sync_get_contact_list() to get the appropriate
    uids and revisions.

commit 289893d7fc0dd99cd8ec025a53f975e2fbedd85f
Author: Tristan Van Berkom <tristan.van.berkom@gmail.com>
Date:   Wed Jul 20 22:49:46 2011 -0400

    Added test case showing e_book_get_revisioned_contact_uid_list() at work.
    
    Test case simply asserts that both Sync and Async apis work.
Comment 2 Tristan Van Berkom 2011-07-21 02:55:13 UTC
Created attachment 192348 [details] [review]
Patch that adds e_book_get_revisioned_contact_uid_list()

In patch form.
Comment 3 Milan Crha 2011-07-25 09:00:03 UTC
There is no need for such API change, use of e_book_client_view_set_fields_of_interest() should be sufficient, I suppose. not talking that this API was meant for cases like this. Also, you patch is not against latest master, I'm sorry.
Comment 4 Tristan Van Berkom 2011-07-25 16:13:25 UTC
Thanks for taking a look.

No there is no patch for master yet, this one targets 'meego-eds'
branch and I was hoping to find the right api/approach before porting it
to master.

This set_fields_of_interest() api... is something like the 'requested_fields' argument of e_book_get_book_view() ?

Since I saw that the 'requested_fields' parameter went away completely
in the new EClient apis I only assumed that stripping/chopping/hashing 
up vcards on the server before pushing them to the client was undesirable 
and completely removed from EDS in the new api. (in favor of other
advantages of less vcard processing and effectiveness of lazy vcard 
parsing).

The problem I'm seeing, if you want to use a field-by-field picking 
kindof api is that well... either you have EDataBook doing it in an 
inefficient way generic for all vcards, or you have every backend 
implementing the "vcard surgery" separately (i.e. when the file backend
gets a request for a vCard with only the UID, it creates a vCard from
the UID and thats it, it does not trim up a vcard after pulling the
whole thing out of the DB and creating a whole entire vCard).

Anyway, that was a mouthful, I'm going to get out of bed, eat something
and go read into the set_fields_of_interest() method and how that is
implemented in master... I'll get back with more comments and/or a patch
update.
Comment 5 Tristan Van Berkom 2011-07-25 23:26:25 UTC
Ok brief status at the end of the day:

For master, this bug/issue should be taken care of by the attachment[0]
on bug 652172.

For meego-eds branch, I will probably be making the same logical
merge of patches with bug 652172 and handle this with the e_book_get_book_view()
'requested_fields'... again all in the same patch for meego-eds branch,
probably tomorrow if nobody has any objections.

[0]https://bugzilla.gnome.org/attachment.cgi?id=192642
Comment 6 Milan Crha 2011-07-26 11:59:06 UTC
(In reply to comment #5)
> Ok brief status at the end of the day:
> 
> For master, this bug/issue should be taken care of by the attachment[0]
> on bug 652172.

OK, I tend to close this as a duplicate of bug #652172, to avoid effort and work duplication.

> For meego-eds branch, I will probably be making the same logical
> merge of patches with bug 652172 and handle this with the
> e_book_get_book_view()
> 'requested_fields'... again all in the same patch for meego-eds branch,
> probably tomorrow if nobody has any objections.

meego-eds is up to you, I do not care of that branch. I also didn't read carefully your changes there, but git master has requested_fields argument totally unused, thus I removed it and replaced its functionality with fields-of-interest.
Comment 7 Tristan Van Berkom 2011-07-26 19:56:18 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Ok brief status at the end of the day:
> > 
> > For master, this bug/issue should be taken care of by the attachment[0]
> > on bug 652172.
> 
> OK, I tend to close this as a duplicate of bug #652172, to avoid effort and
> work duplication.
> 
> > For meego-eds branch, I will probably be making the same logical
> > merge of patches with bug 652172 and handle this with the
> > e_book_get_book_view()
> > 'requested_fields'... again all in the same patch for meego-eds branch,
> > probably tomorrow if nobody has any objections.
> 
> meego-eds is up to you, I do not care of that branch. I also didn't read
> carefully your changes there, but git master has requested_fields argument
> totally unused, thus I removed it and replaced its functionality with
> fields-of-interest.

Yes, currently our patch for meego-eds on bug 652172 implements the
'requested_fields' parameter and uses that to filter down to UID-only.

So, probably it's best to do this in meego-eds both using the
'requested_fields' parameter and that should allow a smooth api
transition whenever meego sources choose to pull in EDS master and
use EBookClient[View] apis.

I prefer to keep this bug open just for the time being until I get some
confirmation that the patch on bug 652172 is the right solution.
Comment 8 Tristan Van Berkom 2011-07-28 18:53:08 UTC
Review of attachment 192348 [details] [review]:

Rejecting this patch based on comment:
  https://bugzilla.gnome.org/show_bug.cgi?id=652172#c46

We already have a special case in the meego-eds branch
for UID only vcards and we wont be needing UID+REV from
meego-eds branch (SyncEvolution if I understand correctly
will depend on a later version of EDS for this functionality).
Comment 9 Tristan Van Berkom 2011-07-30 00:18:10 UTC
Closing this as a duplicate of bug 652172 which generally handles 
the 'fields-of-interest'.

(as mentioned in the previous comment, we dont need a special
solution for this in meego-eds branch).

*** This bug has been marked as a duplicate of bug 652172 ***
Comment 10 Murray Cumming 2011-08-01 12:14:37 UTC
Milan, Tristan could use your advice on the equivalent for libecal:
https://bugzilla.gnome.org/show_bug.cgi?id=652180#c1