Skip to content

Commit

Permalink
Fix #66
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Jul 8, 2015
1 parent f65d042 commit 0480206
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 4 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Joda (http://joda-time.sourceforge.net/) data types.
</scm>
<properties>
<version.jackson.annotations>2.6.0-rc3</version.jackson.annotations>
<version.jackson.core>2.6.0-rc3</version.jackson.core>
<version.jackson.core>2.6.0-rc4-SNAPSHOT</version.jackson.core>
<!-- Generate PackageVersion.java into this directory. -->
<packageVersion.dir>com/fasterxml/jackson/datatype/joda</packageVersion.dir>
<packageVersion.package>${project.groupId}.joda</packageVersion.package>
Expand Down
1 change: 1 addition & 0 deletions release-notes/VERSION
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Project: jackson-datatype-joda
#62: Allow use of numbers-as-Strings for LocalDate (in array)
(contributed by Michal Z)
#64: Support `@JsonFormat(pattern=...)` for deserialization
#66: Support `SerializationFeature.WRITE_DATES_WITH_ZONE_ID`

2.5.4 (not yet released)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,45 @@ public ReadableDateTime deserialize(JsonParser p, DeserializationContext ctxt)
if (str.length() == 0) { // [JACKSON-360]
return null;
}
// 08-Jul-2015, tatu: as per [datatype-joda#44], optional TimeZone inclusion
// NOTE: on/off feature only for serialization; on deser should accept both
int ix = str.indexOf('[');
if (ix > 0) {
int ix2 = str.lastIndexOf(']');
String tzId = (ix2 < ix)
? str.substring(ix+1)
: str.substring(ix+1, ix2);
DateTimeZone tz;
try {
tz = DateTimeZone.forID(tzId);
} catch (IllegalArgumentException e) {
throw ctxt.mappingException(String.format("Unknown DateTimeZone id '%s'", tzId));
}
str = str.substring(0, ix);

// One more thing; do we have plain timestamp?
if (_allDigits(str)) {
return new DateTime(Long.parseLong(str), tz);
}
return _format.createParser(ctxt)
.parseDateTime(str)
.withZone(tz);
}

// Not sure if it should use timezone or not...
return _format.createParser(ctxt).parseDateTime(str);
}
throw ctxt.mappingException(handledType());
}

