From 42648dcb5fd2d15ba25cc1c694b4522dddcb9361 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Wed, 17 Jul 2024 18:18:22 +0000
Subject: [PATCH 1/9] feat: allow custom `transport` in config yaml

---
 library_generation/README.md                  | 48 ++++++++++---------
 .../generate_composed_library.py              |  7 ++-
 library_generation/model/library_config.py    |  4 ++
 library_generation/model/repo_config.py       |  6 +++
 ...repo-metadata-custom-transport-golden.json | 16 +++++++
 .../test/utilities_unit_tests.py              | 24 +++++++++-
 library_generation/utils/utilities.py         |  8 ++--
 7 files changed, 83 insertions(+), 30 deletions(-)
 create mode 100644 library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json

diff --git a/library_generation/README.md b/library_generation/README.md
index 7ffe6a4a57..aa272ffa06 100644
--- a/library_generation/README.md
+++ b/library_generation/README.md
@@ -105,29 +105,31 @@ The library level parameters define how to generate a (multi-versions) GAPIC
 library.
 They are shared by all GAPICs of a library.
 
-| Name                  | Required | Notes                                                                              |
-|:----------------------|:--------:|:-----------------------------------------------------------------------------------|
-| api_shortname         |   Yes    |                                                                                    |
-| api_description       |   Yes    |                                                                                    |
-| name_pretty           |   Yes    |                                                                                    |
-| product_docs          |   Yes    |                                                                                    |
-| library_type          |    No    | `GAPIC_AUTO` if not specified                                                      |
-| release_level         |    No    | `preview` if not specified                                                         |
-| api_id                |    No    | `{api_shortname}.googleapis.com` if not specified                                  |
-| api_reference         |    No    |                                                                                    |
-| codeowner_team        |    No    |                                                                                    |
-| client_documentation  |    No    |                                                                                    |
-| distribution_name     |    No    | `{group_id}:google-{cloud_prefix}{library_name}` if not specified                  |
-| excluded_poms         |    No    |                                                                                    |
-| excluded_dependencies |    No    |                                                                                    |
-| googleapis_commitish  |    No    | use repository level `googleapis_commitish` if not specified.                      |
-| group_id              |    No    | `com.google.cloud` if not specified                                                |
-| issue_tracker         |    No    |                                                                                    |
-| library_name          |    No    | `api_shortname` is not specified. This value should be unique among all libraries. |
-| rest_documentation    |    No    |                                                                                    |
-| rpc_documentation     |    No    |                                                                                    |
-| cloud_api             |    No    | `true` if not specified                                                            |
-| requires-billing      |    No    | `true` if not specified                                                            |
+| Name                  | Required | Notes                                                                                                           |
+|:----------------------|:--------:|:----------------------------------------------------------------------------------------------------------------|
+| api_shortname         |   Yes    |                                                                                                                 |
+| api_description       |   Yes    |                                                                                                                 |
+| name_pretty           |   Yes    |                                                                                                                 |
+| product_docs          |   Yes    |                                                                                                                 |
+| library_type          |    No    | `GAPIC_AUTO` if not specified                                                                                   |
+| release_level         |    No    | `preview` if not specified                                                                                      |
+| api_id                |    No    | `{api_shortname}.googleapis.com` if not specified                                                               |
+| api_reference         |    No    |                                                                                                                 |
+| codeowner_team        |    No    |                                                                                                                 |
+| client_documentation  |    No    |                                                                                                                 |
+| distribution_name     |    No    | `{group_id}:google-{cloud_prefix}{library_name}` if not specified                                               |
+| excluded_poms         |    No    |                                                                                                                 |
+| excluded_dependencies |    No    |                                                                                                                 |
+| googleapis_commitish  |    No    | use repository level `googleapis_commitish` if not specified.                                                   |
+| group_id              |    No    | `com.google.cloud` if not specified                                                                             |
+| issue_tracker         |    No    |                                                                                                                 |
+| library_name          |    No    | `api_shortname` is not specified. This value should be unique among all libraries.                              |
+| rest_documentation    |    No    |                                                                                                                 |
+| rpc_documentation     |    No    |                                                                                                                 |
+| cloud_api             |    No    | `true` if not specified                                                                                         |
+| requires-billing      |    No    | `true` if not specified                                                                                         |
+| transport             |    No    | must be one of `grpc`, `rest`, `http` (alias) or `both`. This will override the value obtained from BUILD.bazel |
+
 
 Note that `cloud_prefix` is `cloud-` if `cloud_api` is `true`; empty otherwise.
 
diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py
index dc094c0b11..36310fb996 100755
--- a/library_generation/generate_composed_library.py
+++ b/library_generation/generate_composed_library.py
@@ -72,13 +72,16 @@ def generate_composed_library(
         gapic_inputs = parse_build_file(build_file_folder, gapic.proto_path)
         # generate postprocessing prerequisite files (.repo-metadata.json, .OwlBot-hermetic.yaml,
         # owlbot.py) here because transport is parsed from BUILD.bazel,
-        # which lives in a versioned proto_path.
+        # which lives in a versioned proto_path. The value of transport will be
+        # overriden by the config object if specified. Note that this override
+        # does not affect library generation but instead used only for
+        # generating postprocessing files such as README.
         util.generate_postprocessing_prerequisite_files(
             config=config,
             library=library,
             proto_path=util.remove_version_from(gapic.proto_path),
-            transport=gapic_inputs.transport,
             library_path=library_path,
+            gapic_inputs = gapic_inputs,
         )
         temp_destination_path = f"java-{gapic.proto_path.replace('/','-')}"
         effective_arguments = __construct_effective_arg(
diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py
index 9855e4e834..be5a4ee0ae 100644
--- a/library_generation/model/library_config.py
+++ b/library_generation/model/library_config.py
@@ -52,6 +52,7 @@ def __init__(
         extra_versioned_modules: Optional[str] = None,
         recommended_package: Optional[str] = None,
         min_java_version: Optional[int] = None,
+        transport: Optional[str] = None,
     ):
         self.api_shortname = api_shortname
         self.api_description = api_description
@@ -78,6 +79,7 @@ def __init__(
         self.recommended_package = recommended_package
         self.min_java_version = min_java_version
         self.distribution_name = self.__get_distribution_name(distribution_name)
+        self.transport = transport
 
     def get_library_name(self) -> str:
         """
@@ -144,6 +146,7 @@ def __eq__(self, other):
             and self.extra_versioned_modules == other.extra_versioned_modules
             and self.recommended_package == other.recommended_package
             and self.min_java_version == other.min_java_version
+            and self.transport == other.transport
         )
 
     def __hash__(self):
@@ -175,6 +178,7 @@ def __hash__(self):
                     self.extra_versioned_modules,
                     self.recommended_package,
                     self.min_java_version,
+                    self.transport,
                 ]
                 + [config.proto_path for config in self.gapic_configs]
             ).encode("utf-8")
diff --git a/library_generation/model/repo_config.py b/library_generation/model/repo_config.py
index 520c505823..58ed3fa3bf 100644
--- a/library_generation/model/repo_config.py
+++ b/library_generation/model/repo_config.py
@@ -57,6 +57,12 @@ def get_library_version(self, artifact_id: str) -> str:
         return self.library_versions.get(artifact_id, NEW_CLIENT_VERSION)
 
     def __parse_versions(self) -> dict[str, str]:
+        """
+        For a given versions.txt file (defined in self.versions_file)
+        creates a map of artifact-id to its version
+
+        :return: a map "artifact-id -> version"
+        """
         library_versions = dict()
         with open(self.versions_file) as f:
             for line in f.readlines():
diff --git a/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json b/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json
new file mode 100644
index 0000000000..2cccd4b889
--- /dev/null
+++ b/library_generation/test/resources/goldens/.repo-metadata-custom-transport-golden.json
@@ -0,0 +1,16 @@
+{
+  "api_shortname": "secretmanager",
+  "name_pretty": "Secret Management",
+  "product_documentation": "https://cloud.google.com/solutions/secrets-management/",
+  "api_description": "allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
+  "client_documentation": "https://cloud.google.com/java/docs/reference/google-cloud-secretmanager/latest/overview",
+  "release_level": "preview",
+  "transport": "http",
+  "language": "java",
+  "repo": "googleapis/google-cloud-java",
+  "repo_short": "java-secretmanager",
+  "distribution_name": "com.google.cloud:google-cloud-secretmanager",
+  "api_id": "secretmanager.googleapis.com",
+  "library_type": "GAPIC_AUTO",
+  "requires_billing": true
+}
\ No newline at end of file
diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py
index c38016fcba..6c901f82a3 100644
--- a/library_generation/test/utilities_unit_tests.py
+++ b/library_generation/test/utilities_unit_tests.py
@@ -23,6 +23,7 @@
 from pathlib import Path
 from library_generation.utils import utilities as util
 from library_generation.model.gapic_config import GapicConfig
+from library_generation.model.gapic_inputs import GapicInputs
 from library_generation.model.generation_config import GenerationConfig
 from library_generation.model.library_config import LibraryConfig
 from library_generation.test.test_utils import FileComparator
@@ -57,6 +58,14 @@
     api_description="example description",
     gapic_configs=list(),
 )
+test_library_with_custom_transport = LibraryConfig(
+  api_shortname="secretmanager",
+  name_pretty="Secret Management",
+  product_documentation="https://cloud.google.com/solutions/secrets-management/",
+  api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
+  gapic_configs=list(),
+  transport="http",
+)
 
 
 class UtilitiesTest(unittest.TestCase):
@@ -225,6 +234,18 @@ def test_generate_postprocessing_prerequisite_files_proto_only_repo_success(self
         )
         self.__remove_postprocessing_prerequisite_files(path=library_path)
 
+    def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_config__success(self):
+        library_path = self.__setup_postprocessing_prerequisite_files(combination=2, library=test_library_with_custom_transport)
+
+        with open(f"{library_path}/.repo-metadata.json", 'r') as f:
+          print(f.read())
+
+        file_comparator.compare_files(
+            f"{library_path}/.repo-metadata.json",
+            f"{library_path}/.repo-metadata-custom-transport-golden.json",
+        )
+        self.__remove_postprocessing_prerequisite_files(path=library_path)
+
     def test_prepare_repo_monorepo_success(self):
         gen_config = self.__get_a_gen_config(2)
         repo_config = util.prepare_repo(
@@ -276,12 +297,11 @@ def __setup_postprocessing_prerequisite_files(
         library.library_type = library_type
         config = self.__get_a_gen_config(combination, library_type=library_type)
         proto_path = "google/cloud/baremetalsolution/v2"
-        transport = "grpc"
         util.generate_postprocessing_prerequisite_files(
             config=config,
             library=library,
             proto_path=proto_path,
-            transport=transport,
+            gapic_inputs=GapicInputs(), # defaults to transport=grpc
             library_path=library_path,
         )
         return library_path
diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py
index 59f238aaea..329be7d159 100755
--- a/library_generation/utils/utilities.py
+++ b/library_generation/utils/utilities.py
@@ -18,6 +18,7 @@
 import shutil
 from pathlib import Path
 from library_generation.model.generation_config import GenerationConfig
+from library_generation.model.gapic_inputs import GapicInputs
 from library_generation.model.library_config import LibraryConfig
 from typing import List
 from library_generation.model.repo_config import RepoConfig
@@ -186,7 +187,7 @@ def generate_postprocessing_prerequisite_files(
     config: GenerationConfig,
     library: LibraryConfig,
     proto_path: str,
-    transport: str,
+    gapic_inputs: GapicInputs,
     library_path: str,
     language: str = "java",
 ) -> None:
@@ -198,11 +199,12 @@ def generate_postprocessing_prerequisite_files(
     :param library: the library configuration
     :param proto_path: the path from the root of googleapis to the location of the service
     protos. If the path contains a version, it will be removed
-    :param transport: transport supported by the library
+    :param gapic_inputs: gapic_inputs obtained from the library's BUILD.bazel
     :param library_path: the path to which the generated file goes
     :param language: programming language of the library
     :return: None
     """
+    transport = gapic_inputs.transport if library.transport is None else library.transport
     library_name = library.get_library_name()
     artifact_id = library.get_artifact_id()
     if config.contains_common_protos():
@@ -224,7 +226,7 @@ def generate_postprocessing_prerequisite_files(
     # is one of grpc, http and both,
     if transport == "grpc":
         converted_transport = "grpc"
-    elif transport == "rest":
+    elif transport in ["rest", "http"]: # http can also be specified via generation_config.yaml
         converted_transport = "http"
     else:
         converted_transport = "both"

From 6e2686cffe112564f041b06bf221fc8b41682e24 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Wed, 17 Jul 2024 18:19:35 +0000
Subject: [PATCH 2/9] format

---
 .../generate_composed_library.py              |  2 +-
 .../test/utilities_unit_tests.py              | 26 +++++++++++--------
 library_generation/utils/utilities.py         |  9 +++++--
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py
index 36310fb996..a630bd3818 100755
--- a/library_generation/generate_composed_library.py
+++ b/library_generation/generate_composed_library.py
@@ -81,7 +81,7 @@ def generate_composed_library(
             library=library,
             proto_path=util.remove_version_from(gapic.proto_path),
             library_path=library_path,
-            gapic_inputs = gapic_inputs,
+            gapic_inputs=gapic_inputs,
         )
         temp_destination_path = f"java-{gapic.proto_path.replace('/','-')}"
         effective_arguments = __construct_effective_arg(
diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py
index 6c901f82a3..32ea908323 100644
--- a/library_generation/test/utilities_unit_tests.py
+++ b/library_generation/test/utilities_unit_tests.py
@@ -59,12 +59,12 @@
     gapic_configs=list(),
 )
 test_library_with_custom_transport = LibraryConfig(
-  api_shortname="secretmanager",
-  name_pretty="Secret Management",
-  product_documentation="https://cloud.google.com/solutions/secrets-management/",
-  api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
-  gapic_configs=list(),
-  transport="http",
+    api_shortname="secretmanager",
+    name_pretty="Secret Management",
+    product_documentation="https://cloud.google.com/solutions/secrets-management/",
+    api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
+    gapic_configs=list(),
+    transport="http",
 )
 
 
@@ -234,11 +234,15 @@ def test_generate_postprocessing_prerequisite_files_proto_only_repo_success(self
         )
         self.__remove_postprocessing_prerequisite_files(path=library_path)
 
-    def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_config__success(self):
-        library_path = self.__setup_postprocessing_prerequisite_files(combination=2, library=test_library_with_custom_transport)
+    def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_config__success(
+        self,
+    ):
+        library_path = self.__setup_postprocessing_prerequisite_files(
+            combination=2, library=test_library_with_custom_transport
+        )
 
-        with open(f"{library_path}/.repo-metadata.json", 'r') as f:
-          print(f.read())
+        with open(f"{library_path}/.repo-metadata.json", "r") as f:
+            print(f.read())
 
         file_comparator.compare_files(
             f"{library_path}/.repo-metadata.json",
@@ -301,7 +305,7 @@ def __setup_postprocessing_prerequisite_files(
             config=config,
             library=library,
             proto_path=proto_path,
-            gapic_inputs=GapicInputs(), # defaults to transport=grpc
+            gapic_inputs=GapicInputs(),  # defaults to transport=grpc
             library_path=library_path,
         )
         return library_path
diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py
index 329be7d159..b0dd336274 100755
--- a/library_generation/utils/utilities.py
+++ b/library_generation/utils/utilities.py
@@ -204,7 +204,9 @@ def generate_postprocessing_prerequisite_files(
     :param language: programming language of the library
     :return: None
     """
-    transport = gapic_inputs.transport if library.transport is None else library.transport
+    transport = (
+        gapic_inputs.transport if library.transport is None else library.transport
+    )
     library_name = library.get_library_name()
     artifact_id = library.get_artifact_id()
     if config.contains_common_protos():
@@ -226,7 +228,10 @@ def generate_postprocessing_prerequisite_files(
     # is one of grpc, http and both,
     if transport == "grpc":
         converted_transport = "grpc"
-    elif transport in ["rest", "http"]: # http can also be specified via generation_config.yaml
+    elif transport in [
+        "rest",
+        "http",
+    ]:  # http can also be specified via generation_config.yaml
         converted_transport = "http"
     else:
         converted_transport = "both"

From faf5830a9e92c66f158fe227874d192372806380 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Wed, 17 Jul 2024 18:22:48 +0000
Subject: [PATCH 3/9] add comment to test

---
 library_generation/test/utilities_unit_tests.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py
index 32ea908323..f007e66bad 100644
--- a/library_generation/test/utilities_unit_tests.py
+++ b/library_generation/test/utilities_unit_tests.py
@@ -237,13 +237,17 @@ def test_generate_postprocessing_prerequisite_files_proto_only_repo_success(self
     def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_config__success(
         self,
     ):
+        """
+        This test generates files for `test_library_with_custom_transport`, which
+        has an explicit value for transport declared (http). This is expected to
+        override the value obtained in BUILD.bazel via gapic_inputs.parse(). For
+        testing purposes, we test with a default GapicInputs object, whose transport
+        is set to "grpc".
+        """
         library_path = self.__setup_postprocessing_prerequisite_files(
             combination=2, library=test_library_with_custom_transport
         )
 
-        with open(f"{library_path}/.repo-metadata.json", "r") as f:
-            print(f.read())
-
         file_comparator.compare_files(
             f"{library_path}/.repo-metadata.json",
             f"{library_path}/.repo-metadata-custom-transport-golden.json",

From be429125d20c4ba946d8cf4b95d171a4bf029efe Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Thu, 18 Jul 2024 15:07:06 +0000
Subject: [PATCH 4/9] clarify effects of transport option in yaml

---
 library_generation/README.md | 48 ++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/library_generation/README.md b/library_generation/README.md
index aa272ffa06..8bc47bad73 100644
--- a/library_generation/README.md
+++ b/library_generation/README.md
@@ -105,30 +105,30 @@ The library level parameters define how to generate a (multi-versions) GAPIC
 library.
 They are shared by all GAPICs of a library.
 
-| Name                  | Required | Notes                                                                                                           |
-|:----------------------|:--------:|:----------------------------------------------------------------------------------------------------------------|
-| api_shortname         |   Yes    |                                                                                                                 |
-| api_description       |   Yes    |                                                                                                                 |
-| name_pretty           |   Yes    |                                                                                                                 |
-| product_docs          |   Yes    |                                                                                                                 |
-| library_type          |    No    | `GAPIC_AUTO` if not specified                                                                                   |
-| release_level         |    No    | `preview` if not specified                                                                                      |
-| api_id                |    No    | `{api_shortname}.googleapis.com` if not specified                                                               |
-| api_reference         |    No    |                                                                                                                 |
-| codeowner_team        |    No    |                                                                                                                 |
-| client_documentation  |    No    |                                                                                                                 |
-| distribution_name     |    No    | `{group_id}:google-{cloud_prefix}{library_name}` if not specified                                               |
-| excluded_poms         |    No    |                                                                                                                 |
-| excluded_dependencies |    No    |                                                                                                                 |
-| googleapis_commitish  |    No    | use repository level `googleapis_commitish` if not specified.                                                   |
-| group_id              |    No    | `com.google.cloud` if not specified                                                                             |
-| issue_tracker         |    No    |                                                                                                                 |
-| library_name          |    No    | `api_shortname` is not specified. This value should be unique among all libraries.                              |
-| rest_documentation    |    No    |                                                                                                                 |
-| rpc_documentation     |    No    |                                                                                                                 |
-| cloud_api             |    No    | `true` if not specified                                                                                         |
-| requires-billing      |    No    | `true` if not specified                                                                                         |
-| transport             |    No    | must be one of `grpc`, `rest`, `http` (alias) or `both`. This will override the value obtained from BUILD.bazel |
+| Name                  | Required | Notes                                                                                                                                     |
+|:----------------------|:--------:|:------------------------------------------------------------------------------------------------------------------------------------------|
+| api_shortname         |   Yes    |                                                                                                                                           |
+| api_description       |   Yes    |                                                                                                                                           |
+| name_pretty           |   Yes    |                                                                                                                                           |
+| product_docs          |   Yes    |                                                                                                                                           |
+| library_type          |    No    | `GAPIC_AUTO` if not specified                                                                                                             |
+| release_level         |    No    | `preview` if not specified                                                                                                                |
+| api_id                |    No    | `{api_shortname}.googleapis.com` if not specified                                                                                         |
+| api_reference         |    No    |                                                                                                                                           |
+| codeowner_team        |    No    |                                                                                                                                           |
+| client_documentation  |    No    |                                                                                                                                           |
+| distribution_name     |    No    | `{group_id}:google-{cloud_prefix}{library_name}` if not specified                                                                         |
+| excluded_poms         |    No    |                                                                                                                                           |
+| excluded_dependencies |    No    |                                                                                                                                           |
+| googleapis_commitish  |    No    | use repository level `googleapis_commitish` if not specified.                                                                             |
+| group_id              |    No    | `com.google.cloud` if not specified                                                                                                       |
+| issue_tracker         |    No    |                                                                                                                                           |
+| library_name          |    No    | `api_shortname` is not specified. This value should be unique among all libraries.                                                        |
+| rest_documentation    |    No    |                                                                                                                                           |
+| rpc_documentation     |    No    |                                                                                                                                           |
+| cloud_api             |    No    | `true` if not specified                                                                                                                   |
+| requires-billing      |    No    | `true` if not specified                                                                                                                   |
+| transport             |    No    | must be one of `grpc`, `rest` or `both`. This value would only be used for generating .repo-metadata.json and relevant sections in README |
 
 
 Note that `cloud_prefix` is `cloud-` if `cloud_api` is `true`; empty otherwise.

From 17008425c05dd5d77f67757d8158f65869decfd0 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Thu, 18 Jul 2024 15:07:35 +0000
Subject: [PATCH 5/9] only allow "rest" as an option to produce "http" metadata

---
 library_generation/utils/utilities.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py
index b0dd336274..84e54e39b0 100755
--- a/library_generation/utils/utilities.py
+++ b/library_generation/utils/utilities.py
@@ -228,10 +228,7 @@ def generate_postprocessing_prerequisite_files(
     # is one of grpc, http and both,
     if transport == "grpc":
         converted_transport = "grpc"
-    elif transport in [
-        "rest",
-        "http",
-    ]:  # http can also be specified via generation_config.yaml
+    elif transport == "rest":
         converted_transport = "http"
     else:
         converted_transport = "both"

From 3bc942545b19a8e55704203d00957b4323d85da4 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Mon, 22 Jul 2024 19:25:36 +0000
Subject: [PATCH 6/9] move transport inference to model class

---
 library_generation/generate_composed_library.py | 2 +-
 library_generation/model/library_config.py      | 8 ++++++++
 library_generation/test/utilities_unit_tests.py | 6 ++++--
 library_generation/utils/utilities.py           | 8 ++------
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py
index a630bd3818..5b503450a9 100755
--- a/library_generation/generate_composed_library.py
+++ b/library_generation/generate_composed_library.py
@@ -81,7 +81,7 @@ def generate_composed_library(
             library=library,
             proto_path=util.remove_version_from(gapic.proto_path),
             library_path=library_path,
-            gapic_inputs=gapic_inputs,
+            transport=library.get_transport(gapic_inputs),
         )
         temp_destination_path = f"java-{gapic.proto_path.replace('/','-')}"
         effective_arguments = __construct_effective_arg(
diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py
index be5a4ee0ae..9527f1b0d3 100644
--- a/library_generation/model/library_config.py
+++ b/library_generation/model/library_config.py
@@ -15,6 +15,7 @@
 from hashlib import sha256
 from typing import Optional
 from library_generation.model.gapic_config import GapicConfig
+from library_generation.model.gapic_inputs import GapicInputs
 
 
 MAVEN_COORDINATE_SEPARATOR = ":"
@@ -103,6 +104,13 @@ def get_artifact_id(self) -> str:
         """
         return self.get_maven_coordinate().split(MAVEN_COORDINATE_SEPARATOR)[-1]
 
+    def get_transport(self, gapic_inputs: GapicInputs) -> str:
+        """
+        Returns the transport of the library. If directly set in library config, return it.
+        Otherwise, return the transport inferred from gapic_inputs
+        """
+        return self.transport if self.transport is not None else gapic_inputs.transport
+
     def __get_distribution_name(self, distribution_name: Optional[str]) -> str:
         LibraryConfig.__check_distribution_name(distribution_name)
         if distribution_name:
diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py
index f007e66bad..d86b5e26ef 100644
--- a/library_generation/test/utilities_unit_tests.py
+++ b/library_generation/test/utilities_unit_tests.py
@@ -64,7 +64,7 @@
     product_documentation="https://cloud.google.com/solutions/secrets-management/",
     api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
     gapic_configs=list(),
-    transport="http",
+    transport="rest",
 )
 
 
@@ -305,11 +305,13 @@ def __setup_postprocessing_prerequisite_files(
         library.library_type = library_type
         config = self.__get_a_gen_config(combination, library_type=library_type)
         proto_path = "google/cloud/baremetalsolution/v2"
+        gapic_inputs = GapicInputs()  # defaults to transport=grpc
+        transport = library.get_transport(gapic_inputs)
         util.generate_postprocessing_prerequisite_files(
             config=config,
             library=library,
             proto_path=proto_path,
-            gapic_inputs=GapicInputs(),  # defaults to transport=grpc
+            transport=transport,
             library_path=library_path,
         )
         return library_path
diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py
index 84e54e39b0..527f91d273 100755
--- a/library_generation/utils/utilities.py
+++ b/library_generation/utils/utilities.py
@@ -18,7 +18,6 @@
 import shutil
 from pathlib import Path
 from library_generation.model.generation_config import GenerationConfig
-from library_generation.model.gapic_inputs import GapicInputs
 from library_generation.model.library_config import LibraryConfig
 from typing import List
 from library_generation.model.repo_config import RepoConfig
@@ -187,7 +186,7 @@ def generate_postprocessing_prerequisite_files(
     config: GenerationConfig,
     library: LibraryConfig,
     proto_path: str,
-    gapic_inputs: GapicInputs,
+    transport: str,
     library_path: str,
     language: str = "java",
 ) -> None:
@@ -199,14 +198,11 @@ def generate_postprocessing_prerequisite_files(
     :param library: the library configuration
     :param proto_path: the path from the root of googleapis to the location of the service
     protos. If the path contains a version, it will be removed
-    :param gapic_inputs: gapic_inputs obtained from the library's BUILD.bazel
+    :param transport: the transport of the library
     :param library_path: the path to which the generated file goes
     :param language: programming language of the library
     :return: None
     """
-    transport = (
-        gapic_inputs.transport if library.transport is None else library.transport
-    )
     library_name = library.get_library_name()
     artifact_id = library.get_artifact_id()
     if config.contains_common_protos():

From f7ef615b8947f79d232bbbbc7d4f3871982a8d61 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Mon, 22 Jul 2024 19:35:22 +0000
Subject: [PATCH 7/9] add tests

---
 library_generation/model/generation_config.py   |  1 +
 library_generation/model/library_config.py      |  9 ++++++++-
 library_generation/test/utilities_unit_tests.py | 14 ++++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/library_generation/model/generation_config.py b/library_generation/model/generation_config.py
index 8a2b0829c4..87242fd1a6 100644
--- a/library_generation/model/generation_config.py
+++ b/library_generation/model/generation_config.py
@@ -143,6 +143,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
             ),
             recommended_package=__optional(library, "recommended_package", None),
             min_java_version=__optional(library, "min_java_version", None),
+            transport=__optional(library, "transport", None),
         )
         parsed_libraries.append(new_library)
 
diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py
index 9527f1b0d3..59f093f782 100644
--- a/library_generation/model/library_config.py
+++ b/library_generation/model/library_config.py
@@ -80,7 +80,7 @@ def __init__(
         self.recommended_package = recommended_package
         self.min_java_version = min_java_version
         self.distribution_name = self.__get_distribution_name(distribution_name)
-        self.transport = transport
+        self.transport = self.__validate_transport(transport)
 
     def get_library_name(self) -> str:
         """
@@ -119,6 +119,13 @@ def __get_distribution_name(self, distribution_name: Optional[str]) -> str:
         library_name = self.get_library_name()
         return f"{self.group_id}:google-{cloud_prefix}{library_name}"
 
+    def __validate_transport(self, transport: str):
+        if transport not in [None, "grpc", "rest", "both"]:
+            raise ValueError(
+                "allowed values for library.transport: grpc, rest, both and None"
+            )
+        return transport
+
     @staticmethod
     def __check_distribution_name(distribution_name: str) -> None:
         if not distribution_name:
diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py
index d86b5e26ef..7d9f09ec2c 100644
--- a/library_generation/test/utilities_unit_tests.py
+++ b/library_generation/test/utilities_unit_tests.py
@@ -254,6 +254,20 @@ def test_generate_postprocessing_prerequisite_files__custom_transport_set_in_con
         )
         self.__remove_postprocessing_prerequisite_files(path=library_path)
 
+    def test_create__library_invalid_transport__fails(
+        self,
+    ):
+
+        with self.assertRaises(ValueError):
+            test_library_with_invalid_transport = LibraryConfig(
+                api_shortname="secretmanager",
+                name_pretty="Secret Management",
+                product_documentation="https://cloud.google.com/solutions/secrets-management/",
+                api_description="allows you to encrypt, store, manage, and audit infrastructure and application-level secrets.",
+                gapic_configs=list(),
+                transport="http",
+            )
+
     def test_prepare_repo_monorepo_success(self):
         gen_config = self.__get_a_gen_config(2)
         repo_config = util.prepare_repo(

From 715667aa3a6a2e1cf4bc3af286b62a39cf85d0dc Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Mon, 22 Jul 2024 19:37:12 +0000
Subject: [PATCH 8/9] restore utils comment

---
 library_generation/utils/utilities.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library_generation/utils/utilities.py b/library_generation/utils/utilities.py
index 527f91d273..59f238aaea 100755
--- a/library_generation/utils/utilities.py
+++ b/library_generation/utils/utilities.py
@@ -198,7 +198,7 @@ def generate_postprocessing_prerequisite_files(
     :param library: the library configuration
     :param proto_path: the path from the root of googleapis to the location of the service
     protos. If the path contains a version, it will be removed
-    :param transport: the transport of the library
+    :param transport: transport supported by the library
     :param library_path: the path to which the generated file goes
     :param language: programming language of the library
     :return: None

From 53681e9c206e08afaa71d8778805a31dcd235860 Mon Sep 17 00:00:00 2001
From: diegomarquezp <diegomarquezp@google.com>
Date: Mon, 22 Jul 2024 19:52:44 +0000
Subject: [PATCH 9/9] improve comment

---
 library_generation/model/library_config.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py
index 59f093f782..f7992f47a2 100644
--- a/library_generation/model/library_config.py
+++ b/library_generation/model/library_config.py
@@ -107,7 +107,9 @@ def get_artifact_id(self) -> str:
     def get_transport(self, gapic_inputs: GapicInputs) -> str:
         """
         Returns the transport of the library. If directly set in library config, return it.
-        Otherwise, return the transport inferred from gapic_inputs
+        Otherwise, return the transport inferred from gapic_inputs. This value is only
+        used for postprocessing - the generation still infers the transport from BUILD
+        files.
         """
         return self.transport if self.transport is not None else gapic_inputs.transport