From ef88336a2da2ac40c531776734ca7f801aad03a2 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Wed, 17 Jan 2024 17:00:26 +0100 Subject: [PATCH] MOBILE-4304 h5p: Update database usage --- .../classes/database/database-table-proxy.ts | 7 + src/core/classes/database/database-table.ts | 12 + .../classes/database/debug-database-table.ts | 9 + .../classes/database/eager-database-table.ts | 19 +- .../classes/database/lazy-database-table.ts | 15 ++ src/core/features/h5p/classes/framework.ts | 245 +++++++++++------- 6 files changed, 215 insertions(+), 92 deletions(-) diff --git a/src/core/classes/database/database-table-proxy.ts b/src/core/classes/database/database-table-proxy.ts index 64762a0cf..7fd63308c 100644 --- a/src/core/classes/database/database-table-proxy.ts +++ b/src/core/classes/database/database-table-proxy.ts @@ -187,6 +187,13 @@ export class CoreDatabaseTableProxy< return this.target.delete(conditions); } + /** + * @inheritdoc + */ + async deleteWhere(conditions: CoreDatabaseConditions): Promise { + return this.target.deleteWhere(conditions); + } + /** * @inheritdoc */ diff --git a/src/core/classes/database/database-table.ts b/src/core/classes/database/database-table.ts index 5cb2131fb..c52cb7352 100644 --- a/src/core/classes/database/database-table.ts +++ b/src/core/classes/database/database-table.ts @@ -311,6 +311,18 @@ export class CoreDatabaseTable< : await this.database.deleteRecords(this.tableName); } + /** + * Delete records matching the given conditions. + * + * This method should be used when it's necessary to apply complex conditions; the simple `delete` + * method should be favored otherwise for better performance. + * + * @param conditions Matching conditions in SQL and JavaScript. + */ + async deleteWhere(conditions: CoreDatabaseConditions): Promise { + await this.database.deleteRecordsSelect(this.tableName, conditions.sql, conditions.sqlParams); + } + /** * Delete a single record identified by its primary key. * diff --git a/src/core/classes/database/debug-database-table.ts b/src/core/classes/database/debug-database-table.ts index c3906df25..cdef34947 100644 --- a/src/core/classes/database/debug-database-table.ts +++ b/src/core/classes/database/debug-database-table.ts @@ -186,6 +186,15 @@ export class CoreDebugDatabaseTable< return this.target.delete(conditions); } + /** + * @inheritdoc + */ + async deleteWhere(conditions: CoreDatabaseConditions): Promise { + this.logger.log('deleteWhere', conditions); + + return this.target.deleteWhere(conditions); + } + /** * @inheritdoc */ diff --git a/src/core/classes/database/eager-database-table.ts b/src/core/classes/database/eager-database-table.ts index 3ed457c50..c5c75f9c9 100644 --- a/src/core/classes/database/eager-database-table.ts +++ b/src/core/classes/database/eager-database-table.ts @@ -202,12 +202,27 @@ export class CoreEagerDatabaseTable< return; } - Object.entries(this.records).forEach(([id, record]) => { + Object.entries(this.records).forEach(([primaryKey, record]) => { if (!this.recordMatches(record, conditions)) { return; } - delete this.records[id]; + delete this.records[primaryKey]; + }); + } + + /** + * @inheritdoc + */ + async deleteWhere(conditions: CoreDatabaseConditions): Promise { + await super.deleteWhere(conditions); + + Object.entries(this.records).forEach(([primaryKey, record]) => { + if (!conditions.js(record)) { + return; + } + + delete record[primaryKey]; }); } diff --git a/src/core/classes/database/lazy-database-table.ts b/src/core/classes/database/lazy-database-table.ts index c8dd76c31..c0e049e43 100644 --- a/src/core/classes/database/lazy-database-table.ts +++ b/src/core/classes/database/lazy-database-table.ts @@ -189,6 +189,21 @@ export class CoreLazyDatabaseTable< } } + /** + * @inheritdoc + */ + async deleteWhere(conditions: CoreDatabaseConditions): Promise { + await super.deleteWhere(conditions); + + Object.entries(this.records).forEach(([primaryKey, record]) => { + if (!record || !conditions.js(record)) { + return; + } + + this.records[primaryKey] = null; + }); + } + /** * @inheritdoc */ diff --git a/src/core/features/h5p/classes/framework.ts b/src/core/features/h5p/classes/framework.ts index 89921a8ab..fb6c79080 100644 --- a/src/core/features/h5p/classes/framework.ts +++ b/src/core/features/h5p/classes/framework.ts @@ -44,12 +44,85 @@ import { CoreH5PLibraryAddTo, CoreH5PLibraryMetadataSettings } from './validator import { CoreH5PMetadata } from './metadata'; import { Translate } from '@singletons'; import { SQLiteDB } from '@classes/sqlitedb'; +import { AsyncInstance, asyncInstance } from '@/core/utils/async-instance'; +import { LazyMap, lazyMap } from '@/core/utils/lazy-map'; +import { CoreDatabaseTable } from '@classes/database/database-table'; +import { CoreDatabaseCachingStrategy } from '@classes/database/database-table-proxy'; /** * Equivalent to Moodle's implementation of H5PFrameworkInterface. */ export class CoreH5PFramework { + protected contentTables: LazyMap>>; + protected librariesTables: LazyMap>>; + protected libraryDependenciesTables: LazyMap>>; + protected contentsLibrariesTables: LazyMap>>; + protected librariesCachedAssetsTables: LazyMap>>; + + constructor() { + this.contentTables = lazyMap( + siteId => asyncInstance( + () => CoreSites.getSiteTable( + CONTENT_TABLE_NAME, + { + siteId, + config: { cachingStrategy: CoreDatabaseCachingStrategy.None }, + onDestroy: () => delete this.contentTables[siteId], + }, + ), + ), + ); + this.librariesTables = lazyMap( + siteId => asyncInstance( + () => CoreSites.getSiteTable( + LIBRARIES_TABLE_NAME, + { + siteId, + config: { cachingStrategy: CoreDatabaseCachingStrategy.None }, + onDestroy: () => delete this.librariesTables[siteId], + }, + ), + ), + ); + this.libraryDependenciesTables = lazyMap( + siteId => asyncInstance( + () => CoreSites.getSiteTable( + LIBRARY_DEPENDENCIES_TABLE_NAME, + { + siteId, + config: { cachingStrategy: CoreDatabaseCachingStrategy.None }, + onDestroy: () => delete this.libraryDependenciesTables[siteId], + }, + ), + ), + ); + this.contentsLibrariesTables = lazyMap( + siteId => asyncInstance( + () => CoreSites.getSiteTable( + CONTENTS_LIBRARIES_TABLE_NAME, + { + siteId, + config: { cachingStrategy: CoreDatabaseCachingStrategy.None }, + onDestroy: () => delete this.contentsLibrariesTables[siteId], + }, + ), + ), + ); + this.librariesCachedAssetsTables = lazyMap( + siteId => asyncInstance( + () => CoreSites.getSiteTable( + LIBRARIES_CACHEDASSETS_TABLE_NAME, + { + siteId, + config: { cachingStrategy: CoreDatabaseCachingStrategy.None }, + onDestroy: () => delete this.librariesCachedAssetsTables[siteId], + }, + ), + ), + ); + } + /** * Will clear filtered params for all the content that uses the specified libraries. * This means that the content dependencies will have to be rebuilt and the parameters re-filtered. @@ -63,12 +136,19 @@ export class CoreH5PFramework { return; } - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); const whereAndParams = SQLiteDB.getInOrEqual(libraryIds); whereAndParams.sql = 'mainlibraryid ' + whereAndParams.sql; - await db.updateRecordsWhere(CONTENT_TABLE_NAME, { filtered: null }, whereAndParams.sql, whereAndParams.params); + await this.contentTables[siteId].updateWhere( + { filtered: null }, + { + sql: whereAndParams.sql, + sqlParams: whereAndParams.params, + js: record => libraryIds.includes(record.mainlibraryid), + }, + ); } /** @@ -79,20 +159,19 @@ export class CoreH5PFramework { * @returns Promise resolved with the removed entries. */ async deleteCachedAssets(libraryId: number, siteId?: string): Promise { - - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); // Get all the hashes that use this library. - const entries = await db.getRecords( - LIBRARIES_CACHEDASSETS_TABLE_NAME, - { libraryid: libraryId }, - ); - + const entries = await this.librariesCachedAssetsTables[siteId].getMany({ libraryid: libraryId }); const hashes = entries.map((entry) => entry.hash); if (hashes.length) { // Delete the entries from DB. - await db.deleteRecordsList(LIBRARIES_CACHEDASSETS_TABLE_NAME, 'hash', hashes); + await this.librariesCachedAssetsTables[siteId].deleteWhere({ + sql: hashes.length === 1 ? 'hash = ?' : `hash IN (${hashes.map(() => '?').join(', ')})`, + sqlParams: hashes, + js: (record) => hashes.includes(record.hash), + }); } return entries; @@ -106,8 +185,7 @@ export class CoreH5PFramework { * @returns Promise resolved when done. */ async deleteContentData(id: number, siteId?: string): Promise { - - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); // The user content should be reset (instead of removed), because this method is called when H5P content needs // to be updated too (and the previous states must be kept, but reset). @@ -115,7 +193,7 @@ export class CoreH5PFramework { await Promise.all([ // Delete the content data. - db.deleteRecords(CONTENT_TABLE_NAME, { id }), + this.contentTables[siteId].deleteByPrimaryKey({ id }), // Remove content library dependencies. this.deleteLibraryUsage(id, siteId), @@ -130,9 +208,9 @@ export class CoreH5PFramework { * @returns Promise resolved when done. */ async deleteLibrary(id: number, siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - await db.deleteRecords(LIBRARIES_TABLE_NAME, { id }); + await this.librariesTables[siteId].deleteByPrimaryKey({ id }); } /** @@ -143,9 +221,9 @@ export class CoreH5PFramework { * @returns Promise resolved when done. */ async deleteLibraryDependencies(libraryId: number, siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - await db.deleteRecords(LIBRARY_DEPENDENCIES_TABLE_NAME, { libraryid: libraryId }); + await this.libraryDependenciesTables[siteId].delete({ libraryid: libraryId }); } /** @@ -156,9 +234,9 @@ export class CoreH5PFramework { * @returns Promise resolved when done. */ async deleteLibraryUsage(id: number, siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - await db.deleteRecords(CONTENTS_LIBRARIES_TABLE_NAME, { h5pid: id }); + await this.contentsLibrariesTables[siteId].delete({ h5pid: id }); } /** @@ -168,9 +246,9 @@ export class CoreH5PFramework { * @returns Promise resolved with the list of content data. */ async getAllContentData(siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - return db.getAllRecords(CONTENT_TABLE_NAME); + return this.contentTables[siteId].getMany(); } /** @@ -181,9 +259,9 @@ export class CoreH5PFramework { * @returns Promise resolved with the content data. */ async getContentData(id: number, siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - return db.getRecord(CONTENT_TABLE_NAME, { id }); + return this.contentTables[siteId].getOneByPrimaryKey({ id }); } /** @@ -194,18 +272,16 @@ export class CoreH5PFramework { * @returns Promise resolved with the content data. */ async getContentDataByUrl(fileUrl: string, siteId?: string): Promise { - const site = await CoreSites.getSite(siteId); - - const db = site.getDb(); + siteId ??= CoreSites.getCurrentSiteId(); // Try to use the folder name, it should be more reliable than the URL. - const folderName = await CoreH5P.h5pCore.h5pFS.getContentFolderNameByUrl(fileUrl, site.getId()); + const folderName = await CoreH5P.h5pCore.h5pFS.getContentFolderNameByUrl(fileUrl, siteId); try { - return await db.getRecord(CONTENT_TABLE_NAME, { foldername: folderName }); + return await this.contentTables[siteId].getOne({ foldername: folderName }); } catch (error) { // Cannot get folder name, the h5p file was probably deleted. Just use the URL. - return db.getRecord(CONTENT_TABLE_NAME, { fileurl: fileUrl }); + return await this.contentTables[siteId].getOne({ fileurl: fileUrl }); } } @@ -216,17 +292,19 @@ export class CoreH5PFramework { * @returns Promise resolved with the latest library version data. */ async getLatestLibraryVersion(machineName: string, siteId?: string): Promise { - - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); try { - const records = await db.getRecords( - LIBRARIES_TABLE_NAME, + const records = await this.librariesTables[siteId].getMany( { machinename: machineName }, - 'majorversion DESC, minorversion DESC, patchversion DESC', - '*', - 0, - 1, + { + limit: 1, + sorting: [ + { majorversion: 'desc' }, + { minorversion: 'desc' }, + { patchversion: 'desc' }, + ], + }, ); if (records && records[0]) { @@ -254,13 +332,12 @@ export class CoreH5PFramework { minorVersion?: string | number, siteId?: string, ): Promise { + siteId ??= CoreSites.getCurrentSiteId(); - const db = await CoreSites.getSiteDb(siteId); - - const libraries = await db.getRecords(LIBRARIES_TABLE_NAME, { + const libraries = await this.librariesTables[siteId].getMany({ machinename: machineName, - majorversion: majorVersion, - minorversion: minorVersion, + majorversion: majorVersion !== undefined ? Number(majorVersion) : undefined, + minorversion: minorVersion !== undefined ? Number(minorVersion) : undefined, }); if (!libraries.length) { @@ -289,9 +366,9 @@ export class CoreH5PFramework { * @returns Promise resolved with the library data, rejected if not found. */ async getLibraryById(id: number, siteId?: string): Promise { - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); - const library = await db.getRecord(LIBRARIES_TABLE_NAME, { id }); + const library = await this.librariesTables[siteId].getOneByPrimaryKey({ id }); return this.parseLibDBData(library); } @@ -669,17 +746,14 @@ export class CoreH5PFramework { folderName: string, siteId?: string, ): Promise { - - const db = await CoreSites.getSiteDb(siteId); + const targetSiteId = siteId ?? CoreSites.getCurrentSiteId(); await Promise.all(Object.keys(dependencies).map(async (key) => { - const data: Partial = { + await this.librariesCachedAssetsTables[targetSiteId].insert({ hash: key, libraryid: dependencies[key].libraryId, foldername: folderName, - }; - - await db.insertRecord(LIBRARIES_CACHEDASSETS_TABLE_NAME, data); + }); })); } @@ -691,6 +765,8 @@ export class CoreH5PFramework { * @returns Promise resolved when done. */ async saveLibraryData(libraryData: CoreH5PLibraryBeingSaved, siteId?: string): Promise { + siteId ??= CoreSites.getCurrentSiteId(); + // Some special properties needs some checking and converting before they can be saved. const preloadedJS = this.libraryParameterValuesToCsv(libraryData, 'preloadedJs', 'path'); const preloadedCSS = this.libraryParameterValuesToCsv(libraryData, 'preloadedCss', 'path'); @@ -708,10 +784,7 @@ export class CoreH5PFramework { embedTypes = libraryData.embedTypes.join(', '); } - const site = await CoreSites.getSite(siteId); - - const db = site.getDb(); - const data: Partial = { + const data: Omit & Partial> = { title: libraryData.title, machinename: libraryData.machineName, majorversion: libraryData.majorVersion, @@ -733,16 +806,14 @@ export class CoreH5PFramework { data.id = libraryData.libraryId; } - await db.insertRecord(LIBRARIES_TABLE_NAME, data); + const libraryId = await this.librariesTables[siteId].insert(data); if (!data.id) { // New library. Get its ID. - const entry = await db.getRecord(LIBRARIES_TABLE_NAME, data); - - libraryData.libraryId = entry.id; + libraryData.libraryId = libraryId; } else { // Updated libary. Remove old dependencies. - await this.deleteLibraryDependencies(data.id, site.getId()); + await this.deleteLibraryDependencies(data.id, siteId); } } @@ -761,8 +832,7 @@ export class CoreH5PFramework { dependencyType: string, siteId?: string, ): Promise { - - const db = await CoreSites.getSiteDb(siteId); + const targetSiteId = siteId ?? CoreSites.getCurrentSiteId(); await Promise.all(dependencies.map(async (dependency) => { // Get the ID of the library. @@ -777,13 +847,15 @@ export class CoreH5PFramework { } // Create the relation. - const entry: Partial = { + if (typeof library.libraryId !== 'string') { + throw new CoreError('Attempted to create dependencies of library without id'); + } + + await this.libraryDependenciesTables[targetSiteId].insert({ libraryid: library.libraryId, requiredlibraryid: dependencyId, dependencytype: dependencyType, - }; - - await db.insertRecord(LIBRARY_DEPENDENCIES_TABLE_NAME, entry); + }); })); } @@ -800,8 +872,7 @@ export class CoreH5PFramework { librariesInUse: {[key: string]: CoreH5PContentDepsTreeDependency}, siteId?: string, ): Promise { - - const db = await CoreSites.getSiteDb(siteId); + const targetSiteId = siteId ?? CoreSites.getCurrentSiteId(); // Calculate the CSS to drop. const dropLibraryCssList: Record = {}; @@ -818,18 +889,17 @@ export class CoreH5PFramework { } } - // Now save the uusage. + // Now save the usage. await Promise.all(Object.keys(librariesInUse).map((key) => { const dependency = librariesInUse[key]; - const data: Partial = { + + return this.contentsLibrariesTables[targetSiteId].insert({ h5pid: id, libraryid: dependency.library.libraryId, dependencytype: dependency.type, dropcss: dropLibraryCssList[dependency.library.machineName] ? 1 : 0, - weight: dependency.weight, - }; - - return db.insertRecord(CONTENTS_LIBRARIES_TABLE_NAME, data); + weight: dependency.weight ?? 0, + }); })); } @@ -843,8 +913,7 @@ export class CoreH5PFramework { * @returns Promise resolved with content ID. */ async updateContent(content: CoreH5PContentBeingSaved, folderName: string, fileUrl: string, siteId?: string): Promise { - - const db = await CoreSites.getSiteDb(siteId); + siteId ??= CoreSites.getCurrentSiteId(); // If the libraryid declared in the package is empty, get the latest version. if (content.library && content.library.libraryId === undefined) { @@ -861,32 +930,31 @@ export class CoreH5PFramework { content.params = JSON.stringify(params); } - const data: Partial = { - id: undefined, - jsoncontent: content.params, + if (typeof content.library?.libraryId !== 'number') { + throw new CoreError('Attempted to create content of library without id'); + } + + const data: Omit & Partial> = { + jsoncontent: content.params ?? '{}', mainlibraryid: content.library?.libraryId, timemodified: Date.now(), filtered: null, foldername: folderName, fileurl: fileUrl, - timecreated: undefined, + timecreated: Date.now(), }; let contentId: number | undefined; if (content.id !== undefined) { data.id = content.id; contentId = content.id; - } else { - data.timecreated = data.timemodified; } - await db.insertRecord(CONTENT_TABLE_NAME, data); + const newContentId = await this.contentTables[siteId].insert(data); if (!contentId) { // New content. Get its ID. - const entry = await db.getRecord(CONTENT_TABLE_NAME, data); - - content.id = entry.id; + content.id = newContentId; contentId = content.id; } @@ -901,12 +969,9 @@ export class CoreH5PFramework { * @param siteId Site ID. If not defined, current site. */ async updateContentFields(id: number, fields: Partial, siteId?: string): Promise { + siteId ??= CoreSites.getCurrentSiteId(); - const db = await CoreSites.getSiteDb(siteId); - - const data = Object.assign({}, fields); - - await db.updateRecords(CONTENT_TABLE_NAME, data, { id }); + await this.contentTables[siteId].update(fields, { id }); } }