Skip to content

Commit

Permalink
Removed leak of values into public API
Browse files Browse the repository at this point in the history 
We messed up and manage to leak `Value` into a public API. Let's remove
as quick as possible.
  • Loading branch information
@pontusmelke
pontusmelke committed Oct 30, 2017
1 parent 670446a commit 46d4e30
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 32 deletions.
5 changes: 2 additions & 3 deletions community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/BatchInserter.java
View file
Expand Up @@ -25,7 +25,6 @@
import org.neo4j.graphdb.RelationshipType;
import org.neo4j.graphdb.schema.ConstraintCreator;
import org.neo4j.graphdb.schema.IndexCreator;
import org.neo4j.values.storable.Value;

/**
* The batch inserter drops support for transactions and concurrency in favor
Expand Down Expand Up @@ -162,7 +161,7 @@ void setRelationshipProperty( long relationship,
*
* @return map containing this node's properties.
*/
Map<String,Value> getNodeProperties( long nodeId );
Map<String,Object> getNodeProperties( long nodeId );

/**
* Returns an iterable over all the relationship ids connected to node with
Expand Down Expand Up @@ -226,7 +225,7 @@ void setRelationshipProperties( long rel,
* @param relId the id of the relationship.
* @return map containing the relationship's properties.
*/
Map<String,Value> getRelationshipProperties( long relId );
Map<String,Object> getRelationshipProperties( long relId );