private static boolean _allDigits(String str)
{
for (int i = 0, len = str.length(); i < len; ++i) {
int c = str.charAt(i);
if (c > '9' | c < '0') {
return false;
}
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,29 @@ public boolean isEmpty(SerializerProvider prov, DateTime value) {
@Override
public void serialize(DateTime value, JsonGenerator gen, SerializerProvider provider) throws IOException
{
if (_useTimestamp(provider)) {
gen.writeNumber(value.getMillis());
// First: simple, non-timezone-included output
if (!provider.isEnabled(SerializationFeature.WRITE_DATES_WITH_ZONE_ID)) {
if (_useTimestamp(provider)) {
gen.writeNumber(value.getMillis());
} else {
gen.writeString(_format.createFormatter(provider).print(value));
}
} else {
gen.writeString(_format.createFormatter(provider).print(value));
// and then as per [datatype-joda#44], optional TimeZone inclusion

StringBuilder sb;

if (_useTimestamp(provider)) {
sb = new StringBuilder(20)
.append(value.getMillis());
} else {
sb = new StringBuilder(40)
.append(_format.createFormatter(provider).print(value));
}
sb = sb.append('[')

This comment has been minimized.

Copy link
@zkiss

zkiss Jul 9, 2015

This implementation differs from jsr310's: that one does not write the zone id in case timestamp serialization is used. Which approach is preferable? To me numbers with timezone look really odd, but maybe that is how it should be done and the feature should not be tied to string serialization.

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Jul 9, 2015

Author Member

Good question, I did not consult jsr-310 code, but based this on idea of a separate contribution which only serialized time as timestamp with timezone id (although using slash as separator).
I agree it looks odd, but at the same time there are some benefits.

Another question that is related is whether there is any way to overload @JsonFormat to allow per-property inclusion/exclusion of zone id. Earlier I thought that maybe pattern should contain zone id, if so, but I don't know if date formats actually support this (they do support tz offset, but zone id)?
OTOH, if timezone id inclusion is supported, then there are even more questions as to whether custom pattern/format should be skipped altogether (current code does not make that easy, there's no way to distinguish, but that could be changed).

.append(value.getZone())
.append(']');
gen.writeString(sb.toString());
}
}
}
129 changes: 129 additions & 0 deletions src/test/java/com/fasterxml/jackson/datatype/joda/TimeZoneTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package com.fasterxml.jackson.datatype.joda;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

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

// for [datatype-joda#44]
public class TimeZoneTest extends JodaTestBase
{
// November 3, 2013 at 1:00a is the fall back DST transition that year in much of the US.
private static final int FALL_BACK_YEAR = 2013;

private static final int FALL_BACK_MONTH = 11;

private static final int FALL_BACK_DAY = 3;

// The first one for America/Los_Angeles happens at 8:00 UTC.
private static final int FIRST_FALL_BACK_HOUR = 8;

// And the second one happens at 9:00 UTC
private static final int SECOND_FALL_BACK_HOUR = 9;

private static final DateTimeZone AMERICA_LOS_ANGELES = DateTimeZone.forID("America/Los_Angeles");

private final DateTime DATE_JAN_1_1970_UTC = new DateTime(0L, DateTimeZone.UTC);

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.WRAPPER_ARRAY, property = "@class")
private static interface TypeInfoMixIn {
}

/*
/**********************************************************
/* Test methods
/**********************************************************
*/

private final ObjectMapper MAPPER = jodaMapper();

public void testSimple() throws Exception
{
// First, no zone id included
ObjectWriter w = MAPPER.writer()
.without(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);

assertEquals("0",
w.with(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(DATE_JAN_1_1970_UTC));
assertEquals(quote("1970-01-01T00:00:00.000Z"),
w.without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(DATE_JAN_1_1970_UTC));

// then with zone id

w = w.with(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);
assertEquals(quote("0[UTC]"),
w.with(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(DATE_JAN_1_1970_UTC));
assertEquals(quote("1970-01-01T00:00:00.000Z[UTC]"),
w.without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(DATE_JAN_1_1970_UTC));
}

public void testRoundTrip() throws Exception
{
ObjectWriter w = MAPPER.writer()
.with(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);
DateTime input = new DateTime(2014, 8, 24, 5, 17, 45, DateTimeZone.forID("America/Chicago")); // arbitrary

// First as timestamp

String json = w.with(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(input);
DateTime result = MAPPER.readValue(json, DateTime.class);
assertEquals("Actual timepoints differ", input.getMillis(), result.getMillis());
assertEquals("TimeZones differ", input, result);

// then as regular tet
json = w.without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.writeValueAsString(input);
result = MAPPER.readValue(json, DateTime.class);
assertEquals("Actual timepoints differ", input.getMillis(), result.getMillis());
assertEquals("TimeZones differ", input, result);
}

/**
* Test that de/serializing an ambiguous time (e.g. a 'fall back' DST transition) works and preserves the proper
* instants in time and time zones.
*/
public void testFallBackTransition() throws Exception
{
DateTime firstOneAmUtc = new DateTime(FALL_BACK_YEAR, FALL_BACK_MONTH, FALL_BACK_DAY, FIRST_FALL_BACK_HOUR, 0, 0,
DateTimeZone.UTC);
DateTime secondOneAmUtc = new DateTime(FALL_BACK_YEAR, FALL_BACK_MONTH, FALL_BACK_DAY, SECOND_FALL_BACK_HOUR, 0, 0,
DateTimeZone.UTC);

DateTime firstOneAm = new DateTime(firstOneAmUtc, AMERICA_LOS_ANGELES);
DateTime secondOneAm = new DateTime(secondOneAmUtc, AMERICA_LOS_ANGELES);

ObjectWriter w = MAPPER.writer()
.without(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS)
.with(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);

String firstOneAmStr = w.writeValueAsString(firstOneAm);
String secondOneAmStr = w.writeValueAsString(secondOneAm);

DateTime firstRoundTrip = MAPPER.readValue(firstOneAmStr, DateTime.class);
DateTime secondRoundTrip = MAPPER.readValue(secondOneAmStr, DateTime.class);

assertEquals("Actual timepoints differ", firstOneAm.getMillis(), firstRoundTrip.getMillis());
assertEquals("TimeZones differ", firstOneAm, firstRoundTrip);

assertEquals("Actual timepoints differ", secondOneAm.getMillis(), secondRoundTrip.getMillis());
assertEquals("TimeZones differ", secondOneAm, secondRoundTrip);
}

public void testSerializationWithTypeInfo() throws Exception
{
// but if re-configured to include the time zone
ObjectMapper m = jodaMapper();
m.enable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
m.enable(SerializationFeature.WRITE_DATES_WITH_ZONE_ID);

m.addMixIn(DateTime.class, TypeInfoMixIn.class);
assertEquals("[\"org.joda.time.DateTime\",\"0[UTC]\"]",
m.writeValueAsString(DATE_JAN_1_1970_UTC));
}
}

0 comments on commit 0480206

Please sign in to comment.