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 741345 - Paging does not work
Paging does not work
Status: RESOLVED FIXED
Product: libgdata
Classification: Platform
Component: Google Documents service
0.16.x
Other All
: Normal normal
: ---
Assigned To: libgdata-maint
libgdata-maint
Depends on:
Blocks: 739008 741472
 
 
Reported: 2014-12-10 16:53 UTC by Debarshi Ray
Modified: 2014-12-13 10:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reproducer (2.36 KB, application/octet-stream)
2014-12-10 16:53 UTC, Debarshi Ray
  Details
core: Use correct relation URI for next/previous feed links (1.40 KB, patch)
2014-12-12 19:48 UTC, Philip Withnall
none Details | Review
core: Eliminate unnecessary is_async argument from parser functions (13.14 KB, patch)
2014-12-12 19:48 UTC, Philip Withnall
none Details | Review
core: Virtualise feed parsing in GDataService (6.36 KB, patch)
2014-12-12 19:48 UTC, Philip Withnall
none Details | Review
core: Use correct relation URI for next/previous feed links (1.40 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
core: Eliminate unnecessary is_async argument from parser functions (13.14 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
core: Virtualise feed parsing in GDataService (7.14 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
core: Allow the GDataFeed type to be passed to _gdata_feed_new() (3.60 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
core: Explicitly support a final page in GDataQuery pagination (5.64 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
documents: Handle final page pagination in GDataDocumentsService (4.08 KB, patch)
2014-12-12 23:22 UTC, Philip Withnall
committed Details | Review
demos: Add a docs-list demo (4.94 KB, patch)
2014-12-12 23:58 UTC, Philip Withnall
committed Details | Review

Description Debarshi Ray 2014-12-10 16:53:48 UTC
Created attachment 292457 [details]
Reproducer

Trying to page through the contents of my Drive using gdata_documents_query_new_with_limits and gdata_query_next_page returns the same set of results every time.
Comment 1 Philip Withnall 2014-12-12 19:48:18 UTC
Created attachment 292628 [details] [review]
core: Use correct relation URI for next/previous feed links

GDataLink enforces that all relations are proper URIs, prefixing
iana.org to those which aren’t — so we need to do the same when calling
gdata_feed_look_up_link().
Comment 2 Philip Withnall 2014-12-12 19:48:22 UTC
Created attachment 292629 [details] [review]
core: Eliminate unnecessary is_async argument from parser functions

Instead, use g_main_context_invoke(), which guarantees to call the
callback synchronously if the GMainContext is owned, and in an idle
callback otherwise. This is exactly what we want for implementing
callbacks which could come from this thread or a worker thread.
Comment 3 Philip Withnall 2014-12-12 19:48:25 UTC
Created attachment 292630 [details] [review]
core: Virtualise feed parsing in GDataService

Split the bulk of feed parsing out into a virtual method on
GDataService. This will allow subclasses of GDataService to implement
service-specific tweaks to feed parsing, which is required for
pagination handling with Google Documents.

New API:
 • GDataServiceClass.parse_feed

This does not break ABI — it consumes one of the struct’s expansion
slots.
Comment 4 Philip Withnall 2014-12-12 19:49:42 UTC
These are work-in-progress patches. I still need to handle termination of the pagination loop, which still isn’t working in Docs. The first patch should be all you need to get going for the moment though.

I also need to include your reproducer as an example app in libgdata, since it’s pretty swish. Thanks!
Comment 5 Debarshi Ray 2014-12-12 21:23:06 UTC
(In reply to comment #4)
> These are work-in-progress patches. I still need to handle termination of the
> pagination loop, which still isn’t working in Docs. The first patch should be
> all you need to get going for the moment though.

I will try it out and see if I can figure out the termination condition.

> I also need to include your reproducer as an example app in libgdata, since
> it’s pretty swish. Thanks!

Sure, feel free to use it whichever way you want to. I guess I will have to modify it to not perpetually go on in a while loop and instead stop when it is done.
Comment 6 Philip Withnall 2014-12-12 23:22:29 UTC
Created attachment 292645 [details] [review]
core: Use correct relation URI for next/previous feed links

GDataLink enforces that all relations are proper URIs, prefixing
iana.org to those which aren’t — so we need to do the same when calling
gdata_feed_look_up_link().
Comment 7 Philip Withnall 2014-12-12 23:22:34 UTC
Created attachment 292646 [details] [review]
core: Eliminate unnecessary is_async argument from parser functions

Instead, use g_main_context_invoke(), which guarantees to call the
callback synchronously if the GMainContext is owned, and in an idle
callback otherwise. This is exactly what we want for implementing
callbacks which could come from this thread or a worker thread.
Comment 8 Philip Withnall 2014-12-12 23:22:39 UTC
Created attachment 292647 [details] [review]
core: Virtualise feed parsing in GDataService

Split the bulk of feed parsing out into a virtual method on
GDataService. This will allow subclasses of GDataService to implement
service-specific tweaks to feed parsing, which is required for
pagination handling with Google Documents.

New API:
 • GDataServiceClass.parse_feed

This does not break ABI — it consumes one of the struct’s expansion
slots.
Comment 9 Philip Withnall 2014-12-12 23:22:43 UTC
Created attachment 292648 [details] [review]
core: Allow the GDataFeed type to be passed to _gdata_feed_new()

This will be used internally later to support constructing custom empty
feeds for pagination purposes.
Comment 10 Philip Withnall 2014-12-12 23:22:48 UTC
Created attachment 292649 [details] [review]
core: Explicitly support a final page in GDataQuery pagination

The pagination model previously used by GDataQuery was to assume that
the server would return an empty GDataFeed once the final page was
passed by calling gdata_query_next_page().

Unfortunately, the Google Documents servers don’t do that — instead,
they provide a next page URI in each GDataFeed, and don’t provide one on
the final page. GDataQuery did not support this kind of explicit
knowledge about the current page being the final one.

Add support for it with a delightful set of hacky internal functions.
Return a dummy empty feed from GDataService if passed a GDataQuery which
is flagged as having reached the end of its pagination.
Comment 11 Philip Withnall 2014-12-12 23:22:52 UTC
Created attachment 292650 [details] [review]
documents: Handle final page pagination in GDataDocumentsService

The Google Documents servers always return a next page URI in a
GDataFeed unless on the final page of results, in which case no URI is
returned. Pass that information through to the GDataQuery so that
pagination works correctly for Google Documents.
Comment 12 Philip Withnall 2014-12-12 23:38:00 UTC
Attachment 292645 [details] pushed as b74bc65 - core: Use correct relation URI for next/previous feed links
Attachment 292646 [details] pushed as 52e05dd - core: Eliminate unnecessary is_async argument from parser functions
Attachment 292647 [details] pushed as 1e5e699 - core: Virtualise feed parsing in GDataService
Attachment 292648 [details] pushed as 18edbc4 - core: Allow the GDataFeed type to be passed to _gdata_feed_new()
Attachment 292649 [details] pushed as b449554 - core: Explicitly support a final page in GDataQuery pagination
Attachment 292650 [details] pushed as f08951e - documents: Handle final page pagination in GDataDocumentsService
Comment 13 Philip Withnall 2014-12-12 23:42:40 UTC
rishi, would it be OK for me to commit your reproducer (attachment #292457 [details]) as a libgdata demo of how to use GOA and Google Docs? I’d make minor modifications, but it would mostly remain the same. Would you be OK with LGPLv2.1+?
Comment 14 Philip Withnall 2014-12-12 23:58:42 UTC
Created attachment 292651 [details] [review]
demos: Add a docs-list demo

Adapted from a bug report reproducer program by Debarshi Ray, this lists
all the documents in the user’s Google Documents account, getting the
account information from GOA.
Comment 15 Debarshi Ray 2014-12-13 08:19:23 UTC
(In reply to comment #13)
> rishi, would it be OK for me to commit your reproducer (attachment
> #292457 [details]) as a libgdata demo of how to use GOA and Google
> Docs? I’d make minor modifications, but it would mostly remain the
> same. Would you be OK with LGPLv2.1+?

I am fine with LGPLv2.1+. Feel free to do whatever you want with it.
Comment 16 Philip Withnall 2014-12-13 10:50:16 UTC
Great, thanks. Committed!

Attachment 292651 [details] pushed as d848eb3 - demos: Add a docs-list demo