ReflectiveValidatorIndex fails when option java_multiple_files=true (#186)

* Replicate java_multiple_files bug in ReflectiveValidatorIndex

Signed-off-by: Ryan Michela <rmichela@salesforce.com>

* Generate exploded java validator classes when java_multiple_files=true

Signed-off-by: Ryan Michela <rmichela@salesforce.com>

* Fix bazel build

Signed-off-by: Ryan Michela <rmichela@salesforce.com>

java/pgv-java-grpc/src/test/java/io/envoyproxy/pgv/grpc/ValidatingClientInterceptorTest.java

@@ -4,6 +4,9 @@ import io.envoyproxy.pgv.ReflectiveValidatorIndex;
 import io.envoyproxy.pgv.ValidationException;
 import io.envoyproxy.pgv.Validator;
 import io.envoyproxy.pgv.ValidatorIndex;
+import io.envoyproxy.pgv.grpc.asubpackage.GreeterGrpc;
+import io.envoyproxy.pgv.grpc.asubpackage.HelloRequest;
+import io.envoyproxy.pgv.grpc.asubpackage.HelloResponse;
 import io.grpc.BindableService;
 import io.grpc.StatusRuntimeException;
 import io.grpc.stub.StreamObserver;
@@ -19,8 +22,8 @@ public class ValidatingClientInterceptorTest {
 
     private BindableService svc = new GreeterGrpc.GreeterImplBase() {
         @Override
-        public void sayHello(Hello.HelloRequest request, StreamObserver<Hello.HelloResponse> responseObserver) {
-            responseObserver.onNext(Hello.HelloResponse.newBuilder().setMessage("Hello " + request.getName()).build());
+        public void sayHello(HelloRequest request, StreamObserver<HelloResponse> responseObserver) {
+            responseObserver.onNext(HelloResponse.newBuilder().setMessage("Hello " + request.getName()).build());
             responseObserver.onCompleted();
         }
     };
@@ -32,7 +35,7 @@ public class ValidatingClientInterceptorTest {
         ValidatingClientInterceptor interceptor = new ValidatingClientInterceptor(ValidatorIndex.ALWAYS_VALID);
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel()).withInterceptors(interceptor);
-        stub.sayHello(Hello.HelloRequest.newBuilder().setName("World").build());
+        stub.sayHello(HelloRequest.newBuilder().setName("World").build());
     }
 
     @Test
@@ -42,7 +45,7 @@ public class ValidatingClientInterceptorTest {
         ValidatingClientInterceptor interceptor = new ValidatingClientInterceptor(new ReflectiveValidatorIndex());
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel()).withInterceptors(interceptor);
-        stub.sayHello(Hello.HelloRequest.newBuilder().setName("World").build());
+        stub.sayHello(HelloRequest.newBuilder().setName("World").build());
     }
 
     @Test
@@ -59,7 +62,7 @@ public class ValidatingClientInterceptorTest {
         });
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel()).withInterceptors(interceptor);
-        assertThatThrownBy(() -> stub.sayHello(Hello.HelloRequest.newBuilder().setName("Foo").build()))
+        assertThatThrownBy(() -> stub.sayHello(HelloRequest.newBuilder().setName("Foo").build()))
                 .isInstanceOf(StatusRuntimeException.class)
                 .hasMessage("INVALID_ARGUMENT: one: is invalid - Got ");
     }
@@ -71,7 +74,7 @@ public class ValidatingClientInterceptorTest {
         ValidatingClientInterceptor interceptor = new ValidatingClientInterceptor(new ReflectiveValidatorIndex());
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel()).withInterceptors(interceptor);
-        assertThatThrownBy(() -> stub.sayHello(Hello.HelloRequest.newBuilder().setName("Foo").build()))
+        assertThatThrownBy(() -> stub.sayHello(HelloRequest.newBuilder().setName("Foo").build()))
                 .isInstanceOf(StatusRuntimeException.class)
                 .hasMessageStartingWith("INVALID_ARGUMENT: .io.envoyproxy.pgv.grpc.HelloRequest.name: must equal World");
     }

java/pgv-java-grpc/src/test/java/io/envoyproxy/pgv/grpc/ValidatingServerInterceptorTest.java

