Centralize field validation

Field validation was unintentionally absent in user-defined function processing. This commit finally introduces it, by reusing the existing procedure field validation logic.
neo4j · Jun 23, 2017 · b45420d · b45420d
1 parent 7c60ccf
commit b45420d
Show file tree

Hide file tree

Showing 17 changed files with 247 additions and 137 deletions.
diff --git a/...ocedure-compiler/processor/src/main/java/org/neo4j/tooling/procedure/CompilerOptions.java b/...ocedure-compiler/processor/src/main/java/org/neo4j/tooling/procedure/CompilerOptions.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.tooling.procedure;
+
+public class CompilerOptions
+{
+    public static final String IGNORE_CONTEXT_WARNINGS_OPTION = "IgnoreContextWarnings";
+
+    private CompilerOptions()
+    {
+    }
+}
diff --git a/...dure-compiler/processor/src/main/java/org/neo4j/tooling/procedure/ProcedureProcessor.java b/...dure-compiler/processor/src/main/java/org/neo4j/tooling/procedure/ProcedureProcessor.java
@@ -42,18 +42,19 @@
 import org.neo4j.tooling.procedure.compilerutils.CustomNameExtractor;
 import org.neo4j.tooling.procedure.messages.CompilationMessage;
 import org.neo4j.tooling.procedure.messages.MessagePrinter;
-import org.neo4j.tooling.procedure.validators.DuplicatedProcedureValidator;
+import org.neo4j.tooling.procedure.validators.DuplicatedExtensionValidator;
 import org.neo4j.tooling.procedure.visitors.StoredProcedureVisitor;
 
