release 0.58.2-beta

db/migrations/0198__randomize_branchIds.sql

@@ -1,7 +0,0 @@
--- "randomize" branchIds so it's clear user should not rely on them
-UPDATE branches SET branchId = '7LSsI2FnZPW2' WHERE parentNoteId = 'hidden' AND noteId = 'search';
-UPDATE branches SET branchId = 'wEcmxk4CNC7G' WHERE parentNoteId = 'singles' AND noteId = 'globalnotemap';
-UPDATE branches SET branchId = '191uVR6Cu6fA' WHERE parentNoteId = 'hidden' AND noteId = 'sqlconsole';
-UPDATE branches SET branchId = 'OjX5Phxp6A4N' WHERE parentNoteId = 'root' AND noteId = 'hidden';
-UPDATE branches SET branchId = 'glNBYFYZRH8P' WHERE parentNoteId = 'hidden' AND noteId = 'bulkaction';
-UPDATE branches SET branchId = 'cAT25wvGMg3K' WHERE parentNoteId = 'root' AND noteId = 'share';

db/migrations/0198__rename_branchIds.sql

@@ -0,0 +1,6 @@
+UPDATE branches SET branchId = '_hidden__search' WHERE parentNoteId = 'hidden' AND noteId = 'search';
+UPDATE branches SET branchId = 'root__globalNoteMap' WHERE parentNoteId = 'singles' AND noteId = 'globalnotemap';
+UPDATE branches SET branchId = '_hidden__sqlConsole' WHERE parentNoteId = 'hidden' AND noteId = 'sqlconsole';
+UPDATE branches SET branchId = 'root__hidden' WHERE parentNoteId = 'root' AND noteId = 'hidden';
+UPDATE branches SET branchId = '_hidden__bulkAction' WHERE parentNoteId = 'hidden' AND noteId = 'bulkaction';
+UPDATE branches SET branchId = '_hidden__share' WHERE parentNoteId = 'root' AND noteId = 'share';

db/migrations/0202__move_global_note_map_under_hidden.sql

@@ -1,2 +1,2 @@
-DELETE FROM branches WHERE noteId = '_globalNoteMap' AND parentNoteId != 'singles'; -- make sure there are no clones which would fail at the next line
+DELETE FROM branches WHERE noteId = '_globalNoteMap' AND parentNoteId != 'singles' AND parentNoteId != '_hidden'; -- make sure there are no clones which would fail at the next line
 UPDATE branches SET parentNoteId = '_hidden' WHERE noteId = '_globalNoteMap';

db/migrations/0210__consistency_checks.js

@@ -0,0 +1,24 @@
+module.exports = async () => {
+    const cls = require("../../src/services/cls");
+    const beccaLoader = require("../../src/becca/becca_loader");
+    const log = require("../../src/services/log");
+    const consistencyChecks = require("../../src/services/consistency_checks");
+    const noteService = require("../../src/services/notes");
+
+    await cls.init(async () => {
+        // precaution for the 0211 migration
+        noteService.eraseDeletedNotesNow();
+
+        beccaLoader.load();
+
+        try {
+            // precaution before running 211 which might produce unique constraint problems if the DB was not consistent
+            consistencyChecks.runOnDemandChecksWithoutExclusiveLock(true);
+        }
+        catch (e) {
+            // consistency checks might start failing in the future if there's some incompatible migration down the road
+            // we can optimistically assume the DB is consistent and still continue
+            log.error(`Consistency checks failed in migration 0210: ${e.message} ${e.stack}`);
+        }
+    });
+};

db/migrations/0211__rename_branchIds.sql

@@ -0,0 +1,12 @@
+-- case based on isDeleted is needed, otherwise 2 branches (1 deleted, 1 not) might get the same ID
+UPDATE entity_changes SET entityId = COALESCE((
+    SELECT
+        CASE isDeleted
+            WHEN 0 THEN parentNoteId || '_' || noteId
+            WHEN 1 THEN branchId
+        END
+    FROM branches WHERE branchId = entityId
+), entityId)
+WHERE entityName = 'branches' AND isErased = 0;
+
+UPDATE branches SET branchId = parentNoteId || '_' || noteId WHERE isDeleted = 0;

db/migrations/0212__delete_all_attributes_of_named_notes.js

@@ -0,0 +1,21 @@
+module.exports = () => {
+    const cls = require("../../src/services/cls");
+    const beccaLoader = require("../../src/becca/becca_loader");
+    const becca = require("../../src/becca/becca");
+
+    cls.init(() => {
+        beccaLoader.load();
+
+        const hidden = becca.getNote("_hidden");
+
+        for (const noteId of hidden.getSubtreeNoteIds({includeHidden: true})) {
+            if (noteId.startsWith("_")) { // is "named" note
+                const note = becca.getNote(noteId);
+
+                for (const attr of note.getOwnedAttributes()) {
+                    attr.markAsDeleted("0212__delete_all_attributes_of_named_notes");
+                }
+            }
+        }
+    });
+};

package.json

@@ -2,7 +2,7 @@
   "name": "trilium",
   "productName": "Trilium Notes",
   "description": "Trilium Notes",
