From 09a92dba9cdd2afc2715c6cff3de9a0d96075879 Mon Sep 17 00:00:00 2001 From: Ryan Michela Date: Mon, 22 Oct 2018 15:38:40 -0700 Subject: [PATCH 1/2] Simplify Jdk8Stub.mustache --- .../jprotoc/JavaMethodStubTest.java | 5 ++++ jprotoc/src/main/resources/Jdk8Stub.mustache | 23 +------------------ 2 files changed, 6 insertions(+), 22 deletions(-) create mode 100644 jprotoc-test/src/test/java/com/salesforce/jprotoc/JavaMethodStubTest.java diff --git a/jprotoc-test/src/test/java/com/salesforce/jprotoc/JavaMethodStubTest.java b/jprotoc-test/src/test/java/com/salesforce/jprotoc/JavaMethodStubTest.java new file mode 100644 index 00000000..5f6dd277 --- /dev/null +++ b/jprotoc-test/src/test/java/com/salesforce/jprotoc/JavaMethodStubTest.java @@ -0,0 +1,5 @@ +package com.salesforce.jprotoc; + +public class JavaMethodStubTest { + +} diff --git a/jprotoc/src/main/resources/Jdk8Stub.mustache b/jprotoc/src/main/resources/Jdk8Stub.mustache index 9c51c7b5..ff46a75f 100644 --- a/jprotoc/src/main/resources/Jdk8Stub.mustache +++ b/jprotoc/src/main/resources/Jdk8Stub.mustache @@ -48,13 +48,12 @@ public class {{className}} { private {{serviceName}}CompletableFutureStub(Channel channel, CallOptions callOptions, Function futureAdapter) { super(channel, callOptions); this.futureAdapter = futureAdapter; - innerStub = {{serviceName}}Grpc.newFutureStub(channel); + innerStub = {{serviceName}}Grpc.newFutureStub(channel).build(channel, callOptions); } @Override protected {{serviceName}}CompletableFutureStub build(Channel channel, CallOptions callOptions) { {{serviceName}}CompletableFutureStub stub = new {{serviceName}}CompletableFutureStub(channel, callOptions, futureAdapter); - forceCallOptions(stub.innerStub, callOptions); return stub; } {{#methods}}{{#deprecated}}@Deprecated{{/deprecated}} @@ -62,25 +61,5 @@ public class {{className}} { return futureAdapter.apply(innerStub.{{methodName}}(request)); } {{/methods}} - - private void forceCallOptions({{serviceName}}Grpc.{{serviceName}}FutureStub stub, CallOptions options) { - try { - callOptionsField.set(stub, options); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - } - - private static final Field callOptionsField = getWritableCallOptonsField(); - - private static Field getWritableCallOptonsField() { - try { - Field callOptionsField = AbstractStub.class.getDeclaredField("callOptions"); - callOptionsField.setAccessible(true); - return callOptionsField; - } catch (NoSuchFieldException e) { - throw new RuntimeException(e); - } } } From 2154bbce0b48305677390ae4923e50fd44832993 Mon Sep 17 00:00:00 2001 From: Ryan Michela Date: Mon, 22 Oct 2018 16:41:18 -0700 Subject: [PATCH 2/2] Generate Javadoc in JDK8 generator --- jprotoc-test/src/test/proto/helloworld.proto | 2 +- .../jprotoc/jdk8/Jdk8Generator.java | 107 ++++++++++++------ jprotoc/src/main/resources/Jdk8Stub.mustache | 13 ++- pom.xml | 1 + 4 files changed, 89 insertions(+), 34 deletions(-) diff --git a/jprotoc-test/src/test/proto/helloworld.proto b/jprotoc-test/src/test/proto/helloworld.proto index 9afc077b..f2fb144f 100644 --- a/jprotoc-test/src/test/proto/helloworld.proto +++ b/jprotoc-test/src/test/proto/helloworld.proto @@ -16,7 +16,7 @@ service Greeter { rpc SayHello (HelloRequest) returns (HelloResponse) {} // Sends the current time - rpc SayTime (google.protobuf.Empty) returns (TimeResponse) {} + rpc SayTime (google.protobuf.Empty) returns (TimeResponse) {option deprecated=true;} rpc SomeParam (my.someparameters.SomeParameter) returns (my.someparameters.SomeParameter) {} } diff --git a/jprotoc/src/main/java/com/salesforce/jprotoc/jdk8/Jdk8Generator.java b/jprotoc/src/main/java/com/salesforce/jprotoc/jdk8/Jdk8Generator.java index 9b4a6328..f4fce0ea 100644 --- a/jprotoc/src/main/java/com/salesforce/jprotoc/jdk8/Jdk8Generator.java +++ b/jprotoc/src/main/java/com/salesforce/jprotoc/jdk8/Jdk8Generator.java @@ -8,6 +8,7 @@ package com.salesforce.jprotoc.jdk8; import com.google.common.base.Strings; +import com.google.common.html.HtmlEscapers; import com.google.protobuf.DescriptorProtos; import com.google.protobuf.compiler.PluginProtos; import com.salesforce.jprotoc.Generator; @@ -16,6 +17,7 @@ import com.salesforce.jprotoc.ProtocPlugin; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -28,6 +30,9 @@ public static void main(String[] args) { private static final String CLASS_SUFFIX = "Grpc8"; + private static final String SERVICE_JAVADOC_PREFIX = " "; + private static final String METHOD_JAVADOC_PREFIX = " "; + @Override public List generateFiles(PluginProtos.CodeGeneratorRequest request) throws GeneratorException { final ProtoTypeMap protoTypeMap = ProtoTypeMap.of(request.getProtoFileList()); @@ -35,7 +40,7 @@ public List generateFiles(PluginProtos. for (DescriptorProtos.FileDescriptorProto protoFile : request.getProtoFileList()) { if (request.getFileToGenerateList().contains(protoFile.getName())) { - for (Context ctx : extractContext(protoTypeMap, protoFile)) { + for (ServiceContext ctx : extractContext(protoTypeMap, protoFile)) { files.add(buildFile(ctx)); } } @@ -44,17 +49,23 @@ public List generateFiles(PluginProtos. return files; } - private List extractContext(ProtoTypeMap protoTypeMap, DescriptorProtos.FileDescriptorProto proto) { - List contexts = new ArrayList<>(); - - for (DescriptorProtos.ServiceDescriptorProto service : proto.getServiceList()) { - Context ctx = extractServiceContext(protoTypeMap, service); - ctx.packageName = extractPackageName(proto); - ctx.protoName = proto.getName(); - contexts.add(ctx); - } + private List extractContext(ProtoTypeMap protoTypeMap, DescriptorProtos.FileDescriptorProto fileProto) { + List serviceContexts = new ArrayList<>(); + + List locations = fileProto.getSourceCodeInfo().getLocationList(); + locations.stream() + .filter(location -> location.getPathCount() == 2 && location.getPath(0) == DescriptorProtos.FileDescriptorProto.SERVICE_FIELD_NUMBER) + .forEach(location -> { + int serviceNumber = location.getPath(1); + DescriptorProtos.ServiceDescriptorProto serviceProto = fileProto.getService(serviceNumber); + ServiceContext ctx = extractServiceContext(protoTypeMap, serviceProto, locations, serviceNumber); + ctx.packageName = extractPackageName(fileProto); + ctx.protoName = fileProto.getName(); + ctx.javaDoc = getJavaDoc(getComments(location), SERVICE_JAVADOC_PREFIX); + serviceContexts.add(ctx); + }); - return contexts; + return serviceContexts; } private String extractPackageName(DescriptorProtos.FileDescriptorProto proto) { @@ -69,26 +80,38 @@ private String extractPackageName(DescriptorProtos.FileDescriptorProto proto) { return Strings.nullToEmpty(proto.getPackage()); } - private Context extractServiceContext( + private static final int METHOD_NUMBER_OF_PATHS = 4; + private ServiceContext extractServiceContext( ProtoTypeMap protoTypeMap, - DescriptorProtos.ServiceDescriptorProto serviceProto) { - Context ctx = new Context(); + DescriptorProtos.ServiceDescriptorProto serviceProto, + List locations, + int serviceNumber) { + ServiceContext ctx = new ServiceContext(); ctx.fileName = serviceProto.getName() + CLASS_SUFFIX + ".java"; ctx.className = serviceProto.getName() + CLASS_SUFFIX; ctx.serviceName = serviceProto.getName(); ctx.deprecated = serviceProto.getOptions() != null && serviceProto.getOptions().getDeprecated(); - // Identify methods to generate a CompletableFuture-based client for. - // Only unary methods are supported. - serviceProto.getMethodList().stream() - .filter(method -> !method.getClientStreaming() && !method.getServerStreaming()) - .forEach(method -> { - ContextMethod ctxMethod = new ContextMethod(); - ctxMethod.methodName = lowerCaseFirst(method.getName()); - ctxMethod.inputType = protoTypeMap.toJavaTypeName(method.getInputType()); - ctxMethod.outputType = protoTypeMap.toJavaTypeName(method.getOutputType()); - ctxMethod.deprecated = method.getOptions() != null && method.getOptions().getDeprecated(); - ctx.methods.add(ctxMethod); + locations.stream() + .filter(location -> location.getPathCount() == METHOD_NUMBER_OF_PATHS && + location.getPath(0) == DescriptorProtos.FileDescriptorProto.SERVICE_FIELD_NUMBER && + location.getPath(1) == serviceNumber && + location.getPath(2) == DescriptorProtos.ServiceDescriptorProto.METHOD_FIELD_NUMBER) + .forEach(location -> { + int methodNumber = location.getPath(METHOD_NUMBER_OF_PATHS - 1); + DescriptorProtos.MethodDescriptorProto methodProto = serviceProto.getMethod(methodNumber); + + // Identify methods to generate a CompletableFuture-based client for. + // Only unary methods are supported. + if (!methodProto.getClientStreaming() && !methodProto.getServerStreaming()) { + MethodContext ctxMethod = new MethodContext(); + ctxMethod.methodName = lowerCaseFirst(methodProto.getName()); + ctxMethod.inputType = protoTypeMap.toJavaTypeName(methodProto.getInputType()); + ctxMethod.outputType = protoTypeMap.toJavaTypeName(methodProto.getOutputType()); + ctxMethod.deprecated = methodProto.getOptions() != null && methodProto.getOptions().getDeprecated(); + ctxMethod.javaDoc = getJavaDoc(getComments(location), METHOD_JAVADOC_PREFIX); + ctx.methods.add(ctxMethod); + } }); return ctx; } @@ -97,7 +120,7 @@ private String lowerCaseFirst(String s) { return Character.toLowerCase(s.charAt(0)) + s.substring(1); } - private String absoluteFileName(Context ctx) { + private String absoluteFileName(ServiceContext ctx) { String dir = ctx.packageName.replace('.', '/'); if (Strings.isNullOrEmpty(dir)) { return ctx.fileName; @@ -106,7 +129,7 @@ private String absoluteFileName(Context ctx) { } } - private PluginProtos.CodeGeneratorResponse.File buildFile(Context context) { + private PluginProtos.CodeGeneratorResponse.File buildFile(ServiceContext context) { String content = applyTemplate("Jdk8Stub.mustache", context); return PluginProtos.CodeGeneratorResponse.File .newBuilder() @@ -115,28 +138,48 @@ private PluginProtos.CodeGeneratorResponse.File buildFile(Context context) { .build(); } + private String getComments(DescriptorProtos.SourceCodeInfo.Location location) { + return location.getLeadingComments().isEmpty() ? location.getTrailingComments() : location.getLeadingComments(); + } + + private String getJavaDoc(String comments, String prefix) { + if (!comments.isEmpty()) { + StringBuilder builder = new StringBuilder("/**\n") + .append(prefix).append(" * 
\n");
+            Arrays.stream(HtmlEscapers.htmlEscaper().escape(comments).split("\n"))
+                    .forEach(line -> builder.append(prefix).append(" * ").append(line).append("\n"));
+            builder
+                    .append(prefix).append(" * 
\n")
+                    .append(prefix).append(" */");
+            return builder.toString();
+        }
+        return null;
+    }
+
     /**
      * Backing class for mustache template.
      */
-    private class Context {
-        // CHECKSTYLE DISABLE VisibilityModifier FOR 7 LINES
+    private class ServiceContext {
+        // CHECKSTYLE DISABLE VisibilityModifier FOR 8 LINES
         public String fileName;
         public String protoName;
         public String packageName;
         public String className;
         public String serviceName;
         public boolean deprecated;
-        public final List methods = new ArrayList<>();
+        public String javaDoc;
+        public final List methods = new ArrayList<>();
     }
 
     /**
      * Backing class for mustache template.
      */
-    private class ContextMethod {
-        // CHECKSTYLE DISABLE VisibilityModifier FOR 4 LINES
+    private class MethodContext {
+        // CHECKSTYLE DISABLE VisibilityModifier FOR 5 LINES
         public String methodName;
         public String inputType;
         public String outputType;
         public boolean deprecated;
+        public String javaDoc;
     }
 }
diff --git a/jprotoc/src/main/resources/Jdk8Stub.mustache b/jprotoc/src/main/resources/Jdk8Stub.mustache
index ff46a75f..739cb80d 100644
--- a/jprotoc/src/main/resources/Jdk8Stub.mustache
+++ b/jprotoc/src/main/resources/Jdk8Stub.mustache
@@ -24,6 +24,7 @@ public class {{className}} {
     /**
     * Creates a new CompletableFuture-style stub that supports unary calls on the service
     */
+    @SuppressWarnings("unchecked")
     public static {{serviceName}}CompletableFutureStub newCompletableFutureStub(Channel channel) {
         return newCompletableFutureStub(channel, MoreFutures::toCompletableFuture);
     }
@@ -31,10 +32,12 @@ public class {{className}} {
     /**
     * Creates a new CompletableFuture-style stub that supports unary calls on the service
     */
+    @SuppressWarnings("unchecked")
     public static {{serviceName}}CompletableFutureStub newCompletableFutureStub(Channel channel, Function futureAdapter) {
         return new {{serviceName}}CompletableFutureStub(channel, futureAdapter);
     }
 
+    {{#javaDoc}}{{{javaDoc}}}{{/javaDoc}}
     public static final class {{serviceName}}CompletableFutureStub extends AbstractStub<{{serviceName}}CompletableFutureStub> {
         private {{serviceName}}Grpc.{{serviceName}}FutureStub innerStub;
         private Function futureAdapter;
@@ -56,7 +59,15 @@ public class {{className}} {
             {{serviceName}}CompletableFutureStub stub = new {{serviceName}}CompletableFutureStub(channel, callOptions, futureAdapter);
             return stub;
         }
-        {{#methods}}{{#deprecated}}@Deprecated{{/deprecated}}
+        {{#methods}}
+
+            {{#javaDoc}}
+        {{{javaDoc}}}
+            {{/javaDoc}}
+            {{#deprecated}}
+        @java.lang.Deprecated
+            {{/deprecated}}
+        @SuppressWarnings("unchecked")
         public CompletableFuture<{{outputType}}> {{methodName}}({{inputType}} request) {
             return futureAdapter.apply(innerStub.{{methodName}}(request));
         }
diff --git a/pom.xml b/pom.xml
index 7b6edd2b..9372b738 100644
--- a/pom.xml
+++ b/pom.xml
@@ -226,6 +226,7 @@
                 
                     ${maven.compiler.source}
                     ${maven.compiler.target}
+                    -Xlint:unchecked