Improving denormalized data update.

This change should remove the unnecessary DB updates, by making the denormalized data update when they first needed.

src/backend/model/database/interfaces/IPersonManager.ts

@@ -1,13 +1,15 @@
 import {PersonEntry} from '../sql/enitites/PersonEntry';
 import {PersonDTO} from '../../../../common/entities/PersonDTO';
 import {IObjectManager} from './IObjectManager';
+import {FaceRegion} from '../../../../common/entities/PhotoDTO';
 
 export interface IPersonManager extends IObjectManager {
   getAll(): Promise<PersonEntry[]>;
 
   get(name: string): Promise<PersonEntry>;
 
-  saveAll(names: string[]): Promise<void>;
+  // saving a Person with a sample region. Person entry cannot exist without a face region
+  saveAll(person: { name: string, faceRegion: FaceRegion }[]): Promise<void>;
 
   updatePerson(name: string, partialPerson: PersonDTO): Promise<PersonEntry>;
 }

src/backend/model/database/interfaces/IPreviewManager.ts

@@ -1,11 +1,11 @@
-import {SavedSearchDTO} from '../../../../common/entities/album/SavedSearchDTO';
 import {PreviewPhotoDTO} from '../../../../common/entities/PhotoDTO';
 import {IObjectManager} from './IObjectManager';
+import {SearchQueryDTO} from '../../../../common/entities/SearchQueryDTO';
 
 export interface IPreviewManager extends IObjectManager {
   getPreviewForDirectory(dir: { id: number, name: string, path: string }): Promise<PreviewPhotoDTOWithID>;
 
-  getAlbumPreview(album: SavedSearchDTO): Promise<PreviewPhotoDTOWithID>;
+  getAlbumPreview(album: { searchQuery: SearchQueryDTO }): Promise<PreviewPhotoDTOWithID>;
 }
 
 // ID is need within the backend so it can be saved to DB (ID is the external key)

src/backend/model/database/memory/PersonManager.ts

@@ -1,7 +1,11 @@
 import {IPersonManager} from '../interfaces/IPersonManager';
 import {PersonDTO} from '../../../../common/entities/PersonDTO';
