Proper close of statement in getProperties

neo4j · Dec 4, 2017 · 26cbd27 · 26cbd27
1 parent 1dbb05e
commit 26cbd27
Showing 1 changed file with 35 additions and 59 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java
@@ -28,6 +28,7 @@
 import java.util.Objects;
 
 import org.neo4j.collection.primitive.PrimitiveIntIterator;
+import org.neo4j.cursor.Cursor;
 import org.neo4j.graphdb.ConstraintViolationException;
 import org.neo4j.graphdb.Direction;
 import org.neo4j.graphdb.GraphDatabaseService;
@@ -57,6 +58,8 @@
 import org.neo4j.kernel.api.exceptions.schema.TooManyLabelsException;
 import org.neo4j.kernel.impl.api.operations.KeyReadOperations;
 import org.neo4j.storageengine.api.EntityType;
+import org.neo4j.storageengine.api.NodeItem;
+import org.neo4j.storageengine.api.PropertyItem;
 import org.neo4j.values.storable.Value;
 import org.neo4j.values.storable.Values;
 
@@ -345,33 +348,35 @@ public Object getProperty( String key, Object defaultValue )
         {
             throw new IllegalArgumentException( "(null) property key is not allowed" );
         }
-        KernelTransaction transaction = actions.kernelTransaction();
-        CursorFactory cursors = transaction.cursors();
-        try ( NodeCursor nodes = cursors.allocateNodeCursor();
-              PropertyCursor properties = cursors.allocatePropertyCursor();
-              Statement ignore = actions.statement()
-        )
+        try ( Statement ignore = actions.statement() )
         {
-            int propertyKey = transaction.token().propertyKey( key );
-            if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY )
-            {
-                return defaultValue;
-            }
-            transaction.dataRead().singleNode( nodeId, nodes );
-            if ( !nodes.next() )
+            KernelTransaction transaction = actions.kernelTransaction();
+            CursorFactory cursors = transaction.cursors();
+            try ( NodeCursor nodes = cursors.allocateNodeCursor();
+                  PropertyCursor properties = cursors.allocatePropertyCursor()
+            )
             {
-                throw new NotFoundException( new EntityNotFoundException( EntityType.NODE, nodeId ) );
-            }
-            nodes.properties( properties );
-            while ( properties.next() )
-            {
-                if ( propertyKey == properties.propertyKey() )
+                int propertyKey = transaction.token().propertyKey( key );
+                if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY )
                 {
-                    Value value = properties.propertyValue();
-                    return value == Values.NO_VALUE ? defaultValue : value.asObjectCopy();
+                    return defaultValue;
                 }
+                transaction.dataRead().singleNode( nodeId, nodes );
+                if ( !nodes.next() )
+                {
+                    throw new NotFoundException( new EntityNotFoundException( EntityType.NODE, nodeId ) );
+                }
+                nodes.properties( properties );
+                while ( properties.next() )
+                {
+                    if ( propertyKey == properties.propertyKey() )
+                    {
+                        Value value = properties.propertyValue();
+                        return value == Values.NO_VALUE ? defaultValue : value.asObjectCopy();
+                    }
+                }
+                return defaultValue;
             }
-            return defaultValue;
         }
     }
 
@@ -421,49 +426,20 @@ public Map<String,Object> getProperties( String... keys )
         Map<String,Object> properties = new HashMap<>( itemsToReturn );
         Token token = transaction.token();
 
-        //Find ids, note we are betting on that the number of keys
-        //is small enough not to use a set here.
-        int[] propertyIds = new int[itemsToReturn];
-        for ( int i = 0; i < itemsToReturn; i++ )
-        {
-            String key = keys[i];
-            if ( key == null )
-            {
-                throw new NullPointerException( String.format( "Key %d was null", i + 1 ) );
-            }
-            propertyIds[i] = token.propertyKey( key );
-        }
-        try ( NodeCursor nodes = cursors.allocateNodeCursor();
-              PropertyCursor propertyCursor = cursors.allocatePropertyCursor();
-              Statement ignore = actions.statement() )
+        try ( Statement statement = actions.statement() )
         {
-            transaction.dataRead().singleNode( nodeId, nodes );
-            if ( !nodes.next() )
+            try ( Cursor<NodeItem> node = statement.readOperations().nodeCursorById( nodeId ) )
             {
-                throw new NotFoundException( new EntityNotFoundException( EntityType.NODE, nodeId ) );
-            }
-            nodes.properties( propertyCursor );
-            while ( propertyCursor.next() )
-            {
-                //Do a linear check if this is a property we are interested in.
-                for ( int propertyId : propertyIds )
+                try ( Cursor<PropertyItem> propertyCursor = statement.readOperations().nodeGetProperties( node.get() ) )
                 {
-                    int currentKey = propertyCursor.propertyKey();
-                    if ( propertyId == currentKey )
-                    {
-                        properties.put( token.propertyKeyGetName( currentKey ),
-                                propertyCursor.propertyValue().asObjectCopy() );
-                    }
+                    return PropertyContainerProxyHelper.getProperties( statement, propertyCursor, keys );
                 }
-
+            }
+            catch ( EntityNotFoundException e )
+            {
+                throw new NotFoundException( "Node not found", e );
             }
         }
-        catch ( PropertyKeyIdNotFoundKernelException e )
-        {
-            throw new IllegalStateException( "Property key retrieved through kernel API should exist.", e );
-        }
-
-        return properties;
     }
 
     @Override