Skip to content

Commit

Permalink
fix: correctly determine auto-paginated field (#156)
Browse files Browse the repository at this point in the history 
* fix: correctly determine auto-paginated field

* fix: gts fix

* fix: fail on error
  • Loading branch information
@alexander-fenster
alexander-fenster authored Nov 22, 2019
1 parent 71733ab commit a2a88e3
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 127 deletions.
29 changes: 28 additions & 1 deletion typescript/src/schema/proto.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,36 @@ function pagingField(messages: MessagesMap, method: MethodDescriptorProto) {
field.label ===
plugin.google.protobuf.FieldDescriptorProto.Label.LABEL_REPEATED
);
if (repeatedFields.length !== 1) {
if (repeatedFields.length === 0) {
return undefined;
}
if (repeatedFields.length === 1) {
return repeatedFields[0];
}
// According to https://aip.dev/client-libraries/4233, if there are two
// or more repeated fields in the output, we must take the the first one
// (in order of appearance in the file AND field number).
// We believe that all proto fields have numbers, hence !.
const minFieldNumber = repeatedFields.reduce(
(min: number, field: plugin.google.protobuf.IFieldDescriptorProto) => {
if (field.number! < min) {
min = field.number!;
}
return min;
},
repeatedFields[0].number!
);
if (minFieldNumber !== repeatedFields[0].number) {
console.warn(
`Warning: method ${method.name} has several repeated fields in the output type and violates https://aip.dev/client-libraries/4233 for auto-pagination. Disabling auto-pagination for this method.`
);
console.warn('Fields considered for pagination:');
console.warn(
repeatedFields.map(field => `${field.name} = ${field.number}`).join('\n')
);
// TODO: an option to ignore errors
throw new Error(`Bad pagination settings for ${method.name}`);
}
return repeatedFields[0];
}

Expand Down
227 changes: 147 additions & 80 deletions typescript/test/testdata/redis/src/v1beta1/cloud_redis_client.ts.baseline
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
// ** All changes to this file may be overwritten. **

import * as gax from 'google-gax';
import {APICallback, Callback, CallOptions, Descriptors, ClientOptions, LROperation} from 'google-gax';
import {APICallback, Callback, CallOptions, Descriptors, ClientOptions, LROperation, PaginationCallback, PaginationResponse} from 'google-gax';
import * as path from 'path';

import { Transform } from 'stream';
import * as protosTypes from '../../protos/protos';
import * as gapicConfig from './cloud_redis_client_config.json';

Expand Down Expand Up @@ -152,6 +153,14 @@ export class CloudRedisClient {
),
};

// Some of the methods on this service return "paged" results,
// (e.g. 50 results at a time, with tokens to get subsequent
// pages). Denote the keys used for pagination and results.
this._descriptors.page = {
listInstances:
new gaxModule.PageDescriptor('pageToken', 'nextPageToken', 'instances')
};

