Skip to content

Commit

Permalink
GenericKeyStateTest, copyFrom, assertCorrectType, compareValueTo, min…
Browse files Browse the repository at this point in the history 
…imalSplitter

Fix issue with minimalSplitter where it did not properly handle arrays or
strings of size 0.

Fix issue where assertCorrectType did not check for spatial arrays.
  • Loading branch information
@burqen
burqen committed Aug 30, 2018
1 parent 644ac56 commit 39e1901
Show file tree
Hide file tree
Showing 4 changed files with 282 additions and 52 deletions.
20 changes: 13 additions & 7 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericKeyState.java
View file
Expand Up @@ -279,7 +279,7 @@ private void copyByteArrayFromIfExists( GenericKeyState key, int targetLength )

static Value assertCorrectType( Value value )
{
if ( Values.isGeometryValue( value ) )
if ( Values.isGeometryValue( value ) || Values.isGeometryArray( value ) )
{
throw new IllegalArgumentException( "Unsupported value " + value );
}
Expand Down Expand Up @@ -391,17 +391,23 @@ public static void minimalSplitter( GenericKeyState left, GenericKeyState right,
private static void minimalSplitterArray( GenericKeyState left, GenericKeyState right, GenericKeyState into,
ArrayElementComparator comparator, ArrayCopier arrayCopier )
{
int index = 0;
int lastEqualIndex = -1;
if ( left.type == right.type )
{
int compare = 0;
int length = min( left.arrayLength, right.arrayLength );
for ( ; compare == 0 && index < length; index++ )
int maxLength = min( left.arrayLength, right.arrayLength );
for ( int index = 0; index < maxLength; index++ )
{
compare = comparator.compare( left, right, index );
if ( comparator.compare( left, right, index ) != 0 )
{
break;
}
lastEqualIndex++;
}
}
arrayCopier.copyArray( into, right, index );
// Convert from last equal index to first index to differ +1
// Convert from index to length +1
// Total +2
arrayCopier.copyArray( into, right, lastEqualIndex + 2 );
}

private static void minimalSplitterText( GenericKeyState left, GenericKeyState right, GenericKeyState into )
Expand Down
17 changes: 10 additions & 7 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/StringLayout.java
View file
Expand Up @@ -22,6 +22,7 @@
import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.io.pagecache.PageCursor;

import static java.lang.Integer.min;
import static java.lang.String.format;
import static org.neo4j.kernel.impl.index.schema.StringIndexKey.ENTITY_ID_SIZE;

Expand Down Expand Up @@ -92,18 +93,20 @@ public void minimalSplitter( StringIndexKey left, StringIndexKey right, StringIn

static int firstPosToDiffer( byte[] leftBytes, int leftLength, byte[] rightBytes, int rightLength )
{
int maxLength = Math.min( leftLength, rightLength );
int targetLength = 0;
for ( ; targetLength < maxLength; targetLength++ )
int lastEqualIndex = -1;
int maxLength = min( leftLength, rightLength );
for ( int index = 0; index < maxLength; index++ )
{
if ( leftBytes[targetLength] != rightBytes[targetLength] )
if ( leftBytes[index] != rightBytes[index] )
{
// Convert to length from array index
targetLength++;
break;
}
lastEqualIndex++;
}
return targetLength;
// Convert from last equal index to first index to differ +1
// Convert from index to length +1
// Total +2
return lastEqualIndex + 2;
}

@Override
Expand Down
245 changes: 207 additions & 38 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/GenericKeyStateTest.java
View file
Expand Up @@ -19,66 +19,81 @@
*/
package org.neo4j.kernel.impl.index.schema;

import org.junit.Rule;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;

import org.neo4j.io.pagecache.ByteArrayPageCursor;
import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.test.extension.Inject;
import org.neo4j.test.extension.RandomExtension;
import org.neo4j.test.rule.RandomRule;
import org.neo4j.values.storable.RandomValues;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.Values;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.neo4j.kernel.impl.index.schema.NativeIndexKey.Inclusion.NEUTRAL;

@ExtendWith( RandomExtension.class )
class GenericKeyStateTest
{
@Rule
private static RandomRule random = new RandomRule().withConfiguration( new RandomValues.Configuration()
{
@Override
public int stringMinLength()
{
return 0;
}
@Inject
private static RandomRule random;

@Override
public int stringMaxLength()
{
return 50;
}
private static final PageCursor cursor = ByteArrayPageCursor.wrap( PageCache.PAGE_SIZE );

@Override
public int arrayMinLength()
@BeforeEach
public void setupRandomConfig()
{
random = random.withConfiguration( new RandomValues.Configuration()
{
return 0;
}
@Override
public int stringMinLength()
{
return 0;
}

@Override
public int arrayMaxLength()
{
return 10;
}
});
@Override
public int stringMaxLength()
{
return 50;
}

private static final PageCursor cursor = ByteArrayPageCursor.wrap( PageCache.PAGE_SIZE );
@Override
public int arrayMinLength()
{
return 0;
}

@Override
public int arrayMaxLength()
{
return 10;
}
} );
random.reset();
}

@ParameterizedTest
@MethodSource( "valueGenerators" )
@MethodSource( "validValueGenerators" )
void readWhatIsWritten( ValueGenerator valueGenerator )
{
// Given
GenericKeyState writeState = new GenericKeyState();
Value value = valueGenerator.next();
System.out.println( value );
int offset = cursor.getOffset();

// When
writeState.writeValue( value, NativeIndexKey.Inclusion.NEUTRAL );
writeState.writeValue( value, NEUTRAL );
writeState.put( cursor );

// Then
Expand All @@ -88,13 +103,144 @@ void readWhatIsWritten( ValueGenerator valueGenerator )
assertTrue( readState.read( cursor, size ), "failed to read" );
assertEquals( 0, readState.compareValueTo( writeState ), "key states are not equal" );
Value readValue = readState.asValue();
System.out.println( "readValue = " + readValue );
assertEquals( value, readValue, "deserialized valued are not equal" );
}

private static Stream<ValueGenerator> valueGenerators()
@ParameterizedTest
@MethodSource( "validValueGenerators" )
void copyShouldCopy( ValueGenerator valueGenerator )
{
// Given
GenericKeyState from = new GenericKeyState();
Value value = valueGenerator.next();
from.writeValue( value, NEUTRAL );
GenericKeyState to = genericKeyStateWithSomePreviousState( valueGenerator );

// When
to.copyFrom( from );

// Then
assertEquals( 0, from.compareValueTo( to ), "states not equals after copy" );
}

@ParameterizedTest
@MethodSource( "validValueGenerators" )
void assertCorrectTypeMustNotFailForValidTypes( ValueGenerator valueGenerator )
{
// Given
Value value = valueGenerator.next();

// When
GenericKeyState.assertCorrectType( value );

// Then
// should not fail
}

@ParameterizedTest
@MethodSource( "invalidValueGenerators" )
void assertCorrectTypeMustFailForInvalidTypes( ValueGenerator valueGenerator )
{
// Given
Value invalidValue = valueGenerator.next();

// When
assertThrows( IllegalArgumentException.class, () -> GenericKeyState.assertCorrectType( invalidValue ),
"did not throw on invalid value " + invalidValue );
}

@ParameterizedTest
@MethodSource( "validValueGenerators" )
void compareToMustAlignWithValuesCompareTo( ValueGenerator valueGenerator )
{
// Given
random.reset();
List<Value> values = new ArrayList<>();
List<GenericKeyState> states = new ArrayList<>();
for ( int i = 0; i < 10; i++ )
{
Value value = valueGenerator.next();
values.add( value );
GenericKeyState state = new GenericKeyState();
state.writeValue( value, NEUTRAL );
states.add( state );
}

// When
values.sort( Values.COMPARATOR );
states.sort( GenericKeyState::compareValueTo );

// Then
for ( int i = 0; i < values.size(); i++ )
{
assertEquals( values.get( i ), states.get( i ).asValue(), "sort order was different" );
}
}

@ParameterizedTest
@MethodSource( "validValueGenerators" )
void mustProduceValidMinimalSplitters( ValueGenerator valueGenerator )
{
// Given
Value value1 = valueGenerator.next();
Value value2;
do
{
value2 = valueGenerator.next();
}
while ( Values.COMPARATOR.compare( value1, value2 ) == 0 );

// When
Value left = pickSmaller( value1, value2 );
Value right = left == value1 ? value2 : value1;

// Then
assertValidMinimalSplitter( left, right );
}

// todo size
// todo initValueAsLowest / Highest

private Value pickSmaller( Value value1, Value value2 )
{
return Values.COMPARATOR.compare( value1, value2 ) < 0 ? value1 : value2;
}

private void assertValidMinimalSplitter( Value left, Value right )
{
GenericKeyState leftState = new GenericKeyState();
leftState.writeValue( left, NEUTRAL );
GenericKeyState rightState = new GenericKeyState();
rightState.writeValue( right, NEUTRAL );

GenericKeyState minimalSplitter = new GenericKeyState();
GenericKeyState.minimalSplitter( leftState, rightState, minimalSplitter );

assertTrue( leftState.compareValueTo( minimalSplitter ) < 0,
"left state not less than minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter );
assertTrue( rightState.compareValueTo( minimalSplitter ) >= 0,
"right state not less than minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter );
}

private static Value nextValidValue()
{
Value value;
do
{
value = random.randomValues().nextValue();
}
while ( isInvalid( value ) );
return value;
}

private static boolean isInvalid( Value value )
{
// todo update when spatial is supported
return Values.isGeometryValue( value ) || Values.isGeometryArray( value );
}

private static Stream<ValueGenerator> validValueGenerators()
{
return Stream.of(
() -> random.randomValues().nextDateTimeValue(),
() -> random.randomValues().nextLocalDateTimeValue(),
Expand Down Expand Up @@ -123,21 +269,44 @@ private static Stream<ValueGenerator> valueGenerators()
() -> random.randomValues().nextIntArray(),
() -> random.randomValues().nextLongArray(),
() -> random.randomValues().nextFloatArray(),
() -> random.randomValues().nextDoubleArray()
() -> random.randomValues().nextDoubleArray(),
// todo GEOMETRY_ARRAY
GenericKeyStateTest::nextValidValue // and a random
);
}

private static Stream<ValueGenerator> invalidValueGenerators()
{
return Stream.of(
() -> random.randomValues().nextGeographicPoint(),
() -> random.randomValues().nextGeographic3DPoint(),
() -> random.randomValues().nextCartesianPoint(),
() -> random.randomValues().nextCartesian3DPoint(),
() -> random.randomValues().nextGeographicPointArray(),
() -> random.randomValues().nextGeographic3DPointArray(),
() -> random.randomValues().nextCartesianPointArray(),
() -> random.randomValues().nextCartesian3DPointArray()
);
// todo Remove when GEOMERTY is supported
}

private GenericKeyState genericKeyStateWithSomePreviousState( ValueGenerator valueGenerator )
{
GenericKeyState to = new GenericKeyState();
if ( random.nextBoolean() )
{
// Previous value
NativeIndexKey.Inclusion inclusion = random.among( NativeIndexKey.Inclusion.values() );
Value value = valueGenerator.next();
to.writeValue( value, inclusion );
}
// No previous state
return to;
}

@FunctionalInterface
private interface ValueGenerator
{
Value next();
}

// todo copyFrom
// todo assertCorrectType
// todo compareValueTo
// todo minimalSplitter
// todo size
// todo initValueAsLowest / Highest
}

0 comments on commit 39e1901

Please sign in to comment.