fix: REST: Make make LRO stub accept APIs of different versions#1622
fix: REST: Make make LRO stub accept APIs of different versions#1622vam-google merged 3 commits intogoogleapis:mainfrom
Conversation
Prior to this fix it worked only with `v1` APIs. `HttpJsonOperationsStub` is shared by multiple different services similarly to `GrpcOperationsStub`. Unlike grpc version, the verison of the actual API that this stub is used by is supposed to be encoded in the http url, so the `HttpJsonOperationsStub` must adjust to the actual version of the API it is being used from. Similarly we also need to make the url patterns less restrictive, to accept operaion names in different formats (this is what caused the `{name=}` pattern changes).
The solution is hacky, but simple and should work for all known APIs. It is based on the fact that even though the `HttpJsonOperationsStub` is shared by all clients (i.e. is in `gax`), its `HttpJsonStubCallbackFactory` argument, on the other hand, is a generated one (i.e. is in a gapic client for a specific API) and its package corresponds to the API version, which is a convention for all google APIs (i.e. version in the proto package corresponds to the version of the API).
Doing this the other way would require a lot of work (parsing yaml, generating operations stub on the fly etc), and we may come back to it if even needed (most probably never).
meltsufin
left a comment
meltsufin
left a comment
There was a problem hiding this comment.
It is hacky indeed, but I'll take your word for that the other way is worse.
Please take a look at the potential security issue with the regex reported by SonarCloud. Anything to be concerned about here?
|
@meltsufin The codesmells and security warnings are all basically about usage of regexps (reading descriptions seems like they will be triggered for any code using the regexps no matter what). Here they are definitely not a concern, because the set of potential strings to parse by the regexp is very limited and restricted, since they are always a package name of a gapic-generated class, so any craziness in that package name is restricted and validated by java compiler before even getting to the regexp part. |
|
That's what I thought. Thanks for the clarification. You can mark that issue resolved with the explanation that you provided above so that it goes away. |
|
SonarCloud Quality Gate failed. |
2 participants
Prior to this fix it worked only with
v1APIs.HttpJsonOperationsStubis shared by multiple different services similarly toGrpcOperationsStub. Unlike grpc version, the verison of the actual API that this stub is used by is supposed to be encoded in the http url, so theHttpJsonOperationsStubmust adjust to the actual version of the API it is being used from. Similarly we also need to make the url patterns less restrictive, to accept operaion names in different formats (this is what caused the{name=}pattern changes).The solution is hacky, but simple and should work for all known APIs. It is based on the fact that even though the
HttpJsonOperationsStubis shared by all clients (i.e. is ingax), itsHttpJsonStubCallbackFactoryargument, on the other hand, is a generated one (i.e. is in a gapic client for a specific API) and its package corresponds to the API version, which is a convention for all google APIs (i.e. version in the proto package corresponds to the version of the API).Doing this the other way would require a lot of work (parsing yaml, generating operations stub on the fly etc), and we may come back to it if even needed (most probably never).