Skip to content

Implemented the listRulesetMetadata() API #622

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 1 commit into from
Aug 19, 2019
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
37 changes: 37 additions & 0 deletions src/security-rules/security-rules-api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export interface RulesetResponse extends RulesetContent {
readonly createTime: string;
}

export interface ListRulesetsResponse {
readonly rulesets: Array<{name: string, createTime: string}>;
readonly nextPageToken?: string;
}

/**
* Class that facilitates sending requests to the Firebase security rules backend API.
*
Expand Down Expand Up @@ -119,6 +124,38 @@ export class SecurityRulesApiClient {
});
}

public listRulesets(pageSize: number = 100, pageToken?: string): Promise<ListRulesetsResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you even want to allow users to customize this number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point. This was approved during the API review process on the grounds that other similar APIs in the admin SDK (e.g. auth.listUsers()) allow setting custom page sizes. So we are being consistent here. But I agree. Most users won't need it.

if (!validator.isNumber(pageSize)) {
const err = new FirebaseSecurityRulesError('invalid-argument', 'Invalid page size.');
return Promise.reject(err);
}
if (pageSize < 1 || pageSize > 100) {
const err = new FirebaseSecurityRulesError(
'invalid-argument', 'Page size must be between 1 and 100.');
return Promise.reject(err);
}
if (typeof pageToken !== 'undefined' && !validator.isNonEmptyString(pageToken)) {
const err = new FirebaseSecurityRulesError(
'invalid-argument', 'Next page token must be a non-empty string.');
return Promise.reject(err);
}

const data = {
pageSize,
pageToken,
};
if (!pageToken) {
delete data.pageToken;
}

const request: HttpRequestConfig = {
method: 'GET',
url: `${this.url}/rulesets`,
data,
};
return this.sendRequest<ListRulesetsResponse>(request);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider providing a convenience method that repeatedly paginates until the end and then returns the entire list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't really done that sort of thing in Node.js before. But that's something we can certainly consider in the future.

I think we haven't done this for APIs like auth.listUsers(), since a project can potentially have millions of users, and we don't want an implementation that buffers all that data in memory. But since there's a hard limit on how many rulesets a project can have, it is probably not a bad idea here.

In some of the other languages like Java and Python we already have established iterable/iterator patterns to support this use case cleanly. e.g. https://firebase.google.com/docs/reference/admin/java/reference/com/google/firebase/auth/ListUsersPage.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clever

}

public getRelease(name: string): Promise<Release> {
return this.getResource<Release>(`releases/${name}`);
}
Expand Down
52 changes: 51 additions & 1 deletion src/security-rules/security-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import { FirebaseServiceInterface, FirebaseServiceInternalsInterface } from '../
import { FirebaseApp } from '../firebase-app';
import * as utils from '../utils/index';
import * as validator from '../utils/validator';
import { SecurityRulesApiClient, RulesetResponse, RulesetContent, Release } from './security-rules-api-client';
import {
SecurityRulesApiClient, RulesetResponse, RulesetContent, ListRulesetsResponse,
} from './security-rules-api-client';
import { AuthorizedHttpClient } from '../utils/api-request';
import { FirebaseSecurityRulesError } from './security-rules-utils';

Expand All @@ -38,6 +40,39 @@ export interface RulesetMetadata {
readonly createTime: string;
}

/**
* A page of ruleset metadata.
*/
export interface RulesetMetadataList {
readonly rulesets: RulesetMetadata[];
readonly nextPageToken?: string;
}

class RulesetMetadataListImpl implements RulesetMetadataList {

public readonly rulesets: RulesetMetadata[];
public readonly nextPageToken?: string;

constructor(response: ListRulesetsResponse) {
if (!validator.isNonNullObject(response) || !validator.isArray(response.rulesets)) {
throw new FirebaseSecurityRulesError(
'invalid-argument',
`Invalid ListRulesets response: ${JSON.stringify(response)}`);
}

this.rulesets = response.rulesets.map((rs) => {
return {
name: stripProjectIdPrefix(rs.name),
createTime: new Date(rs.createTime).toUTCString(),
};
});

if (response.nextPageToken) {
this.nextPageToken = response.nextPageToken;
}
}
}

/**
* Represents a set of Firebase security rules.
*/
Expand Down Expand Up @@ -270,6 +305,21 @@ export class SecurityRules implements FirebaseServiceInterface {
return this.client.deleteRuleset(name);
}

