[epiphany/wip/sync: 1/31] sync: Correctly detect bookmarks that were deleted from Firefox



commit efc1270b8131028a7d87a7a2cfd0f8d19f3d6123
Author: Gabriel Ivascu <ivascu gabriel59 gmail com>
Date:   Thu Mar 30 16:19:57 2017 +0300

    sync: Correctly detect bookmarks that were deleted from Firefox

 src/bookmarks/ephy-bookmarks-manager.c |   73 +++++++++++++++-----------
 src/sync/ephy-sync-service.c           |   89 +++++++------------------------
 src/sync/ephy-synchronizable-manager.c |   11 ++--
 src/sync/ephy-synchronizable-manager.h |   16 +++---
 src/sync/ephy-synchronizable.c         |   31 ++++++++++-
 src/sync/ephy-synchronizable.h         |    3 +-
 6 files changed, 106 insertions(+), 117 deletions(-)
---
diff --git a/src/bookmarks/ephy-bookmarks-manager.c b/src/bookmarks/ephy-bookmarks-manager.c
index 3702084..11ed3a8 100644
--- a/src/bookmarks/ephy-bookmarks-manager.c
+++ b/src/bookmarks/ephy-bookmarks-manager.c
@@ -306,9 +306,9 @@ synchronizable_manager_remove (EphySynchronizableManager *manager,
 static void
 synchronizable_manager_merge_remotes (EphySynchronizableManager  *manager,
                                       gboolean                    is_initial,
-                                      GList                      *remotes,
-                                      GList                     **to_upload,
-                                      GList                     **to_test)
+                                      GList                      *remotes_deleted,
+                                      GList                      *remotes_updated,
+                                      GList                     **to_upload)
 {
   EphyBookmarksManager *self;
   EphyBookmark *remote;
@@ -318,15 +318,37 @@ synchronizable_manager_merge_remotes (EphySynchronizableManager  *manager,
   GHashTable *handled;
   char *type;
   char *parent_id;
+  const char *id;
 
   self = EPHY_BOOKMARKS_MANAGER (manager);
-  handled = g_hash_table_new (g_direct_hash, g_direct_equal);
+  handled = g_hash_table_new (g_str_hash, g_str_equal);
 
-  if (!remotes)
-    goto handle_local_bookmarks;
+  /* Ignore remote deleted objects if this is an initial sync. */
+  if (is_initial)
+    goto handle_updated;
 
-  for (GList *r = remotes; r && r->data; r = r->next) {
-    remote = EPHY_BOOKMARK (r->data);
+  for (GList *l = remotes_deleted; l && l->data; l = l->next) {
+    remote = EPHY_BOOKMARK (l->data);
+    id = ephy_bookmark_get_id (remote);
+    local = ephy_bookmarks_manager_get_bookmark_by_id (self, id);
+
+    if (!local) {
+      g_object_unref (remote);
+      continue;
+    }
+
+    if (ephy_bookmark_get_modification_time (local) > ephy_bookmark_get_modification_time (remote)) {
+      *to_upload = g_list_prepend (*to_upload, local);
+      g_hash_table_add (handled, (char *)id);
+    } else {
+      ephy_bookmarks_manager_remove_bookmark (self, local);
+    }
+  }
+
+handle_updated:
+  for (GList *l = remotes_updated; l && l->data; l = l->next) {
+    remote = EPHY_BOOKMARK (l->data);
+    id = ephy_bookmark_get_id (remote);
     g_object_get (remote, "type", &type, "parentid", &parent_id, NULL);
 
     /* Ignore mobile/unfiled bookmarks and everything that is not bookmark. */
@@ -338,49 +360,40 @@ synchronizable_manager_merge_remotes (EphySynchronizableManager  *manager,
     if (!ephy_bookmark_get_time_added (remote))
       ephy_bookmark_set_time_added (remote, g_get_real_time ());
 
-    local = ephy_bookmarks_manager_get_bookmark_by_id (self, ephy_bookmark_get_id (remote));
+    local = ephy_bookmarks_manager_get_bookmark_by_id (self, id);
     if (!local) {
-      /* Add remote bookmark, together with its tags (duplicate tags are automatically ignored). */
+      /* Add remote bookmark. */
       ephy_bookmarks_manager_add_bookmark (self, remote);
       ephy_bookmarks_manager_create_tags_from_bookmark (self, remote);
-      g_hash_table_add (handled, remote);
     } else {
       /* Keep the bookmark with most recent modified timestamp. */
       if (ephy_bookmark_get_modification_time (remote) > ephy_bookmark_get_modification_time (local)) {
         ephy_bookmarks_manager_remove_bookmark (self, local);
         ephy_bookmarks_manager_add_bookmark (self, remote);
         ephy_bookmarks_manager_create_tags_from_bookmark (self, remote);
-        g_hash_table_add (handled, remote);
-      } else {
-        if (ephy_bookmark_get_modification_time (local) > ephy_bookmark_get_modification_time (remote))
-          *to_upload = g_list_prepend (*to_upload, local);
-        g_hash_table_add (handled, local);
+      } else if (ephy_bookmark_get_modification_time (local) > ephy_bookmark_get_modification_time (remote)) 
{
+        ephy_bookmarks_manager_create_tags_from_bookmark (self, remote);
+        *to_upload = g_list_prepend (*to_upload, local);
       }
     }
 
+    g_hash_table_add (handled, (char *)id);
 next:
-    g_free(type);
-    g_free(parent_id);
+    g_free (type);
+    g_free (parent_id);
     g_object_unref (remote);
   }
 
-handle_local_bookmarks:
+  /* Upload unhandled bookmarks to server. */
   bookmarks = ephy_bookmarks_manager_get_bookmarks (self);
   for (iter = g_sequence_get_begin_iter (bookmarks);
        !g_sequence_iter_is_end (iter); iter = g_sequence_iter_next (iter)) {
+
     local = EPHY_BOOKMARK (g_sequence_get (iter));
-    if (!g_hash_table_contains (handled, local)) {
-      if (is_initial) {
-        /* In case of a first time sync, upload all remaining locals to server. */
+    id = ephy_bookmark_get_id (local);
+    if (!g_hash_table_contains (handled, id)) {
+      if (is_initial || (!is_initial && !ephy_bookmark_is_uploaded (local)))
         *to_upload = g_list_prepend (*to_upload, local);
-      } else {
-        /* In case of a normal sync, upload only locals that are not uploaded
-         * and remove the rest if they don't exist on the server any longer. */
-        if (!ephy_bookmark_is_uploaded (local))
-          *to_upload = g_list_prepend (*to_upload, local);
-        else
-          *to_test = g_list_prepend (*to_test, local);
-      }
     }
   }
 
diff --git a/src/sync/ephy-sync-service.c b/src/sync/ephy-sync-service.c
index 293f355..dc0caf9 100644
--- a/src/sync/ephy-sync-service.c
+++ b/src/sync/ephy-sync-service.c
@@ -1166,56 +1166,6 @@ ephy_sync_service_do_sign_out (EphySyncService *self)
 }
 
 static void
-test_and_remove_synchronizable_cb (SoupSession *session,
-                                   SoupMessage *msg,
-                                   gpointer     user_data)
-{
-  EphySyncService *service;
-  SyncAsyncData *data;
-
-  data = (SyncAsyncData *)user_data;
-  service = ephy_shell_get_sync_service (ephy_shell_get_default ());
-
-  if (msg->status_code == 404) {
-    LOG ("Removed local object with id %s", ephy_synchronizable_get_id (data->synchronizable));
-    ephy_synchronizable_manager_remove (data->manager, data->synchronizable);
-  } else if (msg->status_code != 200) {
-    g_warning ("Failed to get object from server. Status code: %u, response: %s",
-               msg->status_code, msg->response_body->data);
-  }
-
-  sync_async_data_free (data);
-  ephy_sync_service_send_next_storage_request (service);
-}
-
-static void
-ephy_sync_service_test_and_remove_synchronizable (EphySyncService           *self,
-                                                  EphySynchronizableManager *manager,
-                                                  EphySynchronizable        *synchronizable)
-{
-  SyncAsyncData *data;
-  char *endpoint;
-  const char *collection;
-  const char *id;
-
-  g_assert (EPHY_IS_SYNC_SERVICE (self));
-  g_assert (EPHY_IS_SYNCHRONIZABLE_MANAGER (manager));
-  g_assert (EPHY_IS_SYNCHRONIZABLE (synchronizable));
-
-  id = ephy_synchronizable_get_id (synchronizable);
-  collection = ephy_synchronizable_manager_get_collection_name (manager);
-  endpoint = g_strdup_printf ("storage/%s/%s", collection, id);
-  data = sync_async_data_new (manager, synchronizable);
-
-  LOG ("Testing local object with id %s...", id);
-  ephy_sync_service_queue_storage_request (self, endpoint,
-                                           SOUP_METHOD_GET, NULL, -1, -1,
-                                           test_and_remove_synchronizable_cb, data);
-
-  g_free (endpoint);
-}
-
-static void
 delete_synchronizable_cb (SoupSession *session,
                           SoupMessage *msg,
                           gpointer     user_data)
@@ -1272,6 +1222,7 @@ download_synchronizable_cb (SoupSession *session,
   GError *error = NULL;
   GType type;
   const char *collection;
+  gboolean is_deleted;
 
   data = (SyncAsyncData *)user_data;
   service = ephy_shell_get_sync_service (ephy_shell_get_default ());
@@ -1298,16 +1249,20 @@ download_synchronizable_cb (SoupSession *session,
   type = ephy_synchronizable_manager_get_synchronizable_type (data->manager);
   collection = ephy_synchronizable_manager_get_collection_name (data->manager);
   bundle = ephy_sync_service_get_key_bundle (service, collection);
-  synchronizable = EPHY_SYNCHRONIZABLE (ephy_synchronizable_from_bso (bso, type, bundle));
+  synchronizable = EPHY_SYNCHRONIZABLE (ephy_synchronizable_from_bso (bso, type, bundle, &is_deleted));
   if (!synchronizable) {
     g_warning ("Failed to create synchronizable object from BSO");
     goto free_parser;
   }
 
-  /* Overwrite the local object. */
+  /* Delete the local object and add the remote one if it is not marked as deleted. */
   ephy_synchronizable_manager_remove (data->manager, data->synchronizable);
-  ephy_synchronizable_manager_add (data->manager, synchronizable);
-  LOG ("Successfully downloaded object");
+  if (!is_deleted) {
+    ephy_synchronizable_manager_add (data->manager, synchronizable);
+    LOG ("Successfully downloaded from server");
+  } else {
+    g_object_unref (synchronizable);
+  }
 
   g_object_unref (synchronizable);
 free_parser:
@@ -1425,12 +1380,13 @@ sync_collection_cb (SoupSession *session,
   JsonNode *node;
   JsonObject *object;
   GError *error = NULL;
-  GList *remotes = NULL;
+  GList *remotes_updated = NULL;
+  GList *remotes_deleted = NULL;
   GList *to_upload = NULL;
-  GList *to_test = NULL;
   GType type;
   const char *collection;
   const char *timestamp;
+  gboolean is_deleted;
 
   service = ephy_shell_get_sync_service (ephy_shell_get_default ());
   data = (SyncCollectionAsyncData *)user_data;
@@ -1471,17 +1427,20 @@ sync_collection_cb (SoupSession *session,
       continue;
     }
     object = json_node_get_object (node);
-    remote = EPHY_SYNCHRONIZABLE (ephy_synchronizable_from_bso (object, type, bundle));
+    remote = EPHY_SYNCHRONIZABLE (ephy_synchronizable_from_bso (object, type, bundle, &is_deleted));
     if (!remote) {
       g_warning ("Failed to create synchronizable object from BSO, skipping...");
       continue;
     }
-    remotes = g_list_prepend (remotes, remote);
+    if (is_deleted)
+      remotes_deleted = g_list_prepend (remotes_deleted, remote);
+    else
+      remotes_updated = g_list_prepend (remotes_updated, remote);
   }
 
 merge_remotes:
   ephy_synchronizable_manager_merge_remotes (data->manager, data->is_initial,
-                                             remotes, &to_upload, &to_test);
+                                             remotes_deleted, remotes_updated, &to_upload);
 
   if (to_upload) {
     LOG ("Uploading local objects to server...");
@@ -1491,22 +1450,14 @@ merge_remotes:
     }
   }
 
-  if (to_test) {
-    LOG ("Testing if any local object needs to be removed...");
-    for (GList *l = to_test; l && l->data; l = l->next) {
-      ephy_sync_service_test_and_remove_synchronizable (service, data->manager,
-                                                        EPHY_SYNCHRONIZABLE (l->data));
-    }
-  }
-
   ephy_synchronizable_manager_set_is_initial_sync (data->manager, FALSE);
   /* Update sync time. */
   timestamp = soup_message_headers_get_one (msg->response_headers, "X-Weave-Timestamp");
   ephy_synchronizable_manager_set_sync_time (data->manager, g_ascii_strtod (timestamp, NULL));
 
   g_list_free (to_upload);
-  g_list_free (to_test);
-  g_list_free (remotes);
+  g_list_free (remotes_updated);
+  g_list_free (remotes_deleted);
 free_parser:
   if (parser)
     g_object_unref (parser);
diff --git a/src/sync/ephy-synchronizable-manager.c b/src/sync/ephy-synchronizable-manager.c
index a30d4b4..97c0655 100644
--- a/src/sync/ephy-synchronizable-manager.c
+++ b/src/sync/ephy-synchronizable-manager.c
@@ -133,17 +133,16 @@ ephy_synchronizable_manager_remove (EphySynchronizableManager *manager,
 
 void
 ephy_synchronizable_manager_merge_remotes (EphySynchronizableManager  *manager,
-                                           gboolean                    first_time,
-                                           GList                      *remotes,
-                                           GList                     **to_updload,
-                                           GList                     **to_test)
+                                           gboolean                    is_initial,
+                                           GList                      *remotes_deleted,
+                                           GList                      *remtoes_updated,
+                                           GList                     **to_updload)
 {
   EphySynchronizableManagerInterface *iface;
 
   g_return_if_fail (EPHY_IS_SYNCHRONIZABLE_MANAGER (manager));
   g_return_if_fail (to_updload);
-  g_return_if_fail (to_test);
 
   iface = EPHY_SYNCHRONIZABLE_MANAGER_GET_IFACE (manager);
-  iface->merge_remotes (manager, first_time, remotes, to_updload, to_test);
+  iface->merge_remotes (manager, is_initial, remotes_deleted, remtoes_updated, to_updload);
 }
diff --git a/src/sync/ephy-synchronizable-manager.h b/src/sync/ephy-synchronizable-manager.h
index b7a5438..3ae101a 100644
--- a/src/sync/ephy-synchronizable-manager.h
+++ b/src/sync/ephy-synchronizable-manager.h
@@ -50,10 +50,10 @@ struct _EphySynchronizableManagerInterface {
                                                    EphySynchronizable         *synchronizable);
 
   void                 (*merge_remotes)           (EphySynchronizableManager  *manager,
-                                                   gboolean                    first_time,
-                                                   GList                      *remotes,
-                                                   GList                     **to_upload,
-                                                   GList                     **to_test);
+                                                   gboolean                    is_initial,
+                                                   GList                      *remotes_deleted,
+                                                   GList                      *remotes_updated,
+                                                   GList                     **to_upload);
 };
 
 const char         *ephy_synchronizable_manager_get_collection_name     (EphySynchronizableManager  
*manager);
@@ -69,9 +69,9 @@ void                ephy_synchronizable_manager_add                     (EphySyn
 void                ephy_synchronizable_manager_remove                  (EphySynchronizableManager  *manager,
                                                                          EphySynchronizable         
*synchronizable);
 void                ephy_synchronizable_manager_merge_remotes           (EphySynchronizableManager  *manager,
-                                                                         gboolean                    
first_time,
-                                                                         GList                      *remotes,
-                                                                         GList                     
**to_upload,
-                                                                         GList                     
**to_test);
+                                                                         gboolean                    
is_initial,
+                                                                         GList                      
*remotes_deleted,
+                                                                         GList                      
*remotes_updated,
+                                                                         GList                     
**to_upload);
 
 G_END_DECLS
diff --git a/src/sync/ephy-synchronizable.c b/src/sync/ephy-synchronizable.c
index 3401d85..93fa18f 100644
--- a/src/sync/ephy-synchronizable.c
+++ b/src/sync/ephy-synchronizable.c
@@ -188,6 +188,7 @@ ephy_synchronizable_to_bso (EphySynchronizable  *synchronizable,
  * @gtype: the #GType of object to construct
  * @bundle: a %SyncCryptoKeyBundle holding the encryption key and the HMAC key
  *          used to validate and decrypt the Basic Storage Object
+ * @is_deleted: return value for a flag that shows whether the object is marked as deleted
  *
  * Converts a JSON object representing the Basic Storage Object
  * from the server's point of view (i.e. the %modified field is present)
@@ -204,16 +205,20 @@ ephy_synchronizable_to_bso (EphySynchronizable  *synchronizable,
 GObject *
 ephy_synchronizable_from_bso (JsonObject          *bso,
                               GType                gtype,
-                              SyncCryptoKeyBundle *bundle)
+                              SyncCryptoKeyBundle *bundle,
+                              gboolean            *is_deleted)
 {
   GObject *object = NULL;
   GError *error = NULL;
+  JsonParser *parser;
+  JsonObject *json;
   char *serialized;
   const char *payload;
   double modified;
 
   g_return_val_if_fail (bso, NULL);
   g_return_val_if_fail (bundle, NULL);
+  g_return_val_if_fail (is_deleted, NULL);
 
   if (!json_object_has_member (bso, "id") ||
       !json_object_has_member (bso, "payload") ||
@@ -230,17 +235,37 @@ ephy_synchronizable_from_bso (JsonObject          *bso,
     goto out;
   }
 
+  parser = json_parser_new ();
+  json_parser_load_from_data (parser, serialized, -1, &error);
+  if (error) {
+    g_warning ("Decrypted text is not a valid JSON: %s", error->message);
+    g_error_free (error);
+    goto free_parser;
+  }
+  if (!JSON_NODE_HOLDS_OBJECT (json_parser_get_root (parser))) {
+    g_warning ("JSON root does not hold a JSON object");
+    goto free_parser;
+  }
+  json = json_node_get_object (json_parser_get_root (parser));
+  if (json_object_has_member (json, "deleted")) {
+    if (JSON_NODE_HOLDS_VALUE (json_object_get_member (json, "deleted")))
+      *is_deleted = json_object_get_boolean_member (json, "deleted");
+  } else {
+    *is_deleted = FALSE;
+  }
+
   object = json_gobject_from_data (gtype, serialized, -1, &error);
   if (error) {
     g_warning ("Failed to create GObject from BSO: %s", error->message);
     g_error_free (error);
-    goto free_serialized;
+    goto free_parser;
   }
 
   ephy_synchronizable_set_modification_time (EPHY_SYNCHRONIZABLE (object), modified);
   ephy_synchronizable_set_is_uploaded (EPHY_SYNCHRONIZABLE (object), TRUE);
 
-free_serialized:
+free_parser:
+  g_object_unref (parser);
   g_free (serialized);
 out:
   return object;
diff --git a/src/sync/ephy-synchronizable.h b/src/sync/ephy-synchronizable.h
index 30dfef7..20ad4d6 100644
--- a/src/sync/ephy-synchronizable.h
+++ b/src/sync/ephy-synchronizable.h
@@ -69,6 +69,7 @@ char                *ephy_synchronizable_to_bso                 (EphySynchroniza
  * Think of it as more of an utility function. */
 GObject             *ephy_synchronizable_from_bso               (JsonObject          *bso,
                                                                  GType                gtype,
-                                                                 SyncCryptoKeyBundle *bundle);
+                                                                 SyncCryptoKeyBundle *bundle,
+                                                                 gboolean            *is_deleted);
 
 G_END_DECLS


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]