core: get rid of noteIds in notebook topics

This is a BREAKING change in the core & will require updating the
clients. The way it works is instead of keeping noteIds of all the
notes in the topic, it keeps an in-memory cache. This in-memory
cache `topicReferences` lives in the notes collection & decouples
notes from notebooks/topics. This also simplifies all the different
actions where references would persist after the note was deleted.
Since the note acts as the source of truth for where it currently is,
there is nothing else to do except rebuild the `topicReferences`
cache.

packages/core/__tests__/notebooks.test.js

@@ -128,38 +128,12 @@ test("merge notebook when local notebook is also edited", () =>
     expect(notebook.topics.has("hello")).toBe(false);
   }));
 
-test("merge notebook when local notebook is also edited should merge noteIds too", () =>
-  notebookTest().then(async ({ db, id }) => {
-    let notebook = db.notebooks.notebook(id);
-
-    let note = await db.notes.add(TEST_NOTE);
-    await db.notes.move(
-      { id: notebook.data.id, topic: notebook.data.topics[0].id },
-      note
-    );
-
-    const newNotebook = { ...notebook.data, remote: true };
-    newNotebook.topics[0].title = "hello (edited)";
-    newNotebook.topics[0].notes.push("hello-new-note");
-
-    await delay(500);
-
-    await notebook.topics.add({
-      ...notebook.topics.all[0],
-      title: "hello (edited too)"
-    });
-
-    await expect(db.notebooks.merge(newNotebook)).resolves.not.toThrow();
-
-    expect(notebook.topics.all[0].notes).toHaveLength(2);
-  }));
-
 test("merging notebook when local notebook is not edited should not update remote notebook dateEdited", () =>
   notebookTest().then(async ({ db, id }) => {
     let notebook = db.notebooks.notebook(id);
 
     let note = await db.notes.add(TEST_NOTE);
-    await db.notes.move(
+    await db.notes.addToNotebook(
       { id: notebook.data.id, topic: notebook.data.topics[0].id },
       note
     );

packages/core/__tests__/notes.test.js

@@ -60,7 +60,9 @@ test("delete note", () =>
     let notebookId = await db.notebooks.add(TEST_NOTEBOOK);
     let topics = db.notebooks.notebook(notebookId).topics;
     let topic = topics.topic("hello");
-    await topic.add(id);
+
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
+
     topic = topics.topic("hello");
 
     expect(topic.all.findIndex((v) => v.id === id)).toBeGreaterThan(-1);
@@ -192,7 +194,9 @@ test("add note to topic", () =>
     let topics = db.notebooks.notebook(notebookId).topics;
     await topics.add("Home");
     let topic = topics.topic("Home");
-    await topic.add(id);
+
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
+
     topic = topics.topic("Home");
     expect(topic.all).toHaveLength(1);
     expect(topic.totalNotes).toBe(1);
@@ -207,7 +211,9 @@ test("duplicate note to topic should not be added", () =>
     let topics = db.notebooks.notebook(notebookId).topics;
     await topics.add("Home");
     let topic = topics.topic("Home");
-    await topic.add(id);
+
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
+
     topic = topics.topic("Home");
     expect(topic.all).toHaveLength(1);
   }));
@@ -218,15 +224,15 @@ test("add the same note to 2 notebooks", () =>
     let topics = db.notebooks.notebook(notebookId).topics;
     await topics.add("Home");
     let topic = topics.topic("Home")._topic;
-    await db.notes.move({ id: notebookId, topic: topic.id }, id);
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
 
     expect(topics.topic(topic.id).has(id)).toBe(true);
 
     let notebookId2 = await db.notebooks.add({ title: "Hello2" });
     let topics2 = db.notebooks.notebook(notebookId2).topics;
     await topics2.add("Home2");
     let topic2 = topics2.topic("Home2")._topic;
-    await db.notes.move({ id: notebookId2, topic: topic2.id }, id);
+    await db.notes.addToNotebook({ id: notebookId2, topic: topic2.id }, id);
 
     let note = db.notes.note(id);
     expect(note.notebooks).toHaveLength(2);
@@ -239,8 +245,10 @@ test("moving note to same notebook and topic should do nothing", () =>
     let topics = db.notebooks.notebook(notebookId).topics;
     await topics.add("Home");
     let topic = topics.topic("Home");
-    await topic.add(id);
-    await db.notes.move({ id: notebookId, topic: "Home" }, id);
+
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
+    await db.notes.addToNotebook({ id: notebookId, topic: topic.id }, id);
+
     let note = db.notes.note(id);
     expect(note.notebooks.some((n) => n.id === notebookId)).toBe(true);
   }));
@@ -343,16 +351,6 @@ test("note content should not contain image base64 data after save", () =>
     expect(content).not.toContain(`src=`);
   }));
 
-test("repairing notebook references should delete non-existent notebooks", () =>
-  noteTest({
-    ...TEST_NOTE,
-    notebooks: [{ id: "hello", topics: ["helloworld"] }]
-  }).then(async ({ db, id }) => {
-    await db.notes.repairReferences();
-    let note = db.notes.note(id);
-    expect(note.notebooks).toHaveLength(0);
-  }));
-
 test("adding a note with an invalid tag should clean the tag array", () =>
   databaseTest().then(async (db) => {
     await expect(

packages/core/__tests__/topics.test.js

@@ -51,25 +51,25 @@ test("add note to topic", () =>
     let topics = db.notebooks.notebook(id).topics;
     let topic = topics.topic("hello");
     let noteId = await db.notes.add(TEST_NOTE);
-    await topic.add(noteId);
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
 
     topic = topics.topic("hello");
     expect(topic.totalNotes).toBe(1);
     expect(db.notebooks.notebook(id).totalNotes).toBe(1);
   }));
 
-test("delete note to topic", () =>
+test("delete note of a topic", () =>
   notebookTest().then(async ({ db, id }) => {
     let topics = db.notebooks.notebook(id).topics;
     let topic = topics.topic("hello");
     let noteId = await db.notes.add(TEST_NOTE);
-    await topic.add(noteId);
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
 
     topic = topics.topic("hello");
     expect(topic.totalNotes).toBe(1);
     expect(db.notebooks.notebook(id).totalNotes).toBe(1);
 
-    await topic.delete(noteId);
+    await db.notes.removeFromNotebook({ id, topic: topic.id }, noteId);
 
     topic = topics.topic("hello");
     expect(topic.totalNotes).toBe(0);
@@ -116,7 +116,8 @@ test("get topic", () =>
     let noteId = await db.notes.add({
       content: TEST_NOTE.content
     });
-    await topic.add(noteId);
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
+
     topic = topics.topic("Home");
     expect(await db.content.get(topic.all[0].contentId)).toBeDefined();
     expect(topic.totalNotes).toBe(1);
@@ -136,7 +137,7 @@ test("delete note from edited topic", () =>
       let topics = db.notebooks.notebook(id).topics;
       await topics.add("Home");
       let topic = topics.topic("Home");
-      await db.notes.move({ id, topic: topic._topic.title }, noteId);
+      await db.notes.addToNotebook({ id, topic: topic._topic.title }, noteId);
       await topics.add({ id: topic._topic.id, title: "Hello22" });
       await db.notes.delete(noteId);
     })

packages/core/__tests__/trash.test.js

@@ -57,7 +57,9 @@ test("permanently delete a note", () =>
 test("restore a deleted note that was in a notebook", () =>
   noteTest().then(async ({ db, id }) => {
     let nbId = await db.notebooks.add(TEST_NOTEBOOK);
-    await db.notebooks.notebook(nbId).topics.topic("hello").add(id);
+    const topic = db.notebooks.notebook(nbId).topics.topic("hello");
+    await db.notes.addToNotebook({ id: nbId, topic: topic.id }, id);
+
     await db.notes.delete(id);
     await db.trash.restore(db.trash.all[0].id);
     expect(db.trash.all).toHaveLength(0);
@@ -68,7 +70,7 @@ test("restore a deleted note that was in a notebook", () =>
     expect(await note.content()).toBe(TEST_NOTE.content.data);
 
     const notebook = db.notebooks.notebook(nbId);
-    expect(notebook.topics.topic("hello").has(id)).toBe(true);
+    expect(notebook.topics.topic(topic.id).has(id)).toBe(true);
 
     expect(note.notebooks.some((n) => n.id === nbId)).toBe(true);
 
@@ -102,7 +104,9 @@ test("restore a deleted locked note", () =>
 test("restore a deleted note that's in a deleted notebook", () =>
   noteTest().then(async ({ db, id }) => {
     let nbId = await db.notebooks.add(TEST_NOTEBOOK);
-    await db.notebooks.notebook(nbId).topics.topic("hello").add(id);
+    const topic = db.notebooks.notebook(nbId).topics.topic("hello");
+    await db.notes.addToNotebook({ id: nbId, topic: topic.id }, id);
+
     await db.notes.delete(id);
     await db.notebooks.delete(nbId);
     const deletedNote = db.trash.all.find(
@@ -117,7 +121,10 @@ test("restore a deleted note that's in a deleted notebook", () =>
 test("delete a notebook", () =>
   notebookTest().then(async ({ db, id }) => {
     let noteId = await db.notes.add(TEST_NOTE);
-    await db.notebooks.notebook(id).topics.topic("hello").add(noteId);
+    let notebook = db.notebooks.notebook(id);
+    const topic = notebook.topics.topic("hello");
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
+
     await db.notebooks.delete(id);
     expect(db.notebooks.notebook(id)).toBeUndefined();
     expect(db.notes.note(noteId).notebook).toBeUndefined();
@@ -126,7 +133,9 @@ test("delete a notebook", () =>
 test("restore a deleted notebook", () =>
   notebookTest().then(async ({ db, id }) => {
     let noteId = await db.notes.add(TEST_NOTE);
-    await db.notebooks.notebook(id).topics.topic("hello").add(noteId);
+    const topic = db.notebooks.notebook(id).topics.topic("hello");
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
+
     await db.notebooks.delete(id);
     await db.trash.restore(id);
 
@@ -137,21 +146,24 @@ test("restore a deleted notebook", () =>
     const noteNotebook = note.notebooks.find((n) => n.id === id);
     expect(noteNotebook).toBeDefined();
     expect(noteNotebook.topics).toHaveLength(1);
-
     expect(notebook.topics.topic(noteNotebook.topics[0])).toBeDefined();
   }));
 
 test("restore a notebook that has deleted notes", () =>
   notebookTest().then(async ({ db, id }) => {
     let noteId = await db.notes.add(TEST_NOTE);
-    await db.notebooks.notebook(id).topics.topic("hello").add(noteId);
+
+    let notebook = db.notebooks.notebook(id);
+    const topic = notebook.topics.topic("hello");
+    await db.notes.addToNotebook({ id, topic: topic.id }, noteId);
+
     await db.notebooks.delete(id);
     await db.notes.delete(noteId);
     const deletedNotebook = db.trash.all.find(
       (v) => v.id === id && v.itemType === "notebook"
     );
     await db.trash.restore(deletedNotebook.id);
-    let notebook = db.notebooks.notebook(id);
+    notebook = db.notebooks.notebook(id);
     expect(notebook).toBeDefined();
     expect(notebook.topics.topic("hello").has(noteId)).toBe(false);
   }));

packages/core/api/__tests__/__snapshots__/debug.test.js.snap

@@ -4,10 +4,10 @@ exports[`strip note with content: stripped-note-with-content 1`] = `"{\\"title\\
 
 exports[`strip note: stripped-note 1`] = `"{\\"title\\":true,\\"description\\":false,\\"headline\\":true,\\"colored\\":false,\\"type\\":\\"note\\",\\"tags\\":[],\\"id\\":\\"hello\\",\\"contentId\\":\\"hello2\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123}"`;
 
-exports[`strip notebook: stripped-notebook 1`] = `"{\\"title\\":true,\\"description\\":true,\\"headline\\":false,\\"colored\\":false,\\"type\\":\\"notebook\\",\\"id\\":\\"hello\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123,\\"additionalData\\":[{\\"type\\":\\"topic\\",\\"id\\":\\"hello\\",\\"notebookId\\":\\"hello23\\",\\"title\\":\\"hello\\",\\"dateCreated\\":123,\\"dateEdited\\":123,\\"notes\\":[],\\"dateModified\\":123}]}"`;
+exports[`strip notebook: stripped-notebook 1`] = `"{\\"title\\":true,\\"description\\":true,\\"headline\\":false,\\"colored\\":false,\\"type\\":\\"notebook\\",\\"id\\":\\"hello\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123,\\"additionalData\\":[{\\"type\\":\\"topic\\",\\"id\\":\\"hello\\",\\"notebookId\\":\\"hello23\\",\\"title\\":\\"hello\\",\\"dateCreated\\":123,\\"dateEdited\\":123,\\"dateModified\\":123}]}"`;
 
 exports[`strip tag: stripped-tag 1`] = `"{\\"title\\":true,\\"description\\":false,\\"headline\\":false,\\"colored\\":false,\\"type\\":\\"tag\\",\\"noteIds\\":[],\\"id\\":\\"hello\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123}"`;
 
-exports[`strip topic: stripped-topic 1`] = `"{\\"title\\":true,\\"description\\":false,\\"headline\\":false,\\"colored\\":false,\\"type\\":\\"topic\\",\\"notes\\":[],\\"id\\":\\"hello\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123}"`;
+exports[`strip topic: stripped-topic 1`] = `"{\\"title\\":true,\\"description\\":false,\\"headline\\":false,\\"colored\\":false,\\"type\\":\\"topic\\",\\"id\\":\\"hello\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateCreated\\":123}"`;
 
 exports[`strip trashed note: stripped-trashed-note 1`] = `"{\\"title\\":true,\\"description\\":false,\\"headline\\":true,\\"colored\\":false,\\"type\\":\\"trash\\",\\"tags\\":[],\\"id\\":\\"hello\\",\\"contentId\\":\\"hello2\\",\\"dateModified\\":123,\\"dateEdited\\":123,\\"dateDeleted\\":123,\\"dateCreated\\":123}"`;

packages/core/api/sync/__tests__/sync.test.skip.js

@@ -296,7 +296,7 @@ test("issue: remove notebook reference from notes that are removed from topic du
   expect(deviceB.notebooks.notebook(id)).toBeDefined();
 
   const noteA = await deviceA.notes.add({ title: "Note 1" });
-  await deviceA.notes.move({ id, topic: "Topic 1" }, noteA);
+  await deviceA.notes.addToNotebook({ id, topic: "Topic 1" }, noteA);
 
   expect(
     deviceA.notebooks.notebook(id).topics.topic("Topic 1").totalNotes
@@ -305,7 +305,7 @@ test("issue: remove notebook reference from notes that are removed from topic du
   await delay(2000);
 
   const noteB = await deviceB.notes.add({ title: "Note 2" });
-  await deviceB.notes.move({ id, topic: "Topic 1" }, noteB);
+  await deviceB.notes.addToNotebook({ id, topic: "Topic 1" }, noteB);
 
   expect(
     deviceB.notebooks.notebook(id).topics.topic("Topic 1").totalNotes

packages/core/api/sync/index.js

@@ -371,6 +371,8 @@ class Sync {
   async onRemoteSyncCompleted(lastSynced) {
     // refresh monographs on sync completed
     await this.db.monographs.init();
+    // refresh topic references
+    this.db.notes.topicReferences.refresh();
 
     await this.start(false, false, lastSynced);
   }

packages/core/collections/notebooks.js

@@ -22,7 +22,6 @@ import Notebook from "../models/notebook";
 import getId from "../utils/id";
 import { CHECK_IDS, checkIsUserPremium } from "../common";
 import { qclone } from "qclone";
-import setManipulator from "../utils/set";
 
 export default class Notebooks extends Collection {
   async merge(remoteNotebook) {
@@ -36,7 +35,6 @@ export default class Notebooks extends Collection {
       const lastSyncedTimestamp = await this._db.lastSynced();
       let isChanged = false;
       // merge new and old topics
-      // We need to handle 3 cases:
       for (let oldTopic of localNotebook.topics) {
         const newTopicIndex = remoteNotebook.topics.findIndex(
           (t) => t.id === oldTopic.id
@@ -60,25 +58,10 @@ export default class Notebooks extends Collection {
         else if (newTopic && oldTopic.dateEdited > newTopic.dateEdited) {
           remoteNotebook.topics[newTopicIndex] = {
             ...oldTopic,
-            notes: setManipulator.union(oldTopic.notes, newTopic.notes),
             dateEdited: Date.now()
           };
           isChanged = true;
         }
-        // CASE 4: if topic exists in both notebooks:
-        //      if newTopic.dateEdited > oldTopic.dateEdited: we iterate
-        //      on all notes that are not in newTopic (if any)
-        //      and dereference them.
-        else if (newTopic && newTopic.dateEdited > oldTopic.dateEdited) {
-          const removedNotes = setManipulator.complement(
-            oldTopic.notes,
-            newTopic.notes
-          );
-
-          await this.notebook(remoteNotebook.id)
-            .topics.topic(oldTopic.id)
-            .delete(...removedNotes);
-        }
       }
       remoteNotebook.remote = !isChanged;
     }
@@ -163,20 +146,10 @@ export default class Notebooks extends Collection {
       let notebook = this.notebook(id);
       if (!notebook) continue;
       const notebookData = qclone(notebook.data);
-      await notebook.topics.delete(...notebook.data.topics);
+      // await notebook.topics.delete(...notebook.data.topics);
       await this._collection.removeItem(id);
       await this._db.settings.unpin(id);
       await this._db.trash.add(notebookData);
     }
   }
-
-  async repairReferences() {
-    for (let notebook of this.all) {
-      const _notebook = this.notebook(notebook);
-      for (let topic of notebook.topics) {
-        const _topic = _notebook.topics.topic(topic.id);
-        await _topic.add(...topic.notes);
-      }
-    }
-  }
 }