Skip to content

Commit

Permalink
Added DeleteModel functionality and tests (#782)
Browse files Browse the repository at this point in the history
* Added DeleteModel functionality and tests
  • Loading branch information
ifielker authored Feb 11, 2020
1 parent 3f2ec51 commit 3db0ebe
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 1 deletion.
12 changes: 12 additions & 0 deletions src/machine-learning/machine-learning-api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Expand Down
2 changes: 1 addition & 1 deletion src/machine-learning/machine-learning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
15 changes: 15 additions & 0 deletions test/integration/machine-learning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
78 changes: 78 additions & 0 deletions test/unit/machine-learning/machine-learning-api-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
20 changes: 20 additions & 0 deletions test/unit/machine-learning/machine-learning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});

0 comments on commit 3db0ebe

Please sign in to comment.