@@ -4,6 +4,9 @@ import io.envoyproxy.pgv.ReflectiveValidatorIndex;
 import io.envoyproxy.pgv.ValidationException;
 import io.envoyproxy.pgv.Validator;
 import io.envoyproxy.pgv.ValidatorIndex;
+import io.envoyproxy.pgv.grpc.asubpackage.GreeterGrpc;
+import io.envoyproxy.pgv.grpc.asubpackage.HelloRequest;
+import io.envoyproxy.pgv.grpc.asubpackage.HelloResponse;
 import io.grpc.BindableService;
 import io.grpc.ServerInterceptors;
 import io.grpc.StatusRuntimeException;
@@ -20,8 +23,8 @@ public class ValidatingServerInterceptorTest {
 
     private BindableService svc = new GreeterGrpc.GreeterImplBase() {
         @Override
-        public void sayHello(Hello.HelloRequest request, StreamObserver<Hello.HelloResponse> responseObserver) {
-            responseObserver.onNext(Hello.HelloResponse.newBuilder().setMessage("Hello " + request.getName()).build());
+        public void sayHello(HelloRequest request, StreamObserver<HelloResponse> responseObserver) {
+            responseObserver.onNext(HelloResponse.newBuilder().setMessage("Hello " + request.getName()).build());
             responseObserver.onCompleted();
         }
     };
@@ -33,7 +36,7 @@ public class ValidatingServerInterceptorTest {
         serverRule.getServiceRegistry().addService(ServerInterceptors.intercept(svc, interceptor));
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel());
-        stub.sayHello(Hello.HelloRequest.newBuilder().setName("World").build());
+        stub.sayHello(HelloRequest.newBuilder().setName("World").build());
     }
 
     @Test
@@ -43,7 +46,7 @@ public class ValidatingServerInterceptorTest {
         serverRule.getServiceRegistry().addService(ServerInterceptors.intercept(svc, interceptor));
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel());
-        stub.sayHello(Hello.HelloRequest.newBuilder().setName("World").build());
+        stub.sayHello(HelloRequest.newBuilder().setName("World").build());
     }
 
     @Test
@@ -60,7 +63,7 @@ public class ValidatingServerInterceptorTest {
         serverRule.getServiceRegistry().addService(ServerInterceptors.intercept(svc, interceptor));
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel());
-        assertThatThrownBy(() -> stub.sayHello(Hello.HelloRequest.newBuilder().setName("World").build()))
+        assertThatThrownBy(() -> stub.sayHello(HelloRequest.newBuilder().setName("World").build()))
             .isInstanceOf(StatusRuntimeException.class)
             .hasMessage("INVALID_ARGUMENT: one: is invalid - Got ");
     }
@@ -72,7 +75,7 @@ public class ValidatingServerInterceptorTest {
         serverRule.getServiceRegistry().addService(ServerInterceptors.intercept(svc, interceptor));
 
         GreeterGrpc.GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(serverRule.getChannel());
-        assertThatThrownBy(() -> stub.sayHello(Hello.HelloRequest.newBuilder().setName("Bananas").build()))
+        assertThatThrownBy(() -> stub.sayHello(HelloRequest.newBuilder().setName("Bananas").build()))
                 .isInstanceOf(StatusRuntimeException.class)
                 .hasMessageStartingWith("INVALID_ARGUMENT: .io.envoyproxy.pgv.grpc.HelloRequest.name: must equal World");
     }

java/pgv-java-grpc/src/test/proto/hello.proto

@@ -2,6 +2,10 @@ syntax = "proto3";
 
 package io.envoyproxy.pgv.grpc;
 
