From ea26e6ddefca9b5f75f3d7485e420729bc038852 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Tue, 5 Jun 2018 13:04:15 +0200 Subject: [PATCH 01/14] google: Do not create .desktop files for native files Currently, .desktop file with a link to the original location is provided, when a native file is opened because Google doesn't provide its proprietary formats and GLocalFile doesn't know G_FILE_TYPE_SHORTCUT. It is a bit tricky and confusing, among other because the .desktop files aren't automatically converted back to native files when writing. What is worse, Nautilus has dropped support for .desktop files recently, so you see "There was an error launching the application." instead of the requested file. It doesn't make more sense to provide this .desktop files, so let's remove this fallback and return error instead. https://gitlab.gnome.org/GNOME/nautilus/issues/448 --- daemon/gvfsbackendgoogle.c | 68 ++++++++------------------------------ 1 file changed, 13 insertions(+), 55 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 23bd8f58..7bea4bff 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -907,7 +907,6 @@ build_file_info (GVfsBackendGoogle *self, gchar *escaped_name = NULL; gchar *content_type = NULL; gchar *copy_name = NULL; - gchar *generated_copy_name = NULL; gint64 atime; gint64 ctime; gint64 mtime; @@ -1025,21 +1024,7 @@ build_file_info (GVfsBackendGoogle *self, g_file_info_set_display_name (info, title); g_file_info_set_edit_name (info, title); - generated_copy_name = generate_copy_name (self, entry); - - /* While copying remote Drive content to local storage, we want to - * create Link-type desktop files because GLocalFile doesn't know - * about shortcuts. That might change in future. - */ - if (file_type == G_FILE_TYPE_SHORTCUT) - { - copy_name = g_strconcat (generated_copy_name, ".desktop", NULL); - } - else - { - copy_name = generated_copy_name; - generated_copy_name = NULL; - } + copy_name = generate_copy_name (self, entry); /* Sanitize copy-name by replacing slashes with dashes. This is * what nautilus does (for desktop files). @@ -1097,7 +1082,6 @@ build_file_info (GVfsBackendGoogle *self, out: g_free (copy_name); - g_free (generated_copy_name); g_free (escaped_name); g_free (content_type); g_list_free (links); @@ -2249,6 +2233,8 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, GError *error; gchar *content_type = NULL; gchar *entry_path = NULL; + GDataAuthorizationDomain *auth_domain; + const gchar *uri; g_rec_mutex_lock (&self->mutex); g_debug ("+ open_for_read: %s\n", filename); @@ -2278,47 +2264,19 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, goto out; } - /* While copying remote Drive content to local storage, we want to - * create Link-type desktop files because GLocalFile doesn't know - * about shortcuts. That might change in future. - */ - if (g_str_has_prefix (content_type, CONTENT_TYPE_PREFIX_GOOGLE)) + if (is_native_file (entry)) { - GDataLink *alternate; - GKeyFile *file; - const gchar *title; - const gchar *uri; - gchar *data; - gsize length; - - file = g_key_file_new (); - - title = gdata_entry_get_title (entry); - g_key_file_set_string (file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_NAME, title); - g_key_file_set_string (file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_TYPE, "Link"); - - alternate = gdata_entry_look_up_link (entry, GDATA_LINK_ALTERNATE); - uri = gdata_link_get_uri (alternate); - g_key_file_set_string (file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_URL, uri); - - data = g_key_file_to_data (file, &length, NULL); - stream = g_memory_input_stream_new_from_data (data, (gssize) length, g_free); - - g_key_file_free (file); + g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_NOT_REGULAR_FILE, _("File is not a regular file")); + goto out; } - else - { - GDataAuthorizationDomain *auth_domain; - const gchar *uri; - auth_domain = gdata_documents_service_get_primary_authorization_domain (); - uri = gdata_entry_get_content_uri (entry); - stream = gdata_download_stream_new (GDATA_SERVICE (self->service), auth_domain, uri, cancellable); - if (stream == NULL) - { - g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_FAILED, _("Error getting data from file")); - goto out; - } + auth_domain = gdata_documents_service_get_primary_authorization_domain (); + uri = gdata_entry_get_content_uri (entry); + stream = gdata_download_stream_new (GDATA_SERVICE (self->service), auth_domain, uri, cancellable); + if (stream == NULL) + { + g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_FAILED, _("Error getting data from file")); + goto out; } g_object_set_data_full (G_OBJECT (stream), "g-vfs-backend-google-entry", g_object_ref (entry), g_object_unref); -- 2.36.1 From fec7d2162966c3574226564b62c6dd252bf4706f Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 23 Jul 2018 13:39:33 +0200 Subject: [PATCH 02/14] google: Drop the (GDestroyNotify) cast "warning: function called through a non-compatible type" is printed by GCC 8 because of (GDestroyNotfiy) cast in g_clear_pointer, see for more info: https://gitlab.gnome.org/GNOME/glib/issues/1425. Let's drop the (GDestroyNotify) cast in order to prevent those warnings. The cast was not needed anyway. --- daemon/gvfsbackendgoogle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 7bea4bff..2e96cca4 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -2866,8 +2866,8 @@ g_vfs_backend_google_dispose (GObject *_self) g_clear_object (&self->service); g_clear_object (&self->root); g_clear_object (&self->client); - g_clear_pointer (&self->entries, (GDestroyNotify) g_hash_table_unref); - g_clear_pointer (&self->dir_entries, (GDestroyNotify) g_hash_table_unref); + g_clear_pointer (&self->entries, g_hash_table_unref); + g_clear_pointer (&self->dir_entries, g_hash_table_unref); G_OBJECT_CLASS (g_vfs_backend_google_parent_class)->dispose (_self); } -- 2.36.1 From 2d5fc426acb1bff385258d860ec97ff30242285d Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Wed, 1 Aug 2018 13:43:16 +0200 Subject: [PATCH 03/14] google: Move debug prints before releasing entry Debug output contains mess because id and title are const gchar * and are released together with GDataEntry on previous line. Let's just swap the lines. --- daemon/gvfsbackendgoogle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 2e96cca4..e157458b 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -510,13 +510,13 @@ remove_entry (GVfsBackendGoogle *self, parent_id = get_parent_id (self, entry); k = dir_entries_key_new (id, parent_id); - g_hash_table_remove (self->dir_entries, k); g_debug (" remove_entry: Removed (%s, %s) -> %p\n", id, parent_id, entry); + g_hash_table_remove (self->dir_entries, k); dir_entries_key_free (k); k = dir_entries_key_new (title, parent_id); - g_hash_table_remove (self->dir_entries, k); g_debug (" remove_entry: Removed (%s, %s) -> %p\n", title, parent_id, entry); + g_hash_table_remove (self->dir_entries, k); dir_entries_key_free (k); for (l = self->dir_collisions; l != NULL; l = l->next) -- 2.36.1 From b3f8db69522fdbdc965219e403da6e8e60997907 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Wed, 1 Aug 2018 13:44:59 +0200 Subject: [PATCH 04/14] google: Remove also dir_collisions entries dir_collisions are not properly invalidated if removed entry is on this list. Let's remove the entry also from this list. --- daemon/gvfsbackendgoogle.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index e157458b..e9eaec1f 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -519,6 +519,13 @@ remove_entry (GVfsBackendGoogle *self, g_hash_table_remove (self->dir_entries, k); dir_entries_key_free (k); + l = g_list_find (self->dir_collisions, entry); + if (l != NULL) + { + self->dir_collisions = g_list_remove_link (self->dir_collisions, l); + g_object_unref (entry); + } + for (l = self->dir_collisions; l != NULL; l = l->next) { GDataEntry *colliding_entry = GDATA_ENTRY (l->data); -- 2.36.1 From ab007b1f3215a30c3ef49492cf22e6ef1383b0fd Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Tue, 31 Jul 2018 18:43:44 +0200 Subject: [PATCH 05/14] google: Ignore entries without parents Entries without parents are not shown on the web and there isn't any reason to list them here. Such entries belongs to some web services and we have no control over them. --- daemon/gvfsbackendgoogle.c | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index e9eaec1f..897df61f 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -347,14 +347,6 @@ get_parent_id (GVfsBackendGoogle *self, } } - if (ret_val == NULL) - { - const gchar *root_id; - - root_id = gdata_entry_get_id (self->root); - ret_val = g_strdup (root_id); - } - g_list_free (links); return ret_val; } @@ -903,10 +895,8 @@ build_file_info (GVfsBackendGoogle *self, { GFileType file_type; GList *authors; - GList *links; gboolean is_folder = FALSE; gboolean is_root = FALSE; - gboolean has_parent = FALSE; const gchar *etag; const gchar *id; const gchar *name; @@ -925,9 +915,6 @@ build_file_info (GVfsBackendGoogle *self, if (entry == self->root) is_root = TRUE; - links = gdata_entry_look_up_links (entry, GDATA_LINK_PARENT); - has_parent = (links != NULL); - g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_RENAME, !is_root); g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, is_folder); @@ -936,9 +923,7 @@ build_file_info (GVfsBackendGoogle *self, g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_STANDARD_IS_VOLATILE, is_symlink); g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_TRASH, FALSE); - g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE, !is_root && has_parent); - - g_file_info_set_is_hidden (info, !has_parent); + g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE, !is_root); if (is_folder) { @@ -1091,7 +1076,6 @@ build_file_info (GVfsBackendGoogle *self, g_free (copy_name); g_free (escaped_name); g_free (content_type); - g_list_free (links); } /* ---------------------------------------------------------------------------------------------------- */ -- 2.36.1 From 631f794a4a660f49be9d30744e5c66a1f60da4de Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Tue, 31 Jul 2018 19:08:39 +0200 Subject: [PATCH 06/14] google: Add support for files with multiple parents One entry can have multiple parents. You can create such entry on the web in a pretty simple way, e.g. Ctrl + Drag&Drop. Such entries are currently shown only on one place in the backend. Also the backend rely on get_parent_id() and get_entry_path() functions which are tottaly wrong. Let's introduce get_parent_ids() and resolve entry_path only from given filename. --- daemon/gvfsbackendgoogle.c | 388 ++++++++++++++++--------------------- 1 file changed, 168 insertions(+), 220 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 897df61f..7f812448 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -99,11 +99,13 @@ typedef struct GDataEntry *document; GDataUploadStream *stream; gchar *filename; + gchar *entry_path; } WriteHandle; static GDataEntry *resolve_dir (GVfsBackendGoogle *self, const gchar *filename, gchar **out_basename, + gchar **out_path, GError **error); /* ---------------------------------------------------------------------------------------------------- */ @@ -160,7 +162,7 @@ entries_in_folder_equal (gconstpointer a, gconstpointer b) /* ---------------------------------------------------------------------------------------------------- */ static WriteHandle * -write_handle_new (GDataEntry *document, GDataUploadStream *stream, const gchar *filename) +write_handle_new (GDataEntry *document, GDataUploadStream *stream, const gchar *filename, const gchar *entry_path) { WriteHandle *handle; @@ -177,6 +179,7 @@ write_handle_new (GDataEntry *document, GDataUploadStream *stream, const gchar * } handle->filename = g_strdup (filename); + handle->entry_path = g_strdup (entry_path); return handle; } @@ -192,6 +195,7 @@ write_handle_free (gpointer data) g_clear_object (&handle->document); g_clear_object (&handle->stream); g_free (handle->filename); + g_free (handle->entry_path); g_slice_free (WriteHandle, handle); } @@ -313,13 +317,13 @@ get_content_type_from_entry (GDataEntry *entry) /* ---------------------------------------------------------------------------------------------------- */ -static gchar * -get_parent_id (GVfsBackendGoogle *self, - GDataEntry *entry) +static GList * +get_parent_ids (GVfsBackendGoogle *self, + GDataEntry *entry) { GList *l; GList *links = NULL; - gchar *ret_val = NULL; + GList *ids = NULL; links = gdata_entry_look_up_links (entry, GDATA_LINK_PARENT); for (l = links; l != NULL; l = l->next) @@ -341,68 +345,13 @@ get_parent_id (GVfsBackendGoogle *self, id = uri + uri_prefix_len; if (id[0] != '\0') { - ret_val = g_strdup (uri + uri_prefix_len); - break; + ids = g_list_prepend (ids, g_strdup (id)); } } } g_list_free (links); - return ret_val; -} - -static gchar * -get_entry_path (GVfsBackendGoogle *self, GDataEntry *entry) -{ - GString *path = NULL; - const gchar *base_id; - const gchar *root_id; - gchar *id = NULL; - gchar *ret_val = NULL; - - if (entry == self->root) - { - ret_val = g_strdup ("/"); - goto out; - } - - base_id = gdata_entry_get_id (entry); - path = g_string_new (base_id); - g_string_prepend_c (path, '/'); - - id = get_parent_id (self, entry); - root_id = gdata_entry_get_id (self->root); - - while (id != NULL) - { - GDataEntry *parent_entry; - - /* The root folder itself has an ID, so path can become - * /root/folder1/folder2/file. Instead, we want it to be - * /folder1/folder2/file. - */ - - if (g_strcmp0 (id, root_id) == 0) - break; - - parent_entry = g_hash_table_lookup (self->entries, id); - if (parent_entry == NULL) - goto out; - - g_string_prepend (path, id); - g_string_prepend_c (path, '/'); - - g_free (id); - id = get_parent_id (self, parent_entry); - } - - ret_val = g_strdup (path->str); - - out: - g_free (id); - if (path != NULL) - g_string_free (path, TRUE); - return ret_val; + return ids; } /* ---------------------------------------------------------------------------------------------------- */ @@ -418,62 +367,66 @@ insert_entry_full (GVfsBackendGoogle *self, const gchar *id; const gchar *old_id; const gchar *title; - gchar *parent_id; + GList *parent_ids, *l; id = gdata_entry_get_id (entry); title = gdata_entry_get_title (entry); g_hash_table_insert (self->entries, g_strdup (id), g_object_ref (entry)); - parent_id = get_parent_id (self, entry); + parent_ids = get_parent_ids (self, entry); + for (l = parent_ids; l != NULL; l = l->next) + { + gchar *parent_id = l->data; - k = dir_entries_key_new (id, parent_id); - g_hash_table_insert (self->dir_entries, k, g_object_ref (entry)); - g_debug (" insert_entry: Inserted (%s, %s) -> %p\n", id, parent_id, entry); + k = dir_entries_key_new (id, parent_id); + g_hash_table_insert (self->dir_entries, k, g_object_ref (entry)); + g_debug (" insert_entry: Inserted (%s, %s) -> %p\n", id, parent_id, entry); - k = dir_entries_key_new (title, parent_id); - old_entry = g_hash_table_lookup (self->dir_entries, k); - if (old_entry != NULL) - { - old_id = gdata_entry_get_id (old_entry); - if (g_strcmp0 (old_id, title) == 0) - { - insert_title = FALSE; - } - else + k = dir_entries_key_new (title, parent_id); + old_entry = g_hash_table_lookup (self->dir_entries, k); + if (old_entry != NULL) { - /* If the collision is not due to the title matching the ID - * of an earlier GDataEntry, then it is due to duplicate - * titles. - */ - if (g_strcmp0 (old_id, id) < 0) - insert_title = FALSE; + old_id = gdata_entry_get_id (old_entry); + if (g_strcmp0 (old_id, title) == 0) + { + insert_title = FALSE; + } + else + { + /* If the collision is not due to the title matching the ID + * of an earlier GDataEntry, then it is due to duplicate + * titles. + */ + if (g_strcmp0 (old_id, id) < 0) + insert_title = FALSE; + } } - } - if (insert_title) - { - if (old_entry != NULL && track_dir_collisions) + if (insert_title) { - self->dir_collisions = g_list_prepend (self->dir_collisions, g_object_ref (old_entry)); - g_debug (" insert_entry: Ejected (%s, %s, %s) -> %p\n", old_id, title, parent_id, old_entry); - } + if (old_entry != NULL && track_dir_collisions) + { + self->dir_collisions = g_list_prepend (self->dir_collisions, g_object_ref (old_entry)); + g_debug (" insert_entry: Ejected (%s, %s, %s) -> %p\n", old_id, title, parent_id, old_entry); + } - g_hash_table_insert (self->dir_entries, k, g_object_ref (entry)); - g_debug (" insert_entry: Inserted (%s, %s) -> %p\n", title, parent_id, entry); - } - else - { - if (track_dir_collisions) - { - self->dir_collisions = g_list_prepend (self->dir_collisions, g_object_ref (entry)); - g_debug (" insert_entry: Skipped (%s, %s, %s) -> %p\n", id, title, parent_id, entry); + g_hash_table_insert (self->dir_entries, k, g_object_ref (entry)); + g_debug (" insert_entry: Inserted (%s, %s) -> %p\n", title, parent_id, entry); } + else + { + if (track_dir_collisions) + { + self->dir_collisions = g_list_prepend (self->dir_collisions, g_object_ref (entry)); + g_debug (" insert_entry: Skipped (%s, %s, %s) -> %p\n", id, title, parent_id, entry); + } - dir_entries_key_free (k); + dir_entries_key_free (k); + } } + g_list_free_full (parent_ids, g_free); - g_free (parent_id); return insert_title; } @@ -492,47 +445,50 @@ remove_entry (GVfsBackendGoogle *self, GList *l; const gchar *id; const gchar *title; - gchar *parent_id; + GList *parent_ids, *ll; id = gdata_entry_get_id (entry); title = gdata_entry_get_title (entry); g_hash_table_remove (self->entries, id); - parent_id = get_parent_id (self, entry); - - k = dir_entries_key_new (id, parent_id); - g_debug (" remove_entry: Removed (%s, %s) -> %p\n", id, parent_id, entry); - g_hash_table_remove (self->dir_entries, k); - dir_entries_key_free (k); - - k = dir_entries_key_new (title, parent_id); - g_debug (" remove_entry: Removed (%s, %s) -> %p\n", title, parent_id, entry); - g_hash_table_remove (self->dir_entries, k); - dir_entries_key_free (k); - - l = g_list_find (self->dir_collisions, entry); - if (l != NULL) + parent_ids = get_parent_ids (self, entry); + for (ll = parent_ids; ll != NULL; ll = ll->next) { - self->dir_collisions = g_list_remove_link (self->dir_collisions, l); - g_object_unref (entry); - } + gchar *parent_id = ll->data; - for (l = self->dir_collisions; l != NULL; l = l->next) - { - GDataEntry *colliding_entry = GDATA_ENTRY (l->data); + k = dir_entries_key_new (id, parent_id); + g_debug (" remove_entry: Removed (%s, %s) -> %p\n", id, parent_id, entry); + g_hash_table_remove (self->dir_entries, k); + dir_entries_key_free (k); + + k = dir_entries_key_new (title, parent_id); + g_debug (" remove_entry: Removed (%s, %s) -> %p\n", title, parent_id, entry); + g_hash_table_remove (self->dir_entries, k); + dir_entries_key_free (k); - if (insert_entry_full (self, colliding_entry, FALSE)) + l = g_list_find (self->dir_collisions, entry); + if (l != NULL) { self->dir_collisions = g_list_remove_link (self->dir_collisions, l); - g_debug (" remove_entry: Restored %p\n", colliding_entry); - g_list_free (l); - g_object_unref (colliding_entry); - break; + g_object_unref (entry); } - } - g_free (parent_id); + for (l = self->dir_collisions; l != NULL; l = l->next) + { + GDataEntry *colliding_entry = GDATA_ENTRY (l->data); + + if (insert_entry_full (self, colliding_entry, FALSE)) + { + self->dir_collisions = g_list_remove_link (self->dir_collisions, l); + g_debug (" remove_entry: Restored %p\n", colliding_entry); + g_list_free (l); + g_object_unref (colliding_entry); + break; + } + } + } + g_list_free_full (parent_ids, g_free); } static void @@ -617,6 +573,7 @@ resolve_child (GVfsBackendGoogle *self, static GDataEntry * resolve (GVfsBackendGoogle *self, const gchar *filename, + gchar **out_path, GError **error) { GDataEntry *parent; @@ -627,11 +584,15 @@ resolve (GVfsBackendGoogle *self, if (g_strcmp0 (filename, "/") == 0) { ret_val = self->root; + + if (out_path != NULL) + *out_path = g_strdup ("/"); + goto out; } local_error = NULL; - parent = resolve_dir (self, filename, &basename, &local_error); + parent = resolve_dir (self, filename, &basename, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); @@ -645,6 +606,14 @@ resolve (GVfsBackendGoogle *self, goto out; } + if (out_path != NULL) + { + gchar *tmp; + tmp = g_build_path ("/", *out_path, gdata_entry_get_id (ret_val), NULL); + g_free (*out_path); + *out_path = tmp; + } + out: g_free (basename); return ret_val; @@ -654,6 +623,7 @@ static GDataEntry * resolve_dir (GVfsBackendGoogle *self, const gchar *filename, gchar **out_basename, + gchar **out_path, GError **error) { GDataEntry *parent; @@ -666,7 +636,7 @@ resolve_dir (GVfsBackendGoogle *self, parent_path = g_path_get_dirname (filename); local_error = NULL; - parent = resolve (self, parent_path, &local_error); + parent = resolve (self, parent_path, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); @@ -699,12 +669,13 @@ static GDataEntry * resolve_and_rebuild (GVfsBackendGoogle *self, const gchar *filename, GCancellable *cancellable, + gchar **out_path, GError **error) { GDataEntry *entry; GDataEntry *ret_val = NULL; - entry = resolve (self, filename, NULL); + entry = resolve (self, filename, out_path, NULL); if (entry == NULL) { GError *local_error; @@ -718,7 +689,7 @@ resolve_and_rebuild (GVfsBackendGoogle *self, } local_error = NULL; - entry = resolve (self, filename, &local_error); + entry = resolve (self, filename, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); @@ -737,6 +708,7 @@ resolve_dir_and_rebuild (GVfsBackendGoogle *self, const gchar *filename, GCancellable *cancellable, gchar **out_basename, + gchar **out_path, GError **error) { GDataEntry *parent; @@ -745,7 +717,7 @@ resolve_dir_and_rebuild (GVfsBackendGoogle *self, gchar *basename = NULL; local_error = NULL; - parent = resolve_dir (self, filename, &basename, &local_error); + parent = resolve_dir (self, filename, &basename, out_path, &local_error); if (local_error != NULL) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) @@ -767,7 +739,7 @@ resolve_dir_and_rebuild (GVfsBackendGoogle *self, } local_error = NULL; - parent = resolve_dir (self, filename, &basename, &local_error); + parent = resolve_dir (self, filename, &basename, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); @@ -819,13 +791,12 @@ get_extension_offset (const char *title) } static gchar * -generate_copy_name (GVfsBackendGoogle *self, GDataEntry *entry) +generate_copy_name (GVfsBackendGoogle *self, GDataEntry *entry, const gchar *entry_path) { GDataEntry *existing_entry; GDataEntry *parent; const gchar *id; const gchar *title; - gchar *entry_path = NULL; gchar *extension = NULL; gchar *extension_offset; gchar *ret_val = NULL; @@ -833,11 +804,7 @@ generate_copy_name (GVfsBackendGoogle *self, GDataEntry *entry) title = gdata_entry_get_title (entry); - entry_path = get_entry_path (self, entry); - if (entry_path == NULL) - goto out; - - parent = resolve_dir (self, entry_path, NULL, NULL); + parent = resolve_dir (self, entry_path, NULL, NULL, NULL); if (parent == NULL) goto out; @@ -859,7 +826,6 @@ generate_copy_name (GVfsBackendGoogle *self, GDataEntry *entry) out: if (ret_val == NULL) ret_val = g_strdup (title); - g_free (entry_path); g_free (extension); g_free (title_without_extension); return ret_val; @@ -890,7 +856,7 @@ build_file_info (GVfsBackendGoogle *self, GFileAttributeMatcher *matcher, gboolean is_symlink, const gchar *symlink_name, - const gchar *symlink_target, + const gchar *entry_path, GError **error) { GFileType file_type; @@ -968,7 +934,7 @@ build_file_info (GVfsBackendGoogle *self, file_type = G_FILE_TYPE_SYMBOLIC_LINK; } - g_file_info_set_symlink_target (info, symlink_target); + g_file_info_set_symlink_target (info, entry_path); } if (content_type != NULL) @@ -1016,7 +982,7 @@ build_file_info (GVfsBackendGoogle *self, g_file_info_set_display_name (info, title); g_file_info_set_edit_name (info, title); - copy_name = generate_copy_name (self, entry); + copy_name = generate_copy_name (self, entry, entry_path); /* Sanitize copy-name by replacing slashes with dashes. This is * what nautilus does (for desktop files). @@ -1116,6 +1082,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, gchar *destination_basename = NULL; gchar *entry_path = NULL; goffset size; + gchar *parent_path = NULL; g_rec_mutex_lock (&self->mutex); g_debug ("+ copy: %s -> %s, %d\n", source, destination, flags); @@ -1130,12 +1097,12 @@ g_vfs_backend_google_copy (GVfsBackend *_self, goto out; } - source_entry = resolve (self, source, NULL); + source_entry = resolve (self, source, NULL, NULL); if (source_entry == NULL) needs_rebuild = TRUE; error = NULL; - destination_parent = resolve_dir (self, destination, &destination_basename, &error); + destination_parent = resolve_dir (self, destination, &destination_basename, &parent_path, &error); if (error != NULL) { if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) @@ -1158,7 +1125,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, } error = NULL; - source_entry = resolve (self, source, &error); + source_entry = resolve (self, source, NULL, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1172,7 +1139,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, destination_basename = NULL; error = NULL; - destination_parent = resolve_dir (self, destination, &destination_basename, &error); + destination_parent = resolve_dir (self, destination, &destination_basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1258,7 +1225,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_entry)); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (new_entry)), NULL); g_debug (" new entry path: %s\n", entry_path); insert_entry (self, GDATA_ENTRY (new_entry)); @@ -1277,6 +1244,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, g_clear_object (&new_entry); g_free (destination_basename); g_free (entry_path); + g_free (parent_path); g_debug ("- copy\n"); g_rec_mutex_unlock (&self->mutex); } @@ -1306,7 +1274,7 @@ g_vfs_backend_google_create_dir_monitor (GVfsBackend *_self, } error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &error); + entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1314,7 +1282,6 @@ g_vfs_backend_google_create_dir_monitor (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); g_debug (" entry path: %s\n", entry_path); if (!GDATA_IS_DOCUMENTS_FOLDER (entry)) @@ -1356,7 +1323,7 @@ g_vfs_backend_google_delete (GVfsBackend *_self, g_debug ("+ delete: %s\n", filename); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &error); + entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1364,7 +1331,6 @@ g_vfs_backend_google_delete (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); g_debug (" entry path: %s\n", entry_path); if (entry == self->root) @@ -1420,7 +1386,8 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, GDataEntry *entry; GError *error; GHashTableIter iter; - gchar *entry_path = NULL; + char *parent_path; + char *id; g_rec_mutex_lock (&self->mutex); g_debug ("+ enumerate: %s\n", filename); @@ -1447,7 +1414,7 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, } error = NULL; - entry = resolve (self, filename, &error); + entry = resolve (self, filename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1455,9 +1422,6 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); - g_debug (" entry path: %s\n", entry_path); - if (!GDATA_IS_DOCUMENTS_FOLDER (entry)) { g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY,_("The file is not a directory")); @@ -1466,39 +1430,37 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, g_vfs_job_succeeded (G_VFS_JOB (job)); + /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ + id = g_strdup (gdata_entry_get_id (entry)); + g_hash_table_iter_init (&iter, self->entries); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &entry)) { - gchar *path; + DirEntriesKey *k; - path = get_entry_path (self, entry); - g_debug (" found entry: %s\n", path); - if (path != NULL) + k = dir_entries_key_new (gdata_entry_get_id (entry), id); + if (g_hash_table_contains (self->dir_entries, k)) { - gchar *parent_path; - - parent_path = g_path_get_dirname (path); - if (g_strcmp0 (entry_path, parent_path) == 0) - { - GFileInfo *info; - - info = g_file_info_new (); - build_file_info (self, entry, flags, info, matcher, FALSE, NULL, NULL, NULL); - g_vfs_job_enumerate_add_info (job, info); - g_object_unref (info); - } - - g_free (parent_path); + GFileInfo *info; + gchar *entry_path; + + info = g_file_info_new (); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (entry)), NULL); + build_file_info (self, entry, flags, info, matcher, FALSE, NULL, entry_path, NULL); + g_vfs_job_enumerate_add_info (job, info); + g_object_unref (info); + g_free (entry_path); } - g_free (path); + dir_entries_key_free (k); } g_vfs_job_enumerate_done (job); out: - g_free (entry_path); g_debug ("- enumerate\n"); + g_free (parent_path); + g_free (id); g_rec_mutex_unlock (&self->mutex); } @@ -1532,7 +1494,7 @@ g_vfs_backend_google_make_directory (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &error); + parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1540,7 +1502,6 @@ g_vfs_backend_google_make_directory (GVfsBackend *_self, goto out; } - parent_path = get_entry_path (self, parent); g_debug (" parent path: %s\n", parent_path); summary_entry = g_hash_table_lookup (self->entries, basename); @@ -1574,7 +1535,7 @@ g_vfs_backend_google_make_directory (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_folder)); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (new_folder)), NULL); g_debug (" new entry path: %s\n", entry_path); insert_entry (self, GDATA_ENTRY (new_folder)); @@ -1744,6 +1705,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, const gchar *title; gchar *destination_basename = NULL; gchar *entry_path = NULL; + gchar *parent_path = NULL; goffset size; g_rec_mutex_lock (&self->mutex); @@ -1777,7 +1739,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, } error = NULL; - destination_parent = resolve_dir_and_rebuild (self, destination, cancellable, &destination_basename, &error); + destination_parent = resolve_dir_and_rebuild (self, destination, cancellable, &destination_basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1922,7 +1884,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_document)); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (new_document)), NULL); g_debug (" new entry path: %s\n", entry_path); if (needs_overwrite) @@ -1960,6 +1922,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, g_clear_object (&ostream); g_free (destination_basename); g_free (entry_path); + g_free (parent_path); g_debug ("- push\n"); g_rec_mutex_unlock (&self->mutex); } @@ -2065,7 +2028,7 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, g_debug ("+ query_info: %s, %d\n", filename, flags); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &error); + entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2073,7 +2036,6 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); if (g_strcmp0 (entry_path, filename) != 0) /* volatile */ { is_symlink = TRUE; @@ -2122,8 +2084,8 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, entry = g_object_get_data (G_OBJECT (stream), "g-vfs-backend-google-entry"); filename = g_object_get_data (G_OBJECT (stream), "g-vfs-backend-google-filename"); + entry_path = g_object_get_data (G_OBJECT (stream), "g-vfs-backend-google-entry-path"); - entry_path = get_entry_path (self, entry); if (g_strcmp0 (entry_path, filename) != 0) /* volatile */ { is_symlink = TRUE; @@ -2152,7 +2114,6 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, g_vfs_job_succeeded (G_VFS_JOB (job)); out: - g_free (entry_path); g_free (symlink_name); g_debug ("- query_info_on_read\n"); } @@ -2170,19 +2131,17 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, GError *error; WriteHandle *wh = (WriteHandle *) handle; gboolean is_symlink = FALSE; - gchar *entry_path = NULL; gchar *symlink_name = NULL; g_debug ("+ query_info_on_write: %p\n", handle); - entry_path = get_entry_path (self, wh->document); - if (g_strcmp0 (entry_path, wh->filename) != 0) /* volatile */ + if (g_strcmp0 (wh->entry_path, wh->filename) != 0) /* volatile */ { is_symlink = TRUE; symlink_name = g_path_get_basename (wh->filename); } - g_debug (" entry path: %s (%d)\n", entry_path, is_symlink); + g_debug (" entry path: %s (%d)\n", wh->entry_path, is_symlink); error = NULL; build_file_info (self, @@ -2192,7 +2151,7 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, matcher, is_symlink, symlink_name, - entry_path, + wh->entry_path, &error); if (error != NULL) { @@ -2204,7 +2163,6 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, g_vfs_job_succeeded (G_VFS_JOB (job)); out: - g_free (entry_path); g_free (symlink_name); g_debug ("- query_info_on_write\n"); return TRUE; @@ -2231,7 +2189,7 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, g_debug ("+ open_for_read: %s\n", filename); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &error); + entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2239,7 +2197,6 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); g_debug (" entry path: %s\n", entry_path); if (GDATA_IS_DOCUMENTS_FOLDER (entry)) @@ -2272,6 +2229,7 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, g_object_set_data_full (G_OBJECT (stream), "g-vfs-backend-google-entry", g_object_ref (entry), g_object_unref); g_object_set_data_full (G_OBJECT (stream), "g-vfs-backend-google-filename", g_strdup (filename), g_free); + g_object_set_data_full (G_OBJECT (stream), "g-vfs-backend-google-entry-path", g_strdup (entry_path), g_free); g_vfs_job_open_for_read_set_handle (job, stream); g_vfs_job_open_for_read_set_can_seek (job, TRUE); g_vfs_job_succeeded (G_VFS_JOB (job)); @@ -2419,7 +2377,7 @@ g_vfs_backend_google_set_display_name (GVfsBackend *_self, g_debug ("+ set_display_name: %s, %s\n", filename, display_name); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &error); + entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2427,7 +2385,6 @@ g_vfs_backend_google_set_display_name (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, entry); g_debug (" entry path: %s\n", entry_path); if (entry == self->root) @@ -2492,7 +2449,7 @@ g_vfs_backend_google_create (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &error); + parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2500,7 +2457,6 @@ g_vfs_backend_google_create (GVfsBackend *_self, goto out; } - parent_path = get_entry_path (self, parent); g_debug (" parent path: %s\n", parent_path); existing_entry = resolve_child (self, parent, basename); @@ -2538,13 +2494,13 @@ g_vfs_backend_google_create (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_document)); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (new_document)), NULL); g_debug (" new entry path: %s\n", entry_path); insert_entry (self, GDATA_ENTRY (new_document)); g_hash_table_foreach (self->monitors, emit_create_event, entry_path); - handle = write_handle_new (GDATA_ENTRY (new_document), NULL, filename); + handle = write_handle_new (GDATA_ENTRY (new_document), NULL, filename, entry_path); g_vfs_job_open_for_write_set_handle (job, handle); g_vfs_job_succeeded (G_VFS_JOB (job)); @@ -2602,7 +2558,7 @@ g_vfs_backend_google_replace (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &error); + parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2610,7 +2566,6 @@ g_vfs_backend_google_replace (GVfsBackend *_self, goto out; } - parent_path = get_entry_path (self, parent); g_debug (" parent path: %s\n", parent_path); existing_entry = resolve_child (self, parent, basename); @@ -2639,7 +2594,7 @@ g_vfs_backend_google_replace (GVfsBackend *_self, { const gchar *title; - entry_path = get_entry_path (self, existing_entry); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (existing_entry), NULL); g_debug (" existing entry path: %s\n", entry_path); title = gdata_entry_get_title (existing_entry); @@ -2660,7 +2615,7 @@ g_vfs_backend_google_replace (GVfsBackend *_self, goto out; } - handle = write_handle_new (NULL, stream, filename); + handle = write_handle_new (NULL, stream, filename, entry_path); } else { @@ -2681,13 +2636,13 @@ g_vfs_backend_google_replace (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_document)); + entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (new_document)), NULL); g_debug (" new entry path: %s\n", entry_path); insert_entry (self, GDATA_ENTRY (new_document)); g_hash_table_foreach (self->monitors, emit_create_event, entry_path); - handle = write_handle_new (GDATA_ENTRY (new_document), NULL, filename); + handle = write_handle_new (GDATA_ENTRY (new_document), NULL, filename, entry_path); } g_vfs_job_open_for_write_set_handle (job, handle); @@ -2718,7 +2673,6 @@ g_vfs_backend_google_write (GVfsBackend *_self, GCancellable *cancellable = G_VFS_JOB (job)->cancellable; GError *error; WriteHandle *wh = (WriteHandle *) handle; - gchar *entry_path = NULL; gssize nwrite; g_debug ("+ write: %p\n", handle); @@ -2751,9 +2705,7 @@ g_vfs_backend_google_write (GVfsBackend *_self, } g_debug (" writing to stream: %p\n", wh->stream); - - entry_path = get_entry_path (self, wh->document); - g_debug (" entry path: %s\n", entry_path); + g_debug (" entry path: %s\n", wh->entry_path); error = NULL; nwrite = g_output_stream_write (G_OUTPUT_STREAM (wh->stream), @@ -2768,12 +2720,11 @@ g_vfs_backend_google_write (GVfsBackend *_self, goto out; } - g_hash_table_foreach (self->monitors, emit_changed_event, entry_path); + g_hash_table_foreach (self->monitors, emit_changed_event, wh->entry_path); g_vfs_job_write_set_written_size (job, (gsize) nwrite); g_vfs_job_succeeded (G_VFS_JOB (job)); out: - g_free (entry_path); g_debug ("- write\n"); } @@ -2789,7 +2740,6 @@ g_vfs_backend_google_close_write (GVfsBackend *_self, GDataDocumentsDocument *new_document = NULL; GError *error; WriteHandle *wh = (WriteHandle *) handle; - gchar *entry_path = NULL; g_debug ("+ close_write: %p\n", handle); @@ -2820,18 +2770,16 @@ g_vfs_backend_google_close_write (GVfsBackend *_self, goto out; } - entry_path = get_entry_path (self, GDATA_ENTRY (new_document)); - g_debug (" new entry path: %s\n", entry_path); + g_debug (" new entry path: %s\n", wh->entry_path); remove_entry (self, wh->document); insert_entry (self, GDATA_ENTRY (new_document)); - g_hash_table_foreach (self->monitors, emit_changes_done_event, entry_path); + g_hash_table_foreach (self->monitors, emit_changes_done_event, wh->entry_path); g_vfs_job_succeeded (G_VFS_JOB (job)); out: g_clear_object (&new_document); write_handle_free (wh); - g_free (entry_path); g_debug ("- close_write\n"); } -- 2.36.1 From 77610a76ae671ec340d100791043f7b3de4bbaf4 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Wed, 1 Aug 2018 11:21:45 +0200 Subject: [PATCH 07/14] google: Remove file just from concrete parent Files with multiple parents are currently removed from all parents. This is unexpected and may cause data loss, because it is not obvious that it is one file. Let's remove the file just from requested folder. --- daemon/gvfsbackendgoogle.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 7f812448..45a48079 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -1314,10 +1314,12 @@ g_vfs_backend_google_delete (GVfsBackend *_self, { GVfsBackendGoogle *self = G_VFS_BACKEND_GOOGLE (_self); GCancellable *cancellable = G_VFS_JOB (job)->cancellable; - GDataAuthorizationDomain *auth_domain; GDataEntry *entry; + GDataEntry *parent; + GDataDocumentsEntry *new_entry = NULL; GError *error; gchar *entry_path = NULL; + GList *parent_ids; g_rec_mutex_lock (&self->mutex); g_debug ("+ delete: %s\n", filename); @@ -1331,6 +1333,14 @@ g_vfs_backend_google_delete (GVfsBackend *_self, goto out; } + parent = resolve_dir (self, filename, cancellable, NULL, NULL, &error); + if (error != NULL) + { + g_vfs_job_failed_from_error (G_VFS_JOB (job), error); + g_error_free (error); + goto out; + } + g_debug (" entry path: %s\n", entry_path); if (entry == self->root) @@ -1339,10 +1349,26 @@ g_vfs_backend_google_delete (GVfsBackend *_self, goto out; } - auth_domain = gdata_documents_service_get_primary_authorization_domain (); + /* It has to be removed before the actual call to properly invalidate dir entries. */ + g_object_ref (entry); + remove_entry (self, entry); error = NULL; - gdata_service_delete_entry (GDATA_SERVICE (self->service), auth_domain, entry, cancellable, &error); + + /* gdata_documents_service_remove_entry_from_folder seems doesn't work for one parent. */ + parent_ids = get_parent_ids (self, entry); + if (g_list_length (parent_ids) > 1) + { + new_entry = gdata_documents_service_remove_entry_from_folder (self->service, GDATA_DOCUMENTS_ENTRY (entry), GDATA_DOCUMENTS_FOLDER (parent), cancellable, &error); + } + else + { + GDataAuthorizationDomain *auth_domain; + + auth_domain = gdata_documents_service_get_primary_authorization_domain (); + gdata_service_delete_entry (GDATA_SERVICE (self->service), auth_domain, entry, cancellable, &error); + } + if (error != NULL) { sanitize_error (&error); @@ -1351,11 +1377,14 @@ g_vfs_backend_google_delete (GVfsBackend *_self, goto out; } - remove_entry (self, entry); + if (new_entry) + insert_entry (self, GDATA_ENTRY (new_entry)); g_hash_table_foreach (self->monitors, emit_delete_event, entry_path); g_vfs_job_succeeded (G_VFS_JOB (job)); out: + g_object_unref (entry); + g_clear_object (&new_entry); g_free (entry_path); g_debug ("- delete\n"); g_rec_mutex_unlock (&self->mutex); -- 2.36.1 From 454e38f9a96505ea5ecaa6a95a8658d58caf24a1 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Mon, 30 Jul 2018 16:42:31 +0200 Subject: [PATCH 08/14] google: Rework cache for better performance The backend is totally unusable if you have too many files on your Drive. This happens because the backend preloads the whole Drive's metadata. Let's build the cache incrementaly per folders. As a result, the backend works smoothly regardless of the total number of Drive files, because the total number of transmitted data is significantly reduced. On the other hand, more requests is done to Drive, but the Drive quotas seem big enough. --- daemon/gvfsbackendgoogle.c | 404 +++++++++++++++---------------------- 1 file changed, 166 insertions(+), 238 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 45a48079..bf50fef6 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -55,15 +55,13 @@ struct _GVfsBackendGoogle GVfsBackend parent; GDataDocumentsService *service; GDataEntry *root; - GHashTable *entries; - GHashTable *dir_entries; + GHashTable *entries; /* gchar *entry_id -> GDataEntry */ + GHashTable *dir_entries; /* DirEntriesKey -> GDataEntry */ GHashTable *monitors; GList *dir_collisions; GRecMutex mutex; /* guards cache */ GoaClient *client; - gboolean entries_stale; gchar *account_identity; - guint entries_stale_timeout; }; struct _GVfsBackendGoogleClass @@ -104,6 +102,7 @@ typedef struct static GDataEntry *resolve_dir (GVfsBackendGoogle *self, const gchar *filename, + GCancellable *cancellable, gchar **out_basename, gchar **out_path, GError **error); @@ -434,12 +433,19 @@ static void insert_entry (GVfsBackendGoogle *self, GDataEntry *entry) { + gint64 *timestamp; + + timestamp = g_new (gint64, 1); + *timestamp = g_get_real_time (); + g_object_set_data_full (G_OBJECT (entry), "timestamp", timestamp, g_free); + insert_entry_full (self, entry, TRUE); } static void -remove_entry (GVfsBackendGoogle *self, - GDataEntry *entry) +remove_entry_full (GVfsBackendGoogle *self, + GDataEntry *entry, + gboolean restore_dir_collisions) { DirEntriesKey *k; GList *l; @@ -474,17 +480,20 @@ remove_entry (GVfsBackendGoogle *self, g_object_unref (entry); } - for (l = self->dir_collisions; l != NULL; l = l->next) + if (restore_dir_collisions) { - GDataEntry *colliding_entry = GDATA_ENTRY (l->data); - - if (insert_entry_full (self, colliding_entry, FALSE)) + for (l = self->dir_collisions; l != NULL; l = l->next) { - self->dir_collisions = g_list_remove_link (self->dir_collisions, l); - g_debug (" remove_entry: Restored %p\n", colliding_entry); - g_list_free (l); - g_object_unref (colliding_entry); - break; + GDataEntry *colliding_entry = GDATA_ENTRY (l->data); + + if (insert_entry_full (self, colliding_entry, FALSE)) + { + self->dir_collisions = g_list_remove_link (self->dir_collisions, l); + g_debug (" remove_entry: Restored %p\n", colliding_entry); + g_list_free (l); + g_object_unref (colliding_entry); + break; + } } } } @@ -492,16 +501,85 @@ remove_entry (GVfsBackendGoogle *self, } static void -rebuild_entries (GVfsBackendGoogle *self, - GCancellable *cancellable, - GError **error) +remove_entry (GVfsBackendGoogle *self, + GDataEntry *entry) +{ + remove_entry_full (self, entry, TRUE); +} + +static void +remove_dir (GVfsBackendGoogle *self, + GDataEntry *parent) +{ + GHashTableIter iter; + GDataEntry *entry; + gchar *parent_id; + GList *l; + + /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ + parent_id = g_strdup (gdata_entry_get_id (parent)); + + g_hash_table_iter_init (&iter, self->entries); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &entry)) + { + DirEntriesKey *k; + + k = dir_entries_key_new (gdata_entry_get_id (entry), parent_id); + if (g_hash_table_contains (self->dir_entries, k)) + { + g_object_ref (entry); + g_hash_table_iter_remove (&iter); + remove_entry_full (self, entry, FALSE); + g_object_unref (entry); + } + + dir_entries_key_free (k); + } + + for (l = self->dir_collisions; l != NULL; l = l->next) + { + GDataEntry *colliding_entry = GDATA_ENTRY (l->data); + + if (insert_entry_full (self, colliding_entry, FALSE)) + { + self->dir_collisions = g_list_remove_link (self->dir_collisions, l); + g_debug (" remove_entry: Restored %p\n", colliding_entry); + g_list_free (l); + g_object_unref (colliding_entry); + break; + } + } + + g_free (parent_id); +} + +static gboolean +is_entry_valid (GDataEntry *entry) +{ + gint64 *timestamp; + + timestamp = g_object_get_data (G_OBJECT (entry), "timestamp"); + return (g_get_real_time () - *timestamp < REBUILD_ENTRIES_TIMEOUT * G_USEC_PER_SEC); +} + +static void +rebuild_dir (GVfsBackendGoogle *self, + GDataEntry *parent, + GCancellable *cancellable, + GError **error) { GDataDocumentsFeed *feed = NULL; GDataDocumentsQuery *query = NULL; gboolean succeeded_once = FALSE; + gchar *search; + const gchar *parent_id; + + parent_id = gdata_entry_get_id (parent); - query = gdata_documents_query_new_with_limits (NULL, 1, MAX_RESULTS); + search = g_strdup_printf ("'%s' in parents", parent_id); + query = gdata_documents_query_new_with_limits (search, 1, MAX_RESULTS); gdata_documents_query_set_show_folders (query, TRUE); + g_free (search); while (TRUE) { @@ -515,18 +593,13 @@ rebuild_entries (GVfsBackendGoogle *self, { sanitize_error (&local_error); g_propagate_error (error, local_error); - self->entries_stale = TRUE; goto out; } if (!succeeded_once) { - g_hash_table_remove_all (self->entries); - g_hash_table_remove_all (self->dir_entries); - - g_list_free_full (self->dir_collisions, g_object_unref); - self->dir_collisions = NULL; + remove_dir (self, parent); succeeded_once = TRUE; } @@ -545,8 +618,6 @@ rebuild_entries (GVfsBackendGoogle *self, g_clear_object (&feed); } - self->entries_stale = FALSE; - out: g_clear_object (&feed); g_clear_object (&query); @@ -555,24 +626,48 @@ rebuild_entries (GVfsBackendGoogle *self, /* ---------------------------------------------------------------------------------------------------- */ static GDataEntry * -resolve_child (GVfsBackendGoogle *self, - GDataEntry *parent, - const gchar *basename) +resolve_child (GVfsBackendGoogle *self, + GDataEntry *parent, + const gchar *basename, + GCancellable *cancellable, + GError **error) { DirEntriesKey *k; GDataEntry *entry; const gchar *parent_id; + GError *local_error = NULL; parent_id = gdata_entry_get_id (parent); k = dir_entries_key_new (basename, parent_id); entry = g_hash_table_lookup (self->dir_entries, k); + // TODO: Rebuild only if dir listing is not valid + if (entry == NULL || !is_entry_valid (entry)) + { + rebuild_dir (self, parent, cancellable, &local_error); + if (local_error != NULL) + { + g_propagate_error (error, local_error); + goto out; + } + + entry = g_hash_table_lookup (self->dir_entries, k); + if (entry == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("No such file or directory")); + goto out; + } + } + + out: dir_entries_key_free (k); + return entry; } static GDataEntry * resolve (GVfsBackendGoogle *self, const gchar *filename, + GCancellable *cancellable, gchar **out_path, GError **error) { @@ -581,6 +676,8 @@ resolve (GVfsBackendGoogle *self, GError *local_error; gchar *basename = NULL; + g_assert (filename && filename[0] == '/'); + if (g_strcmp0 (filename, "/") == 0) { ret_val = self->root; @@ -592,17 +689,17 @@ resolve (GVfsBackendGoogle *self, } local_error = NULL; - parent = resolve_dir (self, filename, &basename, out_path, &local_error); + parent = resolve_dir (self, filename, cancellable, &basename, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); goto out; } - ret_val = resolve_child (self, parent, basename); + ret_val = resolve_child (self, parent, basename, cancellable, &local_error); if (ret_val == NULL) { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("No such file or directory")); + g_propagate_error (error, local_error); goto out; } @@ -622,6 +719,7 @@ resolve (GVfsBackendGoogle *self, static GDataEntry * resolve_dir (GVfsBackendGoogle *self, const gchar *filename, + GCancellable *cancellable, gchar **out_basename, gchar **out_path, GError **error) @@ -636,7 +734,7 @@ resolve_dir (GVfsBackendGoogle *self, parent_path = g_path_get_dirname (filename); local_error = NULL; - parent = resolve (self, parent_path, out_path, &local_error); + parent = resolve (self, parent_path, cancellable, out_path, &local_error); if (local_error != NULL) { g_propagate_error (error, local_error); @@ -665,103 +763,6 @@ resolve_dir (GVfsBackendGoogle *self, /* ---------------------------------------------------------------------------------------------------- */ -static GDataEntry * -resolve_and_rebuild (GVfsBackendGoogle *self, - const gchar *filename, - GCancellable *cancellable, - gchar **out_path, - GError **error) -{ - GDataEntry *entry; - GDataEntry *ret_val = NULL; - - entry = resolve (self, filename, out_path, NULL); - if (entry == NULL) - { - GError *local_error; - - local_error = NULL; - rebuild_entries (self, cancellable, &local_error); - if (local_error != NULL) - { - g_propagate_error (error, local_error); - goto out; - } - - local_error = NULL; - entry = resolve (self, filename, out_path, &local_error); - if (local_error != NULL) - { - g_propagate_error (error, local_error); - goto out; - } - } - - ret_val = entry; - - out: - return ret_val; -} - -static GDataEntry * -resolve_dir_and_rebuild (GVfsBackendGoogle *self, - const gchar *filename, - GCancellable *cancellable, - gchar **out_basename, - gchar **out_path, - GError **error) -{ - GDataEntry *parent; - GDataEntry *ret_val = NULL; - GError *local_error; - gchar *basename = NULL; - - local_error = NULL; - parent = resolve_dir (self, filename, &basename, out_path, &local_error); - if (local_error != NULL) - { - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) - { - g_propagate_error (error, local_error); - goto out; - } - else - { - g_error_free (local_error); - } - - local_error = NULL; - rebuild_entries (self, cancellable, &local_error); - if (local_error != NULL) - { - g_propagate_error (error, local_error); - goto out; - } - - local_error = NULL; - parent = resolve_dir (self, filename, &basename, out_path, &local_error); - if (local_error != NULL) - { - g_propagate_error (error, local_error); - goto out; - } - } - - if (out_basename != NULL) - { - *out_basename = basename; - basename = NULL; - } - - ret_val = parent; - - out: - g_free (basename); - return ret_val; -} - -/* ---------------------------------------------------------------------------------------------------- */ - static char * get_extension_offset (const char *title) { @@ -804,11 +805,11 @@ generate_copy_name (GVfsBackendGoogle *self, GDataEntry *entry, const gchar *ent title = gdata_entry_get_title (entry); - parent = resolve_dir (self, entry_path, NULL, NULL, NULL); + parent = resolve_dir (self, entry_path, NULL, NULL, NULL, NULL); if (parent == NULL) goto out; - existing_entry = resolve_child (self, parent, title); + existing_entry = resolve_child (self, parent, title, NULL, NULL); if (existing_entry == entry) goto out; @@ -1074,8 +1075,6 @@ g_vfs_backend_google_copy (GVfsBackend *_self, GDataEntry *source_entry; GError *error; GType source_entry_type; - gboolean needs_rebuild = FALSE; - gboolean destination_not_directory = FALSE; const gchar *etag; const gchar *id; const gchar *summary; @@ -1097,56 +1096,21 @@ g_vfs_backend_google_copy (GVfsBackend *_self, goto out; } - source_entry = resolve (self, source, NULL, NULL); - if (source_entry == NULL) - needs_rebuild = TRUE; - error = NULL; - destination_parent = resolve_dir (self, destination, &destination_basename, &parent_path, &error); + source_entry = resolve (self, source, cancellable, NULL, &error); if (error != NULL) { - if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) - destination_not_directory = TRUE; - else - needs_rebuild = TRUE; - + g_vfs_job_failed_from_error (G_VFS_JOB (job), error); g_error_free (error); + goto out; } - if (needs_rebuild) + destination_parent = resolve_dir (self, destination, cancellable, &destination_basename, &parent_path, &error); + if (error != NULL) { - error = NULL; - rebuild_entries (self, cancellable, &error); - if (error != NULL) - { - g_vfs_job_failed_from_error (G_VFS_JOB (job), error); - g_error_free (error); - goto out; - } - - error = NULL; - source_entry = resolve (self, source, NULL, &error); - if (error != NULL) - { - g_vfs_job_failed_from_error (G_VFS_JOB (job), error); - g_error_free (error); - goto out; - } - - if (!destination_not_directory) - { - g_free (destination_basename); - destination_basename = NULL; - - error = NULL; - destination_parent = resolve_dir (self, destination, &destination_basename, &parent_path, &error); - if (error != NULL) - { - g_vfs_job_failed_from_error (G_VFS_JOB (job), error); - g_error_free (error); - goto out; - } - } + g_vfs_job_failed_from_error (G_VFS_JOB (job), error); + g_error_free (error); + goto out; } etag = gdata_entry_get_etag (source_entry); @@ -1165,13 +1129,7 @@ g_vfs_backend_google_copy (GVfsBackend *_self, goto out; } - if (destination_not_directory) - { - g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY, _("The file is not a directory")); - goto out; - } - - existing_entry = resolve_child (self, destination_parent, destination_basename); + existing_entry = resolve_child (self, destination_parent, destination_basename, cancellable, NULL); if (existing_entry != NULL) { if (flags & G_FILE_COPY_OVERWRITE) @@ -1274,7 +1232,7 @@ g_vfs_backend_google_create_dir_monitor (GVfsBackend *_self, } error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); + entry = resolve (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1325,7 +1283,7 @@ g_vfs_backend_google_delete (GVfsBackend *_self, g_debug ("+ delete: %s\n", filename); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); + entry = resolve (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1392,17 +1350,6 @@ g_vfs_backend_google_delete (GVfsBackend *_self, /* ---------------------------------------------------------------------------------------------------- */ -static gboolean -rebuild_entries_timeout_cb (gpointer user_data) -{ - GVfsBackendGoogle *self = G_VFS_BACKEND_GOOGLE (user_data); - - self->entries_stale = TRUE; - self->entries_stale_timeout = 0; - - return G_SOURCE_REMOVE; -} - static void g_vfs_backend_google_enumerate (GVfsBackend *_self, GVfsJobEnumerate *job, @@ -1421,29 +1368,8 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, g_rec_mutex_lock (&self->mutex); g_debug ("+ enumerate: %s\n", filename); - if (self->entries_stale_timeout == 0) - { - self->entries_stale_timeout = g_timeout_add_seconds_full (G_PRIORITY_DEFAULT, - REBUILD_ENTRIES_TIMEOUT, - rebuild_entries_timeout_cb, - g_object_ref (self), - g_object_unref); - } - - if (self->entries_stale) - { - error = NULL; - rebuild_entries (self, cancellable, &error); - if (error != NULL) - { - g_vfs_job_failed_from_error (G_VFS_JOB (job), error); - g_error_free (error); - goto out; - } - } - error = NULL; - entry = resolve (self, filename, &parent_path, &error); + entry = resolve (self, filename, cancellable, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1457,6 +1383,15 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, goto out; } + // TODO: Rebuild only if dir listing is not valid + rebuild_dir (self, entry, cancellable, &error); + if (error != NULL) + { + g_vfs_job_failed_from_error (G_VFS_JOB (job), error); + g_error_free (error); + goto out; + } + g_vfs_job_succeeded (G_VFS_JOB (job)); /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ @@ -1523,7 +1458,7 @@ g_vfs_backend_google_make_directory (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); + parent = resolve_dir (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1539,7 +1474,7 @@ g_vfs_backend_google_make_directory (GVfsBackend *_self, else summary = gdata_entry_get_summary (summary_entry); - existing_entry = resolve_child (self, parent, basename); + existing_entry = resolve_child (self, parent, basename, cancellable, NULL); if (existing_entry != NULL) { g_vfs_job_failed (G_VFS_JOB (job), G_IO_ERROR, G_IO_ERROR_EXISTS, _("Target file already exists")); @@ -1768,7 +1703,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, } error = NULL; - destination_parent = resolve_dir_and_rebuild (self, destination, cancellable, &destination_basename, &parent_path, &error); + destination_parent = resolve_dir (self, destination, cancellable, &destination_basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -1776,7 +1711,7 @@ g_vfs_backend_google_push (GVfsBackend *_self, goto out; } - existing_entry = resolve_child (self, destination_parent, destination_basename); + existing_entry = resolve_child (self, destination_parent, destination_basename, cancellable, NULL); if (existing_entry != NULL) { if (flags & G_FILE_COPY_OVERWRITE) @@ -2057,7 +1992,7 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, g_debug ("+ query_info: %s, %d\n", filename, flags); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); + entry = resolve (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2218,7 +2153,7 @@ g_vfs_backend_google_open_for_read (GVfsBackend *_self, g_debug ("+ open_for_read: %s\n", filename); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); + entry = resolve (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2406,7 +2341,7 @@ g_vfs_backend_google_set_display_name (GVfsBackend *_self, g_debug ("+ set_display_name: %s, %s\n", filename, display_name); error = NULL; - entry = resolve_and_rebuild (self, filename, cancellable, &entry_path, &error); + entry = resolve (self, filename, cancellable, &entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2478,7 +2413,7 @@ g_vfs_backend_google_create (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); + parent = resolve_dir (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2488,7 +2423,7 @@ g_vfs_backend_google_create (GVfsBackend *_self, g_debug (" parent path: %s\n", parent_path); - existing_entry = resolve_child (self, parent, basename); + existing_entry = resolve_child (self, parent, basename, cancellable, NULL); if (existing_entry != NULL) { if (flags & G_FILE_CREATE_REPLACE_DESTINATION) @@ -2587,7 +2522,7 @@ g_vfs_backend_google_replace (GVfsBackend *_self, } error = NULL; - parent = resolve_dir_and_rebuild (self, filename, cancellable, &basename, &parent_path, &error); + parent = resolve_dir (self, filename, cancellable, &basename, &parent_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2597,7 +2532,7 @@ g_vfs_backend_google_replace (GVfsBackend *_self, g_debug (" parent path: %s\n", parent_path); - existing_entry = resolve_child (self, parent, basename); + existing_entry = resolve_child (self, parent, basename, cancellable, NULL); if (existing_entry != NULL) { if (GDATA_IS_DOCUMENTS_FOLDER (existing_entry)) @@ -2819,12 +2754,6 @@ g_vfs_backend_google_dispose (GObject *_self) { GVfsBackendGoogle *self = G_VFS_BACKEND_GOOGLE (_self); - if (self->entries_stale_timeout != 0) - { - g_source_remove (self->entries_stale_timeout); - self->entries_stale_timeout = 0; - } - if (self->dir_collisions != NULL) { g_list_free_full (self->dir_collisions, g_object_unref); @@ -2896,5 +2825,4 @@ g_vfs_backend_google_init (GVfsBackendGoogle *self) g_object_unref); self->monitors = g_hash_table_new (NULL, NULL); g_rec_mutex_init (&self->mutex); - self->entries_stale = TRUE; } -- 2.36.1 From 8c8bc7a8c37bbf1493b14cd76b21b077136e3260 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Thu, 16 Aug 2018 15:00:52 +0200 Subject: [PATCH 09/14] google: Use cache for enumeration also The reworked cache hasn't been used for enumeration results and also for missing files checks, which always caused rebuilding cache. Let's save timestamps also for enumerations and use it to prevent redundant cache rebuilds. --- daemon/gvfsbackendgoogle.c | 62 ++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index bf50fef6..4eb8dbfe 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -57,6 +57,7 @@ struct _GVfsBackendGoogle GDataEntry *root; GHashTable *entries; /* gchar *entry_id -> GDataEntry */ GHashTable *dir_entries; /* DirEntriesKey -> GDataEntry */ + GHashTable *dir_timestamps; /* gchar *entry_id -> gint64 *timestamp */ GHashTable *monitors; GList *dir_collisions; GRecMutex mutex; /* guards cache */ @@ -463,6 +464,8 @@ remove_entry_full (GVfsBackendGoogle *self, { gchar *parent_id = ll->data; + g_hash_table_remove (self->dir_timestamps, parent_id); + k = dir_entries_key_new (id, parent_id); g_debug (" remove_entry: Removed (%s, %s) -> %p\n", id, parent_id, entry); g_hash_table_remove (self->dir_entries, k); @@ -519,6 +522,8 @@ remove_dir (GVfsBackendGoogle *self, /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ parent_id = g_strdup (gdata_entry_get_id (parent)); + g_hash_table_remove (self->dir_timestamps, parent_id); + g_hash_table_iter_init (&iter, self->entries); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &entry)) { @@ -562,6 +567,18 @@ is_entry_valid (GDataEntry *entry) return (g_get_real_time () - *timestamp < REBUILD_ENTRIES_TIMEOUT * G_USEC_PER_SEC); } +static gboolean +is_dir_listing_valid (GVfsBackendGoogle *self, GDataEntry *parent) +{ + gint64 *timestamp; + + timestamp = g_hash_table_lookup (self->dir_timestamps, gdata_entry_get_id (parent)); + if (timestamp != NULL) + return (g_get_real_time () - *timestamp < REBUILD_ENTRIES_TIMEOUT * G_USEC_PER_SEC); + + return FALSE; +} + static void rebuild_dir (GVfsBackendGoogle *self, GDataEntry *parent, @@ -572,9 +589,10 @@ rebuild_dir (GVfsBackendGoogle *self, GDataDocumentsQuery *query = NULL; gboolean succeeded_once = FALSE; gchar *search; - const gchar *parent_id; + gchar *parent_id; - parent_id = gdata_entry_get_id (parent); + /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ + parent_id = g_strdup (gdata_entry_get_id (parent)); search = g_strdup_printf ("'%s' in parents", parent_id); query = gdata_documents_query_new_with_limits (search, 1, MAX_RESULTS); @@ -599,8 +617,14 @@ rebuild_dir (GVfsBackendGoogle *self, if (!succeeded_once) { + gint64 *timestamp; + remove_dir (self, parent); + timestamp = g_new (gint64, 1); + *timestamp = g_get_real_time (); + g_hash_table_insert (self->dir_timestamps, g_strdup (parent_id), timestamp); + succeeded_once = TRUE; } @@ -621,6 +645,7 @@ rebuild_dir (GVfsBackendGoogle *self, out: g_clear_object (&feed); g_clear_object (&query); + g_free (parent_id); } /* ---------------------------------------------------------------------------------------------------- */ @@ -640,8 +665,8 @@ resolve_child (GVfsBackendGoogle *self, parent_id = gdata_entry_get_id (parent); k = dir_entries_key_new (basename, parent_id); entry = g_hash_table_lookup (self->dir_entries, k); - // TODO: Rebuild only if dir listing is not valid - if (entry == NULL || !is_entry_valid (entry)) + if ((entry == NULL && !is_dir_listing_valid (self, parent)) || + (entry != NULL && !is_entry_valid (entry))) { rebuild_dir (self, parent, cancellable, &local_error); if (local_error != NULL) @@ -651,11 +676,12 @@ resolve_child (GVfsBackendGoogle *self, } entry = g_hash_table_lookup (self->dir_entries, k); - if (entry == NULL) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("No such file or directory")); - goto out; - } + } + + if (entry == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, _("No such file or directory")); + goto out; } out: @@ -1363,7 +1389,7 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, GError *error; GHashTableIter iter; char *parent_path; - char *id; + char *id = NULL; g_rec_mutex_lock (&self->mutex); g_debug ("+ enumerate: %s\n", filename); @@ -1383,13 +1409,15 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, goto out; } - // TODO: Rebuild only if dir listing is not valid - rebuild_dir (self, entry, cancellable, &error); - if (error != NULL) + if (!is_dir_listing_valid (self, entry)) { - g_vfs_job_failed_from_error (G_VFS_JOB (job), error); - g_error_free (error); - goto out; + rebuild_dir (self, entry, cancellable, &error); + if (error != NULL) + { + g_vfs_job_failed_from_error (G_VFS_JOB (job), error); + g_error_free (error); + goto out; + } } g_vfs_job_succeeded (G_VFS_JOB (job)); @@ -2765,6 +2793,7 @@ g_vfs_backend_google_dispose (GObject *_self) g_clear_object (&self->client); g_clear_pointer (&self->entries, g_hash_table_unref); g_clear_pointer (&self->dir_entries, g_hash_table_unref); + g_clear_pointer (&self->dir_timestamps, g_hash_table_unref); G_OBJECT_CLASS (g_vfs_backend_google_parent_class)->dispose (_self); } @@ -2823,6 +2852,7 @@ g_vfs_backend_google_init (GVfsBackendGoogle *self) entries_in_folder_equal, dir_entries_key_free, g_object_unref); + self->dir_timestamps = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); self->monitors = g_hash_table_new (NULL, NULL); g_rec_mutex_init (&self->mutex); } -- 2.36.1 From 79e1878e1a43e32c0dbce585ea377d0222f85f27 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Wed, 1 Aug 2018 16:17:42 +0200 Subject: [PATCH 10/14] google: Handle child of volatile also as volatile Files in volatile folder should be also marked as volatile. Volatile handling is a bit simplified as a part of this patch also. --- daemon/gvfsbackendgoogle.c | 58 +++++++++++++------------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 4eb8dbfe..3823cfe4 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -881,8 +881,7 @@ build_file_info (GVfsBackendGoogle *self, GFileQueryInfoFlags flags, GFileInfo *info, GFileAttributeMatcher *matcher, - gboolean is_symlink, - const gchar *symlink_name, + const gchar *filename, const gchar *entry_path, GError **error) { @@ -890,6 +889,7 @@ build_file_info (GVfsBackendGoogle *self, GList *authors; gboolean is_folder = FALSE; gboolean is_root = FALSE; + gboolean is_symlink = FALSE; const gchar *etag; const gchar *id; const gchar *name; @@ -897,6 +897,7 @@ build_file_info (GVfsBackendGoogle *self, gchar *escaped_name = NULL; gchar *content_type = NULL; gchar *copy_name = NULL; + gchar *symlink_name = NULL; gint64 atime; gint64 ctime; gint64 mtime; @@ -908,6 +909,12 @@ build_file_info (GVfsBackendGoogle *self, if (entry == self->root) is_root = TRUE; + if (filename != NULL && g_strcmp0 (entry_path, filename) != 0) /* volatile */ + { + is_symlink = TRUE; + symlink_name = g_path_get_basename (filename); + } + g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_RENAME, !is_root); g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, is_folder); @@ -1066,6 +1073,7 @@ build_file_info (GVfsBackendGoogle *self, } out: + g_free (symlink_name); g_free (copy_name); g_free (escaped_name); g_free (content_type); @@ -1435,10 +1443,12 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, { GFileInfo *info; gchar *entry_path; + gchar *child_filename; info = g_file_info_new (); entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (entry)), NULL); - build_file_info (self, entry, flags, info, matcher, FALSE, NULL, entry_path, NULL); + child_filename = g_build_filename (filename, gdata_entry_get_id (GDATA_ENTRY (entry)), NULL); + build_file_info (self, entry, flags, info, matcher, child_filename, entry_path, NULL); g_vfs_job_enumerate_add_info (job, info); g_object_unref (info); g_free (entry_path); @@ -2012,9 +2022,7 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, GCancellable *cancellable = G_VFS_JOB (job)->cancellable; GDataEntry *entry; GError *error; - gboolean is_symlink = FALSE; gchar *entry_path = NULL; - gchar *symlink_name = NULL; g_rec_mutex_lock (&self->mutex); g_debug ("+ query_info: %s, %d\n", filename, flags); @@ -2028,16 +2036,10 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, goto out; } - if (g_strcmp0 (entry_path, filename) != 0) /* volatile */ - { - is_symlink = TRUE; - symlink_name = g_path_get_basename (filename); - } - - g_debug (" entry path: %s (%d)\n", entry_path, is_symlink); + g_debug (" entry path: %s\n", entry_path); error = NULL; - build_file_info (self, entry, flags, info, matcher, is_symlink, symlink_name, entry_path, &error); + build_file_info (self, entry, flags, info, matcher, filename, entry_path, &error); if (error != NULL) { g_vfs_job_failed_from_error (G_VFS_JOB (job), error); @@ -2049,7 +2051,6 @@ g_vfs_backend_google_query_info (GVfsBackend *_self, out: g_free (entry_path); - g_free (symlink_name); g_debug ("- query_info\n"); g_rec_mutex_unlock (&self->mutex); } @@ -2067,10 +2068,8 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, GDataEntry *entry; GError *error; GInputStream *stream = G_INPUT_STREAM (handle); - gboolean is_symlink = FALSE; const gchar *filename; gchar *entry_path = NULL; - gchar *symlink_name = NULL; g_debug ("+ query_info_on_read: %p\n", handle); @@ -2078,13 +2077,7 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, filename = g_object_get_data (G_OBJECT (stream), "g-vfs-backend-google-filename"); entry_path = g_object_get_data (G_OBJECT (stream), "g-vfs-backend-google-entry-path"); - if (g_strcmp0 (entry_path, filename) != 0) /* volatile */ - { - is_symlink = TRUE; - symlink_name = g_path_get_basename (filename); - } - - g_debug (" entry path: %s (%d)\n", entry_path, is_symlink); + g_debug (" entry path: %s\n", entry_path); error = NULL; build_file_info (self, @@ -2092,8 +2085,7 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, G_FILE_QUERY_INFO_NONE, info, matcher, - is_symlink, - symlink_name, + filename, entry_path, &error); if (error != NULL) @@ -2106,7 +2098,6 @@ g_vfs_backend_google_query_info_on_read (GVfsBackend *_self, g_vfs_job_succeeded (G_VFS_JOB (job)); out: - g_free (symlink_name); g_debug ("- query_info_on_read\n"); } @@ -2122,18 +2113,9 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, GVfsBackendGoogle *self = G_VFS_BACKEND_GOOGLE (_self); GError *error; WriteHandle *wh = (WriteHandle *) handle; - gboolean is_symlink = FALSE; - gchar *symlink_name = NULL; g_debug ("+ query_info_on_write: %p\n", handle); - - if (g_strcmp0 (wh->entry_path, wh->filename) != 0) /* volatile */ - { - is_symlink = TRUE; - symlink_name = g_path_get_basename (wh->filename); - } - - g_debug (" entry path: %s (%d)\n", wh->entry_path, is_symlink); + g_debug (" entry path: %s\n", wh->entry_path); error = NULL; build_file_info (self, @@ -2141,8 +2123,7 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, G_FILE_QUERY_INFO_NONE, info, matcher, - is_symlink, - symlink_name, + wh->filename, wh->entry_path, &error); if (error != NULL) @@ -2155,7 +2136,6 @@ g_vfs_backend_google_query_info_on_write (GVfsBackend *_self, g_vfs_job_succeeded (G_VFS_JOB (job)); out: - g_free (symlink_name); g_debug ("- query_info_on_write\n"); return TRUE; } -- 2.36.1 From 46444e0aacd07a152e3d7958dcc263195cb69433 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Fri, 12 Jul 2019 10:35:47 +0200 Subject: [PATCH 11/14] google: Do not enumerate volatile entries if title matches id Currently, the volatile entry is enumerated if its title matches id of another entry. But we don't want to enumerate volatile entries to not confuse our clients. Let's add simple check to prevent this. --- daemon/gvfsbackendgoogle.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 3823cfe4..60d6142f 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -1437,23 +1437,38 @@ g_vfs_backend_google_enumerate (GVfsBackend *_self, while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &entry)) { DirEntriesKey *k; + GDataEntry *child; + gchar *child_id; - k = dir_entries_key_new (gdata_entry_get_id (entry), id); - if (g_hash_table_contains (self->dir_entries, k)) + /* g_strdup() is necessary to prevent segfault because gdata_entry_get_id() calls g_free() */ + child_id = g_strdup (gdata_entry_get_id (entry)); + + k = dir_entries_key_new (child_id, id); + if ((child = g_hash_table_lookup (self->dir_entries, k)) != NULL) { GFileInfo *info; gchar *entry_path; gchar *child_filename; + /* Be sure that we are not matching title of volatile file */ + if (g_strcmp0 (child_id, gdata_entry_get_id (child)) != 0) + { + g_debug ("Skipping %s as it is volatile path for %s\n", child_id, gdata_entry_get_id (child)); + g_free (child_id); + dir_entries_key_free (k); + continue; + } + info = g_file_info_new (); - entry_path = g_build_path ("/", parent_path, gdata_entry_get_id (GDATA_ENTRY (entry)), NULL); - child_filename = g_build_filename (filename, gdata_entry_get_id (GDATA_ENTRY (entry)), NULL); + entry_path = g_build_path ("/", parent_path, child_id, NULL); + child_filename = g_build_filename (filename, child_id, NULL); build_file_info (self, entry, flags, info, matcher, child_filename, entry_path, NULL); g_vfs_job_enumerate_add_info (job, info); g_object_unref (info); g_free (entry_path); } + g_free (child_id); dir_entries_key_free (k); } -- 2.36.1 From 7a6419a1a8702516f82002c4a003eb6468ac5e7b Mon Sep 17 00:00:00 2001 From: Mayank Sharma Date: Thu, 18 Jul 2019 01:18:43 +0530 Subject: [PATCH 12/14] google: Fix issue with stale entries remaining after rename operation Currently, whenever we perform a rename operation, we set the `entry`'s title to new display name, but at the time of removal of this entry from cache, we still use the newer title. Hence, each time rename is done, a single stale entry remains. This commit fixes the issue by reverting back the title to the original display name of `entry` and then performing a removal from cache. Fixes: https://gitlab.gnome.org/GNOME/gvfs/issues/410 --- daemon/gvfsbackendgoogle.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 60d6142f..1147e8f1 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -2380,6 +2380,11 @@ g_vfs_backend_google_set_display_name (GVfsBackend *_self, goto out; } + /* The internal ref count has to be increased before removing the entry since + * remove_entry_full calls g_object_unref(). */ + g_object_ref (entry); + remove_entry (self, entry); + gdata_entry_set_title (entry, display_name); auth_domain = gdata_documents_service_get_primary_authorization_domain (); @@ -2390,14 +2395,15 @@ g_vfs_backend_google_set_display_name (GVfsBackend *_self, sanitize_error (&error); g_vfs_job_failed_from_error (G_VFS_JOB (job), error); g_error_free (error); + g_object_unref (entry); goto out; } - remove_entry (self, entry); insert_entry (self, new_entry); g_hash_table_foreach (self->monitors, emit_attribute_changed_event, entry_path); g_vfs_job_set_display_name_set_new_path (job, entry_path); g_vfs_job_succeeded (G_VFS_JOB (job)); + g_object_unref (entry); out: g_clear_object (&new_entry); -- 2.36.1 From 00d1e161f47adedf2f05d3574beb05d06c76b4c0 Mon Sep 17 00:00:00 2001 From: Mayank Sharma Date: Tue, 23 Jul 2019 09:51:41 +0530 Subject: [PATCH 13/14] google: Fix crashes when deleting if the file isn't found Currently in delete operation, if the entry gets resolved but parent resolution fails, the jump to `out` label (while handling error) will cause the existing entry's ref_count to decrease by 1 (since `out` label calls g_object_unref on entry). We fix the issue by removing g_object_unref from `out` label and suitably unreffing the entry. --- daemon/gvfsbackendgoogle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 1147e8f1..230b89eb 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -1366,6 +1366,7 @@ g_vfs_backend_google_delete (GVfsBackend *_self, sanitize_error (&error); g_vfs_job_failed_from_error (G_VFS_JOB (job), error); g_error_free (error); + g_object_unref (entry); goto out; } @@ -1373,9 +1374,9 @@ g_vfs_backend_google_delete (GVfsBackend *_self, insert_entry (self, GDATA_ENTRY (new_entry)); g_hash_table_foreach (self->monitors, emit_delete_event, entry_path); g_vfs_job_succeeded (G_VFS_JOB (job)); + g_object_unref (entry); out: - g_object_unref (entry); g_clear_object (&new_entry); g_free (entry_path); g_debug ("- delete\n"); -- 2.36.1 From d842aa6c729866343d7a3b0514a6574c01be30ed Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Fri, 29 Jan 2021 14:06:16 +0100 Subject: [PATCH 14/14] google: Increase number of results in batches for better performance Currently, only 50 results are returned in one batch, which means that multiple network requests are needed for folders with a lot files, which makes it slow. I am not sure there was some reason for this limitation earlier (e.g. before the cache rework), however, I don't see any rationals for it currently. Let's increase this to its maximum for better performance. --- daemon/gvfsbackendgoogle.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/daemon/gvfsbackendgoogle.c b/daemon/gvfsbackendgoogle.c index 230b89eb..f97c038c 100644 --- a/daemon/gvfsbackendgoogle.c +++ b/daemon/gvfsbackendgoogle.c @@ -79,8 +79,6 @@ G_DEFINE_TYPE(GVfsBackendGoogle, g_vfs_backend_google, G_VFS_TYPE_BACKEND) #define CONTENT_TYPE_PREFIX_GOOGLE "application/vnd.google-apps" -#define MAX_RESULTS 50 - #define REBUILD_ENTRIES_TIMEOUT 60 /* s */ #define URI_PREFIX "https://www.googleapis.com/drive/v2/files/" @@ -595,7 +593,7 @@ rebuild_dir (GVfsBackendGoogle *self, parent_id = g_strdup (gdata_entry_get_id (parent)); search = g_strdup_printf ("'%s' in parents", parent_id); - query = gdata_documents_query_new_with_limits (search, 1, MAX_RESULTS); + query = gdata_documents_query_new_with_limits (search, 1, G_MAXUINT); gdata_documents_query_set_show_folders (query, TRUE); g_free (search); -- 2.36.1