From 9c0316bfed890ed7d8e40340a3510d2058dc5d71 Mon Sep 17 00:00:00 2001 From: Joe Wang <106995533+JoeWang1127@users.noreply.github.com> Date: Thu, 14 Sep 2023 00:18:12 +0000 Subject: [PATCH] fix: skip generating `grpc-*` directory if transport is `rest` (#1979) * chore: add proto_path in integration test * exclude gapic_metadata.json and package-info.java * add additional protos * search transport twice * skip grpc-* if transport is rest * refactor utility functions * add utility function * add tests * restore workflow * restore diff command * remove unrelated change --- library_generation/generate_library.sh | 21 ++- .../test/generate_library_integration_test.sh | 21 +-- .../test/generate_library_unit_tests.sh | 106 +++++++---- .../test/resources/misc/BUILD_grpc.bazel | 5 + .../test/resources/misc/BUILD_grpc_rest.bazel | 5 + .../misc/BUILD_include_samples_empty.bazel | 5 + .../misc/BUILD_include_samples_false.bazel | 5 + .../misc/BUILD_include_samples_true.bazel | 5 + .../test/resources/misc/BUILD_rest.bazel | 5 + .../misc/BUILD_rest_numeric_enums_empty.bazel | 5 + .../misc/BUILD_rest_numeric_enums_false.bazel | 5 + .../misc/BUILD_rest_numeric_enums_true.bazel | 5 + .../test/resources/misc/TESTBUILD.bazel | 166 ------------------ library_generation/utilities.sh | 82 +++++++-- 14 files changed, 195 insertions(+), 246 deletions(-) create mode 100644 library_generation/test/resources/misc/BUILD_grpc.bazel create mode 100644 library_generation/test/resources/misc/BUILD_grpc_rest.bazel create mode 100644 library_generation/test/resources/misc/BUILD_include_samples_empty.bazel create mode 100644 library_generation/test/resources/misc/BUILD_include_samples_false.bazel create mode 100644 library_generation/test/resources/misc/BUILD_include_samples_true.bazel create mode 100644 library_generation/test/resources/misc/BUILD_rest.bazel create mode 100644 library_generation/test/resources/misc/BUILD_rest_numeric_enums_empty.bazel create mode 100644 library_generation/test/resources/misc/BUILD_rest_numeric_enums_false.bazel create mode 100644 library_generation/test/resources/misc/BUILD_rest_numeric_enums_true.bazel delete mode 100644 library_generation/test/resources/misc/TESTBUILD.bazel diff --git a/library_generation/generate_library.sh b/library_generation/generate_library.sh index 8355b4a6a5..d48f19fae5 100755 --- a/library_generation/generate_library.sh +++ b/library_generation/generate_library.sh @@ -95,15 +95,18 @@ download_tools "${gapic_generator_version}" "${protobuf_version}" "${grpc_versio ##################### Section 1 ##################### # generate grpc-*/ ##################################################### -"${protoc_path}"/protoc "--plugin=protoc-gen-rpc-plugin=protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" \ -"--rpc-plugin_out=:${destination_path}/java_grpc.jar" \ -${proto_files} # Do not quote because this variable should not be treated as one long string. -# unzip java_grpc.jar to grpc-*/src/main/java -unzip_src_files "grpc" -# remove empty files in grpc-*/src/main/java -remove_empty_files "grpc" -# remove grpc version in *ServiceGrpc.java file so the content is identical with bazel build. -remove_grpc_version +if [[ ! "${transport}" == "rest" ]]; then + # do not need to generate grpc-* if the transport is `rest`. + "${protoc_path}"/protoc "--plugin=protoc-gen-rpc-plugin=protoc-gen-grpc-java-${grpc_version}-${os_architecture}.exe" \ + "--rpc-plugin_out=:${destination_path}/java_grpc.jar" \ + ${proto_files} # Do not quote because this variable should not be treated as one long string. + # unzip java_grpc.jar to grpc-*/src/main/java + unzip_src_files "grpc" + # remove empty files in grpc-*/src/main/java + remove_empty_files "grpc" + # remove grpc version in *ServiceGrpc.java file so the content is identical with bazel build. + remove_grpc_version +fi ###################### Section 2 ##################### ## generate gapic-*/, part of proto-*/, samples/ ###################################################### diff --git a/library_generation/test/generate_library_integration_test.sh b/library_generation/test/generate_library_integration_test.sh index fa1585346e..391b46c243 100755 --- a/library_generation/test/generate_library_integration_test.sh +++ b/library_generation/test/generate_library_integration_test.sh @@ -55,24 +55,9 @@ grpc_version=$(get_version_from_WORKSPACE "_grpc_version" WORKSPACE "=") echo "The version of protoc-gen-grpc-java plugin is ${gapic_generator_version}." # parse GAPIC options from proto_path/BUILD.bazel proto_build_file_path="${proto_path}/BUILD.bazel" -transport=$(get_config_from_BUILD \ - "${proto_build_file_path}" \ - "java_gapic_library(" \ - "grpc+rest" \ - "grpc" -) -rest_numeric_enums=$(get_config_from_BUILD \ - "${proto_build_file_path}" \ - "java_gapic_library(" \ - "rest_numeric_enums = False" \ - "true" -) -include_samples=$(get_config_from_BUILD \ - "${proto_build_file_path}" \ - "java_gapic_assembly_gradle_pkg(" \ - "include_samples = True" \ - "false" -) +transport=$(get_transport_from_BUILD "${proto_build_file_path}") +rest_numeric_enums=$(get_rest_numeric_enums_from_BUILD "${proto_build_file_path}") +include_samples=$(get_include_samples_from_BUILD "${proto_build_file_path}") echo "GAPIC options are transport=${transport}, rest_numeric_enums=${rest_numeric_enums}, include_samples=${include_samples}." # generate GAPIC client library echo "Generating library from ${proto_path}, to ${destination_path}..." diff --git a/library_generation/test/generate_library_unit_tests.sh b/library_generation/test/generate_library_unit_tests.sh index 285f6dbe11..b74b0f6215 100755 --- a/library_generation/test/generate_library_unit_tests.sh +++ b/library_generation/test/generate_library_unit_tests.sh @@ -230,40 +230,67 @@ generate_library_failed_with_invalid_grpc_version() { cleanup "${destination}" } -get_config_from_valid_BUILD_matched_test() { - build_file="${script_dir}/resources/misc/TESTBUILD.bazel" - rule="java_gapic_library(" - # the pattern we expect to find in the BUILD file - pattern_should_match="name" - # default value if the pattern was not found - if_matched_return="got-a-match" - if_not_matched_return="no-match" - pattern_matched_result=$(get_config_from_BUILD \ - "${build_file}" \ - "${rule}" \ - "${pattern_should_match}" \ - "${if_not_matched_return}" \ - "${if_matched_return}" - ) - assertEquals "${if_matched_return}" "${pattern_matched_result}" -} - -get_config_from_valid_BUILD_not_match_test() { - build_file="${script_dir}/resources/misc/TESTBUILD.bazel" - rule="java_gapic_library(" - # the pattern that we should not find in the BUILD file - pattern_should_not_match="should-not-match" - # default value if the pattern was not found - if_matched_return="got-a-match" - if_not_matched_return="no-match" - pattern_not_matched_result=$(get_config_from_BUILD \ - "${build_file}" \ - "${rule}" \ - "${pattern_should_not_match}" \ - "${if_not_matched_return}" \ - "${if_matched_return}" - ) - assertEquals "${if_not_matched_return}" "${pattern_not_matched_result}" +get_transport_from_BUILD_grpc_rest_test() { + local build_file="${script_dir}/resources/misc/BUILD_grpc_rest.bazel" + local transport + transport=$(get_transport_from_BUILD "${build_file}") + assertEquals "grpc+rest" "${transport}" +} + +get_transport_from_BUILD_grpc_test() { + local build_file="${script_dir}/resources/misc/BUILD_grpc.bazel" + local transport + transport=$(get_transport_from_BUILD "${build_file}") + assertEquals "grpc" "${transport}" +} + +get_transport_from_BUILD_rest_test() { + local build_file="${script_dir}/resources/misc/BUILD_rest.bazel" + local transport + transport=$(get_transport_from_BUILD "${build_file}") + assertEquals "rest" "${transport}" +} + +get_rest_numeric_enums_from_BUILD_true_test() { + local build_file="${script_dir}/resources/misc/BUILD_rest_numeric_enums_true.bazel" + local rest_numeric_enums + rest_numeric_enums=$(get_rest_numeric_enums_from_BUILD "${build_file}") + assertEquals "true" "${rest_numeric_enums}" +} + +get_rest_numeric_enums_from_BUILD_false_test() { + local build_file="${script_dir}/resources/misc/BUILD_rest_numeric_enums_false.bazel" + local rest_numeric_enums + rest_numeric_enums=$(get_rest_numeric_enums_from_BUILD "${build_file}") + assertEquals "false" "${rest_numeric_enums}" +} + +get_rest_numeric_enums_from_BUILD_empty_test() { + local build_file="${script_dir}/resources/misc/BUILD_rest_numeric_enums_empty.bazel" + local rest_numeric_enums + rest_numeric_enums=$(get_rest_numeric_enums_from_BUILD "${build_file}") + assertEquals "false" "${rest_numeric_enums}" +} + +get_include_samples_from_BUILD_true_test() { + local build_file="${script_dir}/resources/misc/BUILD_include_samples_true.bazel" + local include_samples + include_samples=$(get_include_samples_from_BUILD "${build_file}") + assertEquals "true" "${include_samples}" +} + +get_include_samples_from_BUILD_false_test() { + local build_file="${script_dir}/resources/misc/BUILD_include_samples_false.bazel" + local include_samples + include_samples=$(get_include_samples_from_BUILD "${build_file}") + assertEquals "false" "${include_samples}" +} + +get_include_samples_from_BUILD_empty_test() { + local build_file="${script_dir}/resources/misc/BUILD_include_samples_empty.bazel" + local include_samples + include_samples=$(get_include_samples_from_BUILD "${build_file}") + assertEquals "false" "${include_samples}" } get_version_from_valid_WORKSPACE_test() { @@ -312,8 +339,15 @@ test_list=( generate_library_failed_with_invalid_generator_version generate_library_failed_with_invalid_protobuf_version generate_library_failed_with_invalid_grpc_version - get_config_from_valid_BUILD_matched_test - get_config_from_valid_BUILD_not_match_test + get_transport_from_BUILD_grpc_rest_test + get_transport_from_BUILD_grpc_test + get_transport_from_BUILD_rest_test + get_rest_numeric_enums_from_BUILD_true_test + get_rest_numeric_enums_from_BUILD_false_test + get_rest_numeric_enums_from_BUILD_empty_test + get_include_samples_from_BUILD_true_test + get_include_samples_from_BUILD_false_test + get_include_samples_from_BUILD_empty_test get_version_from_valid_WORKSPACE_test get_generator_version_from_valid_versions_txt_test get_gax_version_from_valid_versions_txt_test diff --git a/library_generation/test/resources/misc/BUILD_grpc.bazel b/library_generation/test/resources/misc/BUILD_grpc.bazel new file mode 100644 index 0000000000..3e59a9bd35 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_grpc.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_transport_from_BUILD` in utilities.sh + +java_gapic_library( + transport = "grpc", +) diff --git a/library_generation/test/resources/misc/BUILD_grpc_rest.bazel b/library_generation/test/resources/misc/BUILD_grpc_rest.bazel new file mode 100644 index 0000000000..8a98e7e646 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_grpc_rest.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_transport_from_BUILD` in utilities.sh + +java_gapic_library( + transport = "grpc+rest", +) diff --git a/library_generation/test/resources/misc/BUILD_include_samples_empty.bazel b/library_generation/test/resources/misc/BUILD_include_samples_empty.bazel new file mode 100644 index 0000000000..b9d4cd56e0 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_include_samples_empty.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_include_samples_from_BUILD` in utilities.sh + +java_gapic_assembly_gradle_pkg( + +) diff --git a/library_generation/test/resources/misc/BUILD_include_samples_false.bazel b/library_generation/test/resources/misc/BUILD_include_samples_false.bazel new file mode 100644 index 0000000000..8e95c3d2a6 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_include_samples_false.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_include_samples_from_BUILD` in utilities.sh + +java_gapic_assembly_gradle_pkg( + include_samples = False, +) diff --git a/library_generation/test/resources/misc/BUILD_include_samples_true.bazel b/library_generation/test/resources/misc/BUILD_include_samples_true.bazel new file mode 100644 index 0000000000..bac72b678f --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_include_samples_true.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_include_samples_from_BUILD` in utilities.sh + +java_gapic_assembly_gradle_pkg( + include_samples = True, +) diff --git a/library_generation/test/resources/misc/BUILD_rest.bazel b/library_generation/test/resources/misc/BUILD_rest.bazel new file mode 100644 index 0000000000..9dff694297 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_rest.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_transport_from_BUILD` in utilities.sh + +java_gapic_library( + transport = "rest", +) diff --git a/library_generation/test/resources/misc/BUILD_rest_numeric_enums_empty.bazel b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_empty.bazel new file mode 100644 index 0000000000..992b91e52c --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_empty.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_rest_numeric_enums_from_BUILD` in utilities.sh + +java_gapic_library( + +) diff --git a/library_generation/test/resources/misc/BUILD_rest_numeric_enums_false.bazel b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_false.bazel new file mode 100644 index 0000000000..a446c6b2a7 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_false.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_rest_numeric_enums_from_BUILD` in utilities.sh + +java_gapic_library( + rest_numeric_enums = False +) diff --git a/library_generation/test/resources/misc/BUILD_rest_numeric_enums_true.bazel b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_true.bazel new file mode 100644 index 0000000000..c4d7fefeb4 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_rest_numeric_enums_true.bazel @@ -0,0 +1,5 @@ +# this file is only used in testing `get_rest_numeric_enums_from_BUILD` in utilities.sh + +java_gapic_library( + rest_numeric_enums = True +) diff --git a/library_generation/test/resources/misc/TESTBUILD.bazel b/library_generation/test/resources/misc/TESTBUILD.bazel deleted file mode 100644 index 21d38a35b9..0000000000 --- a/library_generation/test/resources/misc/TESTBUILD.bazel +++ /dev/null @@ -1,166 +0,0 @@ -# Test BUILD file, copied from sdk-platform-java/showcase/BUILD.bazel - -load( - "//rules_java_gapic:java_gapic.bzl", - "java_gapic_library", - "java_gapic_test", -) -load("@io_grpc_grpc_java//:java_grpc_library.bzl", "java_grpc_library") -load("@rules_gapic//:gapic.bzl", "proto_library_with_info") -load("@rules_proto//proto:defs.bzl", "proto_library") -load("//rules_java_gapic:java_gapic_pkg.bzl", "java_gapic_assembly_gradle_pkg") - -package(default_visibility = ["//visibility:public"]) - -proto_library_with_info( - name = "showcase_proto_with_info", - deps = [ - "@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_proto", -# "//showcase/gapic-showcase-extended/proto:showcase_proto_extended", - "@com_google_googleapis//google/cloud:common_resources_proto", - "@com_google_googleapis//google/cloud/location:location_proto", - "@com_google_googleapis//google/iam/v1:iam_policy_proto" - ], -) - -java_proto_library( - name = "showcase_java_proto", - deps = [ - "@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_proto", -# "//showcase/gapic-showcase-extended/proto:showcase_proto_extended" -], -) - -java_grpc_library( - name = "showcase_java_grpc", - srcs = ["@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_proto"], - deps = [":showcase_java_proto"], -) - -#java_grpc_library( -# name = "showcase_java_grpc_extended", -# srcs = [ -# "//showcase/gapic-showcase-extended/proto:showcase_proto_extended", -# ], -# deps = [":showcase_java_proto"], -#) - -java_gapic_library( - name = "showcase_java_gapic", - srcs = [":showcase_proto_with_info"], - gapic_yaml = None, - grpc_service_config = "@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_grpc_service_config.json", - # TODO(#1285): Enable rest_numeric_enums once https://github.com/googleapis/gapic-showcase/issues/1255 is - # fixed. - rest_numeric_enums = False, - service_yaml = "@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_v1beta1.yaml", - test_deps = [ - ":showcase_java_grpc", -# ":showcase_java_grpc_extended", - "@com_google_googleapis//google/cloud/location:location_java_grpc", - "@com_google_googleapis//google/iam/v1:iam_java_grpc" - ], - transport = "grpc+rest", - deps = [ - ":showcase_java_proto", - "@com_google_googleapis//google/api:api_java_proto", - "@com_google_googleapis//google/cloud/location:location_java_proto", - "@com_google_googleapis//google/iam/v1:iam_java_proto" - ], -) - -# Open Source Packages -java_gapic_assembly_gradle_pkg( - name = "google-cloud-showcase-v1beta1-java", - transport = "grpc+rest", - deps = [ - ":showcase_java_gapic", - # TODO(lawrenceqiu): Not adding :showcase_java_grpc_extended dep as that includes WickedGrpc.java - # Need to figure out why it's being included - ":showcase_java_grpc", - ":showcase_java_proto", - "@com_google_gapic_showcase//schema/google/showcase/v1beta1:showcase_proto", - ], -) - -# Golden File Directories -filegroup( - name = "gapic_showcase_files", - srcs = glob(["gapic-showcase/src/**"]), -) - -filegroup( - name = "grpc_gapic_showcase_files", - srcs = glob(["grpc-gapic-showcase-v1beta1/src/**"]), -) - -filegroup( - name = "proto_gapic_showcase_files", - srcs = glob(["proto-gapic-showcase-v1beta1/src/**"]), -) - -# GAPIC Showcase : Update and Verify -GAPIC_DATA = [ - "showcase_java_gapic_srcjar_raw.srcjar", - ":gapic_showcase_files", - "//showcase:showcase_java_gapic", -] - -sh_binary( - name = "update_gapic", - srcs = ["//showcase/scripts:update.sh"], - args = ["gapic"], - data = GAPIC_DATA, -) - -sh_binary( - name = "verify_gapic", - srcs = ["//showcase/scripts:verify.sh"], - args = ["gapic"], - data = GAPIC_DATA, -) - -# GRPC Showcase : Update and Verify -GRPC_DATA = [ - "libshowcase_java_grpc-src.jar", -# "libshowcase_java_grpc_extended-src.jar", - ":grpc_gapic_showcase_files", - ":showcase_java_grpc", -# ":showcase_java_grpc_extended" -] - -sh_binary( - name = "update_grpc", - srcs = ["//showcase/scripts:update.sh"], - args = ["grpc"], - data = GRPC_DATA, -) - -sh_binary( - name = "verify_grpc", - srcs = ["//showcase/scripts:verify.sh"], - args = ["grpc"], - data = GRPC_DATA, -) - -# Proto Showcase : Update and Verify -PROTO_DATA = [ - "proto-google-cloud-showcase-v1beta1-java.tar.gz", - ":proto_gapic_showcase_files", - ":showcase_java_proto", -# ":showcase_java_proto_extended", -] - -sh_binary( - name = "update_proto", - srcs = ["//showcase/scripts:update.sh"], - args = ["proto"], - data = PROTO_DATA, -) - -sh_binary( - name = "verify_proto", - srcs = ["//showcase/scripts:verify.sh"], - args = ["proto"], - data = PROTO_DATA, -) diff --git a/library_generation/utilities.sh b/library_generation/utilities.sh index d7647f217a..804d964fc9 100755 --- a/library_generation/utilities.sh +++ b/library_generation/utilities.sh @@ -2,8 +2,28 @@ set -xeo pipefail -# define utility functions +# private functions that should not be called outside this file. +# Used to obtain configuration values from a bazel BUILD file +# +# inspects a $build_file for a certain $rule (e.g. java_gapic_library). If the +# first 15 lines after the declaration of the rule contain $pattern, then +# it will return $if_match if $pattern is found, otherwise $default +__get_config_from_BUILD() { + build_file=$1 + rule=$2 + pattern=$3 + default=$4 + if_match=$5 + + result="${default}" + if grep -A 15 "${rule}" "${build_file}" | grep -q "${pattern}"; then + result="${if_match}" + fi + echo "${result}" +} + +# define utility functions extract_folder_name() { local destination_path=$1 local folder_name=${destination_path##*/} @@ -230,23 +250,51 @@ get_version_from_WORKSPACE() { echo "${version}" } -# Used to obtain configuration values from a bazel BUILD file -# -# inspects a $build_file for a certain $rule (e.g. java_gapic_library). If the -# first 15 lines after the declaration of the rule contain $pattern, then -# it will return $if_match if $pattern is found, otherwise $default -get_config_from_BUILD() { - build_file=$1 - rule=$2 - pattern=$3 - default=$4 - if_match=$5 +get_transport_from_BUILD() { + local build_file=$1 + local transport + transport=$(__get_config_from_BUILD \ + "${build_file}" \ + "java_gapic_library(" \ + "grpc+rest" \ + "grpc" \ + "grpc+rest" + ) + # search again because the transport maybe `rest`. + transport=$(__get_config_from_BUILD \ + "${build_file}" \ + "java_gapic_library(" \ + "transport = \"rest\"" \ + "${transport}" \ + "rest" + ) + echo "${transport}" +} - result="${default}" - if grep -A 15 "${rule}" "${build_file}" | grep -q "${pattern}"; then - result="${if_match}" - fi - echo "${result}" +get_rest_numeric_enums_from_BUILD() { + local build_file=$1 + local rest_numeric_enums + rest_numeric_enums=$(__get_config_from_BUILD \ + "${build_file}" \ + "java_gapic_library(" \ + "rest_numeric_enums = True" \ + "false" \ + "true" + ) + echo "${rest_numeric_enums}" +} + +get_include_samples_from_BUILD() { + local build_file=$1 + local include_samples + include_samples=$(__get_config_from_BUILD \ + "${build_file}" \ + "java_gapic_assembly_gradle_pkg(" \ + "include_samples = True" \ + "false" \ + "true" + ) + echo "${include_samples}" } # Convenience function to clone only the necessary folders from a git repository