+option java_multiple_files = true;
+option java_package = "io.envoyproxy.pgv.grpc.asubpackage";
+option java_outer_classname = "Ponycopter";
+
 import "validate/validate.proto";
 
 service Greeter {

module/BUILD

@@ -10,6 +10,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//templates:go_default_library",
+        "//templates/java:go_default_library",
         "//validate:go_default_library",
         "//vendor/github.com/lyft/protoc-gen-star:go_default_library",
         "//vendor/github.com/lyft/protoc-gen-star/lang/go:go_default_library",

module/validate.go

 package module
 
 import (
-	"github.com/lyft/protoc-gen-star"
-	"github.com/lyft/protoc-gen-star/lang/go"
 	"github.com/envoyproxy/protoc-gen-validate/templates"
+	"github.com/envoyproxy/protoc-gen-validate/templates/java"
+	pgs "github.com/lyft/protoc-gen-star"
+	pgsgo "github.com/lyft/protoc-gen-star/lang/go"
 )
 
 const (
@@ -46,7 +47,15 @@ func (m *Module) Execute(targets map[string]pgs.File, pkgs map[string]pgs.Packag
 			// implementation-specific FilePathFor implementations.
 			// Ex: Don't generate Java validators for files that don't reference PGV.
 			if out != nil {
-				m.AddGeneratorTemplateFile(out.String(), tpl, f)
+				if opts := f.Descriptor().GetOptions(); opts != nil && opts.GetJavaMultipleFiles() {
+					// TODO: Only Java supports multiple file generation. If more languages add multiple file generation
+					// support, the implementation should be made more inderect.
+					for _, msg := range f.Messages() {
+						m.AddGeneratorTemplateFile(java.JavaMultiFilePath(f, msg).String(), tpl, msg)
+					}
+				} else {
+					m.AddGeneratorTemplateFile(out.String(), tpl, f)
+				}
 			}
 		}
 

templates/java/file.go

 package java
 
 const fileTpl = `// Code generated by protoc-gen-validate. DO NOT EDIT.
-// source: {{ .InputPath }}
-
-package {{ javaPackage . }};
+// source: {{ .File.InputPath }}
 
+package {{ javaPackage .File }};
+{{ if isOfFileType . }}
 public class {{ classNameFile . }}Validator {
 	public static io.envoyproxy.pgv.ValidatorImpl validatorFor(Class clazz) {
 		{{ range .AllMessages }}
@@ -17,4 +17,16 @@ public class {{ classNameFile . }}Validator {
 	{{- template "msg" . -}}
 {{- end }}
 }
+{{ else }}
+/**
+* Validates {@code {{ simpleName . }}} protobuf objects.
+*/
+public class {{ classNameMessage .}}Validator implements io.envoyproxy.pgv.ValidatorImpl<{{ qualifiedName . }}>{
+	public static io.envoyproxy.pgv.ValidatorImpl validatorFor(Class clazz) {
+		if (clazz.equals({{ qualifiedName . }}.class)) return new {{ simpleName .}}Validator();
+		return null;
+	}
+	{{- template "msgInner" . -}}
+}
+{{ end }}
 `

templates/java/msg.go

@@ -5,25 +5,29 @@ const msgTpl = `
 	 * Validates {@code {{ simpleName . }}} protobuf objects.
 	 */
 	public static class {{ simpleName . }}Validator implements io.envoyproxy.pgv.ValidatorImpl<{{ qualifiedName . }}> {
-		{{- range .NonOneOfFields }}
-			{{ renderConstants (context .) }}
-		{{ end }}
-		{{ range .OneOfs }}
-			{{ template "oneOfConst" . }}
-		{{ end }}
+		{{- template "msgInner" . -}}
+	}
+`
+
+const msgInnerTpl = `
+	{{- range .NonOneOfFields }}
+		{{ renderConstants (context .) }}
+	{{ end }}
+	{{ range .OneOfs }}
+		{{ template "oneOfConst" . }}
+	{{ end }}
 
-		public void assertValid({{ qualifiedName . }} proto, io.envoyproxy.pgv.ValidatorIndex index) throws io.envoyproxy.pgv.ValidationException {
-		{{ if disabled . }}
-			// Validate is disabled for {{ simpleName . }}
-			return;
-		{{- else -}}
-		{{ range .NonOneOfFields -}}
-			{{ render (context .) }}
-		{{ end -}}
-		{{ range .OneOfs }}
-			{{ template "oneOf" . }}
-		{{- end -}}
-		{{- end }}
-		}
+	public void assertValid({{ qualifiedName . }} proto, io.envoyproxy.pgv.ValidatorIndex index) throws io.envoyproxy.pgv.ValidationException {
+	{{ if disabled . }}
+		// Validate is disabled for {{ simpleName . }}
+		return;
+	{{- else -}}
+	{{ range .NonOneOfFields -}}
+		{{ render (context .) }}
+	{{ end -}}
+	{{ range .OneOfs }}
+		{{ template "oneOf" . }}
+	{{- end -}}
+	{{- end }}
 	}
 `

templates/java/register.go

@@ -3,6 +3,7 @@ package java
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"strings"
 	"text/template"
 	"unicode"
@@ -35,6 +36,7 @@ func Register(tpl *template.Template, params pgs.Parameters) {
 		"byteArrayLit":             fns.byteArrayLit,
 		"camelCase":                fns.camelCase,
 		"classNameFile":            classNameFile,
+		"classNameMessage":         classNameMessage,
 		"durLit":                   fns.durLit,
 		"fieldName":                fns.fieldName,
 		"javaPackage":              javaPackage,
@@ -47,6 +49,7 @@ func Register(tpl *template.Template, params pgs.Parameters) {
 		"simpleName":               fns.Name,
 		"tsLit":                    fns.tsLit,
 		"qualifiedName":            fns.qualifiedName,
+		"isOfFileType":             fns.isOfFileType,
 		"isOfMessageType":          fns.isOfMessageType,
 		"isOfStringType":           fns.isOfStringType,
 		"unwrap":                   fns.unwrap,
@@ -56,6 +59,7 @@ func Register(tpl *template.Template, params pgs.Parameters) {
 
 	template.Must(tpl.Parse(fileTpl))
 	template.Must(tpl.New("msg").Parse(msgTpl))
+	template.Must(tpl.New("msgInner").Parse(msgInnerTpl))
 
 	template.Must(tpl.New("none").Parse(noneTpl))
 
@@ -119,12 +123,19 @@ func JavaFilePath(f pgs.File, ctx pgsgo.Context, tpl *template.Template) *pgs.Fi
 		return nil
 	}
 
-	fullPath := strings.Replace(javaPackage(f), ".", "/", -1)
+	fullPath := strings.Replace(javaPackage(f), ".", string(os.PathSeparator), -1)
 	fileName := classNameFile(f) + "Validator.java"
 	filePath := pgs.JoinPaths(fullPath, fileName)
 	return &filePath
 }
 
+func JavaMultiFilePath(f pgs.File, m pgs.Message) pgs.FilePath {
+	fullPath := strings.Replace(javaPackage(f), ".", string(os.PathSeparator), -1)
+	fileName := classNameMessage(m) + "Validator.java"
+	filePath := pgs.JoinPaths(fullPath, fileName)
+	return filePath
+}
+
 func importsPvg(f pgs.File) bool {
 	for _, dep := range f.Descriptor().Dependency {
 		if strings.HasSuffix(dep, "validate.proto") {
@@ -134,20 +145,29 @@ func importsPvg(f pgs.File) bool {
 	return false
 }
 
-func classNameFile(file pgs.File) string {
+func classNameFile(f pgs.File) string {
 	// Explicit outer class name overrides implicit name
-	options := file.Descriptor().GetOptions()
+	options := f.Descriptor().GetOptions()
 	if options != nil && !options.GetJavaMultipleFiles() && options.JavaOuterClassname != nil {
 		return options.GetJavaOuterClassname()
 	}
 
-	protoName := pgs.FilePath(file.Name().String()).BaseName()
+	protoName := pgs.FilePath(f.Name().String()).BaseName()
+
+	className := sanitizeClassName(protoName)
+	className = appendOuterClassName(className, f)
+
+	return className
+}
 
-	className := makeInvalidClassnameCharactersUnderscores(protoName)
+func classNameMessage(m pgs.Message) string {
+	return sanitizeClassName(m.Name().String())
+}
+
+func sanitizeClassName(className string) string {
+	className = makeInvalidClassnameCharactersUnderscores(className)
 	className = strcase.ToCamel(strcase.ToSnake(className))
 	className = upperCaseAfterNumber(className)
-	className = appendOuterClassName(className, file)
-
 	return className
 }
 
@@ -420,6 +440,15 @@ func (fns javaFuncs) oneofTypeName(f pgs.Field) pgsgo.TypeName {
 	return pgsgo.TypeName(fmt.Sprintf("%s", strings.ToUpper(f.Name().String())))
 }
 
+func (fns javaFuncs) isOfFileType(o interface{}) bool {
+	switch o.(type) {
+	case pgs.File:
+		return true
+	default:
+		return false
+	}
+}
+
 func (fns javaFuncs) isOfMessageType(f pgs.Field) bool {
 	return f.Type().ProtoType() == pgs.MessageT
 }