diff --git a/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java b/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java index eb1f042e3b5ac..60e6e4c15e161 100644 --- a/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java +++ b/community/kernel/src/test/java/org/neo4j/graphdb/SpatialMocks.java @@ -139,5 +139,11 @@ public CRS getCRS() { return crs; } + + @Override + public String toString() + { + return geometryType; + } } } diff --git a/community/server/src/main/java/org/neo4j/server/rest/transactional/Neo4jJsonCodec.java b/community/server/src/main/java/org/neo4j/server/rest/transactional/Neo4jJsonCodec.java index ef72c6b5e948a..6a7fa0561c92d 100644 --- a/community/server/src/main/java/org/neo4j/server/rest/transactional/Neo4jJsonCodec.java +++ b/community/server/src/main/java/org/neo4j/server/rest/transactional/Neo4jJsonCodec.java @@ -44,15 +44,15 @@ import org.neo4j.graphdb.spatial.Coordinate; import org.neo4j.graphdb.spatial.Geometry; import org.neo4j.graphdb.spatial.Point; -import org.neo4j.values.storable.PointValue; +import static java.util.Objects.requireNonNull; import static org.neo4j.helpers.collection.MapUtil.genericMap; public class Neo4jJsonCodec extends ObjectMapper { private enum Neo4jJsonMetaType { - node, relationship, datetime, time, localdatetime, date, localtime, duration, point2d, point3d + node, relationship, datetime, time, localdatetime, date, localtime, duration, point } private TransitionalPeriodTransactionMessContainer container; @@ -114,9 +114,13 @@ else if ( value instanceof CRS ) writeMap( out, genericMap( new LinkedHashMap<>(), "name", crs.getType(), "type", "link", "properties", genericMap( new LinkedHashMap<>(), "href", crs.getHref() + "ogcwkt/", "type", "ogcwkt" ) ) ); } - else if ( value instanceof Temporal || value instanceof TemporalAmount ) + else if ( value instanceof Temporal ) + { + writeObject( out, parseTemporalType( (Temporal) value ), value.toString() ); + } + else if ( value instanceof TemporalAmount ) { - super.writeValue( out, value.toString() ); + writeObject( out, Neo4jJsonMetaType.duration, value.toString() ); } else { @@ -124,6 +128,11 @@ else if ( value instanceof Temporal || value instanceof TemporalAmount ) } } + private void writeObject( JsonGenerator out, Neo4jJsonMetaType type, String value ) throws IOException + { + writeMap( out, genericMap( new LinkedHashMap<>(), "type", type.name(), "value", value ) ); + } + private void writeMap( JsonGenerator out, Map value ) throws IOException { out.writeStartObject(); @@ -238,7 +247,7 @@ void writeMeta( JsonGenerator out, Object value ) throws IOException Node node = (Node) value; try ( TransactionStateChecker stateChecker = TransactionStateChecker.create( container ) ) { - writeNodeOrRelationshipMeta( out, node.getId(), Neo4jJsonMetaType.node.name(), stateChecker.isNodeDeletedInCurrentTx( node.getId() ) ); + writeNodeOrRelationshipMeta( out, node.getId(), Neo4jJsonMetaType.node, stateChecker.isNodeDeletedInCurrentTx( node.getId() ) ); } } else if ( value instanceof Relationship ) @@ -246,7 +255,7 @@ else if ( value instanceof Relationship ) Relationship relationship = (Relationship) value; try ( TransactionStateChecker transactionStateChecker = TransactionStateChecker.create( container ) ) { - writeNodeOrRelationshipMeta( out, relationship.getId(), Neo4jJsonMetaType.relationship.name(), + writeNodeOrRelationshipMeta( out, relationship.getId(), Neo4jJsonMetaType.relationship, transactionStateChecker.isRelationshipDeletedInCurrentTx( relationship.getId() ) ); } } @@ -275,11 +284,11 @@ else if ( value instanceof Geometry ) } else if ( value instanceof Temporal ) { - writeTemporalTypeMeta( out, (Temporal) value ); + writeObjectMeta( out, parseTemporalType( (Temporal) value ) ); } else if ( value instanceof TemporalAmount ) { - writeTypeMeta( out, Neo4jJsonMetaType.duration.name() ); + writeObjectMeta( out, Neo4jJsonMetaType.duration ); } else { @@ -290,28 +299,19 @@ else if ( value instanceof TemporalAmount ) private void writeGeometryTypeMeta( JsonGenerator out, Geometry value ) throws IOException { Neo4jJsonMetaType type = null; - if ( value instanceof PointValue ) + if ( value instanceof Point ) { - PointValue p = (PointValue) value; - int size = p.coordinate().length; - if ( size == 2 ) - { - type = Neo4jJsonMetaType.point2d; - } - else if ( size == 3 ) - { - type = Neo4jJsonMetaType.point3d; - } + type = Neo4jJsonMetaType.point; } if ( type == null ) { throw new IllegalArgumentException( - String.format( "Unsupported Geometry type: type=%s, value=%s", value.getClass().getSimpleName(), value.toString() ) ); + String.format( "Unsupported Geometry type: type=%s, value=%s", value.getClass().getSimpleName(), value ) ); } - writeTypeMeta( out, type.name() ); + writeObjectMeta( out, type ); } - private void writeTemporalTypeMeta( JsonGenerator out, Temporal value ) throws IOException + private Neo4jJsonMetaType parseTemporalType( Temporal value ) { Neo4jJsonMetaType type = null; if ( value instanceof ZonedDateTime ) @@ -334,13 +334,12 @@ else if ( value instanceof LocalTime ) { type = Neo4jJsonMetaType.localtime; } - if ( type == null ) { throw new IllegalArgumentException( - String.format( "Unsupported Temporal type: type=%s, value=%s", value.getClass().getSimpleName(), value.toString() ) ); + String.format( "Unsupported Temporal type: type=%s, value=%s", value.getClass().getSimpleName(), value ) ); } - writeTypeMeta( out, type.name() ); + return type; } private void writeMetaPath( JsonGenerator out, Path value ) throws IOException @@ -359,13 +358,14 @@ private void writeMetaPath( JsonGenerator out, Path value ) throws IOException } } - private void writeTypeMeta( JsonGenerator out, String type ) + private void writeObjectMeta( JsonGenerator out, Neo4jJsonMetaType type ) throws IOException { + requireNonNull( type, "The meta type cannot be null for known types." ); out.writeStartObject(); try { - out.writeStringField( "type", type ); + out.writeStringField( "type", type.name() ); } finally { @@ -373,14 +373,15 @@ private void writeTypeMeta( JsonGenerator out, String type ) } } - private void writeNodeOrRelationshipMeta( JsonGenerator out, long id, String type, boolean isDeleted ) + private void writeNodeOrRelationshipMeta( JsonGenerator out, long id, Neo4jJsonMetaType type, boolean isDeleted ) throws IOException { + requireNonNull( type, "The meta type could not be null for node or relationship." ); out.writeStartObject(); try { out.writeNumberField( "id", id ); - out.writeStringField( "type", type ); + out.writeStringField( "type", type.name() ); out.writeBooleanField( "deleted", isDeleted ); } finally diff --git a/community/server/src/test/java/org/neo4j/server/rest/AbstractRestFunctionalTestBase.java b/community/server/src/test/java/org/neo4j/server/rest/AbstractRestFunctionalTestBase.java index f3fed15323c61..474a34ad6b3b9 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/AbstractRestFunctionalTestBase.java +++ b/community/server/src/test/java/org/neo4j/server/rest/AbstractRestFunctionalTestBase.java @@ -282,8 +282,8 @@ public String getSchemaConstraintLabelUniquenessPropertyUri( String label, Strin public static int getLocalHttpPort() { - ConnectorPortRegister connectorPortRegister = server().getDatabase().getGraph().getDependencyResolver() - .resolveDependency( ConnectorPortRegister.class ); + ConnectorPortRegister connectorPortRegister = + server().getDatabase().getGraph().getDependencyResolver().resolveDependency( ConnectorPortRegister.class ); return connectorPortRegister.getLocalAddress( "http" ).getPort(); } diff --git a/community/server/src/test/java/org/neo4j/server/rest/transactional/ExecutionResultSerializerTest.java b/community/server/src/test/java/org/neo4j/server/rest/transactional/ExecutionResultSerializerTest.java index 47372eb88514f..a46575b396c87 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/transactional/ExecutionResultSerializerTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/transactional/ExecutionResultSerializerTest.java @@ -27,6 +27,15 @@ import java.io.IOException; import java.io.OutputStream; import java.net.URI; +import java.time.Duration; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.OffsetTime; +import java.time.ZoneId; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -64,8 +73,10 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.asList; import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; @@ -405,15 +416,11 @@ public void shouldSerializePointsAsListOfMapsOfProperties() throws Exception ByteArrayOutputStream output = new ByteArrayOutputStream(); ExecutionResultSerializer serializer = getSerializerWith( output ); - List points = new ArrayList<>(); - points.add( new Coordinate( 1, 2 ) ); - points.add( new Coordinate( 2, 3 ) ); Result executionResult = mockExecutionResult( map( "geom", SpatialMocks.mockPoint( 12.3, 45.6, mockWGS84() ) ), map( "geom", SpatialMocks.mockPoint( 123, 456, mockCartesian() ) ), map( "geom", SpatialMocks.mockPoint( 12.3, 45.6, 78.9, mockWGS84_3D() ) ), - map( "geom", SpatialMocks.mockPoint( 123, 456, 789, mockCartesian_3D() ) ), - map( "geom", SpatialMocks.mockGeometry( "LineString", points, mockCartesian() ) ) ); + map( "geom", SpatialMocks.mockPoint( 123, 456, 789, mockCartesian_3D() ) ) ); // when serializer.statementResult( executionResult, false ); @@ -425,27 +432,90 @@ public void shouldSerializePointsAsListOfMapsOfProperties() throws Exception "{\"row\":[{\"type\":\"Point\",\"coordinates\":[12.3,45.6],\"crs\":" + "{\"name\":\"WGS-84\",\"type\":\"link\",\"properties\":" + "{\"href\":\"http://spatialreference.org/ref/epsg/4326/ogcwkt/\",\"type\":\"ogcwkt\"}" + - "}}],\"meta\":[null]}," + + "}}],\"meta\":[{\"type\":\"point\"}]}," + "{\"row\":[{\"type\":\"Point\",\"coordinates\":[123.0,456.0],\"crs\":" + "{\"name\":\"cartesian\",\"type\":\"link\",\"properties\":" + "{\"href\":\"http://spatialreference.org/ref/sr-org/7203/ogcwkt/\",\"type\":\"ogcwkt\"}" + - "}}],\"meta\":[null]}," + + "}}],\"meta\":[{\"type\":\"point\"}]}," + "{\"row\":[{\"type\":\"Point\",\"coordinates\":[12.3,45.6,78.9],\"crs\":" + "{\"name\":\"WGS-84-3D\",\"type\":\"link\",\"properties\":" + "{\"href\":\"http://spatialreference.org/ref/epsg/4979/ogcwkt/\",\"type\":\"ogcwkt\"}" + - "}}],\"meta\":[null]}," + + "}}],\"meta\":[{\"type\":\"point\"}]}," + "{\"row\":[{\"type\":\"Point\",\"coordinates\":[123.0,456.0,789.0],\"crs\":" + "{\"name\":\"cartesian-3D\",\"type\":\"link\",\"properties\":" + "{\"href\":\"http://spatialreference.org/ref/sr-org/9157/ogcwkt/\",\"type\":\"ogcwkt\"}" + - "}}],\"meta\":[null]}," + - "{\"row\":[{\"type\":\"LineString\",\"coordinates\":[[1.0,2.0],[2.0,3.0]],\"crs\":" + - "{\"name\":\"cartesian\",\"type\":\"link\",\"properties\":" + - "{\"href\":\"http://spatialreference.org/ref/sr-org/7203/ogcwkt/\",\"type\":\"ogcwkt\"}" + - "}}],\"meta\":[null]}" + - "]}],\"errors\":[]}", + "}}],\"meta\":[{\"type\":\"point\"}]}" + + "]}],\"errors\":[]}", result ); } + @Test + public void shouldSerializeTemporalAsListOfMapsOfProperties() throws Exception + { + // given + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ExecutionResultSerializer serializer = getSerializerWith( output ); + + Result executionResult = mockExecutionResult( + map( "temporal", LocalDate.of( 2018, 3, 12 ) ), + map( "temporal", ZonedDateTime.of( 2018, 3, 12, 13, 2, 10, 10, ZoneId.of( "UTC+1" ) ) ), + map( "temporal", OffsetTime.of( 12, 2, 4, 71, ZoneOffset.UTC ) ), + map( "temporal", LocalDateTime.of( 2018, 3, 12, 13, 2, 10, 10 ) ), + map( "temporal", LocalTime.of( 13, 2, 10, 10 ) ), + map( "temporal", Duration.of( 12, ChronoUnit.HOURS ) ) ); + + // when + serializer.statementResult( executionResult, false ); + serializer.finish(); + + // then + String result = output.toString( UTF_8.name() ); + assertEquals( "{\"results\":[{\"columns\":[\"temporal\"],\"data\":[" + + "{\"row\":[{\"type\":\"date\",\"value\":\"2018-03-12\"}],\"meta\":[{\"type\":\"date\"}]}," + + "{\"row\":[{\"type\":\"datetime\",\"value\":\"2018-03-12T13:02:10.000000010+01:00[UTC+01:00]\"}],\"meta\":[{\"type\":\"datetime\"}]}," + + "{\"row\":[{\"type\":\"time\",\"value\":\"12:02:04.000000071Z\"}],\"meta\":[{\"type\":\"time\"}]}," + + "{\"row\":[{\"type\":\"localdatetime\",\"value\":\"2018-03-12T13:02:10.000000010\"}],\"meta\":[{\"type\":\"localdatetime\"}]}," + + "{\"row\":[{\"type\":\"localtime\",\"value\":\"13:02:10.000000010\"}],\"meta\":[{\"type\":\"localtime\"}]}," + + "{\"row\":[{\"type\":\"duration\",\"value\":\"PT12H\"}],\"meta\":[{\"type\":\"duration\"}]}" + + "]}],\"errors\":[]}", + result ); + } + + @Test + public void shouldErrorWhenSerializingUnknownGeometryType() throws Exception + { + // given + ByteArrayOutputStream output = new ByteArrayOutputStream(); + ExecutionResultSerializer serializer = getSerializerWith( output ); + + List points = new ArrayList<>(); + points.add( new Coordinate( 1, 2 ) ); + points.add( new Coordinate( 2, 3 ) ); + Result executionResult = mockExecutionResult( + map( "geom", SpatialMocks.mockGeometry( "LineString", points, mockCartesian() ) ) ); + + // when + try + { + serializer.statementResult( executionResult, false ); + fail( "should have thrown exception" ); + } + catch ( RuntimeException e ) + { + serializer.errors( asList( new Neo4jError( Status.Statement.ExecutionFailed, e ) ) ); + } + + // then + String result = output.toString( UTF_8.name() ); + assertThat( result, startsWith( "{\"results\":[{\"columns\":[\"geom\"],\"data\":[" + + "{\"row\":[{\"type\":\"LineString\",\"coordinates\":[[1.0,2.0],[2.0,3.0]],\"crs\":" + + "{\"name\":\"cartesian\",\"type\":\"link\",\"properties\":" + + "{\"href\":\"http://spatialreference.org/ref/sr-org/7203/ogcwkt/\",\"type\":\"ogcwkt\"}}}],\"meta\":[]}]}]," + + "\"errors\":[{\"code\":\"Neo.DatabaseError.Statement.ExecutionFailed\"," + + "\"message\":\"Unsupported Geometry type: type=MockGeometry, value=LineString\"," + + "\"stackTrace\":\"java.lang.IllegalArgumentException: Unsupported Geometry type: type=MockGeometry, value=LineString" ) ); + } + @Test public void shouldProduceWellFormedJsonEvenIfResultIteratorThrowsExceptionOnNext() throws Exception { diff --git a/community/server/src/test/java/org/neo4j/server/rest/transactional/Neo4jJsonCodecTest.java b/community/server/src/test/java/org/neo4j/server/rest/transactional/Neo4jJsonCodecTest.java index 01046ec420437..6f4142b922a41 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/transactional/Neo4jJsonCodecTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/transactional/Neo4jJsonCodecTest.java @@ -296,16 +296,4 @@ public void testGeometryWriting() throws IOException //Then verify( jsonGenerator, times( 3 ) ).writeEndObject(); } - - @Test - public void testDateTimeWriting() throws Throwable - { - // Given - - - - // When - - // Then - } } diff --git a/community/server/src/test/java/org/neo4j/server/rest/transactional/integration/TemporalTypeIT.java b/community/server/src/test/java/org/neo4j/server/rest/transactional/integration/TemporalTypeIT.java index 3a773e30ca40b..50f5e1d7c27b3 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/transactional/integration/TemporalTypeIT.java +++ b/community/server/src/test/java/org/neo4j/server/rest/transactional/integration/TemporalTypeIT.java @@ -40,8 +40,7 @@ public void shouldWorkWithDateTime() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "0001-10-02T00:00+01:00", getSingle( data, "row" ).asText() ); - assertEquals( "datetime", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "0001-10-02T00:00+01:00", "datetime" ); } @Test @@ -52,8 +51,7 @@ public void shouldWorkWithTime() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "23:19:55-07:00", getSingle( data, "row" ).asText() ); - assertEquals( "time", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "23:19:55-07:00", "time" ); } @Test @@ -64,8 +62,7 @@ public void shouldWorkWithLocalDateTime() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "1984-10-21T12:34", getSingle( data, "row" ).asText() ); - assertEquals( "localdatetime", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "1984-10-21T12:34", "localdatetime" ); } @Test @@ -76,8 +73,7 @@ public void shouldWorkWithDate() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "1984-10-11", getSingle( data, "row" ).asText() ); - assertEquals( "date", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "1984-10-11", "date" ); } @Test @@ -88,8 +84,7 @@ public void shouldWorkWithLocalTime() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "12:31:14.645876123", getSingle( data, "row" ).asText() ); - assertEquals( "localtime", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "12:31:14.645876123", "localtime" ); } @Test @@ -100,27 +95,38 @@ public void shouldWorkWithDuration() throws Throwable assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); - assertEquals( "P17D", getSingle( data, "row" ).asText() ); - assertEquals( "duration", getSingle( data, "meta" ).get( "type" ).asText() ); + assertTemporalEquals( data, "P17D", "duration" ); } @Test public void shouldOnlyGetNodeTypeInMetaAsNodeProperties() throws Throwable { - HTTP.Response response = - runQuery( "CREATE (account {creationTime: localdatetime({year: 1984, month: 10, day: 21, hour: 12, minute: 34})}) RETURN account" ); + HTTP.Response response = runQuery( + "CREATE (account {name: \\\"zhen\\\", creationTime: localdatetime({year: 1984, month: 10, day: 21, hour: 12, minute: 34})}) RETURN account" ); assertEquals( 200, response.status() ); assertNoErrors( response ); JsonNode data = getSingleData( response ); JsonNode row = getSingle( data, "row" ); - assertThat( row.get( "creationTime" ).asText(), equalTo( "1984-10-21T12:34" ) ); + assertThat( row.get( "creationTime" ).get( "type" ).asText(), equalTo( "localdatetime" ) ); + assertThat( row.get( "creationTime" ).get( "value" ).asText(), equalTo( "1984-10-21T12:34" ) ); + assertThat( row.get( "name" ).asText(), equalTo( "zhen" ) ); JsonNode meta = getSingle( data, "meta" ); assertThat( meta.get( "type" ).asText(), equalTo( "node" ) ); } + private void assertTemporalEquals( JsonNode data, String value, String type ) + { + JsonNode row = getSingle( data, "row" ); + assertThat( row.get( "type" ).asText(), equalTo( type ) ); + assertThat( row.get( "value" ).asText(), equalTo( value ) ); + + JsonNode meta = getSingle( data, "meta" ); + assertEquals( type, meta.get( "type" ).asText() ); + } + private static JsonNode getSingleData( HTTP.Response response ) throws JsonParseException { JsonNode data = response.get( "results" ).get( 0 ).get( "data" );