-  "version": "0.58.1-beta",
+  "version": "0.58.2-beta",
   "license": "AGPL-3.0-only",
   "main": "electron.js",
   "bin": {
@@ -14,7 +14,9 @@
   },
   "scripts": {
     "start-server": "cross-env TRILIUM_DATA_DIR=./data TRILIUM_ENV=dev node ./src/www",
+    "start-server-no-dir": "cross-env TRILIUM_ENV=dev node ./src/www",
     "start-electron": "cross-env TRILIUM_DATA_DIR=./data TRILIUM_ENV=dev electron --inspect=5858 .",
+    "start-electron-no-dir": "cross-env TRILIUM_ENV=dev electron --inspect=5858 .",
     "switch-server": "rm -r ./node_modules/better-sqlite3 && npm install",
     "switch-electron": "rm -r ./node_modules/better-sqlite3 && npm install && ./node_modules/.bin/electron-rebuild",
     "build-backend-docs": "rm -r ./docs/backend_api && ./node_modules/.bin/jsdoc -c jsdoc-conf.json -d ./docs/backend_api src/becca/entities/*.js src/services/backend_script_api.js src/services/sql.js",

spec/search/search.spec.js

@@ -13,7 +13,7 @@ describe("Search", () => {
         becca.reset();
 
         rootNote = new NoteBuilder(new Note({noteId: 'root', title: 'root', type: 'text'}));
-        new Branch({branchId: 'root', noteId: 'root', parentNoteId: 'none', notePosition: 10});
+        new Branch({branchId: 'none_root', noteId: 'root', parentNoteId: 'none', notePosition: 10});
     });
 
     it("simple path match", () => {

src/becca/becca_service.js

@@ -180,7 +180,7 @@ function getNotePath(noteId) {
         let branchId;
 
         if (note.isRoot()) {
-            branchId = 'root';
+            branchId = 'none_root';
         }
         else {
             const parentNote = note.parents[0];

src/becca/entities/abstract_entity.js

@@ -46,7 +46,10 @@ class AbstractEntity {
         return this.utcDateModified || this.utcDateCreated;
     }
 
-    /** @protected */
+    /**
+     * @protected
+     * @returns {Becca}
+     */
     get becca() {
         if (!becca) {
             becca = require('../becca');
@@ -75,7 +78,7 @@ class AbstractEntity {
     /**
      * Saves entity - executes SQL, but doesn't commit the transaction on its own
      *
-     * @returns {AbstractEntity}
+     * @returns {this}
      */
     save() {
         const entityName = this.constructor.entityName;

src/becca/entities/branch.js

@@ -78,7 +78,7 @@ class Branch extends AbstractEntity {
             childNote.parentBranches.push(this);
         }
 
-        if (this.branchId === 'root') {
+        if (this.noteId === 'root') {
             return;
         }
 
@@ -165,8 +165,7 @@ class Branch extends AbstractEntity {
             }
         }
 
-        if (this.branchId === 'root'
-            || this.noteId === 'root'
+        if (this.noteId === 'root'
             || this.noteId === cls.getHoistedNoteId()) {
 
             throw new Error("Can't delete root or hoisted branch/note");
@@ -209,11 +208,19 @@ class Branch extends AbstractEntity {
     }
 
     beforeSaving() {
+        if (!this.noteId || !this.parentNoteId) {
+            throw new Error(`noteId and parentNoteId are mandatory properties for Branch`);
+        }
+
+        this.branchId = `${this.parentNoteId}_${this.noteId}`;
+
         if (this.notePosition === undefined || this.notePosition === null) {
             let maxNotePos = 0;
 
             for (const childBranch of this.parentNote.getChildBranches()) {
-                if (maxNotePos < childBranch.notePosition && childBranch.noteId !== '_hidden') {
+                if (maxNotePos < childBranch.notePosition
+                    && childBranch.noteId !== '_hidden' // hidden has very large notePosition to always stay last
+                ) {
                     maxNotePos = childBranch.notePosition;
                 }
             }
@@ -225,6 +232,10 @@ class Branch extends AbstractEntity {
             this.isExpanded = false;
         }
 
+        if (!this.prefix?.trim()) {
+            this.prefix = null;
+        }
+
         this.utcDateModified = dateUtils.utcNowDateTime();
 
         super.beforeSaving();
@@ -246,13 +257,20 @@ class Branch extends AbstractEntity {
     }
 
     createClone(parentNoteId, notePosition) {
-        return new Branch({
-            noteId: this.noteId,
-            parentNoteId: parentNoteId,
-            notePosition: notePosition,
-            prefix: this.prefix,
-            isExpanded: this.isExpanded
-        });
+        const existingBranch = this.becca.getBranchFromChildAndParent(this.noteId, parentNoteId);
+
+        if (existingBranch) {
+            existingBranch.notePosition = notePosition;
+            return existingBranch;
+        } else {
+            return new Branch({
+                noteId: this.noteId,
+                parentNoteId: parentNoteId,
+                notePosition: notePosition,
+                prefix: this.prefix,
+                isExpanded: this.isExpanded
+            });
+        }
     }
 }
 

src/becca/entities/note.js

@@ -945,13 +945,14 @@ class Note extends AbstractEntity {
         };
     }
 
-    /** @returns {String[]} */
-    getSubtreeNoteIds({includeArchived = true, resolveSearch = false} = {}) {
-        return this.getSubtree({includeArchived, resolveSearch})
+    /** @returns {String[]} - includes the subtree node as well */
+    getSubtreeNoteIds({includeArchived = true, includeHidden = false, resolveSearch = false} = {}) {
+        return this.getSubtree({includeArchived, includeHidden, resolveSearch})
             .notes
             .map(note => note.noteId);
     }
 
+    /** @deprecated use getSubtreeNoteIds() instead */
     getDescendantNoteIds() {
         return this.getSubtreeNoteIds();
     }
@@ -1171,7 +1172,8 @@ class Note extends AbstractEntity {
      * @param {string} type - attribute type (label / relation)
      * @param {string} name - name of the attribute, not including the leading ~/#
      * @param {string} [value] - value of the attribute - text for labels, target note ID for relations; optional.
-     *
+     * @param {boolean} [isInheritable=false]
+     * @param {int} [position]
      * @return {Attribute}
      */
     addAttribute(type, name, value = "", isInheritable = false, position = 1000) {
@@ -1192,7 +1194,7 @@ class Note extends AbstractEntity {
      *
      * @param {string} name - name of the label, not including the leading #
      * @param {string} [value] - text value of the label; optional
-     *
+     * @param {boolean} [isInheritable=false]
      * @return {Attribute}
      */
     addLabel(name, value = "", isInheritable = false) {
@@ -1204,8 +1206,8 @@ class Note extends AbstractEntity {
      * returned.
      *
      * @param {string} name - name of the relation, not including the leading ~
-     * @param {string} value - ID of the target note of the relation
-     *
+     * @param {string} targetNoteId
+     * @param {boolean} [isInheritable=false]
      * @return {Attribute}
      */
     addRelation(name, targetNoteId, isInheritable = false) {

src/etapi/branches.js

@@ -35,15 +35,14 @@ function register(router) {
             existing.save();
 
             return res.status(200).json(mappers.mapBranchToPojo(existing));
-        }
+        } else {
+            try {
+                const branch = new Branch(params).save();
 
-        try {
-            const branch = new Branch(params).save();
-
-            res.status(201).json(mappers.mapBranchToPojo(branch));
-        }
-        catch (e) {
-            throw new eu.EtapiError(400, eu.GENERIC_CODE, e.message);
+                res.status(201).json(mappers.mapBranchToPojo(branch));
+            } catch (e) {
+                throw new eu.EtapiError(400, eu.GENERIC_CODE, e.message);
+            }
         }
     });
 

src/public/app/services/branches.js

@@ -10,7 +10,9 @@ async function moveBeforeBranch(branchIdsToMove, beforeBranchId) {
     branchIdsToMove = filterRootNote(branchIdsToMove);
     branchIdsToMove = filterSearchBranches(branchIdsToMove);
 
-    if (['root', '_lbRoot', '_lbAvailableLaunchers', '_lbVisibleLaunchers'].includes(beforeBranchId)) {
+    const beforeBranch = await froca.getBranch(beforeBranchId);
+
+    if (['root', '_lbRoot', '_lbAvailableLaunchers', '_lbVisibleLaunchers'].includes(beforeBranch.noteId)) {
         toastService.showError('Cannot move notes here.');
         return;
     }

src/public/app/services/frontend_script_api.js

@@ -41,6 +41,24 @@ function FrontendScriptApi(startNote, currentNote, originEntity = null, $contain
     /** @property {NoteContextAwareWidget} */
     this.NoteContextAwareWidget = NoteContextAwareWidget;
 
+    /**
+     * @property {NoteContextAwareWidget}
+     * @deprecated use NoteContextAwareWidget instead
+     */
+    this.TabAwareWidget = NoteContextAwareWidget;
+
+    /**
+     * @property {NoteContextAwareWidget}
+     * @deprecated use NoteContextAwareWidget instead
+     */
+    this.TabCachingWidget = NoteContextAwareWidget;
+
+    /**
+     * @property {NoteContextAwareWidget}
+     * @deprecated use NoteContextAwareWidget instead
+     */
+    this.NoteContextCachingWidget = NoteContextAwareWidget;
+
     /** @property {BasicWidget} */
     this.BasicWidget = BasicWidget;
 

src/public/app/widgets/note_tree.js

@@ -637,7 +637,7 @@ export default class NoteTreeWidget extends NoteContextAwareWidget {
     }
 
     prepareRootNode() {
-        return this.prepareNode(froca.getBranch('root'));
+        return this.prepareNode(froca.getBranch('none_root'));
     }
 
     /**

src/routes/api/attributes.js

@@ -172,7 +172,7 @@ function updateNoteAttributes(req) {
             continue;
         }
 
-        // no existing attribute has been matched so we need to create a new one
+        // no existing attribute has been matched, so we need to create a new one
         // type, name and isInheritable are immutable so even if there is an attribute with matching type & name, we need to create a new one and delete the former one
 
         note.addAttribute(incAttr.type, incAttr.name, incAttr.value, incAttr.isInheritable, position);

src/routes/api/branches.js

@@ -143,7 +143,7 @@ function setExpanded(req) {
     const {branchId} = req.params;
     const expanded = parseInt(req.params.expanded);
 
-    if (branchId !== 'root') {
+    if (branchId !== 'none_root') {
         sql.execute("UPDATE branches SET isExpanded = ? WHERE branchId = ?", [expanded, branchId]);
         // we don't sync expanded label
         // also this does not trigger updates to the frontend, this would trigger too many reloads
@@ -172,7 +172,7 @@ function setExpandedForSubtree(req) {
         SELECT branchId FROM tree`, [branchId]);
 
     // root is always expanded
-    branchIds = branchIds.filter(branchId => branchId !== 'root');
+    branchIds = branchIds.filter(branchId => branchId !== 'none_root');
 
     sql.executeMany(`UPDATE branches SET isExpanded = ${expanded} WHERE branchId IN (???)`, branchIds);
 

src/routes/api/tree.js

@@ -66,7 +66,7 @@ function getNotesAndBranchesAndAttributes(noteIds) {
 
     if (noteIds.has('root')) {
         branches.push({
-            branchId: 'root',
+            branchId: 'none_root',
             noteId: 'root',
             parentNoteId: 'none',
             notePosition: 0,

src/services/app_info.js

@@ -4,8 +4,8 @@ const build = require('./build');
 const packageJson = require('../../package');
 const {TRILIUM_DATA_DIR} = require('./data_dir');
 
-const APP_DB_VERSION = 209;
-const SYNC_VERSION = 28;
+const APP_DB_VERSION = 212;
+const SYNC_VERSION = 29;
 const CLIPPER_PROTOCOL_VERSION = "1.0";
 
 module.exports = {

src/services/build.js

@@ -1 +1 @@
-module.exports = { buildDate:"2022-12-26T23:57:31+01:00", buildRevision: "7a8bbdced83c3bf906feec87b97aba4959ffa7bc" };
+module.exports = { buildDate:"2022-12-29T00:12:54+01:00", buildRevision: "d36cf47974cd8bc6bd45c1da774a9a55d45f998e" };

src/services/cloning.js

@@ -35,7 +35,7 @@ function cloneNoteToNote(noteId, parentNoteId, prefix) {
         isExpanded: 0
     }).save();
 
-    log.info(`Cloned note ${noteId} to new parent note ${parentNoteId} with prefix ${prefix}`);
+    log.info(`Cloned note '${noteId}' to new parent note '${parentNoteId}' with prefix '${prefix}'`);
 
     return {
         success: true,
@@ -78,14 +78,14 @@ function ensureNoteIsPresentInParent(noteId, parentNoteId, prefix) {
         return validationResult;
     }
 
-    new Branch({
+    const branch = new Branch({
         noteId: noteId,
         parentNoteId: parentNoteId,
         prefix: prefix,
         isExpanded: 0
     }).save();
 
-    log.info(`Ensured note '${noteId}' is in parent note '${parentNoteId}' with prefix '${prefix}'`);
+    log.info(`Ensured note '${noteId}' is in parent note '${parentNoteId}' with prefix '${branch.prefix}'`);
 
     return { success: true };
 }
@@ -169,7 +169,7 @@ function cloneNoteAfter(noteId, afterBranchId) {
         isExpanded: 0
     }).save();
 
-    log.info(`Cloned note '${noteId}' into parent note '${afterNote.parentNoteId}' after note '${afterNote.noteId}', branch ${afterBranchId}`);
+    log.info(`Cloned note '${noteId}' into parent note '${afterNote.parentNoteId}' after note '${afterNote.noteId}', branch '${afterBranchId}'`);
 
     return { success: true, branchId: branch.branchId };
 }

src/services/consistency_checks.js

@@ -12,7 +12,7 @@ const Branch = require('../becca/entities/branch');
 const noteRevisionService = require('./note_revisions');
 const becca = require("../becca/becca");
 const utils = require("../services/utils");
-const {sanitizeAttributeName} = require("./sanitize_attribute_name.js");
+const {sanitizeAttributeName} = require("./sanitize_attribute_name");
 const noteTypes = require("../services/note_types").getNoteTypeNames();
 
 class ConsistencyChecks {
@@ -72,7 +72,7 @@ class ConsistencyChecks {
                         return true;
                     }
                     else {
-                        logError(`Tree cycle detected at parent-child relationship: ${parentNoteId} - ${noteId}, whole path: ${path}`);
+                        logError(`Tree cycle detected at parent-child relationship: '${parentNoteId}' - '${noteId}', whole path: '${path}'`);
 
                         this.unrecoveredConsistencyErrors = true;
                     }
@@ -133,9 +133,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Branch ${branchId} has been deleted since it references missing note ${noteId}`);
+                    logFix(`Branch '${branchId}' has been deleted since it references missing note '${noteId}'`);
                 } else {
-                    logError(`Branch ${branchId} references missing note ${noteId}`);
+                    logError(`Branch '${branchId}' references missing note '${noteId}'`);
                 }
             });
 
@@ -144,7 +144,7 @@ class ConsistencyChecks {
                     FROM branches
                       LEFT JOIN notes ON notes.noteId = branches.parentNoteId
                     WHERE branches.isDeleted = 0
-                      AND branches.branchId != 'root'
+                      AND branches.noteId != 'root'
                       AND notes.noteId IS NULL`,
             ({branchId, parentNoteId}) => {
                 if (this.autoFix) {
@@ -154,9 +154,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Branch ${branchId} was set to root parent since it was referencing missing parent note ${parentNoteId}`);
+                    logFix(`Branch '${branchId}' was set to root parent since it was referencing missing parent note '${parentNoteId}'`);
                 } else {
-                    logError(`Branch ${branchId} references missing parent note ${parentNoteId}`);
+                    logError(`Branch '${branchId}' references missing parent note '${parentNoteId}'`);
                 }
             });
 
@@ -173,9 +173,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Attribute ${attributeId} has been deleted since it references missing source note ${noteId}`);
+                    logFix(`Attribute '${attributeId}' has been deleted since it references missing source note '${noteId}'`);
                 } else {
-                    logError(`Attribute ${attributeId} references missing source note ${noteId}`);
+                    logError(`Attribute '${attributeId}' references missing source note '${noteId}'`);
                 }
             });
 
@@ -193,9 +193,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Relation ${attributeId} has been deleted since it references missing note ${noteId}`)
+                    logFix(`Relation '${attributeId}' has been deleted since it references missing note '${noteId}'`)
                 } else {
-                    logError(`Relation ${attributeId} references missing note ${noteId}`)
+                    logError(`Relation '${attributeId}' references missing note '${noteId}'`)
                 }
             });
     }
@@ -220,9 +220,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Branch ${branchId} has been deleted since associated note ${noteId} is deleted.`);
+                    logFix(`Branch '${branchId}' has been deleted since associated note '${noteId}' is deleted.`);
                 } else {
-                    logError(`Branch ${branchId} is not deleted even though associated note ${noteId} is deleted.`)
+                    logError(`Branch '${branchId}' is not deleted even though associated note '${noteId}' is deleted.`)
                 }
             });
 
@@ -240,9 +240,9 @@ class ConsistencyChecks {
 
                 this.reloadNeeded = true;
 
-                logFix(`Branch ${branchId} has been deleted since associated parent note ${parentNoteId} is deleted.`);
+                logFix(`Branch '${branchId}' has been deleted since associated parent note '${parentNoteId}' is deleted.`);
             } else {
-                logError(`Branch ${branchId} is not deleted even though associated parent note ${parentNoteId} is deleted.`)
+                logError(`Branch '${branchId}' is not deleted even though associated parent note '${parentNoteId}' is deleted.`)
             }
         });
 
@@ -262,9 +262,9 @@ class ConsistencyChecks {
 
                 this.reloadNeeded = true;
 
-                logFix(`Created missing branch ${branch.branchId} for note ${noteId}`);
+                logFix(`Created missing branch '${branch.branchId}' for note '${noteId}'`);
             } else {
-                logError(`No undeleted branch found for note ${noteId}`);
+                logError(`No undeleted branch found for note '${noteId}'`);
             }
         });
 
@@ -296,12 +296,12 @@ class ConsistencyChecks {
                     for (const branch of branches.slice(1)) {
                         branch.markAsDeleted();
 
-                        logFix(`Removing branch ${branch.branchId} since it's parent-child duplicate of branch ${origBranch.branchId}`);
+                        logFix(`Removing branch '${branch.branchId}' since it's a parent-child duplicate of branch '${origBranch.branchId}'`);
                     }
 
                     this.reloadNeeded = true;
                 } else {
-                    logError(`Duplicate branches for note ${noteId} and parent ${parentNoteId}`);
+                    logError(`Duplicate branches for note '${noteId}' and parent '${parentNoteId}'`);
                 }
             });
     }
@@ -322,9 +322,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Note ${noteId} type has been change to file since it had invalid type=${type}`)
+                    logFix(`Note '${noteId}' type has been change to file since it had invalid type '${type}'`)
                 } else {
-                    logError(`Note ${noteId} has invalid type=${type}`);
+                    logError(`Note '${noteId}' has invalid type '${type}'`);
                 }
             });
 
@@ -361,9 +361,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Note ${noteId} content was set to empty string since there was no corresponding row`);
+                    logFix(`Note '${noteId}' content was set to empty string since there was no corresponding row`);
                 } else {
-                    logError(`Note ${noteId} content row does not exist`);
+                    logError(`Note '${noteId}' content row does not exist`);
                 }
             });
 
@@ -385,9 +385,9 @@ class ConsistencyChecks {
 
                         this.reloadNeeded = true;
 
-                        logFix(`Note ${noteId} content was set to "${blankContent}" since it was null even though it is not deleted`);
+                        logFix(`Note '${noteId}' content was set to '${blankContent}' since it was null even though it is not deleted`);
                     } else {
-                        logError(`Note ${noteId} content is null even though it is not deleted`);
+                        logError(`Note '${noteId}' content is null even though it is not deleted`);
                     }
                 });
         }
@@ -404,9 +404,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Note revision content ${noteRevisionId} was created and set to erased since it did not exist.`);
+                    logFix(`Note revision content '${noteRevisionId}' was created and set to erased since it did not exist.`);
                 } else {
-                    logError(`Note revision content ${noteRevisionId} does not exist`);
+                    logError(`Note revision content '${noteRevisionId}' does not exist`);
                 }
             });
 
@@ -431,12 +431,12 @@ class ConsistencyChecks {
                         branch.parentNoteId = 'root';
                         branch.save();
 
-                        logFix(`Child branch ${branch.branchId} has been moved to root since it was a child of a search note ${parentNoteId}`)
+                        logFix(`Child branch '${branch.branchId}' has been moved to root since it was a child of a search note '${parentNoteId}'`)
                     }
 
                     this.reloadNeeded = true;
                 } else {
-                    logError(`Search note ${parentNoteId} has children`);
+                    logError(`Search note '${parentNoteId}' has children`);
                 }
             });
 
@@ -453,9 +453,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Removed relation ${relation.attributeId} of name "${relation.name} with empty target.`);
+                    logFix(`Removed relation '${relation.attributeId}' of name '${relation.name}' with empty target.`);
                 } else {
-                    logError(`Relation ${attributeId} has empty target.`);
+                    logError(`Relation '${attributeId}' has empty target.`);
                 }
             });
 
@@ -474,9 +474,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Attribute ${attributeId} type was changed to label since it had invalid type '${type}'`);
+                    logFix(`Attribute '${attributeId}' type was changed to label since it had invalid type '${type}'`);
                 } else {
-                    logError(`Attribute ${attributeId} has invalid type '${type}'`);
+                    logError(`Attribute '${attributeId}' has invalid type '${type}'`);
                 }
             });
 
@@ -494,9 +494,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Removed attribute ${attributeId} because owning note ${noteId} is also deleted.`);
+                    logFix(`Removed attribute '${attributeId}' because owning note '${noteId}' is also deleted.`);
                 } else {
-                    logError(`Attribute ${attributeId} is not deleted even though owning note ${noteId} is deleted.`);
+                    logError(`Attribute '${attributeId}' is not deleted even though owning note '${noteId}' is deleted.`);
                 }
             });
 
@@ -515,9 +515,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Removed attribute ${attributeId} because target note ${targetNoteId} is also deleted.`);
+                    logFix(`Removed attribute '${attributeId}' because target note '${targetNoteId}' is also deleted.`);
                 } else {
-                    logError(`Attribute ${attributeId} is not deleted even though target note ${targetNoteId} is deleted.`);
+                    logError(`Attribute '${attributeId}' is not deleted even though target note '${targetNoteId}' is deleted.`);
                 }
             });
     }
@@ -545,9 +545,9 @@ class ConsistencyChecks {
                         isSynced: entityName !== 'options' || entity.isSynced
                     });
 
-                    logFix(`Created missing entity change for entityName=${entityName}, entityId=${entityId}`);
+                    logFix(`Created missing entity change for entityName '${entityName}', entityId '${entityId}'`);
                 } else {
-                    logError(`Missing entity change for entityName=${entityName}, entityId=${entityId}`);
+                    logError(`Missing entity change for entityName '${entityName}', entityId '${entityId}'`);
                 }
             });
 
@@ -565,9 +565,9 @@ class ConsistencyChecks {
                     if (this.autoFix) {
                         sql.execute("DELETE FROM entity_changes WHERE entityName = ? AND entityId = ?", [entityName, entityId]);
 
-                        logFix(`Deleted extra entity change id=${id}, entityName=${entityName}, entityId=${entityId}`);
+                        logFix(`Deleted extra entity change id '${id}', entityName '${entityName}', entityId '${entityId}'`);
                     } else {
-                        logError(`Unrecognized entity change id=${id}, entityName=${entityName}, entityId=${entityId}`);
+                        logError(`Unrecognized entity change id '${id}', entityName '${entityName}', entityId '${entityId}'`);
                     }
                 });
 
@@ -586,9 +586,9 @@ class ConsistencyChecks {
 
                     this.reloadNeeded = true;
 
-                    logFix(`Erasing entityName=${entityName}, entityId=${entityId} since entity change id=${id} has it as erased.`);
+                    logFix(`Erasing entityName '${entityName}', entityId '${entityId}' since entity change id '${id}' has it as erased.`);
                 } else {
-                    logError(`Entity change id=${id} has entityName=${entityName}, entityId=${entityId} as erased, but it's not.`);
+                    logError(`Entity change id '${id}' has entityName '${entityName}', entityId '${entityId}' as erased, but it's not.`);
                 }
             });
     }
@@ -621,12 +621,12 @@ class ConsistencyChecks {
                     this.fixedIssues = true;
                     this.reloadNeeded = true;
 
-                    logFix(`Renamed incorrectly named attributes "${origName}" to ${fixedName}`);
+                    logFix(`Renamed incorrectly named attributes '${origName}' to '${fixedName}'`);
                 }
                 else {
                     this.unrecoveredConsistencyErrors = true;
 
-                    logFix(`There are incorrectly named attributes "${origName}"`);
+                    logFix(`There are incorrectly named attributes '${origName}'`);
                 }
             }
         }
@@ -670,7 +670,7 @@ class ConsistencyChecks {
         this.findSyncIssues();
 
         // root branch should always be expanded
-        sql.execute("UPDATE branches SET isExpanded = 1 WHERE branchId = 'root'");
+        sql.execute("UPDATE branches SET isExpanded = 1 WHERE noteId = 'root'");
 
         if (!this.unrecoveredConsistencyErrors) {
             // we run this only if basic checks passed since this assumes basic data consistency
@@ -701,13 +701,7 @@ class ConsistencyChecks {
         let elapsedTimeMs;
 
         await syncMutexService.doExclusively(() => {
-            const startTimeMs = Date.now();
-
-            this.runDbDiagnostics();
-
-            this.runAllChecksAndFixers();
-
-            elapsedTimeMs = Date.now() - startTimeMs;
+            elapsedTimeMs = this.runChecksInner();
         });
 
         if (this.unrecoveredConsistencyErrors) {
@@ -721,6 +715,16 @@ class ConsistencyChecks {
             );
         }
     }
+
+    runChecksInner() {
+        const startTimeMs = Date.now();
+
+        this.runDbDiagnostics();
+
+        this.runAllChecksAndFixers();
+
+        return Date.now() - startTimeMs;
+    }
 }
 
 function getBlankContent(isProtected, type, mime) {
@@ -750,9 +754,14 @@ function runPeriodicChecks() {
     consistencyChecks.runChecks();
 }
 
-function runOnDemandChecks(autoFix) {
+async function runOnDemandChecks(autoFix) {
     const consistencyChecks = new ConsistencyChecks(autoFix);
-    consistencyChecks.runChecks();
+    await consistencyChecks.runChecks();
+}
+
+function runOnDemandChecksWithoutExclusiveLock(autoFix) {
+    const consistencyChecks = new ConsistencyChecks(autoFix);
+    consistencyChecks.runChecksInner();
 }
 
 function runEntityChangesChecks() {
@@ -769,5 +778,6 @@ sqlInit.dbReady.then(() => {
 
 module.exports = {
     runOnDemandChecks,
+    runOnDemandChecksWithoutExclusiveLock,
     runEntityChangesChecks
 };

src/services/entity_changes.js

@@ -8,7 +8,7 @@ const becca = require("../becca/becca");
 
 let maxEntityChangeId = 0;
 
-function addEntityChangeWithinstanceId(origEntityChange, instanceId) {
+function addEntityChangeWithInstanceId(origEntityChange, instanceId) {
     const ec = {...origEntityChange, instanceId};
 
     return addEntityChange(ec);
@@ -71,7 +71,7 @@ function addEntityChangesForSector(entityName, sector) {
         }
     });
 
-    log.info(`Added sector ${sector} of ${entityName} to sync queue in ${Date.now() - startTime}ms.`);
+    log.info(`Added sector ${sector} of '${entityName}' to sync queue in ${Date.now() - startTime}ms.`);
 }
 
 function cleanupEntityChangesForMissingEntities(entityName, entityPrimaryKey) {
@@ -85,45 +85,38 @@ function cleanupEntityChangesForMissingEntities(entityName, entityPrimaryKey) {
 }
 
 function fillEntityChanges(entityName, entityPrimaryKey, condition = '') {
-    try {
-        cleanupEntityChangesForMissingEntities(entityName, entityPrimaryKey);
+    cleanupEntityChangesForMissingEntities(entityName, entityPrimaryKey);
 
-        sql.transactional(() => {
-            const entityIds = sql.getColumn(`SELECT ${entityPrimaryKey} FROM ${entityName}`
-                + (condition ? ` WHERE ${condition}` : ''));
+    sql.transactional(() => {
+        const entityIds = sql.getColumn(`SELECT ${entityPrimaryKey} FROM ${entityName}`
+            + (condition ? ` WHERE ${condition}` : ''));
 
-            let createdCount = 0;
+        let createdCount = 0;
 
-            for (const entityId of entityIds) {
-                const existingRows = sql.getValue("SELECT COUNT(1) FROM entity_changes WHERE entityName = ? AND entityId = ?", [entityName, entityId]);
+        for (const entityId of entityIds) {
+            const existingRows = sql.getValue("SELECT COUNT(1) FROM entity_changes WHERE entityName = ? AND entityId = ?", [entityName, entityId]);
 
-                // we don't want to replace existing entities (which would effectively cause full resync)
-                if (existingRows === 0) {
-                    createdCount++;
+            // we don't want to replace existing entities (which would effectively cause full resync)
+            if (existingRows === 0) {
+                createdCount++;
 
-                    const entity = becca.getEntity(entityName, entityId);
+                const entity = becca.getEntity(entityName, entityId);
 
-                    addEntityChange({
-                        entityName,
-                        entityId,
-                        hash: entity.generateHash(),
-                        isErased: false,
-                        utcDateChanged: entity.getUtcDateChanged(),
-                        isSynced: entityName !== 'options' || !!entity.isSynced
-                    });
-                }
+                addEntityChange({
+                    entityName,
+                    entityId,
+                    hash: entity.generateHash(),
+                    isErased: false,
+                    utcDateChanged: entity.getUtcDateChanged(),
+                    isSynced: entityName !== 'options' || !!entity.isSynced
+                });
             }
+        }
 
-            if (createdCount > 0) {
-                log.info(`Created ${createdCount} missing entity changes for ${entityName}.`);
-            }
-        });
-    }
-    catch (e) {
-        // this is to fix migration from 0.30 to 0.32, can be removed later
-        // see https://github.com/zadam/trilium/issues/557
-        log.error(`Filling entity changes failed for ${entityName} ${entityPrimaryKey} with error "${e.message}", continuing`);
-    }
+        if (createdCount > 0) {
+            log.info(`Created ${createdCount} missing entity changes for ${entityName}.`);
+        }
+    });
 }
 
 function fillAllEntityChanges() {
@@ -145,7 +138,7 @@ module.exports = {
     addNoteReorderingEntityChange,
     moveEntityChangeToTop,
     addEntityChange,
-    addEntityChangeWithinstanceId,
+    addEntityChangeWithInstanceId,
     fillAllEntityChanges,
     addEntityChangesForSector,
     getMaxEntityChangeId: () => maxEntityChangeId

src/services/hidden_subtree.js

@@ -1,5 +1,6 @@
 const becca = require("../becca/becca");
 const noteService = require("./notes");
+const Attribute = require("../becca/entities/attribute.js");
 
 const LBTPL_ROOT = "_lbTplRoot";
 const LBTPL_BASE = "_lbTplBase";
@@ -10,6 +11,12 @@ const LBTPL_BUILTIN_WIDGET = "_lbTplBuiltinWidget";
 const LBTPL_SPACER = "_lbTplSpacer";
 const LBTPL_CUSTOM_WIDGET = "_lbTplCustomWidget";
 
+/*
+ * Hidden subtree is generated as a "predictable structure" which means that it avoids generating random IDs to always
+ * produce same structure. This is needed because it is run on multiple instances in the sync cluster which might produce
+ * duplicate subtrees. This way, all instances will generate the same structure with same IDs.
+ */
+
 const HIDDEN_SUBTREE_DEFINITION = {
     id: '_hidden',
     title: 'Hidden Notes',
@@ -237,13 +244,7 @@ function checkHiddenSubtreeRecursively(parentNoteId, item) {
     }
 
     let note = becca.notes[item.id];
-    let branch = becca.branches[item.id];
-
-    const attrs = [...(item.attributes || [])];
-
-    if (item.icon) {
-        attrs.push({ type: 'label', name: 'iconClass', value: `bx ${item.icon}` });
-    }
+    let branch;
 
     if (!note) {
         ({note, branch} = noteService.createNewNote({
@@ -254,27 +255,35 @@ function checkHiddenSubtreeRecursively(parentNoteId, item) {
             content: '',
             ignoreForbiddenParents: true
         }));
+    } else {
+        branch = note.getParentBranches().find(branch => branch.parentNoteId === parentNoteId);
+    }
 
-        if (item.type === 'launcher') {
-            if (item.command) {
-                attrs.push({ type: 'relation', name: 'template', value: LBTPL_COMMAND });
-                attrs.push({ type: 'label', name: 'command', value: item.command });
-            } else if (item.builtinWidget) {
-                if (item.builtinWidget === 'spacer') {
-                    attrs.push({ type: 'relation', name: 'template', value: LBTPL_SPACER });
-                    attrs.push({ type: 'label', name: 'baseSize', value: item.baseSize });
-                    attrs.push({ type: 'label', name: 'growthFactor', value: item.growthFactor });
-                } else {
-                    attrs.push({ type: 'relation', name: 'template', value: LBTPL_BUILTIN_WIDGET });
-                }
+    const attrs = [...(item.attributes || [])];
 
-                attrs.push({ type: 'label', name: 'builtinWidget', value: item.builtinWidget });
-             } else if (item.targetNoteId) {
-                attrs.push({ type: 'relation', name: 'template', value: LBTPL_NOTE_LAUNCHER });
-                attrs.push({ type: 'relation', name: 'target', value: item.targetNoteId });
+    if (item.icon) {
+        attrs.push({ type: 'label', name: 'iconClass', value: `bx ${item.icon}` });
+    }
+
+    if (item.type === 'launcher') {
+        if (item.command) {
+            attrs.push({ type: 'relation', name: 'template', value: LBTPL_COMMAND });
+            attrs.push({ type: 'label', name: 'command', value: item.command });
+        } else if (item.builtinWidget) {
+            if (item.builtinWidget === 'spacer') {
+                attrs.push({ type: 'relation', name: 'template', value: LBTPL_SPACER });
+                attrs.push({ type: 'label', name: 'baseSize', value: item.baseSize });
+                attrs.push({ type: 'label', name: 'growthFactor', value: item.growthFactor });
             } else {
-                throw new Error(`No action defined for launcher ${JSON.stringify(item)}`);
+                attrs.push({ type: 'relation', name: 'template', value: LBTPL_BUILTIN_WIDGET });
             }
+
+            attrs.push({ type: 'label', name: 'builtinWidget', value: item.builtinWidget });
+        } else if (item.targetNoteId) {
+            attrs.push({ type: 'relation', name: 'template', value: LBTPL_NOTE_LAUNCHER });
+            attrs.push({ type: 'relation', name: 'target', value: item.targetNoteId });
+        } else {
+            throw new Error(`No action defined for launcher ${JSON.stringify(item)}`);
         }
     }
 
@@ -299,8 +308,17 @@ function checkHiddenSubtreeRecursively(parentNoteId, item) {
     }
 
     for (const attr of attrs) {
-        if (!note.hasAttribute(attr.type, attr.name)) {
-            note.addAttribute(attr.type, attr.name, attr.value);
+        const attrId = note.noteId + "_" + attr.type.charAt(0) + attr.name;
+
+        if (!note.getAttributes().find(attr => attr.attributeId === attrId)) {
+            new Attribute({
+                attributeId: attrId,
+                noteId: note.noteId,
+                type: attr.type,
+                name: attr.name,
+                value: attr.value,
+                isInheritable: false
+            }).save();
         }
     }
 

src/services/migration.js

@@ -22,7 +22,7 @@ async function migrate() {
     }
 
     fs.readdirSync(resourceDir.MIGRATIONS_DIR).forEach(file => {
-        const match = file.match(/([0-9]{4})__([a-zA-Z0-9_ ]+)\.(sql|js)/);
+        const match = file.match(/^([0-9]{4})__([a-zA-Z0-9_ ]+)\.(sql|js)$/);
 
         if (match) {
             const dbVersion = parseInt(match[1]);
@@ -62,9 +62,10 @@ async function migrate() {
                 log.info(`Migration to version ${mig.dbVersion} has been successful.`);
             } catch (e) {
                 log.error(`error during migration to version ${mig.dbVersion}: ${e.stack}`);
-                log.error("migration failed, crashing hard"); // this is not very user friendly :-/
+                log.error("migration failed, crashing hard"); // this is not very user-friendly :-/
 
                 utils.crash();
+                break; // crash() above does not seem to work right away
             }
         }
     });

src/services/notes.js

@@ -155,6 +155,9 @@ function createNewNote(params) {
                 cls.disableEntityEvents();
             }
 
+            // TODO: think about what can happen if the note already exists with the forced ID
+            //       I guess on DB it's going to be fine, but becca references between entities
+            //       might get messed up (two Note instance for the same ID existing in the references)
             note = new Note({
                 noteId: params.noteId, // optionally can force specific noteId
                 title: params.title,

src/services/scheduler.js

@@ -51,9 +51,9 @@ function runNotesWithLabel(runAttrValue) {
 }
 
 sqlInit.dbReady.then(() => {
-    if (!process.env.TRILIUM_SAFE_MODE) {
-        cls.init(() => hiddenSubtreeService.checkHiddenSubtree());
+    cls.init(() => hiddenSubtreeService.checkHiddenSubtree());
 
+    if (!process.env.TRILIUM_SAFE_MODE) {
         setTimeout(cls.wrap(() => runNotesWithLabel('backendStartup')), 10 * 1000);
 
         setInterval(cls.wrap(() => runNotesWithLabel('hourly')), 3600 * 1000);

src/services/sql_init.js

@@ -77,7 +77,6 @@ async function createInitialDatabase() {
         rootNote.setContent('');
 
         new Branch({
-            branchId: 'root',
             noteId: 'root',
             parentNoteId: 'none',
             isExpanded: true,

src/services/sync_update.js

@@ -42,7 +42,7 @@ function updateEntity(entityChange, entityRow, instanceId) {
     }
 }
 
-function updateNormalEntity(remoteEntityChange, entity, instanceId) {
+function updateNormalEntity(remoteEntityChange, remoteEntityRow, instanceId) {
     const localEntityChange = sql.getRow(`
         SELECT utcDateChanged, hash, isErased
         FROM entity_changes 
@@ -54,7 +54,7 @@ function updateNormalEntity(remoteEntityChange, entity, instanceId) {
 
             sql.execute(`DELETE FROM ${remoteEntityChange.entityName} WHERE ${primaryKey} = ?`, remoteEntityChange.entityId);
 
-            entityChangesService.addEntityChangeWithinstanceId(remoteEntityChange, instanceId);
+            entityChangesService.addEntityChangeWithInstanceId(remoteEntityChange, instanceId);
         });
 
         return true;
@@ -65,13 +65,13 @@ function updateNormalEntity(remoteEntityChange, entity, instanceId) {
         || localEntityChange.hash !== remoteEntityChange.hash // sync error, we should still update
     ) {
         if (['note_contents', 'note_revision_contents'].includes(remoteEntityChange.entityName)) {
-            entity.content = handleContent(entity.content);
+            remoteEntityRow.content = handleContent(remoteEntityRow.content);
         }
 
         sql.transactional(() => {
-            sql.replace(remoteEntityChange.entityName, entity);
+            sql.replace(remoteEntityChange.entityName, remoteEntityRow);
 
-            entityChangesService.addEntityChangeWithinstanceId(remoteEntityChange, instanceId);
+            entityChangesService.addEntityChangeWithInstanceId(remoteEntityChange, instanceId);
         });
 
         return true;
@@ -86,15 +86,16 @@ function updateNoteReordering(entityChange, entity, instanceId) {
             sql.execute("UPDATE branches SET notePosition = ? WHERE branchId = ?", [entity[key], key]);
         }
 
-        entityChangesService.addEntityChangeWithinstanceId(entityChange, instanceId);
+        entityChangesService.addEntityChangeWithInstanceId(entityChange, instanceId);
     });
 
     return true;
 }
 
 function handleContent(content) {
-    // we always use Buffer object which is different from normal saving - there we use simple string type for "string notes"
-    // the problem is that in general it's not possible to whether a note_content is string note or note (syncs can arrive out of order)
+    // we always use Buffer object which is different from normal saving - there we use simple string type for
+    // "string notes". The problem is that in general it's not possible to detect whether a note_content
+    // is string note or note (syncs can arrive out of order)
     content = content === null ? null : Buffer.from(content, 'base64');
 
     if (content && content.byteLength === 0) {
@@ -109,7 +110,7 @@ function eraseEntity(entityChange, instanceId) {
     const {entityName, entityId} = entityChange;
 
     if (!["notes", "note_contents", "branches", "attributes", "note_revisions", "note_revision_contents"].includes(entityName)) {
-        log.error(`Cannot erase entity ${entityName}, id ${entityId}`);
+        log.error(`Cannot erase entity '${entityName}', id '${entityId}'`);
         return;
     }
 
@@ -119,7 +120,7 @@ function eraseEntity(entityChange, instanceId) {
 
     eventService.emit(eventService.ENTITY_DELETE_SYNCED, { entityName, entityId });
 
-    entityChangesService.addEntityChangeWithinstanceId(entityChange, instanceId);
+    entityChangesService.addEntityChangeWithInstanceId(entityChange, instanceId);
 }
 
 module.exports = {