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 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:
 
 
Reported: 2011-12-19 15:25 UTC by David Zeuthen (not reading bugmail)
Modified: 2012-06-04 19:21 UTC
See Also:
GNOME target: ---
GNOME version: ---



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.