feat: add a CLI tool to validate generation configuration

library_generation/cli/entry_point.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import os
+import sys
 
 import click as click
 from library_generation.generate_pr_description import generate_pr_descriptions
@@ -142,5 +143,28 @@ def generate(
     )
 
 
+@main.command()
+@click.option(
+    "--generation-config-path",
+    required=False,
+    type=str,
+    help="""
+    Absolute or relative path to a generation_config.yaml.
+    Default to generation_config.yaml in the current working directory.
+    """,
+)
+def validate_generation_config(generation_config_path: str) -> None:
+    """
+    Validate the given generation configuration.
+    """
+    if generation_config_path is None:
+        generation_config_path = "generation_config.yaml"
+    try:
+        from_yaml(os.path.abspath(generation_config_path))
+    except ValueError as err:
+        print(err)
+        sys.exit(1)
+
+
 if __name__ == "__main__":
     main()

library_generation/model/generation_config.py

@@ -46,6 +46,7 @@ def __init__(
         self.libraries = libraries
         self.grpc_version = grpc_version
         self.protoc_version = protoc_version
+        self.__validate()
 
     def get_proto_path_to_library_name(self) -> dict[str, str]:
         """
@@ -62,6 +63,19 @@ def get_proto_path_to_library_name(self) -> dict[str, str]:
     def is_monorepo(self) -> bool:
         return len(self.libraries) > 1
 
+    def __validate(self) -> None:
+        seen_library_names = dict()
+        for library in self.libraries:
+            library_name = library.get_library_name()
+            if library_name in seen_library_names:
+                raise ValueError(
+                    f"Both {library.name_pretty} and "
+                    f"{seen_library_names.get(library_name)} have the same "
+                    f"library name: {library_name}, please update one of the "
+                    f"library to have a different library name."
+                )
+            seen_library_names[library_name] = library.name_pretty
+
 
 def from_yaml(path_to_yaml: str) -> GenerationConfig:
     """
@@ -130,7 +144,7 @@ def from_yaml(path_to_yaml: str) -> GenerationConfig:
 
 def __required(config: Dict, key: str):
     if key not in config:
-        raise ValueError(f"required key {key} not found in yaml")
+        raise ValueError(f"required key {key} is not found in yaml.")
     return config[key]
 
 

library_generation/test/cli/entry_point_unit_tests.py

@@ -11,9 +11,13 @@
 # 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.
+import os
 import unittest
 from click.testing import CliRunner
-from library_generation.cli.entry_point import generate
+from library_generation.cli.entry_point import generate, validate_generation_config
+
+script_dir = os.path.dirname(os.path.realpath(__file__))
+test_resource_dir = os.path.join(script_dir, "..", "resources", "test-config")
 
 
 class EntryPointTest(unittest.TestCase):
@@ -44,3 +48,32 @@ def test_entry_point_with_baseline_without_current_raise_file_exception(self):
             "current_generation_config is not specified when "
             "baseline_generation_config is specified.",
         )
+
+    def test_validate_generation_config_succeeds(
+        self,
+    ):
+        runner = CliRunner()
+        # noinspection PyTypeChecker
+        result = runner.invoke(
+            validate_generation_config,
+            [f"--generation-config-path={test_resource_dir}/generation_config.yaml"],
+        )
+        self.assertEqual(0, result.exit_code)
+
+    def test_validate_generation_config_with_duplicate_library_name_raise_file_exception(
+        self,
+    ):
+        runner = CliRunner()
+        # noinspection PyTypeChecker
+        result = runner.invoke(
+            validate_generation_config,
+            [
+                f"--generation-config-path={test_resource_dir}/generation_config_with_duplicate_library_name.yaml"
+            ],
+        )
+        self.assertEqual(1, result.exit_code)
+        self.assertEqual(SystemExit, result.exc_info[0])
+        self.assertRegex(
+            result.output,
+            "have the same library name",
+        )

library_generation/test/model/generation_config_unit_test.py

@@ -14,6 +14,7 @@
 import os
 import unittest
 from pathlib import Path
+from parameterized import parameterized
 from library_generation.model.generation_config import from_yaml, GenerationConfig
 from library_generation.model.library_config import LibraryConfig
 
@@ -133,3 +134,82 @@ def test_is_monorepo_with_two_libraries_returns_true(self):
             libraries=[library_1, library_2],
         )
         self.assertTrue(config.is_monorepo())
