[us40] Run all rules automatically after adding a new rule, a new file or updating an existing rule

Author: Musholic

src/app/database/database-backup-and-restore.service.spec.ts

@@ -165,7 +165,7 @@ describe('DatabaseBackupAndRestoreService', () => {
       mockFilesCacheService([dbBackupFile]);
 
       // Act
-      let restorePromise = lastValueFrom(databaseBackupAndRestoreService.restore(), {defaultValue: undefined});
+      let restorePromise = lastValueFrom(databaseBackupAndRestoreService.restore());
 
       // Assert
       tick();

src/app/database/database-backup-and-restore.service.ts

@@ -50,7 +50,7 @@ export class DatabaseBackupAndRestoreService {
           finalize(() => this.updateLastDbBackupTime())
         );
     } else {
-      return of();
+      return of(undefined);
     }
   }
 

src/app/rules/rule.repository.ts

@@ -2,11 +2,18 @@ import {Injectable} from '@angular/core';
 import {db} from "../database/db";
 import {DatabaseBackupAndRestoreService} from "../database/database-backup-and-restore.service";
 
+interface FileRun {
+  id: string;
+  value: boolean;
+}
+
 export interface Rule {
   id?: number;
   name: string;
   category: string[];
   script: string;
+  // List of all file ids the rule was already run on to avoid running the rule twice in the same condition
+  fileRuns?: FileRun[];
 }
 
 @Injectable()
@@ -34,6 +41,8 @@ export class RuleRepository {
   async update(rule: Rule) {
     if (rule.id) {
       await db.rules.update(rule.id, rule);
+      // TODO: Postpone backup until the end of running all rules? Or don't show backup progress?
+      //  Or don't upload it to google drive after each local backup? (and then only show message when uploading to google drive)
       this.databaseBackupAndRestoreService.backup().subscribe();
     }
   }

src/app/rules/rule.service.spec.ts

@@ -56,6 +56,11 @@ function mockElectricityBillSample(file: FileElement, fileService: FileService)
   when(() => ruleRepository.findAll())
     .thenResolve(getSampleRules());
 
+  // The first rule should be flagged as matching
+  let ruleAfterRun = getSampleRules()[0];
+  ruleAfterRun.fileRuns = [{id: file.id, value: true}];
+  when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+
   mockFilesCacheService([file], true);
 
   return service;
@@ -117,6 +122,41 @@ describe('RuleService', () => {
       // No unexpected calls to fileService.setCategory
     }));
 
