chore(user-data): remove redundant withStats methods

Author: gribnoysup

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`;
-  }
 }

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 () {

packages/compass-user-data/src/user-data.ts

@@ -10,14 +10,12 @@ const { log, mongoLogId } = createLogger('COMPASS-USER-STORAGE');
 
 type SerializeContent<I> = (content: I) => string;
 type DeserializeContent = (content: string) => unknown;
-type GetFileName = (id: string) => string;
 
 export type FileUserDataOptions<Input> = {
   subdir: string;
   basePath?: string;
   serialize?: SerializeContent<Input>;
   deserialize?: DeserializeContent;
-  getFileName?: GetFileName;
 };
 
 export type AtlasUserDataOptions<Input> = {
@@ -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<T extends z.Schema> {
   data: z.output<T>[];
   errors: Error[];
 }
 
-export interface ReadAllWithStatsResult<T extends z.Schema> {
-  data: [z.output<T>, Stats][];
-  errors: Error[];
-}
-
 export abstract class IUserData<T extends z.Schema> {
   protected readonly validator: T;
   protected readonly serialize: SerializeContent<z.input<T>>;
@@ -100,7 +64,6 @@ export abstract class IUserData<T extends z.Schema> {
 export class FileUserData<T extends z.Schema> extends IUserData<T> {
   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<T extends z.Schema> extends IUserData<T> {
       basePath,
       serialize,
       deserialize,
-      getFileName = (id) => `${id}.json`,
     }: FileUserDataOptions<z.input<T>>
   ) {
     super(validator, { serialize, deserialize });
     this.subdir = subdir;
     this.basePath = basePath;
-    this.getFileName = getFileName;
+  }
+
+  private getFileName(id: string) {
+    return `${id}.json`;
   }
 
   private async getEnsuredBasePath(): Promise<string> {
@@ -148,21 +113,15 @@ export class FileUserData<T extends z.Schema> extends IUserData<T> {
     return path.resolve(root, pathRelativeToRoot);
   }
 
-  private async readAndParseFileWithStats(
+  private async readAndParseFile(
     absolutePath: string,
     options: ReadOptions
-  ): Promise<[z.output<T>, Stats] | undefined> {
+  ): Promise<z.output<T> | 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<T extends z.Schema> extends IUserData<T> {
       }
       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<T extends z.Schema> extends IUserData<T> {
     }
   }
 
-  async readAllWithStats(
+  async readAll(
     options: ReadOptions = {
       ignoreErrors: true,
     }
-  ): Promise<ReadAllWithStatsResult<T>> {
-    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<T> = {
+  ): Promise<ReadAllResult<T>> {
+    const result: ReadAllResult<T> = {
       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<T>, Stats]>;
-  async readOneWithStats(
-    id: string,
-    options?: { ignoreErrors: true }
-  ): Promise<[z.output<T>, Stats] | undefined>;
-  async readOneWithStats(
-    id: string,
-    options?: ReadOptions
-  ): Promise<[z.output<T>, 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<ReadAllResult<T>> {
-    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<T extends z.Schema> extends IUserData<T> {
       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(

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<SavedPipeline[]> {
     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<SavedPipeline> {
-    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));

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)),
   })
 );