Browse Source

Merge pull request #98 from yiming187/release_fix

Ingore empty item when build release
Jason Song 9 years ago
parent
commit
954a562e2d

+ 5 - 0
apollo-adminservice/pom.xml

@@ -64,6 +64,11 @@
 				</exclusion>
 			</exclusions>
 		</dependency>
+		<dependency>
+			<groupId>com.h2database</groupId>
+			<artifactId>h2</artifactId>
+			<scope>test</scope>
+		</dependency>
 	</dependencies>
 	<build>
 		<plugins>

+ 4 - 0
apollo-biz/src/main/java/com/ctrip/apollo/biz/service/ReleaseService.java

@@ -15,6 +15,7 @@ import com.ctrip.apollo.biz.repository.ItemRepository;
 import com.ctrip.apollo.biz.repository.NamespaceRepository;
 import com.ctrip.apollo.biz.repository.ReleaseRepository;
 import com.ctrip.apollo.core.exception.NotFoundException;
+import com.ctrip.apollo.core.utils.StringUtils;
 import com.google.gson.Gson;
 
 /**
@@ -50,6 +51,9 @@ public class ReleaseService {
     List<Item> items = itemRepository.findByNamespaceIdOrderByLineNumAsc(namespace.getId());
     Map<String, String> configurations = new HashMap<String, String>();
     for (Item item : items) {
+      if (StringUtils.isEmpty(item.getKey())) {
+        continue;
+      }
       configurations.put(item.getKey(), item.getValue());
     }
 

+ 42 - 33
apollo-client/src/test/java/com/ctrip/apollo/integration/ConfigIntegrationTest.java

@@ -1,27 +1,13 @@
 package com.ctrip.apollo.integration;
 
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-
-import com.ctrip.apollo.Config;
-import com.ctrip.apollo.ConfigChangeListener;
-import com.ctrip.apollo.ConfigService;
-import com.ctrip.apollo.core.ConfigConsts;
-import com.ctrip.apollo.core.dto.ApolloConfig;
-import com.ctrip.apollo.core.utils.ClassLoaderUtil;
-import com.ctrip.apollo.model.ConfigChangeEvent;
-
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.handler.AbstractHandler;
-import org.eclipse.jetty.server.handler.ContextHandler;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
+import static org.hamcrest.core.IsEqual.equalTo;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -31,9 +17,23 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import static org.hamcrest.core.IsEqual.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.ctrip.apollo.Config;
+import com.ctrip.apollo.ConfigChangeListener;
+import com.ctrip.apollo.ConfigService;
+import com.ctrip.apollo.core.ConfigConsts;
+import com.ctrip.apollo.core.dto.ApolloConfig;
+import com.ctrip.apollo.core.utils.ClassLoaderUtil;
+import com.ctrip.apollo.model.ConfigChangeEvent;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 
 /**
  * @author Jason Song(song_s@ctrip.com)
@@ -56,8 +56,8 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
   @Override
   @After
   public void tearDown() throws Exception {
-    super.tearDown();
     recursiveDelete(configDir);
+    super.tearDown();
   }
 
   private void recursiveDelete(File file) {
@@ -69,7 +69,12 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
         recursiveDelete(f);
       }
     }
-    file.delete();
+    try {
+      Files.deleteIfExists(file.toPath());
+    } catch (IOException e) {
+      e.printStackTrace();
+    }
+
   }
 
   @Test
@@ -108,8 +113,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
 
   @Test
   public void testGetConfigWithNoLocalFileAndRemoteConfigError() throws Exception {
-    ContextHandler
-        handler =
+    ContextHandler handler =
         mockConfigServerHandler(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
     startServerWithHandlers(handler);
 
@@ -129,13 +133,11 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     properties.put(someKey, someValue);
     createLocalCachePropertyFile(properties);
 
-    ContextHandler
-        handler =
+    ContextHandler handler =
         mockConfigServerHandler(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
     startServerWithHandlers(handler);
 
     Config config = ConfigService.getConfig();
-
     assertEquals(someValue, config.getProperty(someKey, null));
   }
 
@@ -166,7 +168,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
         assertEquals(1, changeEvent.getChanges().size());
         assertEquals(someValue, changeEvent.getChange(someKey).getOldValue());
         assertEquals(anotherValue, changeEvent.getChange(someKey).getNewValue());
-        //if there is any assertion failed above, this line won't be executed
+        // if there is any assertion failed above, this line won't be executed
         changeEvents.add(changeEvent);
       }
     });
@@ -186,7 +188,7 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
     context.setHandler(new AbstractHandler() {
       @Override
       public void handle(String target, Request baseRequest, HttpServletRequest request,
-                         HttpServletResponse response) throws IOException, ServletException {
+          HttpServletResponse response) throws IOException, ServletException {
         response.setContentType("application/json;charset=UTF-8");
         response.setStatus(statusCode);
 
@@ -210,12 +212,19 @@ public class ConfigIntegrationTest extends BaseIntegrationTest {
 
   private File createLocalCachePropertyFile(Properties properties) throws IOException {
     File file = new File(configDir, assembleLocalCacheFileName());
-    properties.store(new FileOutputStream(file), "Persisted by ConfigIntegrationTest");
+    FileOutputStream in = null;
+    try {
+      in = new FileOutputStream(file);
+      properties.store(in, "Persisted by ConfigIntegrationTest");
+    } finally {
+      if (in != null) {
+        in.close();
+      }
+    }
     return file;
   }
 
   private String assembleLocalCacheFileName() {
-    return String.format("%s-%s-%s.properties", someAppId,
-        someClusterName, someNamespace);
+    return String.format("%s-%s-%s.properties", someAppId, someClusterName, someNamespace);
   }
 }

+ 1 - 1
apollo-portal/src/main/java/com/ctrip/apollo/portal/api/AdminServiceAPI.java

@@ -76,7 +76,7 @@ public class AdminServiceAPI {
 
     public List<ItemDTO> findItems(String appId, Env env, String clusterName, String namespace) {
       if (StringUtils.isContainEmpty(appId, clusterName, namespace)) {
-        return Collections.EMPTY_LIST;
+        return Collections.emptyList();
       }
 
       return Arrays.asList(restTemplate.getForObject(getAdminServiceHost(env) + String

+ 1 - 3
apollo-portal/src/main/java/com/ctrip/apollo/portal/controller/ConfigController.java

@@ -11,7 +11,6 @@ import com.ctrip.apollo.portal.entity.form.NamespaceReleaseModel;
 import com.ctrip.apollo.portal.service.ConfigService;
 
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
@@ -39,7 +38,7 @@ public class ConfigController {
 
   @RequestMapping(value = "/apps/{appId}/env/{env}/clusters/{clusterName}/namespaces/{namespaceName}/items", method = RequestMethod.PUT, consumes = {
       "application/json"})
-  public ResponseEntity modifyItems(@PathVariable String appId, @PathVariable String env,
+  public void modifyItems(@PathVariable String appId, @PathVariable String env,
       @PathVariable String clusterName, @PathVariable String namespaceName,
       @RequestBody NamespaceTextModel model) {
 
@@ -56,7 +55,6 @@ public class ConfigController {
     }
 
     configService.updateConfigItemByText(model);
-    return ResponseEntity.ok().build();
   }
 
   @RequestMapping(value = "/apps/{appId}/env/{env}/clusters/{clusterName}/namespaces/{namespaceName}/release", method = RequestMethod.POST, consumes = {

+ 15 - 13
apollo-portal/src/main/java/com/ctrip/apollo/portal/service/ConfigService.java

@@ -1,6 +1,5 @@
 package com.ctrip.apollo.portal.service;
 
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -47,6 +46,7 @@ public class ConfigService {
 
   /**
    * load cluster all namespace info with items
+   * 
    * @param appId
    * @param env
    * @param clusterName
@@ -56,7 +56,7 @@ public class ConfigService {
 
     List<NamespaceDTO> namespaces = groupAPI.findGroupsByAppAndCluster(appId, env, clusterName);
     if (namespaces == null || namespaces.size() == 0) {
-      return Collections.EMPTY_LIST;
+      return Collections.emptyList();
     }
 
     List<NamespaceVO> namespaceVOs = new LinkedList<>();
@@ -67,8 +67,8 @@ public class ConfigService {
         namespaceVO = parseNamespace(appId, env, clusterName, namespace);
         namespaceVOs.add(namespaceVO);
       } catch (Exception e) {
-        logger.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}", appId, env, clusterName,
-                     namespace.getNamespaceName(), e);
+        logger.error("parse namespace error. app id:{}, env:{}, clusterName:{}, namespace:{}",
+            appId, env, clusterName, namespace.getNamespaceName(), e);
         return namespaceVOs;
       }
     }
@@ -76,8 +76,8 @@ public class ConfigService {
     return namespaceVOs;
   }
 
+  @SuppressWarnings("unchecked")
   private NamespaceVO parseNamespace(String appId, Env env, String clusterName, NamespaceDTO namespace) {
-
     NamespaceVO namespaceVO = new NamespaceVO();
     namespaceVO.setNamespace(namespace);
 
@@ -133,7 +133,8 @@ public class ConfigService {
 
   /**
    * parse config text and update config items
-   * @return  parse result
+   * 
+   * @return parse result
    */
   public void updateConfigItemByText(NamespaceTextModel model) {
     String appId = model.getAppId();
@@ -143,8 +144,8 @@ public class ConfigService {
     long namespaceId = model.getNamespaceId();
     String configText = model.getConfigText();
 
-    ItemChangeSets changeSets =
-        resolver.resolve(namespaceId, configText, itemAPI.findItems(appId, env, clusterName, namespaceName));
+    ItemChangeSets changeSets = resolver.resolve(namespaceId, configText,
+        itemAPI.findItems(appId, env, clusterName, namespaceName));
     try {
       changeSets.setModifyBy(model.getModifyBy());
       enrichChangeSetBaseInfo(changeSets);
@@ -155,18 +156,19 @@ public class ConfigService {
 
   }
 
-  private void enrichChangeSetBaseInfo(ItemChangeSets changeSets){
-    for (ItemDTO item: changeSets.getCreateItems()){
+  private void enrichChangeSetBaseInfo(ItemChangeSets changeSets) {
+    for (ItemDTO item : changeSets.getCreateItems()) {
       item.setDataChangeCreatedTime(new Date());
     }
   }
 
   /**
    * createRelease config items
+   * 
    * @return
    */
-  public ReleaseDTO createRelease(NamespaceReleaseModel model){
-    return releaseAPI.release(model.getAppId(), model.getEnv(), model.getClusterName(), model.getNamespaceName(),
-                              model.getReleaseBy(), model.getReleaseComment());
+  public ReleaseDTO createRelease(NamespaceReleaseModel model) {
+    return releaseAPI.release(model.getAppId(), model.getEnv(), model.getClusterName(),
+        model.getNamespaceName(), model.getReleaseBy(), model.getReleaseComment());
   }
 }

+ 1 - 1
apollo-portal/src/main/java/com/ctrip/apollo/portal/service/txtresolver/PropertyResolver.java

@@ -45,7 +45,7 @@ public class PropertyResolver implements ConfigTextResolver {
     }
 
     ItemChangeSets changeSets = new ItemChangeSets();
-    Map<Integer, String> newLineNumMapItem = new HashMap();//use for delete blank and comment item
+    Map<Integer, String> newLineNumMapItem = new HashMap<Integer, String>();//use for delete blank and comment item
     int lineCounter = 1;
     for (String newItem : newItems) {
       newItem = newItem.trim();