/**
* Retrieves a page of rulesets.
*
* @param {number=} pageSize The page size, 100 if undefined. This is also the maximum allowed limit.
* @param {string=} nextPageToken The next page token. If not specified, returns rulesets starting
* without any offset.
* @returns {Promise<RulesetMetadataList>} A promise that fulfills a page of rulesets.
*/
public listRulesetMetadata(pageSize: number = 100, nextPageToken?: string): Promise<RulesetMetadataList> {
return this.client.listRulesets(pageSize, nextPageToken)
.then((response) => {
return new RulesetMetadataListImpl(response);
});
}

private getRulesetForRelease(releaseName: string): Promise<Ruleset> {
return this.client.getRelease(releaseName)
.then((release) => {
Expand Down
128 changes: 128 additions & 0 deletions test/unit/security-rules/security-rules-api-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,134 @@ describe('SecurityRulesApiClient', () => {
});
});

describe('listRulesets', () => {
const LIST_RESPONSE = {
rulesets: [
{
name: 'rs1',
createTime: 'date1',
},
],
nextPageToken: 'next',
};

const invalidPageSizes: any[] = [null, '', '10', true, {}, []];
invalidPageSizes.forEach((invalidPageSize) => {
it(`should reject when called with invalid page size: ${JSON.stringify(invalidPageSize)}`, () => {
return apiClient.listRulesets(invalidPageSize)
.should.eventually.be.rejected.and.have.property(
'message', 'Invalid page size.');
});
});

const outOfRangePageSizes: number[] = [-1, 0, 101];
outOfRangePageSizes.forEach((invalidPageSize) => {
it(`should reject when called with invalid page size: ${invalidPageSize}`, () => {
return apiClient.listRulesets(invalidPageSize)
.should.eventually.be.rejected.and.have.property(
'message', 'Page size must be between 1 and 100.');
});
});

const invalidPageTokens: any[] = [null, 0, '', true, {}, []];
invalidPageTokens.forEach((invalidPageToken) => {
it(`should reject when called with invalid page token: ${JSON.stringify(invalidPageToken)}`, () => {
return apiClient.listRulesets(10, invalidPageToken)
.should.eventually.be.rejected.and.have.property(
'message', 'Next page token must be a non-empty string.');
});
});

it('should resolve on success when called without any arguments', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
.resolves(utils.responseFrom(LIST_RESPONSE));
stubs.push(stub);
return apiClient.listRulesets()
.then((resp) => {
expect(resp).to.deep.equal(LIST_RESPONSE);
expect(stub).to.have.been.calledOnce.and.calledWith({
method: 'GET',
url: 'https://firebaserules.googleapis.com/v1/projects/test-project/rulesets',
data: {pageSize: 100},
});
});
});

it('should resolve on success when called with a page size', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
.resolves(utils.responseFrom(LIST_RESPONSE));
stubs.push(stub);
return apiClient.listRulesets(50)
.then((resp) => {
expect(resp).to.deep.equal(LIST_RESPONSE);
expect(stub).to.have.been.calledOnce.and.calledWith({
method: 'GET',
url: 'https://firebaserules.googleapis.com/v1/projects/test-project/rulesets',
data: {pageSize: 50},
});
});
});

it('should resolve on success when called with a page token', () => {
const stub = sinon
.stub(HttpClient.prototype, 'send')
.resolves(utils.responseFrom(LIST_RESPONSE));
stubs.push(stub);
return apiClient.listRulesets(50, 'next')
.then((resp) => {
expect(resp).to.deep.equal(LIST_RESPONSE);
expect(stub).to.have.been.calledOnce.and.calledWith({
method: 'GET',
url: 'https://firebaserules.googleapis.com/v1/projects/test-project/rulesets',
data: {pageSize: 50, pageToken: 'next'},
});
});
});

it('should throw 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 FirebaseSecurityRulesError('not-found', 'Requested entity not found');
return apiClient.listRulesets()
.should.eventually.be.rejected.and.deep.equal(expected);
});

it('should throw 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 FirebaseSecurityRulesError('unknown-error', 'Unknown server error: {}');
return apiClient.listRulesets()
.should.eventually.be.rejected.and.deep.equal(expected);
});

it('should throw 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 FirebaseSecurityRulesError(
'unknown-error', 'Unexpected response with status: 404 and body: not json');
return apiClient.listRulesets()
.should.eventually.be.rejected.and.deep.equal(expected);
});

