From 7f91d91bb7ef8e94037ecd91c9311d646513cf65 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 15 Mar 2024 21:33:16 +0000 Subject: [PATCH 01/21] feat: add a generation config comparator --- .../utils/generation_config_comparator.py | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 library_generation/utils/generation_config_comparator.py diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py new file mode 100644 index 0000000000..43fbe9bc93 --- /dev/null +++ b/library_generation/utils/generation_config_comparator.py @@ -0,0 +1,78 @@ +# 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. +from enum import Enum +from typing import Dict +from typing import List +from library_generation.model.generation_config import from_yaml +from library_generation.model.library_config import LibraryConfig + + +class DiffType(Enum): + GOOGLEAPIS_COMMIT_UPDATE = 1 + GENERATOR_UPDATE = 2 + OWLBOT_CLI_UPDATE = 3 + SYNTHTOOL_UPDATE = 4 + PROTOBUF_UPDATE = 5 + GRPC_UPDATE = 6 + TEMPLATE_EXCLUDES_UPDATE = 7 + LIBRARIES_UPDATE = 8 + + +def compare_config( + path_to_baseline_config_yaml: str, path_to_latest_config_yaml: str +) -> Dict[DiffType, List[str]]: + """ + Compare two GenerationConfig object and output a mapping from DiffType + to a list of library_name of affected libraries. + All libraries in the latest configuration will be affected if the library + list is empty. + + :param path_to_baseline_config_yaml: the path to the baseline configuration + file. + :param path_to_latest_config_yaml: the path to the latest configuration + file. + :return: + """ + diff = {} + baseline_config = from_yaml(path_to_baseline_config_yaml) + latest_config = from_yaml(path_to_latest_config_yaml) + if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: + diff[DiffType.GOOGLEAPIS_COMMIT_UPDATE] = [] + if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: + diff[DiffType.GENERATOR_UPDATE] = [] + if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: + diff[DiffType.OWLBOT_CLI_UPDATE] = [] + if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: + diff[DiffType.SYNTHTOOL_UPDATE] = [] + if baseline_config.protobuf_version != latest_config.protobuf_version: + diff[DiffType.PROTOBUF_UPDATE] = [] + if baseline_config.grpc_version != latest_config.grpc_version: + diff[DiffType.GRPC_UPDATE] = [] + if baseline_config.template_excludes != latest_config.template_excludes: + diff[DiffType.TEMPLATE_EXCLUDES_UPDATE] = [] + + __compare_libraries( + diff=diff, + baseline_libraries=baseline_config.libraries, + latest_libraries=latest_config.libraries, + ) + return diff + + +def __compare_libraries( + diff: Dict[DiffType, List[str]], + baseline_libraries: List[LibraryConfig], + latest_libraries: List[LibraryConfig], +) -> None: + pass From e5aae8eb1ce9fb58f0dbe83c3a336226195391b6 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sat, 16 Mar 2024 00:42:42 +0000 Subject: [PATCH 02/21] compare library configs --- library_generation/model/gapic_config.py | 9 + library_generation/model/library_config.py | 59 ++++ .../utils/generation_config_comparator.py | 261 ++++++++++++++++-- 3 files changed, 307 insertions(+), 22 deletions(-) diff --git a/library_generation/model/gapic_config.py b/library_generation/model/gapic_config.py index bec1645823..91bec701d4 100644 --- a/library_generation/model/gapic_config.py +++ b/library_generation/model/gapic_config.py @@ -12,6 +12,7 @@ # 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. +from hashlib import sha1 class GapicConfig: @@ -22,3 +23,11 @@ class GapicConfig: def __init__(self, proto_path: str): self.proto_path = proto_path + + def __eq__(self, other): + return self.proto_path == other.proto_path + + def __hash__(self): + m = sha1() + m.update([self.proto_path]) + return m.hexdigest() diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py index b6f8ae1b48..582c8a0d3f 100644 --- a/library_generation/model/library_config.py +++ b/library_generation/model/library_config.py @@ -12,6 +12,7 @@ # 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. +from hashlib import sha1 from typing import List, Optional from library_generation.model.gapic_config import GapicConfig @@ -71,3 +72,61 @@ def __init__( self.cloud_api = cloud_api self.requires_billing = requires_billing self.extra_versioned_modules = extra_versioned_modules + + def __eq__(self, other): + return ( + self.api_shortname == other.api_shortname + and self.api_description == other.api_description + and self.name_pretty == other.name_pretty + and self.product_documentation == other.product_documentation + and self.gapic_configs == other.gapic_configs + and self.library_type == other.library_type + and self.release_level == other.release_level + and self.api_id == other.api_id + and self.api_reference == other.api_reference + and self.codeowner_team == other.codeowner_team + and self.excluded_dependencies == other.excluded_dependencies + and self.excluded_poms == other.excluded_poms + and self.client_documentation == other.client_documentation + and self.distribution_name == other.distribution_name + and self.googleapis_commitish == other.googleapis_commitish + and self.group_id == other.group_id + and self.issue_tracker == other.issue_tracker + and self.library_name == other.library_name + and self.rest_documentation == other.rest_documentation + and self.rpc_documentation == other.rpc_documentation + and self.cloud_api == other.cloud_api + and self.requires_billing == other.requires_billing + and self.extra_versioned_modules == other.extra_versioned_modules + ) + + def __hash__(self): + m = sha1() + m.update( + [ + self.api_shortname, + self.api_description, + self.name_pretty, + self.product_documentation, + self.library_type, + self.release_level, + self.api_id, + self.api_reference, + self.codeowner_team, + self.excluded_dependencies, + self.excluded_poms, + self.client_documentation, + self.distribution_name, + self.googleapis_commitish, + self.group_id, + self.issue_tracker, + self.library_name, + self.rest_documentation, + self.rpc_documentation, + self.cloud_api, + self.requires_billing, + self.extra_versioned_modules, + ] + + [hash(config) for config in self.gapic_configs] + ) + return m.hexdigest() diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 43fbe9bc93..6d90653de3 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -16,17 +16,35 @@ from typing import List from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig +from library_generation.utilities import get_library_name class DiffType(Enum): - GOOGLEAPIS_COMMIT_UPDATE = 1 - GENERATOR_UPDATE = 2 - OWLBOT_CLI_UPDATE = 3 - SYNTHTOOL_UPDATE = 4 - PROTOBUF_UPDATE = 5 - GRPC_UPDATE = 6 - TEMPLATE_EXCLUDES_UPDATE = 7 - LIBRARIES_UPDATE = 8 + GOOGLEAPIS_COMMIT = 1 + GENERATOR = 2 + OWLBOT_CLI = 3 + SYNTHTOOL = 4 + PROTOBUF = 5 + GRPC = 6 + TEMPLATE_EXCLUDES = 7 + LIBRARIES_ADDITION = 8 + LIBRARIES_REMOVAL = 9 + API_DESCRIPTION = 10 + NAME_PRETTY = 11 + PRODUCT_DOCS = 12 + LIBRARY_TYPE = 13 + RELEASE_LEVEL = 14 + API_ID = 15 + API_REFERENCE = 16 + CODEOWNER_TEAM = 17 + EXCLUDED_DEPENDENCIES = 18 + EXCLUDED_POMS = 19 + CLIENT_DOCS = 20 + ISSUE_TRACKER = 21 + REST_DOCS = 22 + RPC_DOCS = 23 + REQUIRES_BILLING = 24 + EXTRA_VERSIONED_MODULES = 25 def compare_config( @@ -48,31 +66,230 @@ def compare_config( baseline_config = from_yaml(path_to_baseline_config_yaml) latest_config = from_yaml(path_to_latest_config_yaml) if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: - diff[DiffType.GOOGLEAPIS_COMMIT_UPDATE] = [] + diff[DiffType.GOOGLEAPIS_COMMIT] = [] if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: - diff[DiffType.GENERATOR_UPDATE] = [] + diff[DiffType.GENERATOR] = [] if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: - diff[DiffType.OWLBOT_CLI_UPDATE] = [] + diff[DiffType.OWLBOT_CLI] = [] if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: - diff[DiffType.SYNTHTOOL_UPDATE] = [] + diff[DiffType.SYNTHTOOL] = [] if baseline_config.protobuf_version != latest_config.protobuf_version: - diff[DiffType.PROTOBUF_UPDATE] = [] + diff[DiffType.PROTOBUF] = [] if baseline_config.grpc_version != latest_config.grpc_version: - diff[DiffType.GRPC_UPDATE] = [] + diff[DiffType.GRPC] = [] if baseline_config.template_excludes != latest_config.template_excludes: - diff[DiffType.TEMPLATE_EXCLUDES_UPDATE] = [] + diff[DiffType.TEMPLATE_EXCLUDES] = [] - __compare_libraries( + compare_libraries( diff=diff, - baseline_libraries=baseline_config.libraries, - latest_libraries=latest_config.libraries, + baseline_library_configs=baseline_config.libraries, + latest_library_configs=latest_config.libraries, ) return diff -def __compare_libraries( +def compare_libraries( diff: Dict[DiffType, List[str]], - baseline_libraries: List[LibraryConfig], - latest_libraries: List[LibraryConfig], + baseline_library_configs: List[LibraryConfig], + latest_library_configs: List[LibraryConfig], ) -> None: - pass + baseline_libraries = convert(baseline_library_configs) + latest_libraries = convert(latest_library_configs) + changed_libraries = [] + # 1st round comparison. + for library_name, (hash_value, _) in baseline_libraries: + # 1. find any library removed from baseline_libraries. + # a library is removed from baseline_libraries if the library_name + # is not in latest_libraries. + if library_name not in latest_libraries: + if DiffType.LIBRARIES_REMOVAL not in diff: + diff[DiffType.LIBRARIES_REMOVAL] = [] + diff[DiffType.LIBRARIES_REMOVAL].append(library_name) + # 2. find any library that exists in both configs but at least one + # parameter is changed, which means the hash value is different. + if ( + library_name in latest_libraries + and hash_value != latest_libraries[library_name][0] + ): + changed_libraries.append(library_name) + # 2nd round comparison. + for library_name, (hash_value, _) in latest_libraries: + # find any library added to latest_libraries. + # a library is added to latest_libraries if the library_name + # is not in baseline_libraries. + if library_name not in baseline_libraries: + if DiffType.LIBRARIES_ADDITION not in diff: + diff[DiffType.LIBRARIES_ADDITION] = [] + diff[DiffType.LIBRARIES_ADDITION].append(library_name) + # 3rd round comparison. + compare_changed_libraries( + diff=diff, + baseline_libraries=baseline_libraries, + latest_libraries=latest_libraries, + changed_libraries=changed_libraries, + ) + + +def convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)]: + """ + Convert a list of LibraryConfig objects to a Dict. + For each library object, the key is the library_name of the object, the + value is a tuple, which contains the hash value of the object and the object + itself. + :param libraries: + :return: + """ + return { + get_library_name(library): (hash(library), library) for library in libraries + } + + +def compare_changed_libraries( + diff: Dict[DiffType, List[str]], + baseline_libraries: Dict[str, (int, LibraryConfig)], + latest_libraries: Dict[str, (int, LibraryConfig)], + changed_libraries: List[str], +) -> None: + """ + Compare each library with the same library_name to find what parameters are + changed. + + Some parameters should not change: + + - api_shortname + - library_name + - distribution_name + - group_id + - cloud_api + + :param diff: + :param baseline_libraries: + :param latest_libraries: + :param changed_libraries: + :return: + """ + for library_name in changed_libraries: + if ( + baseline_libraries[library_name][1].api_description + != latest_libraries[library_name][1].api_description + ): + if DiffType.API_DESCRIPTION not in diff: + diff[DiffType.API_DESCRIPTION] = [] + diff[DiffType.API_DESCRIPTION].append(library_name) + if ( + baseline_libraries[library_name][1].name_pretty + != latest_libraries[library_name][1].name_pretty + ): + if DiffType.NAME_PRETTY not in diff: + diff[DiffType.NAME_PRETTY] = [] + diff[DiffType.NAME_PRETTY].append(library_name) + if ( + baseline_libraries[library_name][1].product_documentation + != latest_libraries[library_name][1].product_documentation + ): + if DiffType.PRODUCT_DOCS not in diff: + diff[DiffType.PRODUCT_DOCS] = [] + diff[DiffType.PRODUCT_DOCS].append(library_name) + + if ( + baseline_libraries[library_name][1].library_type + != latest_libraries[library_name][1].library_type + ): + if DiffType.LIBRARY_TYPE not in diff: + diff[DiffType.LIBRARY_TYPE] = [] + diff[DiffType.LIBRARY_TYPE].append(library_name) + + if ( + baseline_libraries[library_name][1].release_level + != latest_libraries[library_name][1].release_level + ): + if DiffType.RELEASE_LEVEL not in diff: + diff[DiffType.RELEASE_LEVEL] = [] + diff[DiffType.RELEASE_LEVEL].append(library_name) + + if ( + baseline_libraries[library_name][1].api_id + != latest_libraries[library_name][1].api_id + ): + if DiffType.API_ID not in diff: + diff[DiffType.API_ID] = [] + diff[DiffType.API_ID].append(library_name) + + if ( + baseline_libraries[library_name][1].api_reference + != latest_libraries[library_name][1].api_reference + ): + if DiffType.API_REFERENCE not in diff: + diff[DiffType.API_REFERENCE] = [] + diff[DiffType.API_REFERENCE].append(library_name) + + if ( + baseline_libraries[library_name][1].codeowner_team + != latest_libraries[library_name][1].codeowner_team + ): + if DiffType.CODEOWNER_TEAM not in diff: + diff[DiffType.CODEOWNER_TEAM] = [] + diff[DiffType.CODEOWNER_TEAM].append(library_name) + if ( + baseline_libraries[library_name][1].excluded_dependencies + != latest_libraries[library_name][1].excluded_dependencies + ): + if DiffType.EXCLUDED_DEPENDENCIES not in diff: + diff[DiffType.EXCLUDED_DEPENDENCIES] = [] + diff[DiffType.EXCLUDED_DEPENDENCIES].append(library_name) + if ( + baseline_libraries[library_name][1].excluded_poms + != latest_libraries[library_name][1].excluded_poms + ): + if DiffType.EXCLUDED_POMS not in diff: + diff[DiffType.EXCLUDED_POMS] = [] + diff[DiffType.EXCLUDED_POMS].append(library_name) + + if ( + baseline_libraries[library_name][1].client_documentation + != latest_libraries[library_name][1].client_documentation + ): + if DiffType.CLIENT_DOCS not in diff: + diff[DiffType.CLIENT_DOCS] = [] + diff[DiffType.CLIENT_DOCS].append(library_name) + + if ( + baseline_libraries[library_name][1].issue_tracker + != latest_libraries[library_name][1].issue_tracker + ): + if DiffType.ISSUE_TRACKER not in diff: + diff[DiffType.ISSUE_TRACKER] = [] + diff[DiffType.ISSUE_TRACKER].append(library_name) + + if ( + baseline_libraries[library_name][1].rest_documentation + != latest_libraries[library_name][1].rest_documentation + ): + if DiffType.REST_DOCS not in diff: + diff[DiffType.REST_DOCS] = [] + diff[DiffType.REST_DOCS].append(library_name) + + if ( + baseline_libraries[library_name][1].rpc_documentation + != latest_libraries[library_name][1].rpc_documentation + ): + if DiffType.RPC_DOCS not in diff: + diff[DiffType.RPC_DOCS] = [] + diff[DiffType.RPC_DOCS].append(library_name) + + if ( + baseline_libraries[library_name][1].requires_billing + != latest_libraries[library_name][1].requires_billing + ): + if DiffType.REQUIRES_BILLING not in diff: + diff[DiffType.REQUIRES_BILLING] = [] + diff[DiffType.REQUIRES_BILLING].append(library_name) + + if ( + baseline_libraries[library_name][1].extra_versioned_modules + != latest_libraries[library_name][1].extra_versioned_modules + ): + if DiffType.EXTRA_VERSIONED_MODULES not in diff: + diff[DiffType.EXTRA_VERSIONED_MODULES] = [] + diff[DiffType.EXTRA_VERSIONED_MODULES].append(library_name) + # compare gapic_configs From 7cb2e97831342ea1e3a074e0caa1fe28d8c40d19 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 20:16:20 +0000 Subject: [PATCH 03/21] code refactor --- .../utils/generation_config_comparator.py | 144 +++++++++--------- 1 file changed, 72 insertions(+), 72 deletions(-) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 6d90653de3..11dd987af8 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -19,7 +19,7 @@ from library_generation.utilities import get_library_name -class DiffType(Enum): +class ChangeType(Enum): GOOGLEAPIS_COMMIT = 1 GENERATOR = 2 OWLBOT_CLI = 3 @@ -49,7 +49,7 @@ class DiffType(Enum): def compare_config( path_to_baseline_config_yaml: str, path_to_latest_config_yaml: str -) -> Dict[DiffType, List[str]]: +) -> Dict[ChangeType, List[str]]: """ Compare two GenerationConfig object and output a mapping from DiffType to a list of library_name of affected libraries. @@ -66,21 +66,21 @@ def compare_config( baseline_config = from_yaml(path_to_baseline_config_yaml) latest_config = from_yaml(path_to_latest_config_yaml) if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: - diff[DiffType.GOOGLEAPIS_COMMIT] = [] + diff[ChangeType.GOOGLEAPIS_COMMIT] = [] if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: - diff[DiffType.GENERATOR] = [] + diff[ChangeType.GENERATOR] = [] if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: - diff[DiffType.OWLBOT_CLI] = [] + diff[ChangeType.OWLBOT_CLI] = [] if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: - diff[DiffType.SYNTHTOOL] = [] + diff[ChangeType.SYNTHTOOL] = [] if baseline_config.protobuf_version != latest_config.protobuf_version: - diff[DiffType.PROTOBUF] = [] + diff[ChangeType.PROTOBUF] = [] if baseline_config.grpc_version != latest_config.grpc_version: - diff[DiffType.GRPC] = [] + diff[ChangeType.GRPC] = [] if baseline_config.template_excludes != latest_config.template_excludes: - diff[DiffType.TEMPLATE_EXCLUDES] = [] + diff[ChangeType.TEMPLATE_EXCLUDES] = [] - compare_libraries( + __compare_libraries( diff=diff, baseline_library_configs=baseline_config.libraries, latest_library_configs=latest_config.libraries, @@ -88,13 +88,13 @@ def compare_config( return diff -def compare_libraries( - diff: Dict[DiffType, List[str]], +def __compare_libraries( + diff: Dict[ChangeType, List[str]], baseline_library_configs: List[LibraryConfig], latest_library_configs: List[LibraryConfig], ) -> None: - baseline_libraries = convert(baseline_library_configs) - latest_libraries = convert(latest_library_configs) + baseline_libraries = __convert(baseline_library_configs) + latest_libraries = __convert(latest_library_configs) changed_libraries = [] # 1st round comparison. for library_name, (hash_value, _) in baseline_libraries: @@ -102,9 +102,9 @@ def compare_libraries( # a library is removed from baseline_libraries if the library_name # is not in latest_libraries. if library_name not in latest_libraries: - if DiffType.LIBRARIES_REMOVAL not in diff: - diff[DiffType.LIBRARIES_REMOVAL] = [] - diff[DiffType.LIBRARIES_REMOVAL].append(library_name) + if ChangeType.LIBRARIES_REMOVAL not in diff: + diff[ChangeType.LIBRARIES_REMOVAL] = [] + diff[ChangeType.LIBRARIES_REMOVAL].append(library_name) # 2. find any library that exists in both configs but at least one # parameter is changed, which means the hash value is different. if ( @@ -118,11 +118,11 @@ def compare_libraries( # a library is added to latest_libraries if the library_name # is not in baseline_libraries. if library_name not in baseline_libraries: - if DiffType.LIBRARIES_ADDITION not in diff: - diff[DiffType.LIBRARIES_ADDITION] = [] - diff[DiffType.LIBRARIES_ADDITION].append(library_name) + if ChangeType.LIBRARIES_ADDITION not in diff: + diff[ChangeType.LIBRARIES_ADDITION] = [] + diff[ChangeType.LIBRARIES_ADDITION].append(library_name) # 3rd round comparison. - compare_changed_libraries( + __compare_changed_libraries( diff=diff, baseline_libraries=baseline_libraries, latest_libraries=latest_libraries, @@ -130,7 +130,7 @@ def compare_libraries( ) -def convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)]: +def __convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)]: """ Convert a list of LibraryConfig objects to a Dict. For each library object, the key is the library_name of the object, the @@ -144,8 +144,8 @@ def convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)]: } -def compare_changed_libraries( - diff: Dict[DiffType, List[str]], +def __compare_changed_libraries( + diff: Dict[ChangeType, List[str]], baseline_libraries: Dict[str, (int, LibraryConfig)], latest_libraries: Dict[str, (int, LibraryConfig)], changed_libraries: List[str], @@ -173,123 +173,123 @@ def compare_changed_libraries( baseline_libraries[library_name][1].api_description != latest_libraries[library_name][1].api_description ): - if DiffType.API_DESCRIPTION not in diff: - diff[DiffType.API_DESCRIPTION] = [] - diff[DiffType.API_DESCRIPTION].append(library_name) + if ChangeType.API_DESCRIPTION not in diff: + diff[ChangeType.API_DESCRIPTION] = [] + diff[ChangeType.API_DESCRIPTION].append(library_name) if ( baseline_libraries[library_name][1].name_pretty != latest_libraries[library_name][1].name_pretty ): - if DiffType.NAME_PRETTY not in diff: - diff[DiffType.NAME_PRETTY] = [] - diff[DiffType.NAME_PRETTY].append(library_name) + if ChangeType.NAME_PRETTY not in diff: + diff[ChangeType.NAME_PRETTY] = [] + diff[ChangeType.NAME_PRETTY].append(library_name) if ( baseline_libraries[library_name][1].product_documentation != latest_libraries[library_name][1].product_documentation ): - if DiffType.PRODUCT_DOCS not in diff: - diff[DiffType.PRODUCT_DOCS] = [] - diff[DiffType.PRODUCT_DOCS].append(library_name) + if ChangeType.PRODUCT_DOCS not in diff: + diff[ChangeType.PRODUCT_DOCS] = [] + diff[ChangeType.PRODUCT_DOCS].append(library_name) if ( baseline_libraries[library_name][1].library_type != latest_libraries[library_name][1].library_type ): - if DiffType.LIBRARY_TYPE not in diff: - diff[DiffType.LIBRARY_TYPE] = [] - diff[DiffType.LIBRARY_TYPE].append(library_name) + if ChangeType.LIBRARY_TYPE not in diff: + diff[ChangeType.LIBRARY_TYPE] = [] + diff[ChangeType.LIBRARY_TYPE].append(library_name) if ( baseline_libraries[library_name][1].release_level != latest_libraries[library_name][1].release_level ): - if DiffType.RELEASE_LEVEL not in diff: - diff[DiffType.RELEASE_LEVEL] = [] - diff[DiffType.RELEASE_LEVEL].append(library_name) + if ChangeType.RELEASE_LEVEL not in diff: + diff[ChangeType.RELEASE_LEVEL] = [] + diff[ChangeType.RELEASE_LEVEL].append(library_name) if ( baseline_libraries[library_name][1].api_id != latest_libraries[library_name][1].api_id ): - if DiffType.API_ID not in diff: - diff[DiffType.API_ID] = [] - diff[DiffType.API_ID].append(library_name) + if ChangeType.API_ID not in diff: + diff[ChangeType.API_ID] = [] + diff[ChangeType.API_ID].append(library_name) if ( baseline_libraries[library_name][1].api_reference != latest_libraries[library_name][1].api_reference ): - if DiffType.API_REFERENCE not in diff: - diff[DiffType.API_REFERENCE] = [] - diff[DiffType.API_REFERENCE].append(library_name) + if ChangeType.API_REFERENCE not in diff: + diff[ChangeType.API_REFERENCE] = [] + diff[ChangeType.API_REFERENCE].append(library_name) if ( baseline_libraries[library_name][1].codeowner_team != latest_libraries[library_name][1].codeowner_team ): - if DiffType.CODEOWNER_TEAM not in diff: - diff[DiffType.CODEOWNER_TEAM] = [] - diff[DiffType.CODEOWNER_TEAM].append(library_name) + if ChangeType.CODEOWNER_TEAM not in diff: + diff[ChangeType.CODEOWNER_TEAM] = [] + diff[ChangeType.CODEOWNER_TEAM].append(library_name) if ( baseline_libraries[library_name][1].excluded_dependencies != latest_libraries[library_name][1].excluded_dependencies ): - if DiffType.EXCLUDED_DEPENDENCIES not in diff: - diff[DiffType.EXCLUDED_DEPENDENCIES] = [] - diff[DiffType.EXCLUDED_DEPENDENCIES].append(library_name) + if ChangeType.EXCLUDED_DEPENDENCIES not in diff: + diff[ChangeType.EXCLUDED_DEPENDENCIES] = [] + diff[ChangeType.EXCLUDED_DEPENDENCIES].append(library_name) if ( baseline_libraries[library_name][1].excluded_poms != latest_libraries[library_name][1].excluded_poms ): - if DiffType.EXCLUDED_POMS not in diff: - diff[DiffType.EXCLUDED_POMS] = [] - diff[DiffType.EXCLUDED_POMS].append(library_name) + if ChangeType.EXCLUDED_POMS not in diff: + diff[ChangeType.EXCLUDED_POMS] = [] + diff[ChangeType.EXCLUDED_POMS].append(library_name) if ( baseline_libraries[library_name][1].client_documentation != latest_libraries[library_name][1].client_documentation ): - if DiffType.CLIENT_DOCS not in diff: - diff[DiffType.CLIENT_DOCS] = [] - diff[DiffType.CLIENT_DOCS].append(library_name) + if ChangeType.CLIENT_DOCS not in diff: + diff[ChangeType.CLIENT_DOCS] = [] + diff[ChangeType.CLIENT_DOCS].append(library_name) if ( baseline_libraries[library_name][1].issue_tracker != latest_libraries[library_name][1].issue_tracker ): - if DiffType.ISSUE_TRACKER not in diff: - diff[DiffType.ISSUE_TRACKER] = [] - diff[DiffType.ISSUE_TRACKER].append(library_name) + if ChangeType.ISSUE_TRACKER not in diff: + diff[ChangeType.ISSUE_TRACKER] = [] + diff[ChangeType.ISSUE_TRACKER].append(library_name) if ( baseline_libraries[library_name][1].rest_documentation != latest_libraries[library_name][1].rest_documentation ): - if DiffType.REST_DOCS not in diff: - diff[DiffType.REST_DOCS] = [] - diff[DiffType.REST_DOCS].append(library_name) + if ChangeType.REST_DOCS not in diff: + diff[ChangeType.REST_DOCS] = [] + diff[ChangeType.REST_DOCS].append(library_name) if ( baseline_libraries[library_name][1].rpc_documentation != latest_libraries[library_name][1].rpc_documentation ): - if DiffType.RPC_DOCS not in diff: - diff[DiffType.RPC_DOCS] = [] - diff[DiffType.RPC_DOCS].append(library_name) + if ChangeType.RPC_DOCS not in diff: + diff[ChangeType.RPC_DOCS] = [] + diff[ChangeType.RPC_DOCS].append(library_name) if ( baseline_libraries[library_name][1].requires_billing != latest_libraries[library_name][1].requires_billing ): - if DiffType.REQUIRES_BILLING not in diff: - diff[DiffType.REQUIRES_BILLING] = [] - diff[DiffType.REQUIRES_BILLING].append(library_name) + if ChangeType.REQUIRES_BILLING not in diff: + diff[ChangeType.REQUIRES_BILLING] = [] + diff[ChangeType.REQUIRES_BILLING].append(library_name) if ( baseline_libraries[library_name][1].extra_versioned_modules != latest_libraries[library_name][1].extra_versioned_modules ): - if DiffType.EXTRA_VERSIONED_MODULES not in diff: - diff[DiffType.EXTRA_VERSIONED_MODULES] = [] - diff[DiffType.EXTRA_VERSIONED_MODULES].append(library_name) + if ChangeType.EXTRA_VERSIONED_MODULES not in diff: + diff[ChangeType.EXTRA_VERSIONED_MODULES] = [] + diff[ChangeType.EXTRA_VERSIONED_MODULES].append(library_name) # compare gapic_configs From a8b1b265951dcff8c4004bb665f4d30ad0a3c2e7 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 20:39:31 +0000 Subject: [PATCH 04/21] add proto_path removal or addition change --- .../utils/generation_config_comparator.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 11dd987af8..66adc17134 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -14,6 +14,8 @@ from enum import Enum from typing import Dict from typing import List + +from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from library_generation.utilities import get_library_name @@ -45,6 +47,8 @@ class ChangeType(Enum): RPC_DOCS = 23 REQUIRES_BILLING = 24 EXTRA_VERSIONED_MODULES = 25 + VERSION_ADDITION = 26 + VERSION_REMOVAL = 27 def compare_config( @@ -293,3 +297,37 @@ def __compare_changed_libraries( diff[ChangeType.EXTRA_VERSIONED_MODULES] = [] diff[ChangeType.EXTRA_VERSIONED_MODULES].append(library_name) # compare gapic_configs + baseline_gapic_configs = baseline_libraries[library_name][1].gapic_configs + latest_gapic_configs = latest_libraries[library_name][1].gapic_configs + __compare_gapic_configs( + diff=diff, + library_name=library_name, + baseline_gapic_configs=baseline_gapic_configs, + latest_gapic_configs=latest_gapic_configs, + ) + + +def __compare_gapic_configs( + diff: Dict[ChangeType, List[str]], + library_name: str, + baseline_gapic_configs: List[GapicConfig], + latest_gapic_configs: List[GapicConfig], +) -> None: + baseline_proto_paths = {config.proto_path for config in baseline_gapic_configs} + latest_proto_paths = {config.proto_path for config in latest_gapic_configs} + # 1st round of comparison, find any versioned proto_path is removed + # from baseline gapic configs. + for proto_path in baseline_proto_paths: + if proto_path in latest_proto_paths: + continue + if ChangeType.VERSION_REMOVAL not in diff: + diff[ChangeType.VERSION_REMOVAL] = [] + diff[ChangeType.VERSION_REMOVAL].append(library_name) + # 2nd round of comparison, find any versioned proto_path is added + # to latest gapic configs. + for proto_path in latest_proto_paths: + if proto_path in baseline_proto_paths: + continue + if ChangeType.VERSION_ADDITION not in diff: + diff[ChangeType.VERSION_ADDITION] = [] + diff[ChangeType.VERSION_ADDITION].append(library_name) From 218e431665c72cca47390b8b8564b5db9d9de66a Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 22:02:44 +0000 Subject: [PATCH 05/21] change to HashLibrary --- library_generation/model/gapic_config.py | 8 -- library_generation/model/library_config.py | 54 +++++------ .../baseline_config_googleapis.yaml | 37 ++++++++ .../latest_config_googleapis.yaml | 37 ++++++++ ...generation_config_comparator_unit_tests.py | 0 .../utils/generation_config_comparator.py | 90 ++++++++++--------- 6 files changed, 151 insertions(+), 75 deletions(-) create mode 100644 library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml create mode 100644 library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml create mode 100644 library_generation/test/utils/generation_config_comparator_unit_tests.py diff --git a/library_generation/model/gapic_config.py b/library_generation/model/gapic_config.py index 91bec701d4..b90381da3d 100644 --- a/library_generation/model/gapic_config.py +++ b/library_generation/model/gapic_config.py @@ -23,11 +23,3 @@ class GapicConfig: def __init__(self, proto_path: str): self.proto_path = proto_path - - def __eq__(self, other): - return self.proto_path == other.proto_path - - def __hash__(self): - m = sha1() - m.update([self.proto_path]) - return m.hexdigest() diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py index 582c8a0d3f..e3fa970c1c 100644 --- a/library_generation/model/library_config.py +++ b/library_generation/model/library_config.py @@ -103,30 +103,32 @@ def __eq__(self, other): def __hash__(self): m = sha1() m.update( - [ - self.api_shortname, - self.api_description, - self.name_pretty, - self.product_documentation, - self.library_type, - self.release_level, - self.api_id, - self.api_reference, - self.codeowner_team, - self.excluded_dependencies, - self.excluded_poms, - self.client_documentation, - self.distribution_name, - self.googleapis_commitish, - self.group_id, - self.issue_tracker, - self.library_name, - self.rest_documentation, - self.rpc_documentation, - self.cloud_api, - self.requires_billing, - self.extra_versioned_modules, - ] - + [hash(config) for config in self.gapic_configs] + str( + [ + self.api_shortname, + self.api_description, + self.name_pretty, + self.product_documentation, + self.library_type, + self.release_level, + self.api_id, + self.api_reference, + self.codeowner_team, + self.excluded_dependencies, + self.excluded_poms, + self.client_documentation, + self.distribution_name, + self.googleapis_commitish, + self.group_id, + self.issue_tracker, + self.library_name, + self.rest_documentation, + self.rpc_documentation, + self.cloud_api, + self.requires_billing, + self.extra_versioned_modules, + ] + + [config.proto_path for config in self.gapic_configs] + ).encode("utf-8") ) - return m.hexdigest() + return int(m.hexdigest(), 16) diff --git a/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml b/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml new file mode 100644 index 0000000000..d84ed3afd2 --- /dev/null +++ b/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml @@ -0,0 +1,37 @@ +gapic_generator_version: 2.34.0 +protobuf_version: 25.2 +googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 +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 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" + 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 diff --git a/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml b/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml new file mode 100644 index 0000000000..d84ed3afd2 --- /dev/null +++ b/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml @@ -0,0 +1,37 @@ +gapic_generator_version: 2.34.0 +protobuf_version: 25.2 +googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 +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 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" + 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 diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 66adc17134..cc4423db68 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -51,6 +51,12 @@ class ChangeType(Enum): VERSION_REMOVAL = 27 +class HashLibrary: + def __init__(self, hash_value: int, library: LibraryConfig): + self.hash_value = hash_value + self.library = library + + def compare_config( path_to_baseline_config_yaml: str, path_to_latest_config_yaml: str ) -> Dict[ChangeType, List[str]]: @@ -101,7 +107,7 @@ def __compare_libraries( latest_libraries = __convert(latest_library_configs) changed_libraries = [] # 1st round comparison. - for library_name, (hash_value, _) in baseline_libraries: + for library_name, hash_library in baseline_libraries.items(): # 1. find any library removed from baseline_libraries. # a library is removed from baseline_libraries if the library_name # is not in latest_libraries. @@ -113,11 +119,11 @@ def __compare_libraries( # parameter is changed, which means the hash value is different. if ( library_name in latest_libraries - and hash_value != latest_libraries[library_name][0] + and hash_library.hash_value != latest_libraries[library_name].hash_value ): changed_libraries.append(library_name) # 2nd round comparison. - for library_name, (hash_value, _) in latest_libraries: + for library_name in latest_libraries: # find any library added to latest_libraries. # a library is added to latest_libraries if the library_name # is not in baseline_libraries. @@ -134,7 +140,7 @@ def __compare_libraries( ) -def __convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)]: +def __convert(libraries: List[LibraryConfig]) -> Dict[str, HashLibrary]: """ Convert a list of LibraryConfig objects to a Dict. For each library object, the key is the library_name of the object, the @@ -144,14 +150,14 @@ def __convert(libraries: List[LibraryConfig]) -> Dict[str, (int, LibraryConfig)] :return: """ return { - get_library_name(library): (hash(library), library) for library in libraries + get_library_name(library): HashLibrary(hash(library), library) for library in libraries } def __compare_changed_libraries( diff: Dict[ChangeType, List[str]], - baseline_libraries: Dict[str, (int, LibraryConfig)], - latest_libraries: Dict[str, (int, LibraryConfig)], + baseline_libraries: Dict[str, HashLibrary], + latest_libraries: Dict[str, HashLibrary], changed_libraries: List[str], ) -> None: """ @@ -173,132 +179,134 @@ def __compare_changed_libraries( :return: """ for library_name in changed_libraries: + baseline_library = baseline_libraries[library_name].library + latest_library = latest_libraries[library_name].library if ( - baseline_libraries[library_name][1].api_description - != latest_libraries[library_name][1].api_description + baseline_library.api_description + != latest_libraries[library_name].library.api_description ): if ChangeType.API_DESCRIPTION not in diff: diff[ChangeType.API_DESCRIPTION] = [] diff[ChangeType.API_DESCRIPTION].append(library_name) if ( - baseline_libraries[library_name][1].name_pretty - != latest_libraries[library_name][1].name_pretty + baseline_library.name_pretty + != latest_libraries[library_name].library.name_pretty ): if ChangeType.NAME_PRETTY not in diff: diff[ChangeType.NAME_PRETTY] = [] diff[ChangeType.NAME_PRETTY].append(library_name) if ( - baseline_libraries[library_name][1].product_documentation - != latest_libraries[library_name][1].product_documentation + baseline_library.product_documentation + != latest_library.product_documentation ): if ChangeType.PRODUCT_DOCS not in diff: diff[ChangeType.PRODUCT_DOCS] = [] diff[ChangeType.PRODUCT_DOCS].append(library_name) if ( - baseline_libraries[library_name][1].library_type - != latest_libraries[library_name][1].library_type + baseline_library.library_type + != latest_library.library_type ): if ChangeType.LIBRARY_TYPE not in diff: diff[ChangeType.LIBRARY_TYPE] = [] diff[ChangeType.LIBRARY_TYPE].append(library_name) if ( - baseline_libraries[library_name][1].release_level - != latest_libraries[library_name][1].release_level + baseline_library.release_level + != latest_library.release_level ): if ChangeType.RELEASE_LEVEL not in diff: diff[ChangeType.RELEASE_LEVEL] = [] diff[ChangeType.RELEASE_LEVEL].append(library_name) if ( - baseline_libraries[library_name][1].api_id - != latest_libraries[library_name][1].api_id + baseline_library.api_id + != latest_library.api_id ): if ChangeType.API_ID not in diff: diff[ChangeType.API_ID] = [] diff[ChangeType.API_ID].append(library_name) if ( - baseline_libraries[library_name][1].api_reference - != latest_libraries[library_name][1].api_reference + baseline_library.api_reference + != latest_library.api_reference ): if ChangeType.API_REFERENCE not in diff: diff[ChangeType.API_REFERENCE] = [] diff[ChangeType.API_REFERENCE].append(library_name) if ( - baseline_libraries[library_name][1].codeowner_team - != latest_libraries[library_name][1].codeowner_team + baseline_library.codeowner_team + != latest_library.codeowner_team ): if ChangeType.CODEOWNER_TEAM not in diff: diff[ChangeType.CODEOWNER_TEAM] = [] diff[ChangeType.CODEOWNER_TEAM].append(library_name) if ( - baseline_libraries[library_name][1].excluded_dependencies - != latest_libraries[library_name][1].excluded_dependencies + baseline_library.excluded_dependencies + != latest_library.excluded_dependencies ): if ChangeType.EXCLUDED_DEPENDENCIES not in diff: diff[ChangeType.EXCLUDED_DEPENDENCIES] = [] diff[ChangeType.EXCLUDED_DEPENDENCIES].append(library_name) if ( - baseline_libraries[library_name][1].excluded_poms - != latest_libraries[library_name][1].excluded_poms + baseline_library.excluded_poms + != latest_library.excluded_poms ): if ChangeType.EXCLUDED_POMS not in diff: diff[ChangeType.EXCLUDED_POMS] = [] diff[ChangeType.EXCLUDED_POMS].append(library_name) if ( - baseline_libraries[library_name][1].client_documentation - != latest_libraries[library_name][1].client_documentation + baseline_library.client_documentation + != latest_library.client_documentation ): if ChangeType.CLIENT_DOCS not in diff: diff[ChangeType.CLIENT_DOCS] = [] diff[ChangeType.CLIENT_DOCS].append(library_name) if ( - baseline_libraries[library_name][1].issue_tracker - != latest_libraries[library_name][1].issue_tracker + baseline_library.issue_tracker + != latest_library.issue_tracker ): if ChangeType.ISSUE_TRACKER not in diff: diff[ChangeType.ISSUE_TRACKER] = [] diff[ChangeType.ISSUE_TRACKER].append(library_name) if ( - baseline_libraries[library_name][1].rest_documentation - != latest_libraries[library_name][1].rest_documentation + baseline_library.rest_documentation + != latest_library.rest_documentation ): if ChangeType.REST_DOCS not in diff: diff[ChangeType.REST_DOCS] = [] diff[ChangeType.REST_DOCS].append(library_name) if ( - baseline_libraries[library_name][1].rpc_documentation - != latest_libraries[library_name][1].rpc_documentation + baseline_library.rpc_documentation + != latest_library.rpc_documentation ): if ChangeType.RPC_DOCS not in diff: diff[ChangeType.RPC_DOCS] = [] diff[ChangeType.RPC_DOCS].append(library_name) if ( - baseline_libraries[library_name][1].requires_billing - != latest_libraries[library_name][1].requires_billing + baseline_library.requires_billing + != latest_library.requires_billing ): if ChangeType.REQUIRES_BILLING not in diff: diff[ChangeType.REQUIRES_BILLING] = [] diff[ChangeType.REQUIRES_BILLING].append(library_name) if ( - baseline_libraries[library_name][1].extra_versioned_modules - != latest_libraries[library_name][1].extra_versioned_modules + baseline_library.extra_versioned_modules + != latest_library.extra_versioned_modules ): if ChangeType.EXTRA_VERSIONED_MODULES not in diff: diff[ChangeType.EXTRA_VERSIONED_MODULES] = [] diff[ChangeType.EXTRA_VERSIONED_MODULES].append(library_name) # compare gapic_configs - baseline_gapic_configs = baseline_libraries[library_name][1].gapic_configs - latest_gapic_configs = latest_libraries[library_name][1].gapic_configs + baseline_gapic_configs = baseline_library.gapic_configs + latest_gapic_configs = latest_library.gapic_configs __compare_gapic_configs( diff=diff, library_name=library_name, From 15e0db75ebc55b3f9f1d6a2a32a078f99818f3a5 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 22:14:31 +0000 Subject: [PATCH 06/21] use object, rather than file path in comparator interface --- .../baseline_config_googleapis.yaml | 37 ---------- .../latest_config_googleapis.yaml | 37 ---------- .../utils/generation_config_comparator.py | 69 +++++-------------- 3 files changed, 17 insertions(+), 126 deletions(-) delete mode 100644 library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml delete mode 100644 library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml diff --git a/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml b/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml deleted file mode 100644 index d84ed3afd2..0000000000 --- a/library_generation/test/resources/test_generation_config_comparator/baseline_config_googleapis.yaml +++ /dev/null @@ -1,37 +0,0 @@ -gapic_generator_version: 2.34.0 -protobuf_version: 25.2 -googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 -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 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" - 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 diff --git a/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml b/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml deleted file mode 100644 index d84ed3afd2..0000000000 --- a/library_generation/test/resources/test_generation_config_comparator/latest_config_googleapis.yaml +++ /dev/null @@ -1,37 +0,0 @@ -gapic_generator_version: 2.34.0 -protobuf_version: 25.2 -googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 -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 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" - 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 diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index cc4423db68..ee9391df3f 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -16,6 +16,7 @@ from typing import List from library_generation.model.gapic_config import GapicConfig +from library_generation.model.generation_config import GenerationConfig from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from library_generation.utilities import get_library_name @@ -58,7 +59,7 @@ def __init__(self, hash_value: int, library: LibraryConfig): def compare_config( - path_to_baseline_config_yaml: str, path_to_latest_config_yaml: str + baseline_config: GenerationConfig, latest_config: GenerationConfig ) -> Dict[ChangeType, List[str]]: """ Compare two GenerationConfig object and output a mapping from DiffType @@ -66,15 +67,11 @@ def compare_config( All libraries in the latest configuration will be affected if the library list is empty. - :param path_to_baseline_config_yaml: the path to the baseline configuration - file. - :param path_to_latest_config_yaml: the path to the latest configuration - file. + :param baseline_config: + :param latest_config: :return: """ diff = {} - baseline_config = from_yaml(path_to_baseline_config_yaml) - latest_config = from_yaml(path_to_latest_config_yaml) if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: diff[ChangeType.GOOGLEAPIS_COMMIT] = [] if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: @@ -150,7 +147,8 @@ def __convert(libraries: List[LibraryConfig]) -> Dict[str, HashLibrary]: :return: """ return { - get_library_name(library): HashLibrary(hash(library), library) for library in libraries + get_library_name(library): HashLibrary(hash(library), library) + for library in libraries } @@ -203,42 +201,27 @@ def __compare_changed_libraries( diff[ChangeType.PRODUCT_DOCS] = [] diff[ChangeType.PRODUCT_DOCS].append(library_name) - if ( - baseline_library.library_type - != latest_library.library_type - ): + if baseline_library.library_type != latest_library.library_type: if ChangeType.LIBRARY_TYPE not in diff: diff[ChangeType.LIBRARY_TYPE] = [] diff[ChangeType.LIBRARY_TYPE].append(library_name) - if ( - baseline_library.release_level - != latest_library.release_level - ): + if baseline_library.release_level != latest_library.release_level: if ChangeType.RELEASE_LEVEL not in diff: diff[ChangeType.RELEASE_LEVEL] = [] diff[ChangeType.RELEASE_LEVEL].append(library_name) - if ( - baseline_library.api_id - != latest_library.api_id - ): + if baseline_library.api_id != latest_library.api_id: if ChangeType.API_ID not in diff: diff[ChangeType.API_ID] = [] diff[ChangeType.API_ID].append(library_name) - if ( - baseline_library.api_reference - != latest_library.api_reference - ): + if baseline_library.api_reference != latest_library.api_reference: if ChangeType.API_REFERENCE not in diff: diff[ChangeType.API_REFERENCE] = [] diff[ChangeType.API_REFERENCE].append(library_name) - if ( - baseline_library.codeowner_team - != latest_library.codeowner_team - ): + if baseline_library.codeowner_team != latest_library.codeowner_team: if ChangeType.CODEOWNER_TEAM not in diff: diff[ChangeType.CODEOWNER_TEAM] = [] diff[ChangeType.CODEOWNER_TEAM].append(library_name) @@ -249,50 +232,32 @@ def __compare_changed_libraries( if ChangeType.EXCLUDED_DEPENDENCIES not in diff: diff[ChangeType.EXCLUDED_DEPENDENCIES] = [] diff[ChangeType.EXCLUDED_DEPENDENCIES].append(library_name) - if ( - baseline_library.excluded_poms - != latest_library.excluded_poms - ): + if baseline_library.excluded_poms != latest_library.excluded_poms: if ChangeType.EXCLUDED_POMS not in diff: diff[ChangeType.EXCLUDED_POMS] = [] diff[ChangeType.EXCLUDED_POMS].append(library_name) - if ( - baseline_library.client_documentation - != latest_library.client_documentation - ): + if baseline_library.client_documentation != latest_library.client_documentation: if ChangeType.CLIENT_DOCS not in diff: diff[ChangeType.CLIENT_DOCS] = [] diff[ChangeType.CLIENT_DOCS].append(library_name) - if ( - baseline_library.issue_tracker - != latest_library.issue_tracker - ): + if baseline_library.issue_tracker != latest_library.issue_tracker: if ChangeType.ISSUE_TRACKER not in diff: diff[ChangeType.ISSUE_TRACKER] = [] diff[ChangeType.ISSUE_TRACKER].append(library_name) - if ( - baseline_library.rest_documentation - != latest_library.rest_documentation - ): + if baseline_library.rest_documentation != latest_library.rest_documentation: if ChangeType.REST_DOCS not in diff: diff[ChangeType.REST_DOCS] = [] diff[ChangeType.REST_DOCS].append(library_name) - if ( - baseline_library.rpc_documentation - != latest_library.rpc_documentation - ): + if baseline_library.rpc_documentation != latest_library.rpc_documentation: if ChangeType.RPC_DOCS not in diff: diff[ChangeType.RPC_DOCS] = [] diff[ChangeType.RPC_DOCS].append(library_name) - if ( - baseline_library.requires_billing - != latest_library.requires_billing - ): + if baseline_library.requires_billing != latest_library.requires_billing: if ChangeType.REQUIRES_BILLING not in diff: diff[ChangeType.REQUIRES_BILLING] = [] diff[ChangeType.REQUIRES_BILLING].append(library_name) From b87635444428baca580d4ab64cef58c88d990897 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 22:14:44 +0000 Subject: [PATCH 07/21] add unit tests --- ...generation_config_comparator_unit_tests.py | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index e69de29bb2..8a7f5efc77 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -0,0 +1,70 @@ +# 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.generation_config import GenerationConfig +from library_generation.utils.generation_config_comparator import ChangeType +from library_generation.utils.generation_config_comparator import compare_config + +baseline_config = GenerationConfig( + gapic_generator_version="", + googleapis_commitish="", + owlbot_cli_image="", + synthtool_commitish="", + template_excludes=[], + path_to_yaml="", + grpc_version="", + protobuf_version="", + libraries=[] +) +latest_config = GenerationConfig( + gapic_generator_version="", + googleapis_commitish="", + owlbot_cli_image="", + synthtool_commitish="", + template_excludes=[], + path_to_yaml="", + grpc_version="", + protobuf_version="", + libraries=[] +) + + +class GenerationConfigComparatorTest(unittest.TestCase): + def test_compare_config_not_change(self): + result = compare_config( + baseline_config=baseline_config, + latest_config=latest_config, + ) + self.assertTrue(len(result) == 0) + + def test_compare_config_googleapis_update(self): + baseline_config.googleapis_commitish = "1a45bf7393b52407188c82e63101db7dc9c72026" + latest_config.googleapis_commitish = "1e6517ef4f949191c9e471857cf5811c8abcab84" + result = compare_config( + baseline_config=baseline_config, + latest_config=latest_config, + ) + self.assertTrue(ChangeType.GOOGLEAPIS_COMMIT in result) + self.assertEqual([], result[ChangeType.GOOGLEAPIS_COMMIT]) + + def test_compare_config_generator_update(self): + baseline_config.gapic_generator_version = "1.2.3" + latest_config.gapic_generator_version = "1.2.4" + result = compare_config( + baseline_config=baseline_config, + latest_config=latest_config, + ) + self.assertTrue(ChangeType.GENERATOR in result) + self.assertEqual([], result[ChangeType.GENERATOR]) \ No newline at end of file From 5348539342edc510c932034cfa7b49bae45ae993 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 22:23:09 +0000 Subject: [PATCH 08/21] add setUp method in ut --- ...generation_config_comparator_unit_tests.py | 73 ++++++++++--------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 8a7f5efc77..f5de5d9f35 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -17,54 +17,59 @@ from library_generation.utils.generation_config_comparator import ChangeType from library_generation.utils.generation_config_comparator import compare_config -baseline_config = GenerationConfig( - gapic_generator_version="", - googleapis_commitish="", - owlbot_cli_image="", - synthtool_commitish="", - template_excludes=[], - path_to_yaml="", - grpc_version="", - protobuf_version="", - libraries=[] -) -latest_config = GenerationConfig( - gapic_generator_version="", - googleapis_commitish="", - owlbot_cli_image="", - synthtool_commitish="", - template_excludes=[], - path_to_yaml="", - grpc_version="", - protobuf_version="", - libraries=[] -) - class GenerationConfigComparatorTest(unittest.TestCase): + def setUp(self) -> None: + self.baseline_config = GenerationConfig( + gapic_generator_version="", + googleapis_commitish="", + owlbot_cli_image="", + synthtool_commitish="", + template_excludes=[], + path_to_yaml="", + grpc_version="", + protobuf_version="", + libraries=[], + ) + self.latest_config = GenerationConfig( + gapic_generator_version="", + googleapis_commitish="", + owlbot_cli_image="", + synthtool_commitish="", + template_excludes=[], + path_to_yaml="", + grpc_version="", + protobuf_version="", + libraries=[], + ) + def test_compare_config_not_change(self): result = compare_config( - baseline_config=baseline_config, - latest_config=latest_config, + baseline_config=self.baseline_config, + latest_config=self.latest_config, ) self.assertTrue(len(result) == 0) def test_compare_config_googleapis_update(self): - baseline_config.googleapis_commitish = "1a45bf7393b52407188c82e63101db7dc9c72026" - latest_config.googleapis_commitish = "1e6517ef4f949191c9e471857cf5811c8abcab84" + self.baseline_config.googleapis_commitish = ( + "1a45bf7393b52407188c82e63101db7dc9c72026" + ) + self.latest_config.googleapis_commitish = ( + "1e6517ef4f949191c9e471857cf5811c8abcab84" + ) result = compare_config( - baseline_config=baseline_config, - latest_config=latest_config, + baseline_config=self.baseline_config, + latest_config=self.latest_config, ) self.assertTrue(ChangeType.GOOGLEAPIS_COMMIT in result) self.assertEqual([], result[ChangeType.GOOGLEAPIS_COMMIT]) def test_compare_config_generator_update(self): - baseline_config.gapic_generator_version = "1.2.3" - latest_config.gapic_generator_version = "1.2.4" + self.baseline_config.gapic_generator_version = "1.2.3" + self.latest_config.gapic_generator_version = "1.2.4" result = compare_config( - baseline_config=baseline_config, - latest_config=latest_config, + baseline_config=self.baseline_config, + latest_config=self.latest_config, ) self.assertTrue(ChangeType.GENERATOR in result) - self.assertEqual([], result[ChangeType.GENERATOR]) \ No newline at end of file + self.assertEqual([], result[ChangeType.GENERATOR]) From 8e6b1c197cd7aac38b15d43efc77432931df88dd Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 22:39:55 +0000 Subject: [PATCH 09/21] add unit tests against repo level changes --- ...generation_config_comparator_unit_tests.py | 56 +++++++++++++++++-- .../utils/generation_config_comparator.py | 1 - 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index f5de5d9f35..8b08496ee5 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -61,8 +61,7 @@ def test_compare_config_googleapis_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(ChangeType.GOOGLEAPIS_COMMIT in result) - self.assertEqual([], result[ChangeType.GOOGLEAPIS_COMMIT]) + self.assertEqual({ChangeType.GOOGLEAPIS_COMMIT: []}, result) def test_compare_config_generator_update(self): self.baseline_config.gapic_generator_version = "1.2.3" @@ -71,5 +70,54 @@ def test_compare_config_generator_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(ChangeType.GENERATOR in result) - self.assertEqual([], result[ChangeType.GENERATOR]) + self.assertEqual({ChangeType.GENERATOR: []}, result) + + def test_compare_config_owlbot_cli_update(self): + self.baseline_config.owlbot_cli_image = "image_version_123" + self.latest_config.owlbot_cli_image = "image_version_456" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.OWLBOT_CLI: []}, result) + + def test_compare_config_synthtool_update(self): + self.baseline_config.synthtool_commitish = "commit123" + self.latest_config.synthtool_commitish = "commit456" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.SYNTHTOOL: []}, result) + + def test_compare_protobuf_update(self): + self.baseline_config.protobuf_version = "3.25.2" + self.latest_config.protobuf_version = "3.27.0" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.PROTOBUF: []}, result) + + def test_compare_config_grpc_update(self): + self.baseline_config.grpc_version = "1.60.0" + self.latest_config.grpc_version = "1.61.0" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.GRPC: []}, result) + + def test_compare_config_template_excludes_update(self): + self.baseline_config.template_excludes = [".github/*", ".kokoro/*"] + self.latest_config.template_excludes = [ + ".github/*", + ".kokoro/*", + "samples/*", + "CODE_OF_CONDUCT.md", + ] + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.TEMPLATE_EXCLUDES: []}, result) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index ee9391df3f..2c6cf73498 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -17,7 +17,6 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig -from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from library_generation.utilities import get_library_name From 4f0f2c1dd4e9ed6b014d1d6f9ff7ebb457b3986e Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 23:26:31 +0000 Subject: [PATCH 10/21] add unit tests against library parameters --- ...generation_config_comparator_unit_tests.py | 175 +++++++++++++++++- 1 file changed, 173 insertions(+), 2 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 8b08496ee5..2fe9144b88 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -14,12 +14,27 @@ import unittest from library_generation.model.generation_config import GenerationConfig +from library_generation.model.library_config import LibraryConfig from library_generation.utils.generation_config_comparator import ChangeType from library_generation.utils.generation_config_comparator import compare_config class GenerationConfigComparatorTest(unittest.TestCase): def setUp(self) -> None: + self.baseline_library = LibraryConfig( + api_shortname="existing_library", + api_description="", + name_pretty="", + product_documentation="", + gapic_configs=[], + ) + self.latest_library = LibraryConfig( + api_shortname="existing_library", + api_description="", + name_pretty="", + product_documentation="", + gapic_configs=[], + ) self.baseline_config = GenerationConfig( gapic_generator_version="", googleapis_commitish="", @@ -29,7 +44,7 @@ def setUp(self) -> None: path_to_yaml="", grpc_version="", protobuf_version="", - libraries=[], + libraries=[self.baseline_library], ) self.latest_config = GenerationConfig( gapic_generator_version="", @@ -40,7 +55,7 @@ def setUp(self) -> None: path_to_yaml="", grpc_version="", protobuf_version="", - libraries=[], + libraries=[self.latest_library], ) def test_compare_config_not_change(self): @@ -121,3 +136,159 @@ def test_compare_config_template_excludes_update(self): latest_config=self.latest_config, ) self.assertEqual({ChangeType.TEMPLATE_EXCLUDES: []}, result) + + def test_compare_config_library_addition(self): + self.latest_config.libraries.append( + LibraryConfig( + api_shortname="new_library", + api_description="", + name_pretty="", + product_documentation="", + gapic_configs=[], + ) + ) + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.LIBRARIES_ADDITION: ["new_library"]}, result) + + def test_compare_config_library_removal(self): + self.latest_config.libraries = [] + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.LIBRARIES_REMOVAL: ["existing_library"]}, result) + + def test_compare_config_api_description_update(self): + self.latest_config.libraries[0].api_description = "updated description" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.API_DESCRIPTION: ["existing_library"]}, result) + + def test_compare_config_name_pretty_update(self): + self.latest_config.libraries[0].name_pretty = "new name" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.NAME_PRETTY: ["existing_library"]}, result) + + def test_compare_config_product_docs_update(self): + self.latest_config.libraries[0].product_documentation = "new docs" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.PRODUCT_DOCS: ["existing_library"]}, result) + + def test_compare_config_library_type_update(self): + self.latest_config.libraries[0].library_type = "GAPIC_COMBO" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.LIBRARY_TYPE: ["existing_library"]}, result) + + def test_compare_config_release_level_update(self): + self.latest_config.libraries[0].release_level = "STABLE" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.RELEASE_LEVEL: ["existing_library"]}, result) + + def test_compare_config_api_id_update(self): + self.latest_config.libraries[0].api_id = "new_id" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.API_ID: ["existing_library"]}, result) + + def test_compare_config_api_reference_update(self): + self.latest_config.libraries[0].api_reference = "new api_reference" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.API_REFERENCE: ["existing_library"]}, result) + + def test_compare_config_code_owner_team_update(self): + self.latest_config.libraries[0].codeowner_team = "new team" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.CODEOWNER_TEAM: ["existing_library"]}, result) + + def test_compare_config_excluded_deps_update(self): + self.latest_config.libraries[0].excluded_dependencies = "group:artifact" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual( + {ChangeType.EXCLUDED_DEPENDENCIES: ["existing_library"]}, result + ) + + def test_compare_config_excluded_poms_update(self): + self.latest_config.libraries[0].excluded_poms = "pom.xml" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.EXCLUDED_POMS: ["existing_library"]}, result) + + def test_compare_config_client_docs_update(self): + self.latest_config.libraries[0].client_documentation = "new client docs" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.CLIENT_DOCS: ["existing_library"]}, result) + + def test_compare_config_issue_tracker_update(self): + self.latest_config.libraries[0].issue_tracker = "new issue tracker" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.ISSUE_TRACKER: ["existing_library"]}, result) + + def test_compare_config_rest_docs_update(self): + self.latest_config.libraries[0].rest_documentation = "new rest docs" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.REST_DOCS: ["existing_library"]}, result) + + def test_compare_config_rpc_docs_update(self): + self.latest_config.libraries[0].rpc_documentation = "new rpc docs" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.RPC_DOCS: ["existing_library"]}, result) + + def test_compare_config_requires_billing_update(self): + self.latest_config.libraries[0].requires_billing = False + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.REQUIRES_BILLING: ["existing_library"]}, result) + + def test_compare_config_extra_versioned_mod_update(self): + self.latest_config.libraries[0].extra_versioned_modules = "extra module" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual( + {ChangeType.EXTRA_VERSIONED_MODULES: ["existing_library"]}, result + ) From e8b2d6bccf440a8e619e328420374b4420555e8c Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 23:31:13 +0000 Subject: [PATCH 11/21] add unit tests against versioned proto_path --- ...generation_config_comparator_unit_tests.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 2fe9144b88..35e37e0eea 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_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.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig from library_generation.utils.generation_config_comparator import ChangeType @@ -292,3 +293,23 @@ def test_compare_config_extra_versioned_mod_update(self): self.assertEqual( {ChangeType.EXTRA_VERSIONED_MODULES: ["existing_library"]}, result ) + + def test_compare_config_version_addition(self): + self.latest_config.libraries[0].gapic_configs = [ + GapicConfig(proto_path="google/new/library/v1") + ] + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.VERSION_ADDITION: ["existing_library"]}, result) + + def test_compare_config_version_removal(self): + self.baseline_config.libraries[0].gapic_configs = [ + GapicConfig(proto_path="google/old/library/v1") + ] + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertEqual({ChangeType.VERSION_REMOVAL: ["existing_library"]}, result) From e5f7a5e5b2eabea1434a262df5982cd983cec9d8 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 17 Mar 2024 23:38:47 +0000 Subject: [PATCH 12/21] restore gapic_config --- library_generation/model/gapic_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/library_generation/model/gapic_config.py b/library_generation/model/gapic_config.py index b90381da3d..bec1645823 100644 --- a/library_generation/model/gapic_config.py +++ b/library_generation/model/gapic_config.py @@ -12,7 +12,6 @@ # 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. -from hashlib import sha1 class GapicConfig: From 2d6b13b1365b83b9afed9148dca9a9c078feec9f Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 18 Mar 2024 13:38:03 +0000 Subject: [PATCH 13/21] add comments --- .../utils/generation_config_comparator.py | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 2c6cf73498..c2a2923b1b 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -52,6 +52,10 @@ class ChangeType(Enum): class HashLibrary: + """ + Data class to group a LibraryConfig object and its hash value together. + """ + def __init__(self, hash_value: int, library: LibraryConfig): self.hash_value = hash_value self.library = library @@ -61,14 +65,15 @@ def compare_config( baseline_config: GenerationConfig, latest_config: GenerationConfig ) -> Dict[ChangeType, List[str]]: """ - Compare two GenerationConfig object and output a mapping from DiffType + Compare two GenerationConfig object and output a mapping from ChangeType to a list of library_name of affected libraries. All libraries in the latest configuration will be affected if the library list is empty. - :param baseline_config: - :param latest_config: - :return: + :param baseline_config: the baseline GenerationConfig object + :param latest_config: the latest GenerationConfig object + :return: a mapping from ChangeType to a list of library_name of affected + libraries. """ diff = {} if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: @@ -99,6 +104,15 @@ def __compare_libraries( baseline_library_configs: List[LibraryConfig], latest_library_configs: List[LibraryConfig], ) -> None: + """ + Compare two lists of LibraryConfig and put the difference into a + given Dict. + + :param diff: a mapping from ChangeType to a list of library_name of + affected libraries. + :param baseline_library_configs: a list of LibraryConfig object + :param latest_library_configs: a list of LibraryConfig object + """ baseline_libraries = __convert(baseline_library_configs) latest_libraries = __convert(latest_library_configs) changed_libraries = [] @@ -140,10 +154,10 @@ def __convert(libraries: List[LibraryConfig]) -> Dict[str, HashLibrary]: """ Convert a list of LibraryConfig objects to a Dict. For each library object, the key is the library_name of the object, the - value is a tuple, which contains the hash value of the object and the object - itself. - :param libraries: - :return: + value is a HashLibrary object. + + :param libraries: a list of LibraryConfig object. + :return: a mapping from library_name to HashLibrary object. """ return { get_library_name(library): HashLibrary(hash(library), library) @@ -169,11 +183,12 @@ def __compare_changed_libraries( - group_id - cloud_api - :param diff: - :param baseline_libraries: - :param latest_libraries: - :param changed_libraries: - :return: + :param diff: a mapping from ChangeType to a list of library_name of + affected libraries. + :param baseline_libraries: a mapping from library_name to HashLibrary + object. + :param latest_libraries: a mapping from library_name to HashLibrary object. + :param changed_libraries: a list of library_name of changed libraries. """ for library_name in changed_libraries: baseline_library = baseline_libraries[library_name].library From 2dc41b32b52862f2cb5f6f7da52c0ead921a5562 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 18 Mar 2024 15:06:45 +0000 Subject: [PATCH 14/21] code refactor --- .../utils/generation_config_comparator.py | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index c2a2923b1b..d386ce8fe6 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -20,6 +20,8 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utilities import get_library_name +ALL_LIBRARIES = [] + class ChangeType(Enum): GOOGLEAPIS_COMMIT = 1 @@ -67,8 +69,8 @@ def compare_config( """ Compare two GenerationConfig object and output a mapping from ChangeType to a list of library_name of affected libraries. - All libraries in the latest configuration will be affected if the library - list is empty. + All libraries in the latest configuration will be affected if one of the + repository level parameters is changed. :param baseline_config: the baseline GenerationConfig object :param latest_config: the latest GenerationConfig object @@ -77,19 +79,19 @@ def compare_config( """ diff = {} if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: - diff[ChangeType.GOOGLEAPIS_COMMIT] = [] + diff[ChangeType.GOOGLEAPIS_COMMIT] = ALL_LIBRARIES if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: - diff[ChangeType.GENERATOR] = [] + diff[ChangeType.GENERATOR] = ALL_LIBRARIES if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: - diff[ChangeType.OWLBOT_CLI] = [] + diff[ChangeType.OWLBOT_CLI] = ALL_LIBRARIES if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: - diff[ChangeType.SYNTHTOOL] = [] + diff[ChangeType.SYNTHTOOL] = ALL_LIBRARIES if baseline_config.protobuf_version != latest_config.protobuf_version: - diff[ChangeType.PROTOBUF] = [] + diff[ChangeType.PROTOBUF] = ALL_LIBRARIES if baseline_config.grpc_version != latest_config.grpc_version: - diff[ChangeType.GRPC] = [] + diff[ChangeType.GRPC] = ALL_LIBRARIES if baseline_config.template_excludes != latest_config.template_excludes: - diff[ChangeType.TEMPLATE_EXCLUDES] = [] + diff[ChangeType.TEMPLATE_EXCLUDES] = ALL_LIBRARIES __compare_libraries( diff=diff, @@ -113,8 +115,8 @@ def __compare_libraries( :param baseline_library_configs: a list of LibraryConfig object :param latest_library_configs: a list of LibraryConfig object """ - baseline_libraries = __convert(baseline_library_configs) - latest_libraries = __convert(latest_library_configs) + baseline_libraries = __convert_to_hashed_library_dict(baseline_library_configs) + latest_libraries = __convert_to_hashed_library_dict(latest_library_configs) changed_libraries = [] # 1st round comparison. for library_name, hash_library in baseline_libraries.items(): @@ -150,7 +152,9 @@ def __compare_libraries( ) -def __convert(libraries: List[LibraryConfig]) -> Dict[str, HashLibrary]: +def __convert_to_hashed_library_dict( + libraries: List[LibraryConfig], +) -> Dict[str, HashLibrary]: """ Convert a list of LibraryConfig objects to a Dict. For each library object, the key is the library_name of the object, the From 6fba2ad407fc7e7985791581eaa5e367d7bbba94 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 18 Mar 2024 22:13:28 +0000 Subject: [PATCH 15/21] change type --- .../generation_config_comparator_unit_tests.py | 4 ++-- .../utils/generation_config_comparator.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 35e37e0eea..f4b630461c 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -302,7 +302,7 @@ def test_compare_config_version_addition(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.VERSION_ADDITION: ["existing_library"]}, result) + self.assertEqual({ChangeType.GAPIC_ADDITION: ["existing_library"]}, result) def test_compare_config_version_removal(self): self.baseline_config.libraries[0].gapic_configs = [ @@ -312,4 +312,4 @@ def test_compare_config_version_removal(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.VERSION_REMOVAL: ["existing_library"]}, result) + self.assertEqual({ChangeType.GAPIC_REMOVAL: ["existing_library"]}, result) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index d386ce8fe6..d7f892d8f5 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -49,8 +49,8 @@ class ChangeType(Enum): RPC_DOCS = 23 REQUIRES_BILLING = 24 EXTRA_VERSIONED_MODULES = 25 - VERSION_ADDITION = 26 - VERSION_REMOVAL = 27 + GAPIC_ADDITION = 26 + GAPIC_REMOVAL = 27 class HashLibrary: @@ -311,14 +311,14 @@ def __compare_gapic_configs( for proto_path in baseline_proto_paths: if proto_path in latest_proto_paths: continue - if ChangeType.VERSION_REMOVAL not in diff: - diff[ChangeType.VERSION_REMOVAL] = [] - diff[ChangeType.VERSION_REMOVAL].append(library_name) + if ChangeType.GAPIC_REMOVAL not in diff: + diff[ChangeType.GAPIC_REMOVAL] = [] + diff[ChangeType.GAPIC_REMOVAL].append(library_name) # 2nd round of comparison, find any versioned proto_path is added # to latest gapic configs. for proto_path in latest_proto_paths: if proto_path in baseline_proto_paths: continue - if ChangeType.VERSION_ADDITION not in diff: - diff[ChangeType.VERSION_ADDITION] = [] - diff[ChangeType.VERSION_ADDITION].append(library_name) + if ChangeType.GAPIC_ADDITION not in diff: + diff[ChangeType.GAPIC_ADDITION] = [] + diff[ChangeType.GAPIC_ADDITION].append(library_name) From cda24a179dd80a49a7a9a42a6ce9f7f4934236d3 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 19 Mar 2024 01:50:45 +0000 Subject: [PATCH 16/21] change enum types --- ...generation_config_comparator_unit_tests.py | 222 ++++++++++-- .../utils/generation_config_comparator.py | 327 ++++++++++-------- 2 files changed, 383 insertions(+), 166 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index f4b630461c..55e975e2f5 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -86,7 +86,10 @@ def test_compare_config_generator_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.GENERATOR: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("GAPIC generator version", config_change.changed_param) + self.assertEqual("1.2.4", config_change.latest_value) def test_compare_config_owlbot_cli_update(self): self.baseline_config.owlbot_cli_image = "image_version_123" @@ -95,7 +98,10 @@ def test_compare_config_owlbot_cli_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.OWLBOT_CLI: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("OwlBot cli commit", config_change.changed_param) + self.assertEqual("image_version_456", config_change.latest_value) def test_compare_config_synthtool_update(self): self.baseline_config.synthtool_commitish = "commit123" @@ -104,7 +110,10 @@ def test_compare_config_synthtool_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.SYNTHTOOL: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("Synthtool commit", config_change.changed_param) + self.assertEqual("commit456", config_change.latest_value) def test_compare_protobuf_update(self): self.baseline_config.protobuf_version = "3.25.2" @@ -113,7 +122,10 @@ def test_compare_protobuf_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.PROTOBUF: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("Protobuf version", config_change.changed_param) + self.assertEqual("3.27.0", config_change.latest_value) def test_compare_config_grpc_update(self): self.baseline_config.grpc_version = "1.60.0" @@ -122,7 +134,10 @@ def test_compare_config_grpc_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.GRPC: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("Grpc version", config_change.changed_param) + self.assertEqual("1.61.0", config_change.latest_value) def test_compare_config_template_excludes_update(self): self.baseline_config.template_excludes = [".github/*", ".kokoro/*"] @@ -136,7 +151,20 @@ def test_compare_config_template_excludes_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.TEMPLATE_EXCLUDES: []}, result) + self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] + self.assertEqual("Template excludes", config_change.changed_param) + self.assertEqual( + str( + [ + ".github/*", + ".kokoro/*", + "samples/*", + "CODE_OF_CONDUCT.md", + ] + ), + config_change.latest_value, + ) def test_compare_config_library_addition(self): self.latest_config.libraries.append( @@ -152,7 +180,9 @@ def test_compare_config_library_addition(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.LIBRARIES_ADDITION: ["new_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) + config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertEqual("new_library", config_change.library_name) def test_compare_config_library_removal(self): self.latest_config.libraries = [] @@ -160,7 +190,35 @@ def test_compare_config_library_removal(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.LIBRARIES_REMOVAL: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) + config_change = result[ChangeType.LIBRARIES_REMOVAL][0] + self.assertEqual("existing_library", config_change.library_name) + + def test_compare_config_api_shortname_update(self): + self.latest_config.libraries[0].api_shortname = "new_api_shortname" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) + config_change = result[ChangeType.LIBRARIES_REMOVAL][0] + self.assertEqual("existing_library", config_change.library_name) + self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) + config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertEqual("new_api_shortname", config_change.library_name) + + def test_compare_config_library_name_update(self): + self.latest_config.libraries[0].library_name = "new_library_name" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) + config_change = result[ChangeType.LIBRARIES_REMOVAL][0] + self.assertEqual("existing_library", config_change.library_name) + self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) + config_change = result[ChangeType.LIBRARIES_ADDITION][0] + self.assertEqual("new_library_name", config_change.library_name) def test_compare_config_api_description_update(self): self.latest_config.libraries[0].api_description = "updated description" @@ -168,7 +226,11 @@ def test_compare_config_api_description_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.API_DESCRIPTION: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("api description", config_change.changed_param) + self.assertEqual("updated description", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_name_pretty_update(self): self.latest_config.libraries[0].name_pretty = "new name" @@ -176,7 +238,11 @@ def test_compare_config_name_pretty_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.NAME_PRETTY: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("name pretty", config_change.changed_param) + self.assertEqual("new name", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_product_docs_update(self): self.latest_config.libraries[0].product_documentation = "new docs" @@ -184,7 +250,11 @@ def test_compare_config_product_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.PRODUCT_DOCS: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("product documentation", config_change.changed_param) + self.assertEqual("new docs", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_library_type_update(self): self.latest_config.libraries[0].library_type = "GAPIC_COMBO" @@ -192,7 +262,11 @@ def test_compare_config_library_type_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.LIBRARY_TYPE: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("library type", config_change.changed_param) + self.assertEqual("GAPIC_COMBO", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_release_level_update(self): self.latest_config.libraries[0].release_level = "STABLE" @@ -200,7 +274,11 @@ def test_compare_config_release_level_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.RELEASE_LEVEL: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("release level", config_change.changed_param) + self.assertEqual("STABLE", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_api_id_update(self): self.latest_config.libraries[0].api_id = "new_id" @@ -208,7 +286,11 @@ def test_compare_config_api_id_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.API_ID: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("api id", config_change.changed_param) + self.assertEqual("new_id", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_api_reference_update(self): self.latest_config.libraries[0].api_reference = "new api_reference" @@ -216,7 +298,11 @@ def test_compare_config_api_reference_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.API_REFERENCE: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("api reference", config_change.changed_param) + self.assertEqual("new api_reference", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_code_owner_team_update(self): self.latest_config.libraries[0].codeowner_team = "new team" @@ -224,7 +310,11 @@ def test_compare_config_code_owner_team_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.CODEOWNER_TEAM: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("code owner team", config_change.changed_param) + self.assertEqual("new team", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_excluded_deps_update(self): self.latest_config.libraries[0].excluded_dependencies = "group:artifact" @@ -232,9 +322,11 @@ def test_compare_config_excluded_deps_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual( - {ChangeType.EXCLUDED_DEPENDENCIES: ["existing_library"]}, result - ) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("excluded dependencies", config_change.changed_param) + self.assertEqual("group:artifact", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_excluded_poms_update(self): self.latest_config.libraries[0].excluded_poms = "pom.xml" @@ -242,7 +334,11 @@ def test_compare_config_excluded_poms_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.EXCLUDED_POMS: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("excluded poms", config_change.changed_param) + self.assertEqual("pom.xml", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_client_docs_update(self): self.latest_config.libraries[0].client_documentation = "new client docs" @@ -250,7 +346,35 @@ def test_compare_config_client_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.CLIENT_DOCS: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("client documentation", config_change.changed_param) + self.assertEqual("new client docs", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) + + def test_compare_config_distribution_name_update(self): + self.latest_config.libraries[0].distribution_name = "new_group:new_artifact" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("distribution name", config_change.changed_param) + self.assertEqual("new_group:new_artifact", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) + + def test_compare_config_group_id_update(self): + self.latest_config.libraries[0].group_id = "new_group" + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("group id", config_change.changed_param) + self.assertEqual("new_group", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_issue_tracker_update(self): self.latest_config.libraries[0].issue_tracker = "new issue tracker" @@ -258,7 +382,11 @@ def test_compare_config_issue_tracker_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.ISSUE_TRACKER: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("issue tracker", config_change.changed_param) + self.assertEqual("new issue tracker", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_rest_docs_update(self): self.latest_config.libraries[0].rest_documentation = "new rest docs" @@ -266,7 +394,11 @@ def test_compare_config_rest_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.REST_DOCS: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("rest documentation", config_change.changed_param) + self.assertEqual("new rest docs", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_rpc_docs_update(self): self.latest_config.libraries[0].rpc_documentation = "new rpc docs" @@ -274,7 +406,23 @@ def test_compare_config_rpc_docs_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.RPC_DOCS: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("rpc documentation", config_change.changed_param) + self.assertEqual("new rpc docs", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) + + def test_compare_config_cloud_api_update(self): + self.latest_config.libraries[0].cloud_api = False + result = compare_config( + baseline_config=self.baseline_config, + latest_config=self.latest_config, + ) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("cloud api", config_change.changed_param) + self.assertEqual("False", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_requires_billing_update(self): self.latest_config.libraries[0].requires_billing = False @@ -282,7 +430,11 @@ def test_compare_config_requires_billing_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.REQUIRES_BILLING: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("requires billing", config_change.changed_param) + self.assertEqual("False", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_extra_versioned_mod_update(self): self.latest_config.libraries[0].extra_versioned_modules = "extra module" @@ -290,9 +442,11 @@ def test_compare_config_extra_versioned_mod_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual( - {ChangeType.EXTRA_VERSIONED_MODULES: ["existing_library"]}, result - ) + self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) + config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] + self.assertEqual("extra versioned modules", config_change.changed_param) + self.assertEqual("extra module", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_version_addition(self): self.latest_config.libraries[0].gapic_configs = [ @@ -302,7 +456,11 @@ def test_compare_config_version_addition(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.GAPIC_ADDITION: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.GAPIC_ADDITION]) == 1) + config_change = result[ChangeType.GAPIC_ADDITION][0] + self.assertEqual("", config_change.changed_param) + self.assertEqual("google/new/library/v1", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) def test_compare_config_version_removal(self): self.baseline_config.libraries[0].gapic_configs = [ @@ -312,4 +470,8 @@ def test_compare_config_version_removal(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertEqual({ChangeType.GAPIC_REMOVAL: ["existing_library"]}, result) + self.assertTrue(len(result[ChangeType.GAPIC_REMOVAL]) == 1) + config_change = result[ChangeType.GAPIC_REMOVAL][0] + self.assertEqual("", config_change.changed_param) + self.assertEqual("google/old/library/v1", config_change.latest_value) + self.assertEqual("existing_library", config_change.library_name) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index d7f892d8f5..e6b4844858 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -11,6 +11,7 @@ # 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. +from collections import defaultdict from enum import Enum from typing import Dict from typing import List @@ -20,37 +21,15 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utilities import get_library_name -ALL_LIBRARIES = [] - class ChangeType(Enum): GOOGLEAPIS_COMMIT = 1 - GENERATOR = 2 - OWLBOT_CLI = 3 - SYNTHTOOL = 4 - PROTOBUF = 5 - GRPC = 6 - TEMPLATE_EXCLUDES = 7 - LIBRARIES_ADDITION = 8 - LIBRARIES_REMOVAL = 9 - API_DESCRIPTION = 10 - NAME_PRETTY = 11 - PRODUCT_DOCS = 12 - LIBRARY_TYPE = 13 - RELEASE_LEVEL = 14 - API_ID = 15 - API_REFERENCE = 16 - CODEOWNER_TEAM = 17 - EXCLUDED_DEPENDENCIES = 18 - EXCLUDED_POMS = 19 - CLIENT_DOCS = 20 - ISSUE_TRACKER = 21 - REST_DOCS = 22 - RPC_DOCS = 23 - REQUIRES_BILLING = 24 - EXTRA_VERSIONED_MODULES = 25 - GAPIC_ADDITION = 26 - GAPIC_REMOVAL = 27 + REPO_LEVEL_CHANGE = 2 + LIBRARIES_ADDITION = 3 + LIBRARIES_REMOVAL = 4 + LIBRARY_LEVEL_CHANGE = 5 + GAPIC_ADDITION = 6 + GAPIC_REMOVAL = 7 class HashLibrary: @@ -63,35 +42,64 @@ def __init__(self, hash_value: int, library: LibraryConfig): self.library = library +class ConfigChange: + def __init__(self, changed_param: str, latest_value: str, library_name: str = ""): + self.changed_param = changed_param + self.latest_value = latest_value + self.library_name = library_name + + def compare_config( baseline_config: GenerationConfig, latest_config: GenerationConfig -) -> Dict[ChangeType, List[str]]: +) -> Dict[ChangeType, list[ConfigChange]]: """ - Compare two GenerationConfig object and output a mapping from ChangeType - to a list of library_name of affected libraries. + Compare two GenerationConfig object and output a mapping from ConfigChange + to a list of ConfigChange objects. All libraries in the latest configuration will be affected if one of the repository level parameters is changed. :param baseline_config: the baseline GenerationConfig object :param latest_config: the latest GenerationConfig object - :return: a mapping from ChangeType to a list of library_name of affected - libraries. + :return: a mapping from ConfigChange to a list of ConfigChange objects. """ - diff = {} + diff = defaultdict(list[ConfigChange]) if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: - diff[ChangeType.GOOGLEAPIS_COMMIT] = ALL_LIBRARIES + diff[ChangeType.GOOGLEAPIS_COMMIT] = [] if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: - diff[ChangeType.GENERATOR] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="GAPIC generator version", + latest_value=latest_config.gapic_generator_version, + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: - diff[ChangeType.OWLBOT_CLI] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="OwlBot cli commit", + latest_value=latest_config.owlbot_cli_image, + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: - diff[ChangeType.SYNTHTOOL] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="Synthtool commit", + latest_value=latest_config.synthtool_commitish, + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) if baseline_config.protobuf_version != latest_config.protobuf_version: - diff[ChangeType.PROTOBUF] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="Protobuf version", + latest_value=latest_config.protobuf_version, + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) if baseline_config.grpc_version != latest_config.grpc_version: - diff[ChangeType.GRPC] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="Grpc version", latest_value=latest_config.grpc_version + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) if baseline_config.template_excludes != latest_config.template_excludes: - diff[ChangeType.TEMPLATE_EXCLUDES] = ALL_LIBRARIES + config_change = ConfigChange( + changed_param="Template excludes", + latest_value=str(latest_config.template_excludes), + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) __compare_libraries( diff=diff, @@ -102,7 +110,7 @@ def compare_config( def __compare_libraries( - diff: Dict[ChangeType, List[str]], + diff: Dict[ChangeType, list[ConfigChange]], baseline_library_configs: List[LibraryConfig], latest_library_configs: List[LibraryConfig], ) -> None: @@ -110,10 +118,9 @@ def __compare_libraries( Compare two lists of LibraryConfig and put the difference into a given Dict. - :param diff: a mapping from ChangeType to a list of library_name of - affected libraries. - :param baseline_library_configs: a list of LibraryConfig object - :param latest_library_configs: a list of LibraryConfig object + :param diff: a mapping from ConfigChange to a list of ConfigChange objects. + :param baseline_library_configs: a list of LibraryConfig object. + :param latest_library_configs: a list of LibraryConfig object. """ baseline_libraries = __convert_to_hashed_library_dict(baseline_library_configs) latest_libraries = __convert_to_hashed_library_dict(latest_library_configs) @@ -124,9 +131,10 @@ def __compare_libraries( # a library is removed from baseline_libraries if the library_name # is not in latest_libraries. if library_name not in latest_libraries: - if ChangeType.LIBRARIES_REMOVAL not in diff: - diff[ChangeType.LIBRARIES_REMOVAL] = [] - diff[ChangeType.LIBRARIES_REMOVAL].append(library_name) + config_change = ConfigChange( + changed_param="", latest_value="", library_name=library_name + ) + diff[ChangeType.LIBRARIES_REMOVAL].append(config_change) # 2. find any library that exists in both configs but at least one # parameter is changed, which means the hash value is different. if ( @@ -140,9 +148,10 @@ def __compare_libraries( # a library is added to latest_libraries if the library_name # is not in baseline_libraries. if library_name not in baseline_libraries: - if ChangeType.LIBRARIES_ADDITION not in diff: - diff[ChangeType.LIBRARIES_ADDITION] = [] - diff[ChangeType.LIBRARIES_ADDITION].append(library_name) + config_change = ConfigChange( + changed_param="", latest_value="", library_name=library_name + ) + diff[ChangeType.LIBRARIES_ADDITION].append(config_change) # 3rd round comparison. __compare_changed_libraries( diff=diff, @@ -170,7 +179,7 @@ def __convert_to_hashed_library_dict( def __compare_changed_libraries( - diff: Dict[ChangeType, List[str]], + diff: Dict[ChangeType, list[ConfigChange]], baseline_libraries: Dict[str, HashLibrary], latest_libraries: Dict[str, HashLibrary], changed_libraries: List[str], @@ -179,16 +188,7 @@ def __compare_changed_libraries( Compare each library with the same library_name to find what parameters are changed. - Some parameters should not change: - - - api_shortname - - library_name - - distribution_name - - group_id - - cloud_api - - :param diff: a mapping from ChangeType to a list of library_name of - affected libraries. + :param diff: a mapping from ConfigChange to a list of ConfigChange objects. :param baseline_libraries: a mapping from library_name to HashLibrary object. :param latest_libraries: a mapping from library_name to HashLibrary object. @@ -197,96 +197,149 @@ def __compare_changed_libraries( for library_name in changed_libraries: baseline_library = baseline_libraries[library_name].library latest_library = latest_libraries[library_name].library - if ( - baseline_library.api_description - != latest_libraries[library_name].library.api_description - ): - if ChangeType.API_DESCRIPTION not in diff: - diff[ChangeType.API_DESCRIPTION] = [] - diff[ChangeType.API_DESCRIPTION].append(library_name) - if ( - baseline_library.name_pretty - != latest_libraries[library_name].library.name_pretty - ): - if ChangeType.NAME_PRETTY not in diff: - diff[ChangeType.NAME_PRETTY] = [] - diff[ChangeType.NAME_PRETTY].append(library_name) + if baseline_library.api_description != latest_library.api_description: + config_change = ConfigChange( + changed_param="api description", + latest_value=latest_library.api_description, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) + if baseline_library.name_pretty != latest_library.name_pretty: + config_change = ConfigChange( + changed_param="name pretty", + latest_value=latest_library.name_pretty, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if ( baseline_library.product_documentation != latest_library.product_documentation ): - if ChangeType.PRODUCT_DOCS not in diff: - diff[ChangeType.PRODUCT_DOCS] = [] - diff[ChangeType.PRODUCT_DOCS].append(library_name) - + config_change = ConfigChange( + changed_param="product documentation", + latest_value=latest_library.product_documentation, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.library_type != latest_library.library_type: - if ChangeType.LIBRARY_TYPE not in diff: - diff[ChangeType.LIBRARY_TYPE] = [] - diff[ChangeType.LIBRARY_TYPE].append(library_name) - + config_change = ConfigChange( + changed_param="library type", + latest_value=latest_library.library_type, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.release_level != latest_library.release_level: - if ChangeType.RELEASE_LEVEL not in diff: - diff[ChangeType.RELEASE_LEVEL] = [] - diff[ChangeType.RELEASE_LEVEL].append(library_name) - + config_change = ConfigChange( + changed_param="release level", + latest_value=latest_library.release_level, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.api_id != latest_library.api_id: - if ChangeType.API_ID not in diff: - diff[ChangeType.API_ID] = [] - diff[ChangeType.API_ID].append(library_name) - + config_change = ConfigChange( + changed_param="api id", + latest_value=latest_library.api_id, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.api_reference != latest_library.api_reference: - if ChangeType.API_REFERENCE not in diff: - diff[ChangeType.API_REFERENCE] = [] - diff[ChangeType.API_REFERENCE].append(library_name) - + config_change = ConfigChange( + changed_param="api reference", + latest_value=latest_library.api_reference, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.codeowner_team != latest_library.codeowner_team: - if ChangeType.CODEOWNER_TEAM not in diff: - diff[ChangeType.CODEOWNER_TEAM] = [] - diff[ChangeType.CODEOWNER_TEAM].append(library_name) + config_change = ConfigChange( + changed_param="code owner team", + latest_value=latest_library.codeowner_team, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if ( baseline_library.excluded_dependencies != latest_library.excluded_dependencies ): - if ChangeType.EXCLUDED_DEPENDENCIES not in diff: - diff[ChangeType.EXCLUDED_DEPENDENCIES] = [] - diff[ChangeType.EXCLUDED_DEPENDENCIES].append(library_name) + config_change = ConfigChange( + changed_param="excluded dependencies", + latest_value=latest_library.excluded_dependencies, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.excluded_poms != latest_library.excluded_poms: - if ChangeType.EXCLUDED_POMS not in diff: - diff[ChangeType.EXCLUDED_POMS] = [] - diff[ChangeType.EXCLUDED_POMS].append(library_name) - + config_change = ConfigChange( + changed_param="excluded poms", + latest_value=latest_library.excluded_poms, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.client_documentation != latest_library.client_documentation: - if ChangeType.CLIENT_DOCS not in diff: - diff[ChangeType.CLIENT_DOCS] = [] - diff[ChangeType.CLIENT_DOCS].append(library_name) - + config_change = ConfigChange( + changed_param="client documentation", + latest_value=latest_library.client_documentation, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) + if baseline_library.distribution_name != latest_library.distribution_name: + config_change = ConfigChange( + changed_param="distribution name", + latest_value=latest_library.distribution_name, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) + if baseline_library.group_id != latest_library.group_id: + config_change = ConfigChange( + changed_param="group id", + latest_value=latest_library.group_id, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.issue_tracker != latest_library.issue_tracker: - if ChangeType.ISSUE_TRACKER not in diff: - diff[ChangeType.ISSUE_TRACKER] = [] - diff[ChangeType.ISSUE_TRACKER].append(library_name) - + config_change = ConfigChange( + changed_param="issue tracker", + latest_value=latest_library.issue_tracker, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.rest_documentation != latest_library.rest_documentation: - if ChangeType.REST_DOCS not in diff: - diff[ChangeType.REST_DOCS] = [] - diff[ChangeType.REST_DOCS].append(library_name) - + config_change = ConfigChange( + changed_param="rest documentation", + latest_value=latest_library.rest_documentation, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.rpc_documentation != latest_library.rpc_documentation: - if ChangeType.RPC_DOCS not in diff: - diff[ChangeType.RPC_DOCS] = [] - diff[ChangeType.RPC_DOCS].append(library_name) - + config_change = ConfigChange( + changed_param="rpc documentation", + latest_value=latest_library.rpc_documentation, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) + if baseline_library.cloud_api != latest_library.cloud_api: + config_change = ConfigChange( + changed_param="cloud api", + latest_value=str(latest_library.cloud_api), + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if baseline_library.requires_billing != latest_library.requires_billing: - if ChangeType.REQUIRES_BILLING not in diff: - diff[ChangeType.REQUIRES_BILLING] = [] - diff[ChangeType.REQUIRES_BILLING].append(library_name) + config_change = ConfigChange( + changed_param="requires billing", + latest_value=str(latest_library.requires_billing), + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) if ( baseline_library.extra_versioned_modules != latest_library.extra_versioned_modules ): - if ChangeType.EXTRA_VERSIONED_MODULES not in diff: - diff[ChangeType.EXTRA_VERSIONED_MODULES] = [] - diff[ChangeType.EXTRA_VERSIONED_MODULES].append(library_name) + config_change = ConfigChange( + changed_param="extra versioned modules", + latest_value=latest_library.extra_versioned_modules, + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) # compare gapic_configs baseline_gapic_configs = baseline_library.gapic_configs latest_gapic_configs = latest_library.gapic_configs @@ -299,7 +352,7 @@ def __compare_changed_libraries( def __compare_gapic_configs( - diff: Dict[ChangeType, List[str]], + diff: Dict[ChangeType, list[ConfigChange]], library_name: str, baseline_gapic_configs: List[GapicConfig], latest_gapic_configs: List[GapicConfig], @@ -311,14 +364,16 @@ def __compare_gapic_configs( for proto_path in baseline_proto_paths: if proto_path in latest_proto_paths: continue - if ChangeType.GAPIC_REMOVAL not in diff: - diff[ChangeType.GAPIC_REMOVAL] = [] - diff[ChangeType.GAPIC_REMOVAL].append(library_name) + config_change = ConfigChange( + changed_param="", latest_value=proto_path, library_name=library_name + ) + diff[ChangeType.GAPIC_REMOVAL].append(config_change) # 2nd round of comparison, find any versioned proto_path is added # to latest gapic configs. for proto_path in latest_proto_paths: if proto_path in baseline_proto_paths: continue - if ChangeType.GAPIC_ADDITION not in diff: - diff[ChangeType.GAPIC_ADDITION] = [] - diff[ChangeType.GAPIC_ADDITION].append(library_name) + config_change = ConfigChange( + changed_param="", latest_value=proto_path, library_name=library_name + ) + diff[ChangeType.GAPIC_ADDITION].append(config_change) From af85a4fa33524d25a7b7bf7af6e1c8a3c2d72000 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 19 Mar 2024 23:41:12 +0000 Subject: [PATCH 17/21] move get_library_name method to LibraryConfig --- library_generation/model/library_config.py | 7 ++++ library_generation/test/model/__init__.py | 0 .../test/model/library_config_unit_tests.py | 39 +++++++++++++++++++ .../test/utilities_unit_tests.py | 6 --- library_generation/utilities.py | 15 +------ .../utils/generation_config_comparator.py | 3 +- 6 files changed, 49 insertions(+), 21 deletions(-) create mode 100644 library_generation/test/model/__init__.py create mode 100644 library_generation/test/model/library_config_unit_tests.py diff --git a/library_generation/model/library_config.py b/library_generation/model/library_config.py index e3fa970c1c..30d923da81 100644 --- a/library_generation/model/library_config.py +++ b/library_generation/model/library_config.py @@ -73,6 +73,13 @@ def __init__( self.requires_billing = requires_billing self.extra_versioned_modules = extra_versioned_modules + def get_library_name(self) -> str: + """ + Return the library name of a given LibraryConfig object + :return: the library name + """ + return self.library_name if self.library_name else self.api_shortname + def __eq__(self, other): return ( self.api_shortname == other.api_shortname diff --git a/library_generation/test/model/__init__.py b/library_generation/test/model/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/test/model/library_config_unit_tests.py b/library_generation/test/model/library_config_unit_tests.py new file mode 100644 index 0000000000..1c0b0d0690 --- /dev/null +++ b/library_generation/test/model/library_config_unit_tests.py @@ -0,0 +1,39 @@ +# 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.library_config import LibraryConfig + + +class LibraryConfigTest(unittest.TestCase): + def test_get_library_returns_library_name(self): + library = LibraryConfig( + api_shortname="secret", + name_pretty="", + product_documentation="", + api_description="", + gapic_configs=list(), + library_name="secretmanager", + ) + self.assertEqual("secretmanager", library.get_library_name()) + + def test_get_library_returns_api_shortname(self): + library = LibraryConfig( + api_shortname="secret", + name_pretty="", + product_documentation="", + api_description="", + gapic_configs=list(), + ) + self.assertEqual("secret", library.get_library_name()) diff --git a/library_generation/test/utilities_unit_tests.py b/library_generation/test/utilities_unit_tests.py index aa6d99ac24..3d114f977f 100644 --- a/library_generation/test/utilities_unit_tests.py +++ b/library_generation/test/utilities_unit_tests.py @@ -391,12 +391,6 @@ def test_remove_version_from_returns_self(self): "google/cloud/aiplatform", util.remove_version_from(proto_path) ) - def test_get_library_returns_library_name(self): - self.assertEqual("bare-metal-solution", util.get_library_name(library_1)) - - def test_get_library_returns_api_shortname(self): - self.assertEqual("secretmanager", util.get_library_name(library_2)) - def test_generate_prerequisite_files_non_monorepo_success(self): library_path = self.__setup_prerequisite_files( num_libraries=1, library_type="GAPIC_COMBO" diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 29e175c1b8..de3dd6ce44 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -39,17 +39,6 @@ def create_argument(arg_key: str, arg_container: object) -> List[str]: return [] -def get_library_name( - library: LibraryConfig, -) -> str: - """ - Return the library name of a given LibraryConfig object - :param library: an object of LibraryConfig - :return: the library name - """ - return library.library_name if library.library_name else library.api_shortname - - def run_process_and_print_output(arguments: List[str], job_name: str = "Job"): """ Runs a process with the given "arguments" list and prints its output. @@ -231,7 +220,7 @@ def generate_prerequisite_files( :return: None """ cloud_prefix = "cloud-" if library.cloud_api else "" - library_name = get_library_name(library) + library_name = library.get_library_name() distribution_name = ( library.distribution_name if library.distribution_name @@ -341,7 +330,7 @@ def get_file_paths(config: GenerationConfig) -> Dict[str, str]: paths = {} for library in config.libraries: for gapic_config in library.gapic_configs: - paths[gapic_config.proto_path] = get_library_name(library) + paths[gapic_config.proto_path] = library.get_library_name() return paths diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index e6b4844858..4129428d59 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -19,7 +19,6 @@ from library_generation.model.gapic_config import GapicConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig -from library_generation.utilities import get_library_name class ChangeType(Enum): @@ -173,7 +172,7 @@ def __convert_to_hashed_library_dict( :return: a mapping from library_name to HashLibrary object. """ return { - get_library_name(library): HashLibrary(hash(library), library) + library.get_library_name(): HashLibrary(hash(library), library) for library in libraries } From 989da4790857258bdeb926b16a71a2fe64362844 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 19 Mar 2024 23:44:50 +0000 Subject: [PATCH 18/21] fix integration tests --- library_generation/test/integration_tests.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 3bf50d0394..35cbf565ac 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -29,7 +29,6 @@ from library_generation.model.generation_config import from_yaml, GenerationConfig from library_generation.test.compare_poms import compare_xml from library_generation.utilities import ( - get_library_name, sh_util as shell_call, run_process_and_print_output, ) @@ -214,7 +213,7 @@ def __pull_repo_to(cls, default_dest: Path, repo: str, committish: str) -> str: def __get_library_names_from_config(cls, config: GenerationConfig) -> List[str]: library_names = [] for library in config.libraries: - library_names.append(f"java-{get_library_name(library)}") + library_names.append(f"java-{library.get_library_name()}") return library_names @@ -248,7 +247,7 @@ def __load_json_to_sorted_list(cls, path: str) -> List[tuple]: @classmethod def __recursive_diff_files( - self, + cls, dcmp: dircmp, diff_files: List[str], left_only: List[str], @@ -256,13 +255,14 @@ def __recursive_diff_files( dirname: str = "", ): """ - recursively compares two subdirectories. The found differences are passed to three expected list references + Recursively compares two subdirectories. The found differences are + passed to three expected list references. """ append_dirname = lambda d: dirname + d diff_files.extend(map(append_dirname, dcmp.diff_files)) left_only.extend(map(append_dirname, dcmp.left_only)) right_only.extend(map(append_dirname, dcmp.right_only)) for sub_dirname, sub_dcmp in dcmp.subdirs.items(): - self.__recursive_diff_files( + cls.__recursive_diff_files( sub_dcmp, diff_files, left_only, right_only, dirname + sub_dirname + "/" ) From 376015d45d4bed35bd40dcab12ed1d6d7d146927 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 20 Mar 2024 00:10:50 +0000 Subject: [PATCH 19/21] comment out library removal and gapic removal type --- ...generation_config_comparator_unit_tests.py | 30 --------------- .../utils/generation_config_comparator.py | 38 ++++++++++++------- 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 55e975e2f5..6455aae23d 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -184,25 +184,12 @@ def test_compare_config_library_addition(self): config_change = result[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library", config_change.library_name) - def test_compare_config_library_removal(self): - self.latest_config.libraries = [] - result = compare_config( - baseline_config=self.baseline_config, - latest_config=self.latest_config, - ) - self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) - config_change = result[ChangeType.LIBRARIES_REMOVAL][0] - self.assertEqual("existing_library", config_change.library_name) - def test_compare_config_api_shortname_update(self): self.latest_config.libraries[0].api_shortname = "new_api_shortname" result = compare_config( baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) - config_change = result[ChangeType.LIBRARIES_REMOVAL][0] - self.assertEqual("existing_library", config_change.library_name) self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) config_change = result[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_api_shortname", config_change.library_name) @@ -213,9 +200,6 @@ def test_compare_config_library_name_update(self): baseline_config=self.baseline_config, latest_config=self.latest_config, ) - self.assertTrue(len(result[ChangeType.LIBRARIES_REMOVAL]) == 1) - config_change = result[ChangeType.LIBRARIES_REMOVAL][0] - self.assertEqual("existing_library", config_change.library_name) self.assertTrue(len(result[ChangeType.LIBRARIES_ADDITION]) == 1) config_change = result[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library_name", config_change.library_name) @@ -461,17 +445,3 @@ def test_compare_config_version_addition(self): self.assertEqual("", config_change.changed_param) self.assertEqual("google/new/library/v1", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) - - def test_compare_config_version_removal(self): - self.baseline_config.libraries[0].gapic_configs = [ - GapicConfig(proto_path="google/old/library/v1") - ] - result = compare_config( - baseline_config=self.baseline_config, - latest_config=self.latest_config, - ) - self.assertTrue(len(result[ChangeType.GAPIC_REMOVAL]) == 1) - config_change = result[ChangeType.GAPIC_REMOVAL][0] - self.assertEqual("", config_change.changed_param) - self.assertEqual("google/old/library/v1", config_change.latest_value) - self.assertEqual("existing_library", config_change.library_name) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 4129428d59..05454ee6fd 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -25,10 +25,14 @@ class ChangeType(Enum): GOOGLEAPIS_COMMIT = 1 REPO_LEVEL_CHANGE = 2 LIBRARIES_ADDITION = 3 - LIBRARIES_REMOVAL = 4 + # As of Mar. 2024, we decide not to produce this type of change because we + # still need to manually remove the libray. + # LIBRARIES_REMOVAL = 4 LIBRARY_LEVEL_CHANGE = 5 GAPIC_ADDITION = 6 - GAPIC_REMOVAL = 7 + # As of Mar. 2024, we decide not to produce this type of change because we + # still need to manually remove the libray. + # GAPIC_REMOVAL = 7 class HashLibrary: @@ -129,11 +133,14 @@ def __compare_libraries( # 1. find any library removed from baseline_libraries. # a library is removed from baseline_libraries if the library_name # is not in latest_libraries. - if library_name not in latest_libraries: - config_change = ConfigChange( - changed_param="", latest_value="", library_name=library_name - ) - diff[ChangeType.LIBRARIES_REMOVAL].append(config_change) + # please see the reason of comment out these lines of code in the + # comment of ChangeType.LIBRARIES_REMOVAL. + # if library_name not in latest_libraries: + # config_change = ConfigChange( + # changed_param="", latest_value="", library_name=library_name + # ) + # diff[ChangeType.LIBRARIES_REMOVAL].append(config_change) + # 2. find any library that exists in both configs but at least one # parameter is changed, which means the hash value is different. if ( @@ -360,13 +367,16 @@ def __compare_gapic_configs( latest_proto_paths = {config.proto_path for config in latest_gapic_configs} # 1st round of comparison, find any versioned proto_path is removed # from baseline gapic configs. - for proto_path in baseline_proto_paths: - if proto_path in latest_proto_paths: - continue - config_change = ConfigChange( - changed_param="", latest_value=proto_path, library_name=library_name - ) - diff[ChangeType.GAPIC_REMOVAL].append(config_change) + # please see the reason of comment out these lines of code in the + # comment of ChangeType.GAPIC_REMOVAL. + # for proto_path in baseline_proto_paths: + # if proto_path in latest_proto_paths: + # continue + # config_change = ConfigChange( + # changed_param="", latest_value=proto_path, library_name=library_name + # ) + # diff[ChangeType.GAPIC_REMOVAL].append(config_change) + # 2nd round of comparison, find any versioned proto_path is added # to latest gapic configs. for proto_path in latest_proto_paths: From 70e0fe84f78ab9ed4fb0b2cce59226fa5fed6198 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 20 Mar 2024 00:44:48 +0000 Subject: [PATCH 20/21] raise an error if api_shortname is changed but library_name remains the same --- .../generation_config_comparator_unit_tests.py | 14 +++++++++++++- .../utils/generation_config_comparator.py | 6 ++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 6455aae23d..04c515cf49 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -184,7 +184,7 @@ def test_compare_config_library_addition(self): config_change = result[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library", config_change.library_name) - def test_compare_config_api_shortname_update(self): + def test_compare_config_api_shortname_update_without_library_name(self): self.latest_config.libraries[0].api_shortname = "new_api_shortname" result = compare_config( baseline_config=self.baseline_config, @@ -194,6 +194,18 @@ def test_compare_config_api_shortname_update(self): config_change = result[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_api_shortname", config_change.library_name) + def test_compare_config_api_shortname_update_with_library_name_raise_error(self): + self.baseline_config.libraries[0].library_name = "old_library_name" + self.latest_config.libraries[0].library_name = "old_library_name" + self.latest_config.libraries[0].api_shortname = "new_api_shortname" + self.assertRaisesRegex( + ValueError, + r"api_shortname.*library_name", + compare_config, + self.baseline_config, + self.latest_config, + ) + def test_compare_config_library_name_update(self): self.latest_config.libraries[0].library_name = "new_library_name" result = compare_config( diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 05454ee6fd..2affc0bf7b 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -199,10 +199,16 @@ def __compare_changed_libraries( object. :param latest_libraries: a mapping from library_name to HashLibrary object. :param changed_libraries: a list of library_name of changed libraries. + :raise ValueError: if api_shortname of a library is changed but library_name + remains the same. """ for library_name in changed_libraries: baseline_library = baseline_libraries[library_name].library latest_library = latest_libraries[library_name].library + if baseline_library.api_shortname != latest_library.api_shortname: + raise ValueError( + f"{library_name}: api_shortname must not change when library_name remains the same." + ) if baseline_library.api_description != latest_library.api_description: config_change = ConfigChange( changed_param="api description", From 42b5b0370e836bc74630fee531433a686269e358 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 20 Mar 2024 15:05:15 +0000 Subject: [PATCH 21/21] use built-in method, `vars`, to list parameters of an object --- ...generation_config_comparator_unit_tests.py | 68 +++-- .../utils/generation_config_comparator.py | 269 ++++++------------ 2 files changed, 119 insertions(+), 218 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index 04c515cf49..c2c025c579 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -88,7 +88,7 @@ def test_compare_config_generator_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("GAPIC generator version", config_change.changed_param) + self.assertEqual("gapic_generator_version", config_change.changed_param) self.assertEqual("1.2.4", config_change.latest_value) def test_compare_config_owlbot_cli_update(self): @@ -100,7 +100,7 @@ def test_compare_config_owlbot_cli_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("OwlBot cli commit", config_change.changed_param) + self.assertEqual("owlbot_cli_image", config_change.changed_param) self.assertEqual("image_version_456", config_change.latest_value) def test_compare_config_synthtool_update(self): @@ -112,7 +112,7 @@ def test_compare_config_synthtool_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("Synthtool commit", config_change.changed_param) + self.assertEqual("synthtool_commitish", config_change.changed_param) self.assertEqual("commit456", config_change.latest_value) def test_compare_protobuf_update(self): @@ -124,7 +124,7 @@ def test_compare_protobuf_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("Protobuf version", config_change.changed_param) + self.assertEqual("protobuf_version", config_change.changed_param) self.assertEqual("3.27.0", config_change.latest_value) def test_compare_config_grpc_update(self): @@ -136,7 +136,7 @@ def test_compare_config_grpc_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("Grpc version", config_change.changed_param) + self.assertEqual("grpc_version", config_change.changed_param) self.assertEqual("1.61.0", config_change.latest_value) def test_compare_config_template_excludes_update(self): @@ -153,16 +153,14 @@ def test_compare_config_template_excludes_update(self): ) self.assertTrue(len(result[ChangeType.REPO_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.REPO_LEVEL_CHANGE][0] - self.assertEqual("Template excludes", config_change.changed_param) + self.assertEqual("template_excludes", config_change.changed_param) self.assertEqual( - str( - [ - ".github/*", - ".kokoro/*", - "samples/*", - "CODE_OF_CONDUCT.md", - ] - ), + [ + ".github/*", + ".kokoro/*", + "samples/*", + "CODE_OF_CONDUCT.md", + ], config_change.latest_value, ) @@ -224,7 +222,7 @@ def test_compare_config_api_description_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("api description", config_change.changed_param) + self.assertEqual("api_description", config_change.changed_param) self.assertEqual("updated description", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -236,7 +234,7 @@ def test_compare_config_name_pretty_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("name pretty", config_change.changed_param) + self.assertEqual("name_pretty", config_change.changed_param) self.assertEqual("new name", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -248,7 +246,7 @@ def test_compare_config_product_docs_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("product documentation", config_change.changed_param) + self.assertEqual("product_documentation", config_change.changed_param) self.assertEqual("new docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -260,7 +258,7 @@ def test_compare_config_library_type_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("library type", config_change.changed_param) + self.assertEqual("library_type", config_change.changed_param) self.assertEqual("GAPIC_COMBO", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -272,7 +270,7 @@ def test_compare_config_release_level_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("release level", config_change.changed_param) + self.assertEqual("release_level", config_change.changed_param) self.assertEqual("STABLE", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -284,7 +282,7 @@ def test_compare_config_api_id_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("api id", config_change.changed_param) + self.assertEqual("api_id", config_change.changed_param) self.assertEqual("new_id", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -296,7 +294,7 @@ def test_compare_config_api_reference_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("api reference", config_change.changed_param) + self.assertEqual("api_reference", config_change.changed_param) self.assertEqual("new api_reference", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -308,7 +306,7 @@ def test_compare_config_code_owner_team_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("code owner team", config_change.changed_param) + self.assertEqual("codeowner_team", config_change.changed_param) self.assertEqual("new team", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -320,7 +318,7 @@ def test_compare_config_excluded_deps_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("excluded dependencies", config_change.changed_param) + self.assertEqual("excluded_dependencies", config_change.changed_param) self.assertEqual("group:artifact", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -332,7 +330,7 @@ def test_compare_config_excluded_poms_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("excluded poms", config_change.changed_param) + self.assertEqual("excluded_poms", config_change.changed_param) self.assertEqual("pom.xml", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -344,7 +342,7 @@ def test_compare_config_client_docs_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("client documentation", config_change.changed_param) + self.assertEqual("client_documentation", config_change.changed_param) self.assertEqual("new client docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -356,7 +354,7 @@ def test_compare_config_distribution_name_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("distribution name", config_change.changed_param) + self.assertEqual("distribution_name", config_change.changed_param) self.assertEqual("new_group:new_artifact", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -368,7 +366,7 @@ def test_compare_config_group_id_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("group id", config_change.changed_param) + self.assertEqual("group_id", config_change.changed_param) self.assertEqual("new_group", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -380,7 +378,7 @@ def test_compare_config_issue_tracker_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("issue tracker", config_change.changed_param) + self.assertEqual("issue_tracker", config_change.changed_param) self.assertEqual("new issue tracker", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -392,7 +390,7 @@ def test_compare_config_rest_docs_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("rest documentation", config_change.changed_param) + self.assertEqual("rest_documentation", config_change.changed_param) self.assertEqual("new rest docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -404,7 +402,7 @@ def test_compare_config_rpc_docs_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("rpc documentation", config_change.changed_param) + self.assertEqual("rpc_documentation", config_change.changed_param) self.assertEqual("new rpc docs", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) @@ -416,8 +414,8 @@ def test_compare_config_cloud_api_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("cloud api", config_change.changed_param) - self.assertEqual("False", config_change.latest_value) + self.assertEqual("cloud_api", config_change.changed_param) + self.assertEqual(False, config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) def test_compare_config_requires_billing_update(self): @@ -428,8 +426,8 @@ def test_compare_config_requires_billing_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("requires billing", config_change.changed_param) - self.assertEqual("False", config_change.latest_value) + self.assertEqual("requires_billing", config_change.changed_param) + self.assertEqual(False, config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) def test_compare_config_extra_versioned_mod_update(self): @@ -440,7 +438,7 @@ def test_compare_config_extra_versioned_mod_update(self): ) self.assertTrue(len(result[ChangeType.LIBRARY_LEVEL_CHANGE]) == 1) config_change = result[ChangeType.LIBRARY_LEVEL_CHANGE][0] - self.assertEqual("extra versioned modules", config_change.changed_param) + self.assertEqual("extra_versioned_modules", config_change.changed_param) self.assertEqual("extra module", config_change.latest_value) self.assertEqual("existing_library", config_change.library_name) diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 2affc0bf7b..4fe8358230 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -13,6 +13,8 @@ # limitations under the License. from collections import defaultdict from enum import Enum +from typing import Any + from typing import Dict from typing import List @@ -66,43 +68,19 @@ def compare_config( :return: a mapping from ConfigChange to a list of ConfigChange objects. """ diff = defaultdict(list[ConfigChange]) - if baseline_config.googleapis_commitish != latest_config.googleapis_commitish: - diff[ChangeType.GOOGLEAPIS_COMMIT] = [] - if baseline_config.gapic_generator_version != latest_config.gapic_generator_version: - config_change = ConfigChange( - changed_param="GAPIC generator version", - latest_value=latest_config.gapic_generator_version, - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) - if baseline_config.owlbot_cli_image != latest_config.owlbot_cli_image: - config_change = ConfigChange( - changed_param="OwlBot cli commit", - latest_value=latest_config.owlbot_cli_image, - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) - if baseline_config.synthtool_commitish != latest_config.synthtool_commitish: - config_change = ConfigChange( - changed_param="Synthtool commit", - latest_value=latest_config.synthtool_commitish, - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) - if baseline_config.protobuf_version != latest_config.protobuf_version: - config_change = ConfigChange( - changed_param="Protobuf version", - latest_value=latest_config.protobuf_version, - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) - if baseline_config.grpc_version != latest_config.grpc_version: - config_change = ConfigChange( - changed_param="Grpc version", latest_value=latest_config.grpc_version - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) - if baseline_config.template_excludes != latest_config.template_excludes: - config_change = ConfigChange( - changed_param="Template excludes", - latest_value=str(latest_config.template_excludes), - ) - diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) + baseline_params = __convert_params_to_sorted_list(baseline_config) + latest_params = __convert_params_to_sorted_list(latest_config) + for baseline_param, latest_param in zip(baseline_params, latest_params): + if baseline_param == latest_param: + continue + if baseline_param[0] == "googleapis_commitish": + diff[ChangeType.GOOGLEAPIS_COMMIT] = [] + else: + config_change = ConfigChange( + changed_param=latest_param[0], + latest_value=latest_param[1], + ) + diff[ChangeType.REPO_LEVEL_CHANGE].append(config_change) __compare_libraries( diff=diff, @@ -205,153 +183,23 @@ def __compare_changed_libraries( for library_name in changed_libraries: baseline_library = baseline_libraries[library_name].library latest_library = latest_libraries[library_name].library - if baseline_library.api_shortname != latest_library.api_shortname: - raise ValueError( - f"{library_name}: api_shortname must not change when library_name remains the same." - ) - if baseline_library.api_description != latest_library.api_description: - config_change = ConfigChange( - changed_param="api description", - latest_value=latest_library.api_description, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.name_pretty != latest_library.name_pretty: - config_change = ConfigChange( - changed_param="name pretty", - latest_value=latest_library.name_pretty, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if ( - baseline_library.product_documentation - != latest_library.product_documentation - ): - config_change = ConfigChange( - changed_param="product documentation", - latest_value=latest_library.product_documentation, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.library_type != latest_library.library_type: - config_change = ConfigChange( - changed_param="library type", - latest_value=latest_library.library_type, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.release_level != latest_library.release_level: - config_change = ConfigChange( - changed_param="release level", - latest_value=latest_library.release_level, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.api_id != latest_library.api_id: - config_change = ConfigChange( - changed_param="api id", - latest_value=latest_library.api_id, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.api_reference != latest_library.api_reference: - config_change = ConfigChange( - changed_param="api reference", - latest_value=latest_library.api_reference, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.codeowner_team != latest_library.codeowner_team: - config_change = ConfigChange( - changed_param="code owner team", - latest_value=latest_library.codeowner_team, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if ( - baseline_library.excluded_dependencies - != latest_library.excluded_dependencies - ): - config_change = ConfigChange( - changed_param="excluded dependencies", - latest_value=latest_library.excluded_dependencies, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.excluded_poms != latest_library.excluded_poms: - config_change = ConfigChange( - changed_param="excluded poms", - latest_value=latest_library.excluded_poms, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.client_documentation != latest_library.client_documentation: - config_change = ConfigChange( - changed_param="client documentation", - latest_value=latest_library.client_documentation, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.distribution_name != latest_library.distribution_name: - config_change = ConfigChange( - changed_param="distribution name", - latest_value=latest_library.distribution_name, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.group_id != latest_library.group_id: - config_change = ConfigChange( - changed_param="group id", - latest_value=latest_library.group_id, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.issue_tracker != latest_library.issue_tracker: - config_change = ConfigChange( - changed_param="issue tracker", - latest_value=latest_library.issue_tracker, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.rest_documentation != latest_library.rest_documentation: - config_change = ConfigChange( - changed_param="rest documentation", - latest_value=latest_library.rest_documentation, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.rpc_documentation != latest_library.rpc_documentation: - config_change = ConfigChange( - changed_param="rpc documentation", - latest_value=latest_library.rpc_documentation, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.cloud_api != latest_library.cloud_api: - config_change = ConfigChange( - changed_param="cloud api", - latest_value=str(latest_library.cloud_api), - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if baseline_library.requires_billing != latest_library.requires_billing: - config_change = ConfigChange( - changed_param="requires billing", - latest_value=str(latest_library.requires_billing), - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) + baseline_params = __convert_params_to_sorted_list(baseline_library) + latest_params = __convert_params_to_sorted_list(latest_library) + for baseline_param, latest_param in zip(baseline_params, latest_params): + if baseline_param == latest_param: + continue + if baseline_param[0] == "api_shortname": + raise ValueError( + f"{library_name}: api_shortname must not change when library_name remains the same." + ) + else: + config_change = ConfigChange( + changed_param=latest_param[0], + latest_value=latest_param[1], + library_name=library_name, + ) + diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) - if ( - baseline_library.extra_versioned_modules - != latest_library.extra_versioned_modules - ): - config_change = ConfigChange( - changed_param="extra versioned modules", - latest_value=latest_library.extra_versioned_modules, - library_name=library_name, - ) - diff[ChangeType.LIBRARY_LEVEL_CHANGE].append(config_change) # compare gapic_configs baseline_gapic_configs = baseline_library.gapic_configs latest_gapic_configs = latest_library.gapic_configs @@ -392,3 +240,58 @@ def __compare_gapic_configs( changed_param="", latest_value=proto_path, library_name=library_name ) diff[ChangeType.GAPIC_ADDITION].append(config_change) + + +def __convert_params_to_sorted_list(obj: Any) -> List[tuple]: + """ + Convert the parameter and its value of a given object to a sorted list of + tuples. + + Only the following types of parameters will be considered: + + - str + - bool + - list[str] + - None + + Note that built-in params, e.g., __str__, and methods will be skipped. + + :param obj: an object + :return: a sorted list of tuples. + """ + param_and_values = [] + for param, value in vars(obj).items(): + if ( + param.startswith("__") # skip built-in params + or callable(getattr(obj, param)) # skip methods + # skip if the type of param is not one of the following types + # 1. str + # 2. bool + # 3. list[str] + # 4. None + or not ( + isinstance(getattr(obj, param), str) + or isinstance(getattr(obj, param), bool) + or __is_list_of_str(obj=obj, param=param) + or getattr(obj, param) is None + ) + ): + continue + param_and_values.append((param, value)) + return sorted(param_and_values) + + +def __is_list_of_str(obj: Any, param: str) -> bool: + """ + Returns True if the type of param of a given object is a list of str; False + otherwise. + + This method is a workaround of https://bugs.python.org/issue28339. + """ + value = getattr(obj, param) + if not isinstance(value, list): + return False + for v in value: + if not isinstance(v, str): + return False + return True