fix(user-data): make create and update return type to be boolean, like queries

Author: myang1220

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

@@ -404,26 +404,9 @@ export class AtlasUserData<T extends z.Schema> extends IUserData<T> {
     data: Partial<z.input<T>>
   ): Promise<boolean> {
     try {
-      // TODO: change this depending on whether or not updateAttributes can provide all current data
-      const getResponse = await this.authenticatedFetch(
-        await this.getResourceUrl(
-          `${this.dataType}/${this.orgId}/${this.projectId}/${id}`
-        ),
-        {
-          method: 'GET',
-        }
-      );
-      if (!getResponse.ok) {
-        throw new Error(
-          `Failed to fetch data: ${getResponse.status} ${getResponse.statusText}`
-        );
-      }
-      const prevData = await getResponse.json();
-      const validPrevData = this.validator.parse(
-        this.deserialize(prevData.data as string)
-      );
+      const prevData = await this.readOne(id);
       const newData: z.input<T> = {
-        ...validPrevData,
+        ...prevData,
         ...data,
       };
 
@@ -460,4 +443,39 @@ export class AtlasUserData<T extends z.Schema> extends IUserData<T> {
       return false;
     }
   }
+
+  // TODO: change this depending on whether or not updateAttributes can provide all current data
+  async readOne(id: string): Promise<z.output<T>> {
+    try {
+      const getResponse = await this.authenticatedFetch(
+        await this.getResourceUrl(
+          `${this.dataType}/${this.orgId}/${this.projectId}/${id}`
+        ),
+        {
+          method: 'GET',
+        }
+      );
+      if (!getResponse.ok) {
+        throw new Error(
+          `Failed to fetch data: ${getResponse.status} ${getResponse.statusText}`
+        );
+      }
+      const json = await getResponse.json();
+      const data = this.validator.parse(this.deserialize(json.data as string));
+      return data;
+    } catch {
+      log.error(
+        mongoLogId(1_001_000_365),
+        'Atlas Backend',
+        'Error reading data',
+        {
+          url: await this.getResourceUrl(
+            `${this.dataType}/${this.orgId}/${this.projectId}/${id}`
+          ),
+          error: (error as Error).message,
+        }
+      );
+      return null;
+    }
+  }
 }

packages/my-queries-storage/src/compass-pipeline-storage.spec.ts

@@ -87,14 +87,12 @@ describe('CompassPipelineStorage', function () {
       expect((e as any).code).to.equal('ENOENT');
     }
 
-    const pipeline = await pipelineStorage.createOrUpdate(data.id, data);
+    const result = await pipelineStorage.createOrUpdate(data.id, data);
 
     // Verify the file exists
     await fs.access(await getEnsuredFilePath(tmpDir, data.id));
 
-    expect(pipeline.id).to.equal(data.id);
-    expect(pipeline.name).to.equal(data.name);
-    expect(pipeline.pipelineText).to.equal(data.pipelineText);
+    expect(result).to.be.true;
   });
 
   it('createOrUpdate - updates a pipeline if it exists', async function () {
@@ -108,14 +106,12 @@ describe('CompassPipelineStorage', function () {
     await createPipeline(tmpDir, data);
     await fs.access(await getEnsuredFilePath(tmpDir, data.id));
 
-    const pipeline = await pipelineStorage.createOrUpdate(data.id, {
+    const result = await pipelineStorage.createOrUpdate(data.id, {
       ...data,
       name: 'modified listings',
     });
 
-    expect(pipeline.id).to.equal(data.id);
-    expect(pipeline.name).to.equal('modified listings');
-    expect(pipeline.pipelineText).to.equal(data.pipelineText);
+    expect(result).to.be.true;
   });
 
   it('updateAttributes - updates a pipeline if it exists', async function () {
@@ -136,17 +132,13 @@ describe('CompassPipelineStorage', function () {
       expect(restOfAggregation).to.deep.equal(data);
     }
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    const { lastModified, pipelineText, ...updatedAggregation } =
-      await pipelineStorage.updateAttributes(data.id, {
-        name: 'updated',
-        namespace: 'airbnb.users',
-      });
-
-    expect(updatedAggregation, 'returns updated pipeline').to.deep.equal({
-      ...data,
+    const result = await pipelineStorage.updateAttributes(data.id, {
       name: 'updated',
+      namespace: 'airbnb.users',
     });
 
+    expect(result).to.be.true;
+
     {
       const aggregations = await pipelineStorage.loadAll();
       expect(aggregations).to.have.length(1);

packages/my-queries-storage/src/compass-pipeline-storage.ts

@@ -27,10 +27,6 @@ export class CompassPipelineStorage implements PipelineStorage {
     return this.loadAll().then((pipelines) => pipelines.filter(predicate));
   }
 
-  private async loadOne(id: string): Promise<SavedPipeline> {
-    return await this.userData.readOne(id);
-  }
-
   async createOrUpdate(
     id: string,
     attributes: Omit<SavedPipeline, 'lastModified'>
@@ -41,24 +37,32 @@ export class CompassPipelineStorage implements PipelineStorage {
       : this.create(attributes));
   }
 
-  async create(data: Omit<SavedPipeline, 'lastModified'>) {
-    await this.userData.write(data.id, {
-      ...data,
-      lastModified: Date.now(),
-    });
-    return await this.loadOne(data.id);
+  async create(data: Omit<SavedPipeline, 'lastModified'>): Promise<boolean> {
+    try {
+      await this.userData.write(data.id, {
+        ...data,
+        lastModified: Date.now(),
+      });
+      return true;
+    } catch {
+      return false;
+    }
   }
 
   async updateAttributes(
     id: string,
     attributes: Partial<SavedPipeline>
-  ): Promise<SavedPipeline> {
-    await this.userData.write(id, {
-      ...(await this.loadOne(id)),
-      ...attributes,
-      lastModified: Date.now(),
-    });
-    return await this.loadOne(id);
+  ): Promise<boolean> {
+    try {
+      await this.userData.write(id, {
+        ...(await this.userData.readOne(id)),
+        ...attributes,
+        lastModified: Date.now(),
+      });
+      return true;
+    } catch {
+      return false;
+    }
   }
 
   async delete(id: string) {

packages/my-queries-storage/src/pipeline-storage.ts

@@ -5,11 +5,11 @@ export interface PipelineStorage {
   loadMany(
     predicate: (arg0: SavedPipeline) => boolean
   ): Promise<SavedPipeline[]>;
-  createOrUpdate(id: string, attributes: SavedPipeline): Promise<SavedPipeline>;
-  create(attributes: SavedPipeline): Promise<SavedPipeline>;
+  createOrUpdate(id: string, attributes: SavedPipeline): Promise<boolean>;
+  create(attributes: SavedPipeline): Promise<boolean>;
   updateAttributes(
     id: string,
     attributes: Partial<SavedPipeline>
-  ): Promise<SavedPipeline>;
+  ): Promise<boolean>;
   delete(id: string): Promise<void>;
 }