Refactor DateTimeFormatterFactory

Refactor DateTimeFormatterFactory into two distinct classes; a general
purpose factory and a specialized FactoryBean.  These changes are
modeled after the existing VelocityEngineFactory and
VelocityEngineFactoryBean classes.

Issue: SPR-9959

spring-context/src/main/java/org/springframework/format/datetime/joda/DateTimeFormatterFactory.java

@@ -22,23 +22,25 @@ import org.joda.time.DateTimeZone;
 import org.joda.time.format.DateTimeFormat;
 import org.joda.time.format.DateTimeFormatter;
 import org.joda.time.format.ISODateTimeFormat;
-
-import org.springframework.beans.factory.FactoryBean;
 import org.springframework.format.annotation.DateTimeFormat.ISO;
 import org.springframework.util.StringUtils;
 
 /**
- * {@link FactoryBean} that creates a Joda {@link DateTimeFormatter}. Formatters will be
+ * Factory that creates a Joda {@link DateTimeFormatter}. Formatters will be
  * created using the defined {@link #setPattern(String) pattern}, {@link #setIso(ISO) ISO},
  * or {@link #setStyle(String) style} (considered in that order).
  *
  * @author Phillip Webb
  * @author Sam Brannen
- * @see #getDateTimeFormatter()
+ * @see #createDateTimeFormatter()
- * @see #getDateTimeFormatter(DateTimeFormatter)
+ * @see #createDateTimeFormatter(DateTimeFormatter)
+ * @see #setPattern(String)
+ * @see #setIso(org.springframework.format.annotation.DateTimeFormat.ISO)
+ * @see #setStyle(String)
+ * @see DateTimeFormatterFactoryBean
  * @since 3.2
  */
