final fixes and refactorings for consistency checks

src/services/consistency_checks.js

@@ -10,7 +10,7 @@ const cls = require('./cls');
 const syncTableService = require('./sync_table');
 const Branch = require('../entities/branch');
 
-let outstandingConsistencyErrors = false;
+let unrecoverableConsistencyErrors = false;
 let fixedIssues = false;
 
 async function findIssues(query, errorCb) {
@@ -19,7 +19,7 @@ async function findIssues(query, errorCb) {
     for (const res of results) {
         logError(errorCb(res));
 
-        outstandingConsistencyErrors = true;
+        unrecoverableConsistencyErrors = true;
     }
 
     return results;
@@ -57,7 +57,7 @@ async function checkTreeCycles() {
         if (!childToParents[noteId] || childToParents[noteId].length === 0) {
             logError(`No parents found for note ${noteId}`);
 
-            outstandingConsistencyErrors = true;
+            unrecoverableConsistencyErrors = true;
             return;
         }
 
@@ -65,7 +65,7 @@ async function checkTreeCycles() {
             if (path.includes(parentNoteId)) {
                 logError(`Tree cycle detected at parent-child relationship: ${parentNoteId} - ${noteId}, whole path: ${path}`);
 
-                outstandingConsistencyErrors = true;
+                unrecoverableConsistencyErrors = true;
             }
             else {
                 const newPath = path.slice();
@@ -84,7 +84,7 @@ async function checkTreeCycles() {
 
     if (childToParents['root'].length !== 1 || childToParents['root'][0] !== 'none') {
         logError('Incorrect root parent: ' + JSON.stringify(childToParents['root']));
-        outstandingConsistencyErrors = true;
+        unrecoverableConsistencyErrors = true;
     }
 }
 
@@ -137,24 +137,8 @@ async function findExistencyIssues() {
     // principle for fixing inconsistencies is that if the note itself is deleted (isDeleted=true) then all related entities should be also deleted (branches, links, attributes)
     // but if note is not deleted, then at least one branch should exist.
 
-    await findAndFixIssues(`
-      SELECT
-        DISTINCT noteId
-      FROM
-        notes
-      WHERE
-          (SELECT COUNT(*) FROM branches WHERE notes.noteId = branches.noteId AND branches.isDeleted = 0) = 0
-        AND notes.isDeleted = 0
-    `, async ({noteId}) => {
-        const branch = await new Branch({
-            parentNoteId: 'root',
-            noteId: noteId,
-            prefix: 'recovered'
-        }).save();
-
-        logFix(`Created missing branch ${branch.branchId} for note ${noteId}`);
-    });
-
+    // the order here is important - first we might need to delete inconsistent branches and after that
+    // another check might create missing branch
     await findAndFixIssues(`
           SELECT
             branchId, noteId
@@ -172,6 +156,42 @@ async function findExistencyIssues() {
             logFix(`Branch ${branchId} has been deleted since associated note ${noteId} is deleted.`);
         });
 
+    await findAndFixIssues(`
+      SELECT
+        branchId, parentNoteId
+      FROM
+        branches
+        JOIN notes AS parentNote ON parentNote.noteId = branches.parentNoteId
+      WHERE
+        parentNote.isDeleted = 1
+        AND branches.isDeleted = 0
+    `, async ({branchId, parentNoteId}) => {
+        const branch = await repository.getBranch(branchId);
+        branch.isDeleted = true;
+        await branch.save();
+
+        logFix(`Branch ${branchId} has been deleted since associated parent note ${parentNoteId} is deleted.`);
+    });
+
+    await findAndFixIssues(`
+      SELECT
+        DISTINCT notes.noteId
+      FROM
+        notes
+        LEFT JOIN branches ON notes.noteId = branches.noteId AND branches.isDeleted = 0
+      WHERE
+        notes.isDeleted = 0
+        AND branches.branchId IS NULL
+    `, async ({noteId}) => {
+        const branch = await new Branch({
+            parentNoteId: 'root',
+            noteId: noteId,
+            prefix: 'recovered'
+        }).save();
+
+        logFix(`Created missing branch ${branch.branchId} for note ${noteId}`);
+    });
+
     // there should be a unique relationship between note and its parent
     await findAndFixIssues(`
       SELECT
@@ -205,7 +225,9 @@ async function findLogicIssues() {
     await findIssues( `
           SELECT noteId, type 
           FROM notes 
-          WHERE type NOT IN ('text', 'code', 'render', 'file', 'image', 'search', 'relation-map')`,
+          WHERE
+            isDeleted = 0    
+            AND type NOT IN ('text', 'code', 'render', 'file', 'image', 'search', 'relation-map')`,
         ({noteId, type}) => `Note ${noteId} has invalid type=${type}`);
 
     await findIssues(`
@@ -222,7 +244,9 @@ async function findLogicIssues() {
             branches
             JOIN notes ON notes.noteId = branches.parentNoteId
           WHERE
-            type == 'search'`,
+            notes.isDeleted = 0
+            AND notes.type == 'search'
+            AND branches.isDeleted = 0`,
         ({parentNoteId}) => `Search note ${parentNoteId} has children`);
 
     await findAndFixIssues(`
@@ -246,24 +270,13 @@ async function findLogicIssues() {
             type
           FROM attributes
           WHERE 
-            type != 'label' 
+            isDeleted = 0    
+            AND type != 'label' 
             AND type != 'label-definition' 
             AND type != 'relation'
             AND type != 'relation-definition'`,
         ({attributeId, type}) => `Attribute ${attributeId} has invalid type '${type}'`);
 
-    await findIssues(`
-          SELECT 
-            attributeId,
-            attributes.noteId 
-          FROM 
-            attributes
-            LEFT JOIN notes ON attributes.noteId = notes.noteId
-          WHERE
-            attributes.isDeleted = 0
-            AND notes.noteId IS NULL`,
-        ({attributeId, noteId}) => `Attribute ${attributeId} reference to the owning note ${noteId} is broken`);
-
     await findAndFixIssues(`
           SELECT
             attributeId,
@@ -307,7 +320,7 @@ async function findLogicIssues() {
           WHERE type NOT IN ('image', 'hyper', 'relation-map')`,
         ({linkId, type}) => `Link ${linkId} has invalid type '${type}'`);
 
-    await findIssues(`
+    await findAndFixIssues(`
           SELECT
             linkId,
             links.noteId AS sourceNoteId
@@ -387,7 +400,8 @@ async function findSyncRowsIssues() {
 }
 
 async function runAllChecks() {
-    outstandingConsistencyErrors = false;
+    unrecoverableConsistencyErrors = false;
+    fixedIssues = false;
 
     await findBrokenReferenceIssues();
 
@@ -397,23 +411,22 @@ async function runAllChecks() {
 
     await findSyncRowsIssues();
 
-    if (outstandingConsistencyErrors) {
+    if (unrecoverableConsistencyErrors) {
         // we run this only if basic checks passed since this assumes basic data consistency
 
         await checkTreeCycles();
     }
 
-    return !outstandingConsistencyErrors;
+    return !unrecoverableConsistencyErrors;
 }
 
 async function runChecks() {
     let elapsedTimeMs;
-    let dbConsistent;
 
     await syncMutexService.doExclusively(async () => {
         const startTime = new Date();
 
-        dbConsistent = await runAllChecks();
+        await runAllChecks();
 
         elapsedTimeMs = new Date().getTime() - startTime.getTime();
     });
@@ -422,7 +435,7 @@ async function runChecks() {
         messagingService.sendMessageToAllClients({ type: 'refresh-tree' });
     }
 
-    if (!dbConsistent) {
+    if (unrecoverableConsistencyErrors) {
         log.info(`Consistency checks failed (took ${elapsedTimeMs}ms)`);
 
         messagingService.sendMessageToAllClients({type: 'consistency-checks-failed'});
@@ -443,7 +456,7 @@ function logError(message) {
 sqlInit.dbReady.then(() => {
     setInterval(cls.wrap(runChecks), 60 * 60 * 1000);
 
-    // kickoff backup immediately
+    // kickoff checks soon after startup (to not block the initial load)
     setTimeout(cls.wrap(runChecks), 0);
 });