From 966566f1461ef5c408569531790a4f23af1c8513 Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Sat, 4 May 2024 00:59:49 +0000 Subject: [PATCH] fix: only append `.api.grpc` suffix to group id if the artifact id starts with `proto-` or `grpc-` (#2731) In this PR: - Only append `.api.grpc` suffix to group id if the corresponding artifact id starts with `proto-` or `grpc-`. We need to support `ad-manager`, which is the first artifact id doesn't start with `proto-`, `grpc-` and `google-`. --- library_generation/README.md | 2 +- library_generation/owlbot/bin/entrypoint.sh | 2 +- .../owlbot/src/{fix-poms.py => fix_poms.py} | 19 +-- library_generation/setup.py | 2 + library_generation/test/compare_poms.py | 10 ++ library_generation/test/owlbot/__init__.py | 0 .../test/owlbot/fix_poms_unit_tests.py | 47 ++++++++ .../java-admanager/.repo-metadata.json | 16 +++ .../ad-manager-bom/pom-golden.xml | 38 ++++++ .../java-admanager/ad-manager/pom-golden.xml | 108 ++++++++++++++++++ .../test-owlbot/java-admanager/pom-golden.xml | 49 ++++++++ .../proto-ad-manager-v1/pom-golden.xml | 37 ++++++ .../test-owlbot/java-admanager/versions.txt | 6 + 13 files changed, 327 insertions(+), 9 deletions(-) rename library_generation/owlbot/src/{fix-poms.py => fix_poms.py} (96%) create mode 100644 library_generation/test/owlbot/__init__.py create mode 100644 library_generation/test/owlbot/fix_poms_unit_tests.py create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/.repo-metadata.json create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/ad-manager-bom/pom-golden.xml create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/pom-golden.xml create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/proto-ad-manager-v1/pom-golden.xml create mode 100644 library_generation/test/resources/test-owlbot/java-admanager/versions.txt diff --git a/library_generation/README.md b/library_generation/README.md index b94fe2de53..41e481c201 100644 --- a/library_generation/README.md +++ b/library_generation/README.md @@ -300,7 +300,7 @@ The transfer was not a verbatim copy, it rather had modifications: * `entrypoint.sh` was modified to have input arguments and slightly modified the way the helper scripts are called * Other helper scripts were modified to have input arguments. - * `fix-poms.py` modified the way the monorepo is detected + * `fix_poms.py` modified the way the monorepo is detected All these modifications imply that whenever we want to reflect a change from the original owlbot in synthtool we may be better off modifying the affected source diff --git a/library_generation/owlbot/bin/entrypoint.sh b/library_generation/owlbot/bin/entrypoint.sh index 789737b2a5..51c0b619d9 100755 --- a/library_generation/owlbot/bin/entrypoint.sh +++ b/library_generation/owlbot/bin/entrypoint.sh @@ -57,7 +57,7 @@ echo "...done" # write or restore pom.xml files echo "Generating missing pom.xml..." -python3 "${scripts_root}/owlbot/src/fix-poms.py" "${versions_file}" "${is_monorepo}" +python3 "${scripts_root}/owlbot/src/fix_poms.py" "${versions_file}" "${is_monorepo}" echo "...done" # write or restore clirr-ignored-differences.xml diff --git a/library_generation/owlbot/src/fix-poms.py b/library_generation/owlbot/src/fix_poms.py similarity index 96% rename from library_generation/owlbot/src/fix-poms.py rename to library_generation/owlbot/src/fix_poms.py index 1de814e9ce..b88cd3d6bb 100644 --- a/library_generation/owlbot/src/fix-poms.py +++ b/library_generation/owlbot/src/fix_poms.py @@ -14,15 +14,12 @@ import sys import glob -import inspect -import itertools import json from lxml import etree import os import re from typing import List, Mapping -from poms import module, templates -from pathlib import Path +from library_generation.owlbot.src.poms import module, templates def load_versions(filename: str, default_group_id: str) -> Mapping[str, module.Module]: @@ -39,9 +36,17 @@ def load_versions(filename: str, default_group_id: str) -> Mapping[str, module.M if len(parts) == 3: artifact_id = parts[0] group_id = ( - default_group_id - if artifact_id.startswith("google-") - else __proto_group_id(default_group_id) + # For artifact id starts with `proto-` or `grpc-`, we + # need special treatments to append `.api.grpc` suffix + # to its corresponding group id. + # For other artifact id, keep the existing group id. + # Other than the two aforementioned artifact id, do not + # assume artifact id always starts with `google-`. Known + # exception is ad-manager. + __proto_group_id(default_group_id) + if artifact_id.startswith("proto-") + or artifact_id.startswith("grpc-") + else default_group_id ) modules[artifact_id] = module.Module( group_id=group_id, diff --git a/library_generation/setup.py b/library_generation/setup.py index bd17cc5fcf..240d9264b3 100755 --- a/library_generation/setup.py +++ b/library_generation/setup.py @@ -19,6 +19,8 @@ "gapic-generator-java-wrapper", "requirements.*", "owlbot/bin/*.sh", + "owlbot/src/*.py", + "owlbot/src/poms/*.py", "owlbot/templates/clirr/*.j2", "owlbot/templates/poms/*.j2", "owlbot/templates/java_library/**/*", diff --git a/library_generation/test/compare_poms.py b/library_generation/test/compare_poms.py index 11cc77c8b4..95041651fb 100644 --- a/library_generation/test/compare_poms.py +++ b/library_generation/test/compare_poms.py @@ -102,6 +102,16 @@ def compare_xml(expected, actual, print_trees): return False +def compare_pom_in_subdir(base_dir: str, subdir: str) -> bool: + golden = os.path.join(base_dir, subdir, "pom-golden.xml") + pom = os.path.join(base_dir, subdir, "pom.xml") + return compare_xml( + golden, + pom, + False, + ) + + if __name__ == "__main__": if len(sys.argv) != 4: eprint( diff --git a/library_generation/test/owlbot/__init__.py b/library_generation/test/owlbot/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/test/owlbot/fix_poms_unit_tests.py b/library_generation/test/owlbot/fix_poms_unit_tests.py new file mode 100644 index 0000000000..496ae49383 --- /dev/null +++ b/library_generation/test/owlbot/fix_poms_unit_tests.py @@ -0,0 +1,47 @@ +# 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 os +import shutil +import unittest +from library_generation.owlbot.src.fix_poms import main +from library_generation.test.compare_poms import compare_pom_in_subdir + +script_dir = os.path.dirname(os.path.realpath(__file__)) +resources_dir = os.path.join(script_dir, "..", "resources", "test-owlbot") + + +class FixPomsTest(unittest.TestCase): + def test_update_poms_group_id_does_not_start_with_google_correctly(self): + ad_manager_resource = os.path.join(resources_dir, "java-admanager") + versions_file = os.path.join(ad_manager_resource, "versions.txt") + os.chdir(ad_manager_resource) + sub_dirs = ["ad-manager", "ad-manager-bom", "proto-ad-manager-v1", "."] + for sub_dir in sub_dirs: + self.__copy__golden(ad_manager_resource, sub_dir) + main(versions_file, "true") + for sub_dir in sub_dirs: + self.assertFalse(compare_pom_in_subdir(ad_manager_resource, sub_dir)) + for sub_dir in sub_dirs: + self.__remove_file_in_subdir(ad_manager_resource, sub_dir) + + @classmethod + def __copy__golden(cls, base_dir: str, subdir: str): + golden = os.path.join(base_dir, subdir, "pom-golden.xml") + pom = os.path.join(base_dir, subdir, "pom.xml") + shutil.copyfile(golden, pom) + + @classmethod + def __remove_file_in_subdir(cls, base_dir: str, subdir: str): + pom = os.path.join(base_dir, subdir, "pom.xml") + os.unlink(pom) diff --git a/library_generation/test/resources/test-owlbot/java-admanager/.repo-metadata.json b/library_generation/test/resources/test-owlbot/java-admanager/.repo-metadata.json new file mode 100644 index 0000000000..1c680ddfea --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/.repo-metadata.json @@ -0,0 +1,16 @@ +{ + "api_shortname": "admanager", + "name_pretty": "Google Ad Manager API", + "product_documentation": "https://developers.google.com/ad-manager/api/beta", + "api_description": "The Ad Manager API enables an app to integrate with Google Ad Manager. You can read Ad Manager data and run reports using the API.", + "client_documentation": "https://cloud.google.com/java/docs/reference/ad-manager/latest/overview", + "release_level": "preview", + "transport": "http", + "language": "java", + "repo": "googleapis/google-cloud-java", + "repo_short": "java-admanager", + "distribution_name": "com.google.api-ads:ad-manager", + "api_id": "admanager.googleapis.com", + "library_type": "GAPIC_AUTO", + "requires_billing": true +} \ No newline at end of file diff --git a/library_generation/test/resources/test-owlbot/java-admanager/ad-manager-bom/pom-golden.xml b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager-bom/pom-golden.xml new file mode 100644 index 0000000000..2adae69c60 --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager-bom/pom-golden.xml @@ -0,0 +1,38 @@ + + + 4.0.0 + com.google.api-ads + ad-manager-bom + 0.2.0-SNAPSHOT + pom + + com.google.cloud + google-cloud-pom-parent + 1.37.0-SNAPSHOT + ../../google-cloud-pom-parent/pom.xml + + + Google Google Ad Manager API BOM + + BOM for Google Ad Manager API + + + + true + + + + + + com.google.api-ads + ad-manager + 0.2.0-SNAPSHOT + + + com.google.api-ads.api.grpc + proto-ad-manager-v1 + 0.2.0-SNAPSHOT + + + + diff --git a/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml new file mode 100644 index 0000000000..106b55c8c5 --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/ad-manager/pom-golden.xml @@ -0,0 +1,108 @@ + + + 4.0.0 + com.google.api-ads + ad-manager + 0.2.0-SNAPSHOT + jar + Google Google Ad Manager API + Google Ad Manager API The Ad Manager API enables an app to integrate with Google Ad Manager. You can read Ad Manager data and run reports using the API. + + com.google.api-ads + ad-manager-parent + 0.2.0-SNAPSHOT + + + ad-manager + + + + io.grpc + grpc-api + + + io.grpc + grpc-stub + + + io.grpc + grpc-protobuf + + + com.google.api + api-common + + + com.google.protobuf + protobuf-java + + + com.google.api.grpc + proto-google-common-protos + + + + com.google.api-ads.api.grpc + proto-ad-manager-v1 + + + com.google.guava + guava + + + com.google.api + gax + + + com.google.api + gax-grpc + + + com.google.api + gax-httpjson + + + com.google.api.grpc + grpc-google-common-protos + + + com.google.api.grpc + proto-google-iam-v1 + + + com.google.api.grpc + grpc-google-iam-v1 + + + org.threeten + threetenbp + + + + + junit + junit + test + + + + + com.google.api + gax + testlib + test + + + com.google.api + gax-grpc + testlib + test + + + com.google.api + gax-httpjson + testlib + test + + + diff --git a/library_generation/test/resources/test-owlbot/java-admanager/pom-golden.xml b/library_generation/test/resources/test-owlbot/java-admanager/pom-golden.xml new file mode 100644 index 0000000000..669d294ae7 --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/pom-golden.xml @@ -0,0 +1,49 @@ + + + 4.0.0 + com.google.api-ads + ad-manager-parent + pom + 0.2.0-SNAPSHOT + Google Google Ad Manager API Parent + + Java idiomatic client for Google Cloud Platform services. + + + + com.google.cloud + google-cloud-jar-parent + 1.37.0-SNAPSHOT + ../google-cloud-jar-parent/pom.xml + + + + UTF-8 + UTF-8 + github + ad-manager-parent + + + + + + com.google.api-ads + ad-manager + 0.2.0-SNAPSHOT + + + com.google.api-ads.api.grpc + proto-ad-manager-v1 + 0.2.0-SNAPSHOT + + + + + + + ad-manager + proto-ad-manager-v1 + ad-manager-bom + + + diff --git a/library_generation/test/resources/test-owlbot/java-admanager/proto-ad-manager-v1/pom-golden.xml b/library_generation/test/resources/test-owlbot/java-admanager/proto-ad-manager-v1/pom-golden.xml new file mode 100644 index 0000000000..eef103e533 --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/proto-ad-manager-v1/pom-golden.xml @@ -0,0 +1,37 @@ + + 4.0.0 + com.google.api-ads.api.grpc + proto-ad-manager-v1 + 0.2.0-SNAPSHOT + proto-ad-manager-v1 + Proto library for ad-manager + + com.google.api-ads + ad-manager-parent + 0.2.0-SNAPSHOT + + + + com.google.protobuf + protobuf-java + + + com.google.api.grpc + proto-google-common-protos + + + com.google.api.grpc + proto-google-iam-v1 + + + com.google.api + api-common + + + com.google.guava + guava + + + diff --git a/library_generation/test/resources/test-owlbot/java-admanager/versions.txt b/library_generation/test/resources/test-owlbot/java-admanager/versions.txt new file mode 100644 index 0000000000..3f073728f3 --- /dev/null +++ b/library_generation/test/resources/test-owlbot/java-admanager/versions.txt @@ -0,0 +1,6 @@ +# Format: +# module:released-version:current-version + +google-cloud-java:1.36.0:1.37.0-SNAPSHOT +ad-manager:0.1.0:0.2.0-SNAPSHOT +proto-ad-manager-v1:0.1.0:0.2.0-SNAPSHOT