Skip to content

Added DeleteModel functionality and tests #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});
});