feat: add UUID autogeneration for fields listed as autoPopulate in service.yaml #1511
Conversation
|
@alexander-fenster, I think the failing tests are due to gapic-tools not recognizing the field_info.proto. Once we publish a new version of gax with the new version of nodejs-proto-files, I can also update gapic-tools and I think it will just work. |
|
Let me review this after #1503 gets in. |
| @@ -22,6 +22,7 @@ import type {Callback, CallOptions, Descriptors, ClientOptions, GrpcClientOption | |||
|
|
|||
| import * as protos from '../../protos/protos'; | |||
| import jsonProtos = require('../../protos/protos.json'); | |||
| import * as crypto from 'crypto'; | |||
There was a problem hiding this comment.
This immediately disqualifies the library from the browser or any other non-Node.js usage. One way to solve this would be to move everything that needs crypto to google-gax and export it from there, and in gax, make it similar to https://github.com/googleapis/google-auth-library-nodejs/tree/main/src/crypto to use either Node.js crypto or window.crypto.subtle, whichever is available.
Also, I don't see any crypto usage in this particular file, which probably means that this import should be conditional.
| @@ -607,6 +608,9 @@ export class {{ service.name }}Client { | |||
| {%- endif %} | |||
| ]>|void { | |||
| request = request || {}; | |||
| {% for field in method.autoPopulatedFields %}if (!request.{{ field.toCamelCase() }}) { | |||
| request.{{ field.toCamelCase() }} = crypto.randomUUID(); | |||
There was a problem hiding this comment.
To make it compatible with browser, we have two options: either use uuid module instead of crypto (which will force us make uuid a dependency for all libraries), or - probably better - export a createUUID() function from gax, and have it there instead.
…cript into addUUID
typescript/src/schema/proto.ts
Outdated
| @@ -74,6 +75,7 @@ export interface MethodDescriptorProto | |||
|
|
|||
| export interface ServiceDescriptorProto | |||
| extends protos.google.protobuf.IServiceDescriptorProto { | |||
| getAutoPopulatedFields: MethodDescriptorProto[]; | |||
There was a problem hiding this comment.
Is this field actually used anywhere? I see the function getAutoPopulatedFields but I don't see if this added field is accessed anywhere.
| @@ -459,6 +461,9 @@ export class EchoClient { | |||
| protos.google.showcase.v1beta1.IEchoRequest|undefined, {}|undefined | |||
| ]>|void { | |||
| request = request || {}; | |||
| if (!request.requestId) { | |||
| request.requestId = gax.makeUUID(); | |||
There was a problem hiding this comment.
Could we have a generated unit test that would check that this code is actually executed? It would be similar to an existing test that makes a request, but in a stub, it can check that the field was indeed assigned to some UUID. Similar to
.
No description provided.