From 9ea62f02471f094b4e9b320320a0e04df6bef24e Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:57:29 -0500 Subject: [PATCH 1/2] Remove Sequentialize --- .../services/lowdb-storage.service.ts | 2 - .../src/platform/misc/sequentialize.spec.ts | 139 ------------------ .../common/src/platform/misc/sequentialize.ts | 62 -------- .../common/src/platform/misc/throttle.spec.ts | 13 -- .../src/platform/sync/default-sync.service.ts | 2 - .../src/vault/services/cipher.service.ts | 2 - 6 files changed, 220 deletions(-) delete mode 100644 libs/common/src/platform/misc/sequentialize.spec.ts delete mode 100644 libs/common/src/platform/misc/sequentialize.ts diff --git a/apps/cli/src/platform/services/lowdb-storage.service.ts b/apps/cli/src/platform/services/lowdb-storage.service.ts index 3cbaeeafc027..a322c0bd9a69 100644 --- a/apps/cli/src/platform/services/lowdb-storage.service.ts +++ b/apps/cli/src/platform/services/lowdb-storage.service.ts @@ -12,7 +12,6 @@ import { AbstractStorageService, StorageUpdate, } from "@bitwarden/common/platform/abstractions/storage.service"; -import { sequentialize } from "@bitwarden/common/platform/misc/sequentialize"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { NodeUtils } from "@bitwarden/node/node-utils"; @@ -42,7 +41,6 @@ export class LowdbStorageService implements AbstractStorageService { this.updates$ = this.updatesSubject.asObservable(); } - @sequentialize(() => "lowdbStorageInit") async init() { if (this.ready) { return; diff --git a/libs/common/src/platform/misc/sequentialize.spec.ts b/libs/common/src/platform/misc/sequentialize.spec.ts deleted file mode 100644 index 0b79cde387e9..000000000000 --- a/libs/common/src/platform/misc/sequentialize.spec.ts +++ /dev/null @@ -1,139 +0,0 @@ -import { clearCaches, sequentialize } from "./sequentialize"; - -describe("sequentialize decorator", () => { - it("should call the function once", async () => { - const foo = new Foo(); - const promises = []; - for (let i = 0; i < 10; i++) { - promises.push(foo.bar(1)); - } - await Promise.all(promises); - - expect(foo.calls).toBe(1); - }); - - it("should call the function once for each instance of the object", async () => { - const foo = new Foo(); - const foo2 = new Foo(); - const promises = []; - for (let i = 0; i < 10; i++) { - promises.push(foo.bar(1)); - promises.push(foo2.bar(1)); - } - await Promise.all(promises); - - expect(foo.calls).toBe(1); - expect(foo2.calls).toBe(1); - }); - - it("should call the function once with key function", async () => { - const foo = new Foo(); - const promises = []; - for (let i = 0; i < 10; i++) { - promises.push(foo.baz(1)); - } - await Promise.all(promises); - - expect(foo.calls).toBe(1); - }); - - it("should call the function again when already resolved", async () => { - const foo = new Foo(); - await foo.bar(1); - expect(foo.calls).toBe(1); - await foo.bar(1); - expect(foo.calls).toBe(2); - }); - - it("should call the function again when already resolved with a key function", async () => { - const foo = new Foo(); - await foo.baz(1); - expect(foo.calls).toBe(1); - await foo.baz(1); - expect(foo.calls).toBe(2); - }); - - it("should call the function for each argument", async () => { - const foo = new Foo(); - await Promise.all([foo.bar(1), foo.bar(1), foo.bar(2), foo.bar(2), foo.bar(3), foo.bar(3)]); - expect(foo.calls).toBe(3); - }); - - it("should call the function for each argument with key function", async () => { - const foo = new Foo(); - await Promise.all([foo.baz(1), foo.baz(1), foo.baz(2), foo.baz(2), foo.baz(3), foo.baz(3)]); - expect(foo.calls).toBe(3); - }); - - it("should return correct result for each call", async () => { - const foo = new Foo(); - const allRes: number[] = []; - - await Promise.all([ - foo.bar(1).then((res) => allRes.push(res)), - foo.bar(1).then((res) => allRes.push(res)), - foo.bar(2).then((res) => allRes.push(res)), - foo.bar(2).then((res) => allRes.push(res)), - foo.bar(3).then((res) => allRes.push(res)), - foo.bar(3).then((res) => allRes.push(res)), - ]); - expect(foo.calls).toBe(3); - expect(allRes.length).toBe(6); - allRes.sort(); - expect(allRes).toEqual([2, 2, 4, 4, 6, 6]); - }); - - it("should return correct result for each call with key function", async () => { - const foo = new Foo(); - const allRes: number[] = []; - - await Promise.all([ - foo.baz(1).then((res) => allRes.push(res)), - foo.baz(1).then((res) => allRes.push(res)), - foo.baz(2).then((res) => allRes.push(res)), - foo.baz(2).then((res) => allRes.push(res)), - foo.baz(3).then((res) => allRes.push(res)), - foo.baz(3).then((res) => allRes.push(res)), - ]); - expect(foo.calls).toBe(3); - expect(allRes.length).toBe(6); - allRes.sort(); - expect(allRes).toEqual([3, 3, 6, 6, 9, 9]); - }); - - describe("clearCaches", () => { - it("should clear all caches", async () => { - const foo = new Foo(); - const promise = Promise.all([foo.bar(1), foo.bar(1)]); - clearCaches(); - await foo.bar(1); - await promise; - // one call for the first two, one for the third after the cache was cleared - expect(foo.calls).toBe(2); - }); - }); -}); - -class Foo { - calls = 0; - - @sequentialize((args) => "bar" + args[0]) - bar(a: number): Promise { - this.calls++; - return new Promise((res) => { - setTimeout(() => { - res(a * 2); - }, Math.random() * 100); - }); - } - - @sequentialize((args) => "baz" + args[0]) - baz(a: number): Promise { - this.calls++; - return new Promise((res) => { - setTimeout(() => { - res(a * 3); - }, Math.random() * 100); - }); - } -} diff --git a/libs/common/src/platform/misc/sequentialize.ts b/libs/common/src/platform/misc/sequentialize.ts deleted file mode 100644 index 852c32db56a5..000000000000 --- a/libs/common/src/platform/misc/sequentialize.ts +++ /dev/null @@ -1,62 +0,0 @@ -const caches = new Map>>(); - -const getCache = (obj: any) => { - let cache = caches.get(obj); - if (cache != null) { - return cache; - } - cache = new Map>(); - caches.set(obj, cache); - return cache; -}; - -export function clearCaches() { - caches.clear(); -} - -/** - * Use as a Decorator on async functions, it will prevent multiple 'active' calls as the same time - * - * If a promise was returned from a previous call to this function, that hasn't yet resolved it will - * be returned, instead of calling the original function again - * - * Results are not cached, once the promise has returned, the next call will result in a fresh call - * - * Read more at https://github.com/bitwarden/jslib/pull/7 - */ -export function sequentialize(cacheKey: (args: any[]) => string) { - return (target: any, propertyKey: string | symbol, descriptor: PropertyDescriptor) => { - const originalMethod: () => Promise = descriptor.value; - - return { - value: function (...args: any[]) { - const cache = getCache(this); - const argsCacheKey = cacheKey(args); - let response = cache.get(argsCacheKey); - if (response != null) { - return response; - } - - const onFinally = () => { - cache.delete(argsCacheKey); - if (cache.size === 0) { - caches.delete(this); - } - }; - response = originalMethod - .apply(this, args) - .then((val: any) => { - onFinally(); - return val; - }) - .catch((err: any) => { - onFinally(); - throw err; - }); - - cache.set(argsCacheKey, response); - return response; - }, - }; - }; -} diff --git a/libs/common/src/platform/misc/throttle.spec.ts b/libs/common/src/platform/misc/throttle.spec.ts index 0947d4af6645..1c1ff6324a60 100644 --- a/libs/common/src/platform/misc/throttle.spec.ts +++ b/libs/common/src/platform/misc/throttle.spec.ts @@ -1,4 +1,3 @@ -import { sequentialize } from "./sequentialize"; import { throttle } from "./throttle"; describe("throttle decorator", () => { @@ -51,17 +50,6 @@ describe("throttle decorator", () => { expect(foo.calls).toBe(10); expect(foo2.calls).toBe(10); }); - - it("should work together with sequentialize", async () => { - const foo = new Foo(); - const promises = []; - for (let i = 0; i < 10; i++) { - promises.push(foo.qux(Math.floor(i / 2) * 2)); - } - await Promise.all(promises); - - expect(foo.calls).toBe(5); - }); }); class Foo { @@ -94,7 +82,6 @@ class Foo { }); } - @sequentialize((args) => "qux" + args[0]) @throttle(1, () => "qux") qux(a: number) { this.calls++; diff --git a/libs/common/src/platform/sync/default-sync.service.ts b/libs/common/src/platform/sync/default-sync.service.ts index eaf804d28665..5db93ee2345d 100644 --- a/libs/common/src/platform/sync/default-sync.service.ts +++ b/libs/common/src/platform/sync/default-sync.service.ts @@ -45,7 +45,6 @@ import { FolderResponse } from "../../vault/models/response/folder.response"; import { LogService } from "../abstractions/log.service"; import { StateService } from "../abstractions/state.service"; import { MessageSender } from "../messaging"; -import { sequentialize } from "../misc/sequentialize"; import { StateProvider } from "../state"; import { CoreSyncService } from "./core-sync.service"; @@ -97,7 +96,6 @@ export class DefaultSyncService extends CoreSyncService { ); } - @sequentialize(() => "fullSync") override async fullSync(forceSync: boolean, allowThrowOnError = false): Promise { const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); this.syncStarted(); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 474976932e85..b19dec3c6080 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -28,7 +28,6 @@ import { ConfigService } from "../../platform/abstractions/config/config.service import { EncryptService } from "../../platform/abstractions/encrypt.service"; import { I18nService } from "../../platform/abstractions/i18n.service"; import { StateService } from "../../platform/abstractions/state.service"; -import { sequentialize } from "../../platform/misc/sequentialize"; import { Utils } from "../../platform/misc/utils"; import Domain from "../../platform/models/domain/domain-base"; import { EncArrayBuffer } from "../../platform/models/domain/enc-array-buffer"; @@ -374,7 +373,6 @@ export class CipherService implements CipherServiceAbstraction { * cached, the cached ciphers are returned. * @deprecated Use `cipherViews$` observable instead */ - @sequentialize(() => "getAllDecrypted") async getAllDecrypted(): Promise { let decCiphers = await this.getDecryptedCiphers(); if (decCiphers != null && decCiphers.length !== 0) { From e249152b420cc14c4528f4513e9ba077afcf8514 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:55:02 -0500 Subject: [PATCH 2/2] Delete `clearCaches` --- apps/browser/src/background/main.background.ts | 3 --- apps/desktop/src/app/app.component.ts | 3 --- 2 files changed, 6 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 764304f4ff9b..7d58a0198c64 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -102,7 +102,6 @@ import { Message, MessageListener, MessageSender } from "@bitwarden/common/platf // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; import { Lazy } from "@bitwarden/common/platform/misc/lazy"; -import { clearCaches } from "@bitwarden/common/platform/misc/sequentialize"; import { Account } from "@bitwarden/common/platform/models/domain/account"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; @@ -1404,8 +1403,6 @@ export default class MainBackground { await this.popupViewCacheBackgroundService.clearState(); await this.accountService.switchAccount(userId); await switchPromise; - // Clear sequentialized caches - clearCaches(); if (userId == null) { await this.refreshBadge(); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 7840c1dd400c..e3a915f67448 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -53,7 +53,6 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SystemService } from "@bitwarden/common/platform/abstractions/system.service"; -import { clearCaches } from "@bitwarden/common/platform/misc/sequentialize"; import { StateEventRunnerService } from "@bitwarden/common/platform/state"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -439,8 +438,6 @@ export class AppComponent implements OnInit, OnDestroy { this.router.navigate(["/remove-password"]); break; case "switchAccount": { - // Clear sequentialized caches - clearCaches(); if (message.userId != null) { await this.accountService.switchAccount(message.userId); }