Merge branch 'master' into python-new-interface

.github/stale.yml

@@ -0,0 +1,21 @@
+# Number of days of inactivity before an issue becomes stale
+daysUntilStale: 30
+# Number of days of inactivity before a stale issue is closed
+daysUntilClose: 7
+# Issues with these labels will never be considered stale
+exemptLabels:
+  - Bug
+  - Bazel
+  - Documentation
+  - Enhancement
+  - Help Wanted
+  - Language Support
+# Label to use when marking an issue as stale
+staleLabel: Stale
+# Comment to post when marking an issue as stale. Set to `false` to disable
+markComment: >
+  This issue has been automatically marked as stale because it has not had
+  recent activity. It will be closed if no further activity occurs. Thank you
+  for your contributions.
+# Comment to post when closing a stale issue. Set to `false` to disable
+closeComment: false

Makefile

@@ -35,6 +35,10 @@ bazel:
 	# generate the PGV plugin with Bazel
 	bazel build //tests/... --incompatible_new_actions_api=false
 
+.PHONY: build_generation_tests
+build_generation_tests:
+	bazel build //tests/generation/...
+
 .PHONY: gazelle
 gazelle: vendor
 	# runs gazelle against the codebase to generate Bazel BUILD files
@@ -134,7 +138,7 @@ tests/harness/java/java-harness:
 	mvn -q -f java/pom.xml clean package -DskipTests
 
 .PHONY: ci
-ci: lint build testcases bazel-harness
+ci: lint bazel testcases bazel-harness build_generation_tests
 
 .PHONY: clean
 clean:

java/RELEASING.md

@@ -11,5 +11,5 @@ curl -X POST -H "Content-Type: application/json" -d '{
     "NEXT": "<next-version>-SNAPSHOT",
     "GIT_USER_EMAIL": "envoy-bot@users.noreply.github.com",
     "GIT_USER_NAME": "Via CircleCI"
