From 524543b62184bc749e953f5986be55bda9f03605 Mon Sep 17 00:00:00 2001 From: Clement Escoffier Date: Tue, 26 Sep 2023 10:24:29 +0200 Subject: [PATCH 1/2] Fix https://github.com/quarkusio/quarkus/issues/36078 Fix missing include statements in the proto compilation and also improve the syntax of the proto include/exclude clauses (comma-separated list accepting spaces) --- .../main/asciidoc/grpc-getting-started.adoc | 4 +-- .../quarkus/grpc/deployment/GrpcCodeGen.java | 33 ++++++++++++------- .../src/main/proto/extended.proto | 25 ++++++++++++++ .../src/main/resources/application.properties | 4 +-- .../src/main/resources/protobuf/base.proto | 28 ++++++++++++++++ .../src/main/resources/protobuf/role.proto | 15 +++++++++ 6 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 integration-tests/grpc-external-proto-test/src/main/proto/extended.proto create mode 100644 integration-tests/grpc-external-proto/src/main/resources/protobuf/base.proto create mode 100644 integration-tests/grpc-external-proto/src/main/resources/protobuf/role.proto diff --git a/docs/src/main/asciidoc/grpc-getting-started.adoc b/docs/src/main/asciidoc/grpc-getting-started.adoc index dc59ef84e1f2d..dd13c07ec2e18 100644 --- a/docs/src/main/asciidoc/grpc-getting-started.adoc +++ b/docs/src/main/asciidoc/grpc-getting-started.adoc @@ -203,8 +203,8 @@ For instance, having the following properties in your `application.properties`: [source,properties] ---- -quarkus.generate-code.grpc.scan-for-proto-includes."\:"=foo/**,bar/**,banana/a-proto.proto -quarkus.generate-code.grpc.scan-for-proto-excludes."\:"=foo/private/**,bar/another-proto.proto +quarkus.generate-code.grpc.scan-for-proto-include."\:"=foo/**,bar/**,banana/a-proto.proto +quarkus.generate-code.grpc.scan-for-proto-exclude."\:"=foo/private/**,bar/another-proto.proto ---- will include: diff --git a/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java b/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java index 9e6ef342368dc..f69f5fcf01d5b 100644 --- a/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java +++ b/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java @@ -17,6 +17,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.microprofile.config.Config; @@ -100,11 +101,14 @@ public boolean trigger(CodeGenContext context) throws CodeGenException { Collection protoFilesFromDependencies = gatherProtosFromDependencies(dirWithProtosFromDependencies, protoDirs, context); if (!protoFilesFromDependencies.isEmpty()) { - protoFilesFromDependencies.stream() - .map(Path::normalize) - .map(Path::toAbsolutePath) - .map(Path::toString) - .forEach(protoFiles::add); + for (Path files : protoFilesFromDependencies) { + var pathToProtoFile = files.normalize().toAbsolutePath(); + var pathToParentDir = files.getParent(); + // Add the proto file to the list of proto to compile, but also add the directory containing the + // proto file to the list of directories to include (it's a set, so no duplicate). + protoFiles.add(pathToProtoFile.toString()); + protoDirs.add(pathToParentDir.toString()); + } } if (!protoFiles.isEmpty()) { @@ -115,12 +119,12 @@ public boolean trigger(CodeGenContext context) throws CodeGenException { List command = new ArrayList<>(); command.add(executables.protoc.toString()); - for (String protoImportDir : protosToImport) { - command.add(String.format("-I=%s", escapeWhitespace(protoImportDir))); - } for (String protoDir : protoDirs) { command.add(String.format("-I=%s", escapeWhitespace(protoDir))); } + for (String protoImportDir : protosToImport) { + command.add(String.format("-I=%s", escapeWhitespace(protoImportDir))); + } command.addAll(asList("--plugin=protoc-gen-grpc=" + executables.grpc, "--plugin=protoc-gen-q-grpc=" + executables.quarkusGrpc, @@ -203,17 +207,21 @@ private Collection gatherProtosFromDependencies(Path workDir, Set } boolean scanAll = "all".equalsIgnoreCase(scanDependencies); - List dependenciesToScan = asList(scanDependencies.split(",")); + List dependenciesToScan = Arrays.stream(scanDependencies.split(",")).map(String::trim) + .collect(Collectors.toList()); ApplicationModel appModel = context.applicationModel(); List protoFilesFromDependencies = new ArrayList<>(); for (ResolvedDependency artifact : appModel.getRuntimeDependencies()) { String packageId = String.format("%s:%s", artifact.getGroupId(), artifact.getArtifactId()); Collection includes = properties - .getOptionalValues(String.format(SCAN_DEPENDENCIES_FOR_PROTO_INCLUDE_PATTERN, packageId), String.class) + .getOptionalValue(String.format(SCAN_DEPENDENCIES_FOR_PROTO_INCLUDE_PATTERN, packageId), String.class) + .map(s -> Arrays.stream(s.split(",")).map(String::trim).collect(Collectors.toList())) .orElse(List.of()); + Collection excludes = properties - .getOptionalValues(String.format(SCAN_DEPENDENCIES_FOR_PROTO_EXCLUDE_PATTERN, packageId), String.class) + .getOptionalValue(String.format(SCAN_DEPENDENCIES_FOR_PROTO_EXCLUDE_PATTERN, packageId), String.class) + .map(s -> Arrays.stream(s.split(",")).map(String::trim).collect(Collectors.toList())) .orElse(List.of()); if (scanAll @@ -247,7 +255,8 @@ private Collection gatherDirectoriesWithImports(Path workDir, CodeGenCon } boolean scanAll = "all".equals(scanForImports.toLowerCase(Locale.getDefault())); - List dependenciesToScan = Arrays.asList(scanForImports.split(",")); + List dependenciesToScan = Arrays.stream(scanForImports.split(",")).map(String::trim) + .collect(Collectors.toList()); Set importDirectories = new HashSet<>(); ApplicationModel appModel = context.applicationModel(); diff --git a/integration-tests/grpc-external-proto-test/src/main/proto/extended.proto b/integration-tests/grpc-external-proto-test/src/main/proto/extended.proto new file mode 100644 index 0000000000000..90f5e6bc6a847 --- /dev/null +++ b/integration-tests/grpc-external-proto-test/src/main/proto/extended.proto @@ -0,0 +1,25 @@ +syntax = "proto3"; + +option java_package = "org.acme.protos.extended"; +option java_outer_classname = "ExtendedProtos"; + +option optimize_for = CODE_SIZE; + +package org.acme.proto.extended; + +// Import the base proto file +import "base.proto"; + +// A message representing detailed user information +message DetailedUser { + org.acme.protos.base.User user = 1; // Use the User message from base.proto + org.acme.protos.base.Address address = 2; // Use the Address message from base.proto + string phone_number = 3; +} + +// Another message using the base Address message +message Company { + string name = 1; + org.acme.protos.base.Address company_address = 2; +} + diff --git a/integration-tests/grpc-external-proto-test/src/main/resources/application.properties b/integration-tests/grpc-external-proto-test/src/main/resources/application.properties index fa03c312f9ed3..147e1cde8aaa2 100644 --- a/integration-tests/grpc-external-proto-test/src/main/resources/application.properties +++ b/integration-tests/grpc-external-proto-test/src/main/resources/application.properties @@ -1,6 +1,6 @@ quarkus.generate-code.grpc.scan-for-proto=io.quarkus:quarkus-integration-test-external-proto -quarkus.generate-code.grpc.scan-for-proto-include."io.quarkus\:quarkus-integration-test-external-proto"=dir/**,invalids/invalid2.proto -quarkus.generate-code.grpc.scan-for-proto-exclude."io.quarkus\:quarkus-integration-test-external-proto"=dir/invalid.proto,invalids/invalid2.proto +quarkus.generate-code.grpc.scan-for-proto-include."io.quarkus\:quarkus-integration-test-external-proto"=dir/**, invalids/invalid2.proto, protobuf/** +quarkus.generate-code.grpc.scan-for-proto-exclude."io.quarkus\:quarkus-integration-test-external-proto"=dir/invalid.proto, invalids/invalid2.proto %vertx.quarkus.grpc.clients.hello.host=localhost %vertx.quarkus.grpc.clients.hello.port=8081 diff --git a/integration-tests/grpc-external-proto/src/main/resources/protobuf/base.proto b/integration-tests/grpc-external-proto/src/main/resources/protobuf/base.proto new file mode 100644 index 0000000000000..1f3d0ffe36cd6 --- /dev/null +++ b/integration-tests/grpc-external-proto/src/main/resources/protobuf/base.proto @@ -0,0 +1,28 @@ +syntax = "proto3"; + +option java_package = "org.acme.protos.base"; +option java_outer_classname = "BASEProtos"; + +option optimize_for = CODE_SIZE; + +// Import the extra proto file +import "role.proto"; + +package org.acme.protos.base; + +// A basic message representing user information +message User { + int32 id = 1; + string name = 2; + string email = 3; + org.acme.protos.role.Role role = 4; // Use the Role message from extra.proto +} + +// A basic message representing an address +message Address { + string street = 1; + string city = 2; + string state = 3; + string zip = 4; +} + diff --git a/integration-tests/grpc-external-proto/src/main/resources/protobuf/role.proto b/integration-tests/grpc-external-proto/src/main/resources/protobuf/role.proto new file mode 100644 index 0000000000000..d38b7958cd093 --- /dev/null +++ b/integration-tests/grpc-external-proto/src/main/resources/protobuf/role.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; + +option java_package = "org.acme.protos.role"; +option java_outer_classname = "RoleProtos"; + +option optimize_for = CODE_SIZE; + +package org.acme.protos.role; + +// A basic message representing a role +message Role { + int32 role_id = 1; + string role_name = 2; +} + From e4c0502f6f7e1d880391c546c0a72e53d1f5654a Mon Sep 17 00:00:00 2001 From: Clement Escoffier Date: Tue, 26 Sep 2023 19:42:41 +0200 Subject: [PATCH 2/2] --amend --- .../java/io/quarkus/grpc/deployment/GrpcCodeGen.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java b/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java index f69f5fcf01d5b..e497a629f1cbb 100644 --- a/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java +++ b/extensions/grpc/codegen/src/main/java/io/quarkus/grpc/deployment/GrpcCodeGen.java @@ -286,8 +286,15 @@ private void extractProtosFromArtifact(Path workDir, Collection protoFiles protoDirectories.add(path.getParent().normalize().toAbsolutePath().toString()); } else { // archive Path relativePath = path.getRoot().relativize(path); + String uniqueName = artifact.getGroupId() + ":" + artifact.getArtifactId(); + if (artifact.getVersion() != null) { + uniqueName += ":" + artifact.getVersion(); + } + if (artifact.getClassifier() != null) { + uniqueName += "-" + artifact.getClassifier(); + } Path protoUnzipDir = workDir - .resolve(HashUtil.sha1(root.normalize().toAbsolutePath().toString())) + .resolve(HashUtil.sha1(uniqueName)) .normalize().toAbsolutePath(); try { Files.createDirectories(protoUnzipDir);