// This API contains "long-running operations", which return a
// an Operation object that allows for tracking of the operation,
// rather than holding a request open.
Expand Down Expand Up @@ -320,85 +329,6 @@ export class CloudRedisClient {
// -------------------
// -- Service calls --
// -------------------
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
options?: gax.CallOptions):
Promise<[
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse,
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|undefined, {}|undefined
]>;
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
options: gax.CallOptions,
callback: Callback<
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse,
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|undefined,
{}|undefined>): void;
/**
* Lists all Redis instances owned by a project in either the specified
* location (region) or all locations.
*
* The location should have the following format:
* * `projects/{project_id}/locations/{location_id}`
*
* If `location_id` is specified as `-` (wildcard), then all regions
* available to the project are queried, and the results are aggregated.
*
* @param {Object} request
* The request object that will be sent.
* @param {string} request.parent
* Required. The resource name of the instance location using the form:
* `projects/{project_id}/locations/{location_id}`
* where `location_id` refers to a GCP region.
* @param {number} request.pageSize
* The maximum number of items to return.
*
* If not specified, a default value of 1000 will be used by the service.
* Regardless of the page_size value, the response may include a partial list
* and a caller should only rely on response's
* [next_page_token][CloudRedis.ListInstancesResponse.next_page_token]
* to determine if there are more instances left to be queried.
* @param {string} request.pageToken
* The next_page_token value returned from a previous List request,
* if any.
* @param {object} [options]
* Call options. See {@link https://googleapis.dev/nodejs/google-gax/latest/interfaces/CallOptions.html|CallOptions} for more details.
* @returns {Promise} - The promise which resolves to an array.
* The first element of the array is an object representing [ListInstancesResponse]{@link google.cloud.redis.v1beta1.ListInstancesResponse}.
* The promise has a method named "cancel" which cancels the ongoing API call.
*/
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
optionsOrCallback?: gax.CallOptions|Callback<
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse,
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|undefined, {}|undefined>,
callback?: Callback<
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse,
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|undefined,
{}|undefined>):
Promise<[
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse,
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|undefined, {}|undefined
]>|void {
request = request || {};
let options: gax.CallOptions;
if (typeof optionsOrCallback === 'function' && callback === undefined) {
callback = optionsOrCallback;
options = {};
}
else {
options = optionsOrCallback as gax.CallOptions;
}
options = options || {};
options.otherArgs = options.otherArgs || {};
options.otherArgs.headers = options.otherArgs.headers || {};
options.otherArgs.headers[
'x-goog-request-params'
] = gax.routingHeader.fromParams({
'parent': request.parent || '',
});
return this._innerApiCalls.listInstances(request, options, callback);
}
getInstance(
request: protosTypes.google.cloud.redis.v1beta1.IGetInstanceRequest,
options?: gax.CallOptions):
Expand Down Expand Up @@ -882,6 +812,143 @@ export class CloudRedisClient {
});
return this._innerApiCalls.deleteInstance(request, options, callback);
}
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
options?: gax.CallOptions):
Promise<[
protosTypes.google.cloud.redis.v1beta1.IInstance[],
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|null,
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse
]>;
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
options: gax.CallOptions,
callback: Callback<
protosTypes.google.cloud.redis.v1beta1.IInstance[],
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|null,
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse>): void;
/**
* Lists all Redis instances owned by a project in either the specified
* location (region) or all locations.
*
* The location should have the following format:
* * `projects/{project_id}/locations/{location_id}`
*
* If `location_id` is specified as `-` (wildcard), then all regions
* available to the project are queried, and the results are aggregated.
*
* @param {Object} request
* The request object that will be sent.
* @param {string} request.parent
* Required. The resource name of the instance location using the form:
* `projects/{project_id}/locations/{location_id}`
* where `location_id` refers to a GCP region.
* @param {number} request.pageSize
* The maximum number of items to return.
*
* If not specified, a default value of 1000 will be used by the service.
* Regardless of the page_size value, the response may include a partial list
* and a caller should only rely on response's
* [next_page_token][CloudRedis.ListInstancesResponse.next_page_token]
* to determine if there are more instances left to be queried.
* @param {string} request.pageToken
* The next_page_token value returned from a previous List request,
* if any.
* @param {object} [options]
* Call options. See {@link https://googleapis.dev/nodejs/google-gax/latest/interfaces/CallOptions.html|CallOptions} for more details.
* @returns {Promise} - The promise which resolves to an array.
* The first element of the array is an object representing [ListInstancesResponse]{@link google.cloud.redis.v1beta1.ListInstancesResponse}.
*
* When autoPaginate: false is specified through options, the array has three elements.
* The first element is Array of [ListInstancesResponse]{@link google.cloud.redis.v1beta1.ListInstancesResponse} in a single response.
* The second element is the next request object if the response
* indicates the next page exists, or null. The third element is
* an object representing [ListInstancesResponse]{@link google.cloud.redis.v1beta1.ListInstancesResponse}.
*
* The promise has a method named "cancel" which cancels the ongoing API call.
*/
listInstances(
request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
optionsOrCallback?: gax.CallOptions|Callback<
protosTypes.google.cloud.redis.v1beta1.IInstance[],
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|null,
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse>,
callback?: Callback<
protosTypes.google.cloud.redis.v1beta1.IInstance[],
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|null,
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse>):
Promise<[
protosTypes.google.cloud.redis.v1beta1.IInstance[],
protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest|null,
protosTypes.google.cloud.redis.v1beta1.IListInstancesResponse
]>|void {
request = request || {};
let options: gax.CallOptions;
if (typeof optionsOrCallback === 'function' && callback === undefined) {
callback = optionsOrCallback;
options = {};
}
else {
options = optionsOrCallback as gax.CallOptions;
}
options = options || {};
options.otherArgs = options.otherArgs || {};
options.otherArgs.headers = options.otherArgs.headers || {};
options.otherArgs.headers[
'x-goog-request-params'
] = gax.routingHeader.fromParams({
'parent': request.parent || '',
});
return this._innerApiCalls.listInstances(request, options, callback);
}

