From 720b48644b0b5a23c57ffaf288592d6fd8c2b722 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 8 Aug 2024 15:32:50 +0200 Subject: [PATCH 1/4] MOBILE-4640 filepool: Avoid re-downloading files when it isn't needed --- src/core/services/filepool.ts | 57 ++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index 7e0b3f450..8267fa326 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -552,6 +552,35 @@ export class CoreFilepoolProvider { } } + /** + * Check if a file is outdated, updating its timemodified if the entry doesn't have it but it's not outdated. + * + * @param siteId Site ID. + * @param entry Entry to check. + * @param revision File revision number. + * @param timemodified The time this file was modified. + * @returns Whether the file is outdated. + */ + protected async checkFileOutdated( + siteId: string, + entry: CoreFilepoolFileEntry, + revision = 0, + timemodified = 0, + ): Promise { + if (this.isFileOutdated(entry, revision, timemodified)) { + return true; + } + + if (timemodified > 0 && !entry.timemodified) { + // Entry is not outdated but it doesn't have timemodified. Update it. + await CoreUtils.ignoreErrors(this.filesTables[siteId].update({ timemodified }, { fileId: entry.fileId })); + + entry.timemodified = timemodified; + } + + return false; + } + /** * Check the queue processing. * @@ -1071,13 +1100,10 @@ export class CoreFilepoolProvider { try { const fileObject = await this.hasFileInPool(siteId, fileId); + const isOutdated = await this.checkFileOutdated(siteId, fileObject, options.revision, options.timemodified); let url: string; - if (!fileObject || - this.isFileOutdated(fileObject, options.revision, options.timemodified) && - CoreNetwork.isOnline() && - !ignoreStale - ) { + if (isOutdated && CoreNetwork.isOnline() && !ignoreStale) { throw new CoreError('Needs to be downloaded'); } @@ -1558,8 +1584,9 @@ export class CoreFilepoolProvider { try { // File is not being downloaded. Check if it's downloaded and if it's outdated. const entry = await this.hasFileInPool(siteId, fileId); + const isOutdated = await this.checkFileOutdated(siteId, entry, revision, timemodified); - if (this.isFileOutdated(entry, revision, timemodified)) { + if (isOutdated) { return DownloadStatus.OUTDATED; } @@ -1627,12 +1654,9 @@ export class CoreFilepoolProvider { try { const entry = await this.hasFileInPool(siteId, fileId); + const isOutdated = await this.checkFileOutdated(siteId, entry, revision, timemodified); - if (entry === undefined) { - throw new CoreError('File not downloaded.'); - } - - if (this.isFileOutdated(entry, revision, timemodified) && CoreNetwork.isOnline()) { + if (isOutdated && CoreNetwork.isOnline()) { throw new CoreError('File is outdated'); } } catch (error) { @@ -2379,8 +2403,9 @@ export class CoreFilepoolProvider { * @returns Whether the file is outdated. */ protected isFileOutdated(entry: CoreFilepoolFileEntry, revision = 0, timemodified = 0): boolean { - // Don't allow undefined values, convert them to 0. - const entryTimemodified = entry.timemodified ?? 0; + // If the entry doesn't have a timemodified, use the download time instead. This is to prevent re-downloading + // files that haven't been updated in the server. + const entryTimemodified = entry.timemodified || Math.floor(entry.downloadTime / 1000); const entryRevision = entry.revision ?? 0; return !!entry.stale || revision > entryRevision || timemodified > entryTimemodified; @@ -2624,7 +2649,11 @@ export class CoreFilepoolProvider { // File not in pool. } - if (entry && !options.isexternalfile && !this.isFileOutdated(entry, options.revision, options.timemodified)) { + if ( + entry && + !options.isexternalfile && + !(await this.checkFileOutdated(siteId, entry, options.revision, options.timemodified)) + ) { // We have the file, it is not stale, we can update links and remove from queue. this.logger.debug('Queued file already in store, ignoring...'); this.addFileLinks(siteId, fileId, links).catch(() => { From 9a1cd83207762586acb6302bf5a31be956971a6c Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Fri, 9 Aug 2024 08:22:53 +0200 Subject: [PATCH 2/4] MOBILE-4640 filepool: Remove unused fillExtensionInFile --- src/core/services/filepool.ts | 42 ----------------------------------- 1 file changed, 42 deletions(-) diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index 8267fa326..1878700aa 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -1185,48 +1185,6 @@ export class CoreFilepoolProvider { })); } - /** - * Fill Missing Extension In the File Object if needed. - * This is to migrate from old versions. - * - * @param entry File object to be migrated. - * @param siteId SiteID to get migrated. - * @returns Promise resolved when done. - */ - protected async fillExtensionInFile(entry: CoreFilepoolFileEntry, siteId: string): Promise { - if (entry.extension !== undefined) { - // Already filled. - return; - } - - const extension = CoreMimetypeUtils.getFileExtension(entry.path); - if (!extension) { - // Files does not have extension. Invalidate file (stale = true). - // Minor problem: file will remain in the filesystem once downloaded again. - this.logger.debug('Staled file with no extension ' + entry.fileId); - - await this.filesTables[siteId].update({ stale: 1 }, { fileId: entry.fileId }); - - return; - } - - // File has extension. Save extension, and add extension to path. - const fileId = entry.fileId; - entry.fileId = CoreMimetypeUtils.removeExtension(fileId); - entry.extension = extension; - - await this.filesTables[siteId].update(entry, { fileId }); - if (entry.fileId == fileId) { - // File ID hasn't changed, we're done. - this.logger.debug('Removed extesion ' + extension + ' from file ' + entry.fileId); - - return; - } - - // Now update the links. - await this.linksTables[siteId].update({ fileId: entry.fileId }, { fileId }); - } - /** * Fix a component ID to always be a Number if possible. * From 2abe483c6daa853fc15d3e10e59f8c302951d514 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Fri, 9 Aug 2024 14:43:43 +0200 Subject: [PATCH 3/4] MOBILE-4640 db: Fix bug when updating primary key in memory database Before this fix, the record was updated but the key was still the old primary key, so the new record wasn't found when using getOneByPrimaryKey. --- .../classes/database/eager-database-table.ts | 4 ++-- .../database/inmemory-database-table.ts | 20 +++++++++++++++++++ .../classes/database/lazy-database-table.ts | 4 ++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/core/classes/database/eager-database-table.ts b/src/core/classes/database/eager-database-table.ts index 3bfbf6e8d..5820dd34f 100644 --- a/src/core/classes/database/eager-database-table.ts +++ b/src/core/classes/database/eager-database-table.ts @@ -172,7 +172,7 @@ export class CoreEagerDatabaseTable< continue; } - Object.assign(record, updates); + this.updateMemoryRecord(record, updates, this.records); } } @@ -187,7 +187,7 @@ export class CoreEagerDatabaseTable< continue; } - Object.assign(record, updates); + this.updateMemoryRecord(record, updates, this.records); } } diff --git a/src/core/classes/database/inmemory-database-table.ts b/src/core/classes/database/inmemory-database-table.ts index 7f01ad912..b3af85408 100644 --- a/src/core/classes/database/inmemory-database-table.ts +++ b/src/core/classes/database/inmemory-database-table.ts @@ -96,4 +96,24 @@ export abstract class CoreInMemoryDatabaseTable< return rowId; } + /** + * Update a record in memory. + * + * @param record Record to update. + * @param updates New values. + * @param records Records object. + */ + protected updateMemoryRecord(record: DBRecord, updates: Partial, records: Record): void { + const previousPrimaryKey = this.serializePrimaryKey(this.getPrimaryKeyFromRecord(record)); + + Object.assign(record, updates); + + const newPrimaryKey = this.serializePrimaryKey(this.getPrimaryKeyFromRecord(record)); + + if (newPrimaryKey !== previousPrimaryKey) { + delete records[previousPrimaryKey]; + records[newPrimaryKey] = record; + } + } + } diff --git a/src/core/classes/database/lazy-database-table.ts b/src/core/classes/database/lazy-database-table.ts index a745e29f4..d06aafc1c 100644 --- a/src/core/classes/database/lazy-database-table.ts +++ b/src/core/classes/database/lazy-database-table.ts @@ -156,7 +156,7 @@ export class CoreLazyDatabaseTable< continue; } - Object.assign(record, updates); + this.updateMemoryRecord(record, updates, this.records); } } @@ -171,7 +171,7 @@ export class CoreLazyDatabaseTable< continue; } - Object.assign(record, updates); + this.updateMemoryRecord(record, updates, this.records); } } From 7118768fe594f84d60bab23175f457ea0221e3f9 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Mon, 12 Aug 2024 09:47:37 +0200 Subject: [PATCH 4/4] MOBILE-4640 core: Fix revision not removed from pluginfile URLs This means that every time a file revision changed, the app downloaded it into a new file when it should replace the existing file with the new version. --- src/core/classes/delegate.ts | 4 +- src/core/services/filepool.ts | 172 +++++++++++++----- src/core/services/plugin-file-delegate.ts | 2 + .../tests/plugin-file-delegate.test.ts | 97 ++++++++++ src/core/singletons/tests/url.test.ts | 13 ++ src/core/singletons/url.ts | 23 +++ 6 files changed, 265 insertions(+), 46 deletions(-) create mode 100644 src/core/services/tests/plugin-file-delegate.test.ts diff --git a/src/core/classes/delegate.ts b/src/core/classes/delegate.ts index c98cd6f3a..c18aa061d 100644 --- a/src/core/classes/delegate.ts +++ b/src/core/classes/delegate.ts @@ -249,12 +249,12 @@ export class CoreDelegate { const key = handler[this.handlerNameProperty] || handler.name; if (this.handlers[key] !== undefined) { - this.logger.log(`Handler '${handler[this.handlerNameProperty]}' already registered`); + this.logger.log(`Handler '${key}' already registered`); return false; } - this.logger.log(`Registered handler '${handler[this.handlerNameProperty]}'`); + this.logger.log(`Registered handler '${key}'`); this.handlers[key] = handler; return true; diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index 1878700aa..7ec4147ca 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -113,6 +113,9 @@ export class CoreFilepoolProvider { protected packagesTables: LazyMap>>; protected queueTable = asyncInstance>(); + // To avoid fixing the same file ID twice at the same time. @deprecated since 4.5 + protected fixFileIdPromises: Record>> = {}; + constructor() { this.logger = CoreLogger.getInstance('CoreFilepoolProvider'); this.filesTables = lazyMap( @@ -770,7 +773,7 @@ export class CoreFilepoolProvider { const extension = CoreMimetypeUtils.guessExtensionFromUrl(fileUrl); const addExtension = filePath === undefined; - const path = filePath || (await this.getFilePath(siteId, fileId, extension)); + const path = filePath || (await this.getFilePath(siteId, fileId, extension, fileUrl)); if (poolFileObject && poolFileObject.fileId !== fileId) { this.logger.error('Invalid object to update passed'); @@ -1099,7 +1102,7 @@ export class CoreFilepoolProvider { }; try { - const fileObject = await this.hasFileInPool(siteId, fileId); + const fileObject = await this.hasFileInPool(siteId, fileId, fileUrl); const isOutdated = await this.checkFileOutdated(siteId, fileObject, options.revision, options.timemodified); let url: string; @@ -1355,6 +1358,43 @@ export class CoreFilepoolProvider { return this.addHashToFilename(url, filename); } + /** + * For a while, the getFileIdByUrl method had a bug that caused revision not to be removed from the URL. + * This function simulates that behaviour and returns the file ID without removing revision. + * This function is temporary and should be removed in the future, it's used to avoid files not being found + * after fixing getFileIdByUrl. + * + * @param fileUrl The absolute URL to the file. + * @returns The file ID. + * @deprecated since 4.5 + */ + protected getFiledIdByUrlBugged(fileUrl: string): string { + let url = fileUrl; + + // If site supports it, since 3.8 we use tokenpluginfile instead of pluginfile. + // For compatibility with files already downloaded, we need to use pluginfile to calculate the file ID. + url = url.replace(/\/tokenpluginfile\.php\/[^/]+\//, '/webservice/pluginfile.php/'); + + // Decode URL. + url = CoreTextUtils.decodeHTML(CoreTextUtils.decodeURIComponent(url)); + + if (url.indexOf('/webservice/pluginfile') !== -1) { + // Remove attributes that do not matter. + this.urlAttributes.forEach((regex) => { + url = url.replace(regex, ''); + }); + } + + // Remove the anchor. + url = CoreUrl.removeUrlParts(url, CoreUrlPartNames.Fragment); + + // Try to guess the filename the target file should have. + // We want to keep the original file name so people can easily identify the files after the download. + const filename = this.guessFilenameFromUrl(url); + + return this.addHashToFilename(url, filename); + } + /** * Get the links of a file. * @@ -1378,15 +1418,16 @@ export class CoreFilepoolProvider { * @param siteId The site ID. * @param fileId The file ID. * @param extension Previously calculated extension. Empty to not add any. Undefined to calculate it. + * @param fileUrl Tmp param to use the bugged file ID if the file isn't found. To be removed with getFiledIdByUrlBugged. * @returns The path to the file relative to storage root. */ - protected async getFilePath(siteId: string, fileId: string, extension?: string): Promise { + protected async getFilePath(siteId: string, fileId: string, extension?: string, fileUrl?: string): Promise { let path = this.getFilepoolFolderPath(siteId) + '/' + fileId; if (extension === undefined) { // We need the extension to be able to open files properly. try { - const entry = await this.hasFileInPool(siteId, fileId); + const entry = await this.hasFileInPool(siteId, fileId, fileUrl); if (entry.extension) { path += '.' + entry.extension; @@ -1412,7 +1453,7 @@ export class CoreFilepoolProvider { const file = await this.fixPluginfileURL(siteId, fileUrl); const fileId = this.getFileIdByUrl(CoreFileHelper.getFileUrl(file)); - return this.getFilePath(siteId, fileId); + return this.getFilePath(siteId, fileId, undefined, CoreFileHelper.getFileUrl(file)); } /** @@ -1531,7 +1572,7 @@ export class CoreFilepoolProvider { } catch (e) { // Check if the file is being downloaded right now. const extension = CoreMimetypeUtils.guessExtensionFromUrl(fileUrl); - filePath = filePath || (await this.getFilePath(siteId, fileId, extension)); + filePath = filePath || (await this.getFilePath(siteId, fileId, extension, fileUrl)); const downloadId = this.getFileDownloadId(fileUrl, filePath); @@ -1541,7 +1582,7 @@ export class CoreFilepoolProvider { try { // File is not being downloaded. Check if it's downloaded and if it's outdated. - const entry = await this.hasFileInPool(siteId, fileId); + const entry = await this.hasFileInPool(siteId, fileId, fileUrl); const isOutdated = await this.checkFileOutdated(siteId, entry, revision, timemodified); if (isOutdated) { @@ -1611,7 +1652,7 @@ export class CoreFilepoolProvider { const fileId = this.getFileIdByUrl(fileUrl); try { - const entry = await this.hasFileInPool(siteId, fileId); + const entry = await this.hasFileInPool(siteId, fileId, fileUrl); const isOutdated = await this.checkFileOutdated(siteId, entry, revision, timemodified); if (isOutdated && CoreNetwork.isOnline()) { @@ -1627,8 +1668,8 @@ export class CoreFilepoolProvider { try { // We found the file entry, now look for the file on disk. const path = mode === 'src' ? - await this.getInternalSrcById(siteId, fileId) : - await this.getInternalUrlById(siteId, fileId); + await this.getInternalSrcById(siteId, fileId, fileUrl) : + await this.getInternalUrlById(siteId, fileId, fileUrl); // Add the anchor to the local URL if any. const anchor = CoreUrl.getUrlAnchor(fileUrl); @@ -1652,14 +1693,15 @@ export class CoreFilepoolProvider { * * @param siteId The site ID. * @param fileId The file ID. + * @param fileUrl Tmp param to use the bugged file ID if the file isn't found. To be removed with getFiledIdByUrlBugged. * @returns Resolved with the internal URL. Rejected otherwise. */ - protected async getInternalSrcById(siteId: string, fileId: string): Promise { + protected async getInternalSrcById(siteId: string, fileId: string, fileUrl?: string): Promise { if (!CoreFile.isAvailable()) { throw new CoreError('File system cannot be used.'); } - const path = await this.getFilePath(siteId, fileId); + const path = await this.getFilePath(siteId, fileId, undefined, fileUrl); const fileEntry = await CoreFile.getFile(path); return CoreFile.convertFileSrc(CoreFile.getFileEntryURL(fileEntry)); @@ -1670,14 +1712,15 @@ export class CoreFilepoolProvider { * * @param siteId The site ID. * @param fileId The file ID. + * @param fileUrl Tmp param to use the bugged file ID if the file isn't found. To be removed with getFiledIdByUrlBugged. * @returns Resolved with the URL. Rejected otherwise. */ - protected async getInternalUrlById(siteId: string, fileId: string): Promise { + protected async getInternalUrlById(siteId: string, fileId: string, fileUrl?: string): Promise { if (!CoreFile.isAvailable()) { throw new CoreError('File system cannot be used.'); } - const path = await this.getFilePath(siteId, fileId); + const path = await this.getFilePath(siteId, fileId, undefined, fileUrl); const fileEntry = await CoreFile.getFile(path); // This URL is usually used to launch files or put them in HTML. @@ -1715,7 +1758,7 @@ export class CoreFilepoolProvider { const file = await this.fixPluginfileURL(siteId, fileUrl); const fileId = this.getFileIdByUrl(CoreFileHelper.getFileUrl(file)); - return this.getInternalSrcById(siteId, fileId); + return this.getInternalSrcById(siteId, fileId, CoreFileHelper.getFileUrl(file)); } /** @@ -1733,7 +1776,7 @@ export class CoreFilepoolProvider { const file = await this.fixPluginfileURL(siteId, fileUrl); const fileId = this.getFileIdByUrl(CoreFileHelper.getFileUrl(file)); - return this.getInternalUrlById(siteId, fileId); + return this.getInternalUrlById(siteId, fileId, CoreFileHelper.getFileUrl(file)); } /** @@ -1888,29 +1931,6 @@ export class CoreFilepoolProvider { } } - /** - * Return the array of arguments of the pluginfile url. - * - * @param url URL to get the args. - * @returns The args found, undefined if not a pluginfile. - */ - protected getPluginFileArgs(url: string): string[] | undefined { - if (!CoreUrl.isPluginFileUrl(url)) { - // Not pluginfile, return. - return; - } - - const relativePath = url.substring(url.indexOf('/pluginfile.php') + 16); - const args = relativePath.split('/'); - - if (args.length < 3) { - // To be a plugin file it should have at least contextId, Component and Filearea. - return; - } - - return args; - } - /** * Get the deferred object for a file in the queue. * @@ -2007,7 +2027,7 @@ export class CoreFilepoolProvider { * @returns Revision number. */ protected getRevisionFromUrl(url: string): number { - const args = this.getPluginFileArgs(url); + const args = CoreUrl.getPluginFileArgs(url); if (!args) { // Not a pluginfile, no revision will be found. return 0; @@ -2193,10 +2213,74 @@ export class CoreFilepoolProvider { * * @param siteId The site ID. * @param fileId The file Id. + * @param fileUrl Tmp param to use the bugged file ID if the file isn't found. To be removed with getFiledIdByUrlBugged. * @returns Resolved with file object from DB on success, rejected otherwise. */ - protected async hasFileInPool(siteId: string, fileId: string): Promise { - return this.filesTables[siteId].getOneByPrimaryKey({ fileId }); + protected async hasFileInPool(siteId: string, fileId: string, fileUrl?: string): Promise { + try { + return await this.filesTables[siteId].getOneByPrimaryKey({ fileId }); + } catch (error) { + if (!fileUrl) { + throw error; + } + + // Entry not found. Check if it's stored with the "bugged" file ID. + const buggedFileId = this.getFiledIdByUrlBugged(fileUrl); // eslint-disable-line deprecation/deprecation + if (buggedFileId === fileId) { + throw error; + } + + const fileEntry = await this.filesTables[siteId].getOneByPrimaryKey({ fileId: buggedFileId }); + + try { + await this.fixBuggedFileId(siteId, fileEntry, fileId); // eslint-disable-line deprecation/deprecation + } catch (error) { + // Ignore errors when fixing the ID, it shouldn't happen. + } + + return fileEntry; + } + } + + /** + * Fix a file entry's wrong file ID. + * + * @param siteId Site ID. + * @param fileEntry File entry to fix. + * @param newFileId New file ID. + * @returns Promise resolved when done. + * @deprecated since 4.5 + */ + protected async fixBuggedFileId(siteId: string, fileEntry: CoreFilepoolFileEntry, newFileId: string): Promise { + if (this.fixFileIdPromises[siteId] && this.fixFileIdPromises[siteId][newFileId] !== undefined) { + return this.fixFileIdPromises[siteId][newFileId]; + } + + const fixFileId = async (): Promise => { + const buggedFileId = fileEntry.fileId; + + const [currentFilePath, newFilePath] = await Promise.all([ + this.getFilePath(siteId, buggedFileId, fileEntry.extension), + this.getFilePath(siteId, newFileId, fileEntry.extension), + ]); + + // Move the file first, it's the step that's easier to fail. + await CoreFile.moveFile(currentFilePath, newFilePath); + + await Promise.all([ + this.filesTables[siteId].update({ fileId: newFileId }, { fileId: buggedFileId }), + CoreUtils.ignoreErrors(this.linksTables[siteId].update({ fileId: newFileId }, { fileId: buggedFileId })), + ]); + + fileEntry.fileId = newFileId; + + delete this.fixFileIdPromises[siteId][newFileId]; + }; + + this.fixFileIdPromises[siteId] = this.fixFileIdPromises[siteId] ?? {}; + this.fixFileIdPromises[siteId][newFileId] = fixFileId(); + + return this.fixFileIdPromises[siteId][newFileId]; } /** @@ -2602,7 +2686,7 @@ export class CoreFilepoolProvider { // Check if the file is already in pool. try { - entry = await this.hasFileInPool(siteId, fileId); + entry = await this.hasFileInPool(siteId, fileId, fileUrl); } catch (error) { // File not in pool. } @@ -2800,7 +2884,7 @@ export class CoreFilepoolProvider { * The revision is used to know if a file has changed. We remove it from the URL to prevent storing a file per revision. */ protected removeRevisionFromUrl(url: string): string { - const args = this.getPluginFileArgs(url); + const args = CoreUrl.getPluginFileArgs(url); if (!args) { // Not a pluginfile, no revision will be found. return url; diff --git a/src/core/services/plugin-file-delegate.ts b/src/core/services/plugin-file-delegate.ts index 456476f74..f7aa0012a 100644 --- a/src/core/services/plugin-file-delegate.ts +++ b/src/core/services/plugin-file-delegate.ts @@ -29,6 +29,8 @@ import { CoreFileHelper } from './file-helper'; @Injectable({ providedIn: 'root' }) export class CorePluginFileDelegateService extends CoreDelegate { + protected handlerNameProperty = 'component'; + constructor() { super('CorePluginFileDelegate'); } diff --git a/src/core/services/tests/plugin-file-delegate.test.ts b/src/core/services/tests/plugin-file-delegate.test.ts new file mode 100644 index 000000000..9fafa1324 --- /dev/null +++ b/src/core/services/tests/plugin-file-delegate.test.ts @@ -0,0 +1,97 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { mock, mockSingleton } from '@/testing/utils'; +import { CoreSite } from '@classes/sites/site'; +import { CorePluginFileDelegateService, CorePluginFileHandler } from '@services/plugin-file-delegate'; +import { CoreSites } from '@services/sites'; +import { CoreUrl } from '@singletons/url'; + +describe('CorePluginFileDelegate', () => { + + let pluginFileDelegate: CorePluginFileDelegateService; + + beforeEach(async () => { + const site = mock(new CoreSite('42', 'https://mysite.com', 'token'), { + isFeatureDisabled: () => false, + }); + + mockSingleton(CoreSites, { getCurrentSite: () => site, getCurrentSiteId: () => '42', isLoggedIn: () => true }); + + pluginFileDelegate = new CorePluginFileDelegateService(); + pluginFileDelegate.registerHandler(new ModFooRevisionHandler()); + + await pluginFileDelegate.updateHandlers(); + }); + + it('removes revision from a URL', () => { + const urlsToTest = [ + // Revision removed by mod_foo handler. + { + value: 'http://mysite.com/webservice/pluginfile.php/6/mod_foo/content/14/foo.txt', + expected: 'http://mysite.com/webservice/pluginfile.php/6/mod_foo/content/0/foo.txt', + }, + // Revision not removed because the component is not mod_foo. + { + value: 'http://mysite.com/webservice/pluginfile.php/6/mod_page/content/14/foo.txt', + expected: 'http://mysite.com/webservice/pluginfile.php/6/mod_page/content/14/foo.txt', + }, + // Revision not removed because it's not a pluginfile URL. + { + value: 'http://mysite.com/6/mod_foo/content/14/foo.txt', + expected: 'http://mysite.com/6/mod_foo/content/14/foo.txt', + }, + ]; + + urlsToTest.forEach(data => { + expect( + pluginFileDelegate.removeRevisionFromUrl(data.value, CoreUrl.getPluginFileArgs(data.value) ?? []), + ).toEqual(data.expected); + }); + }); + +}); + +class ModFooRevisionHandler implements CorePluginFileHandler { + + name = 'ModFooHandler'; + component = 'mod_foo'; + + /** + * @inheritdoc + */ + getComponentRevisionRegExp(args: string[]): RegExp | undefined { + // Check filearea. + if (args[2] == 'content') { + // Component + Filearea + Revision + return new RegExp('/mod_foo/content/([0-9]+)/'); + } + } + + /** + * @inheritdoc + */ + getComponentRevisionReplace(): string { + // Component + Filearea + Revision + return '/mod_foo/content/0/'; + } + + /** + * @inheritdoc + */ + async isEnabled(): Promise { + return true; + } + +} diff --git a/src/core/singletons/tests/url.test.ts b/src/core/singletons/tests/url.test.ts index 08410360b..0e4980e05 100644 --- a/src/core/singletons/tests/url.test.ts +++ b/src/core/singletons/tests/url.test.ts @@ -260,4 +260,17 @@ describe('CoreUrl singleton', () => { .toEqual(`${siteUrl}/media/player/vimeo/wsplayer.php?video=123456&token=${token}&h=foo`); }); + it('extracts args from pluginfile URLs', () => { + expect(CoreUrl.getPluginFileArgs('http://mysite.com/pluginfile.php/6/mod_foo/content/14/foo.txt')) + .toEqual(['6', 'mod_foo', 'content', '14', 'foo.txt']); + expect(CoreUrl.getPluginFileArgs('http://mysite.com/webservice/pluginfile.php/6/mod_foo/content/14/foo.txt')) + .toEqual(['6', 'mod_foo', 'content', '14', 'foo.txt']); + + // It doesn't work with tokenpluginfile or other URLs, and also when pluginfile doesn't have enough params. + expect(CoreUrl.getPluginFileArgs('http://mysite.com')).toEqual(undefined); + expect(CoreUrl.getPluginFileArgs('http://mysite.com/pluginfile.php/6/')).toEqual(undefined); + expect(CoreUrl.getPluginFileArgs('http://mysite.com/tokenpluginfile.php/abcdef123456/6/mod_foo/content/14/foo.txt')) + .toEqual(undefined); + }); + }); diff --git a/src/core/singletons/url.ts b/src/core/singletons/url.ts index d033268dc..e9080b1e3 100644 --- a/src/core/singletons/url.ts +++ b/src/core/singletons/url.ts @@ -642,6 +642,29 @@ export class CoreUrl { return path.split('/').pop() ?? ''; } + /** + * Return the array of arguments of the pluginfile url. + * + * @param url URL to get the args. + * @returns The args found, undefined if not a pluginfile. + */ + static getPluginFileArgs(url: string): string[] | undefined { + if (!CoreUrl.isPluginFileUrl(url)) { + // Not pluginfile, return. + return; + } + + const relativePath = url.substring(url.indexOf('/pluginfile.php') + 16); + const args = relativePath.split('/'); + + if (args.length < 3) { + // To be a plugin file it should have at least contextId, Component and Filearea. + return; + } + + return args; + } + /** * Get the protocol from a URL. * E.g. http://www.google.com returns 'http'.