GH-768 - Explicitly use ISO based date- and timeformatters.

Use - analogue to Jackson - explicit ISO based date- and timeformatters and not the default toString and parse methods of various temporals. Backport with tests adapted to pass in the string values as the Jackson-Setup in 3.1.x doesn’t have the JavaTimeModule ready.
neo4j · Feb 24, 2020 · 5d01711 · 5d01711
1 parent 92685da
commit 5d01711
Show file tree

Hide file tree

Showing 7 changed files with 266 additions and 10 deletions.
diff --git a/core/src/main/java/org/neo4j/ogm/typeconversion/InstantStringConverter.java b/core/src/main/java/org/neo4j/ogm/typeconversion/InstantStringConverter.java
@@ -65,6 +65,6 @@ public Instant toEntityAttribute(String value) {
         if (value == null || (lenient && StringUtils.isBlank(value))) {
             return null;
         }
-        return Instant.from(formatter.parse(value));
+        return formatter.parse(value, Instant::from);
     }
 }
diff --git a/core/src/main/java/org/neo4j/ogm/typeconversion/LocalDateStringConverter.java b/core/src/main/java/org/neo4j/ogm/typeconversion/LocalDateStringConverter.java
@@ -19,6 +19,7 @@
 package org.neo4j.ogm.typeconversion;
 
 import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
 
 /**
  * Converter to convert between {@link LocalDate} and {@link String}.
@@ -27,20 +28,23 @@
  *
  * @author Nicolas Mervaillie
  * @author Róbert Papp
+ * @author Michael J. Simons
  */
 public class LocalDateStringConverter implements AttributeConverter<LocalDate, String> {
 
+    private final DateTimeFormatter formatter = DateTimeFormatter.ISO_LOCAL_DATE;
+
     @Override
     public String toGraphProperty(LocalDate value) {
         if (value == null)
             return null;
-        return value.toString();
+        return formatter.format(value);
     }
 
     @Override
     public LocalDate toEntityAttribute(String value) {
         if (value == null)
             return null;
-        return LocalDate.parse(value);
+        return formatter.parse(value, LocalDate::from);
     }
 }
diff --git a/core/src/main/java/org/neo4j/ogm/typeconversion/LocalDateTimeStringConverter.java b/core/src/main/java/org/neo4j/ogm/typeconversion/LocalDateTimeStringConverter.java
@@ -19,6 +19,7 @@
 package org.neo4j.ogm.typeconversion;
 
 import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
 
 /**
  * Converter to convert between {@link LocalDateTime} and {@link String}.
@@ -27,20 +28,23 @@
  *
  * @author Frantisek Hartman
  * @author Róbert Papp
+ * @author Michael J. Simons
  */
 public class LocalDateTimeStringConverter implements AttributeConverter<LocalDateTime, String> {
 
+    private final DateTimeFormatter formatter = DateTimeFormatter.ISO_LOCAL_DATE_TIME;
+
     @Override
     public String toGraphProperty(LocalDateTime value) {
         if (value == null)
             return null;
-        return value.toString();
+        return formatter.format(value);
     }
 
     @Override
     public LocalDateTime toEntityAttribute(String value) {
         if (value == null)
             return null;
-        return LocalDateTime.parse(value);
+        return formatter.parse(value, LocalDateTime::from);
     }
 }
diff --git a/core/src/main/java/org/neo4j/ogm/typeconversion/OffsettDateTimeStringConverter.java b/core/src/main/java/org/neo4j/ogm/typeconversion/OffsettDateTimeStringConverter.java
@@ -19,6 +19,7 @@
 package org.neo4j.ogm.typeconversion;
 
 import java.time.OffsetDateTime;
+import java.time.format.DateTimeFormatter;
 
 /**
  * Converter to convert between {@link OffsetDateTime} and {@link String}.
@@ -27,20 +28,23 @@
  *
  * @author Frantisek Hartman
  * @author Róbert Papp
+ * @author Michael J. Simons
  */
 public class OffsettDateTimeStringConverter implements AttributeConverter<OffsetDateTime, String> {
 
+    private final DateTimeFormatter formatter = DateTimeFormatter.ISO_OFFSET_DATE_TIME;
+
     @Override
     public String toGraphProperty(OffsetDateTime value) {
         if (value == null)
             return null;
-        return value.toString();
+        return formatter.format(value);
     }
 
     @Override
     public OffsetDateTime toEntityAttribute(String value) {
         if (value == null)
             return null;
-        return OffsetDateTime.parse(value);
+        return formatter.parse(value, OffsetDateTime::from);
     }
 }
