Skip to content

Commit

Permalink
Fixes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@pontusmelke
pontusmelke committed Dec 4, 2017
1 parent 3c8b782 commit e56ab3a
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 35 deletions.
63 changes: 43 additions & 20 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/core/NodeProxy.java
View file
Expand Up @@ -28,7 +28,6 @@
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;
Expand Down Expand Up @@ -58,8 +57,6 @@
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;

Expand Down Expand Up @@ -351,16 +348,17 @@ public Object getProperty( String key, Object defaultValue )
try ( Statement ignore = actions.statement() )
{
KernelTransaction transaction = actions.kernelTransaction();
int propertyKey = transaction.token().propertyKey( key );
if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY )
{
return defaultValue;
}
CursorFactory cursors = transaction.cursors();
try ( NodeCursor nodes = cursors.allocateNodeCursor();
PropertyCursor properties = cursors.allocatePropertyCursor()
)
{
int propertyKey = transaction.token().propertyKey( key );
if ( propertyKey == KeyReadOperations.NO_SUCH_PROPERTY_KEY )
{
return defaultValue;
}

transaction.dataRead().singleNode( nodeId, nodes );
if ( !nodes.next() )
{
Expand Down Expand Up @@ -420,25 +418,50 @@ public Map<String,Object> getProperties( String... keys )
return Collections.emptyMap();
}

KernelTransaction transaction = actions.kernelTransaction();
CursorFactory cursors = transaction.cursors();
int itemsToReturn = keys.length;
Map<String,Object> properties = new HashMap<>( itemsToReturn );
Token token = transaction.token();

try ( Statement statement = actions.statement() )
try ( Statement ignore = actions.statement() )
{
try ( Cursor<NodeItem> node = statement.readOperations().nodeCursorById( nodeId ) )
KernelTransaction transaction = actions.kernelTransaction();
CursorFactory cursors = transaction.cursors();
int itemsToReturn = keys.length;
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++ )
{
try ( Cursor<PropertyItem> propertyCursor = statement.readOperations().nodeGetProperties( node.get() ) )
propertyIds[i] = token.propertyKey( keys[i] );
}
try ( NodeCursor nodes = cursors.allocateNodeCursor();
PropertyCursor propertyCursor = cursors.allocatePropertyCursor() )
{
transaction.dataRead().singleNode( nodeId, nodes );
if ( !nodes.next() )
{
return PropertyContainerProxyHelper.getProperties( statement, propertyCursor, keys );
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 )
{
int currentKey = propertyCursor.propertyKey();
if ( propertyId == currentKey )
{
properties.put( token.propertyKeyGetName( currentKey ),
propertyCursor.propertyValue().asObjectCopy() );
}
}

}
}
catch ( EntityNotFoundException e )
catch ( PropertyKeyIdNotFoundKernelException e )
{
throw new NotFoundException( "Node not found", e );
throw new IllegalStateException( "Property key retrieved through kernel API should exist.", e );
}
return properties;
}
}

Expand Down
6 changes: 3 additions & 3 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeCursor.java
View file
Expand Up @@ -189,10 +189,10 @@ public long propertiesReference()
//Mark the reference so that property cursor checks both
//tx state as well as disk.
ref = References.setTxStateFlag( propertiesReference );
//stores the node state mapped to the current property
//reference so that property cursor is able to retrieve the state later.
txState.registerProperties( ref, nodeState );
}
//stores the node state mapped to the current property
//reference so that property cursor is able to retrieve the state later.
txState.registerProperties( ref, nodeState );
return ref;
}
}
Expand Down
20 changes: 10 additions & 10 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/PropertyCursor.java
View file
Expand Up @@ -58,7 +58,7 @@ public class PropertyCursor extends PropertyRecord implements org.neo4j.internal
private PageCursor stringPage;
private PageCursor arrayPage;
private PropertyContainerState propertiesState;
private Iterator<StorageProperty> changedProperties;
private Iterator<StorageProperty> txStateChangedProperties;
private StorageProperty txStateValue;
private AssertOpen assertOpen;

Expand Down Expand Up @@ -90,25 +90,25 @@ void init( long reference, Read read, AssertOpen assertOpen )
if ( References.hasTxStateFlag( reference ) ) // both in tx-state and store
{
this.propertiesState = read.txState().getPropertiesState( reference );
this.changedProperties = this.propertiesState.addedAndChangedProperties();
this.txStateChangedProperties = this.propertiesState.addedAndChangedProperties();
this.next = References.clearFlags( reference );
}
else if ( References.hasNodeFlag( reference ) ) // only in tx-state
{
this.propertiesState = read.txState().getNodeState( References.clearFlags( reference ) );
this.changedProperties = this.propertiesState.addedAndChangedProperties();
this.txStateChangedProperties = this.propertiesState.addedAndChangedProperties();
this.next = NO_ID;
}
else if ( References.hasRelationshipFlag( reference ) ) // only in tx-state
{
this.propertiesState = read.txState().getRelationshipState( References.clearFlags( reference ) );
this.changedProperties = this.propertiesState.addedAndChangedProperties();
this.txStateChangedProperties = this.propertiesState.addedAndChangedProperties();
this.next = NO_ID;
}
else
{
this.propertiesState = null;
this.changedProperties = null;
this.txStateChangedProperties = null;
this.txStateValue = null;
this.next = reference;
}
Expand All @@ -117,16 +117,16 @@ else if ( References.hasRelationshipFlag( reference ) ) // only in tx-state
@Override
public boolean next()
{
if ( changedProperties != null )
if ( txStateChangedProperties != null )
{
if ( changedProperties.hasNext() )
if ( txStateChangedProperties.hasNext() )
{
txStateValue = changedProperties.next();
txStateValue = txStateChangedProperties.next();
return true;
}
else
{
changedProperties = null;
txStateChangedProperties = null;
txStateValue = null;
}
}
Expand Down Expand Up @@ -194,7 +194,7 @@ public void close()
arrayPage = null;
}
propertiesState = null;
changedProperties = null;
txStateChangedProperties = null;
txStateValue = null;
read = null;
clear();
Expand Down
3 changes: 1 addition & 2 deletions ...nity/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionTerminationTest.java
View file
Expand Up @@ -344,8 +344,7 @@ private static class TestKernelTransaction extends KernelTransactionImplementati
TransactionTracer.NULL,
LockTracer.NONE, PageCursorTracerSupplier.NULL,
mock( StorageEngine.class, RETURNS_MOCKS ), new CanWrite(), mock( Cursors.class ),
AutoIndexing.UNSUPPORTED, mock(
ExplicitIndexStore.class ) );
AutoIndexing.UNSUPPORTED, mock( ExplicitIndexStore.class ) );

this.monitor = monitor;
}
Expand Down

0 comments on commit e56ab3a

Please sign in to comment.