-public class DateTimeFormatterFactory implements FactoryBean<DateTimeFormatter> {
+public class DateTimeFormatterFactory {
 
 	private ISO iso;
 
@@ -64,33 +66,21 @@ public class DateTimeFormatterFactory implements FactoryBean<DateTimeFormatter>
 	}
 
 
-	public boolean isSingleton() {
-		return true;
-	}
-
-	public Class<?> getObjectType() {
-		return DateTimeFormatter.class;
-	}
-
-	public DateTimeFormatter getObject() throws Exception {
-		return getDateTimeFormatter();
-	}
-
 	/**
-	 * Get a new {@code DateTimeFormatter} using this factory. If no specific
+	 * Create a new {@code DateTimeFormatter} using this factory. If no specific
 	 * {@link #setStyle(String) style}, {@link #setIso(ISO) ISO}, or
 	 * {@link #setPattern(String) pattern} have been defined the
 	 * {@link DateTimeFormat#mediumDateTime() medium date time format} will be used.
 	 * @return a new date time formatter
 	 * @see #getObject()
-	 * @see #getDateTimeFormatter(DateTimeFormatter)
+	 * @see #createDateTimeFormatter(DateTimeFormatter)
 	 */
-	public DateTimeFormatter getDateTimeFormatter() {
+	public DateTimeFormatter createDateTimeFormatter() {
-		return getDateTimeFormatter(DateTimeFormat.mediumDateTime());
+		return createDateTimeFormatter(DateTimeFormat.mediumDateTime());
 	}
 
 	/**
-	 * Get a new {@code DateTimeFormatter} using this factory. If no specific
+	 * Create a new {@code DateTimeFormatter} using this factory. If no specific
 	 * {@link #setStyle(String) style}, {@link #setIso(ISO) ISO}, or
 	 * {@link #setPattern(String) pattern} have been defined the supplied
 	 * {@code fallbackFormatter} will be used.
@@ -98,34 +88,32 @@ public class DateTimeFormatterFactory implements FactoryBean<DateTimeFormatter>
 	 *        properties have been set (can be {@code null}).
 	 * @return a new date time formatter
 	 */
-	public DateTimeFormatter getDateTimeFormatter(DateTimeFormatter fallbackFormatter) {
+	public DateTimeFormatter createDateTimeFormatter(DateTimeFormatter fallbackFormatter) {
-		DateTimeFormatter dateTimeFormatter = createDateTimeFormatter();
+		DateTimeFormatter dateTimeFormatter = null;
-		if(dateTimeFormatter != null && this.timeZone != null) {
+		if (StringUtils.hasLength(pattern)) {
+			dateTimeFormatter = DateTimeFormat.forPattern(pattern);
+		}
+		else if (iso != null && iso != ISO.NONE) {
+			if (iso == ISO.DATE) {
+				dateTimeFormatter = ISODateTimeFormat.date();
+			}
+			else if (iso == ISO.TIME) {
+				dateTimeFormatter = ISODateTimeFormat.time();
+			}
+			else {
+				dateTimeFormatter = ISODateTimeFormat.dateTime();
+			}
+		}
+		else if (StringUtils.hasLength(style)) {
+			dateTimeFormatter = DateTimeFormat.forStyle(style);
+		}
+
+		if (dateTimeFormatter != null && this.timeZone != null) {
 			dateTimeFormatter = dateTimeFormatter.withZone(DateTimeZone.forTimeZone(this.timeZone));
 		}
 		return (dateTimeFormatter != null ? dateTimeFormatter : fallbackFormatter);
 	}
 
-	private DateTimeFormatter createDateTimeFormatter() {
-		if (StringUtils.hasLength(pattern)) {
-			return DateTimeFormat.forPattern(pattern);
-		}
-		if (iso != null && iso != ISO.NONE) {
-			if (iso == ISO.DATE) {
-				return ISODateTimeFormat.date();
-			}
-			if (iso == ISO.TIME) {
-				return ISODateTimeFormat.time();
-			}
-			return ISODateTimeFormat.dateTime();
-		}
-		if (StringUtils.hasLength(style)) {
-			return DateTimeFormat.forStyle(style);
-		}
-		return null;
-	}
-
-
 	/**
 	 * Set the {@code TimeZone} to normalize the date values into, if any.
 	 * @param timeZone the time zone
@@ -166,5 +154,4 @@ public class DateTimeFormatterFactory implements FactoryBean<DateTimeFormatter>
 	public void setPattern(String pattern) {
 		this.pattern = pattern;
 	}
-
 }

spring-context/src/main/java/org/springframework/format/datetime/joda/DateTimeFormatterFactoryBean.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * 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.springframework.format.datetime.joda;
+
+import org.joda.time.format.DateTimeFormatter;
+import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.InitializingBean;
+
+/**
+ * {@link FactoryBean} that creates a Joda {@link DateTimeFormatter}. See the base class
+ * {@linkplain DateTimeFormatterFactory} for configuration details.
+ *
+ * @author Phillip Webb
+ * @author Sam Brannen
+ * @see #setPattern(String)
+ * @see #setIso(org.springframework.format.annotation.DateTimeFormat.ISO)
+ * @see #setStyle(String)
+ * @see DateTimeFormatterFactory
+ * @since 3.2
+ */
+public class DateTimeFormatterFactoryBean extends DateTimeFormatterFactory implements
+		FactoryBean<DateTimeFormatter>, InitializingBean {
+
+	private DateTimeFormatter dateTimeFormatter;
+
+
+	public void afterPropertiesSet() throws Exception {
+		this.dateTimeFormatter = createDateTimeFormatter();
+	}
+
+	public DateTimeFormatter getObject() throws Exception {
+		return this.dateTimeFormatter;
+	}
+
+	public Class<?> getObjectType() {
+		return DateTimeFormatter.class;
+	}
+
+	public boolean isSingleton() {
+		return true;
+	}
+}

spring-context/src/main/java/org/springframework/format/datetime/joda/JodaDateTimeFormatAnnotationFormatterFactory.java

@@ -119,6 +119,6 @@ public class JodaDateTimeFormatAnnotationFormatterFactory
 		factory.setStyle(resolveEmbeddedValue(annotation.style()));
 		factory.setIso(annotation.iso());
 		factory.setPattern(resolveEmbeddedValue(annotation.pattern()));
-		return factory.getDateTimeFormatter();
+		return factory.createDateTimeFormatter();
 	}
 }

spring-context/src/main/java/org/springframework/format/datetime/joda/JodaTimeFormatterRegistrar.java

@@ -47,6 +47,7 @@ import org.springframework.format.annotation.DateTimeFormat.ISO;
  * @see #setUseIsoFormat
  * @see FormatterRegistrar#registerFormatters
  * @see org.springframework.format.datetime.DateFormatterRegistrar
+ * @see DateTimeFormatterFactoryBean
  */
 public class JodaTimeFormatterRegistrar implements FormatterRegistrar {
 
@@ -185,7 +186,7 @@ public class JodaTimeFormatterRegistrar implements FormatterRegistrar {
 			return formatter;
 		}
 		DateTimeFormatter fallbackFormatter = getFallbackFormatter(type);
-		return factories.get(type).getDateTimeFormatter(fallbackFormatter );
+		return factories.get(type).createDateTimeFormatter(fallbackFormatter );
 	}
 
 	private DateTimeFormatter getFallbackFormatter(Type type) {

spring-context/src/test/java/org/springframework/format/datetime/joda/DateTimeFormatterFactoryBeanTests.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2002-2012 the original author or authors.
+ *
+ * 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.springframework.format.datetime.joda;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.junit.Assert.*;
+
+import org.joda.time.format.DateTimeFormat;
+import org.joda.time.format.DateTimeFormatter;
+import org.junit.Test;
+
+/**
+ * Tests for {@link DateTimeFormatterFactoryBean}.
+ *
+ * @author Phillip Webb
+ * @author Sam Brannen
+ */
+public class DateTimeFormatterFactoryBeanTests {
+
+	private DateTimeFormatterFactoryBean factory = new DateTimeFormatterFactoryBean();
+
+	@Test
+	public void isSingleton() throws Exception {
+		assertThat(factory.isSingleton(), is(true));
+	}
+
+	@Test
+	@SuppressWarnings("rawtypes")
+	public void getObjectType() throws Exception {
+		assertThat(factory.getObjectType(), is(equalTo((Class) DateTimeFormatter.class)));
+	}
+
+	@Test
+	public void getObject() throws Exception {
+		factory.afterPropertiesSet();
+		assertThat(factory.getObject(), is(equalTo(DateTimeFormat.mediumDateTime())));
+	}
+
+	@Test
+	public void getObjectIsAlwaysSingleton() throws Exception {
+		factory.afterPropertiesSet();
+		DateTimeFormatter formatter = factory.getObject();
+		assertThat(formatter, is(equalTo(DateTimeFormat.mediumDateTime())));
+		factory.setStyle("LL");
+		assertThat(factory.getObject(), is(sameInstance(formatter)));
+	}
+}

spring-context/src/test/java/org/springframework/format/datetime/joda/DateTimeFormatterFactoryTests.java

@@ -37,90 +37,67 @@ import org.springframework.format.annotation.DateTimeFormat.ISO;
  */
 public class DateTimeFormatterFactoryTests {
 
+	// Potential test timezone, both have daylight savings on October 21st
+	private static final TimeZone ZURICH = TimeZone.getTimeZone("Europe/Zurich");
+	private static final TimeZone NEW_YORK = TimeZone.getTimeZone("America/New_York");
+
+	// Ensure that we are testing against a timezone other than the default.
+	private static final TimeZone TEST_TIMEZONE = ZURICH.equals(TimeZone.getDefault()) ? NEW_YORK : ZURICH;
+
+
 	private DateTimeFormatterFactory factory = new DateTimeFormatterFactory();
 
 	private DateTime dateTime = new DateTime(2009, 10, 21, 12, 10, 00, 00);
 
 
 	@Test
-	public void shouldDefaultToMediumFormat() throws Exception {
+	public void createDateTimeFormatter() throws Exception {
-		assertThat(factory.getObject(), is(equalTo(DateTimeFormat.mediumDateTime())));
+		assertThat(factory.createDateTimeFormatter(), is(equalTo(DateTimeFormat.mediumDateTime())));
-		assertThat(factory.getDateTimeFormatter(), is(equalTo(DateTimeFormat.mediumDateTime())));
 	}
 
 	@Test
-	public void shouldCreateFromPattern() throws Exception {
+	public void createDateTimeFormatterWithPattern() throws Exception {
 		factory = new DateTimeFormatterFactory("yyyyMMddHHmmss");
-		DateTimeFormatter formatter = factory.getObject();
+		DateTimeFormatter formatter = factory.createDateTimeFormatter();
 		assertThat(formatter.print(dateTime), is("20091021121000"));
 	}
 
 	@Test
-	public void shouldBeSingleton() throws Exception {
+	public void createDateTimeFormatterWithNullFallback() throws Exception {
-		assertThat(factory.isSingleton(), is(true));
+		DateTimeFormatter formatter = factory.createDateTimeFormatter(null);
-	}
-
-	@Test
-	@SuppressWarnings("rawtypes")
-	public void shouldCreateDateTimeFormatter() throws Exception {
-		assertThat(factory.getObjectType(), is(equalTo((Class) DateTimeFormatter.class)));
-	}
-
-	@Test
-	public void shouldGetDateTimeFormatterNullFallback() throws Exception {
-		DateTimeFormatter formatter = factory.getDateTimeFormatter(null);
 		assertThat(formatter, is(nullValue()));
 	}
 
 	@Test
-	public void shouldGetDateTimeFormatterFallback() throws Exception {
+	public void createDateTimeFormatterWithFallback() throws Exception {
 		DateTimeFormatter fallback = DateTimeFormat.forStyle("LL");
-		DateTimeFormatter formatter = factory.getDateTimeFormatter(fallback);
+		DateTimeFormatter formatter = factory.createDateTimeFormatter(fallback);
 		assertThat(formatter, is(sameInstance(fallback)));
 	}
 
 	@Test
-	public void shouldGetDateTimeFormatter() throws Exception {
+	public void createDateTimeFormatterInOrderOfPropertyPriority() throws Exception {
 		factory.setStyle("SS");
-		assertThat(applyLocale(factory.getDateTimeFormatter()).print(dateTime), is("10/21/09 12:10 PM"));
+		assertThat(applyLocale(factory.createDateTimeFormatter()).print(dateTime), is("10/21/09 12:10 PM"));
 
 		factory.setIso(ISO.DATE);
-		assertThat(applyLocale(factory.getDateTimeFormatter()).print(dateTime), is("2009-10-21"));
+		assertThat(applyLocale(factory.createDateTimeFormatter()).print(dateTime), is("2009-10-21"));
 
 		factory.setPattern("yyyyMMddHHmmss");
-		assertThat(factory.getDateTimeFormatter().print(dateTime), is("20091021121000"));
+		assertThat(factory.createDateTimeFormatter().print(dateTime), is("20091021121000"));
 	}
 
 	@Test
-	public void shouldGetWithTimeZone() throws Exception {
+	public void createDateTimeFormatterWithTimeZone() throws Exception {
-
-		TimeZone zurich = TimeZone.getTimeZone("Europe/Zurich");
-		TimeZone newYork = TimeZone.getTimeZone("America/New_York");
-
-		// Ensure that we are testing against a timezone other than the default.
-		TimeZone testTimeZone;
-		String offset;
-
-		if (zurich.equals(TimeZone.getDefault())) {
-			testTimeZone = newYork;
-			offset = "-0400"; // Daylight savings on October 21st
-		}
-		else {
-			testTimeZone = zurich;
-			offset = "+0200"; // Daylight savings on October 21st
-		}
-
 		factory.setPattern("yyyyMMddHHmmss Z");
-		factory.setTimeZone(testTimeZone);
+		factory.setTimeZone(TEST_TIMEZONE);
-
+		DateTimeZone dateTimeZone = DateTimeZone.forTimeZone(TEST_TIMEZONE);
-		DateTimeZone dateTimeZone = DateTimeZone.forTimeZone(testTimeZone);
 		DateTime dateTime = new DateTime(2009, 10, 21, 12, 10, 00, 00, dateTimeZone);
-
+		String offset = (TEST_TIMEZONE.equals(NEW_YORK) ? "-0400" : "+0200");
-		assertThat(factory.getDateTimeFormatter().print(dateTime), is("20091021121000 " + offset));
+		assertThat(factory.createDateTimeFormatter().print(dateTime), is("20091021121000 " + offset));
 	}
 
 	private DateTimeFormatter applyLocale(DateTimeFormatter dateTimeFormatter) {
 		return dateTimeFormatter.withLocale(Locale.US);
 	}
-
 }