From 55317956397370a91b1a06ecd476e55f58789807 Mon Sep 17 00:00:00 2001
From: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
Date: Thu, 18 Nov 2021 11:40:14 -0700
Subject: [PATCH] fix(snippetgen): don't create duplicate requests for required
 oneofs (#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.
---
 gapic/samplegen/samplegen.py                  | 12 +++--
 ...pets_one_of_method_required_field_async.py | 48 +++++++++++++++++++
 ...ppets_one_of_method_required_field_sync.py | 48 +++++++++++++++++++
 tests/snippetgen/snippets.proto               | 13 +++++
 4 files changed, 118 insertions(+), 3 deletions(-)
 create mode 100644 tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_async.py
 create mode 100644 tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_sync.py

diff --git a/gapic/samplegen/samplegen.py b/gapic/samplegen/samplegen.py
index ab1b817300..35667f5af6 100644
--- a/gapic/samplegen/samplegen.py
+++ b/gapic/samplegen/samplegen.py
@@ -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
@@ -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
diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_async.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_async.py
new file mode 100644
index 0000000000..4f074994b7
--- /dev/null
+++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_async.py
@@ -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]
diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_sync.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_sync.py
new file mode 100644
index 0000000000..6e480d5000
--- /dev/null
+++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_one_of_method_required_field_sync.py
@@ -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]
diff --git a/tests/snippetgen/snippets.proto b/tests/snippetgen/snippets.proto
index fbffcea258..d5ead4ed33 100644
--- a/tests/snippetgen/snippets.proto
+++ b/tests/snippetgen/snippets.proto
@@ -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 {
@@ -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];
+  }
+}
\ No newline at end of file