diff --git a/test/src/main/java/org/neo4j/ogm/testutil/MultiDriverTestClass.java b/test/src/main/java/org/neo4j/ogm/testutil/MultiDriverTestClass.java
@@ -27,6 +27,8 @@
 import java.util.Map;
 
 import org.junit.BeforeClass;
+import org.neo4j.driver.v1.AuthTokens;
+import org.neo4j.driver.v1.GraphDatabase;
 import org.neo4j.graphdb.GraphDatabaseService;
 import org.neo4j.graphdb.Result;
 import org.neo4j.ogm.config.ClasspathConfigurationSource;
@@ -99,6 +101,15 @@ public static Configuration.Builder getBaseConfiguration() {
         return Configuration.Builder.copy(baseConfiguration);
     }
 
+    protected static final org.neo4j.driver.v1.Driver getBoltConnection() {
+
+        if (testServer != null) {
+            return GraphDatabase.driver(testServer.getUri(), AuthTokens.none());
+        }
+        throw new IllegalStateException("Bolt connection can only be provided into a test server.");
+    }
+
+
     public static GraphDatabaseService getGraphDatabaseService() {
         // if using an embedded config, return the db from the driver
         if (baseConfiguration.build().getURI().startsWith("file")) {

diff --git a/test/src/test/java/org/neo4j/ogm/music/TimeHolder.java b/test/src/test/java/org/neo4j/ogm/music/TimeHolder.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2002-2020 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.ogm.domain.music;
+
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
+
+import org.neo4j.ogm.annotation.GeneratedValue;
+import org.neo4j.ogm.annotation.Id;
+import org.neo4j.ogm.annotation.NodeEntity;
+
+/**
+ * Holder for various temporal objects that are all subject to XXXStringConverter.
+ *
+ * @author Michael J. Simons
+ */
+@NodeEntity(label = "Data")
+public class TimeHolder {
+
+    @Id
+    @GeneratedValue
+    private Long graphId;
+
+    private OffsetDateTime someTime;
+
+    private LocalDateTime someLocalDateTime;
+
+    private LocalDate someLocalDate;
+
+    public Long getGraphId() {
+        return graphId;
+    }
+
+    public void setGraphId(Long graphId) {
+        this.graphId = graphId;
+    }
+
+    public OffsetDateTime getSomeTime() {
+        return someTime;
+    }
+
+    public void setSomeTime(OffsetDateTime someTime) {
+        this.someTime = someTime;
+    }
+
+    public LocalDateTime getSomeLocalDateTime() {
+        return someLocalDateTime;
+    }
+
+    public void setSomeLocalDateTime(LocalDateTime someLocalDateTime) {
+        this.someLocalDateTime = someLocalDateTime;
+    }
+
+    public LocalDate getSomeLocalDate() {
+        return someLocalDate;
+    }
+
+    public void setSomeLocalDate(LocalDate someLocalDate) {
+        this.someLocalDate = someLocalDate;
+    }
+}
diff --git a/test/src/test/java/org/neo4j/ogm/typeconversion/TypeConversionIntegrationTest.java b/test/src/test/java/org/neo4j/ogm/typeconversion/TypeConversionIntegrationTest.java
@@ -18,19 +18,34 @@
  */
 package org.neo4j.ogm.typeconversion;
 
+import static org.assertj.core.api.Assertions.*;
+
 import java.io.IOException;
 import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.OffsetDateTime;
 import java.time.ZoneId;
+import java.time.format.DateTimeFormatter;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.Map;
 
-import org.assertj.core.api.Assertions;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.neo4j.driver.v1.Driver;
+import org.neo4j.driver.v1.Record;
+import org.neo4j.driver.v1.Value;
+import org.neo4j.driver.v1.Values;
+import org.neo4j.graphdb.GraphDatabaseService;
+import org.neo4j.graphdb.Node;
+import org.neo4j.graphdb.Transaction;
 import org.neo4j.ogm.domain.music.Album;
 import org.neo4j.ogm.domain.music.Artist;
+import org.neo4j.ogm.domain.music.TimeHolder;
+import org.neo4j.ogm.model.Result;
 import org.neo4j.ogm.session.Session;
 import org.neo4j.ogm.session.SessionFactory;
 import org.neo4j.ogm.testutil.MultiDriverTestClass;
@@ -78,7 +93,147 @@ public void convertibleReturnTypesShouldBeHandled() {
         session.save(album);
         session.clear();
 
-        final Date latestReleases = session.queryForObject(Date.class, "MATCH (n:`l'album`) RETURN MAX(n.releasedAt)", new HashMap<>());
-        Assertions.assertThat(latestReleases).isEqualTo(queen2ReleaseDate);
+        final Date latestReleases = session
+            .queryForObject(Date.class, "MATCH (n:`l'album`) RETURN MAX(n.releasedAt)", new HashMap<>());
+        assertThat(latestReleases).isEqualTo(queen2ReleaseDate);
+    }
+
+    @Test // GH-766
+    public void savedTimestampAsMappingIsReadBackAsIs() {
+
+        OffsetDateTime someTime = OffsetDateTime.parse("2024-05-01T21:18:15.650+07:00");
+        LocalDateTime someLocalDateTime = LocalDateTime.parse("2024-05-01T21:18:15");
+        LocalDate someLocalDate = LocalDate.parse("2024-05-01");
+
+        TimeHolder timeHolder = new TimeHolder();
+        timeHolder.setSomeTime(someTime);
+        timeHolder.setSomeLocalDateTime(someLocalDateTime);
+        timeHolder.setSomeLocalDate(someLocalDate);
+
+        session.save(timeHolder);
+
+        verify(timeHolder.getGraphId(), someTime, someLocalDateTime, someLocalDate);
+    }
+
+    @Test // GH-766
+    public void savedTimestampAsParameterToBatchedCreateIsReadBackAsIs() {
+
+        String someTimeStringValue = "2024-05-01T21:18:15.65+07:00";
+        OffsetDateTime someTime = OffsetDateTime.parse(someTimeStringValue);
+        String someLocalDateTimeStringValue = "2024-05-01T21:18:15";
+        LocalDateTime someLocalDateTime = LocalDateTime.parse(someLocalDateTimeStringValue);
+        String someLocalDateStringValue = "2024-05-01";
+        LocalDate someLocalDate = LocalDate.parse(someLocalDateStringValue);
+
+        Map<String, Object> props = new HashMap<>();
+        props.put("someTime", someTimeStringValue);
+        props.put("someLocalDateTime", someLocalDateTimeStringValue);
+        props.put("someLocalDate", someLocalDateStringValue);
+
+        Map<String, Object> row = new HashMap<>();
+        row.put("nodeRef", -1);
+        row.put("props", props);
+
+        Map<String, Object> parameters = new HashMap<>();
+        parameters.put("type", "node");
+        parameters.put("rows", Collections.singletonList(row));
+
+        Result result = session.query(
+            // same query as ouput by org.neo4j.ogm.drivers.bolt.request.BoltRequest(BoltRequest.java:178)
+            "UNWIND {rows} AS row CREATE (n:`Data`) SET n=row.props RETURN row.nodeRef AS ref, id(n) AS id, {type} AS type",
+            parameters);
+
+        verify((Long) result.queryResults().iterator().next().get("id"), someTime, someLocalDateTime, someLocalDate);
+    }
+
+    @Test // GH-766
+    public void dataStoredInNotRealIsoFormatShouldStillBeParsed() {
+
+        OffsetDateTime someTime1 = OffsetDateTime.parse("2024-05-01T21:18:15.650+07:00");
+        OffsetDateTime someTime2 = OffsetDateTime.parse("2024-05-01T21:18:15.65+07:00");
+        OffsetDateTime someTime3 = DateTimeFormatter.ISO_OFFSET_DATE_TIME
+            .parse("2024-05-01T21:18:15.650+07:00", OffsetDateTime::from);
+        OffsetDateTime someTime4 = DateTimeFormatter.ISO_OFFSET_DATE_TIME
+            .parse("2024-05-01T21:18:15.65+07:00", OffsetDateTime::from);
+
+        assertThat(someTime1).isEqualTo(someTime2);
+        assertThat(someTime2).isEqualTo(someTime3);
+        assertThat(someTime3).isEqualTo(someTime4);
+
+        String withDifferentMillis = "2024-05-01T21:18:15.651+07:00";
+        OffsetDateTime a = OffsetDateTime.parse(withDifferentMillis);
+        OffsetDateTime b = DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(withDifferentMillis, OffsetDateTime::from);
+        assertThat(a).isEqualTo(b);
+    }
+
+    @Test // GH-766
+    public void savedTimestampAsParameterToSimpleCreateIsReadBackAsIs() {
+
+        String someTimeStringValue = "2024-05-01T21:18:15.65+07:00";
+        OffsetDateTime someTime = OffsetDateTime.parse(someTimeStringValue);
+        String someLocalDateTimeStringValue = "2024-05-01T21:18:15";
+        LocalDateTime someLocalDateTime = LocalDateTime.parse(someLocalDateTimeStringValue);
+        String someLocalDateStringValue = "2024-05-01";
+        LocalDate someLocalDate = LocalDate.parse(someLocalDateStringValue);
+
+        Map<String, Object> props = new HashMap<>();
+        props.put("someTime", someTimeStringValue);
+        props.put("someLocalDateTime", someLocalDateTimeStringValue);
+        props.put("someLocalDate", someLocalDateStringValue);
+
+        TimeHolder timeHolder = session.queryForObject(
+            TimeHolder.class,
+            "CREATE (d:Data {someTime: $someTime, someLocalDateTime: $someLocalDateTime, someLocalDate: $someLocalDate}) RETURN d",
+            props
+        );
+        verify(timeHolder.getGraphId(), someTime, someLocalDateTime, someLocalDate);
+    }
+
+    private void verify(Long graphId, OffsetDateTime expectedOffsetDateTime, LocalDateTime expectedLocalDateTime,
+        LocalDate expectedLocalDate) {
+
+        // opening a new Session to prevent shared data
+        TimeHolder reloaded = sessionFactory.openSession().load(TimeHolder.class, graphId);
+
+        assertThat(reloaded.getSomeTime()).isEqualTo(expectedOffsetDateTime);
+        assertThat(reloaded.getSomeLocalDateTime()).isEqualTo(expectedLocalDateTime);
+        assertThat(reloaded.getSomeLocalDate()).isEqualTo(expectedLocalDate);
+
+        String offsetDateTimeValue = null;
+        String localDateTimeValue = null;
+        String localDateValue = null;
+
+        try (Driver driver = getBoltConnection()) {
+            try (org.neo4j.driver.v1.Session driverSession = driver.session()) {
+                Record record = driverSession
+                    .run("MATCH (n) WHERE id(n) = $id RETURN n", Values.parameters("id", graphId)).single();
+
+                Value n = record.get("n");
+                offsetDateTimeValue = n.get("someTime").asString();
+                localDateTimeValue = n.get("someLocalDateTime").asString();
+                localDateValue = n.get("someLocalDate").asString();
+            }
+        } catch (IllegalStateException e) {
+
+            GraphDatabaseService graphDatabaseService = getGraphDatabaseService();
+            try (Transaction tx = graphDatabaseService.beginTx()) {
+
+                Node node = graphDatabaseService.getNodeById(graphId);
+                offsetDateTimeValue = node.getProperty("someTime").toString();
+                localDateTimeValue = node.getProperty("someLocalDateTime").toString();
+                localDateValue = node.getProperty("someLocalDate").toString();
+            }
+        }
+
+        String expectedStringValue;
+
+        expectedStringValue = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(expectedOffsetDateTime);
+        assertThat(offsetDateTimeValue).isEqualTo(expectedStringValue);
+
+        expectedStringValue = DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(expectedLocalDateTime);
+        assertThat(localDateTimeValue).isEqualTo(expectedStringValue);
+
+        expectedStringValue = DateTimeFormatter.ISO_LOCAL_DATE.format(expectedLocalDate);
+        assertThat(localDateValue).isEqualTo(expectedStringValue);
     }
 }