it('should throw when rejected 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.listRulesets()
.should.eventually.be.rejected.and.deep.equal(expected);
});
});

describe('getRelease', () => {
it('should resolve with the requested release on success', () => {
const stub = sinon
Expand Down
129 changes: 129 additions & 0 deletions test/unit/security-rules/security-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,4 +730,133 @@ describe('SecurityRules', () => {
return securityRules.deleteRuleset('foo');
});
});

describe('listRulesetMetadata', () => {
const LIST_RULESETS_RESPONSE = {
rulesets: [
{
name: 'projects/test-project/rulesets/rs1',
createTime: '2019-03-08T23:45:23.288047Z',
},
{
name: 'projects/test-project/rulesets/rs2',
createTime: '2019-03-08T23:45:23.288047Z',
},
],
nextPageToken: 'next',
};

it('should propagate API errors', () => {
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.rejects(EXPECTED_ERROR);
stubs.push(stub);
return securityRules.listRulesetMetadata()
.should.eventually.be.rejected.and.deep.equal(EXPECTED_ERROR);
});

it('should reject when API response is invalid', () => {
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(null);
stubs.push(stub);
return securityRules.listRulesetMetadata()
.should.eventually.be.rejected.and.have.property(
'message', 'Invalid ListRulesets response: null');
});

it('should reject when API response does not contain rulesets', () => {
const response: any = deepCopy(LIST_RULESETS_RESPONSE);
response.rulesets = '';
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(response);
stubs.push(stub);
return securityRules.listRulesetMetadata()
.should.eventually.be.rejected.and.have.property(
'message', `Invalid ListRulesets response: ${JSON.stringify(response)}`);
});

it('should resolve with RulesetMetadataList on success', () => {
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(LIST_RULESETS_RESPONSE);
stubs.push(stub);

return securityRules.listRulesetMetadata()
.then((result) => {
expect(result.rulesets.length).equals(2);
expect(result.rulesets[0].name).equals('rs1');
expect(result.rulesets[0].createTime).equals(CREATE_TIME_UTC);
expect(result.rulesets[1].name).equals('rs2');
expect(result.rulesets[1].createTime).equals(CREATE_TIME_UTC);

expect(result.nextPageToken).equals('next');

expect(stub).to.have.been.calledOnce.and.calledWith(100);
});
});

it('should resolve with RulesetMetadataList on success when called with page size', () => {
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(LIST_RULESETS_RESPONSE);
stubs.push(stub);

return securityRules.listRulesetMetadata(10)
.then((result) => {
expect(result.rulesets.length).equals(2);
expect(result.rulesets[0].name).equals('rs1');
expect(result.rulesets[0].createTime).equals(CREATE_TIME_UTC);
expect(result.rulesets[1].name).equals('rs2');
expect(result.rulesets[1].createTime).equals(CREATE_TIME_UTC);

expect(result.nextPageToken).equals('next');

expect(stub).to.have.been.calledOnce.and.calledWith(10);
});
});

it('should resolve with RulesetMetadataList on success when called with page token', () => {
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(LIST_RULESETS_RESPONSE);
stubs.push(stub);

return securityRules.listRulesetMetadata(10, 'next')
.then((result) => {
expect(result.rulesets.length).equals(2);
expect(result.rulesets[0].name).equals('rs1');
expect(result.rulesets[0].createTime).equals(CREATE_TIME_UTC);
expect(result.rulesets[1].name).equals('rs2');
expect(result.rulesets[1].createTime).equals(CREATE_TIME_UTC);

expect(result.nextPageToken).equals('next');

expect(stub).to.have.been.calledOnce.and.calledWith(10, 'next');
});
});

it('should resolve with RulesetMetadataList when the response contains no page token', () => {
const response = deepCopy(LIST_RULESETS_RESPONSE);
delete response.nextPageToken;
const stub = sinon
.stub(SecurityRulesApiClient.prototype, 'listRulesets')
.resolves(response);
stubs.push(stub);

return securityRules.listRulesetMetadata(10, 'next')
.then((result) => {
expect(result.rulesets.length).equals(2);
expect(result.rulesets[0].name).equals('rs1');
expect(result.rulesets[0].createTime).equals(CREATE_TIME_UTC);
expect(result.rulesets[1].name).equals('rs2');
expect(result.rulesets[1].createTime).equals(CREATE_TIME_UTC);

expect(result.nextPageToken).to.be.undefined;

expect(stub).to.have.been.calledOnce.and.calledWith(10, 'next');
});
});
});
});