/**
* Equivalent to {@link listInstances}, but returns a NodeJS Stream object.
*
* This fetches the paged responses for {@link listInstances} continuously
* and invokes the callback registered for 'data' event for each element in the
* responses.
*
* The returned object has 'end' method when no more elements are required.
*
* autoPaginate option will be ignored.
*
* @see {@link https://nodejs.org/api/stream.html}
*
* @param {Object} request
* The request object that will be sent.
* @param {string} request.parent
* Required. The resource name of the instance location using the form:
* `projects/{project_id}/locations/{location_id}`
* where `location_id` refers to a GCP region.
* @param {number} request.pageSize
* The maximum number of items to return.
*
* If not specified, a default value of 1000 will be used by the service.
* Regardless of the page_size value, the response may include a partial list
* and a caller should only rely on response's
* [next_page_token][CloudRedis.ListInstancesResponse.next_page_token]
* to determine if there are more instances left to be queried.
* @param {string} request.pageToken
* The next_page_token value returned from a previous List request,
* if any.
* @param {object} [options]
* Call options. See {@link https://googleapis.dev/nodejs/google-gax/latest/interfaces/CallOptions.html|CallOptions} for more details.
* @returns {Stream}
* An object stream which emits an object representing [Instance]{@link google.cloud.redis.v1beta1.Instance} on 'data' event.
*/
listInstancesStream(
request?: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest,
options?: gax.CallOptions | {}):
Transform{
request = request || {};
const callSettings = new gax.CallSettings(options);
return this._descriptors.page.listInstances.createStream(
this._innerApiCalls.listInstances as gax.GaxCall,
request,
callSettings
);
}
// --------------------
// -- Path templates --
// --------------------
Expand Down
92 changes: 46 additions & 46 deletions typescript/test/testdata/redis/test/gapic-cloud_redis-v1beta1.ts.baseline
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,52 +95,6 @@ describe('v1beta1.CloudRedisClient', () => {
});
assert(client);
});
describe('listInstances', () => {
it('invokes listInstances without error', done => {
const client = new cloudredisModule.v1beta1.CloudRedisClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});
// Mock request
const request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest = {};
// Mock response
const expectedResponse = {};
// Mock gRPC layer
client._innerApiCalls.listInstances = mockSimpleGrpcMethod(
request,
expectedResponse,
null
);
client.listInstances(request, (err: {}, response: {}) => {
assert.ifError(err);
assert.deepStrictEqual(response, expectedResponse);
done();
})
});

it('invokes listInstances with error', done => {
const client = new cloudredisModule.v1beta1.CloudRedisClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});
// Mock request
const request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest = {};
// Mock response
const expectedResponse = {};
// Mock gRPC layer
client._innerApiCalls.listInstances = mockSimpleGrpcMethod(
request,
null,
error
);
client.listInstances(request, (err: FakeError, response: {}) => {
assert(err instanceof FakeError);
assert.strictEqual(err.code, FAKE_STATUS_CODE);
assert(typeof response === 'undefined');
done();
})
});
});
describe('getInstance', () => {
it('invokes getInstance without error', done => {
const client = new cloudredisModule.v1beta1.CloudRedisClient({
Expand Down Expand Up @@ -509,4 +463,50 @@ describe('v1beta1.CloudRedisClient', () => {
});
});
});
describe('listInstances', () => {
it('invokes listInstances without error', done => {
const client = new cloudredisModule.v1beta1.CloudRedisClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});
// Mock request
const request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest = {};
// Mock response
const expectedResponse = {};
// Mock Grpc layer
client._innerApiCalls.listInstances = (actualRequest: {}, options: {}, callback: Callback) => {
assert.deepStrictEqual(actualRequest, request);
callback(null, expectedResponse);
};
client.listInstances(request, (err: FakeError, response: {}) => {
assert.ifError(err);
assert.deepStrictEqual(response, expectedResponse);
done();
});
});
});
describe('listInstancesStream', () => {
it('invokes listInstancesStream without error', done => {
const client = new cloudredisModule.v1beta1.CloudRedisClient({
credentials: {client_email: 'bogus', private_key: 'bogus'},
projectId: 'bogus',
});
// Mock request
const request: protosTypes.google.cloud.redis.v1beta1.IListInstancesRequest = {};
// Mock response
const expectedResponse = {};
// Mock Grpc layer
client._innerApiCalls.listInstances = (actualRequest: {}, options: {}, callback: Callback) => {
assert.deepStrictEqual(actualRequest, request);
callback(null, expectedResponse);
};
const stream = client.listInstancesStream(request, {}).on('data', (response: {}) =>{
assert.deepStrictEqual(response, expectedResponse);
done();
}).on('error', (err: FakeError) => {
done(err);
});
stream.write(request);
});
});
});

0 comments on commit a2a88e3

Please sign in to comment.