Modified according to CodeReview feedback

apolloconfig · Jan 21, 2024 · 4532209 · 4532209
1 parent a6aac60
commit 4532209
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 8 deletions.
diff --git a/...src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java b/...src/main/java/com/ctrip/framework/apollo/spring/annotation/ApolloAnnotationProcessor.java
@@ -20,6 +20,7 @@
 import com.ctrip.framework.apollo.ConfigChangeListener;
 import com.ctrip.framework.apollo.ConfigService;
 import com.ctrip.framework.apollo.build.ApolloInjector;
+import com.ctrip.framework.apollo.core.utils.StringUtils;
 import com.ctrip.framework.apollo.model.ConfigChangeEvent;
 import com.ctrip.framework.apollo.spring.property.PlaceholderHelper;
 import com.ctrip.framework.apollo.spring.property.SpringValue;
@@ -252,13 +253,20 @@ private Object resolvePropertyValue(String beanName, String placeHolder) {
 
   private Object parseJsonValue(String json, Type targetType, String datePattern) {
     try {
-      return DATEPATTERN_GSON_MAP.computeIfAbsent(datePattern, (pattern) -> new GsonBuilder().setDateFormat(pattern).create()).fromJson(json, targetType);
+      return DATEPATTERN_GSON_MAP.computeIfAbsent(datePattern, this::buildGson).fromJson(json, targetType);
     } catch (Throwable ex) {
       logger.error("Parsing json '{}' to type {} failed!", json, targetType, ex);
       throw ex;
     }
   }
 
+  private Gson buildGson(String datePattern) {
+    if (StringUtils.isBlank(datePattern)) {
+      return new Gson();
+    }
+    return new GsonBuilder().setDateFormat(datePattern).create();
+  }
+
   @Override
   public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
     this.configurableBeanFactory = (ConfigurableBeanFactory) beanFactory;

diff --git a/.../main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java b/.../main/java/com/ctrip/framework/apollo/spring/property/AutoUpdateConfigChangeListener.java
@@ -18,6 +18,7 @@
 
 import com.ctrip.framework.apollo.ConfigChangeListener;
 import com.ctrip.framework.apollo.build.ApolloInjector;
+import com.ctrip.framework.apollo.core.utils.StringUtils;
 import com.ctrip.framework.apollo.model.ConfigChangeEvent;
 import com.ctrip.framework.apollo.spring.annotation.ApolloJsonValue;
 import com.ctrip.framework.apollo.spring.events.ApolloConfigChangeEvent;
@@ -135,14 +136,20 @@ private Object resolvePropertyValue(SpringValue springValue) {
 
   private Object parseJsonValue(String json, Type targetType, String datePattern) {
     try {
-      return datePatternGsonMap.computeIfAbsent(datePattern, (pattern) -> new GsonBuilder().setDateFormat(datePattern).create())
-              .fromJson(json, targetType);
+      return datePatternGsonMap.computeIfAbsent(datePattern, this::buildGson).fromJson(json, targetType);
     } catch (Throwable ex) {
       logger.error("Parsing json '{}' to type {} failed!", json, targetType, ex);
       throw ex;
     }
   }
 
+  private Gson buildGson(String datePattern) {
+    if (StringUtils.isBlank(datePattern)) {
+      return new Gson();
+    }
+    return new GsonBuilder().setDateFormat(datePattern).create();
+  }
+
   private boolean testTypeConverterHasConvertIfNecessaryWithFieldParameter() {
     try {
       TypeConverter.class.getMethod("convertIfNecessary", Object.class, Class.class, Field.class);

diff --git a/.../src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java b/.../src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderAutoUpdateTest.java
@@ -873,7 +873,7 @@ public void testAutoUpdateWithAllKindsOfDataTypes() throws Exception {
     assertEquals(someDate, bean.getDateProperty());
     assertEquals("astring", bean.getJsonBeanList().get(0).getA());
     assertEquals(10, bean.getJsonBeanList().get(0).getB());
-    assertNotNull(bean.getJsonDateBean().getStartTime());
+    assertEquals("2024-01-20 00:00:00.000", simpleDateFormat.format(bean.getJsonDateBean().getStartTime()));
 
     Properties newProperties = new Properties();
     newProperties.setProperty("intProperty", String.valueOf(someNewInt));
@@ -906,7 +906,7 @@ public void testAutoUpdateWithAllKindsOfDataTypes() throws Exception {
     assertEquals(someNewDate, bean.getDateProperty());
     assertEquals("newString", bean.getJsonBeanList().get(0).getA());
     assertEquals(20, bean.getJsonBeanList().get(0).getB());
-    assertNotNull(bean.getJsonDateBean().getStartTime());
+    assertEquals("2024-02-21 00:00:00.000", simpleDateFormat.format(bean.getJsonDateBean().getStartTime()));
   }
 
   @Test

diff --git a/apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java b/apollo-client/src/test/java/com/ctrip/framework/apollo/spring/JavaConfigPlaceholderTest.java
@@ -28,8 +28,10 @@
 import com.ctrip.framework.apollo.core.ConfigConsts;
 import com.ctrip.framework.apollo.spring.annotation.ApolloJsonValue;
 import com.ctrip.framework.apollo.spring.annotation.EnableApolloConfig;
+import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 import java.util.Properties;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -396,6 +398,9 @@ public void testApolloDateJsonValue() {
     String dateFormatJson2 = "{\"startTime\":\"2024-01-20T16:51:48\",\"endTime\":\"2024-01-20T16:51:48\"}";
     String dateFormatJson3 = "{\"startTime\":\"2024/01/20\",\"endTime\":\"2024/01/20\"}";
 
+    String someDateFormat = "yyyy-MM-dd HH:mm:ss.SSS";
+    SimpleDateFormat simpleDateFormat = new SimpleDateFormat(someDateFormat, Locale.US);
+
     Config config = mock(Config.class);
     when(config.getProperty(eq(DATE_FORMAT_JSON_PROPERTY1), Mockito.nullable(String.class))).thenReturn(dateFormatJson1);
     when(config.getProperty(eq(DATE_FORMAT_JSON_PROPERTY2), Mockito.nullable(String.class))).thenReturn(dateFormatJson2);
@@ -406,9 +411,9 @@ public void testApolloDateJsonValue() {
         AppConfig12.class);
 
     TestJsonDatePropertyBean datePropertyBean = context.getBean(TestJsonDatePropertyBean.class);
-    assertNotNull(datePropertyBean.getPattern1());
-    assertNotNull(datePropertyBean.getPattern2());
-    assertNotNull(datePropertyBean.getPattern3());
+    assertEquals("2024-01-20 00:00:00.000", simpleDateFormat.format(datePropertyBean.getPattern1().getStartTime()));
+    assertEquals("2024-01-20 16:51:48.000", simpleDateFormat.format(datePropertyBean.getPattern2().getStartTime()));
+    assertEquals("2024-01-20 00:00:00.000", simpleDateFormat.format(datePropertyBean.getPattern3().getStartTime()));
   }
 
   @Test(expected = BeanCreationException.class)