Fix issue with race condition when alternating UUIDs, removing locally, and mapping

app/assets/javascripts/app/services/modelManager.js

@@ -6,6 +6,7 @@ class ModelManager {
     this.tags = [];
     this.itemSyncObservers = [];
     this.itemChangeObservers = [];
+    this.itemsPendingRemoval = [];
     this.items = [];
     this._extensions = [];
     this.acceptableContentTypes = ["Note", "Tag", "Extension", "SN|Editor", "SN|Theme", "SN|Component", "SF|Extension"];
@@ -30,7 +31,7 @@ class ModelManager {
     })
   }
 
-  alternateUUIDForItem(item, callback) {
+  alternateUUIDForItem(item, callback, removeOriginal) {
     // we need to clone this item and give it a new uuid, then delete item with old uuid from db (you can't mofidy uuid's in our indexeddb setup)
     var newItem = this.createItem(item);
 
@@ -41,12 +42,20 @@ class ModelManager {
 
     this.informModelsOfUUIDChangeForItem(newItem, item.uuid, newItem.uuid);
 
-    this.removeItemLocally(item, function(){
+    var block = () => {
       this.addItem(newItem);
       newItem.setDirty(true);
       newItem.markAllReferencesDirty();
       callback();
-    }.bind(this));
+    }
+
+    if(removeOriginal) {
+      this.removeItemLocally(item, function(){
+        block();
+      });
+    } else {
+      block();
+    }
   }
 
   informModelsOfUUIDChangeForItem(newItem, oldUUID, newUUID) {
@@ -94,15 +103,23 @@ class ModelManager {
     // first loop should add and process items
     for (var json_obj of items) {
       json_obj = _.omit(json_obj, omitFields || [])
-      var item = this.findItem(json_obj["uuid"]);
-
-      _.omit(json_obj, omitFields);
+      var item = this.findItem(json_obj.uuid);
 
       if(item) {
         item.updateFromJSON(json_obj);
       }
 
-      if(json_obj["deleted"] == true || !_.includes(this.acceptableContentTypes, json_obj["content_type"])) {
+      if(!json_obj.content && !item) {
+        // A new incoming item must have a content field. If not, something has set an invalid state.
+        console.error("Content is missing for new item.");
+      }
+
+      if(this.itemsPendingRemoval.includes(json_obj.uuid)) {
+        _.pull(this.itemsPendingRemoval, json_obj.uuid);
+        continue;
+      }
+
+      if(json_obj.deleted == true || !_.includes(this.acceptableContentTypes, json_obj["content_type"])) {
         if(item) {
           allModels.push(item);
           this.removeItemLocally(item)
@@ -302,6 +319,8 @@ class ModelManager {
 
     item.isBeingRemovedLocally();
 
+    this.itemsPendingRemoval.push(item.uuid);
+
     if(item.content_type == "Tag") {
       _.pull(this.tags, item);
     } else if(item.content_type == "Note") {

app/assets/javascripts/app/services/syncManager.js

@@ -75,13 +75,16 @@ class SyncManager {
     Alternating here forces us to to create duplicates of the items instead.
    */
   markAllItemsDirtyAndSaveOffline(callback, alternateUUIDs) {
-    var originalItems = this.modelManager.allItems;
 
-    var block = (items) => {
-      for(var item of items) {
+    // use a copy, as alternating uuid will affect array
+    var originalItems = this.modelManager.allItems.slice();
+
+    var block = () => {
+      var allItems = this.modelManager.allItems;
+      for(var item of allItems) {
         item.setDirty(true);
       }
-      this.writeItemsToLocalStorage(items, false, callback);
+      this.writeItemsToLocalStorage(allItems, false, callback);
     }
 
     if(alternateUUIDs) {
@@ -90,18 +93,19 @@ class SyncManager {
       let alternateNextItem = () => {
         if(index >= originalItems.length) {
           // We don't use originalItems as altnerating UUID will have deleted them.
-          block(this.modelManager.allItems);
+          block();
           return;
         }
 
         var item = originalItems[index];
-        this.modelManager.alternateUUIDForItem(item, alternateNextItem);
-        ++index;
+        index++;
+        // false => dont remove original. We want to keep both copies
+        this.modelManager.alternateUUIDForItem(item, alternateNextItem, false);
       }
 
       alternateNextItem();
     } else {
-      block(originalItems);
+      block();
     }
   }
 
@@ -361,7 +365,7 @@ class SyncManager {
         // UUID conflicts can occur if a user attempts to
         // import an old data archive with uuids from the old account into a new account
         handled = true;
-        this.modelManager.alternateUUIDForItem(item, handleNext);
+        this.modelManager.alternateUUIDForItem(item, handleNext, true);
       }
 
       else if(error.tag === "sync_conflict") {

app/assets/templates/frontend/notes.html.haml

@@ -39,7 +39,7 @@
 
     .scrollable
       .infinite-scroll{"infinite-scroll" => "ctrl.paginate()", "can-load" => "true", "threshold" => "200"}
-        .note{"ng-repeat" => "note in (ctrl.sortedNotes = (ctrl.tag.notes | filter: ctrl.filterNotes | sortBy: ctrl.sortBy| limitTo:ctrl.notesToDisplay))",
+        .note{"ng-repeat" => "note in (ctrl.sortedNotes = (ctrl.tag.notes | filter: ctrl.filterNotes | sortBy: ctrl.sortBy| limitTo:ctrl.notesToDisplay)) track by note.uuid",
             "ng-click" => "ctrl.selectNote(note)", "ng-class" => "{'selected' : ctrl.selectedNote == note}"}
           %strong.red.medium{"ng-if" => "note.conflict_of"} Conflicted copy
           %strong.red.medium{"ng-if" => "note.errorDecrypting"} Error decrypting

app/assets/templates/frontend/tags.html.haml

@@ -10,7 +10,7 @@
         .info
           %input.title{"ng-disabled" => "true", "ng-model" => "ctrl.allTag.title"}
           .count {{ctrl.noteCount(ctrl.allTag)}}
-      .tag{"ng-repeat" => "tag in ctrl.tags", "ng-click" => "ctrl.selectTag(tag)", "ng-class" => "{'selected' : ctrl.selectedTag == tag}"}
+      .tag{"ng-repeat" => "tag in ctrl.tags track by tag.uuid", "ng-click" => "ctrl.selectTag(tag)", "ng-class" => "{'selected' : ctrl.selectedTag == tag}"}
         .info
           %input.title{"ng-attr-id" => "tag-{{tag.uuid}}", "ng-click" => "ctrl.selectTag(tag)", "ng-model" => "tag.title",
           "ng-keyup" => "$event.keyCode == 13 && ctrl.saveTag($event, tag)", "mb-autofocus" => "true", "should-focus" => "ctrl.newTag || ctrl.editingTag == tag",