-
Notifications
You must be signed in to change notification settings - Fork 68
fix: [gapic-generator-java] handle response and metadata type ambiguity in LRO parsing #1726
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
Changes from 11 commits
8096dbd
c5e0b30
ca740ba
7ce4084
76d8fba
2e1c69e
e4c4916
7c759a9
67fecec
30d8a0d
b73d72a
de5e9c3
b7952a4
69e3a27
1ff61f4
3703269
06dbf1f
ba60786
ba1d1af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -824,39 +824,34 @@ static LongrunningOperation parseLro( | |||||||||||||||||||||||
| boolean isResponseTypeNameShortOnly = lastDotIndex < 0; | ||||||||||||||||||||||||
| String responseTypeShortName = | ||||||||||||||||||||||||
| lastDotIndex >= 0 ? responseTypeName.substring(lastDotIndex + 1) : responseTypeName; | ||||||||||||||||||||||||
| // When only shortname is provided, match on same proto package as method (See | ||||||||||||||||||||||||
| // https://aip.dev/151) | ||||||||||||||||||||||||
| String responseTypeFullName = | ||||||||||||||||||||||||
| isResponseTypeNameShortOnly | ||||||||||||||||||||||||
| ? methodDescriptor.getFile().getPackage() + "." + responseTypeShortName | ||||||||||||||||||||||||
| : responseTypeName; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| lastDotIndex = metadataTypeName.lastIndexOf('.'); | ||||||||||||||||||||||||
| boolean isMetadataTypeNameShortOnly = lastDotIndex < 0; | ||||||||||||||||||||||||
| String metadataTypeShortName = | ||||||||||||||||||||||||
| lastDotIndex >= 0 ? metadataTypeName.substring(lastDotIndex + 1) : metadataTypeName; | ||||||||||||||||||||||||
| // When only shortname is provided, match on same proto package as method (See | ||||||||||||||||||||||||
| // https://aip.dev/151) | ||||||||||||||||||||||||
| String metadataTypeFullName = | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| Preconditions.checkNotNull( | |
| responseMessage, | |
| String.format( | |
| "LRO response message %s not found on method %s", | |
| responseTypeName, methodDescriptor.getName())); | |
| // TODO(miraleung): Check that the packages are equal if those strings are not empty. | |
| Preconditions.checkNotNull( | |
| metadataMessage, | |
| String.format( | |
| "LRO metadata message %s not found in method %s", | |
| metadataTypeName, methodDescriptor.getName())); |
But this behavior should be tested (through true unit tests rather than goldens). Will look into replacing showcase-extended setup in this PR with true unit testing for Parser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted changes to showcase-extended setup, and added unit tests separately for respective Parser and Writer fixes (more in PR description).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright 2023 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| syntax = "proto3"; | ||
|
|
||
| import "google/api/client.proto"; | ||
| import "google/longrunning/operations.proto"; | ||
|
|
||
| package google.showcase.v1beta1; | ||
|
|
||
| option java_package = "com.google.showcase.v1beta1"; | ||
| option java_multiple_files = true; | ||
|
|
||
| // This service exercises scenarios where short names of types | ||
| // exhibit ambiguity or collide with other types | ||
| service Collisions { | ||
|
|
||
| option (google.api.default_host) = "localhost:7469"; | ||
|
|
||
| rpc doSomething(Request) returns (google.longrunning.Operation) { | ||
| option (google.longrunning.operation_info) = { | ||
| response_type: "Annotation" | ||
| metadata_type: "Location" | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| message Request { | ||
| string name = 1; | ||
| Annotation annotation = 2; | ||
| Location location = 3; | ||
| } | ||
|
|
||
| message Annotation { | ||
| string name = 1; | ||
| } | ||
|
|
||
| message Location { | ||
| string name = 1; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /* | ||
| * Copyright 2023 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.showcase.v1beta1.samples; | ||
|
|
||
| // [START localhost7469_v1beta1_generated_Collisions_Create_SetCredentialsProvider_sync] | ||
| import com.google.api.gax.core.FixedCredentialsProvider; | ||
| import com.google.showcase.v1beta1.CollisionsClient; | ||
| import com.google.showcase.v1beta1.CollisionsSettings; | ||
| import com.google.showcase.v1beta1.myCredentials; | ||
|
|
||
| public class SyncCreateSetCredentialsProvider { | ||
|
|
||
| public static void main(String[] args) throws Exception { | ||
| syncCreateSetCredentialsProvider(); | ||
| } | ||
|
|
||
| public static void syncCreateSetCredentialsProvider() throws Exception { | ||
| // This snippet has been automatically generated and should be regarded as a code template only. | ||
| // It will require modifications to work: | ||
| // - It may require correct/in-range values for request initialization. | ||
| // - It may require specifying regional endpoints when creating the service client as shown in | ||
| // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library | ||
| CollisionsSettings collisionsSettings = | ||
| CollisionsSettings.newBuilder() | ||
| .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials)) | ||
| .build(); | ||
| CollisionsClient collisionsClient = CollisionsClient.create(collisionsSettings); | ||
| } | ||
| } | ||
| // [END localhost7469_v1beta1_generated_Collisions_Create_SetCredentialsProvider_sync] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /* | ||
| * Copyright 2023 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.showcase.v1beta1.samples; | ||
|
|
||
| // [START localhost7469_v1beta1_generated_Collisions_Create_SetCredentialsProvider1_sync] | ||
| import com.google.showcase.v1beta1.CollisionsClient; | ||
| import com.google.showcase.v1beta1.CollisionsSettings; | ||
|
|
||
| public class SyncCreateSetCredentialsProvider1 { | ||
|
|
||
| public static void main(String[] args) throws Exception { | ||
| syncCreateSetCredentialsProvider1(); | ||
| } | ||
|
|
||
| public static void syncCreateSetCredentialsProvider1() throws Exception { | ||
| // This snippet has been automatically generated and should be regarded as a code template only. | ||
| // It will require modifications to work: | ||
| // - It may require correct/in-range values for request initialization. | ||
| // - It may require specifying regional endpoints when creating the service client as shown in | ||
| // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library | ||
| CollisionsSettings collisionsSettings = CollisionsSettings.newHttpJsonBuilder().build(); | ||
| CollisionsClient collisionsClient = CollisionsClient.create(collisionsSettings); | ||
| } | ||
| } | ||
| // [END localhost7469_v1beta1_generated_Collisions_Create_SetCredentialsProvider1_sync] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /* | ||
| * Copyright 2023 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.showcase.v1beta1.samples; | ||
|
|
||
| // [START localhost7469_v1beta1_generated_Collisions_Create_SetEndpoint_sync] | ||
| import com.google.showcase.v1beta1.CollisionsClient; | ||
| import com.google.showcase.v1beta1.CollisionsSettings; | ||
| import com.google.showcase.v1beta1.myEndpoint; | ||
|
|
||
| public class SyncCreateSetEndpoint { | ||
|
|
||
| public static void main(String[] args) throws Exception { | ||
| syncCreateSetEndpoint(); | ||
| } | ||
|
|
||
| public static void syncCreateSetEndpoint() throws Exception { | ||
| // This snippet has been automatically generated and should be regarded as a code template only. | ||
| // It will require modifications to work: | ||
| // - It may require correct/in-range values for request initialization. | ||
| // - It may require specifying regional endpoints when creating the service client as shown in | ||
| // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library | ||
| CollisionsSettings collisionsSettings = | ||
| CollisionsSettings.newBuilder().setEndpoint(myEndpoint).build(); | ||
| CollisionsClient collisionsClient = CollisionsClient.create(collisionsSettings); | ||
| } | ||
| } | ||
| // [END localhost7469_v1beta1_generated_Collisions_Create_SetEndpoint_sync] |
Uh oh!
There was an error while loading. Please reload this page.