Skip to content

Commit

Permalink
Fixes for FasterXML#1661
Browse files Browse the repository at this point in the history
In summary, the custom DateFormat always has its TimeZone forced to whatever `Settings.getTimeZone()` will return.
Provide an additional fix in `withTimeZone` to handle the case where `_dateFormat` is null (since this is allowed by `withDateFormat`).
Update test case to reflect the fixed behavior.
  • Loading branch information
brenuart committed Jun 16, 2017
1 parent dc2c003 commit f6b2a99
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 26 deletions.
46 changes: 39 additions & 7 deletions src/main/java/com/fasterxml/jackson/databind/cfg/BaseSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -238,14 +238,24 @@ public BaseSettings withTypeResolverBuilder(TypeResolverBuilder<?> typer) {
_timeZone, _defaultBase64);
}

/**
* Set the {@link DateFormat} instance to use with this settings.
* <p>
* Note that the TimeZone value originally set on the DateFormat will be ignored and
* reset reset to use the value configured on this settings object.
* The locale will remains unchanged however.
*
* @param df the {@link DateFormat} instance to use
* @return a new instance of the settings with the DateFormat set to the desired value
*/
public BaseSettings withDateFormat(DateFormat df) {
if (_dateFormat == df) {
return this;
}
// 26-Sep-2015, tatu: Related to [databind#939], let's try to force TimeZone if
// (but only if!) it has been set explicitly.
if ((df != null) && hasExplicitTimeZone()) {
df = _force(df, _timeZone);

// Configure the DateFormat with the TimeZone mandated by this settings
if (df != null ) {
df = _force(df, getTimeZone());
}
return new BaseSettings(_classIntrospector, _annotationIntrospector, _visibilityChecker, _propertyNamingStrategy, _typeFactory,
_typeResolverBuilder, df, _handlerInstantiator, _locale,
Expand Down Expand Up @@ -284,7 +294,11 @@ public BaseSettings with(TimeZone tz)
return this;
}

DateFormat df = _force(_dateFormat, tz);
// Reconfigure the DateFormat with the new TimeZone
DateFormat df = _dateFormat;
if( df != null ) {
df = _force(_dateFormat, tz);
}
return new BaseSettings(_classIntrospector, _annotationIntrospector,
_visibilityChecker, _propertyNamingStrategy, _typeFactory,
_typeResolverBuilder, df, _handlerInstantiator, _locale,
Expand Down Expand Up @@ -346,16 +360,27 @@ public Locale getLocale() {
return _locale;
}

/**
* Get the {@link TimeZone} explicitly set on this settings object or the
* default value otherwise.
*
* @return the {@link TimeZone} to use according to this settings object (not null).
*
* @see #with(TimeZone)
* @see #hasExplicitTimeZone()
* @see #DEFAULT_TIMEZONE
*/
public TimeZone getTimeZone() {
TimeZone tz = _timeZone;
return (tz == null) ? DEFAULT_TIMEZONE : tz;
}

/**
* Accessor that may be called to determine whether this settings object
* has been explicitly configured with a TimeZone (true), or is still
* relying on the default settings (false).
* has been explicitly configured with a TimeZone {@code true}, or is still
* relying on the default settings {@code false}.
*
* @return {@code true} if a TimeZone has been explicitly configured
* @since 2.7
*/
public boolean hasExplicitTimeZone() {
Expand All @@ -372,6 +397,13 @@ public Base64Variant getBase64Variant() {
/**********************************************************
*/

/**
* Set the timeZone on the given DateFormat
*
* @param df the DateFormat instance to configure
* @param tz the TimeZone to set on the DateFormat
* @return a potentially new instance of the DateFormat with the TimeZone set to the desired value
*/
private DateFormat _force(DateFormat df, TimeZone tz)
{
if (df instanceof StdDateFormat) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,23 +439,28 @@ public void testDateUtil_Annotation_TimeZone() throws Exception
*/
public void testDateUtil_customDateFormat_withoutTZ() throws Exception
{
// FIXME
//
// The general rule with the StdDateFormat is:
// the TimeZone of the ObjectMapper is used if the JSON doesn't hold
// any timezone/offset information.
// The general rule is:
// the TimeZone of the ObjectMapper is used when parsing dates if the JSON
// doesn't contain any timezone/offset information.
//
// This rule remains valid with the @JsonFormat annotation unless it forces
// an explicit timezeone, in which case the latter takes precedence.
//
// One would expect the same behavior when the StdDateFormat is replaced by a
// custom DateFormat on the ObjectMapper. In other words, the timezone of the
// DateFormat is of no importance: the ObjectMapper's default should be used
// whenever it is needed.
// an explicit timezone, in which case the latter takes precedence.


// Test first with a vanilla ObjectMapper (defaults to UTC)
{
DateFormat df = new SimpleDateFormat("yyyy-MM-dd'X'HH:mm:ss");
df.setTimeZone( TimeZone.getTimeZone("GMT+4") ); // TZ different from mapper's default

ObjectMapper mapper = new ObjectMapper();
mapper.setDateFormat(df);
// Note it is important NOT TO CALL mapper.setTimeZone(...) in this test..

// The mapper's default TZ is used...
verify(mapper, "2000-01-02X04:00:00", judate(2000, 1, 2, 4, 00, 00, 00, "UTC"));
}

// Test first with a non default TZ on the ObjectMapper
// --> OK: the mapper's default TZ is used to parse the date.
// Test with a non default TZ on the ObjectMapper
{
DateFormat df = new SimpleDateFormat("yyyy-MM-dd'X'HH:mm:ss");
df.setTimeZone( TimeZone.getTimeZone("GMT+4") ); // TZ different from mapper's default
Expand All @@ -468,19 +473,18 @@ public void testDateUtil_customDateFormat_withoutTZ() throws Exception
verify(mapper, "2000-01-02X04:00:00", judate(2000, 1, 2, 4, 00, 00, 00, LOCAL_TZ));
}

// Test a second time with the default TZ on the ObjectMapper
// Note it is important NOT TO CALL mapper.setTimeZone(...) in this test..
// --> KO: the custom format's TZ is used instead of the mapper's default as above.
//
// Same test as above but set the DateFormat before the TimeZone (make sure order of
// settings doesn't matter)
{
DateFormat df = new SimpleDateFormat("yyyy-MM-dd'X'HH:mm:ss");
df.setTimeZone( TimeZone.getTimeZone("GMT+4") ); // TZ different from mapper's default

ObjectMapper mapper = new ObjectMapper();
mapper.setDateFormat(df);
mapper.setTimeZone( TimeZone.getTimeZone(LOCAL_TZ) );

// FIXME mapper's default TZ should have been used
verify(mapper, "2000-01-02X04:00:00", judate(2000, 1, 2, 4, 00, 00, 00, "GMT+4"));
// The mapper's default TZ is used...
verify(mapper, "2000-01-02X04:00:00", judate(2000, 1, 2, 4, 00, 00, 00, LOCAL_TZ));
}
}

Expand Down

0 comments on commit f6b2a99

Please sign in to comment.