Bug 666535 - Support for SkyDrive / Microsoft Office
Support for SkyDrive / Microsoft Office
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2011-12-19 15:25 UTC by David Zeuthen (not reading bugmail)
Modified: 2012-06-04 19:21 UTC (History)
4 users (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments

Description David Zeuthen (not reading bugmail) 2011-12-19 15:25:05 UTC
Now that we have Windows Live support in GOA, see

 http://people.freedesktop.org/~david/goa-windows-live.png

it would be nice to support this in the Documents application as well (similar to how Google Docs is supported).
Comment 1 David Zeuthen (not reading bugmail) 2011-12-20 17:16:06 UTC
Btw, just stumbled upon this blog entry when going through unread items in Google Reader:

 "What is the API for accessing content on SkyDrive?"
 http://blogs.msdn.com/b/oldnewthing/archive/2011/12/12/10246542.aspx
Comment 2 Cosimo Cecchi 2011-12-20 17:19:44 UTC
Makes sense. I also found this link on the subject, which I am posting here for future reference

http://windowsteamblog.com/windows_live/b/windowslive/archive/2011/12/07/skydrive-apis-for-docs-and-photos-now-ready-to-cloud-enable-apps-on-windows-8-windows-phone-and-more.aspx
Comment 3 Debarshi Ray 2012-05-23 19:52:56 UTC
I have started some work on this:
http://git.gnome.org/browse/gnome-documents/log/?h=wip/skydrive
Comment 4 Debarshi Ray 2012-06-01 23:08:10 UTC
The wip/skydrive branch is ready for review. Should I attach the patches separately? There are 28 of them.
Comment 5 Cosimo Cecchi 2012-06-02 01:09:10 UTC
Thanks Debarshi...no, it won't be necessary - I'll try to have a go at reviewing the branch over the weekend!
Comment 6 Cosimo Cecchi 2012-06-04 13:14:36 UTC
The branch looks mostly good to me; I just have some minor comments below (per-commit)

798b856d5743f84c57e74b0d94298f42d2779a76
+  entries = zpj_skydrive_list_folder_id (job->service,
+                                         folder_id,
+                                         job->cancellable,
+                                         error);
+  if (entries == NULL)
+    return;

You should check for (error != NULL) instead of (entries == NULL) and free it if it's set.

dba58f54ff3a0cbaa9107354d7b2e94e2cf3acc9
+  gd_miner_tracker_sparql_connection_set_triple
+     (job->connection, job->cancellable, error,
+      identifier, resource,
+      "nie:dataSource", datasource_urn);
+
+  g_free (datasource_urn);

Missing error check after this

b2f9863df0adb7af26cc3c5d408f562039874e61
+  gchar *datasource_urn = NULL;
+
+  datasource_urn = g_strconcat ("gd:goa-account:",
+                                goa_account_get_id (job->account),
+                                NULL);

Can you save datasource_urn in AccountMinerJob or make this a helper func instead of g_strconcat()-ing in two places?

1effe8e234b5e779af64003caa34a9ced3c40952
+        // HACK: GJS doesn't support introspecting GTypes, so we need to use
+        // GObject.type_from_name(); but for that to work, we need at least one
+        // instance of the GType in question to have ever been created. Ensure that
+        let temp = new Zpj.SkydriveFile();

This is only needed in the GData implementation because the public API wants a GType, and should not be required for zpj
updateTypeDescription() could probably be shared with the GData implementation (not a blocker though)

664981e9d9c593b65a3a2aaa8379b0dcd0eff1ba
 static void
+pdf_load_job_zpj_refresh_cache (PdfLoadJob *job)
+{
+  GInputStream *stream;
+  GError *error = NULL;

Unused stream and error?
Comment 7 Debarshi Ray 2012-06-04 17:40:42 UTC
(In reply to comment #6)
> The branch looks mostly good to me; I just have some minor comments below
> (per-commit)
> 
> 798b856d5743f84c57e74b0d94298f42d2779a76
> +  entries = zpj_skydrive_list_folder_id (job->service,
> +                                         folder_id,
> +                                         job->cancellable,
> +                                         error);
> +  if (entries == NULL)
> +    return;
> 
> You should check for (error != NULL) instead of (entries == NULL) and free it
> if it's set.

I changed it to check for error and if set I abort. The error is propagated upwards. I also added a similar check to the recursive call to account_miner_job_traverse_folder.

The error handling is currently like this:
+ Absorb failures in account_miner_job_process_entry
+ Propagate failures in zpj_skydrive_list_folder_id

This is so because a failure in account_miner_job_process_entry means that once account_miner_job_cleanup_previous is run, we will still have the stale entries about the existing SkyDrive objects. However, a failure in zpj_skydrive_list_folder_id means cleaning up job->previous_resources can remove entries about existing SkyDrive objects, and hence that step needs to be skipped by propagating the error.

What do you think?

> dba58f54ff3a0cbaa9107354d7b2e94e2cf3acc9
> +  gd_miner_tracker_sparql_connection_set_triple
> +     (job->connection, job->cancellable, error,
> +      identifier, resource,
> +      "nie:dataSource", datasource_urn);
> +
> +  g_free (datasource_urn);
> 
> Missing error check after this

Fixed.

> b2f9863df0adb7af26cc3c5d408f562039874e61
> [...]
> Can you save datasource_urn in AccountMinerJob or make this a helper func
> instead of g_strconcat()-ing in two places?

Fixed. I added it to AccountMinerJob.

> 1effe8e234b5e779af64003caa34a9ced3c40952
> [...]
> This is only needed in the GData implementation because the public API wants a
> GType, and should not be required for zpj
> updateTypeDescription() could probably be shared with the GData implementation
> (not a blocker though)

Fixed.

> 664981e9d9c593b65a3a2aaa8379b0dcd0eff1ba
>  static void
> +pdf_load_job_zpj_refresh_cache (PdfLoadJob *job)
> +{
> +  GInputStream *stream;
> +  GError *error = NULL;
> 
> Unused stream and error?

Fixed.
Comment 8 Debarshi Ray 2012-06-04 19:19:36 UTC
From #documents:

17:41 <rishi> cosimoc: Updated branch.
17:48 <cosimoc> rishi, awesome...your comment in BZ makes sense (the GData      
      miner works the same IIRC); assuming the other commits didn't change,     
      feel free to merge :)
Comment 9 Debarshi Ray 2012-06-04 19:21:56 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

Note You need to log in before you can comment on or make changes to this bug.