From 2b89bc49ab1019c383514b49cca8e8ba55a7d790 Mon Sep 17 00:00:00 2001 From: "Patrik J. Braun" Date: Sat, 28 Aug 2021 11:42:06 +0200 Subject: [PATCH] Improving denormalized data update. This change should remove the unnecessary DB updates, by making the denormalized data update when they first needed. --- .../database/interfaces/IPersonManager.ts | 4 +- .../database/interfaces/IPreviewManager.ts | 4 +- .../model/database/memory/PersonManager.ts | 7 +- .../model/database/sql/AlbumManager.ts | 55 ++++++---- .../model/database/sql/IndexingManager.ts | 17 +-- .../model/database/sql/PersonManager.ts | 101 ++++++++++++------ .../model/database/sql/PreviewManager.ts | 3 +- .../unit/model/sql/PersonManager.spec.ts | 6 +- .../unit/model/sql/PreviewManager.spec.ts | 12 --- .../unit/model/sql/SearchManager.spec.ts | 2 - 10 files changed, 128 insertions(+), 83 deletions(-) diff --git a/src/backend/model/database/interfaces/IPersonManager.ts b/src/backend/model/database/interfaces/IPersonManager.ts index 13c17520..32e4a46f 100644 --- a/src/backend/model/database/interfaces/IPersonManager.ts +++ b/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; get(name: string): Promise; - saveAll(names: string[]): Promise; + // saving a Person with a sample region. Person entry cannot exist without a face region + saveAll(person: { name: string, faceRegion: FaceRegion }[]): Promise; updatePerson(name: string, partialPerson: PersonDTO): Promise; } diff --git a/src/backend/model/database/interfaces/IPreviewManager.ts b/src/backend/model/database/interfaces/IPreviewManager.ts index a9a2f441..5b4f7653 100644 --- a/src/backend/model/database/interfaces/IPreviewManager.ts +++ b/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; - getAlbumPreview(album: SavedSearchDTO): Promise; + getAlbumPreview(album: { searchQuery: SearchQueryDTO }): Promise; } // ID is need within the backend so it can be saved to DB (ID is the external key) diff --git a/src/backend/model/database/memory/PersonManager.ts b/src/backend/model/database/memory/PersonManager.ts index bc75a19e..67f8ac15 100644 --- a/src/backend/model/database/memory/PersonManager.ts +++ b/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 { + throw new Error('not supported by memory DB'); + } getAll(): Promise { 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 { - throw new Error('not supported by memory DB'); - } onGalleryIndexUpdate(): Promise { throw new Error('not supported by memory DB'); diff --git a/src/backend/model/database/sql/AlbumManager.ts b/src/backend/model/database/sql/AlbumManager.ts index 3da855c4..9feae090 100644 --- a/src/backend/model/database/sql/AlbumManager.ts +++ b/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 { + 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 { 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 { 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 { @@ -38,6 +62,7 @@ export class AlbumManager implements IAlbumManager { } public async getAlbums(): Promise { + 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 { - await this.updateAlbums(); + this.isDBValid = false; } - private async updateAlbums(): Promise { - 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 { - 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; } } diff --git a/src/backend/model/database/sql/IndexingManager.ts b/src/backend/model/database/sql/IndexingManager.ts index 0b91b10d..be6a92a3 100644 --- a/src/backend/model/database/sql/IndexingManager.ts +++ b/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 { 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]); } } diff --git a/src/backend/model/database/sql/PersonManager.ts b/src/backend/model/database/sql/PersonManager.ts index b81bc2d2..6590016c 100644 --- a/src/backend/model/database/sql/PersonManager.ts +++ b/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 { + 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 { + 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 { + 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 { - const toSave: { name: string }[] = []; + public async saveAll(persons: { name: string, faceRegion: FaceRegion }[]): Promise { + 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 { - await this.updateCounts(); - await this.updateSamplePhotos(); - await this.loadAll(); + this.persons = null; + this.isDBValid = false; } private async loadAll(): Promise { + 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 { - 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 async updateSamplePhotos(): Promise { - 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)'); - + /** + * Person table contains derived, denormalized data for faster select, this needs to be updated after data change + * @private + */ + private async updateDerivedValues(): Promise { + if (this.isDBValid === true) { + return; + } + Logger.debug(LOG_TAG, 'Updating derived persons data'); + await PersonManager.updateCounts(); + await PersonManager.updateSamplePhotos(); + this.isDBValid = false; } } diff --git a/src/backend/model/database/sql/PreviewManager.ts b/src/backend/model/database/sql/PreviewManager.ts index 8f2bef1d..91565f12 100644 --- a/src/backend/model/database/sql/PreviewManager.ts +++ b/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 { + public async getAlbumPreview(album: { searchQuery: SearchQueryDTO }): Promise { const albumQuery = await (ObjectManagers.getInstance().SearchManager as ISQLSearchManager).prepareAndBuildWhereQuery(album.searchQuery); const connection = await SQLConnection.getConnection(); diff --git a/test/backend/unit/model/sql/PersonManager.spec.ts b/test/backend/unit/model/sql/PersonManager.spec.ts index f5a73ef9..2cd420bd 100644 --- a/test/backend/unit/model/sql/PersonManager.spec.ts +++ b/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); }); diff --git a/test/backend/unit/model/sql/PreviewManager.spec.ts b/test/backend/unit/model/sql/PreviewManager.spec.ts index c7df10ef..7ca36262 100644 --- a/test/backend/unit/model/sql/PreviewManager.spec.ts +++ b/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' diff --git a/test/backend/unit/model/sql/SearchManager.spec.ts b/test/backend/unit/model/sql/SearchManager.spec.ts index 75f55eef..54f0a73c 100644 --- a/test/backend/unit/model/sql/SearchManager.spec.ts +++ b/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];