/**
* Removes the property named {@code property} from the node with id
Expand Down
10 changes: 5 additions & 5 deletions community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java
View file
Expand Up @@ -921,7 +921,7 @@ public boolean nodeExists( long nodeId )
}

@Override
public Map<String,Value> getNodeProperties( long nodeId )
public Map<String,Object> getNodeProperties( long nodeId )
{
NodeRecord record = getNodeRecord( nodeId ).forReadingData();
if ( record.getNextProp() != Record.NO_NEXT_PROPERTY.intValue() )
Expand Down Expand Up @@ -969,7 +969,7 @@ public BatchRelationship getRelationshipById( long relId )
}

@Override
public Map<String,Value> getRelationshipProperties( long relId )
public Map<String,Object> getRelationshipProperties( long relId )
{
RelationshipRecord record = recordAccess.getRelRecords().getOrLoad( relId, null ).forChangingData();
if ( record.getNextProp() != Record.NO_NEXT_PROPERTY.intValue() )
Expand Down Expand Up @@ -1026,14 +1026,14 @@ public String toString()
return "EmbeddedBatchInserter[" + storeDir + "]";
}

private Map<String, Value> getPropertyChain( long nextProp )
private Map<String, Object> getPropertyChain( long nextProp )
{
final Map<String, Value> map = new HashMap<>();
final Map<String, Object> map = new HashMap<>();
propertyTraverser.getPropertyChain( nextProp, recordAccess.getPropertyRecords(), propBlock ->
{
String key = propertyKeyTokens.byId( propBlock.getKeyIndexId() ).name();
Value propertyValue = propBlock.newPropertyValue( propertyStore );
map.put( key, propertyValue );
map.put( key, propertyValue.asObject() );
} );
return map;
}
Expand Down
5 changes: 2 additions & 3 deletions ...l/src/main/java/org/neo4j/unsafe/batchinsert/internal/FileSystemClosingBatchInserter.java
View file
Expand Up @@ -31,7 +31,6 @@
import org.neo4j.kernel.impl.index.IndexConfigStore;
import org.neo4j.unsafe.batchinsert.BatchInserter;
import org.neo4j.unsafe.batchinsert.BatchRelationship;
import org.neo4j.values.storable.Value;

public class FileSystemClosingBatchInserter implements BatchInserter, IndexConfigStoreProvider
{
Expand Down Expand Up @@ -114,7 +113,7 @@ public void setRelationshipProperty( long relationship, String propertyName, Obj
}

@Override
public Map<String,Value> getNodeProperties( long nodeId )
public Map<String,Object> getNodeProperties( long nodeId )
{
return delegate.getNodeProperties( nodeId );
}
Expand Down Expand Up @@ -150,7 +149,7 @@ public void setRelationshipProperties( long rel, Map<String,Object> properties )
}

@Override
public Map<String,Value> getRelationshipProperties( long relId )
public Map<String,Object> getRelationshipProperties( long relId )
{
return delegate.getRelationshipProperties( relId );
}
Expand Down
37 changes: 16 additions & 21 deletions community/kernel/src/test/java/org/neo4j/unsafe/batchinsert/internal/BatchInsertTest.java
View file
Expand Up @@ -283,7 +283,7 @@ public void shouldUpdateStringArrayPropertiesOnNodesUsingBatchInserter1() throws
inserter.setNodeProperty( id2, "array", array2 );

// Then
assertThat( inserter.getNodeProperties( id1 ).get( "array" ), equalTo( Values.of( array1 ) ) );
assertThat( inserter.getNodeProperties( id1 ).get( "array" ), equalTo( array1 ) );
}

@Test
Expand Down Expand Up @@ -1058,8 +1058,8 @@ public void propertiesCanBeReSetUsingBatchInserter() throws Exception
batchInserter.setNodeProperty( nodeId, "additional", "something" );

// THEN there should be no problems doing so
assertEquals( Values.of( "YetAnotherOne" ), batchInserter.getNodeProperties( nodeId ).get( "name" ) );
assertEquals( Values.of( "something" ), batchInserter.getNodeProperties( nodeId ).get( "additional" ) );
assertEquals( "YetAnotherOne", batchInserter.getNodeProperties( nodeId ).get( "name" ) );
assertEquals("something", batchInserter.getNodeProperties( nodeId ).get( "additional" ) );
}

/**
Expand Down Expand Up @@ -1119,7 +1119,7 @@ public void propertiesCanBeReSetUsingBatchInserter2() throws Exception
batchInserter.setNodeProperty( id, "test", "small test" );

// THEN
assertEquals( Values.of( "small test" ), batchInserter.getNodeProperties( id ).get( "test" ) );
assertEquals( "small test", batchInserter.getNodeProperties( id ).get( "test" ) );
}

@Test
Expand All @@ -1138,7 +1138,7 @@ public void replaceWithBiggerPropertySpillsOverIntoNewPropertyRecord() throws Ex
batchInserter.setNodeProperty( id, "count", "something" );

// THEN
assertEquals( Values.of( "something" ), batchInserter.getNodeProperties( id ).get( "count" ) );
assertEquals( "something", batchInserter.getNodeProperties( id ).get( "count" ) );
}

@Test
Expand Down Expand Up @@ -1475,8 +1475,15 @@ private long dbWithIndexAndSingleIndexedNode() throws Exception
private void setAndGet( BatchInserter inserter, Object value )
{
long nodeId = inserter.createNode( map( "key", value ) );
Value readValue = inserter.getNodeProperties( nodeId ).get( "key" );
assertEquals( Values.of( value ), readValue );
Object readValue = inserter.getNodeProperties( nodeId ).get( "key" );
if ( readValue.getClass().isArray() )
{
assertTrue( Arrays.equals( (int[])value, (int[])readValue ) );
}
else
{
assertEquals( value, readValue );
}
}

private int[] intArray()
Expand Down Expand Up @@ -1536,23 +1543,11 @@ private Pair<Label[],Set<String>> manyLabels( int count )

private Map<String,Object> getNodeProperties( BatchInserter inserter, long nodeId )
{
Map<String,Value> asValues = inserter.getNodeProperties( nodeId );
return valueMapToObjectMap( asValues );
return inserter.getNodeProperties( nodeId );
}

private Map<String,Object> getRelationshipProperties( BatchInserter inserter, long relId )
{
Map<String,Value> asValues = inserter.getRelationshipProperties( relId );
return valueMapToObjectMap( asValues );
}

private Map<String,Object> valueMapToObjectMap( Map<String,Value> asValues )
{
Map<String,Object> asObjects = new HashMap<>();
for ( Map.Entry<String,Value> entry : asValues.entrySet() )
{
asObjects.put( entry.getKey(), entry.getValue().asObjectCopy() );
}
return asObjects;
return inserter.getRelationshipProperties( relId );
}
}

0 comments on commit 46d4e30

Please sign in to comment.