Skip to content

Commit

Permalink
feat: populate .repo-metadata.json from highest version (#2890)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JoeWang1127 committed Jun 19, 2024
1 parent 9829721 commit f587541
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 3 deletions.
2 changes: 1 addition & 1 deletion library_generation/generate_composed_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
75 changes: 75 additions & 0 deletions library_generation/model/gapic_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
7 changes: 5 additions & 2 deletions library_generation/model/library_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
122 changes: 122 additions & 0 deletions library_generation/test/model/gapic_config_unit_tests.py
Original file line number Diff line number Diff line change
@@ -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",
)
27 changes: 27 additions & 0 deletions library_generation/test/model/library_config_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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(),
)

0 comments on commit f587541

Please sign in to comment.