Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: protoPaths as import params to protoc (Maven) #1768

Merged
merged 8 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ jobs:
- name: Test Maven Java
run: |-
cd plugin-tester-java
mvn -Dakka.grpc.project.version=`cat ~/.version` akka-grpc:generate compile
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

akka-grpc:generate should not be needed explicitly as it is already run bin compile (otherwise it would fail I assume).

mvn -Dakka.grpc.project.version=`cat ~/.version` compile

- name: Test Maven Scala
run: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,25 @@ abstract class AbstractGenerateMojo @Inject() (buildContext: BuildContext) exten

def addGeneratedSourceRoot(generatedSourcesDir: String): Unit

//https://maven.apache.org/plugin-developers/common-bugs.html#Resolving_Relative_Paths
def normalize(protoPath: String): File = {
val protoFile = new File(protoPath)
if (!protoFile.isAbsolute()) {
new File(project.getBasedir(), protoPath).toPath().normalize().toFile()
} else {
protoFile
}
}

lazy val normalizedProtoPaths = protoPaths.asScala.map(normalize)

override def execute(): Unit = {
val chosenLanguage = parseLanguage(language)

var directoryFound = false
protoPaths.forEach { protoPath =>

normalizedProtoPaths.foreach { protoDir =>
// verify proto dir exists
//https://maven.apache.org/plugin-developers/common-bugs.html#Resolving_Relative_Paths
val protoDir = {
val protoFile = new File(protoPath)
if (!protoFile.isAbsolute()) {
new File(project.getBasedir(), protoPath).toPath().normalize().toFile()
} else {
protoFile
}
}
if (protoDir.exists()) {
directoryFound = true
// generated sources should be compiled
Expand Down Expand Up @@ -213,15 +217,16 @@ abstract class AbstractGenerateMojo @Inject() (buildContext: BuildContext) exten
private[this] def executeProtoc(
protocCommand: Seq[String] => Int,
schemas: Set[File],
protoDir: File,
protocOptions: Seq[String],
targets: Seq[Target]): Int =
try {
val incPath = "-I" + protoDir.getCanonicalPath
val protocIncludePaths = normalizedProtoPaths.map { includePath =>
"-I" + includePath.getCanonicalPath
}
protocbridge.ProtocBridge.execute(
ProtocRunner.fromFunction((args, _) => protocCommand(args)),
targets,
Seq(incPath) ++ protocOptions ++ schemas.map(_.getCanonicalPath),
protocIncludePaths ++ protocOptions ++ schemas.map(_.getCanonicalPath),
artifact =>
throw new RuntimeException(
s"The version of sbt-protoc you are using is incompatible with '${artifact}' code generator. Please update sbt-protoc to a version >= 0.99.33"))
Expand Down Expand Up @@ -256,7 +261,7 @@ abstract class AbstractGenerateMojo @Inject() (buildContext: BuildContext) exten

getLog.info("Compiling protobuf")
val (out, err, exitCode) = captureStdOutAnderr {
executeProtoc(protocCommand, schemas, protoDir, protocOptions, generatedTargets)
executeProtoc(protocCommand, schemas, protocOptions, generatedTargets)
}
if (exitCode != 0) {
err.split("\n\r").map(_.trim).map(parseError).foreach {
Expand Down
4 changes: 2 additions & 2 deletions plugin-tester-java/README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# sbt-plugin Tester for Scala
# sbt-plugin Tester for Java

Use this project to test the behavior of the sbt plugin without the need of separately releasing it from within the main build.

```
project akka-grpc-plugin-tester-java
test
```
```
78 changes: 72 additions & 6 deletions plugin-tester-java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
<akka.http.cors.version>1.1.0</akka.http.cors.version>
<grpc.version>1.48.1</grpc.version> <!-- checked synced by VersionSyncCheckPlugin -->
<project.encoding>UTF-8</project.encoding>
<build-helper-maven-plugin>3.3.0</build-helper-maven-plugin>
<protobuf-java.version>3.22.2</protobuf-java.version>
<proto-google-common-protos.version>2.15.0</proto-google-common-protos.version>
</properties>

<dependencies>
Expand All @@ -41,6 +44,56 @@
</dependencies>
<build>
<plugins>

<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<version>${maven-dependency-plugin.version}</version>
<executions>
<execution>
<id>getClasspathFilenames</id>
<goals>
<!-- provides the jars of the classpath as properties inside of maven
so that we can refer to one of the jars in the exec plugin config below -->
<goal>properties</goal>
</goals>
</execution>
<execution>
<id>unpack</id>
<phase>generate-sources</phase>
<goals>
<goal>unpack</goal>
</goals>
<configuration>
<artifactItems>
<!-- to use protobuf java types -->
<artifactItem>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
<version>${protobuf-java.version}</version>
<type>jar</type>
<overWrite>true</overWrite>
<outputDirectory>${project.build.directory}/proto</outputDirectory>
<includes>**/*.proto</includes>
</artifactItem>

<!-- to use protobuf common types -->
<artifactItem>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-common-protos</artifactId>
<version>${proto-google-common-protos.version}</version>
<type>jar</type>
<overWrite>true</overWrite>
<outputDirectory>${project.build.directory}/proto</outputDirectory>
<includes>**/*.proto</includes>
</artifactItem>
</artifactItems>
<overWriteReleases>false</overWriteReleases>
<overWriteSnapshots>true</overWriteSnapshots>
</configuration>
</execution>
</executions>
</plugin>
sebastian-alfers marked this conversation as resolved.
Show resolved Hide resolved

<plugin>
<groupId>com.lightbend.akka.grpc</groupId>
<artifactId>akka-grpc-maven-plugin</artifactId>
Expand All @@ -57,23 +110,35 @@
<serverPowerApis>true</serverPowerApis>
</generatorSettings>
<includeStdTypes>true</includeStdTypes>
<language>Java</language>
<protoPaths>
<protoPath>target/proto</protoPath>
<protoPath>src/main/proto</protoPath>
<protoPath>src/main/protobuf</protoPath>
</protoPaths>
</configuration>
</plugin>

<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<version>${maven-dependency-plugin.version}</version>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<version>${build-helper-maven-plugin}</version>
<executions>
<execution>
<id>getClasspathFilenames</id>
<id>add-source</id>
<phase>generate-sources</phase>
<goals>
<!-- provides the jars of the classpath as properties inside of maven
so that we can refer to one of the jars in the exec plugin config below -->
<goal>properties</goal>
<goal>add-source</goal>
</goals>
<configuration>
<sources>
<source>${project.build.directory}/generated-sources/akka-grpc-java/</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
Expand All @@ -87,6 +152,7 @@
</arguments>
</configuration>
</plugin>

</plugins>
</build>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,22 @@
package example.myapp.helloworld;

import akka.actor.ActorSystem;
import akka.http.javadsl.ConnectHttp;
import akka.grpc.javadsl.ServerReflection;
import akka.grpc.javadsl.ServiceHandler;

import akka.http.javadsl.Http;
import akka.http.javadsl.ServerBinding;
import akka.http.javadsl.model.HttpRequest;
import akka.http.javadsl.model.HttpResponse;
import akka.japi.function.Function;
import akka.stream.SystemMaterializer;
import akka.stream.Materializer;
import com.typesafe.config.Config;
import com.typesafe.config.ConfigFactory;
import example.myapp.helloworld.grpc.GreeterService;
import example.myapp.helloworld.grpc.GreeterServiceHandlerFactory;

import java.util.Arrays;
import java.util.concurrent.CompletionStage;

class GreeterServer {
Expand All @@ -40,10 +46,17 @@ public static CompletionStage<ServerBinding> run(ActorSystem sys) throws Excepti
// Instantiate implementation
GreeterService impl = new GreeterServiceImpl(mat);

// Create the reflection handler for multiple services
Function<HttpRequest, CompletionStage<HttpResponse>> reflectionPartial =
ServerReflection.create(Arrays.asList(GreeterService.description), sys);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, but helped me to test locally.


Function<HttpRequest, CompletionStage<HttpResponse>> serviceHandlers =
ServiceHandler.concatOrNotFound(GreeterServiceHandlerFactory.create(impl, sys), reflectionPartial);

return Http
.get(sys)
.newServerAt("127.0.0.1", 8090)
.bind(GreeterServiceHandlerFactory.create(impl, sys));
.bind(serviceHandlers);
}
}
//#full-server
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
import akka.stream.Materializer;
import akka.stream.javadsl.Sink;
import akka.stream.javadsl.Source;

import com.google.api.HttpBody;
import com.google.protobuf.BytesValue;
import com.google.protobuf.Timestamp;
import example.myapp.helloworld.grpc.*;

Expand All @@ -35,6 +36,16 @@ public CompletionStage<HelloReply> sayHello(HelloRequest in) {
return CompletableFuture.completedFuture(reply);
}

@Override
public CompletionStage<HttpBody> sayHelloHttp(HelloRequest in) {
System.out.println("sayHelloHttp to " + in.getName());
HttpBody reply = HttpBody.newBuilder().setData(
com.google.protobuf.ByteString.copyFrom("test".getBytes())
).build();

return CompletableFuture.completedFuture(reply);
}

@Override
public CompletionStage<HelloReply> itKeepsTalking(Source<HelloRequest, NotUsed> in) {
System.out.println("sayHello to in stream...");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import akka.stream.Materializer;
import akka.stream.javadsl.Sink;
import akka.stream.javadsl.Source;
import com.google.api.HttpBody;
import com.google.protobuf.ByteString;
import example.myapp.helloworld.grpc.GreeterServicePowerApi;
import example.myapp.helloworld.grpc.HelloReply;
import example.myapp.helloworld.grpc.HelloRequest;
Expand All @@ -34,6 +36,16 @@ public CompletionStage<HelloReply> sayHello(HelloRequest in, Metadata metadata)
return CompletableFuture.completedFuture(reply);
}

@Override
public CompletionStage<HttpBody> sayHelloHttp(HelloRequest in, Metadata metadata) {
System.out.println("sayHelloHttp to " + in.getName());
HttpBody reply = HttpBody.newBuilder().setData(
com.google.protobuf.ByteString.copyFrom("test".getBytes())
).build();

return CompletableFuture.completedFuture(reply);
}

@Override
public CompletionStage<HelloReply> itKeepsTalking(Source<HelloRequest, NotUsed> in, Metadata metadata) {
System.out.println("sayHello to in stream...");
Expand Down
5 changes: 5 additions & 0 deletions plugin-tester-java/src/main/protobuf/helloworld.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ option java_outer_classname = "HelloWorldProto";

package helloworld;

import "google/api/annotations.proto";
import "google/api/httpbody.proto";

////////////////////////////////////// The greeting service definition.
service GreeterService {
//////////////////////
Expand All @@ -17,6 +20,8 @@ service GreeterService {
////////*****/////////
rpc SayHello (HelloRequest) returns (HelloReply) {}

rpc SayHelloHttp (HelloRequest) returns (google.api.HttpBody) {}

// Comment spanning
// on several lines
rpc ItKeepsTalking (stream HelloRequest) returns (HelloReply) {}
Expand Down
8 changes: 7 additions & 1 deletion project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ object Dependencies {
// If changing this, remember to update protoc plugin version to align in
// maven-plugin/src/main/maven/plugin.xml and akka.grpc.sbt.AkkaGrpcPlugin
val googleProtobuf = "3.21.9" // checked synced by VersionSyncCheckPlugin
val googleApi = "2.15.0" // checked synced by VersionSyncCheckPlugin
sebastian-alfers marked this conversation as resolved.
Show resolved Hide resolved

val scalaTest = "3.2.12"

Expand Down Expand Up @@ -82,6 +83,9 @@ object Dependencies {
object Protobuf {
val protobufJava = "com.google.protobuf" % "protobuf-java" % Versions.googleProtobuf
val googleCommonProtos = "com.google.protobuf" % "protobuf-java" % Versions.googleProtobuf % "protobuf"

val googleApi = "com.google.api.grpc" % "proto-google-common-protos" % Versions.googleApi
val googleApiProtos = "com.google.api.grpc" % "proto-google-common-protos" % Versions.googleApi % "protobuf"
}

object Plugins {
Expand All @@ -93,6 +97,7 @@ object Dependencies {
val codegen = l ++= Seq(
Compile.scalapbCompilerPlugin,
Protobuf.protobufJava, // or else scalapb pulls older version in transitively
Protobuf.googleApi, // or else scalapb pulls older version in transitively
sebastian-alfers marked this conversation as resolved.
Show resolved Hide resolved
Compile.grpcProtobuf,
Test.scalaTest)

Expand Down Expand Up @@ -143,5 +148,6 @@ object Dependencies {
Compile.akkaHttpCors,
Test.scalaTest,
Test.scalaTestPlusJunit,
Protobuf.googleCommonProtos)
Protobuf.googleCommonProtos,
Protobuf.googleApiProtos)
}