feat: [REGAPIC] Add support for additional bindings #1680
Conversation
Additional bindings are currently relevant only for generated unit tests
gax-httpjson/src/main/java/com/google/api/gax/httpjson/ProtoMessageRequestFormatter.java
Outdated
Show resolved
Hide resolved
| @@ -136,7 +156,8 @@ public ProtoMessageRequestFormatter<RequestT> build() { | |||
| queryParamsExtractor, | |||
| rawPath, | |||
| PathTemplate.create(rawPath), | |||
| pathVarsExtractor); | |||
| pathVarsExtractor, | |||
| rawAdditionalPaths.stream().map(PathTemplate::create).collect(Collectors.toList())); | |||
There was a problem hiding this comment.
Can we make it an ImmutableList? Collectors.toList() is not guaranteed to be immutable.
There was a problem hiding this comment.
ImmutableList collectors are not part of a more lightweight versions of guava (androind one does not have them), so to keep better portability I believe it is better to stick to standard library collectors.
If they are only for generated unit tests, do we really need this change? Can you provide an example usage of this additional bindings? |
This PR depends on the corresponding one in gax: googleapis/gax-java#1680
|
@blakeli0 It will affect execution of the generated tests, but not their code directly. Currently additional bindings are validated only as part of |
|
@blakeli0 PTAL |
| /* {@inheritDoc} */ | ||
| @Override | ||
| public PathTemplate getPathTemplate() { | ||
| return pathTemplate; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Do we have to override hashcode() and equals() methods for this class? I think if you don't compare them or put them in a collection, then you don't have to.
There was a problem hiding this comment.
There is toBuilder() semantics in this class, so I added equals()/hashCode() to do testing of that logic, but seems like adding equals/hashcode added it own problems of branch coverage. I guess I'll just remove those as they cause more trouble than they should.
Does it mean that these new fields are populated but not really used by any features yet? Do we populate them just in case or there are features plan to use them? |
|
@blakeli0 they are used in MockService validation logic https://github.com/googleapis/gax-java/pull/1680/files#diff-4910ec43f3337af2793083d9c8c1f0864769deac8c2d7b9b87778e47a5504471R215. MockService needs additional bindings, because they are essential to match a |
|
Kudos, SonarCloud Quality Gate passed! |
This PR depends on the corresponding one in gax: googleapis/gax-java#1680
This PR depends on the corresponding one in gax: googleapis/gax-java#1680
Additional bindings are currently relevant only for generated unit tests
[edit] Also update IAM dependency in dependencies.properties (it is used in generated build gralde files for self-service)