Made two different errors depending on how the loading failed

neo4j · Feb 8, 2017 · 44eb129 · 44eb129
1 parent 457cbe5
commit 44eb129
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 54 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java
@@ -175,26 +175,7 @@ List<CallableProcedure> compileProcedure( Class<?> procDefinition, Optional<Stri
             ArrayList<CallableProcedure> out = new ArrayList<>( procedureMethods.size() );
             for ( Method method : procedureMethods )
             {
-                String valueName = method.getAnnotation( Procedure.class ).value();
-                String definedName = method.getAnnotation( Procedure.class ).name();
-                QualifiedName procName = extractName( procDefinition, method, valueName, definedName );
-                List<FieldSignature> inputSignature = inputSignatureDeterminer.signatureFor( method );
-                OutputMapper outputMapper = outputMappers.mapper( method );
-                try
-                {
-                    out.add( compileProcedure( procDefinition, constructor, method, warning, fullAccess, procName,
-                            inputSignature, outputMapper ) );
-                }
-                catch ( ProcedureInjectionException e )
-                {
-                    log.warn( e.getMessage() );
-                    Optional<String> description = Optional.of(
-                            procName + " is not available due to not having access to one or more components." );
-                    ProcedureSignature signature =
-                            new ProcedureSignature( procName, inputSignature, outputMapper.signature(), Mode.DEFAULT,
-                                    Optional.empty(), new String[0], description, warning );
-                    out.add( new LoadFailProcedure( signature ) );
-                }
+                out.add( compileProcedure( procDefinition, constructor, method, warning, fullAccess ) );
             }
             out.sort( Comparator.comparing( a -> a.signature().name().toString() ) );
             return out;
@@ -210,21 +191,17 @@ List<CallableProcedure> compileProcedure( Class<?> procDefinition, Optional<Stri
         }
     }
 
-    private ReflectiveProcedure compileProcedure( Class<?> procDefinition, MethodHandle constructor, Method method,
-            Optional<String> warning, boolean fullAccess, QualifiedName procName, List<FieldSignature> inputSignature,
-            OutputMapper outputMapper )
+    private CallableProcedure compileProcedure( Class<?> procDefinition, MethodHandle constructor, Method method,
+            Optional<String> warning, boolean fullAccess )
             throws ProcedureException, IllegalAccessException
     {
         MethodHandle procedureMethod = lookup.unreflect( method );
-        List<FieldInjections.FieldSetter> setters;
-        if ( fullAccess || config.fullAccessFor( procName.toString() ) )
-        {
-            setters = allFieldInjections.setters( procDefinition );
-        }
-        else
-        {
-            setters = safeFieldInjections.setters( procDefinition );
-        }
+
+        String valueName = method.getAnnotation( Procedure.class ).value();
+        String definedName = method.getAnnotation( Procedure.class ).name();
+        QualifiedName procName = extractName( procDefinition, method, valueName, definedName );
+        List<FieldSignature> inputSignature = inputSignatureDeterminer.signatureFor( method );
+        OutputMapper outputMapper = outputMappers.mapper( method );
 
         Optional<String> description = description( method );
         Procedure procedure = method.getAnnotation( Procedure.class );
@@ -245,10 +222,29 @@ private ReflectiveProcedure compileProcedure( Class<?> procDefinition, MethodHan
         Optional<String> deprecated = deprecated( method, procedure::deprecatedBy,
                 "Use of @Procedure(deprecatedBy) without @Deprecated in " + procName );
 
-        ProcedureSignature signature =
-                new ProcedureSignature( procName, inputSignature, outputMapper.signature(),
-                        mode, deprecated, config.rolesFor( procName.toString() ), description, warning );
+        List<FieldInjections.FieldSetter> setters;
+        setters = allFieldInjections.setters( procDefinition );
+        ProcedureSignature signature;
 
+        if ( !fullAccess && !config.fullAccessFor( procName.toString() ) )
+        {
+            try
+            {
+                setters = safeFieldInjections.setters( procDefinition );
+            }
+            catch ( ProcedureInjectionException e )
+            {
+                log.warn( e.getMessage() );
+                description = Optional.of( procName.toString() +
+                        "is not available due to not having unrestricted access rights, check configuration." );
+
+                signature = new ProcedureSignature( procName, inputSignature, outputMapper.signature(), Mode.DEFAULT,
+                        Optional.empty(), new String[0], description, warning );
+                return new LoadFailProcedure( signature );
+            }
+        }
+        signature = new ProcedureSignature( procName, inputSignature, outputMapper.signature(), mode, deprecated,
+                config.rolesFor( procName.toString() ), description, warning );
         return new ReflectiveProcedure( signature, constructor, procedureMethod, outputMapper, setters );
     }
 

diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ResourceInjectionTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ResourceInjectionTest.java
@@ -70,26 +70,15 @@ public void shouldCompileAndRunProcedure() throws Throwable
     @Test
     public void shouldFailNicelyWhenUnknownAPI() throws Throwable
     {
-        // When
-        List<CallableProcedure> procList = compile( procedureWithUnknownAPI.class, true );
+        //When
+        exception.expect( ProcedureException.class );
+        exception.expectMessage( "Unable to set up injection for procedure `procedureWithUnknownAPI`, " +
+                "the field `api` has type `class org.neo4j.kernel.impl.proc.ResourceInjectionTest$UnknownAPI` " +
+                "which is not a known injectable component." );
 
-        //Then
-        verify( log )
-                .warn( "Unable to set up injection for procedure `procedureWithUnknownAPI`, the field `api` has type" +
-                        " `class org.neo4j.kernel.impl.proc.ResourceInjectionTest$UnknownAPI`" +
-                        " which is not a known injectable component." );
+        // Then
+        compile( procedureWithUnknownAPI.class, true );
 
-        assertThat( procList.size(), equalTo( 1 ) );
-        try
-        {
-            procList.get( 0 ).apply( new BasicContext(), new Object[0] );
-            fail();
-        }
-        catch ( ProcedureException e )
-        {
-            assertThat( e.getMessage(), containsString(
-                    "org.neo4j.kernel.impl.proc.listCoolPeople is not available due to not having access to one or more components." ) );
-        }
     }
 
     @Test
@@ -127,7 +116,8 @@ public void shouldFailNicelyWhenUnsafeAPISafeMode() throws Throwable
         catch ( ProcedureException e )
         {
             assertThat( e.getMessage(), containsString(
-                    "org.neo4j.kernel.impl.proc.listCoolPeople is not available due to not having access to one or more components." ) );
+                    "org.neo4j.kernel.impl.proc.listCoolPeopleis not " +
+                            "available due to not having unrestricted access rights, check configuration." ) );
         }
 
     }