From d023f2456179b1aeff718ed2770b99c45277774b Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 13:18:20 -0700 Subject: [PATCH 1/7] fix: support LRO package.name annotations --- .../api/generator/gapic/protoparser/Parser.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index f667402210..ec7d6a83d5 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -64,6 +64,7 @@ public class Parser { private static final String COMMA = ","; private static final String COLON = ":"; + private static final String DOT = "."; private static final String DEFAULT_PORT = "443"; // Allow other parsers to access this. @@ -320,12 +321,23 @@ static LongrunningOperation parseLro( OperationInfo lroInfo = methodDescriptor.getOptions().getExtension(OperationsProto.operationInfo); + String responseTypeName = lroInfo.getResponseType(); + String responseTypePackage = ""; + if (responseTypeName.contains(DOT)) { + responseTypeName = responseTypeName.substring(responseTypeName.lastIndexOf(DOT) + 1); + } + String metadataTypeName = lroInfo.getMetadataType(); + if (metadataTypeName.contains(DOT)) { + metadataTypeName = metadataTypeName.substring(metadataTypeName.lastIndexOf(DOT) + 1); + } + Message responseMessage = messageTypes.get(responseTypeName); Message metadataMessage = messageTypes.get(metadataTypeName); Preconditions.checkNotNull( responseMessage, String.format("LRO response message %s not found", responseTypeName)); + // TODO(miraleung): Check that the packages are equal if those strings are not empty. Preconditions.checkNotNull( metadataMessage, String.format("LRO metadata message %s not found", metadataTypeName)); From baa5aa6882402307f76d00e05542e67ff8fb87e5 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 13:23:47 -0700 Subject: [PATCH 2/7] feat: add microgenerator rules for cloud/asset --- .githooks/pre-commit | 4 ++-- repositories.bzl | 4 ++-- test/integration/BUILD.bazel | 26 ++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 test/integration/BUILD.bazel diff --git a/.githooks/pre-commit b/.githooks/pre-commit index a0ef4987d4..1a1c841589 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -100,7 +100,7 @@ fi if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] || [ $NUM_BAZEL_FILES_CHANGED -gt 0 ] then echo_status "Checking the build..." - bazel --batch build --disk_cache="$BAZEL_CACHE_DIR" //... + bazel --batch build --disk_cache="$BAZEL_CACHE_DIR" //src/... BUILD_STATUS=$? if [ $BUILD_STATUS != 0 ] then @@ -112,7 +112,7 @@ fi if [ $NUM_JAVA_FILES_CHANGED -gt 0 ] then echo_status "Checking tests..." - bazel --batch test --disk_cache="$BAZEL_CACHE_DIR" //... + bazel --batch test --disk_cache="$BAZEL_CACHE_DIR" //src/test/... TEST_STATUS=$? if [ $TEST_STATUS != 0 ] then diff --git a/repositories.bzl b/repositories.bzl index 9da79a5faf..9b1a8cd45b 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -59,9 +59,9 @@ def gapic_generator_java_repositories(): _maybe( http_archive, name = "com_google_googleapis", - strip_prefix = "googleapis-dbdd8ddeb6d9556952aa8784d9e48f2566c9911a", + strip_prefix = "googleapis-bda7ce951def5ae6e5c4258d0e569188dd4ae02b", urls = [ - "https://github.com/googleapis/googleapis/archive/dbdd8ddeb6d9556952aa8784d9e48f2566c9911a.zip", + "https://github.com/googleapis/googleapis/archive/bda7ce951def5ae6e5c4258d0e569188dd4ae02b.zip", ], ) diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel new file mode 100644 index 0000000000..60a4ca6aa9 --- /dev/null +++ b/test/integration/BUILD.bazel @@ -0,0 +1,26 @@ +package(default_visibility = ["//visibility:public"]) + +#################################################### +# API Library Rules +#################################################### +# These will eventually go away once more APIs in googleapis have been migrated to the microgenerator. + +load( + "@com_google_googleapis_imports//:imports.bzl", + java_gapic_library = "java_gapic_library2", +) + +java_gapic_library( + name = "asset_java_gapic", + srcs = ["@com_google_googleapis//google/cloud/asset/v1:asset_proto_with_info"], + grpc_service_config = "@com_google_googleapis//google/cloud/asset/v1:cloudasset_grpc_service_config.json", + package = "google.cloud.asset.v1", + test_deps = [ + "@com_google_googleapis//google/cloud/asset/v1:asset_java_grpc", + "@com_google_googleapis//google/iam/v1:iam_java_grpc", + ], + deps = [ + "@com_google_googleapis//google/cloud/asset/v1:asset_java_proto", + "@com_google_googleapis//google/iam/v1:iam_java_proto", + ], +) From 462812b3ecf2c088efb0ab0d2e6e201ae7960376 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 14:16:47 -0700 Subject: [PATCH 3/7] fix: update CircleCI --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5a5afd7c9d..f64b58040c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -32,12 +32,12 @@ jobs: name: Build targets for gapic-generator-java command: | cd /tmp/gapic-generator-java - bazel --batch build //... + bazel --batch build //src/... - run: name: Run unit tests for gapic-generator-java command: | cd /tmp/gapic-generator-java - bazel --batch test //... --noshow_progress + bazel --batch test //src/test/... --noshow_progress find . -type f -regex ".*/bazel-testlogs/.*xml" -exec cp {} ${TEST_REPORTS_DIR} \; - store_test_results: path: bazel/reports/gapic-generator-java From 63f9156fe8121ab19c61fb769a7d7b483e1187cd Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 14:35:33 -0700 Subject: [PATCH 4/7] fix: sort message fields in asc. index order --- .../composer/ServiceStubSettingsClassComposer.java | 3 ++- .../api/generator/gapic/protoparser/Parser.java | 8 +++++--- .../generator/gapic/protoparser/ParserTest.java | 14 +++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index fe8456bd00..1bc36da476 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -355,13 +355,14 @@ private static List createPagingStaticAssignExprs( Preconditions.checkState( field != null, String.format("Null field found for message %s", pagedResponseMessage.name())); - if (field.isRepeated()) { + if (field.isRepeated() && !field.isMap()) { // Field is currently a List-type. Preconditions.checkState( !field.type().reference().generics().isEmpty(), String.format("No generics found for field reference %s", field.type().reference())); repeatedResponseType = TypeNode.withReference(field.type().reference().generics().get(0)); repeatedFieldName = field.name(); + break; } } Preconditions.checkNotNull( diff --git a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index ec7d6a83d5..43e7e7c435 100644 --- a/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -364,9 +364,11 @@ static String sanitizeDefaultHost(String rawDefaultHost) { } private static List parseFields(Descriptor messageDescriptor) { - return messageDescriptor.getFields().stream() - .map(f -> parseField(f, messageDescriptor)) - .collect(Collectors.toList()); + List fields = new ArrayList<>(messageDescriptor.getFields()); + // Sort by ascending field index order. This is important for paged responses, where the first + // repeated type is taken. + fields.sort((f1, f2) -> f1.getIndex() - f2.getIndex()); + return fields.stream().map(f -> parseField(f, messageDescriptor)).collect(Collectors.toList()); } private static Field parseField(FieldDescriptor fieldDescriptor, Descriptor messageDescriptor) { diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index e05c4203d6..b310cd661e 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -250,20 +250,20 @@ public void parseMethodSignatures_basic() { ImmutableList.of(), argument); - // Signature contents: ["name"], String variant. - methodArgs = methodSignatures.get(3); - assertEquals(1, methodArgs.size()); - argument = methodArgs.get(0); - assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument); - // Signature contents: ["name"], resource helper variant. - methodArgs = methodSignatures.get(4); + methodArgs = methodSignatures.get(3); assertEquals(1, methodArgs.size()); argument = methodArgs.get(0); TypeNode foobarNameType = TypeNode.withReference( VaporReference.builder().setName("FoobarName").setPakkage(ECHO_PACKAGE).build()); assertMethodArgumentEquals("name", foobarNameType, ImmutableList.of(), argument); + + // Signature contents: ["name"], String variant. + methodArgs = methodSignatures.get(4); + assertEquals(1, methodArgs.size()); + argument = methodArgs.get(0); + assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument); } @Test From 3746915d854f6b8d864eb5fe7224f2ee080c77b7 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 14:53:04 -0700 Subject: [PATCH 5/7] fix: update parser test method sig order --- .../api/generator/gapic/protoparser/ParserTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index e05c4203d6..e132056c1e 100644 --- a/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -250,12 +250,6 @@ public void parseMethodSignatures_basic() { ImmutableList.of(), argument); - // Signature contents: ["name"], String variant. - methodArgs = methodSignatures.get(3); - assertEquals(1, methodArgs.size()); - argument = methodArgs.get(0); - assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument); - // Signature contents: ["name"], resource helper variant. methodArgs = methodSignatures.get(4); assertEquals(1, methodArgs.size()); @@ -264,6 +258,12 @@ public void parseMethodSignatures_basic() { TypeNode.withReference( VaporReference.builder().setName("FoobarName").setPakkage(ECHO_PACKAGE).build()); assertMethodArgumentEquals("name", foobarNameType, ImmutableList.of(), argument); + + // Signature contents: ["name"], String variant. + methodArgs = methodSignatures.get(3); + assertEquals(1, methodArgs.size()); + argument = methodArgs.get(0); + assertMethodArgumentEquals("name", TypeNode.STRING, ImmutableList.of(), argument); } @Test From 59ef6689061b088d09980908a8ce8054367caa64 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 14:59:03 -0700 Subject: [PATCH 6/7] fix: update circleci config --- .circleci/config.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f64b58040c..730e03fe18 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -5,7 +5,7 @@ jobs: gapic-generator-java-tests: working_directory: /tmp/ environment: - TEST_REPORTS_DIR: /tmp/workspace/bazel/reports/gapic-generator-java + TEST_REPORTS_DIR: ~/.cache/bazel BAZEL_VERSION: 3.5.1 PYTHON_VERSION: 3.5.2 machine: true @@ -28,6 +28,7 @@ jobs: name: Make reports directory command: | mkdir -p ${TEST_REPORTS_DIR} + echo "Test reports will be stored in ${TEST_REPORTS_DIR}" - run: name: Build targets for gapic-generator-java command: | @@ -40,9 +41,9 @@ jobs: bazel --batch test //src/test/... --noshow_progress find . -type f -regex ".*/bazel-testlogs/.*xml" -exec cp {} ${TEST_REPORTS_DIR} \; - store_test_results: - path: bazel/reports/gapic-generator-java + path: ~/.cache/bazel - store_artifacts: - path: bazel/reports/gapic-generator-java + path: ~/.cache/bazel google-java-format: docker: - image: l.gcr.io/google/bazel From 51011eedcf6e5f7d31631df56bb321b84200cf46 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 9 Oct 2020 16:52:19 -0700 Subject: [PATCH 7/7] fix: linter --- test/integration/BUILD.bazel | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/BUILD.bazel b/test/integration/BUILD.bazel index 60a4ca6aa9..af5762b6e8 100644 --- a/test/integration/BUILD.bazel +++ b/test/integration/BUILD.bazel @@ -1,3 +1,8 @@ +load( + "@com_google_googleapis_imports//:imports.bzl", + java_gapic_library = "java_gapic_library2", +) + package(default_visibility = ["//visibility:public"]) #################################################### @@ -5,11 +10,6 @@ package(default_visibility = ["//visibility:public"]) #################################################### # These will eventually go away once more APIs in googleapis have been migrated to the microgenerator. -load( - "@com_google_googleapis_imports//:imports.bzl", - java_gapic_library = "java_gapic_library2", -) - java_gapic_library( name = "asset_java_gapic", srcs = ["@com_google_googleapis//google/cloud/asset/v1:asset_proto_with_info"],