+
+    def test_validate_with_duplicate_library_name_raise_exception(self):
+        self.assertRaisesRegex(
+            ValueError,
+            "the same library name",
+            GenerationConfig,
+            gapic_generator_version="",
+            googleapis_commitish="",
+            libraries_bom_version="",
+            owlbot_cli_image="",
+            synthtool_commitish="",
+            template_excludes=[],
+            libraries=[
+                LibraryConfig(
+                    api_shortname="secretmanager",
+                    name_pretty="Secret API",
+                    product_documentation="",
+                    api_description="",
+                    gapic_configs=list(),
+                ),
+                LibraryConfig(
+                    api_shortname="another-secret",
+                    name_pretty="Another Secret API",
+                    product_documentation="",
+                    api_description="",
+                    gapic_configs=list(),
+                    library_name="secretmanager",
+                ),
+            ],
+        )
+
+    # parameterized tests need to run from the class, see
+    # https://github.com/wolever/parameterized/issues/37 for more info.
+    # This test confirms that a ValueError with an error message about a
+    # missing key (specified in the first parameter of each `parameterized`
+    # tuple) when parsing a test configuration yaml (second parameter) will
+    # be raised.
+    @parameterized.expand(
+        [
+            ("libraries", f"{test_config_dir}/config_without_libraries.yaml"),
+            ("GAPICs", f"{test_config_dir}/config_without_gapics.yaml"),
+            ("proto_path", f"{test_config_dir}/config_without_proto_path.yaml"),
+            ("api_shortname", f"{test_config_dir}/config_without_api_shortname.yaml"),
+            (
+                "api_description",
+                f"{test_config_dir}/config_without_api_description.yaml",
+            ),
+            ("name_pretty", f"{test_config_dir}/config_without_name_pretty.yaml"),
+            (
+                "product_documentation",
+                f"{test_config_dir}/config_without_product_docs.yaml",
+            ),
+            (
+                "gapic_generator_version",
+                f"{test_config_dir}/config_without_generator.yaml",
+            ),
+            (
+                "googleapis_commitish",
+                f"{test_config_dir}/config_without_googleapis.yaml",
+            ),
+            (
+                "libraries_bom_version",
+                f"{test_config_dir}/config_without_libraries_bom_version.yaml",
+            ),
+            ("owlbot_cli_image", f"{test_config_dir}/config_without_owlbot.yaml"),
+            ("synthtool_commitish", f"{test_config_dir}/config_without_synthtool.yaml"),
+            (
+                "template_excludes",
+                f"{test_config_dir}/config_without_temp_excludes.yaml",
+            ),
+        ]
+    )
+    def test_from_yaml_without_key_fails(self, error_message_contains, path_to_yaml):
+        self.assertRaisesRegex(
+            ValueError,
+            error_message_contains,
+            from_yaml,
+            path_to_yaml,
+        )

library_generation/test/resources/test-config/generation_config_with_duplicate_library_name.yaml

@@ -0,0 +1,51 @@
+gapic_generator_version: 2.34.0
+protoc_version: 25.2
+googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026
+libraries_bom_version: 26.37.0
+owlbot_cli_image: sha256:623647ee79ac605858d09e60c1382a716c125fb776f69301b72de1cd35d49409
+synthtool_commitish: 6612ab8f3afcd5e292aecd647f0fa68812c9f5b5
+template_excludes:
+  - ".github/*"
+  - ".kokoro/*"
+  - "samples/*"
+  - "CODE_OF_CONDUCT.md"
+  - "CONTRIBUTING.md"
+  - "LICENSE"
+  - "SECURITY.md"
+  - "java.header"
+  - "license-checks.xml"
+  - "renovate.json"
+  - ".gitignore"
+libraries:
+  - api_shortname: cloudasset
+    name_pretty: Cloud Asset
+    product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    api_description: "provides inventory services based on a time series database."
+    library_name: "asset"
+    client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
+    distribution_name: "com.google.cloud:google-cloud-asset"
+    release_level: "stable"
+    issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
+    api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    codeowner_team: "@googleapis/analytics-dpe"
+    excluded_poms: proto-google-iam-v1-bom,google-iam-policy,proto-google-iam-v1
+    excluded_dependencies: google-iam-policy
+    GAPICs:
+      - proto_path: google/cloud/asset/v1
+      - proto_path: google/cloud/asset/v1p1beta1
+      - proto_path: google/cloud/asset/v1p2beta1
+      - proto_path: google/cloud/asset/v1p5beta1
+      - proto_path: google/cloud/asset/v1p7beta1
+
+  - api_shortname: another-cloudasset
+    name_pretty: Cloud Asset Inventory
+    product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    api_description: "provides inventory services based on a time series database."
+    library_name: "asset"
+    client_documentation: "https://cloud.google.com/java/docs/reference/google-cloud-asset/latest/overview"
+    distribution_name: "com.google.cloud:google-cloud-asset"
+    release_level: "stable"
+    issue_tracker: "https://issuetracker.google.com/issues/new?component=187210&template=0"
+    api_reference: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview"
+    GAPICs:
+    - proto_path: google/cloud/asset/v1

