From 89597dad729d30f29b2365ce8017a0edf3216417 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Mon, 27 May 2024 16:44:50 +0200 Subject: [PATCH] MOBILE-4470 storagemanager: Fix removing courses Fixes a regression caused by https://github.com/moodlehq/moodleapp/commit/91c56256ed40caa6479acd9c2233410d4a25e58f --- .../pages/courses-storage/courses-storage.ts | 17 ++++++--- src/core/classes/queue-runner.ts | 13 ++++++- src/core/classes/tests/queue-runner.test.ts | 38 +++++++++++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 src/core/classes/tests/queue-runner.test.ts diff --git a/src/addons/storagemanager/pages/courses-storage/courses-storage.ts b/src/addons/storagemanager/pages/courses-storage/courses-storage.ts index 9e1a3f9b5..ea4340c56 100644 --- a/src/addons/storagemanager/pages/courses-storage/courses-storage.ts +++ b/src/addons/storagemanager/pages/courses-storage/courses-storage.ts @@ -14,6 +14,7 @@ import { DownloadStatus } from '@/core/constants'; import { Component, OnDestroy, OnInit } from '@angular/core'; +import { CoreQueueRunner } from '@classes/queue-runner'; import { CoreCourse, CoreCourseProvider } from '@features/course/services/course'; import { CoreCourseHelper } from '@features/course/services/course-helper'; import { CoreCourseModulePrefetchDelegate } from '@features/course/services/module-prefetch-delegate'; @@ -50,6 +51,8 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy courseStatusObserver?: CoreEventObserver; siteId: string; + private downloadedCoursesQueue = new CoreQueueRunner(); + constructor() { this.siteId = CoreSites.getCurrentSiteId(); } @@ -87,7 +90,7 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy } } - await this.setDownloadedCourses(downloadedCourses); + await this.downloadedCoursesQueue.run(() => this.setDownloadedCourses(downloadedCourses)); this.loaded = true; } @@ -124,7 +127,9 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy try { await Promise.all(deletedCourseIds.map((courseId) => CoreCourseHelper.deleteCourseFiles(courseId))); - await this.setDownloadedCourses(this.downloadedCourses.filter((course) => !deletedCourseIds.includes(course.id))); + await this.downloadedCoursesQueue.run(async () => { + await this.setDownloadedCourses(this.downloadedCourses.filter((course) => !deletedCourseIds.includes(course.id))); + }); } catch (error) { CoreDomUtils.showErrorModalDefault(error, Translate.instant('core.errordeletefile')); } finally { @@ -160,7 +165,9 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy try { await CoreCourseHelper.deleteCourseFiles(course.id); - await this.setDownloadedCourses(CoreArray.withoutItem(this.downloadedCourses, course)); + await this.downloadedCoursesQueue.run(async () => { + await this.setDownloadedCourses(CoreArray.withoutItem(this.downloadedCourses, course)); + }); } catch (error) { CoreDomUtils.showErrorModalDefault(error, Translate.instant('core.errordeletefile')); } finally { @@ -175,7 +182,7 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy */ private async onCourseUpdated(courseId: number, status: DownloadStatus): Promise { if (courseId == CoreCourseProvider.ALL_COURSES_CLEARED) { - await this.setDownloadedCourses([]); + await this.downloadedCoursesQueue.run(() => this.setDownloadedCourses([])); return; } @@ -189,7 +196,7 @@ export class AddonStorageManagerCoursesStoragePage implements OnInit, OnDestroy course.isDownloading = status === DownloadStatus.DOWNLOADING; course.totalSize = await this.calculateDownloadedCourseSize(course.id); - await this.setDownloadedCourses(this.downloadedCourses); + await this.downloadedCoursesQueue.run(() => this.setDownloadedCourses(this.downloadedCourses)); } /** diff --git a/src/core/classes/queue-runner.ts b/src/core/classes/queue-runner.ts index cb3536054..6795af7ad 100644 --- a/src/core/classes/queue-runner.ts +++ b/src/core/classes/queue-runner.ts @@ -121,8 +121,17 @@ export class CoreQueueRunner { * @param options Options. * @returns Promise resolved when the function has been executed. */ - run(id: string, fn: CoreQueueRunnerFunction, options?: CoreQueueRunnerAddOptions): Promise { - options = options || {}; + run(fn: CoreQueueRunnerFunction, options?: CoreQueueRunnerAddOptions): Promise; + run(id: string, fn: CoreQueueRunnerFunction, options?: CoreQueueRunnerAddOptions): Promise; + run( + idOrFn: string | CoreQueueRunnerFunction, + fnOrOptions?: CoreQueueRunnerFunction | CoreQueueRunnerAddOptions, + options: CoreQueueRunnerAddOptions = {}, + ): Promise { + let id = typeof idOrFn === 'string' ? idOrFn : this.getUniqueId('anonymous'); + const fn = typeof idOrFn === 'function' ? idOrFn : fnOrOptions as CoreQueueRunnerFunction; + + options = typeof fnOrOptions === 'object' ? fnOrOptions : options; if (id in this.queue) { if (!options.allowRepeated) { diff --git a/src/core/classes/tests/queue-runner.test.ts b/src/core/classes/tests/queue-runner.test.ts new file mode 100644 index 000000000..748c7c848 --- /dev/null +++ b/src/core/classes/tests/queue-runner.test.ts @@ -0,0 +1,38 @@ +// (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 { CoreQueueRunner } from '@classes/queue-runner'; +import { CoreUtils } from '@services/utils/utils'; + +describe('CoreQueueRunner', () => { + + it('Locks threads launched synchronously', async () => { + // Arrange + const concurrency = 100; + const range = Array.from({ length: concurrency }, (_, item) => item); + const lock = new CoreQueueRunner(); + const items: string[] = []; + + // Act + await Promise.all(range.map((i) => lock.run(async () => { + await CoreUtils.wait(Math.floor(Math.random() * 10)); + + items.push(`Item #${i}`); + }))); + + // Assert + expect(items).toEqual(range.map(i => `Item #${i}`)); + }); + +});