+import {FaceRegion} from '../../../../common/entities/PhotoDTO';
 
 export class PersonManager implements IPersonManager {
+  saveAll(person: { name: string; faceRegion: FaceRegion }[]): Promise<void> {
+    throw new Error('not supported by memory DB');
+  }
 
   getAll(): Promise<any[]> {
     throw new Error('not supported by memory DB');
@@ -11,9 +15,6 @@ export class PersonManager implements IPersonManager {
     throw new Error('not supported by memory DB');
   }
 
-  saveAll(names: string[]): Promise<void> {
-    throw new Error('not supported by memory DB');
-  }
 
   onGalleryIndexUpdate(): Promise<void> {
     throw new Error('not supported by memory DB');

src/backend/model/database/sql/AlbumManager.ts

@@ -7,8 +7,32 @@ import {ISQLSearchManager} from './ISearchManager';
 import {SearchQueryDTO} from '../../../../common/entities/SearchQueryDTO';
 import {SavedSearchEntity} from './enitites/album/SavedSearchEntity';
 import {IAlbumManager} from '../interfaces/IAlbumManager';
+import {Logger} from '../../../Logger';
+
+const LOG_TAG = '[AlbumManager]';
 
 export class AlbumManager implements IAlbumManager {
+
+  /**
+   * Person table contains denormalized data that needs to update when isDBValid = false
+   */
+  private isDBValid = false;
+
+  private static async updateAlbum(album: SavedSearchEntity): Promise<void> {
+    const connection = await SQLConnection.getConnection();
+    const preview = await ObjectManagers.getInstance().PreviewManager
+      .getAlbumPreview(album);
+    const count = await (ObjectManagers.getInstance().SearchManager as ISQLSearchManager)
+      .getCount((album as SavedSearchDTO).searchQuery);
+
+    await connection
+      .createQueryBuilder()
+      .update(AlbumBaseEntity)
+      .set({preview, count})
+      .where('id = :id', {id: album.id})
+      .execute();
+  }
+
   public async addIfNotExistSavedSearch(name: string, searchQuery: SearchQueryDTO, lockedAlbum: boolean): Promise<void> {
     const connection = await SQLConnection.getConnection();
     const album = await connection.getRepository(SavedSearchEntity)
@@ -22,7 +46,7 @@ export class AlbumManager implements IAlbumManager {
   public async addSavedSearch(name: string, searchQuery: SearchQueryDTO, lockedAlbum?: boolean): Promise<void> {
     const connection = await SQLConnection.getConnection();
     const a = await connection.getRepository(SavedSearchEntity).save({name, searchQuery, locked: lockedAlbum});
-    await this.updateAlbum(a);
+    await AlbumManager.updateAlbum(a);
   }
 
   public async deleteAlbum(id: number): Promise<void> {
@@ -38,6 +62,7 @@ export class AlbumManager implements IAlbumManager {
   }
 
   public async getAlbums(): Promise<AlbumBaseDTO[]> {
+    await this.updateAlbums();
     const connection = await SQLConnection.getConnection();
     return await connection.getRepository(AlbumBaseEntity)
       .createQueryBuilder('album')
@@ -49,31 +74,21 @@ export class AlbumManager implements IAlbumManager {
   }
 
   public async onNewDataVersion(): Promise<void> {
-    await this.updateAlbums();
+    this.isDBValid = false;
   }
 
-
   private async updateAlbums(): Promise<void> {
-    const albums = await this.getAlbums();
+    if (this.isDBValid === true) {
+      return;
+    }
+    Logger.debug(LOG_TAG, 'Updating derived album data');
+    const connection = await SQLConnection.getConnection();
+    const albums = await connection.getRepository(AlbumBaseEntity).find();
 
     for (const a of albums) {
-      await this.updateAlbum(a as SavedSearchEntity);
+      await AlbumManager.updateAlbum(a as SavedSearchEntity);
     }
-  }
-
-  private async updateAlbum(album: SavedSearchEntity): Promise<void> {
-    const connection = await SQLConnection.getConnection();
-    const preview = await ObjectManagers.getInstance().PreviewManager
-      .getAlbumPreview(album);
-    const count = await (ObjectManagers.getInstance().SearchManager as ISQLSearchManager)
-      .getCount((album as SavedSearchDTO).searchQuery);
-
-    await connection
-      .createQueryBuilder()
-      .update(AlbumBaseEntity)
-      .set({preview, count})
-      .where('id = :id', {id: album.id})
-      .execute();
+    this.isDBValid = true;
   }
 
 }

src/backend/model/database/sql/IndexingManager.ts

@@ -22,6 +22,7 @@ import {ProjectPath} from '../../../ProjectPath';
 import * as path from 'path';
 import * as fs from 'fs';
 import {SearchQueryDTO} from '../../../../common/entities/SearchQueryDTO';
+import {PersonEntry} from './enitites/PersonEntry';
 
 const LOG_TAG = '[IndexingManager]';
 
@@ -328,16 +329,18 @@ export class IndexingManager implements IIndexingManager {
 
   protected async saveFaces(connection: Connection, parentDirId: number, scannedFaces: FaceRegion[]): Promise<void> {
     const faceRepository = connection.getRepository(FaceRegionEntry);
+    const personRepository = connection.getRepository(PersonEntry);
 
-    const persons: string[] = [];
+    const persons: { name: string, faceRegion: FaceRegion }[] = [];
 
     for (const face of scannedFaces) {
-      if (persons.indexOf(face.name) === -1) {
-        persons.push(face.name);
+      if (persons.findIndex(f => f.name === face.name) === -1) {
+        persons.push({name: face.name, faceRegion: face});
       }
     }
     await ObjectManagers.getInstance().PersonManager.saveAll(persons);
-
+    // get saved persons without triggering denormalized data update (i.e.: do not use PersonManager.get).
+    const savedPersons = await personRepository.find();
 
     const indexedFaces = await faceRepository.createQueryBuilder('face')
       .leftJoin('face.media', 'media')
@@ -351,6 +354,8 @@ export class IndexingManager implements IIndexingManager {
     const faceToInsert = [];
     // tslint:disable-next-line:prefer-for-of
     for (let i = 0; i < scannedFaces.length; i++) {
+
+      // was the face region already indexed
       let face: FaceRegionEntry = null;
       for (let j = 0; j < indexedFaces.length; j++) {
         if (indexedFaces[j].box.height === scannedFaces[i].box.height &&
@@ -360,12 +365,12 @@ export class IndexingManager implements IIndexingManager {
           indexedFaces[j].person.name === scannedFaces[i].name) {
           face = indexedFaces[j];
           indexedFaces.splice(j, 1);
-          break;
+          break; // region found, stop processing
         }
       }
 
       if (face == null) {
-        (scannedFaces[i] as FaceRegionEntry).person = await ObjectManagers.getInstance().PersonManager.get(scannedFaces[i].name);
+        (scannedFaces[i] as FaceRegionEntry).person = savedPersons.find(p => p.name === scannedFaces[i].name);
         faceToInsert.push(scannedFaces[i]);
       }
     }

src/backend/model/database/sql/PersonManager.ts

@@ -3,13 +3,46 @@ import {PersonEntry} from './enitites/PersonEntry';
 import {FaceRegionEntry} from './enitites/FaceRegionEntry';
 import {PersonDTO} from '../../../../common/entities/PersonDTO';
 import {ISQLPersonManager} from './IPersonManager';
+import {Logger} from '../../../Logger';
+import {FaceRegion} from '../../../../common/entities/PhotoDTO';
 
 
+const LOG_TAG = '[PersonManager]';
+
 export class PersonManager implements ISQLPersonManager {
-  // samplePhotos: { [key: string]: PhotoDTO } = {};
   persons: PersonEntry[] = null;
+  /**
+   * Person table contains denormalized data that needs to update when isDBValid = false
+   */
+  private isDBValid = false;
+
+  private static async updateCounts(): Promise<void> {
+    const connection = await SQLConnection.getConnection();
+    await connection.query('UPDATE person_entry SET count = ' +
+      ' (SELECT COUNT(1) FROM face_region_entry WHERE face_region_entry.personId = person_entry.id)');
+
+    // remove persons without photo
+    await connection
+      .createQueryBuilder()
+      .delete()
+      .from(PersonEntry)
+      .where('count = 0')
+      .execute();
+  }
+
+  private static async updateSamplePhotos(): Promise<void> {
+    const connection = await SQLConnection.getConnection();
+    await connection.query('update person_entry set sampleRegionId = ' +
+      '(Select face_region_entry.id from  media_entity ' +
+      'left join face_region_entry on media_entity.id = face_region_entry.mediaId ' +
+      'where face_region_entry.personId=person_entry.id ' +
+      'order by media_entity.metadataCreationdate desc ' +
+      'limit 1)');
+
+  }
 
   async updatePerson(name: string, partialPerson: PersonDTO): Promise<PersonEntry> {
+    this.isDBValid = false;
     const connection = await SQLConnection.getConnection();
     const repository = connection.getRepository(PersonEntry);
     const person = await repository.createQueryBuilder('person')
@@ -54,36 +87,45 @@ export class PersonManager implements ISQLPersonManager {
     return this.persons.find((p): boolean => p.name === name);
   }
 
-  public async saveAll(names: string[]): Promise<void> {
-    const toSave: { name: string }[] = [];
+  public async saveAll(persons: { name: string, faceRegion: FaceRegion }[]): Promise<void> {
+    const toSave: { name: string, faceRegion: FaceRegion }[] = [];
     const connection = await SQLConnection.getConnection();
     const personRepository = connection.getRepository(PersonEntry);
-    await this.loadAll();
+    const faceRegionRepository = connection.getRepository(FaceRegionEntry);
 
-    for (const item of names) {
+    const savedPersons =  await personRepository.find();
+    // filter already existing persons
+    for (const personToSave of persons) {
 
-      const person = this.persons.find((p): boolean => p.name === item);
+      const person = savedPersons.find((p): boolean => p.name === personToSave.name);
       if (!person) {
-        toSave.push({name: item});
+        toSave.push(personToSave);
       }
     }
 
+
     if (toSave.length > 0) {
       for (let i = 0; i < toSave.length / 200; i++) {
-        await personRepository.insert(toSave.slice(i * 200, (i + 1) * 200));
+        const saving = toSave.slice(i * 200, (i + 1) * 200);
+        const inserted = await personRepository.insert(saving.map(p => ({name: p.name})));
+        // setting Person id
+        inserted.identifiers.forEach((idObj: { id: number }, j: number) => {
+          (saving[j].faceRegion as FaceRegionEntry).person = idObj as any;
+        });
+        await faceRegionRepository.insert(saving.map(p => p.faceRegion));
       }
-      await this.loadAll();
     }
+    this.isDBValid = false;
 
   }
 
   public async onNewDataVersion(): Promise<void> {
-    await this.updateCounts();
-    await this.updateSamplePhotos();
-    await this.loadAll();
+    this.persons = null;
+    this.isDBValid = false;
   }
 
   private async loadAll(): Promise<void> {
+    await this.updateDerivedValues();
     const connection = await SQLConnection.getConnection();
     const personRepository = connection.getRepository(PersonEntry);
     this.persons = await personRepository.find({
@@ -93,29 +135,18 @@ export class PersonManager implements ISQLPersonManager {
     });
   }
 
-  private async updateCounts(): Promise<void> {
-    const connection = await SQLConnection.getConnection();
-    await connection.query('UPDATE person_entry SET count = ' +
-      ' (SELECT COUNT(1) FROM face_region_entry WHERE face_region_entry.personId = person_entry.id)');
-
-    // remove persons without photo
-    await connection
-      .createQueryBuilder()
-      .delete()
-      .from(PersonEntry)
-      .where('count = 0')
-      .execute();
+  /**
+   * Person table contains derived, denormalized data for faster select, this needs to be updated after data change
+   * @private
+   */
+  private async updateDerivedValues(): Promise<void> {
+    if (this.isDBValid === true) {
+      return;
     }
-
-  private async updateSamplePhotos(): Promise<void> {
-    const connection = await SQLConnection.getConnection();
-    await connection.query('update person_entry set sampleRegionId = ' +
-      '(Select face_region_entry.id from  media_entity ' +
-      'left join face_region_entry on media_entity.id = face_region_entry.mediaId ' +
-      'where face_region_entry.personId=person_entry.id ' +
-      'order by media_entity.metadataCreationdate desc ' +
-      'limit 1)');
-
+    Logger.debug(LOG_TAG, 'Updating derived persons data');
+    await PersonManager.updateCounts();
+    await PersonManager.updateSamplePhotos();
+    this.isDBValid = false;
   }
 
 }

src/backend/model/database/sql/PreviewManager.ts

@@ -9,6 +9,7 @@ import {ISQLSearchManager} from './ISearchManager';
 import {IPreviewManager, PreviewPhotoDTOWithID} from '../interfaces/IPreviewManager';
 import {SQLConnection} from './SQLConnection';
 import {SavedSearchDTO} from '../../../../common/entities/album/SavedSearchDTO';
+import {SearchQueryDTO} from '../../../../common/entities/SearchQueryDTO';
 
 
 const LOG_TAG = '[PreviewManager]';
@@ -45,7 +46,7 @@ export class PreviewManager implements IPreviewManager {
     return query;
   }
 
-  public async getAlbumPreview(album: SavedSearchDTO): Promise<PreviewPhotoDTOWithID> {
+  public async getAlbumPreview(album: { searchQuery: SearchQueryDTO }): Promise<PreviewPhotoDTOWithID> {
 
     const albumQuery = await (ObjectManagers.getInstance().SearchManager as ISQLSearchManager).prepareAndBuildWhereQuery(album.searchQuery);
     const connection = await SQLConnection.getConnection();

test/backend/unit/model/sql/PersonManager.spec.ts

@@ -67,7 +67,11 @@ describe('PersonManager', (sqlHelper: DBTestHelper) => {
     const pm = new PersonManager();
     const person = Utils.clone(savedPerson[0]);
 
-    expect(await pm.get('Boba Fett')).to.deep.equal(person);
+    const selected = Utils.clone(await pm.get('Boba Fett'));
+    delete selected.sampleRegion;
+    delete person.sampleRegion;
+    person.count = 1;
+    expect(selected).to.deep.equal(person);
 
     expect((await pm.get('Boba Fett') as PersonWithSampleRegion).sampleRegion.media.name).to.deep.equal(p.name);
   });

test/backend/unit/model/sql/PreviewManager.spec.ts

@@ -181,10 +181,6 @@ describe('PreviewManager', (sqlHelper: DBTestHelper) => {
     const pm = new PreviewManager();
     Config.Server.Preview.SearchQuery = null;
     expect(Utils.clone(await pm.getAlbumPreview({
-      name: 'test',
-      id: 0,
-      count: 0,
-      locked: false,
       searchQuery: {
         type: SearchQueryTypes.any_text,
         text: 'sw'
@@ -192,10 +188,6 @@ describe('PreviewManager', (sqlHelper: DBTestHelper) => {
     }))).to.deep.equalInAnyOrder(previewifyMedia(p4));
     Config.Server.Preview.SearchQuery = {type: SearchQueryTypes.any_text, text: 'Boba'} as TextSearch;
     expect(Utils.clone(await pm.getAlbumPreview({
-      name: 'test',
-      id: 0,
-      count: 0,
-      locked: false,
       searchQuery: {
         type: SearchQueryTypes.any_text,
         text: 'sw'
@@ -203,10 +195,6 @@ describe('PreviewManager', (sqlHelper: DBTestHelper) => {
     }))).to.deep.equalInAnyOrder(previewifyMedia(p));
     Config.Server.Preview.SearchQuery = {type: SearchQueryTypes.any_text, text: 'Derem'} as TextSearch;
     expect(Utils.clone(await pm.getAlbumPreview({
-      name: 'test',
-      id: 0,
-      count: 0,
-      locked: false,
       searchQuery: {
         type: SearchQueryTypes.any_text,
         text: 'sw'

test/backend/unit/model/sql/SearchManager.spec.ts

@@ -1171,8 +1171,6 @@ describe('SearchManager', (sqlHelper: DBTestHelper) => {
 
           if (lengths[0] !== lengths[lengths.length - 1]) {
             for (const l of (q as SearchListQuery).list) {
-              console.log(shortestDepth(l));
-              console.log(parser.stringify(l));
             }
           }
           return lengths[0];