feat: populate .repo-metadata.json from highest version (#2890)

JoeWang1127 · web-flow · commit f58754178d16 · 2024-06-19T08:19:34.000-04:00
In this PR: - populate `.repo-metadata.json` from highest stable version of proto paths. The `.repo-metadata.json` is only generated once per library and the first item in proto paths is used, so the proto paths with the highest stable version should be the first item after sorting the proto paths in the library. The method used to compare two proto paths, `a` and `b`: 1. if both of `a` and `b` don't have a version, the one with lower depth is smaller. 2. if either `a` or `b` has a version, the one with the version is smaller. 3. if either `a` or `b` has a stable version, but not both, the one with the stable version is smaller, e.g., `google/spanner/v2` is smaller than `google/spanner/v3beta`. 4. if both `a` and `b` have a non-stable version, the one with higher version is smaller, e.g., `google/spanner/v2beta1` is smaller than `google/spanner/v2alpha2`. 5. if both `a` and `b` have a stable version, the one with lower depth is smaller, e.g., `google/spanner/v1` is smaller than `google/spanner/admin/v1`, `google/spanner/v1` is smaller than `google/spanner/admin/v2` 6. if both `a` and `b` have a stable version and the depth is the same, the one with higher version is smaller, e.g., `google/spanner/v2` is smaller than `google/spanner/v1`. Test the change: 1. build the image locally: ``` docker build -f .cloudbuild/library_generation/library_generation.Dockerfile -t local:latest . ``` 2. build the generator locally: ``` mvn -V -B -ntp clean install -DskipTests ``` 3. run the image: ``` docker run --rm -u "$(id -u):$(id -g)" -v "$(pwd):/workspace" -v "$HOME/.m2:/home/.m2" local:latest ``` Result: 1. no change in common protos 2. `transport` changed to `both` in `java-iam/.repo-metadata.json`. This is expected since the `transport` in [`google/iam/v2/BUILD.bazel`](https://github.com/googleapis/googleapis/blob/master/google/iam/v2/BUILD.bazel#L94) is `grpc+rest`. Without this change, the `transport` is parsed from `google/iam/v2/BUILD.bazel`, which doesn't have a `java_gapic_library` target, thus the value is `grpc` by default.
diff --git a/library_generation/generate_composed_library.py b/library_generation/generate_composed_library.py
@@ -66,7 +66,7 @@ def generate_composed_library(
     base_arguments = __construct_tooling_arg(config=config)
     owlbot_cli_source_folder = util.sh_util("mktemp -d")
     os.makedirs(f"{library_path}", exist_ok=True)
-    for gapic in library.gapic_configs:
+    for gapic in library.get_sorted_gapic_configs():
         build_file_folder = Path(f"{output_folder}/{gapic.proto_path}").resolve()
         print(f"build_file_folder: {build_file_folder}")
         gapic_inputs = parse_build_file(build_file_folder, gapic.proto_path)
diff --git a/library_generation/model/gapic_config.py b/library_generation/model/gapic_config.py
@@ -12,6 +12,11 @@
 # 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 re
+from typing import Optional
+
+ALPHA_VERSION = "alpha"
+BETA_VERSION = "beta"
 
 
 class GapicConfig:
@@ -22,3 +27,73 @@ class GapicConfig:
 
     def __init__(self, proto_path: str):
         self.proto_path = proto_path
+        self.version = self.__parse_version()
+
+    def is_stable(self):
+        return (
+            self.version
+            and (ALPHA_VERSION not in self.version)
+            and (BETA_VERSION not in self.version)
+        )
+
+    def get_version(self):
+        return self.version
+
+    def __parse_version(self) -> Optional[str]:
+        version_regex = re.compile(r"^v[1-9]+(p[1-9]+)*(alpha|beta)?.*")
+        for directory in self.proto_path.split("/"):
+            if version_regex.search(directory):
+                return directory
+        return None
+
+    def __eq__(self, other) -> bool:
+        if not isinstance(other, GapicConfig):
+            return False
+        return self.proto_path == other.proto_path
+
+    def __lt__(self, other) -> bool:
+        if not isinstance(other, GapicConfig):
+            raise ValueError(
+                f"Type {type(other)} can't be comparable " f"with GapicConfig."
+            )
+
+        self_version = self.get_version()
+        other_version = other.get_version()
+        self_dirs = self.proto_path.split("/")
+        other_dirs = other.proto_path.split("/")
+        # Case 1: if both of the configs don't have a version in proto_path,
+        # the one with lower depth is smaller.
+        if (not self_version) and (not other_version):
+            return len(self_dirs) < len(other_dirs)
+        # Case 2: if only one config has a version in proto_path, it is smaller
+        # than the other one.
+        if self_version and (not other_version):
+            return True
+        if (not self_version) and other_version:
+            return False
+        # Two configs both have a version in proto_path.
+        self_stable = self.is_stable()
+        other_stable = other.is_stable()
+        # Case 3, if only config has a stable version in proto_path, it is
+        # smaller than the other one.
+        if self_stable and (not other_stable):
+            return True
+        if (not self_stable) and other_stable:
+            return False
+        # Case 4, if two configs have a non-stable version in proto_path,
+        # the one with higher version is smaller.
+        # Note that using string comparison may lead unexpected result,
+        # e.g., v1beta10 is smaller than v1beta2.
+        # In reality, however, there's unlikely that a library has >10 beta
+        # versions.
+        if (not self_stable) and (not other_stable):
+            return self_version > other_version
+        # Two configs both have a stable version in proto_path.
+        # Case 5, if two configs have different depth in proto_path, the one
+        # with lower depth is smaller.
+        if len(self_dirs) != len(other_dirs):
+            return len(self_dirs) < len(other_dirs)
+        # Case 6, the config with higher stable version is smaller.
+        self_num = int(self_version[1:])
+        other_num = int(other_version[1:])
+        return self_num > other_num
diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py
@@ -14,7 +14,7 @@
 # limitations under the License.
 from hashlib import sha1
 
-from typing import List, Optional
+from typing import Optional
 from library_generation.model.gapic_config import GapicConfig
 
 
@@ -29,7 +29,7 @@ def __init__(
         api_description: str,
         name_pretty: str,
         product_documentation: str,
-        gapic_configs: List[GapicConfig],
+        gapic_configs: list[GapicConfig],
         library_type: Optional[str] = None,
         release_level: Optional[str] = None,
         api_id: Optional[str] = None,
@@ -84,6 +84,9 @@ def get_library_name(self) -> str:
         """
         return self.library_name if self.library_name else self.api_shortname
 
+    def get_sorted_gapic_configs(self) -> list[GapicConfig]:
+        return sorted(self.gapic_configs)
+
     def __eq__(self, other):
         return (
             self.api_shortname == other.api_shortname
diff --git a/library_generation/test/model/gapic_config_unit_tests.py b/library_generation/test/model/gapic_config_unit_tests.py
@@ -0,0 +1,122 @@
+# Copyright 2024 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.
+import unittest
+
+from library_generation.model.gapic_config import GapicConfig
+
+
+class GapicConfigTest(unittest.TestCase):
+    def test_get_version_returns_none(self):
+        self.assertIsNone(GapicConfig(proto_path="example/dir1/dir2").get_version())
+
+    def test_get_version_returns_non_stable_version(self):
+        self.assertEqual(
+            "v2p2beta1",
+            GapicConfig(proto_path="example/dir1/dir2/v2p2beta1").get_version(),
+        )
+        self.assertEqual(
+            "v2beta1",
+            GapicConfig(proto_path="example/dir1/dir2/v2beta1").get_version(),
+        )
+
+    def test_get_version_returns_stable_version(self):
+        self.assertEqual(
+            "v20",
+            GapicConfig(proto_path="example/dir1/dir2/v20").get_version(),
+        )
+
+    def test_is_stable_with_no_version_returns_false(self):
+        self.assertFalse(
+            GapicConfig(proto_path="example/dir1/dir2/non_version").is_stable(),
+        )
+
+    def test_is_stable_with_non_stable_version_returns_false(self):
+        self.assertFalse(
+            GapicConfig(proto_path="example/dir1/dir2/v20alpha").is_stable(),
+        )
+        self.assertFalse(
+            GapicConfig(proto_path="example/dir1/dir2/v20beta2").is_stable(),
+        )
+
+    def test_is_stable_with_stable_version_returns_true(self):
+        self.assertTrue(
+            GapicConfig(proto_path="example/dir1/dir2/v30").is_stable(),
+        )
+
+    def test_compare_configs_without_a_version(self):
+        config_len_3 = GapicConfig(proto_path="example/dir1/dir2")
+        config_len_4 = GapicConfig(proto_path="example/dir1/dir2/dir3")
+        self.assertLess(
+            config_len_3,
+            config_len_4,
+            "config_len_3 should be smaller since it has a lower depth.",
+        )
+
+    def test_compare_configs_only_one_has_a_stable_version(self):
+        versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v1")
+        non_versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3")
+        self.assertLess(
+            versioned_config,
+            non_versioned_config,
+            "versioned_config should be smaller since it has a version.",
+        )
+
+    def test_compare_configs_only_one_has_a_non_stable_version(self):
+        non_stable_versioned_config = GapicConfig(
+            proto_path="example/dir1/dir2/dir3/dir4/v1beta"
+        )
+        non_versioned_config = GapicConfig(proto_path="example/dir1/dir2/dir3")
+        self.assertLess(
+            non_stable_versioned_config,
+            non_versioned_config,
+            "non_stable_versioned_config should be smaller since it has a version.",
+        )
+
+    def test_compare_configs_one_has_non_stable_and_one_has_stable_version(self):
+        stable_versioned_config = GapicConfig(
+            proto_path="example/dir1/dir2/dir3/dir4/v1"
+        )
+        non_stable_versioned_config = GapicConfig(proto_path="example/dir1/dir2/v2beta")
+        self.assertLess(
+            stable_versioned_config,
+            non_stable_versioned_config,
+            "stable_versioned_config should be smaller since it has a stable version.",
+        )
+
+    def test_compare_configs_two_have_non_stable_version(self):
+        v3p2beta = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v3p2beta")
+        v2p4beta = GapicConfig(proto_path="example/dir1/dir2/v2p4beta")
+        self.assertLess(
+            v3p2beta,
+            v2p4beta,
+            "v3p2beta should be smaller since it has a higher version.",
+        )
+
+    def test_compare_configs_two_have_stable_version_different_depth(self):
+        v3 = GapicConfig(proto_path="example/dir1/dir2/v3")
+        v4 = GapicConfig(proto_path="example/dir1/dir2/dir3/dir4/v4")
+        self.assertLess(
+            v3,
+            v4,
+            "v3 should be smaller since it has lower depth",
+        )
+
+    def test_compare_configs_two_have_stable_version_same_depth(self):
+        v4 = GapicConfig(proto_path="example/dir1/dir2/v4")
+        v3 = GapicConfig(proto_path="example/dir1/dir2/v3")
+        self.assertLess(
+            v4,
+            v3,
+            "v4 should be smaller since it has a higher version",
+        )
diff --git a/library_generation/test/model/library_config_unit_tests.py b/library_generation/test/model/library_config_unit_tests.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 import unittest
 
+from library_generation.model.gapic_config import GapicConfig
 from library_generation.model.library_config import LibraryConfig
 
 
@@ -37,3 +38,29 @@ def test_get_library_returns_api_shortname(self):
             gapic_configs=list(),
         )
         self.assertEqual("secret", library.get_library_name())
+
+    def test_get_sorted_gapic_configs_returns_correct_order(self):
+        v1beta1 = GapicConfig(proto_path="google/spanner/v1beta1")
+        v1 = GapicConfig(proto_path="google/spanner/v1")
+        v1alpha1 = GapicConfig(proto_path="google/spanner/v1alpha")
+        v2 = GapicConfig(proto_path="google/spanner/v2")
+        admin_v2 = GapicConfig(proto_path="google/spanner/admin/v2")
+        non_versioned = GapicConfig(proto_path="google/spanner/type")
+        library = LibraryConfig(
+            api_shortname="secret",
+            name_pretty="",
+            product_documentation="",
+            api_description="",
+            gapic_configs=[v1alpha1, v1, v2, admin_v2, non_versioned, v1beta1],
+        )
+        self.assertEqual(
+            [
+                v2,
+                v1,
+                admin_v2,
+                v1beta1,
+                v1alpha1,
+                non_versioned,
+            ],
+            library.get_sorted_gapic_configs(),
+        )
