Skip to content

Supported parsing date time text with valid zero zone offsets #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 15, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,21 @@ public class InstantDeserializer<T extends Temporal>
{
private static final long serialVersionUID = 1L;

/**
* Constants used to check if the time offset is zero. See [jackson-modules-java8#18]
*
* @since 2.9.0
*/
private static final String ISO8601_UTC_ZERO_OFFSET_SUFFIX_REGEX = "\\+00:?(00)?$";
private static final String ISO8601_UTC_ZERO_OFFSET_REGEX = "[^\\+]+" + ISO8601_UTC_ZERO_OFFSET_SUFFIX_REGEX;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be pre-compiled as Pattern for performance reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, will fix


public static final InstantDeserializer<Instant> INSTANT = new InstantDeserializer<>(
Instant.class, DateTimeFormatter.ISO_INSTANT,
Instant::from,
a -> Instant.ofEpochMilli(a.value),
a -> Instant.ofEpochSecond(a.integer, a.fraction),
null,
true // yes, replace +0000 with Z
true // yes, replace zero offset with Z
);

public static final InstantDeserializer<OffsetDateTime> OFFSET_DATE_TIME = new InstantDeserializer<>(
Expand All @@ -66,7 +74,7 @@ public class InstantDeserializer<T extends Temporal>
a -> OffsetDateTime.ofInstant(Instant.ofEpochMilli(a.value), a.zoneId),
a -> OffsetDateTime.ofInstant(Instant.ofEpochSecond(a.integer, a.fraction), a.zoneId),
(d, z) -> d.withOffsetSameInstant(z.getRules().getOffset(d.toLocalDateTime())),
true // yes, replace +0000 with Z
true // yes, replace zero offset with Z
);

public static final InstantDeserializer<ZonedDateTime> ZONED_DATE_TIME = new InstantDeserializer<>(
Expand All @@ -75,7 +83,7 @@ public class InstantDeserializer<T extends Temporal>
a -> ZonedDateTime.ofInstant(Instant.ofEpochMilli(a.value), a.zoneId),
a -> ZonedDateTime.ofInstant(Instant.ofEpochSecond(a.integer, a.fraction), a.zoneId),
ZonedDateTime::withZoneSameInstant,
false // keep +0000 and Z separate since zones explicitly supported
false // keep zero offset and Z separate since zones explicitly supported
);

protected final Function<FromIntegerArguments, T> fromMilliseconds;
Expand All @@ -87,13 +95,13 @@ public class InstantDeserializer<T extends Temporal>
protected final BiFunction<T, ZoneId, T> adjust;

/**
* In case of vanilla `Instant` we seem to need to translate "+0000"
* In case of vanilla `Instant` we seem to need to translate "+0000 | +00:00 | +00"
* timezone designator into plain "Z" for some reason; see
* [datatype-jsr310#79] for more info
* [jackson-modules-java8#18] for more info
*
* @since 2.7.5
* @since 2.9.0
*/
protected final boolean replace0000AsZ;
protected final boolean replaceZeroOffsetAsZ;

/**
* Flag for <code>JsonFormat.Feature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE</code>
Expand All @@ -108,14 +116,14 @@ protected InstantDeserializer(Class<T> supportedType,
Function<FromIntegerArguments, T> fromMilliseconds,
Function<FromDecimalArguments, T> fromNanoseconds,
BiFunction<T, ZoneId, T> adjust,
boolean replace0000AsZ)
boolean replaceZeroOffsetAsZ)
{
super(supportedType, formatter);
this.parsedToValue = parsedToValue;
this.fromMilliseconds = fromMilliseconds;
this.fromNanoseconds = fromNanoseconds;
this.adjust = adjust == null ? ((d, z) -> d) : adjust;
this.replace0000AsZ = replace0000AsZ;
this.replaceZeroOffsetAsZ = replaceZeroOffsetAsZ;
_adjustToContextTZOverride = null;
}

Expand All @@ -127,7 +135,7 @@ protected InstantDeserializer(InstantDeserializer<T> base, DateTimeFormatter f)
fromMilliseconds = base.fromMilliseconds;
fromNanoseconds = base.fromNanoseconds;
adjust = base.adjust;
replace0000AsZ = (_formatter == DateTimeFormatter.ISO_INSTANT);
replaceZeroOffsetAsZ = (_formatter == DateTimeFormatter.ISO_INSTANT);
_adjustToContextTZOverride = base._adjustToContextTZOverride;
}

Expand All @@ -139,7 +147,7 @@ protected InstantDeserializer(InstantDeserializer<T> base, Boolean adjustToConte
fromMilliseconds = base.fromMilliseconds;
fromNanoseconds = base.fromNanoseconds;
adjust = base.adjust;
replace0000AsZ = base.replace0000AsZ;
replaceZeroOffsetAsZ = base.replaceZeroOffsetAsZ;
_adjustToContextTZOverride = adjustToContextTimezoneOverride;
}

Expand Down Expand Up @@ -189,13 +197,8 @@ public T deserialize(JsonParser parser, DeserializationContext context) throws I
// fall through to default handling, to get error there
}
}
// 24-May-2016, tatu: as per [datatype-jsr310#79] seems like we need
// some massaging in some cases...
if (replace0000AsZ) {
if (string.endsWith("+0000")) {
string = string.substring(0, string.length() - 5) + "Z";
}
}

string = replaceZeroOffsetAsZIfNecessary(string);
}

T value;
Expand Down Expand Up @@ -285,6 +288,20 @@ private ZoneId getZone(DeserializationContext context)
return (_valueClass == Instant.class) ? null : context.getTimeZone().toZoneId();
}

private String replaceZeroOffsetAsZIfNecessary(String text)
{
if (replaceZeroOffsetAsZ && endsWithZeroOffset(text)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to evaluate regexp twice, first to match, then to replace: would it be possible to make that just one operation? Regexp evaluation has non-trivial cost, even considering that textual date handling itself is rather expensive operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment @cowtowncoder, I just checked the doc, your point makes perfect sense, I was wrong, I remember if no match found that would throw an exception. will fix

return text.replaceFirst(ISO8601_UTC_ZERO_OFFSET_SUFFIX_REGEX, "Z");
}

return text;
}

private boolean endsWithZeroOffset(String text)
{
return text.matches(ISO8601_UTC_ZERO_OFFSET_REGEX);
}

public static class FromIntegerArguments // since 2.8.3
{
public final long value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@

package com.fasterxml.jackson.datatype.jsr310;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import org.junit.Test;

import java.time.Instant;
import java.time.LocalDate;
Expand All @@ -29,10 +31,9 @@
import java.time.temporal.ChronoUnit;
import java.time.temporal.Temporal;

import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.databind.*;

import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

public class TestInstantSerialization extends ModuleTestBase
{
Expand Down Expand Up @@ -443,4 +444,33 @@ public void testRoundTripOfInstantAndJavaUtilDate() throws Exception

assertEquals(givenInstant, actual);
}

// [jackson-modules-java8#18]
@Test
public void testDeserializationFromStringWithZeroZoneOffset01() throws Exception {
Instant date = Instant.now();
String json = formatWithZeroZoneOffset(date, "+00:00");
Instant result = MAPPER.readValue(json, Instant.class);
assertEquals("The value is not correct.", date, result);
}

@Test
public void testDeserializationFromStringWithZeroZoneOffset02() throws Exception {
Instant date = Instant.now();
String json = formatWithZeroZoneOffset(date, "+0000");
Instant result = MAPPER.readValue(json, Instant.class);
assertEquals("The value is not correct.", date, result);
}

@Test
public void testDeserializationFromStringWithZeroZoneOffset03() throws Exception {
Instant date = Instant.now();
String json = formatWithZeroZoneOffset(date, "+00");
Instant result = MAPPER.readValue(json, Instant.class);
assertEquals("The value is not correct.", date, result);
}

private String formatWithZeroZoneOffset(Instant date, String offset){
return '"' + FORMATTER.format(date).replaceFirst("Z$", offset) + '"';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ public void testBadDeserializationAsString01() throws Throwable
expectFailure("'notanoffsetdatetime'");
}

@Test
public void testDeserializationAsWithZeroZoneOffset01() throws Exception
{
expectSuccess(OffsetDateTime.of(2000, 1, 1, 12, 0, 0, 0, ZoneOffset.UTC), "'2000-01-01T12:00+00:00'");
}

@Test
public void testDeserializationAsWithZeroZoneOffset02() throws Exception
{
expectSuccess(OffsetDateTime.of(2000, 1, 1, 12, 0, 0, 0, ZoneOffset.UTC), "'2000-01-01T12:00+0000'");
}

@Test
public void testDeserializationAsWithZeroZoneOffset03() throws Exception
{
expectSuccess(OffsetDateTime.of(2000, 1, 1, 12, 0, 0, 0, ZoneOffset.UTC), "'2000-01-01T12:00+00'");
}

private void expectFailure(String json) throws Exception {
try {
read(json);
Expand Down