Added DeleteModel functionality and tests (#782)

ifielker · web-flow · commit 3db0ebe66a12 · 2020-02-11T11:25:03.000-05:00
* Added DeleteModel functionality and tests
diff --git a/src/machine-learning/machine-learning-api-client.ts b/src/machine-learning/machine-learning-api-client.ts
@@ -83,6 +83,18 @@ export class MachineLearningApiClient {
       });
   }
 
+  public deleteModel(modelId: string): Promise<void> {
+    return this.getUrl()
+      .then((url) => {
+        const modelName = this.getModelName(modelId);
+        const request: HttpRequestConfig = {
+          method: 'DELETE',
+          url: `${url}/${modelName}`,
+        };
+        return this.sendRequest<void>(request);
+      });
+  }
+
   /**
    * Gets the specified resource from the ML API. Resource names must be the short names without project
    * ID prefix (e.g. `models/123456789`).
diff --git a/src/machine-learning/machine-learning.ts b/src/machine-learning/machine-learning.ts
@@ -168,7 +168,7 @@ export class MachineLearning implements FirebaseServiceInterface {
    * @param {string} modelId The id of the model to delete.
    */
   public deleteModel(modelId: string): Promise<void> {
-    throw new Error('NotImplemented');
+    return this.client.deleteModel(modelId);
   }
 }
 
diff --git a/test/integration/machine-learning.spec.ts b/test/integration/machine-learning.spec.ts
@@ -31,4 +31,19 @@ describe('admin.machineLearning', () => {
           'code', 'machine-learning/invalid-argument');
     });
   });
+
+  describe('deleteModel()', () => {
+    it('rejects with not-found when the Model does not exist', () => {
+      const nonExistingName = '00000000';
+      return admin.machineLearning().deleteModel(nonExistingName)
+        .should.eventually.be.rejected.and.have.property(
+          'code', 'machine-learning/not-found');
+    });
+
+    it('rejects with invalid-argument when the Model ID is invalid', () => {
+      return admin.machineLearning().deleteModel('invalid-model-id')
+      .should.eventually.be.rejected.and.have.property(
+        'code', 'machine-learning/invalid-argument');
+    });
+  });
 });
diff --git a/test/unit/machine-learning/machine-learning-api-client.spec.ts b/test/unit/machine-learning/machine-learning-api-client.spec.ts
@@ -156,4 +156,82 @@ describe('MachineLearningApiClient', () => {
         .should.eventually.be.rejected.and.deep.equal(expected);
     });
   });
+
+  describe('deleteModel', () => {
+    const INVALID_NAMES: any[] = [null, undefined, '', 1, true, {}, []];
+    INVALID_NAMES.forEach((invalidName) => {
+      it(`should reject when called with: ${JSON.stringify(invalidName)}`, () => {
+        return apiClient.deleteModel(invalidName)
+          .should.eventually.be.rejected.and.have.property(
+            'message', 'Model ID must be a non-empty string.');
+      });
+    });
+
+    it(`should reject when called with prefixed name`, () => {
+      return apiClient.deleteModel('projects/foo/rulesets/bar')
+        .should.eventually.be.rejected.and.have.property(
+          'message', 'Model ID must not contain any "/" characters.');
+    });
+
+    it(`should reject when project id is not available`, () => {
+      return clientWithoutProjectId.deleteModel(MODEL_ID)
+        .should.eventually.be.rejectedWith(noProjectId);
+    });
+
+    it('should resolve on success', () => {
+      const stub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .resolves(utils.responseFrom({}));
+      stubs.push(stub);
+      return apiClient.deleteModel(MODEL_ID)
+        .then(() => {
+          expect(stub).to.have.been.calledOnce.and.calledWith({
+            method: 'DELETE',
+            url: 'https://mlkit.googleapis.com/v1beta1/projects/test-project/models/1234567',
+            headers: EXPECTED_HEADERS,
+          });
+        });
+    });
+
+    it('should reject when a full platform error response is received', () => {
+      const stub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .rejects(utils.errorFrom(ERROR_RESPONSE, 404));
+      stubs.push(stub);
+      const expected = new FirebaseMachineLearningError('not-found', 'Requested entity not found');
+      return apiClient.deleteModel(MODEL_ID)
+        .should.eventually.be.rejected.and.deep.equal(expected);
+    });
+
+    it('should reject with unknown-error when error code is not present', () => {
+      const stub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .rejects(utils.errorFrom({}, 404));
+      stubs.push(stub);
+      const expected = new FirebaseMachineLearningError('unknown-error', 'Unknown server error: {}');
+      return apiClient.deleteModel(MODEL_ID)
+        .should.eventually.be.rejected.and.deep.equal(expected);
+    });
+
+    it('should reject with unknown-error for non-json response', () => {
+      const stub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .rejects(utils.errorFrom('not json', 404));
+      stubs.push(stub);
+      const expected = new FirebaseMachineLearningError(
+        'unknown-error', 'Unexpected response with status: 404 and body: not json');
+      return apiClient.deleteModel(MODEL_ID)
+        .should.eventually.be.rejected.and.deep.equal(expected);
+    });
+
+    it('should reject when failed with a FirebaseAppError', () => {
+      const expected = new FirebaseAppError('network-error', 'socket hang up');
+      const stub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .rejects(expected);
+      stubs.push(stub);
+      return apiClient.deleteModel(MODEL_ID)
+        .should.eventually.be.rejected.and.deep.equal(expected);
+    });
+  });
 });
diff --git a/test/unit/machine-learning/machine-learning.spec.ts b/test/unit/machine-learning/machine-learning.spec.ts
@@ -241,4 +241,24 @@ describe('MachineLearning', () => {
         });
     });
   });
+
+  describe('deleteModel', () => {
+    it('should propagate API errors', () => {
+      const stub = sinon
+        .stub(MachineLearningApiClient.prototype, 'deleteModel')
+        .rejects(EXPECTED_ERROR);
+      stubs.push(stub);
+      return machineLearning.deleteModel('1234567')
+        .should.eventually.be.rejected.and.deep.equal(EXPECTED_ERROR);
+    });
+
+    it('should resolve on success', () => {
+      const stub = sinon
+        .stub(MachineLearningApiClient.prototype, 'deleteModel')
+        .resolves({});
+      stubs.push(stub);
+
+      return machineLearning.deleteModel('1234567');
+    });
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ export class MachineLearning implements FirebaseServiceInterface {`
`168`	`168`	`* @param {string} modelId The id of the model to delete.`
`169`	`169`	`*/`
`170`	`170`	`public deleteModel(modelId: string): Promise<void> {`
`171`		`- throw new Error('NotImplemented');`
	`171`	`+ return this.client.deleteModel(modelId);`
`172`	`172`	`}`
`173`	`173`	`}`
`174`	`174`