+import static org.neo4j.tooling.procedure.CompilerOptions.IGNORE_CONTEXT_WARNINGS_OPTION;
+
 @AutoService( Processor.class )
 public class ProcedureProcessor extends AbstractProcessor
 {
 
-    public static final String IGNORE_CONTEXT_WARNINGS_OPTION = "IgnoreContextWarnings";
     private static final Class<Procedure> sprocType = Procedure.class;
     private final Set<Element> visitedProcedures = new LinkedHashSet<>();
 
-    private Function<Collection<Element>,Stream<CompilationMessage>> duplicationPredicate;
+    private Function<Collection<Element>,Stream<CompilationMessage>> duplicationValidator;
     private ElementVisitor<Stream<CompilationMessage>,Void> visitor;
     private MessagePrinter messagePrinter;
 
@@ -86,8 +87,8 @@ public synchronized void init( ProcessingEnvironment processingEnv )
         messagePrinter = new MessagePrinter( processingEnv.getMessager() );
         visitor = new StoredProcedureVisitor( typeUtils, elementUtils,
                 processingEnv.getOptions().containsKey( IGNORE_CONTEXT_WARNINGS_OPTION ) );
-        duplicationPredicate =
-                new DuplicatedProcedureValidator<>( elementUtils, sprocType,
+        duplicationValidator =
+                new DuplicatedExtensionValidator<>( elementUtils, sprocType,
                         ( proc ) -> CustomNameExtractor.getName( proc::name, proc::value ) );
     }
 
@@ -98,7 +99,7 @@ public boolean process( Set<? extends TypeElement> annotations, RoundEnvironment
         processElements( roundEnv );
         if ( roundEnv.processingOver() )
         {
-            duplicationPredicate.apply( visitedProcedures ).forEach( messagePrinter::print );
+            duplicationValidator.apply( visitedProcedures ).forEach( messagePrinter::print );
         }
         return false;
     }

diff --git a/...e-compiler/processor/src/main/java/org/neo4j/tooling/procedure/UserFunctionProcessor.java b/...e-compiler/processor/src/main/java/org/neo4j/tooling/procedure/UserFunctionProcessor.java
@@ -21,15 +21,8 @@
 
 import com.google.auto.service.AutoService;
 
-import org.neo4j.tooling.procedure.compilerutils.CustomNameExtractor;
-import org.neo4j.tooling.procedure.compilerutils.TypeMirrorUtils;
-import org.neo4j.tooling.procedure.messages.CompilationMessage;
-import org.neo4j.tooling.procedure.messages.MessagePrinter;
-import org.neo4j.tooling.procedure.validators.DuplicatedProcedureValidator;
-import org.neo4j.tooling.procedure.visitors.UserFunctionVisitor;
-
 import java.util.Collection;
-import java.util.HashSet;
+import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.function.Function;
@@ -46,23 +39,35 @@
 import javax.lang.model.util.Types;
 
 import org.neo4j.procedure.UserFunction;
+import org.neo4j.tooling.procedure.compilerutils.CustomNameExtractor;
+import org.neo4j.tooling.procedure.compilerutils.TypeMirrorUtils;
+import org.neo4j.tooling.procedure.messages.CompilationMessage;
+import org.neo4j.tooling.procedure.messages.MessagePrinter;
+import org.neo4j.tooling.procedure.validators.DuplicatedExtensionValidator;
+import org.neo4j.tooling.procedure.visitors.UserFunctionVisitor;
+
+import static org.neo4j.tooling.procedure.CompilerOptions.IGNORE_CONTEXT_WARNINGS_OPTION;
 
 @AutoService( Processor.class )
 public class UserFunctionProcessor extends AbstractProcessor
 {
     private static final Class<UserFunction> userFunctionType = UserFunction.class;
     private final Set<Element> visitedFunctions = new LinkedHashSet<>();
 
+    private Function<Collection<Element>,Stream<CompilationMessage>> duplicationValidator;
     private ElementVisitor<Stream<CompilationMessage>,Void> visitor;
     private MessagePrinter messagePrinter;
-    private Function<Collection<Element>,Stream<CompilationMessage>> duplicationPredicate;
+
+    @Override
+    public Set<String> getSupportedOptions()
+    {
+        return Collections.singleton( IGNORE_CONTEXT_WARNINGS_OPTION );
+    }
 
     @Override
     public Set<String> getSupportedAnnotationTypes()
     {
-        Set<String> types = new HashSet<>();
-        types.add( userFunctionType.getName() );
-        return types;
+        return Collections.singleton( userFunctionType.getName() );
     }
 
     @Override
@@ -80,8 +85,9 @@ public synchronized void init( ProcessingEnvironment processingEnv )
 
         visitedFunctions.clear();
         messagePrinter = new MessagePrinter( processingEnv.getMessager() );
-        visitor = new UserFunctionVisitor( typeUtils, elementUtils, new TypeMirrorUtils( typeUtils, elementUtils ) );
-        duplicationPredicate = new DuplicatedProcedureValidator<>( elementUtils, userFunctionType,
+        visitor = new UserFunctionVisitor( typeUtils, elementUtils, new TypeMirrorUtils( typeUtils, elementUtils ),
+                processingEnv.getOptions().containsKey( IGNORE_CONTEXT_WARNINGS_OPTION ) );
+        duplicationValidator = new DuplicatedExtensionValidator<>( elementUtils, userFunctionType,
                 ( function ) -> CustomNameExtractor.getName( function::name, function::value ) );
     }
 
@@ -91,7 +97,7 @@ public boolean process( Set<? extends TypeElement> annotations, RoundEnvironment
         processElements( roundEnv );
         if ( roundEnv.processingOver() )
         {
-            duplicationPredicate.apply( visitedFunctions ).forEach( messagePrinter::print );
+            duplicationValidator.apply( visitedFunctions ).forEach( messagePrinter::print );
         }
         return false;
     }

diff --git a/...ocedureMissingPublicNoArgConstructor.java → ...tensionMissingPublicNoArgConstructor.java b/...ocedureMissingPublicNoArgConstructor.java → ...tensionMissingPublicNoArgConstructor.java
@@ -21,13 +21,13 @@
 
 import javax.lang.model.element.Element;
 
-public class ProcedureMissingPublicNoArgConstructor implements CompilationMessage
+public class ExtensionMissingPublicNoArgConstructor implements CompilationMessage
 {
 
     private final Element element;
     private final String errorMessage;
 
-    public ProcedureMissingPublicNoArgConstructor( Element element, String message, Object... args )
+    public ExtensionMissingPublicNoArgConstructor( Element element, String message, Object... args )
     {
 
         this.element = element;

diff --git a/...idators/DuplicatedProcedureValidator.java → ...idators/DuplicatedExtensionValidator.java b/...idators/DuplicatedProcedureValidator.java → ...idators/DuplicatedExtensionValidator.java
@@ -38,15 +38,15 @@
 
 import static java.util.stream.Collectors.groupingBy;
 
-public class DuplicatedProcedureValidator<T extends Annotation>
+public class DuplicatedExtensionValidator<T extends Annotation>
         implements Function<Collection<Element>,Stream<CompilationMessage>>
 {
 
     private final Elements elements;
     private final Class<T> annotationType;
     private final Function<T,Optional<String>> customNameExtractor;
 
-    public DuplicatedProcedureValidator( Elements elements, Class<T> annotationType,
+    public DuplicatedExtensionValidator( Elements elements, Class<T> annotationType,
             Function<T,Optional<String>> customNameExtractor )
     {
         this.elements = elements;

diff --git a/...ler/processor/src/main/java/org/neo4j/tooling/procedure/visitors/ContextFieldVisitor.java b/...ler/processor/src/main/java/org/neo4j/tooling/procedure/visitors/ContextFieldVisitor.java
@@ -48,6 +48,8 @@
 import org.neo4j.tooling.procedure.messages.ContextFieldError;
 import org.neo4j.tooling.procedure.messages.ContextFieldWarning;
 
+import static org.neo4j.tooling.procedure.CompilerOptions.IGNORE_CONTEXT_WARNINGS_OPTION;
+
 class ContextFieldVisitor extends SimpleElementVisitor8<Stream<CompilationMessage>,Void>
 {
     private static final Set<String> SUPPORTED_TYPES = new LinkedHashSet<>(
@@ -108,7 +110,7 @@ private Stream<CompilationMessage> validateInjectedTypes( VariableElement field
                     "The procedure will not load unless declared via the configuration option 'dbms.security.procedures.unrestricted'.\n" +
                     "You can ignore this warning by passing the option -A%s to the Java compiler",
                     Context.class.getName(), fieldType.toString(), fieldFullName( field ),
-                    ProcedureProcessor.IGNORE_CONTEXT_WARNINGS_OPTION ) );
+                    IGNORE_CONTEXT_WARNINGS_OPTION ) );
         }
 
         return Stream.of( new ContextFieldError( field,

diff --git a/...visitors/StoredProcedureClassVisitor.java → ...edure/visitors/ExtensionClassVisitor.java b/...visitors/StoredProcedureClassVisitor.java → ...edure/visitors/ExtensionClassVisitor.java
@@ -32,27 +32,27 @@
 import javax.lang.model.util.Types;
 
 import org.neo4j.tooling.procedure.messages.CompilationMessage;
-import org.neo4j.tooling.procedure.messages.ProcedureMissingPublicNoArgConstructor;
+import org.neo4j.tooling.procedure.messages.ExtensionMissingPublicNoArgConstructor;
 
 import static javax.lang.model.util.ElementFilter.constructorsIn;
 
-public class StoredProcedureClassVisitor extends SimpleElementVisitor8<Stream<CompilationMessage>,Void>
+public class ExtensionClassVisitor extends SimpleElementVisitor8<Stream<CompilationMessage>,Void>
 {
 
     private final Set<TypeElement> visitedElements = new HashSet<>();
     private final FieldVisitor fieldVisitor;
 
-    public StoredProcedureClassVisitor( Types types, Elements elements, boolean ignoresWarnings )
+    public ExtensionClassVisitor( Types types, Elements elements, boolean ignoresWarnings )
     {
         fieldVisitor = new FieldVisitor( types, elements, ignoresWarnings );
     }
 
     @Override
-    public Stream<CompilationMessage> visitType( TypeElement procedureClass, Void ignored )
+    public Stream<CompilationMessage> visitType( TypeElement extensionClass, Void ignored )
     {
-        if ( isFirstVisit( procedureClass ) )
+        if ( isFirstVisit( extensionClass ) )
         {
-            return Stream.concat( validateFields( procedureClass ), validateConstructor( procedureClass ) );
+            return Stream.concat( validateFields( extensionClass ), validateConstructor( extensionClass ) );
         }
         return Stream.empty();
     }
@@ -74,17 +74,17 @@ private Stream<CompilationMessage> validateFields( TypeElement e )
         return e.getEnclosedElements().stream().flatMap( fieldVisitor::visit );
     }
 
-    private Stream<CompilationMessage> validateConstructor( Element procedureClass )
+    private Stream<CompilationMessage> validateConstructor( Element extensionClass )
     {
         Optional<ExecutableElement> publicNoArgConstructor =
-                constructorsIn( procedureClass.getEnclosedElements() ).stream()
+                constructorsIn( extensionClass.getEnclosedElements() ).stream()
                         .filter( c -> c.getModifiers().contains( Modifier.PUBLIC ) )
                         .filter( c -> c.getParameters().isEmpty() ).findFirst();
 
         if ( !publicNoArgConstructor.isPresent() )
         {
-            return Stream.of( new ProcedureMissingPublicNoArgConstructor( procedureClass,
-                    "Procedure class %s should contain a public no-arg constructor, none found.", procedureClass ) );
+            return Stream.of( new ExtensionMissingPublicNoArgConstructor( extensionClass,
+                    "Extension class %s should contain a public no-arg constructor, none found.", extensionClass ) );
         }
         return Stream.empty();
     }

diff --git a/.../processor/src/main/java/org/neo4j/tooling/procedure/visitors/StoredProcedureVisitor.java b/.../processor/src/main/java/org/neo4j/tooling/procedure/visitors/StoredProcedureVisitor.java
@@ -54,7 +54,7 @@ public StoredProcedureVisitor( Types typeUtils, Elements elementUtils, boolean i
 
         this.typeUtils = typeUtils;
         this.elementUtils = elementUtils;
-        this.classVisitor = new StoredProcedureClassVisitor( typeUtils, elementUtils, ignoresWarnings );
+        this.classVisitor = new ExtensionClassVisitor( typeUtils, elementUtils, ignoresWarnings );
         this.recordVisitor = new RecordTypeVisitor( typeUtils, typeMirrors );
         this.parameterVisitor = new ParameterVisitor( new ParameterTypeVisitor( typeUtils, typeMirrors ) );
         this.performsWriteVisitor = new PerformsWriteMethodVisitor();

diff --git a/...ler/processor/src/main/java/org/neo4j/tooling/procedure/visitors/UserFunctionVisitor.java b/...ler/processor/src/main/java/org/neo4j/tooling/procedure/visitors/UserFunctionVisitor.java
@@ -19,13 +19,8 @@
  */
 package org.neo4j.tooling.procedure.visitors;
 
-import org.neo4j.tooling.procedure.compilerutils.TypeMirrorUtils;
-import org.neo4j.tooling.procedure.messages.CompilationMessage;
-import org.neo4j.tooling.procedure.messages.FunctionInRootNamespaceError;
-import org.neo4j.tooling.procedure.messages.ReturnTypeError;
-import org.neo4j.tooling.procedure.validators.AllowedTypesValidator;
-
 import java.util.List;
+import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 import javax.lang.model.element.ElementVisitor;
@@ -38,27 +33,34 @@
 import javax.lang.model.util.Types;
 
 import org.neo4j.procedure.UserFunction;
+import org.neo4j.tooling.procedure.compilerutils.TypeMirrorUtils;
+import org.neo4j.tooling.procedure.messages.CompilationMessage;
+import org.neo4j.tooling.procedure.messages.FunctionInRootNamespaceError;
+import org.neo4j.tooling.procedure.messages.ReturnTypeError;
+import org.neo4j.tooling.procedure.validators.AllowedTypesValidator;
 
 public class UserFunctionVisitor extends SimpleElementVisitor8<Stream<CompilationMessage>,Void>
 {
 
     private final ElementVisitor<Stream<CompilationMessage>,Void> parameterVisitor;
     private final Predicate<TypeMirror> allowedTypesValidator;
     private final Elements elements;
+    private final ElementVisitor<Stream<CompilationMessage>,Void> classVisitor;
 
-    public UserFunctionVisitor( Types types, Elements elements, TypeMirrorUtils typeMirrorUtils )
+    public UserFunctionVisitor( Types types, Elements elements, TypeMirrorUtils typeMirrorUtils, boolean ignoresWarnings )
     {
+        this.classVisitor = new ExtensionClassVisitor( types, elements, ignoresWarnings );
         this.parameterVisitor = new ParameterVisitor( new ParameterTypeVisitor( types, typeMirrorUtils ) );
         this.allowedTypesValidator = new AllowedTypesValidator( typeMirrorUtils, types );
         this.elements = elements;
     }
 
     @Override
-    public Stream<CompilationMessage> visitExecutable( ExecutableElement method, Void ignored )
+    public Stream<CompilationMessage> visitExecutable( ExecutableElement executableElement, Void ignored )
     {
-        return Stream
-                .concat( Stream.concat( validateParameters( method.getParameters(), ignored ), validateName( method ) ),
-                        validateReturnType( method ) );
+        return Stream.of( classVisitor.visit( executableElement.getEnclosingElement() ),
+                validateParameters( executableElement.getParameters(), ignored ), validateName( executableElement ),
+                validateReturnType( executableElement ) ).flatMap( Function.identity() );
     }
 
     private Stream<CompilationMessage> validateParameters( List<? extends VariableElement> parameters, Void ignored )