From ef94ff861f4c268b7dee8ddad851f1c3a586b167 Mon Sep 17 00:00:00 2001 From: Sergey Petushkov Date: Wed, 23 Jul 2025 11:49:00 +0200 Subject: [PATCH 1/4] chore(user-data): remove redundant withStats methods --- .../src/user-storage.ts | 4 - .../compass-user-data/src/user-data.spec.ts | 54 ------- packages/compass-user-data/src/user-data.ts | 139 +++++------------- .../src/compass-pipeline-storage.ts | 25 +--- .../src/pipeline-storage-schema.ts | 4 +- 5 files changed, 39 insertions(+), 187 deletions(-) diff --git a/packages/compass-preferences-model/src/user-storage.ts b/packages/compass-preferences-model/src/user-storage.ts index fbaed7f7b27..941c3f25065 100644 --- a/packages/compass-preferences-model/src/user-storage.ts +++ b/packages/compass-preferences-model/src/user-storage.ts @@ -79,8 +79,4 @@ export class UserStorageImpl implements UserStorage { await this.userData.write(user.id, user); return this.getUser(user.id); } - - private getFileName(id: string) { - return `${id}.json`; - } } diff --git a/packages/compass-user-data/src/user-data.spec.ts b/packages/compass-user-data/src/user-data.spec.ts index 155a020092a..e610a072fb9 100644 --- a/packages/compass-user-data/src/user-data.spec.ts +++ b/packages/compass-user-data/src/user-data.spec.ts @@ -125,39 +125,6 @@ describe('user-data', function () { expect(result.data).to.have.lengthOf(0); expect(result.errors).to.have.lengthOf(2); }); - - it('returns file stats', async function () { - await Promise.all( - [ - ['data1.json', JSON.stringify({ name: 'VSCode' })], - ['data2.json', JSON.stringify({ name: 'Mongosh' })], - ].map(([filepath, data]) => writeFileToStorage(filepath, data)) - ); - - const { data } = await getUserData().readAllWithStats({ - ignoreErrors: true, - }); - - { - const vscodeData = data.find((x) => x[0].name === 'VSCode'); - expect(vscodeData?.[0]).to.deep.equal({ - name: 'VSCode', - hasDarkMode: true, - hasWebSupport: false, - }); - expect(vscodeData?.[1]).to.be.instanceOf(Stats); - } - - { - const mongoshData = data.find((x) => x[0].name === 'Mongosh'); - expect(mongoshData?.[0]).to.deep.equal({ - name: 'Mongosh', - hasDarkMode: true, - hasWebSupport: false, - }); - expect(mongoshData?.[1]).to.be.instanceOf(Stats); - } - }); }); context('UserData.readOne', function () { @@ -302,27 +269,6 @@ describe('user-data', function () { company: 'MongoDB', }); }); - - it('return file stats', async function () { - await writeFileToStorage( - 'data.json', - JSON.stringify({ - name: 'Mongosh', - company: 'MongoDB', - }) - ); - - const [data, stats] = await getUserData().readOneWithStats('data', { - ignoreErrors: false, - }); - - expect(data).to.deep.equal({ - name: 'Mongosh', - hasDarkMode: true, - hasWebSupport: false, - }); - expect(stats).to.be.instanceOf(Stats); - }); }); context('UserData.write', function () { diff --git a/packages/compass-user-data/src/user-data.ts b/packages/compass-user-data/src/user-data.ts index ec4cad6868f..1216f151e79 100644 --- a/packages/compass-user-data/src/user-data.ts +++ b/packages/compass-user-data/src/user-data.ts @@ -10,14 +10,12 @@ const { log, mongoLogId } = createLogger('COMPASS-USER-STORAGE'); type SerializeContent = (content: I) => string; type DeserializeContent = (content: string) => unknown; -type GetFileName = (id: string) => string; export type FileUserDataOptions = { subdir: string; basePath?: string; serialize?: SerializeContent; deserialize?: DeserializeContent; - getFileName?: GetFileName; }; export type AtlasUserDataOptions = { @@ -29,45 +27,11 @@ type ReadOptions = { ignoreErrors: boolean; }; -// Copied from the Node.js fs module. -export interface Stats { - isFile(): boolean; - isDirectory(): boolean; - isBlockDevice(): boolean; - isCharacterDevice(): boolean; - isSymbolicLink(): boolean; - isFIFO(): boolean; - isSocket(): boolean; - dev: number; - ino: number; - mode: number; - nlink: number; - uid: number; - gid: number; - rdev: number; - size: number; - blksize: number; - blocks: number; - atimeMs: number; - mtimeMs: number; - ctimeMs: number; - birthtimeMs: number; - atime: Date; - mtime: Date; - ctime: Date; - birthtime: Date; -} - export interface ReadAllResult { data: z.output[]; errors: Error[]; } -export interface ReadAllWithStatsResult { - data: [z.output, Stats][]; - errors: Error[]; -} - export abstract class IUserData { protected readonly validator: T; protected readonly serialize: SerializeContent>; @@ -100,7 +64,6 @@ export abstract class IUserData { export class FileUserData extends IUserData { private readonly subdir: string; private readonly basePath?: string; - private readonly getFileName: GetFileName; protected readonly semaphore = new Semaphore(100); constructor( @@ -110,13 +73,15 @@ export class FileUserData extends IUserData { basePath, serialize, deserialize, - getFileName = (id) => `${id}.json`, }: FileUserDataOptions> ) { super(validator, { serialize, deserialize }); this.subdir = subdir; this.basePath = basePath; - this.getFileName = getFileName; + } + + private getFileName(id: string) { + return `${id}.json`; } private async getEnsuredBasePath(): Promise { @@ -148,21 +113,15 @@ export class FileUserData extends IUserData { return path.resolve(root, pathRelativeToRoot); } - private async readAndParseFileWithStats( + private async readAndParseFile( absolutePath: string, options: ReadOptions - ): Promise<[z.output, Stats] | undefined> { + ): Promise | undefined> { let data: string; - let stats: Stats; - let handle: fs.FileHandle | undefined = undefined; let release: (() => void) | undefined = undefined; try { release = await this.semaphore.waitForRelease(); - handle = await fs.open(absolutePath, 'r'); - [stats, data] = await Promise.all([ - handle.stat(), - handle.readFile('utf-8'), - ]); + data = await fs.readFile(absolutePath, 'utf-8'); } catch (error) { log.error(mongoLogId(1_001_000_234), 'Filesystem', 'Error reading file', { path: absolutePath, @@ -173,13 +132,12 @@ export class FileUserData extends IUserData { } throw error; } finally { - await handle?.close(); release?.(); } try { const content = this.deserialize(data); - return [this.validator.parse(content), stats]; + return this.validator.parse(content); } catch (error) { log.error(mongoLogId(1_001_000_235), 'Filesystem', 'Error parsing data', { path: absolutePath, @@ -235,70 +193,37 @@ export class FileUserData extends IUserData { } } - async readAllWithStats( + async readAll( options: ReadOptions = { ignoreErrors: true, } - ): Promise> { - const absolutePath = await this.getFileAbsolutePath(); - const filePathList = await fs.readdir(absolutePath); - - const data = await Promise.allSettled( - filePathList.map((x) => - this.readAndParseFileWithStats(path.join(absolutePath, x), options) - ) - ); - - const result: ReadAllWithStatsResult = { + ): Promise> { + const result: ReadAllResult = { data: [], errors: [], }; - - for (const item of data) { - if (item.status === 'fulfilled' && item.value) { - result.data.push(item.value); + try { + const absolutePath = await this.getFileAbsolutePath(); + const filePathList = await fs.readdir(absolutePath); + for (const settled of await Promise.allSettled( + filePathList.map((x) => { + return this.readAndParseFile(path.join(absolutePath, x), options); + }) + )) { + if (settled.status === 'fulfilled' && settled.value) { + result.data.push(settled.value); + } + if (settled.status === 'rejected') { + result.errors.push(settled.reason); + } } - if (item.status === 'rejected') { - result.errors.push(item.reason); + return result; + } catch (err) { + if (options.ignoreErrors) { + return result; } + throw err; } - - return result; - } - - async readOneWithStats( - id: string, - options?: { ignoreErrors: false } - ): Promise<[z.output, Stats]>; - async readOneWithStats( - id: string, - options?: { ignoreErrors: true } - ): Promise<[z.output, Stats] | undefined>; - async readOneWithStats( - id: string, - options?: ReadOptions - ): Promise<[z.output, Stats] | undefined>; - async readOneWithStats( - id: string, - options: ReadOptions = { - ignoreErrors: true, - } - ) { - const filepath = this.getFileName(id); - const absolutePath = await this.getFileAbsolutePath(filepath); - return await this.readAndParseFileWithStats(absolutePath, options); - } - - async readAll( - options: ReadOptions = { - ignoreErrors: true, - } - ): Promise> { - const result = await this.readAllWithStats(options); - return { - data: result.data.map(([data]) => data), - errors: result.errors, - }; } async readOne( @@ -319,7 +244,9 @@ export class FileUserData extends IUserData { ignoreErrors: true, } ) { - return (await this.readOneWithStats(id, options))?.[0]; + const filepath = this.getFileName(id); + const absolutePath = await this.getFileAbsolutePath(filepath); + return await this.readAndParseFile(absolutePath, options); } async updateAttributes( diff --git a/packages/my-queries-storage/src/compass-pipeline-storage.ts b/packages/my-queries-storage/src/compass-pipeline-storage.ts index 766fc0166a0..9b9f868a71d 100644 --- a/packages/my-queries-storage/src/compass-pipeline-storage.ts +++ b/packages/my-queries-storage/src/compass-pipeline-storage.ts @@ -1,4 +1,3 @@ -import type { Stats } from '@mongodb-js/compass-user-data'; import { FileUserData } from '@mongodb-js/compass-user-data'; import { PipelineSchema } from './pipeline-storage-schema'; import type { SavedPipeline } from './pipeline-storage-schema'; @@ -13,21 +12,10 @@ export class CompassPipelineStorage implements PipelineStorage { }); } - private mergeStats(pipeline: SavedPipeline, stats: Stats): SavedPipeline { - return { - ...pipeline, - lastModified: new Date(stats.ctimeMs), - }; - } - async loadAll(): Promise { try { - const { data } = await this.userData.readAllWithStats({ - ignoreErrors: false, - }); - return data.map(([item, stats]) => { - return this.mergeStats(item, stats); - }); + const { data } = await this.userData.readAll(); + return data; } catch { return []; } @@ -41,16 +29,11 @@ export class CompassPipelineStorage implements PipelineStorage { } private async loadOne(id: string): Promise { - const [item, stats] = await this.userData.readOneWithStats(id); - return this.mergeStats(item, stats); + return await this.userData.readOne(id); } async createOrUpdate(id: string, attributes: SavedPipeline) { - const pipelineExists = Boolean( - await this.userData.readOne(id, { - ignoreErrors: true, - }) - ); + const pipelineExists = Boolean(await this.userData.readOne(id)); return await (pipelineExists ? this.updateAttributes(id, attributes) : this.create(attributes)); diff --git a/packages/my-queries-storage/src/pipeline-storage-schema.ts b/packages/my-queries-storage/src/pipeline-storage-schema.ts index 35bfbe53ada..9173c4eea24 100644 --- a/packages/my-queries-storage/src/pipeline-storage-schema.ts +++ b/packages/my-queries-storage/src/pipeline-storage-schema.ts @@ -62,8 +62,8 @@ export const PipelineSchema = z.preprocess( pipelineText: z.string(), lastModified: z .number() - .transform((x) => new Date(x)) - .optional(), + .default(0) + .transform((x) => new Date(x)), }) ); From 3f8405add6f40c867e9073216e467929fbb6e78c Mon Sep 17 00:00:00 2001 From: Sergey Petushkov Date: Wed, 23 Jul 2025 13:17:18 +0200 Subject: [PATCH 2/4] fix(user-data): remove nonexistent exports --- packages/compass-user-data/src/index.ts | 2 +- packages/compass-user-data/src/user-data.spec.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/compass-user-data/src/index.ts b/packages/compass-user-data/src/index.ts index b216fd77f99..14a3b49677c 100644 --- a/packages/compass-user-data/src/index.ts +++ b/packages/compass-user-data/src/index.ts @@ -1,3 +1,3 @@ -export type { Stats, ReadAllResult, ReadAllWithStatsResult } from './user-data'; +export type { ReadAllResult } from './user-data'; export { IUserData, FileUserData } from './user-data'; export { z } from 'zod'; diff --git a/packages/compass-user-data/src/user-data.spec.ts b/packages/compass-user-data/src/user-data.spec.ts index e610a072fb9..88c785a2dc9 100644 --- a/packages/compass-user-data/src/user-data.spec.ts +++ b/packages/compass-user-data/src/user-data.spec.ts @@ -1,5 +1,4 @@ import fs from 'fs/promises'; -import { Stats } from 'fs'; import os from 'os'; import path from 'path'; import { expect } from 'chai'; From 7f07ddd8d611926679bba0df10ae03eb475fb146 Mon Sep 17 00:00:00 2001 From: Sergey Petushkov Date: Wed, 23 Jul 2025 16:25:04 +0200 Subject: [PATCH 3/4] fix(my-queries-storage): adjust types to match the method behavior --- .../my-queries-storage/src/compass-pipeline-storage.ts | 7 +++++-- packages/my-queries-storage/src/pipeline-storage.ts | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/my-queries-storage/src/compass-pipeline-storage.ts b/packages/my-queries-storage/src/compass-pipeline-storage.ts index 9b9f868a71d..069707ae1e7 100644 --- a/packages/my-queries-storage/src/compass-pipeline-storage.ts +++ b/packages/my-queries-storage/src/compass-pipeline-storage.ts @@ -32,14 +32,17 @@ export class CompassPipelineStorage implements PipelineStorage { return await this.userData.readOne(id); } - async createOrUpdate(id: string, attributes: SavedPipeline) { + async createOrUpdate( + id: string, + attributes: Omit + ) { const pipelineExists = Boolean(await this.userData.readOne(id)); return await (pipelineExists ? this.updateAttributes(id, attributes) : this.create(attributes)); } - private async create(data: SavedPipeline) { + private async create(data: Omit) { await this.userData.write(data.id, { ...data, lastModified: Date.now(), diff --git a/packages/my-queries-storage/src/pipeline-storage.ts b/packages/my-queries-storage/src/pipeline-storage.ts index 7924645e6fe..faca39feb35 100644 --- a/packages/my-queries-storage/src/pipeline-storage.ts +++ b/packages/my-queries-storage/src/pipeline-storage.ts @@ -5,7 +5,10 @@ export interface PipelineStorage { loadMany( predicate: (arg0: SavedPipeline) => boolean ): Promise; - createOrUpdate(id: string, attributes: SavedPipeline): Promise; + createOrUpdate( + id: string, + attributes: Omit + ): Promise; updateAttributes( id: string, attributes: Partial From e09d426222a3c2bdf66130916109159f052b92ce Mon Sep 17 00:00:00 2001 From: Sergey Petushkov Date: Wed, 23 Jul 2025 16:53:32 +0200 Subject: [PATCH 4/4] fix(saved-aggregations-queries): adjust test fixture --- .../compass-saved-aggregations-queries/src/index.spec.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/compass-saved-aggregations-queries/src/index.spec.tsx b/packages/compass-saved-aggregations-queries/src/index.spec.tsx index eb1f7d1686d..b7cff3427ca 100644 --- a/packages/compass-saved-aggregations-queries/src/index.spec.tsx +++ b/packages/compass-saved-aggregations-queries/src/index.spec.tsx @@ -227,9 +227,11 @@ describe('AggregationsAndQueriesAndUpdatemanyList', function () { queryStorageLoadAllStub = sandbox .stub(queryStorage, 'loadAll') .resolves(queries.map((item) => item.query)); - sandbox - .stub(pipelineStorage, 'loadAll') - .resolves(pipelines.map((item) => item.aggregation)); + sandbox.stub(pipelineStorage, 'loadAll').resolves( + pipelines.map((item) => { + return { ...item.aggregation, lastModified: new Date() }; + }) + ); renderPlugin();