Handle procedures in root package, and improve error message when usi…

…ng stdlib types as records.

community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/ProcedureSignature.scala

@@ -28,14 +28,14 @@ sealed trait ProcedureMode {
   def call(ctx: QueryContext, signature: ProcedureSignature, args: Seq[Any]): Iterator[Array[AnyRef]]
 }
 
-case object procReadOnly extends ProcedureMode {
+case object ProcReadOnly extends ProcedureMode {
   override val queryType: InternalQueryType = READ_ONLY
 
   override def call(ctx: QueryContext, signature: ProcedureSignature, args: Seq[Any]): Iterator[Array[AnyRef]] =
     ctx.callReadOnlyProcedure(signature, args)
 }
 
-case object procReadWrite extends ProcedureMode {
+case object ProcReadWrite extends ProcedureMode {
   override val queryType: InternalQueryType = READ_WRITE
 
   override def call(ctx: QueryContext, signature: ProcedureSignature, args: Seq[Any]): Iterator[Array[AnyRef]] =
@@ -45,7 +45,7 @@ case object procReadWrite extends ProcedureMode {
 case class ProcedureSignature(name: ProcedureName,
                               inputSignature: Seq[FieldSignature],
                               outputSignature: Seq[FieldSignature],
-                              mode: ProcedureMode = procReadOnly )
+                              mode: ProcedureMode = ProcReadOnly )
 
 case class ProcedureName(namespace: Seq[String], name: String)
 

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_0/TransactionBoundPlanContext.scala

@@ -133,10 +133,10 @@ class TransactionBoundPlanContext(initialStatement: Statement, val gdb: GraphDat
   }
 
   private def asCypherProcMode(mode: KernelProcedureSignature.Mode): ProcedureMode = mode match {
-    case KernelProcedureSignature.Mode.READ_ONLY => procReadOnly
-    case KernelProcedureSignature.Mode.READ_WRITE => procReadWrite
+    case KernelProcedureSignature.Mode.READ_ONLY => ProcReadOnly
+    case KernelProcedureSignature.Mode.READ_WRITE => ProcReadWrite
     case _ => throw new CypherExecutionException(
-      "Unable to execute procedure, because it requires an unrecognized excution mode: " + mode.name(), null )
+      "Unable to execute procedure, because it requires an unrecognized execution mode: " + mode.name(), null )
   }
 
   private def asCypherType(neoType: AnyType): CypherType = neoType match {

community/graphdb-api/src/main/java/org/neo4j/graphdb/index/IndexHits.java

@@ -22,13 +22,16 @@
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 
 import org.neo4j.graphdb.Node;
 import org.neo4j.graphdb.Relationship;
 import org.neo4j.graphdb.ResourceIterable;
 import org.neo4j.graphdb.ResourceIterator;
 import org.neo4j.graphdb.Transaction;
 
+import static java.util.Spliterators.spliteratorUnknownSize;
+
 /**
  * An {@link Iterator} with additional {@link #size()} and {@link #close()}
  * methods on it, used for iterating over index query results. It is first and
@@ -90,10 +93,18 @@ public interface IndexHits<T> extends ResourceIterator<T>, ResourceIterable<T>
      */
     void close();
 
-    @Override
+    /**
+     * @return these index hits in a {@link Stream}
+     */
     default Stream<T> stream()
     {
-        return iterator().stream();
+        // Implementation note: we need this for two reasons:
+        // one, to disambiguate #stream between ResourceIterator and ResourceIterable,
+        // two, because implementations of this return themselves on #iterator, so we can't
+        //      use #iterator().stream(), that then causes stack overflows.
+        return StreamSupport
+                .stream( spliteratorUnknownSize( this, 0 ), false )
+                .onClose( this::close );
     }
 
     /**

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/OutputMappers.java

@@ -147,6 +147,8 @@ public OutputMapper mapper( Method method ) throws ProcedureException
 
     public OutputMapper mapper( Class<?> userClass ) throws ProcedureException
     {
+        assertIsValidRecordClass( userClass );
+
         List<Field> fields = instanceFields( userClass );
         FieldSignature[] signature = new FieldSignature[fields.size()];
         FieldMapper[] fieldMappers = new FieldMapper[fields.size()];
@@ -185,6 +187,24 @@ public OutputMapper mapper( Class<?> userClass ) throws ProcedureException
         return new OutputMapper( signature, fieldMappers );
     }
 
+    private void assertIsValidRecordClass( Class<?> userClass ) throws ProcedureException
+    {
+        if( userClass.isPrimitive() || userClass.isArray()
+            || userClass.getPackage() != null && userClass.getPackage().getName().startsWith( "java." ) )
+        {
+            throw new ProcedureException( Status.Procedure.TypeError,
+                    "Procedures must return a Stream of records, where a record is a concrete class " +
+                    "that you define, with public non-final fields defining the fields in the record. " +
+                    "If you'd like your procedure to return `%s`, you could define a record class like:\n" +
+                    "public class Output {\n" +
+                    "    public %s out;\n" +
+                    "}\n" +
+                    "\n" +
+                    "And then define your procedure as returning `Stream<Output>`.",
+                    userClass.getSimpleName(), userClass.getSimpleName() );
+        }
+    }
+
     private List<Field> instanceFields( Class<?> userClass )
     {
         return asList( userClass.getDeclaredFields() ).stream().filter( ( f ) -> !isStatic( f.getModifiers() ) ).collect( toList() );

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ReflectiveProcedureCompiler.java

@@ -134,7 +134,9 @@ private MethodHandle constructor( Class<?> procDefinition ) throws ProcedureExce
 
     private ProcedureName extractName( Class<?> procDefinition, Method m )
     {
-        String[] namespace = procDefinition.getPackage().getName().split( "\\." );
+        Package pkg = procDefinition.getPackage();
+        // Package is null if class is in root package
+        String[] namespace = pkg == null ? new String[0] : pkg.getName().split( "\\." );
         String name = m.getName();
         return new ProcedureName( namespace, name );
     }

community/kernel/src/test/java/org/neo4j/kernel/impl/proc/OutputMappersTest.java

@@ -147,6 +147,29 @@ public void shouldGiveHelpfulErrorOnMapWithNonStringKeyMap() throws Throwable
         mapper( RecordWithNonStringKeyMap.class );
     }
 
+    @Test
+    public void shouldWarnAgainstStdLibraryClassesSinceTheseIndicateUserError() throws Throwable
+    {
+        // Impl note: We may want to change this behavior and actually allow procedures to return `Long` etc,
+        //            with a default column name. So Stream<Long> would become records like (out: Long)
+        //            Drawback of that is that it'd cause cognitive dissonance, it's not obvious what's a record
+        //            and what is a primitive value..
+
+        // Expect
+        exception.expect( ProcedureException.class );
+        exception.expectMessage( "Procedures must return a Stream of records, where a record is a concrete class that you define, with public non-final " +
+                                 "fields defining the fields in the record. If you'd like your procedure to return `Long`, you could define a record class " +
+                                 "like:\n" +
+                                 "public class Output {\n" +
+                                 "    public Long out;\n" +
+                                 "}\n" +
+                                 "\n" +
+                                 "And then define your procedure as returning `Stream<Output>`." );
+
+        // When
+        mapper(Long.class);
+    }
+
     private OutputMapper mapper( Class<?> clazz ) throws ProcedureException
     {
         return new OutputMappers( new TypeMappers() ).mapper( clazz );

community/neo4j-harness/src/main/java/org/neo4j/harness/junit/Neo4jRule.java

@@ -140,7 +140,7 @@ public Neo4jRule copyFrom( File sourceDirectory )
     }
 
     @Override
-    public TestServerBuilder withProcedure( Class<?> procedureClass )
+    public Neo4jRule withProcedure( Class<?> procedureClass )
     {
         builder = builder.withProcedure( procedureClass );
         return this;