-}}' "https://circleci.com/api/v1.1/project/github/envoyproxy/java-control-plane/tree/master?circle-token=<my-token>"
+}}' "https://circleci.com/api/v1.1/project/github/envoyproxy/protoc-gen-validate/tree/master?circle-token=<my-token>"
 ```

java/pgv-artifacts/pom.xml

@@ -3,7 +3,7 @@
     <parent>
         <artifactId>pgv-java</artifactId>
         <groupId>io.envoyproxy.protoc-gen-validate</groupId>
-        <version>0.2.0-SNAPSHOT</version>
+        <version>0.3.0-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

java/pgv-java-grpc/pom.xml

@@ -3,7 +3,7 @@
     <parent>
         <artifactId>pgv-java</artifactId>
         <groupId>io.envoyproxy.protoc-gen-validate</groupId>
-        <version>0.2.0-SNAPSHOT</version>
+        <version>0.3.0-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

java/pgv-java-stub/pom.xml

@@ -3,7 +3,7 @@
     <parent>
         <artifactId>pgv-java</artifactId>
         <groupId>io.envoyproxy.protoc-gen-validate</groupId>
-        <version>0.2.0-SNAPSHOT</version>
+        <version>0.3.0-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

java/pgv-java-stub/src/main/java/io/envoyproxy/pgv/ExplicitValidatorIndex.java

@@ -4,11 +4,20 @@
 
 /**
  * {@code ExplicitValidatorIndex} is an explicit registry of {@link Validator} instances. If no validator is registered
- * for {@code type}, a default {@code ALWAYS_VALID} validator will be used.
+ * for {@code type}, a fallback validator will be used (default ALWAYS_VALID).
  */
-public class ExplicitValidatorIndex implements ValidatorIndex {
+public final class ExplicitValidatorIndex implements ValidatorIndex {
     private final ConcurrentHashMap<Class, ValidatorImpl> VALIDATOR_IMPL_INDEX = new ConcurrentHashMap<>();
     private final ConcurrentHashMap<Class, Validator> VALIDATOR_INDEX = new ConcurrentHashMap<>();
+    private final ValidatorIndex fallbackIndex;
+
+    public ExplicitValidatorIndex() {
+        this(ValidatorIndex.ALWAYS_VALID);
+    }
+
+    public ExplicitValidatorIndex(ValidatorIndex fallbackIndex) {
+        this.fallbackIndex = fallbackIndex;
+    }
 
     /**
      * Adds a {@link Validator} to the set of known validators.
@@ -24,7 +33,7 @@ public <T> ExplicitValidatorIndex add(Class<T> forType, ValidatorImpl<T> validat
     @SuppressWarnings("unchecked")
     public <T> Validator<T> validatorFor(Class clazz) {
         return VALIDATOR_INDEX.computeIfAbsent(clazz, c ->
-                proto -> VALIDATOR_IMPL_INDEX.getOrDefault(c, ValidatorImpl.ALWAYS_VALID)
+                proto -> VALIDATOR_IMPL_INDEX.getOrDefault(c, (p, i) -> fallbackIndex.validatorFor(c))
                         .assertValid(proto, ExplicitValidatorIndex.this));
     }
 }

java/pgv-java-stub/src/main/java/io/envoyproxy/pgv/ReflectiveValidatorIndex.java

@@ -1,14 +1,27 @@
 package io.envoyproxy.pgv;
 
+import java.sql.Ref;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * {@code ReflectiveValidatorIndex} uses reflection to discover {@link Validator} implementations lazily the first
- * time a type is validated. If no validator can be found for {@code type}, a default {@code ALWAYS_VALID} validator
- * will be used.
+ * time a type is validated. If no validator can be found for {@code type}, a fallback validator
+ * will be used (default ALWAYS_VALID).
  */
 public final class ReflectiveValidatorIndex implements ValidatorIndex {
     private final ConcurrentHashMap<Class, Validator> VALIDATOR_INDEX = new ConcurrentHashMap<>();
+    private final ValidatorIndex fallbackIndex;
+
+    public ReflectiveValidatorIndex() {
+        this(ValidatorIndex.ALWAYS_VALID);
+    }
+
+    /**
+     * @param fallbackIndex a {@link ValidatorIndex} implementation to use if reflective validator discovery fails.
+     */
+    public ReflectiveValidatorIndex(ValidatorIndex fallbackIndex) {
+        this.fallbackIndex = fallbackIndex;
+    }
 
     /**
      * Returns the validator for {@code <T>}, or {@code ALWAYS_VALID} if not found.
@@ -20,7 +33,7 @@ public <T> Validator<T> validatorFor(Class clazz) {
             try {
                 return reflectiveValidatorFor(c);
             } catch (ReflectiveOperationException ex) {
-                return Validator.ALWAYS_VALID;
+                return fallbackIndex.validatorFor(clazz);
             }
         });
     }

java/pgv-java-stub/src/main/java/io/envoyproxy/pgv/Validator.java

@@ -32,4 +32,8 @@ default boolean isValid(T proto) {
     Validator ALWAYS_VALID = (proto) -> {
         // Do nothing. Always valid.
     };
+
+    Validator ALWAYS_INVALID = (proto) -> {
+        throw new ValidationException("UNKNOWN", new Object(), "Explicitly invalid");
+    };
 }

java/pgv-java-stub/src/main/java/io/envoyproxy/pgv/ValidatorIndex.java

@@ -25,4 +25,12 @@ public <T> Validator<T> validatorFor(Class clazz) {
             return Validator.ALWAYS_VALID;
         }
     };
+
+    ValidatorIndex ALWAYS_INVALID = new ValidatorIndex() {
+        @Override
+        @SuppressWarnings("unchecked")
+        public <T> Validator<T> validatorFor(Class clazz) {
+            return Validator.ALWAYS_INVALID;
+        }
+    };
 }

java/pgv-java-stub/src/test/java/io/envoyproxy/pgv/ExplicitValidatorIndexTest.java

@@ -0,0 +1,47 @@
+package io.envoyproxy.pgv;
+
+import org.junit.Test;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@SuppressWarnings("unchecked")
+public class ExplicitValidatorIndexTest {
+    class Thing {}
+
+    @Test
+    public void indexFindsValidator() throws ValidationException {
+        AtomicBoolean called = new AtomicBoolean();
+        ValidatorImpl validatorImpl = (ValidatorImpl<Thing>) (proto, index) -> called.set(true);
+
+        ExplicitValidatorIndex index = new ExplicitValidatorIndex();
+        index.add(Thing.class, validatorImpl);
+
+        Thing thing = new Thing();
+        Validator<Thing> validator = index.validatorFor(thing);
+        assertThat(validator).isNotEqualTo(ValidatorImpl.ALWAYS_VALID);
+        validator.assertValid(thing);
+        assertThat(called).isTrue();
+    }
+
+    @Test
+    public void indexFallsBack() throws ValidationException {
+        AtomicBoolean called = new AtomicBoolean();
+        ValidatorIndex fallback = new ValidatorIndex() {
+            @Override
+            @SuppressWarnings("unchecked")
+            public <T> Validator<T> validatorFor(Class clazz) {
+                called.set(true);
+                return Validator.ALWAYS_VALID;
+            }
+        };
+
+        ExplicitValidatorIndex index = new ExplicitValidatorIndex(fallback);
+        Thing thing = new Thing();
+        Validator<Thing> validator = index.validatorFor(thing);
+        assertThat(validator).isNotEqualTo(ValidatorImpl.ALWAYS_VALID);
+        validator.assertValid(thing);
+        assertThat(called).isTrue();
+    }
+}

java/pgv-java-stub/src/test/java/io/envoyproxy/pgv/ReflectiveValidatorIndexTest.java

@@ -1,9 +1,13 @@
 package io.envoyproxy.pgv;
 
 import io.envoyproxy.pvg.cases.TokenUse;
+import org.assertj.core.api.AtomicBooleanAssert;
 import org.junit.Test;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.in;
 
 public class ReflectiveValidatorIndexTest {
     @Test
@@ -35,4 +39,22 @@ public void indexFindsDoubleEmbeddedMessage() throws ValidationException {
         assertThat(validator).withFailMessage("Unexpected Validator.ALWAYS_VALID").isNotEqualTo(Validator.ALWAYS_VALID);
         validator.assertValid(token);
     }
+
+    @Test
+    public void indexFallsBack() throws ValidationException {
+        AtomicBoolean called = new AtomicBoolean();
+        ValidatorIndex fallback = new ValidatorIndex() {
+            @Override
+            @SuppressWarnings("unchecked")
+            public <T> Validator<T> validatorFor(Class clazz) {
+                called.set(true);
+                return Validator.ALWAYS_VALID;
+            }
+        };
+
+        ReflectiveValidatorIndex index = new ReflectiveValidatorIndex(fallback);
+        index.validatorFor(fallback);
+
+        assertThat(called).isTrue();
+    }
 }

java/pgv-java-stub/src/test/proto/underscoreName.proto

@@ -0,0 +1,18 @@
+syntax = "proto3";
+package io.envoyproxy.pgv.grpc;
+
+option java_multiple_files = true;
+option java_package = "io.envoyproxy.pgv.validate";
+import "validate/validate.proto";
+
+message MessageNameWith_Underscore {
+    string v = 1 [(validate.rules).string.min_len = 2];
+}
+
+message AnotherMessageNameWith_Underscore {
+    string v = 1 [(validate.rules).string.min_len = 2];
+}
+
+message MessageNameWith_Two_Underscore {
+    string v = 1 [(validate.rules).string.min_len = 2];
+}

java/pgv-java-validation/pom.xml

@@ -3,7 +3,7 @@
     <parent>
         <artifactId>pgv-java</artifactId>
         <groupId>io.envoyproxy.protoc-gen-validate</groupId>
-        <version>0.2.0-SNAPSHOT</version>
+        <version>0.3.0-SNAPSHOT</version>
     </parent>
     <modelVersion>4.0.0</modelVersion>
 

java/pom.xml

@@ -4,7 +4,7 @@
 
     <groupId>io.envoyproxy.protoc-gen-validate</groupId>
     <artifactId>pgv-java</artifactId>
-    <version>0.2.0-SNAPSHOT</version>
+    <version>0.3.0-SNAPSHOT</version>
     <modules>
         <module>pgv-java-stub</module>
         <module>pgv-java-validation</module>

module/validate.go

@@ -47,7 +47,7 @@ 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 {
-				if opts := f.Descriptor().GetOptions(); opts != nil && opts.GetJavaMultipleFiles() {
+				if opts := f.Descriptor().GetOptions(); opts != nil && opts.GetJavaMultipleFiles() && lang == "java" {
 					// 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() {

templates/go/file.go

@@ -44,7 +44,7 @@ var (
 )
 
 // define the regex for a UUID once up-front
-var _{{ .File.InputPath.BaseName }}_uuidPattern = regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")
+var _{{ snakeCase .File.InputPath.BaseName }}_uuidPattern = regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")
 
 {{ range .AllMessages }}
 	{{ template "msg" . }}

templates/gogo/file.go

@@ -44,7 +44,7 @@ var (
 )
 
 // define the regex for a UUID once up-front
-var _{{ .File.InputPath.BaseName }}_uuidPattern = regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")
+var _{{ snakeCase .File.InputPath.BaseName }}_uuidPattern = regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")
 
 {{ range .AllMessages }}
 	{{ template "msg" . }}

templates/goshared/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//templates/shared: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",
+        "//vendor/github.com/iancoleman/strcase:go_default_library",
         "@com_github_golang_protobuf//ptypes:go_default_library",
         "@com_github_golang_protobuf//ptypes/duration:go_default_library",
         "@com_github_golang_protobuf//ptypes/timestamp:go_default_library",

templates/goshared/known.go

@@ -56,7 +56,7 @@ const emailTpl = `
 
 const uuidTpl = `
 	func (m {{ (msgTyp .).Pointer }}) _validateUuid(uuid string) error {
-		if matched := _{{ .File.InputPath.BaseName }}_uuidPattern.MatchString(uuid); !matched {
+		if matched := _{{ snakeCase .File.InputPath.BaseName }}_uuidPattern.MatchString(uuid); !matched {
 			return errors.New("invalid uuid format")
 		}
 

templates/goshared/register.go

@@ -2,6 +2,7 @@ package goshared
 
 import (
 	"fmt"
+	"github.com/iancoleman/strcase"
 	"reflect"
 	"strings"
 	"text/template"
@@ -20,6 +21,7 @@ func Register(tpl *template.Template, params pgs.Parameters) {
 	tpl.Funcs(map[string]interface{}{
 		"accessor":      fns.accessor,
 		"byteStr":       fns.byteStr,
+		"snakeCase":	 fns.snakeCase,
 		"cmt":           pgs.C80,
 		"durGt":         fns.durGt,
 		"durLit":        fns.durLit,
@@ -311,3 +313,7 @@ func (fns goSharedFuncs) enumPackages(enums []pgs.Enum) map[pgs.FilePath]pgs.Nam
 
 	return out
 }
+
+func (fns goSharedFuncs) snakeCase(name string) string {
+	return strcase.ToSnake(name)
+}

templates/java/register.go

@@ -161,7 +161,15 @@ func classNameFile(f pgs.File) string {
 }
 
 func classNameMessage(m pgs.Message) string {
-	return sanitizeClassName(m.Name().String())
+	className := m.Name().String()
+	// This is really silly, but when the multiple files option is true, protoc puts underscores in file names.
+	// When multiple files is false, underscores are stripped. Short of rewriting all the name sanitization
+	// logic for java, using "UnderscoreUnderscoreUnderscore" is an escape sequence seems to work with an extremely
+	// small likelihood of name conflict.
+	className = strings.Replace(className, "_", "UnderscoreUnderscoreUnderscore", -1)
+	className = sanitizeClassName(className)
+	className = strings.Replace(className, "UnderscoreUnderscoreUnderscore", "_", -1)
+	return className
 }
 
 func sanitizeClassName(className string) string {