library_generation/test/utilities_unit_tests.py

@@ -26,7 +26,6 @@
 from library_generation.model.gapic_config import GapicConfig
 from library_generation.model.generation_config import GenerationConfig
 from library_generation.model.gapic_inputs import parse as parse_build_file
-from library_generation.model.generation_config import from_yaml
 from library_generation.model.library_config import LibraryConfig
 from library_generation.test.test_utils import FileComparator
 from library_generation.test.test_utils import cleanup
@@ -114,55 +113,6 @@ def test_eprint_valid_input_succeeds(self):
         # print() appends a `\n` each time it's called
         self.assertEqual(test_input + "\n", result)
 
-    # parameterized tests need to run from the class, see
-    # https://github.com/wolever/parameterized/issues/37 for more info.
-    # This test confirms that a ValueError with an error message about a
-    # missing key (specified in the first parameter of each `parameterized`
-    # tuple) when parsing a test configuration yaml (second parameter) will
-    # be raised.
-    @parameterized.expand(
-        [
-            ("libraries", f"{test_config_dir}/config_without_libraries.yaml"),
-            ("GAPICs", f"{test_config_dir}/config_without_gapics.yaml"),
-            ("proto_path", f"{test_config_dir}/config_without_proto_path.yaml"),
-            ("api_shortname", f"{test_config_dir}/config_without_api_shortname.yaml"),
-            (
-                "api_description",
-                f"{test_config_dir}/config_without_api_description.yaml",
-            ),
-            ("name_pretty", f"{test_config_dir}/config_without_name_pretty.yaml"),
-            (
-                "product_documentation",
-                f"{test_config_dir}/config_without_product_docs.yaml",
-            ),
-            (
-                "gapic_generator_version",
-                f"{test_config_dir}/config_without_generator.yaml",
-            ),
-            (
-                "googleapis_commitish",
-                f"{test_config_dir}/config_without_googleapis.yaml",
-            ),
-            (
-                "libraries_bom_version",
-                f"{test_config_dir}/config_without_libraries_bom_version.yaml",
-            ),
-            ("owlbot_cli_image", f"{test_config_dir}/config_without_owlbot.yaml"),
-            ("synthtool_commitish", f"{test_config_dir}/config_without_synthtool.yaml"),
-            (
-                "template_excludes",
-                f"{test_config_dir}/config_without_temp_excludes.yaml",
-            ),
-        ]
-    )
-    def test_from_yaml_without_key_fails(self, error_message_contains, path_to_yaml):
-        self.assertRaisesRegex(
-            ValueError,
-            error_message_contains,
-            from_yaml,
-            path_to_yaml,
-        )
-
     @parameterized.expand(
         [
             ("BUILD_no_additional_protos.bazel", " "),
@@ -344,17 +294,6 @@ def test_prepare_repo_monorepo_success(self):
             ["java-bare-metal-solution", "java-secretmanager"], library_path
         )
 
-    def test_prepare_repo_monorepo_duplicated_library_name_failed(self):
-        gen_config = self.__get_a_gen_config(3)
-        self.assertRaisesRegex(
-            ValueError,
-            "secretmanager",
-            util.prepare_repo,
-            gen_config,
-            gen_config.libraries,
-            f"{resources_dir}/misc",
-        )
-
     def test_prepare_repo_monorepo_failed(self):
         gen_config = self.__get_a_gen_config(2)
         self.assertRaises(

library_generation/utils/utilities.py

@@ -130,10 +130,6 @@ def prepare_repo(
         # use absolute path because docker requires absolute path
         # in volume name.
         absolute_library_path = str(Path(library_path).resolve())
-        if absolute_library_path in libraries:
-            # check whether the java_library is unique among all libraries
-            # because two libraries should not go to the same destination.
-            raise ValueError(f"{absolute_library_path} already exists.")
         libraries[absolute_library_path] = library
         # remove existing .repo-metadata.json
         json_name = ".repo-metadata.json"