+    it('should not run a rule for a file it was already run on', fakeAsync(async () => {
+      // Arrange
+      let backgroundTaskService = mockBackgroundTaskService();
+      let progress = mock<BehaviorSubject<Progress>>();
+      when(() => backgroundTaskService.showProgress("Running all rules", "", 2))
+        .thenReturn(progress);
+      when(() => progress.next({
+        index: 2,
+        value: 100,
+      })).thenReturn();
+
+      let file = mockFileElement('electricity_bill.txt');
+
+      const service = MockRender(RuleService).point.componentInstance;
+
+      let ruleRepository = ngMocks.findInstance(RuleRepository);
+      when(() => ruleRepository.findAll())
+        .thenResolve([{
+          name: 'Electric bill',
+          category: ['Electricity', 'Bills'],
+          script: 'return fileContent.startsWith("Electricity Bill");',
+          fileRuns: [{id: file.id, value: false}]
+        }]);
+
+      mockFilesCacheService([file]);
+
+      // Act
+      let runAllPromise = lastValueFrom(service.runAll(), {defaultValue: undefined});
+
+      // Assert
+      tick();
+      await runAllPromise;
+      // No failure in mock setup
+    }));
+
     it('should automatically categorize a file (using txt file content)', fakeAsync(async () => {
       // Arrange
       let backgroundTaskService = mockBackgroundTaskService();
@@ -183,6 +223,31 @@ describe('RuleService', () => {
           script: 'return false'
         }]);
 
+      // The first rule should be flagged as matching
+      let ruleAfterRun = {
+        name: 'Electric bill',
+        category: ['Electricity', 'Bills'],
+        script: 'return fileContent.startsWith("Electricity Bill");',
+        fileRuns: [{id: file.id, value: true}]
+      };
+      when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+
+      // Both rules should be flagged as not matching for the other file
+      ruleAfterRun = {
+        name: 'Electric bill',
+        category: ['Electricity', 'Bills'],
+        script: 'return fileContent.startsWith("Electricity Bill");',
+        fileRuns: [{id: file.id, value: true}, {id: otherFile.id, value: false}]
+      };
+      when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+      ruleAfterRun = {
+        name: 'Dumb rule',
+        category: ['Dumb'],
+        script: 'return false',
+        fileRuns: [{id: otherFile.id, value: false}]
+      };
+      when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+
       mockFilesCacheService([file, otherFile], true);
 
       // Act
@@ -225,7 +290,15 @@ describe('RuleService', () => {
           script: 'return fileContent.includes("test")'
         }]);
 
-      mockFilesCacheService([file], true);
+      let ruleAfterRun = {
+        name: 'Dumb file content rule',
+        category: ['Dumb'],
+        script: 'return fileContent.includes("test")',
+        fileRuns: [{id: file.id, value: false}]
+      };
+      when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+
+      mockFilesCacheService([file]);
 
       // Act
       let runAllPromise = lastValueFrom(service.runAll(), {defaultValue: undefined});
@@ -283,6 +356,15 @@ describe('RuleService', () => {
           script: 'return fileContent.startsWith("Dummy");'
         }]);
 
+      // The first rule should be flagged as matching
+      let ruleAfterRun = {
+        name: 'Dummy',
+        category: ['Dummy'],
+        script: 'return fileContent.startsWith("Dummy");',
+        fileRuns: [{id: file.id, value: true}]
+      };
+      when(() => ruleRepository.update(ruleAfterRun)).thenResolve();
+
       mockFilesCacheService([file], true);
 
       // Act

src/app/rules/rule.service.ts

@@ -1,6 +1,6 @@
 import {Injectable} from '@angular/core';
 import {FileService} from "../file-list/file.service";
-import {BehaviorSubject, concatMap, filter, find, from, last, map, mergeMap, Observable, of, tap, zip} from "rxjs";
+import {BehaviorSubject, concatMap, filter, find, from, map, mergeMap, Observable, of, tap, zip} from "rxjs";
 import {FileElement, isFileElement} from "../file-list/file-list.component";
 import {Rule, RuleRepository} from "./rule.repository";
 import {FilesCacheService} from "../files-cache/files-cache.service";
@@ -34,25 +34,42 @@ export class RuleService {
     this.worker = new Worker(new URL('./rule.worker', import.meta.url));
     const pdfWorkerSrc = `https://cdnjs.cloudflare.com/ajax/libs/pdf.js/${pdfjs.version}/pdf.worker.min.js`;
     pdfjs.GlobalWorkerOptions.workerSrc = pdfWorkerSrc;
+
+    //TODO: mark each file to know which rules have already been run and when, then every time we load the page,
+    // we check for all pair of rules/files which have not run or are outdated
+  }
+
+  private static isRuleRunNeeded(rules: Rule[], file: FileElement) {
+    for (const rule of rules) {
+      let previousFileRun = rule.fileRuns?.find(fileRun => fileRun.id === file.id);
+      if (previousFileRun && previousFileRun.value) {
+        // We already know the matching rule
+        return false;
+      }
+      if (!previousFileRun) {
+        // There is a rule we need to run which has not been run before
+        return true;
+      }
+    }
+    // All rules have already been run
+    return false;
   }
 
   runAll(): Observable<void> {
     return from(this.ruleRepository.findAll())
       .pipe(mergeMap(rules => {
         let fileOrFolders = this.filesCacheService.getAll()
         // Get all files
-        let files = fileOrFolders.filter(file => isFileElement(file))
-          .map(value => value as FileElement);
+        let files = fileOrFolders
+          .filter((file): file is FileElement => isFileElement(file));
+
 
         // Run the script for each file to get the associated category
         // The amount of step is one download per file and one per rule running for each file
         let stepAmount = files.length * (1 + rules.length);
         let progress = this.backgroundTaskService.showProgress('Running all rules', '', stepAmount);
-        return this.computeFileToCategoryMap(files, rules, progress)
-          .pipe(mergeMap(fileToCategoryMap => {
-            // Set the computed category for each files
-            return this.setAllFileCategory(fileToCategoryMap);
-          }), tap({complete: () => progress.next({value: 100, index: stepAmount})}))
+        return this.runAllAndSetCategories(files, rules, progress)
+          .pipe(tap({complete: () => progress.next({value: 100, index: stepAmount})}));
       }));
   }
 
