Skip to content

Commit

Permalink
Polished by PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
fickludd committed Mar 1, 2017
1 parent fe58634 commit b804b64
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 77 deletions.
Expand Up @@ -19,9 +19,6 @@
*/ */
package org.neo4j.kernel.api.exceptions.index; package org.neo4j.kernel.api.exceptions.index;


import java.util.Arrays;

import org.neo4j.helpers.Strings;
import org.neo4j.kernel.api.TokenNameLookup; import org.neo4j.kernel.api.TokenNameLookup;
import org.neo4j.kernel.api.exceptions.KernelException; import org.neo4j.kernel.api.exceptions.KernelException;
import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor; import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor;
Expand All @@ -30,7 +27,6 @@
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor; import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor;


import static java.lang.String.format; import static java.lang.String.format;
import static java.lang.String.valueOf;
import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_NODE; import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_NODE;


/** /**
Expand All @@ -42,10 +38,15 @@ public class IndexEntryConflictException extends Exception
private final long addedNodeId; private final long addedNodeId;
private final long existingNodeId; private final long existingNodeId;


public IndexEntryConflictException( long existingNodeId, long addedNodeId, Object propertyValue )
{
this( existingNodeId, addedNodeId, OrderedPropertyValues.of( propertyValue ) );
}

public IndexEntryConflictException( long existingNodeId, long addedNodeId, OrderedPropertyValues propertyValues ) public IndexEntryConflictException( long existingNodeId, long addedNodeId, OrderedPropertyValues propertyValues )
{ {
super( format( "Both node %d and node %d share the property value %s", super( format( "Both node %d and node %d share the property value %s",
existingNodeId, addedNodeId, quote( propertyValues ) ) ); existingNodeId, addedNodeId, propertyValues ) );
this.existingNodeId = existingNodeId; this.existingNodeId = existingNodeId;
this.addedNodeId = addedNodeId; this.addedNodeId = addedNodeId;
this.propertyValues = propertyValues; this.propertyValues = propertyValues;
Expand All @@ -65,7 +66,7 @@ public RuntimeException notAllowed( NewIndexDescriptor descriptor )


public String evidenceMessage( TokenNameLookup tokenNameLookup, LabelSchemaDescriptor schema ) public String evidenceMessage( TokenNameLookup tokenNameLookup, LabelSchemaDescriptor schema )
{ {
assert schema.getPropertyIds().length == propertyValues.values().length; assert schema.getPropertyIds().length == propertyValues.size();


String labelName = tokenNameLookup.labelGetName( schema.getLabelId() ); String labelName = tokenNameLookup.labelGetName( schema.getLabelId() );
if ( addedNodeId == NO_SUCH_NODE ) if ( addedNodeId == NO_SUCH_NODE )
Expand All @@ -87,8 +88,7 @@ public OrderedPropertyValues getPropertyValues()


public Object getSinglePropertyValue() public Object getSinglePropertyValue()
{ {
assert propertyValues.values().length == 1; return propertyValues.getSinglePropertyValue();
return propertyValues.values()[0];
} }


public long getAddedNodeId() public long getAddedNodeId()
Expand Down Expand Up @@ -133,7 +133,7 @@ public int hashCode()
public String toString() public String toString()
{ {
return "IndexEntryConflictException{" + return "IndexEntryConflictException{" +
"propertyValues=" + Strings.prettyPrint( propertyValues.values() ) + "propertyValues=" + propertyValues +
", addedNodeId=" + addedNodeId + ", addedNodeId=" + addedNodeId +
", existingNodeId=" + existingNodeId + ", existingNodeId=" + existingNodeId +
'}'; '}';
Expand All @@ -150,61 +150,8 @@ private String propertyString( TokenNameLookup tokenNameLookup, int[] propertyId
sb.append( '`' ); sb.append( '`' );
sb.append( tokenNameLookup.propertyKeyGetName( propertyIds[i] ) ); sb.append( tokenNameLookup.propertyKeyGetName( propertyIds[i] ) );
sb.append( "` = " ); sb.append( "` = " );
sb.append( quote( propertyValues.values()[i] ) ); sb.append( OrderedPropertyValues.quote( propertyValues.values()[i] ) );
}
return sb.toString();
}

private static String quote( OrderedPropertyValues propertyValues )
{
StringBuilder sb = new StringBuilder();
String sep = "( ";
for ( Object value : propertyValues.values() )
{
sb.append( sep );
sep = ", ";
sb.append( quote( value ) );
} }
sb.append( " )" );
return sb.toString(); return sb.toString();
} }

private static String quote( Object propertyValue )
{
if ( propertyValue instanceof String )
{
return format( "'%s'", propertyValue );
}
else if ( propertyValue.getClass().isArray() )
{
Class<?> type = propertyValue.getClass().getComponentType();
if ( type == Boolean.TYPE )
{
return Arrays.toString( (boolean[]) propertyValue );
} else if ( type == Byte.TYPE )
{
return Arrays.toString( (byte[]) propertyValue );
} else if ( type == Short.TYPE )
{
return Arrays.toString( (short[]) propertyValue );
} else if ( type == Character.TYPE )
{
return Arrays.toString( (char[]) propertyValue );
} else if ( type == Integer.TYPE )
{
return Arrays.toString( (int[]) propertyValue );
} else if ( type == Long.TYPE )
{
return Arrays.toString( (long[]) propertyValue );
} else if ( type == Float.TYPE )
{
return Arrays.toString( (float[]) propertyValue );
} else if ( type == Double.TYPE )
{
return Arrays.toString( (double[]) propertyValue );
}
return Arrays.toString( (Object[]) propertyValue );
}
return valueOf( propertyValue );
}
} }
Expand Up @@ -21,14 +21,22 @@


import java.util.Arrays; import java.util.Arrays;


import static java.lang.String.format;
import static java.lang.String.valueOf;

/** /**
* Holder for n property values, ordered according to a schema descriptor property id order * Holder for n property values, ordered according to a schema descriptor property id order
*/ */
public class OrderedPropertyValues public class OrderedPropertyValues
{ {
private final Object[] values; private final Object[] values;


public OrderedPropertyValues( Object... values ) public static OrderedPropertyValues of( Object... values )
{
return new OrderedPropertyValues( values );
}

private OrderedPropertyValues( Object[] values )
{ {
this.values = values; this.values = values;
} }
Expand Down Expand Up @@ -60,4 +68,69 @@ public int hashCode()
{ {
return Arrays.deepHashCode( values ); return Arrays.deepHashCode( values );
} }

public int size()
{
return values.length;
}

public Object getSinglePropertyValue()
{
assert values.length == 1 : "Assumed single property but had " + values.length;
return values[0];
}

@Override
public String toString()
{
StringBuilder sb = new StringBuilder();
String sep = "( ";
for ( Object value : values )
{
sb.append( sep );
sep = ", ";
sb.append( quote( value ) );
}
sb.append( " )" );
return sb.toString();
}

public static String quote( Object propertyValue )
{
if ( propertyValue instanceof String )
{
return format( "'%s'", propertyValue );
}
else if ( propertyValue.getClass().isArray() )
{
Class<?> type = propertyValue.getClass().getComponentType();
if ( type == Boolean.TYPE )
{
return Arrays.toString( (boolean[]) propertyValue );
} else if ( type == Byte.TYPE )
{
return Arrays.toString( (byte[]) propertyValue );
} else if ( type == Short.TYPE )
{
return Arrays.toString( (short[]) propertyValue );
} else if ( type == Character.TYPE )
{
return Arrays.toString( (char[]) propertyValue );
} else if ( type == Integer.TYPE )
{
return Arrays.toString( (int[]) propertyValue );
} else if ( type == Long.TYPE )
{
return Arrays.toString( (long[]) propertyValue );
} else if ( type == Float.TYPE )
{
return Arrays.toString( (float[]) propertyValue );
} else if ( type == Double.TYPE )
{
return Arrays.toString( (double[]) propertyValue );
}
return Arrays.toString( (Object[]) propertyValue );
}
return valueOf( propertyValue );
}
} }
Expand Up @@ -180,7 +180,7 @@ private void validateNoExistingNodeWithLabelAndProperty(
{ {
throw new UniquePropertyValueValidationException( constraint, throw new UniquePropertyValueValidationException( constraint,
ConstraintValidationException.Phase.VALIDATION, ConstraintValidationException.Phase.VALIDATION,
new IndexEntryConflictException( existing, NO_SUCH_NODE, new OrderedPropertyValues( value ) ) ); new IndexEntryConflictException( existing, NO_SUCH_NODE, value ) );
} }
} }
catch ( IndexNotFoundKernelException | IndexBrokenKernelException | IndexNotApplicableKernelException e ) catch ( IndexNotFoundKernelException | IndexBrokenKernelException | IndexNotApplicableKernelException e )
Expand Down
Expand Up @@ -38,8 +38,7 @@ public class IndexEntryConflictExceptionTest
public void shouldMakeEntryConflicts() public void shouldMakeEntryConflicts()
{ {
LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2 ); LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2 );
OrderedPropertyValues values = new OrderedPropertyValues( "hi" ); IndexEntryConflictException e = new IndexEntryConflictException( 0L, 1L, "hi" );
IndexEntryConflictException e = new IndexEntryConflictException( 0L, 1L, values );


assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ), assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ),
equalTo( "Both Node(0) and Node(1) have the label `label[1]` and property `property[2]` = 'hi'" ) ); equalTo( "Both Node(0) and Node(1) have the label `label[1]` and property `property[2]` = 'hi'" ) );
Expand All @@ -49,8 +48,7 @@ public void shouldMakeEntryConflicts()
public void shouldMakeEntryConflictsForOneNode() public void shouldMakeEntryConflictsForOneNode()
{ {
LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2 ); LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2 );
OrderedPropertyValues values = new OrderedPropertyValues( "hi" ); IndexEntryConflictException e = new IndexEntryConflictException( 0L, StatementConstants.NO_SUCH_NODE, "hi" );
IndexEntryConflictException e = new IndexEntryConflictException( 0L, StatementConstants.NO_SUCH_NODE, values );


assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ), assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ),
equalTo( "Node(0) already exists with label `label[1]` and property `property[2]` = 'hi'" ) ); equalTo( "Node(0) already exists with label `label[1]` and property `property[2]` = 'hi'" ) );
Expand All @@ -60,7 +58,7 @@ public void shouldMakeEntryConflictsForOneNode()
public void shouldMakeCompositeEntryConflicts() public void shouldMakeCompositeEntryConflicts()
{ {
LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2, 3, 4 ); LabelSchemaDescriptor schema = SchemaDescriptorFactory.forLabel( labelId, 2, 3, 4 );
OrderedPropertyValues values = new OrderedPropertyValues( true, "hi", new long[]{6L, 4L} ); OrderedPropertyValues values = OrderedPropertyValues.of( true, "hi", new long[]{6L, 4L} );
IndexEntryConflictException e = new IndexEntryConflictException( 0L, 1L, values ); IndexEntryConflictException e = new IndexEntryConflictException( 0L, 1L, values );


assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ), assertThat( e.evidenceMessage( SchemaUtil.idTokenNameLookup, schema ),
Expand Down
Expand Up @@ -81,7 +81,7 @@ public void shouldProvidePopulatorThatEnforcesUniqueConstraints() throws Excepti
catch ( IndexEntryConflictException conflict ) catch ( IndexEntryConflictException conflict )
{ {
assertEquals( nodeId1, conflict.getExistingNodeId() ); assertEquals( nodeId1, conflict.getExistingNodeId() );
assertEquals( new OrderedPropertyValues( value ), conflict.getPropertyValues() ); assertEquals( OrderedPropertyValues.of( value ), conflict.getPropertyValues() );
assertEquals( nodeId2, conflict.getAddedNodeId() ); assertEquals( nodeId2, conflict.getAddedNodeId() );
} }
} }
Expand Down
Expand Up @@ -119,7 +119,7 @@ public void shouldDropIndexIfPopulationFails() throws Exception
.thenReturn( 2468L ); .thenReturn( 2468L );
IndexProxy indexProxy = mock( IndexProxy.class ); IndexProxy indexProxy = mock( IndexProxy.class );
when( indexingService.getIndexProxy( 2468L ) ).thenReturn( indexProxy ); when( indexingService.getIndexProxy( 2468L ) ).thenReturn( indexProxy );
IndexEntryConflictException cause = new IndexEntryConflictException( 2, 1, new OrderedPropertyValues( "a" ) ); IndexEntryConflictException cause = new IndexEntryConflictException( 2, 1, "a" );
doThrow( new IndexPopulationFailedKernelException( SchemaBoundary.map( descriptor ), "some index", cause ) ) doThrow( new IndexPopulationFailedKernelException( SchemaBoundary.map( descriptor ), "some index", cause ) )
.when( indexProxy ).awaitStoreScanCompleted(); .when( indexProxy ).awaitStoreScanCompleted();
PropertyAccessor propertyAccessor = mock( PropertyAccessor.class ); PropertyAccessor propertyAccessor = mock( PropertyAccessor.class );
Expand Down
Expand Up @@ -119,8 +119,7 @@ public void visitEntry( Object key, Set<Long> nodeIds ) throws Exception
if ( entries.containsKey( value ) ) if ( entries.containsKey( value ) )
{ {
long existingNodeId = entries.get( value ); long existingNodeId = entries.get( value );
throw new IndexEntryConflictException( existingNodeId, nodeId, throw new IndexEntryConflictException( existingNodeId, nodeId, value );
new OrderedPropertyValues( value ) );
} }
entries.put( value, nodeId ); entries.put( value, nodeId );
} }
Expand Down
Expand Up @@ -1381,7 +1381,7 @@ public void uniquenessConstraintShouldBeCheckedOnBatchInserterShutdownAndFailIfV
catch ( RuntimeException ex ) catch ( RuntimeException ex )
{ {
// good // good
assertEquals( new IndexEntryConflictException( 0, 1, new OrderedPropertyValues( value ) ), ex.getCause() ); assertEquals( new IndexEntryConflictException( 0, 1, value ), ex.getCause() );
} }
} }


Expand Down
Expand Up @@ -94,8 +94,7 @@ private void doCollect( int doc ) throws IOException, KernelException, IndexEntr
} }
else if ( property.valueEquals( value ) ) else if ( property.valueEquals( value ) )
{ {
throw new IndexEntryConflictException( current.nodeId[i], nodeId, throw new IndexEntryConflictException( current.nodeId[i], nodeId, value );
new OrderedPropertyValues( value ) );
} }
} }
current = current.next; current = current.next;
Expand Down

0 comments on commit b804b64

Please sign in to comment.