diff --git a/community/csv/pom.xml b/community/csv/pom.xml index 6d94afc7e7710..730f34772cf58 100644 --- a/community/csv/pom.xml +++ b/community/csv/pom.xml @@ -64,6 +64,11 @@ the relevant Commercial Agreement. neo4j-collections ${project.version} + + org.neo4j + neo4j-values + ${project.version} + diff --git a/community/csv/src/main/java/org/neo4j/csv/reader/Extractors.java b/community/csv/src/main/java/org/neo4j/csv/reader/Extractors.java index 754c1a148e44c..e9e237ec3da09 100644 --- a/community/csv/src/main/java/org/neo4j/csv/reader/Extractors.java +++ b/community/csv/src/main/java/org/neo4j/csv/reader/Extractors.java @@ -20,9 +20,14 @@ package org.neo4j.csv.reader; import java.lang.reflect.Field; +import java.nio.CharBuffer; import java.util.HashMap; import java.util.Map; +import org.neo4j.values.AnyValue; +import org.neo4j.values.storable.PointValue; +import org.neo4j.values.storable.Values; + import static java.lang.Character.isWhitespace; import static java.lang.reflect.Modifier.isStatic; import static org.neo4j.collection.primitive.PrimitiveLongCollections.EMPTY_LONG_ARRAY; @@ -85,6 +90,7 @@ public class Extractors private final Extractor longArray; private final Extractor floatArray; private final Extractor doubleArray; + private final PointExtractor point_; public Extractors( char arrayDelimiter ) { @@ -135,6 +141,7 @@ public Extractors( char arrayDelimiter, boolean emptyStringsAsNull, boolean trim add( longArray = new LongArrayExtractor( arrayDelimiter ) ); add( floatArray = new FloatArrayExtractor( arrayDelimiter ) ); add( doubleArray = new DoubleArrayExtractor( arrayDelimiter ) ); + add( point_ = new PointExtractor() ); } catch ( IllegalAccessException e ) { @@ -242,6 +249,11 @@ public Extractor doubleArray() return doubleArray; } + public PointExtractor point_() + { + return point_; + } + private abstract static class AbstractExtractor implements Extractor { private final String name; @@ -911,6 +923,35 @@ protected void extract0( char[] data, int offset, int length ) } } + public static class PointExtractor extends AbstractSingleValueExtractor + { + private AnyValue value; + + PointExtractor() + { + super( "Point" ); + } + + @Override + protected void clear() + { + value = Values.NO_VALUE; + } + + @Override + protected boolean extract0( char[] data, int offset, int length ) + { + value = PointValue.parse( CharBuffer.wrap( data, offset, length ) ); + return true; + } + + @Override + public AnyValue value() + { + return value; + } + } + private static long extractLong( char[] data, int originalOffset, int fullLength ) { long result = 0; diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala index 62c949eb61865..47de434b4999b 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala +++ b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala @@ -22,12 +22,11 @@ package org.neo4j.cypher.internal.runtime.interpreted.commands.expressions import java.util.function.BiConsumer import org.neo4j.cypher.internal.util.v3_4.CypherTypeException -import org.neo4j.kernel.impl.util.ValueUtils import org.neo4j.cypher.internal.runtime.interpreted.ExecutionContext import org.neo4j.cypher.internal.runtime.interpreted.IsMap import org.neo4j.cypher.internal.runtime.interpreted.pipes.QueryState import org.neo4j.values.AnyValue -import org.neo4j.values.storable.Values +import org.neo4j.values.storable.{PointValue, Values} import org.neo4j.values.virtual.MapValue case class PointFunction(data: Expression) extends NullInNullOutExpression(data) { @@ -35,7 +34,7 @@ case class PointFunction(data: Expression) extends NullInNullOutExpression(data) case IsMap(mapCreator) => val map = mapCreator(state.query) if (containsNull(map)) Values.NO_VALUE - else ValueUtils.pointFromMap(map) + else PointValue.fromMap(map) case x => throw new CypherTypeException(s"Expected a map but got $x") } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java index ab2b11f9ad42d..b09a42de5c75c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java @@ -254,70 +254,6 @@ public static MapValue asMapValue( Map map ) return map( mapValues( map ) ); } - public static PointValue pointFromMap( MapValue map ) - { - CoordinateReferenceSystem crs; - double[] coordinates; - if ( map.containsKey( "crs" ) ) - { - TextValue crsName = (TextValue) map.get( "crs" ); - crs = CoordinateReferenceSystem.byName( crsName.stringValue() ); - if ( crs == null ) - { - throw new IllegalArgumentException( "Unknown coordinate reference system: " + crsName.stringValue() ); - } - } - else - { - crs = null; - } - if ( map.containsKey( "x" ) && map.containsKey( "y" ) ) - { - double x = ((NumberValue) map.get( "x" )).doubleValue(); - double y = ((NumberValue) map.get( "y" )).doubleValue(); - coordinates = map.containsKey( "z" ) ? new double[]{x, y, ((NumberValue) map.get( "z" )).doubleValue()} : new double[]{x, y}; - if ( crs == null ) - { - crs = coordinates.length == 3 ? CoordinateReferenceSystem.Cartesian_3D : CoordinateReferenceSystem.Cartesian; - } - } - else if ( map.containsKey( "latitude" ) && map.containsKey( "longitude" ) ) - { - double x = ((NumberValue) map.get( "longitude" )).doubleValue(); - double y = ((NumberValue) map.get( "latitude" )).doubleValue(); - // TODO Consider supporting key 'height' - if ( map.containsKey( "z" ) ) - { - coordinates = new double[]{x, y, ((NumberValue) map.get( "z" )).doubleValue()}; - } - else if ( map.containsKey( "height" ) ) - { - coordinates = new double[]{x, y, ((NumberValue) map.get( "height" )).doubleValue()}; - } - else - { - coordinates = new double[]{x, y}; - } - if ( crs == null ) - { - crs = coordinates.length == 3 ? CoordinateReferenceSystem.WGS84_3D : CoordinateReferenceSystem.WGS84; - } - if ( !crs.isGeographic() ) - { - throw new IllegalArgumentException( "Geographic points does not support coordinate reference system: " + crs ); - } - } - else - { - throw new IllegalArgumentException( "A point must contain either 'x' and 'y' or 'latitude' and 'longitude'" ); - } - if ( crs.getDimension() != coordinates.length ) - { - throw new IllegalArgumentException( "Cannot create " + crs.getDimension() + "D point with " + coordinates.length + " coordinates" ); - } - return Values.pointValue( crs, coordinates ); - } - private static Map mapValues( Map map ) { HashMap newMap = new HashMap<>( map.size() ); @@ -338,4 +274,18 @@ public static RelationshipValue fromRelationshipProxy( Relationship relationship { return new RelationshipProxyWrappingValue( relationship ); } + + /** + * Creates a {@link Value} from the given object, or if it is already a Value it is returned as it is. + *

+ * This is different from {@link Values#of} which explicitly fails if given a Value. + */ + public static Value asValue( Object value ) + { + if ( value instanceof Value ) + { + return (Value) value; + } + return Values.of( value ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java b/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java index d46a84f48b422..93dabb93c5ccd 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/batchinsert/internal/BatchInserterImpl.java @@ -148,6 +148,7 @@ import org.neo4j.kernel.impl.transaction.state.RelationshipGroupGetter; import org.neo4j.kernel.impl.transaction.state.storeview.NeoStoreIndexStoreView; import org.neo4j.kernel.impl.util.Dependencies; +import org.neo4j.kernel.impl.util.ValueUtils; import org.neo4j.kernel.internal.EmbeddedGraphDatabase; import org.neo4j.kernel.internal.locker.GlobalStoreLocker; import org.neo4j.kernel.internal.locker.StoreLocker; @@ -161,7 +162,6 @@ import org.neo4j.unsafe.batchinsert.BatchInserter; import org.neo4j.unsafe.batchinsert.BatchRelationship; import org.neo4j.values.storable.Value; -import org.neo4j.values.storable.Values; import static java.lang.Boolean.parseBoolean; import static java.util.Collections.emptyIterator; @@ -408,7 +408,7 @@ private void setPrimitiveProperty( RecordProxy p int propertyKey = getOrCreatePropertyKeyId( propertyName ); RecordAccess propertyRecords = recordAccess.getPropertyRecords(); - propertyCreator.primitiveSetProperty( primitiveRecord, propertyKey, Values.of( propertyValue ), propertyRecords ); + propertyCreator.primitiveSetProperty( primitiveRecord, propertyKey, ValueUtils.asValue( propertyValue ), propertyRecords ); } private void validateIndexCanBeCreated( int labelId, int[] propertyKeyIds ) @@ -773,7 +773,7 @@ private Iterator propertiesIterator( Map properti protected PropertyBlock underlyingObjectToObject( Entry property ) { return propertyCreator.encodePropertyValue( - getOrCreatePropertyKeyId( property.getKey() ), Values.of( property.getValue() ) ); + getOrCreatePropertyKeyId( property.getKey() ), ValueUtils.asValue( property.getValue() ) ); } }; } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/EntityImporter.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/EntityImporter.java index 233d0b337e7d6..63608088dd712 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/EntityImporter.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/EntityImporter.java @@ -29,11 +29,11 @@ import org.neo4j.kernel.impl.store.record.PropertyBlock; import org.neo4j.kernel.impl.store.record.PropertyRecord; import org.neo4j.kernel.impl.store.record.Record; +import org.neo4j.kernel.impl.util.ValueUtils; import org.neo4j.unsafe.impl.batchimport.DataImporter.Monitor; import org.neo4j.unsafe.impl.batchimport.input.InputEntityVisitor; import org.neo4j.unsafe.impl.batchimport.store.BatchingNeoStores; import org.neo4j.unsafe.impl.batchimport.store.BatchingTokenRepository.BatchingPropertyKeyTokenRepository; -import org.neo4j.values.storable.Values; /** * Abstract class containing logic for importing properties for an entity (node/relationship). @@ -119,7 +119,7 @@ private PropertyBlock nextPropertyBlock() private void encodeProperty( PropertyBlock block, int key, Object value ) { - PropertyStore.encodeValue( block, key, Values.of( value ), dynamicStringRecordAllocator, dynamicArrayRecordAllocator, + PropertyStore.encodeValue( block, key, ValueUtils.asValue( value ), dynamicStringRecordAllocator, dynamicArrayRecordAllocator, propertyStore.allowStorePointsAndTemporal() ); } diff --git a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/input/Inputs.java b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/input/Inputs.java index 923bda0fd814a..d408f22ae09a4 100644 --- a/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/input/Inputs.java +++ b/community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/input/Inputs.java @@ -21,12 +21,12 @@ import java.util.function.ToIntFunction; +import org.neo4j.kernel.impl.util.ValueUtils; import org.neo4j.unsafe.impl.batchimport.InputIterable; import org.neo4j.unsafe.impl.batchimport.cache.NumberArrayFactory; import org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMapper; import org.neo4j.unsafe.impl.batchimport.input.Input.Estimates; import org.neo4j.values.storable.Value; -import org.neo4j.values.storable.Values; public class Inputs { @@ -133,7 +133,7 @@ public static int calculatePropertySize( InputEntity entity, ToIntFunction randomNodeData() InputEntity node = new InputEntity(); node.id( UUID.randomUUID().toString(), Group.GLOBAL ); node.property( "name", "Node " + i ); + node.property( "point", "\" { x : 3, y : " + i + ", crs: WGS-84 } \""); node.labels( randomLabels( random ) ); nodes.add( node ); } @@ -227,14 +230,17 @@ private File nodeDataAsFile( List nodeData ) throws IOException try ( Writer writer = fileSystemRule.get().openAsWriter( file, StandardCharsets.UTF_8, false ) ) { // Header - println( writer, "id:ID,name,some-labels:LABEL" ); + println( writer, "id:ID,name,point:Point,some-labels:LABEL" ); // Data for ( InputEntity node : nodeData ) { String csvLabels = csvLabels( node.labels() ); - println( writer, node.id() + "," + node.properties()[1] + "," + + println( writer, node.id() + "," + node.properties()[1] + "," + node.properties()[3] + "," + (csvLabels != null && csvLabels.length() > 0 ? csvLabels : "") ); + // TODO: Remove me + //System.out.println( node.id() + "," + node.properties()[1] + "," + node.properties()[3] + "," + + // (csvLabels != null && csvLabels.length() > 0 ? csvLabels : "") ); } } return file; @@ -283,12 +289,13 @@ private void verifyImportedData( List nodeData, // Build up expected data for the verification below Map expectedNodes = new HashMap<>(); Map expectedNodeNames = new HashMap<>(); + Map> expectedNodeProperties = new HashMap<>(); Map>> expectedRelationships = new AutoCreatingHashMap<>( nested( String.class, nested( String.class, values( AtomicInteger.class ) ) ) ); Map expectedNodeCounts = new AutoCreatingHashMap<>( values( AtomicLong.class ) ); Map>> expectedRelationshipCounts = new AutoCreatingHashMap<>( nested( String.class, nested( String.class, values( AtomicLong.class ) ) ) ); - buildUpExpectedData( nodeData, relationshipData, expectedNodes, expectedNodeNames, expectedRelationships, + buildUpExpectedData( nodeData, relationshipData, expectedNodes, expectedNodeNames, expectedNodeProperties, expectedRelationships, expectedNodeCounts, expectedRelationshipCounts ); // Do the verification @@ -301,6 +308,13 @@ private void verifyImportedData( List nodeData, String name = (String) node.getProperty( "name" ); String[] labels = expectedNodeNames.remove( name ); assertEquals( asSet( labels ), names( node.getLabels() ) ); + Map expectedProperties = expectedNodeProperties.remove( name ); + Map actualProperties = node.getAllProperties(); + actualProperties.remove( "id" ); + // TODO: Fix the expectations for the points + PointValue actualPoint = (PointValue) actualProperties.remove( "point" ); + String expectedPoint = (String) expectedProperties.remove( "point" ); + assertEquals( expectedProperties, actualProperties ); } assertEquals( 0, expectedNodeNames.size() ); @@ -444,6 +458,7 @@ private void buildUpExpectedData( List relationshipData, Map expectedNodes, Map expectedNodeNames, + Map> expectedNodeProperties, Map>> expectedRelationships, Map nodeCounts, Map>> relationshipCounts ) @@ -452,6 +467,15 @@ private void buildUpExpectedData( { expectedNodes.put( (String) node.id(), node ); expectedNodeNames.put( nameOf( node ), node.labels() ); + + assert node.hasIntPropertyKeyIds == false; + Map properties = new TreeMap<>(); + for ( int i = 0; i < node.propertyCount(); i++ ) + { + properties.put( (String) node.propertyKey( i ), node.propertyValue( i ) ); + } + expectedNodeProperties.put( nameOf( node ), properties ); + countNodeLabels( nodeCounts, node.labels() ); } for ( InputEntity relationship : relationshipData ) diff --git a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java index bde13f5c53376..676437b1d483c 100644 --- a/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java +++ b/community/kernel/src/test/java/org/neo4j/unsafe/impl/batchimport/input/csv/CsvInputTest.java @@ -52,6 +52,8 @@ import org.neo4j.unsafe.impl.batchimport.input.InputEntityDecorators; import org.neo4j.unsafe.impl.batchimport.input.InputEntityVisitor; import org.neo4j.unsafe.impl.batchimport.input.InputException; +import org.neo4j.values.storable.CoordinateReferenceSystem; +import org.neo4j.values.storable.Values; import static java.util.Arrays.asList; import static org.junit.Assert.assertArrayEquals; @@ -456,6 +458,30 @@ public void shouldIgnoreEmptyIntPropertyValues() throws Exception } } + @Test + public void shouldParsePointPropertyValues() throws Exception + { + // GIVEN + DataFactory data = data( + ":ID,name,point:Point\n" + + "0,Mattias,\"{x: 2.7, y:3.2 }\"\n" + + "1,Johan,\" { height :0.01 ,longitude:5, latitude : -4.2 } \"\n" ); + Iterable dataIterable = dataIterable( data ); + Input input = new CsvInput( dataIterable, defaultFormatNodeFileHeader(), datas(), defaultFormatRelationshipFileHeader(), + IdType.ACTUAL, config( COMMAS ), silentBadCollector( 0 ) ); + + // WHEN + try ( InputIterator nodes = input.nodes().iterator() ) + { + // THEN + assertNextNode( nodes, 0L, new Object[]{"name", "Mattias", "point", + Values.pointValue( CoordinateReferenceSystem.Cartesian, 2.7, 3.2) }, labels() ); + assertNextNode( nodes, 1L, new Object[]{"name", "Johan", "point", + Values.pointValue( CoordinateReferenceSystem.WGS84_3D, 5, -4.2, 0.01)}, labels() ); + assertFalse( readNext( nodes ) ); + } + } + @Test public void shouldFailOnArrayDelimiterBeingSameAsDelimiter() { diff --git a/community/values/src/main/java/org/neo4j/values/storable/PointValue.java b/community/values/src/main/java/org/neo4j/values/storable/PointValue.java index a181ae56a0b6d..6c969c0916f02 100644 --- a/community/values/src/main/java/org/neo4j/values/storable/PointValue.java +++ b/community/values/src/main/java/org/neo4j/values/storable/PointValue.java @@ -21,15 +21,21 @@ import java.util.Arrays; import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.neo4j.graphdb.spatial.CRS; import org.neo4j.graphdb.spatial.Coordinate; import org.neo4j.graphdb.spatial.Point; +import org.neo4j.values.AnyValue; import org.neo4j.values.ValueMapper; import org.neo4j.values.utils.PrettyPrinter; +import org.neo4j.values.virtual.MapValue; import static java.lang.String.format; import static java.util.Collections.singletonList; +import static org.neo4j.values.storable.ValueGroup.NUMBER; +import static org.neo4j.values.storable.ValueGroup.TEXT; public class PointValue extends ScalarValue implements Point, Comparable { @@ -255,4 +261,240 @@ public boolean withinRange( PointValue lower, boolean includeLower, PointValue u } return true; } + + public static PointValue fromMap( MapValue map ) + { + AnyValue[] fields = new Value[PointValueField.__MAX_VALUE__.ordinal()]; + for ( PointValueField f : PointValueField.values() ) + { + if ( f != PointValueField.__MAX_VALUE__ ) + { + AnyValue fieldValue = map.get( f.name().toLowerCase() ); + fields[f.ordinal()] = fieldValue != Values.NO_VALUE ? fieldValue : null; + } + } + return fromInputFields( fields ); + } + +// private static Pattern keyValuePattern = Pattern.compile( +// "\\s*+((?x)|(?y)|(?longitude)|(?latitude))\\s*+:\\s*+(?\\S+)|((?crs)\\s*+:\\s*+(?\\S+))" ); +// private static Pattern keyValuePattern = Pattern.compile( "(?[a-z_A-Z]\\w*+)\\s*:\\s*(?\\S+)" ); + private static Pattern mapPattern = Pattern.compile( "\\{(.*)\\}" ); + private static Pattern keyValuePattern = + Pattern.compile( "(?:\\A|,)\\s*+(?[a-z_A-Z]\\w*+)\\s*:\\s*(?[^\\s,]+)" ); + + private static Pattern quotesPattern = Pattern.compile( "^[\"']|[\"']$" ); + + public static PointValue parse( CharSequence text ) + { + Matcher mapMatcher = mapPattern.matcher( text ); + if ( !(mapMatcher.find() && mapMatcher.groupCount() == 1 ) ) + { + String errorMessage = format( "Failed to parse point value: '%s'", text ); + throw new IllegalArgumentException( errorMessage ); + } + + String mapContents = mapMatcher.group( 1 ); + if ( mapContents.isEmpty() ) + { + String errorMessage = format( "Failed to parse point value: '%s'", text ); + throw new IllegalArgumentException( errorMessage ); + } + + Matcher matcher = keyValuePattern.matcher( mapContents ); + if ( !(matcher.find() ) ) + { + String errorMessage = format( "Failed to parse point value: '%s'", text ); + throw new IllegalArgumentException( errorMessage ); + } + + Value[] fields = new Value[PointValueField.__MAX_VALUE__.ordinal()]; + + do + { + String key = matcher.group( "k" ); + if ( key != null ) + { + try + { + // NOTE: We let the key be case-insensitive here + PointValueField field = PointValueField.valueOf( PointValueField.class, key.toUpperCase() ); + + String value = matcher.group( "v" ); + if ( value != null ) + { + switch ( field.valueType() ) + { + case NUMBER: + { + DoubleValue doubleValue = Values.doubleValue( Double.parseDouble( value ) ); + fields[field.ordinal()] = doubleValue; + break; + } + + case TEXT: + { + // Eliminate any quoutes + String unquotedValue = quotesPattern.matcher( value ).replaceAll( "" ); + fields[field.ordinal()] = Values.stringValue( unquotedValue ); + break; + } + + default: + // Just ignore unknown fields + } + } + } + catch ( IllegalArgumentException e ) + { + // Ignore unknown fields + } + } + } while ( matcher.find() ); + + return fromInputFields( fields ); + } + + /** + * This contains the logic to decide the default coordinate reference system based on the input fields + */ + private static PointValue fromInputFields( AnyValue[] fields ) + { + CoordinateReferenceSystem crs; + double[] coordinates; + + AnyValue crsValue = fields[PointValueField.CRS.ordinal()]; + if ( crsValue != null ) + { + TextValue crsName = (TextValue) crsValue; + crs = CoordinateReferenceSystem.byName( crsName.stringValue() ); + if ( crs == null ) + { + throw new IllegalArgumentException( "Unknown coordinate reference system: " + crsName.stringValue() ); + } + } + else + { + crs = null; + } + + AnyValue xValue = fields[PointValueField.X.ordinal()]; + AnyValue yValue = fields[PointValueField.Y.ordinal()]; + AnyValue latitudeValue = fields[PointValueField.LATITUDE.ordinal()]; + AnyValue longitudeValue = fields[PointValueField.LONGITUDE.ordinal()]; + + if ( xValue != null && yValue != null ) + { + double x = ((NumberValue) xValue).doubleValue(); + double y = ((NumberValue) yValue).doubleValue(); + AnyValue zValue = fields[PointValueField.Z.ordinal()]; + + coordinates = zValue != null ? new double[]{x, y, ((NumberValue) zValue).doubleValue()} : new double[]{x, y}; + if ( crs == null ) + { + crs = coordinates.length == 3 ? CoordinateReferenceSystem.Cartesian_3D : CoordinateReferenceSystem.Cartesian; + } + } + else if ( latitudeValue != null && longitudeValue != null ) + { + double x = ((NumberValue) longitudeValue).doubleValue(); + double y = ((NumberValue) latitudeValue).doubleValue(); + AnyValue zValue = fields[PointValueField.Z.ordinal()]; + AnyValue heightValue = fields[PointValueField.HEIGHT.ordinal()]; + if ( zValue != null ) + { + coordinates = new double[]{x, y, ((NumberValue) zValue).doubleValue()}; + } + else if ( heightValue != null ) + { + coordinates = new double[]{x, y, ((NumberValue) heightValue).doubleValue()}; + } + else + { + coordinates = new double[]{x, y}; + } + if ( crs == null ) + { + crs = coordinates.length == 3 ? CoordinateReferenceSystem.WGS84_3D : CoordinateReferenceSystem.WGS84; + } + if ( !crs.isGeographic() ) + { + throw new IllegalArgumentException( "Geographic points does not support coordinate reference system: " + crs ); + } + } + else + { + if ( crs == CoordinateReferenceSystem.Cartesian ) + { + throw new IllegalArgumentException( "A " + CoordinateReferenceSystem.Cartesian.getName() + " point must contain 'x' and 'y'" ); + } + else if ( crs == CoordinateReferenceSystem.Cartesian_3D ) + { + throw new IllegalArgumentException( "A " + CoordinateReferenceSystem.Cartesian_3D.getName() + " point must contain 'x', 'y' and 'z'" ); + } + else if ( crs == CoordinateReferenceSystem.WGS84 ) + { + throw new IllegalArgumentException( "A " + CoordinateReferenceSystem.WGS84.getName() + " point must contain 'latitude' and 'longitude'" ); + } + else if ( crs == CoordinateReferenceSystem.WGS84_3D ) + { + throw new IllegalArgumentException( "A " + CoordinateReferenceSystem.WGS84_3D.getName() + " point must contain 'latitude', 'longitude' and 'height'" ); + } + throw new IllegalArgumentException( "A point must contain either 'x' and 'y' or 'latitude' and 'longitude'" ); + } + + if ( crs.getDimension() != coordinates.length ) + { + throw new IllegalArgumentException( "Cannot create " + crs.getDimension() + "D point with " + coordinates.length + " coordinates" ); + } + return Values.pointValue( crs, coordinates ); + } + + private static enum PointValueField + { + X + { + @Override + ValueGroup valueType() { return NUMBER; } + }, + Y + { + @Override + ValueGroup valueType() { return NUMBER; } + }, + Z + { + @Override + ValueGroup valueType() { return NUMBER; } + }, + LATITUDE + { + @Override + ValueGroup valueType() + { + return NUMBER; + } + }, + LONGITUDE + { + @Override + ValueGroup valueType() + { + return NUMBER; + } + }, + HEIGHT + { + @Override + ValueGroup valueType() { return NUMBER; } + }, + CRS + { + @Override + ValueGroup valueType() { return TEXT; } + }, + __MAX_VALUE__; + + ValueGroup valueType() { return ValueGroup.NO_VALUE; } + } } diff --git a/community/values/src/test/java/org/neo4j/values/storable/PointTest.java b/community/values/src/test/java/org/neo4j/values/storable/PointTest.java index 62131accc493b..7eb497e11df3d 100644 --- a/community/values/src/test/java/org/neo4j/values/storable/PointTest.java +++ b/community/values/src/test/java/org/neo4j/values/storable/PointTest.java @@ -23,7 +23,9 @@ import static org.junit.Assert.assertTrue; import static org.neo4j.values.storable.CoordinateReferenceSystem.Cartesian; +import static org.neo4j.values.storable.CoordinateReferenceSystem.Cartesian_3D; import static org.neo4j.values.storable.CoordinateReferenceSystem.WGS84; +import static org.neo4j.values.storable.CoordinateReferenceSystem.WGS84_3D; import static org.neo4j.values.storable.Values.pointValue; import static org.neo4j.values.utils.AnyValueTestUtil.assertEqual; import static org.neo4j.values.utils.AnyValueTestUtil.assertNotEqual; @@ -74,4 +76,89 @@ public void shouldHaveValueGroup() assertTrue( pointValue( Cartesian, 1, 2 ).valueGroup() != null ); assertTrue( pointValue( WGS84, 1, 2 ).valueGroup() != null ); } + + @Test + public void shouldBeAbleToParsePoints() + { + assertEqual( pointValue( WGS84, 13.2, 56.7 ), + PointValue.parse( "{latitude: 56.7, longitude: 13.2}" ) ); + assertEqual( pointValue( WGS84, -74.0060, 40.7128 ), + PointValue.parse( "{latitude: 40.7128, longitude: -74.0060, crs: 'wgs-84'}" ) ); // - explicitly WGS84 + assertEqual( pointValue( Cartesian, -21, -45.3 ), + PointValue.parse( "{x: -21, y: -45.3}" ) ); // - default to cartesian 2D + assertEqual( pointValue( Cartesian, 17, -52.8 ), + PointValue.parse( "{x: 17, y: -52.8, crs: 'cartesian'}" ) ); // - explicit cartesian 2D + assertEqual( pointValue( WGS84_3D, 13.2, 56.7, 123.4 ), + PointValue.parse( "{latitude: 56.7, longitude: 13.2, height: 123.4}" ) ); // - defaults to WGS84-3D + assertEqual( pointValue( WGS84_3D, 13.2, 56.7, 123.4 ), + PointValue.parse( "{latitude: 56.7, longitude: 13.2, z: 123.4}" ) ); // - defaults to WGS84-3D + assertEqual( pointValue( WGS84_3D, -74.0060, 40.7128, 567.8 ), + PointValue.parse( "{latitude: 40.7128, longitude: -74.0060, height: 567.8, crs: 'wgs-84-3D'}" ) ); // - explicitly WGS84-3D + assertEqual( pointValue( Cartesian_3D, -21, -45.3, 7.2 ), + PointValue.parse( "{x: -21, y: -45.3, z: 7.2}" ) ); // - default to cartesian 3D + assertEqual( pointValue( Cartesian_3D, 17, -52.8, -83.1 ), + PointValue.parse( "{x: 17, y: -52.8, z: -83.1, crs: 'cartesian-3D'}" ) ); // - explicit cartesian 3D + } + + @Test + public void shouldBeAbleToParsePointWithUnquotedCrs() + { + assertEqual( pointValue( WGS84_3D, -74.0060, 40.7128, 567.8 ), + PointValue.parse( "{latitude: 40.7128, longitude: -74.0060, height: 567.8, crs:wgs-84-3D}" ) ); // - explicitly WGS84-3D, without quotes + } + + @Test + public void shouldBeAbleToParseWeirdlyFormattedPoints() + { + assertEqual( pointValue( WGS84, 1.0, 2.0 ), PointValue.parse( " \t\n { latitude : 2.0 ,longitude :1.0 } \t" ) ); + // TODO: Should some/all of these fail? + assertEqual( pointValue( WGS84, 1.0, 2.0 ), PointValue.parse( " \t\n { latitude : 2.0 ,longitude :1.0 , } \t" ) ); + assertEqual( pointValue( Cartesian, 2.0E-8, -1.0E7 ), PointValue.parse( " \t\n { x :+.2e-7,y: -1.0E07 , } \t" ) ); + assertEqual( pointValue( Cartesian, 2.0E-8, -1.0E7 ), PointValue.parse( " \t\n { x :+.2e-7,y: -1.0E07 , garbage} \t" ) ); + assertEqual( pointValue( Cartesian, 2.0E-8, -1.0E7 ), PointValue.parse( " \t\n { gar ba ge,x :+.2e-7,y: -1.0E07} \t" ) ); + } + + @Test + public void shouldBeAbleToParsePointAndIgnoreUnknownFields() + { + assertEqual( pointValue( WGS84, 1.0, 2.0 ), PointValue.parse( "{latitude:2.0,longitude:1.0,unknown_field:\"hello\"}" ) ); + } + + @Test + public void shouldNotBeAbleToParsePointsWithConflictingDuplicateFields() + { + // TODO: Make this fail + assertEqual( pointValue( WGS84, 1.0, 3.0 ), PointValue.parse( "{latitude: 2.0, longitude: 1.0, latitude: 3.0}" ) ); + assertEqual( pointValue( Cartesian, 1.0, 3.0 ), PointValue.parse( "{crs: 'cartesian', x: 2.0, x: 1.0, y: 3}" ) ); + assertEqual( pointValue( Cartesian, 1.0, 3.0 ), PointValue.parse( "{crs: 'invalid crs', x: 1.0, y: 3, crs: 'cartesian'}" ) ); + } + + @Test + public void shouldNotBeAbleToParseIncompletePoints() + { + assertCannotParse( "{latitude: 56.7, longitude:}" ); + assertCannotParse( "{latitude: 56.7}" ); + assertCannotParse( "{}" ); + assertCannotParse( "{only_a_key}" ); + assertCannotParse( "{crs:'WGS-84'}" ); + assertCannotParse( "{a:a}" ); + assertCannotParse( "{ : 2.0, x : 1.0 }" ); + assertCannotParse( "x:1,y:2" ); + assertCannotParse( "{x:1,y:'2'}" ); + assertCannotParse( "{crs:WGS-84 , lat:1, y:2}" ); + } + + private IllegalArgumentException assertCannotParse( String text ) + { + PointValue value; + try + { + value = PointValue.parse( text ); + } + catch ( IllegalArgumentException e ) + { + return e; + } + throw new AssertionError( String.format( "'%s' parsed to %s", text, value ) ); + } } diff --git a/community/values/src/test/java/org/neo4j/values/utils/AnyValueTestUtil.java b/community/values/src/test/java/org/neo4j/values/utils/AnyValueTestUtil.java index 0ab5cedb85396..1427f0535ff39 100644 --- a/community/values/src/test/java/org/neo4j/values/utils/AnyValueTestUtil.java +++ b/community/values/src/test/java/org/neo4j/values/utils/AnyValueTestUtil.java @@ -31,20 +31,22 @@ public class AnyValueTestUtil { public static void assertEqual( AnyValue a, AnyValue b ) { - assertTrue( - String.format( "%s should be equivalent to %s", a.getClass().getSimpleName(), b.getClass().getSimpleName() ), + assertTrue( formatMessage( "should be equivalent to", a, b ), a.equals( b ) ); assertTrue( - String.format( "%s should be equivalent to %s", a.getClass().getSimpleName(), b.getClass().getSimpleName() ), + formatMessage( "should be equivalent to", b, a ), b.equals( a ) ); - assertTrue( - String.format( "%s should be equal %s", a.getClass().getSimpleName(), b.getClass().getSimpleName() ), + assertTrue( formatMessage( "should be equal to", a, b ), a.ternaryEquals( b ) ); - assertTrue( - String.format( "%s should be equal %s", a.getClass().getSimpleName(), b.getClass().getSimpleName() ), + assertTrue( formatMessage( "should be equal to", b, a ), b.ternaryEquals( a ) ); - assertTrue( String.format( "%s should have same hashcode as %s", a.getClass().getSimpleName(), - b.getClass().getSimpleName() ), a.hashCode() == b.hashCode() ); + assertTrue( formatMessage( "should have same hashcode as", a, b ), + a.hashCode() == b.hashCode() ); + } + + private static String formatMessage( String should, AnyValue a, AnyValue b ) + { + return String.format( "%s(%s) %s %s(%s)", a.getClass().getSimpleName(), a.toString(), should, b.getClass().getSimpleName(), b.toString() ); } public static void assertEqualValues( AnyValue a, AnyValue b )