@@ -73,63 +90,97 @@ export class RuleService {
   }
 
   /**
-   * Run the given rules on the given files and return the associated category for each file that got a matching rule
+   * Run the given rules on the given files and set the associated category for each file that got a matching rule
    */
-  private computeFileToCategoryMap(files: FileElement[], rules: Rule[], progress: BehaviorSubject<Progress>) {
-    let fileToCategoryMap = new Map<FileElement, string[]>();
+  private runAllAndSetCategories(files: FileElement[], rules: Rule[], progress: BehaviorSubject<Progress>) {
     return zip(from(files)
       .pipe(concatMap((file, fileIndex) => {
         let progressIndex = 1 + fileIndex * (rules.length + 1);
-
-        let fileContentObservable: Observable<string>;
-        if (this.isFileContentReadable(file)) {
-          progress.next({
-            index: progressIndex,
-            value: 0,
-            description: "Downloading file content of '" + file.name + "'"
-          });
-          fileContentObservable = this.fileService.downloadFile(file, progress)
-            .pipe(mergeMap(blobContent => {
-              if (file.mimeType === 'application/pdf') {
-                return fromPromise(blobContent.arrayBuffer()
-                  .then(arrayBuffer => pdfjs.getDocument(arrayBuffer).promise)
-                  .then(pdfDocument => pdfDocument.getPage(1))
-                  .then(firstPage => firstPage.getTextContent())
-                  .then(textContent => textContent.items
-                    .filter((item): item is TextItem => item !== undefined)
-                    .map(item => "" + item.str).join()));
-              } else {
-                return fromPromise(blobContent.text());
-              }
-            }));
-        } else {
-          fileContentObservable = of("");
+        if (!RuleService.isRuleRunNeeded(rules, file)) {
+          return of(undefined);
         }
-        return fileContentObservable.pipe(
+        return this.getFileContent(file, progress, progressIndex).pipe(
           mergeMap(fileContent => {
             // Find the first rule which matches
-            return from(rules).pipe(concatMap((rule, ruleIndex) => {
-                progress.next({
-                  index: progressIndex + 1 + ruleIndex,
-                  value: 0,
-                  description: "Running rule '" + rule.name + "' for '" + file.name + "'"
-                });
-                return this.run(rule, file, fileContent, progress, progressIndex + 1 + ruleIndex);
-              }),
-              // Find will stop running further scripts once we got a match
-              find(result => {
-                return result.value;
-              }),
-              map(result => {
+            return this.runAllRules(rules, progress, progressIndex, file, fileContent)
+              .pipe(mergeMap(result => {
+                // TODO: do the call to change the category immediately instead of constructing this map
+                // TODO: How to handle rules that have not run due to finding another matching rule? flag the matching files?
                 if (result) {
-                  fileToCategoryMap.set(file, result.rule.category);
+                  return this.findOrCreateCategories(Object.assign([], result.rule.category), this.filesCacheService.getBaseFolder())
+                    // There is no need to set the category if the current category is correct
+                    .pipe(filter(categoryId => file.parentId !== categoryId),
+                      mergeMap(categoryId => {
+                        return this.fileService.setCategory(file.id, categoryId);
+                      }));
+                } else {
+                  return of();
                 }
               }));
           }));
       })))
-      .pipe(last(),
-        map(() => fileToCategoryMap));
+      .pipe(map(() => {
+      }));
+
+  }
 
+  private runAllRules(rulesToRun: Rule[], progress: BehaviorSubject<Progress>, progressIndex: number, file: FileElement, fileContent: string) {
+    return from(rulesToRun).pipe(concatMap((rule, ruleIndex) => {
+        let previousFileRun = rule.fileRuns?.find(fileRun => fileRun.id === file.id);
+        if (previousFileRun) {
+          // The rule was run previously, so we already know the result
+          let result: RuleResult = {
+            rule: rule,
+            value: previousFileRun.value
+          };
+          return of(result);
+        }
+        progress.next({
+          index: progressIndex + 1 + ruleIndex,
+          value: 0,
+          description: "Running rule '" + rule.name + "' for '" + file.name + "'"
+        });
+        return this.run(rule, file, fileContent, progress, progressIndex + 1 + ruleIndex)
+          .pipe(tap((result) => {
+            // Add this file run to the rule fileRuns to avoid doing the same run again
+            let rule = result.rule;
+            if (!rule.fileRuns) {
+              rule.fileRuns = [];
+            }
+            rule.fileRuns.push({id: file.id, value: result.value});
+            this.ruleRepository.update(rule);
+          }));
+      }),
+      // Find will stop running further scripts once we got a match
+      find(result => {
+        return result.value;
+      }));
+  }
+
+  private getFileContent(file: FileElement, progress: BehaviorSubject<Progress>, progressIndex: number) {
+    if (this.isFileContentReadable(file)) {
+      progress.next({
+        index: progressIndex,
+        value: 0,
+        description: "Downloading file content of '" + file.name + "'"
+      });
+      return this.fileService.downloadFile(file, progress)
+        .pipe(mergeMap(blobContent => {
+          if (file.mimeType === 'application/pdf') {
+            return fromPromise(blobContent.arrayBuffer()
+              .then(arrayBuffer => pdfjs.getDocument(arrayBuffer).promise)
+              .then(pdfDocument => pdfDocument.getPage(1))
+              .then(firstPage => firstPage.getTextContent())
+              .then(textContent => textContent.items
+                .filter((item): item is TextItem => item !== undefined)
+                .map(item => "" + item.str).join()));
+          } else {
+            return fromPromise(blobContent.text());
+          }
+        }));
+    } else {
+      return of("");
+    }
   }
 
   private isFileContentReadable(file: FileElement) {
@@ -151,27 +202,7 @@ export class RuleService {
     });
   }
 
-  /**
-   * Find or create the categories for each file and associate them
-   */
-  private setAllFileCategory(fileToCategoryMap: Map<FileElement, string[]>): Observable<void> {
-    let baseFolderId = this.filesCacheService.getBaseFolder();
-    let categoryRequests: Observable<void>[] = [];
-    fileToCategoryMap
-      .forEach((category, file) => {
-        categoryRequests.push(this.findOrCreateCategories(category, baseFolderId)
-          // There is no need to set the category if the current category is correct
-          .pipe(filter(categoryId => file.parentId !== categoryId),
-            mergeMap(categoryId => {
-              return this.fileService.setCategory(file.id, categoryId);
-            })));
-      });
-    return zip(categoryRequests).pipe(map(() => {
-    }));
-  }
-
   // TODO: move and refactor duplicate to FileService
-
   private findOrCreateCategories(categories: string[], categoryId: string): Observable<string> {
     let categoryName = categories.shift();
     if (categoryName !== undefined) {