Skip to content

Commit

Permalink
fix(snippetgen): don't create duplicate requests for required oneofs (#…
Browse files Browse the repository at this point in the history
…1088)

Some APIs mark every field in a oneof as "required" to express that the oneof itself is required.

https://github.com/googleapis/googleapis/blob/55726d62556966c095a096aed1ffda5da231f36d/google/cloud/channel/v1/service.proto#L1057-L1069

```proto
// Request message for [CloudChannelService.ImportCustomer][google.cloud.channel.v1.CloudChannelService.ImportCustomer]
message ImportCustomerRequest {
  // Specifies the identity of the transfer customer.
  // A customer's cloud_identity_id or domain is required to look up the
  // customer's Cloud Identity. For Team customers, only the cloud_identity_id
  // option is valid.
  oneof customer_identity {
    // Required. Customer domain.
    string domain = 2 [(google.api.field_behavior) = REQUIRED];

    // Required. Customer's Cloud Identity ID
    string cloud_identity_id = 3 [(google.api.field_behavior) = REQUIRED];
  }
...

``` 

This causes an error in the current logic since two requests are generated for the same field.
  • Loading branch information
busunkim96 authored Nov 18, 2021
1 parent 309cc66 commit 5531795
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 3 deletions.
12 changes: 9 additions & 3 deletions gapic/samplegen/samplegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from gapic.schema import wrappers

from collections import defaultdict, namedtuple, ChainMap as chainmap
from typing import Any, ChainMap, Dict, FrozenSet, Generator, List, Mapping, Optional, Tuple, Sequence
from typing import Any, ChainMap, Dict, FrozenSet, Generator, List, Mapping, Optional, Sequence

# There is no library stub file for this module, so ignore it.
from google.api import resource_pb2 # type: ignore
Expand Down Expand Up @@ -981,10 +981,16 @@ def generate_request_object(api_schema: api.API, service: wrappers.Service, mess

request_fields: List[wrappers.Field] = []

# Choose the first option for each oneof
# There is no standard syntax to mark a oneof as "required" in protos.
# Assume every oneof is required and pick the first option
# in each oneof.
selected_oneofs: List[wrappers.Field] = [oneof_fields[0]
for oneof_fields in message.oneof_fields().values()]
request_fields = selected_oneofs + message.required_fields

# Don't add required fields if they're also marked as oneof
required_fields = [
field for field in message.required_fields if not field.oneof]
request_fields = selected_oneofs + required_fields

for field in request_fields:
# TransformedRequest expects nested fields to be referenced like
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# -*- coding: utf-8 -*-
# Copyright 2020 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
#
# http://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.
#
# Generated code. DO NOT EDIT!
#
# Snippet for OneOfMethodRequiredField
# NOTE: This snippet has been automatically generated for illustrative purposes only.
# It may require modifications to work in your environment.

# To install the latest published package dependency, execute the following:
# python3 -m pip install animalia-mollusca


# [START mollusca_generated_mollusca_v1_Snippets_OneOfMethodRequiredField_async]
from animalia import mollusca_v1


async def sample_one_of_method_required_field():
"""Snippet for one_of_method_required_field"""

# Create a client
client = mollusca_v1.SnippetsAsyncClient()

# Initialize request argument(s)
request = mollusca_v1.OneOfRequestWithRequiredField(
my_string="my_string_value",
non_one_of_string="non_one_of_string_value",
)

# Make the request
response = await client.one_of_method_required_field(request=request)

# Handle response
print(response)

# [END mollusca_generated_mollusca_v1_Snippets_OneOfMethodRequiredField_async]
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# -*- coding: utf-8 -*-
# Copyright 2020 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
#
# http://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.
#
# Generated code. DO NOT EDIT!
#
# Snippet for OneOfMethodRequiredField
# NOTE: This snippet has been automatically generated for illustrative purposes only.
# It may require modifications to work in your environment.

# To install the latest published package dependency, execute the following:
# python3 -m pip install animalia-mollusca


# [START mollusca_generated_mollusca_v1_Snippets_OneOfMethodRequiredField_sync]
from animalia import mollusca_v1


def sample_one_of_method_required_field():
"""Snippet for one_of_method_required_field"""

# Create a client
client = mollusca_v1.SnippetsClient()

# Initialize request argument(s)
request = mollusca_v1.OneOfRequestWithRequiredField(
my_string="my_string_value",
non_one_of_string="non_one_of_string_value",
)

# Make the request
response = client.one_of_method_required_field(request=request)

# Handle response
print(response)

# [END mollusca_generated_mollusca_v1_Snippets_OneOfMethodRequiredField_sync]
13 changes: 13 additions & 0 deletions tests/snippetgen/snippets.proto
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ service Snippets {
rpc MethodBidiStreaming(stream SignatureRequestOneRequiredField) returns (stream Response);

rpc OneOfMethod(OneOfRequest) returns (Response);

rpc OneOfMethodRequiredField(OneOfRequestWithRequiredField) returns (Response);
}

enum Enum {
Expand Down Expand Up @@ -149,3 +151,14 @@ message OneOfRequest {
int32 my_number = 3;
}
}


message OneOfRequestWithRequiredField {
string non_one_of_string = 1 [(google.api.field_behavior) = REQUIRED];

// Some APIs mark every field in a "required" oneof as required
oneof my_one_of {
string my_string = 2 [(google.api.field_behavior) = REQUIRED];
int32 my_number = 3 [(google.api.field_behavior) = REQUIRED];
}
}

0 comments on commit 5531795

Please sign in to comment.