GNOME Bugzilla – Bug 748253
gdata: Treat the GDataEntry:id as an opaque string
Last modified: 2015-06-03 16:53:59 UTC
We have been assuming that GDataEntry:id is a string with "https://docs.google.com" as the prefix. The ID is stored verbatim as the nao:identifier, and gnome-documents uses the presence of "https://docs.google.com" in nao:identifier to decide whether to create a GoogleDocument instance or something else. This assumption is flawed because we should be treating the ID as an opaque string. In fact, in the new Drive v2 API (see bug 684920), the ID is just an alphanumeric string. I suggest that we add our own prefix to the ID before storing it as nao:identifier. eg., google:drive:xxxxyyyyzzzz. This is what we do for PicasaWeb and OneDrive, and I don't see why we shouldn't be doing it for Drive as well.
Bumping the importance because things will break when we merge the Drive v2 support in libgdata.
Created attachment 302076 [details] [review] gdata: Treat the GDataEntry:id as an opaque string
Created attachment 302077 [details] [review] gdata: Use pre-defined constants
Created attachment 302079 [details] [review] documents: Adapt to changes in nao:identifier syntax for Google items
Review of attachment 302076 [details] [review]: ::: src/gom-gdata-miner.c @@ +83,2 @@ link = gdata_entry_look_up_link (entry, GDATA_LINK_SELF); + identifier = g_strdup_printf ("gd:collection:google:drive:%s", gdata_link_get_uri (link)); I would suggest ‘google:drive:v2:’ just in case the ID format changes again for v3, necessitating another re-index. Also, it would be neater to move this out as a #define.
Review of attachment 302077 [details] [review]: ++
Review of attachment 302079 [details] [review]: ::: src/documents.js @@ +792,3 @@ + const gdata_prefix = "google:drive:"; + let gdata_id = this.identifier.substring(gdata_prefix.length); Moving the ‘google:drive:’ prefix into some kind of const would be good.
Review of attachment 302076 [details] [review]: ::: src/gom-gdata-miner.c @@ +83,2 @@ link = gdata_entry_look_up_link (entry, GDATA_LINK_SELF); + identifier = g_strdup_printf ("gd:collection:google:drive:%s", gdata_link_get_uri (link)); > I would suggest ‘google:drive:v2:’ just in case the ID format changes again for v3, necessitating another re-index. Actually, ignore me, that was a stupid suggestion.
Review of attachment 302076 [details] [review]: Looks good to me as well, with Philip's suggestion of moving the google drive string to a #define
Created attachment 302128 [details] [review] gdata: Treat the GDataEntry:id as an opaque string Added a #define for the prefix
Created attachment 302129 [details] [review] gdata: Use a #define for the PicasaWeb prefix
Created attachment 302132 [details] [review] documents: Remove unused constants
Created attachment 302133 [details] [review] Adapt to changes in nao:identifier syntax for Google items
Review of attachment 302128 [details] [review]: Looks good
Review of attachment 302129 [details] [review]: Looks good
Review of attachment 302132 [details] [review]: Sure
Review of attachment 302133 [details] [review]: Looks good
Review of attachment 302128 [details] [review]: ::: src/gom-gdata-miner.c @@ +31,3 @@ #define MINER_IDENTIFIER "gd:gdata:miner:86ec9bc9-c242-427f-aa19-77b5a2c9b6f0" #define PARENT_LINK_REL "http://schemas.google.com/docs/2007#parent" +#define PREFIX_DRIVE "google:drive:" I would also suggest putting a comment above here explaining briefly why on earth we add out own prefix to all the IDs, so it doesn't get 'optimised out' by someone in future.
Created attachment 302168 [details] [review] gdata: Explain the need for adding our own prefix to the IDs
I forgot to adapt the sharing dialog to the new nao:identifier syntax for Google items. Reopening.
Created attachment 304532 [details] [review] sharing: Adapt to changes in nao:identifier syntax for Google items
Review of attachment 304532 [details] [review]: Looks good to me.
Comment on attachment 304532 [details] [review] sharing: Adapt to changes in nao:identifier syntax for Google items Thanks Philip. Pushed to master, gnome-3-16 and gnome-3-14.