Skip to content

Commit c8c61c0

Browse files
injectivesinjectives
committed
Defer unknown zone information failures to datetime value access stage
If server supplies zone information that is not supported by driver runtime, it will be represented by an unknown datetime value that fails on datetime access. This prevents an immediate failure during data consumption and allows usage of other values.
1 parent 6c3c1c7 commit c8c61c0

File tree

6 files changed

+236
-39
lines changed

6 files changed

+236
-39
lines changed

‎driver/src/main/java/org/neo4j/driver/Value.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.neo4j.driver;
2020

21+
import java.time.DateTimeException;
2122
import java.time.LocalDate;
2223
import java.time.LocalDateTime;
2324
import java.time.LocalTime;
@@ -197,7 +198,8 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue {
197198
* 64-bit precision. This is why these types return java {@link Long} and
198199
* {@link Double}, respectively.
199200
*
200-
* @return the value as a Java Object
201+
* @return the value as a Java Object.
202+
* @throws DateTimeException if zone information supplied by server is not supported by driver runtime. Applicable to datetime values only.
201203
*/
202204
Object asObject();
203205

@@ -416,12 +418,14 @@ public interface Value extends MapAccessor, MapAccessorWithDefaultValue {
416418
/**
417419
* @return the value as a {@link java.time.OffsetDateTime}, if possible.
418420
* @throws Uncoercible if value types are incompatible.
421+
* @throws DateTimeException if zone information supplied by server is not supported by driver runtime.
419422
*/
420423
OffsetDateTime asOffsetDateTime();
421424

422425
/**
423426
* @return the value as a {@link ZonedDateTime}, if possible.
424427
* @throws Uncoercible if value types are incompatible.
428+
* @throws DateTimeException if zone information supplied by server is not supported by driver runtime.
425429
*/
426430
ZonedDateTime asZonedDateTime();
427431

‎driver/src/main/java/org/neo4j/driver/internal/messaging/common/CommonValueUnpacker.java

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.neo4j.driver.Values.value;
2525

2626
import java.io.IOException;
27+
import java.time.DateTimeException;
2728
import java.time.Instant;
2829
import java.time.LocalDate;
2930
import java.time.LocalDateTime;
@@ -37,6 +38,7 @@
3738
import java.util.Collections;
3839
import java.util.List;
3940
import java.util.Map;
41+
import java.util.function.Supplier;
4042
import org.neo4j.driver.Value;
4143
import org.neo4j.driver.exceptions.ClientException;
4244
import org.neo4j.driver.exceptions.ProtocolException;
@@ -54,6 +56,7 @@
5456
import org.neo4j.driver.internal.value.NodeValue;
5557
import org.neo4j.driver.internal.value.PathValue;
5658
import org.neo4j.driver.internal.value.RelationshipValue;
59+
import org.neo4j.driver.internal.value.UnsupportedDateTimeValue;
5760
import org.neo4j.driver.types.Node;
5861
import org.neo4j.driver.types.Path;
5962
import org.neo4j.driver.types.Relationship;
@@ -189,28 +192,28 @@ private Value unpackStruct(long size, byte type) throws IOException {
189192
case DATE_TIME_WITH_ZONE_OFFSET:
190193
if (!dateTimeUtcEnabled) {
191194
ensureCorrectStructSize(TypeConstructor.DATE_TIME, DATE_TIME_STRUCT_SIZE, size);
192-
return unpackDateTimeWithZoneOffset();
195+
return unpackDateTime(true, false);
193196
} else {
194197
throw instantiateExceptionForUnknownType(type);
195198
}
196199
case DATE_TIME_WITH_ZONE_OFFSET_UTC:
197200
if (dateTimeUtcEnabled) {
198201
ensureCorrectStructSize(TypeConstructor.DATE_TIME, DATE_TIME_STRUCT_SIZE, size);
199-
return unpackDateTimeUtcWithZoneOffset();
202+
return unpackDateTime(true, true);
200203
} else {
201204
throw instantiateExceptionForUnknownType(type);
202205
}
203206
case DATE_TIME_WITH_ZONE_ID:
204207
if (!dateTimeUtcEnabled) {
205208
ensureCorrectStructSize(TypeConstructor.DATE_TIME, DATE_TIME_STRUCT_SIZE, size);
206-
return unpackDateTimeWithZoneId();
209+
return unpackDateTime(false, false);
207210
} else {
208211
throw instantiateExceptionForUnknownType(type);
209212
}
210213
case DATE_TIME_WITH_ZONE_ID_UTC:
211214
if (dateTimeUtcEnabled) {
212215
ensureCorrectStructSize(TypeConstructor.DATE_TIME, DATE_TIME_STRUCT_SIZE, size);
213-
return unpackDateTimeUtcWithZoneId();
216+
return unpackDateTime(false, true);
214217
} else {
215218
throw instantiateExceptionForUnknownType(type);
216219
}
@@ -374,34 +377,26 @@ private Value unpackLocalDateTime() throws IOException {
374377
return value(LocalDateTime.ofEpochSecond(epochSecondUtc, nano, UTC));
375378
}
376379

377-
private Value unpackDateTimeWithZoneOffset() throws IOException {
378-
long epochSecondLocal = unpacker.unpackLong();
379-
int nano = Math.toIntExact(unpacker.unpackLong());
380-
int offsetSeconds = Math.toIntExact(unpacker.unpackLong());
381-
return value(newZonedDateTime(epochSecondLocal, nano, ZoneOffset.ofTotalSeconds(offsetSeconds)));
382-
}
383-
384-
private Value unpackDateTimeUtcWithZoneOffset() throws IOException {
385-
long epochSecondLocal = unpacker.unpackLong();
386-
int nano = Math.toIntExact(unpacker.unpackLong());
387-
int offsetSeconds = Math.toIntExact(unpacker.unpackLong());
388-
ZoneOffset offset = ZoneOffset.ofTotalSeconds(offsetSeconds);
389-
return value(newZonedDateTimeUsingUtcBaseline(epochSecondLocal, nano, offset));
390-
}
391-
392-
private Value unpackDateTimeWithZoneId() throws IOException {
393-
long epochSecondLocal = unpacker.unpackLong();
394-
int nano = Math.toIntExact(unpacker.unpackLong());
395-
String zoneIdString = unpacker.unpackString();
396-
return value(newZonedDateTime(epochSecondLocal, nano, ZoneId.of(zoneIdString)));
397-
}
398-
399-
private Value unpackDateTimeUtcWithZoneId() throws IOException {
400-
long epochSecondLocal = unpacker.unpackLong();
401-
int nano = Math.toIntExact(unpacker.unpackLong());
402-
String zoneIdString = unpacker.unpackString();
403-
ZoneId zoneId = ZoneId.of(zoneIdString);
404-
return value(newZonedDateTimeUsingUtcBaseline(epochSecondLocal, nano, zoneId));
380+
private Value unpackDateTime(boolean unpackOffset, boolean useUtcBaseline) throws IOException {
381+
var epochSecondLocal = unpacker.unpackLong();
382+
var nano = Math.toIntExact(unpacker.unpackLong());
383+
Supplier<ZoneId> zoneIdSupplier;
384+
if (unpackOffset) {
385+
var offsetSeconds = Math.toIntExact(unpacker.unpackLong());
386+
zoneIdSupplier = () -> ZoneOffset.ofTotalSeconds(offsetSeconds);
387+
} else {
388+
var zoneIdString = unpacker.unpackString();
389+
zoneIdSupplier = () -> ZoneId.of(zoneIdString);
390+
}
391+
ZoneId zoneId;
392+
try {
393+
zoneId = zoneIdSupplier.get();
394+
} catch (DateTimeException e) {
395+
return new UnsupportedDateTimeValue(e);
396+
}
397+
return useUtcBaseline
398+
? value(newZonedDateTimeUsingUtcBaseline(epochSecondLocal, nano, zoneId))
399+
: value(newZonedDateTime(epochSecondLocal, nano, zoneId));
405400
}
406401

407402
private Value unpackDuration() throws IOException {

‎driver/src/main/java/org/neo4j/driver/internal/value/UnsupportedDateTimeValue.java

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal.value;
20+
21+
import java.lang.reflect.InvocationTargetException;
22+
import java.time.DateTimeException;
23+
import java.time.OffsetDateTime;
24+
import java.time.ZonedDateTime;
25+
import org.neo4j.driver.internal.types.InternalTypeSystem;
26+
import org.neo4j.driver.types.Type;
27+
28+
public class UnsupportedDateTimeValue extends ValueAdapter {
29+
final DateTimeException exception;
30+
31+
public UnsupportedDateTimeValue(DateTimeException exception) {
32+
this.exception = exception;
33+
}
34+
35+
@Override
36+
public OffsetDateTime asOffsetDateTime() {
37+
throw instantiateDateTimeException();
38+
}
39+
40+
@Override
41+
public ZonedDateTime asZonedDateTime() {
42+
throw instantiateDateTimeException();
43+
}
44+
45+
@Override
46+
public Object asObject() {
47+
throw instantiateDateTimeException();
48+
}
49+
50+
@Override
51+
public Type type() {
52+
return InternalTypeSystem.TYPE_SYSTEM.DATE_TIME();
53+
}
54+
55+
@Override
56+
public boolean equals(Object obj) {
57+
return this == obj;
58+
}
59+
60+
@Override
61+
public int hashCode() {
62+
return System.identityHashCode(this);
63+
}
64+
65+
@Override
66+
public String toString() {
67+
return "Unsupported datetime value.";
68+
}
69+
70+
private DateTimeException instantiateDateTimeException() {
71+
DateTimeException newException;
72+
try {
73+
newException = exception
74+
.getClass()
75+
.getDeclaredConstructor(String.class, Throwable.class)
76+
.newInstance(exception.getMessage(), exception);
77+
} catch (NoSuchMethodException
78+
| InvocationTargetException
79+
| InstantiationException
80+
| IllegalAccessException e) {
81+
newException = new DateTimeException(exception.getMessage(), exception);
82+
}
83+
return newException;
84+
}
85+
}

‎driver/src/test/java/org/neo4j/driver/internal/value/UnsupportedDateTimeValueTest.java

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal.value;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
23+
import static org.junit.jupiter.api.Assertions.assertThrows;
24+
import static org.mockito.BDDMockito.given;
25+
import static org.mockito.MockitoAnnotations.openMocks;
26+
27+
import java.time.DateTimeException;
28+
import java.time.OffsetDateTime;
29+
import java.time.ZonedDateTime;
30+
import java.util.List;
31+
import java.util.function.Function;
32+
import org.junit.jupiter.api.BeforeEach;
33+
import org.junit.jupiter.api.Named;
34+
import org.junit.jupiter.api.Test;
35+
import org.junit.jupiter.params.ParameterizedTest;
36+
import org.junit.jupiter.params.provider.Arguments;
37+
import org.junit.jupiter.params.provider.MethodSource;
38+
import org.mockito.Mock;
39+
import org.neo4j.driver.internal.types.InternalTypeSystem;
40+
41+
public class UnsupportedDateTimeValueTest {
42+
@Mock
43+
private DateTimeException exception;
44+
45+
@BeforeEach
46+
void beforeEach() {
47+
openMocks(this);
48+
}
49+
50+
@MethodSource("throwingDateTimeAccessMethods")
51+
@ParameterizedTest
52+
void shouldThrowOnDateTimeAccess(Function<UnsupportedDateTimeValue, ?> throwingMethod) {
53+
// GIVEN
54+
given(exception.getMessage()).willReturn("message");
55+
var value = new UnsupportedDateTimeValue(exception);
56+
57+
// WHEN
58+
var actualException = assertThrows(DateTimeException.class, () -> throwingMethod.apply(value));
59+
60+
// THEN
61+
assertEquals(actualException.getMessage(), exception.getMessage());
62+
assertEquals(actualException.getCause(), exception);
63+
}
64+
65+
static List<Arguments> throwingDateTimeAccessMethods() {
66+
return List.of(
67+
Arguments.of(Named.<Function<UnsupportedDateTimeValue, ?>>of(
68+
"asOffsetDateTime", UnsupportedDateTimeValue::asOffsetDateTime)),
69+
Arguments.of(Named.<Function<UnsupportedDateTimeValue, ?>>of(
70+
"asOffsetDateTime(OffsetDateTime)", v -> v.asOffsetDateTime(OffsetDateTime.now()))),
71+
Arguments.of(Named.<Function<UnsupportedDateTimeValue, ?>>of(
72+
"asZonedDateTime", UnsupportedDateTimeValue::asZonedDateTime)),
73+
Arguments.of(Named.<Function<UnsupportedDateTimeValue, ?>>of(
74+
"asZonedDateTime(ZonedDateTime)", v -> v.asZonedDateTime(ZonedDateTime.now()))),
75+
Arguments.of(Named.<Function<UnsupportedDateTimeValue, ?>>of(
76+
"asObject", UnsupportedDateTimeValue::asObject)));
77+
}
78+
79+
@Test
80+
void shouldEqualToItself() {
81+
// GIVEN
82+
var value = new UnsupportedDateTimeValue(exception);
83+
84+
// WHEN & THEN
85+
assertEquals(value, value);
86+
}
87+
88+
@Test
89+
void shouldNotEqualToAnotherInstance() {
90+
// GIVEN
91+
var value0 = new UnsupportedDateTimeValue(exception);
92+
var value1 = new UnsupportedDateTimeValue(exception);
93+
94+
// WHEN & THEN
95+
assertNotEquals(value0, value1);
96+
}
97+
98+
@Test
99+
void shouldSupplyIdentityHashcode() {
100+
// GIVEN
101+
var value0 = new UnsupportedDateTimeValue(exception);
102+
var value1 = new UnsupportedDateTimeValue(exception);
103+
104+
// WHEN & THEN
105+
assertNotEquals(value0.hashCode(), value1.hashCode());
106+
}
107+
108+
@Test
109+
void shouldSupplyDateTimeType() {
110+
// GIVEN
111+
var value = new UnsupportedDateTimeValue(exception);
112+
113+
// WHEN & THEN
114+
assertEquals(InternalTypeSystem.TYPE_SYSTEM.DATE_TIME(), value.type());
115+
}
116+
}

‎testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/ResultNext.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,17 @@
2222
import lombok.Setter;
2323
import neo4j.org.testkit.backend.messages.AbstractResultNext;
2424
import org.neo4j.driver.Record;
25+
import org.neo4j.driver.Value;
2526

2627
@Setter
2728
@Getter
2829
public class ResultNext extends AbstractResultNext {
30+
private static final String DATE_TIME = "DATE_TIME";
2931
private ResultNextBody data;
3032

3133
@Override
3234
protected neo4j.org.testkit.backend.messages.responses.TestkitResponse createResponse(Record record) {
35+
record.values().stream().filter(v -> DATE_TIME.equals(v.type().name())).forEach(Value::asObject);
3336
return neo4j.org.testkit.backend.messages.responses.Record.builder()
3437
.data(neo4j.org.testkit.backend.messages.responses.Record.RecordBody.builder()
3538
.values(record)

‎testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/StartTest.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ public class StartTest implements TestkitRequest {
8383
"^.*\\.TestOptimizations\\.test_uses_implicit_default_arguments_multi_query$", skipMessage);
8484
COMMON_SKIP_PATTERN_TO_REASON.put(
8585
"^.*\\.TestOptimizations\\.test_uses_implicit_default_arguments_multi_query_nested$", skipMessage);
86-
COMMON_SKIP_PATTERN_TO_REASON.put(
87-
"^.*\\.test_unknown_then_known_zoned_date_time(_patched)?$",
88-
"Unknown zone names make the driver close the connection.");
89-
COMMON_SKIP_PATTERN_TO_REASON.put(
90-
"^.*\\.test_unknown_zoned_date_time(_patched)?$",
91-
"Unknown zone names make the driver close the connection.");
9286

9387
ASYNC_SKIP_PATTERN_TO_REASON.putAll(COMMON_SKIP_PATTERN_TO_REASON);
9488

0 commit comments

Comments
 (0)