Refactor the createCommit method

apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java

@@ -79,15 +79,9 @@ public ItemDTO create(@PathVariable("appId") String appId,
     }
     entity = itemService.save(entity);
     dto = BeanUtils.transform(ItemDTO.class, entity);
-
-    Commit commit = new Commit();
-    commit.setAppId(appId);
-    commit.setClusterName(clusterName);
-    commit.setNamespaceName(namespaceName);
-    commit.setChangeSets(new ConfigChangeContentBuilder().createItem(entity).build());
-    commit.setDataChangeCreatedBy(dto.getDataChangeLastModifiedBy());
-    commit.setDataChangeLastModifiedBy(dto.getDataChangeLastModifiedBy());
-    commitService.save(commit);
+    commitService.createCommit(appId, clusterName, namespaceName, new ConfigChangeContentBuilder().createItem(entity).build(),
+        dto.getDataChangeLastModifiedBy()
+    );
 
     return dto;
   }
@@ -154,14 +148,7 @@ public ItemDTO update(@PathVariable("appId") String appId,
     itemDTO = BeanUtils.transform(ItemDTO.class, entity);
 
     if (builder.hasContent()) {
-      Commit commit = new Commit();
-      commit.setAppId(appId);
-      commit.setClusterName(clusterName);
-      commit.setNamespaceName(namespaceName);
-      commit.setChangeSets(builder.build());
-      commit.setDataChangeCreatedBy(itemDTO.getDataChangeLastModifiedBy());
-      commit.setDataChangeLastModifiedBy(itemDTO.getDataChangeLastModifiedBy());
-      commitService.save(commit);
+      commitService.createCommit(appId, clusterName, namespaceName, builder.build(), itemDTO.getDataChangeLastModifiedBy());
     }
 
     return itemDTO;
@@ -178,14 +165,10 @@ public void delete(@PathVariable("itemId") long itemId, @RequestParam String ope
 
     Namespace namespace = namespaceService.findOne(entity.getNamespaceId());
 
-    Commit commit = new Commit();
-    commit.setAppId(namespace.getAppId());
-    commit.setClusterName(namespace.getClusterName());
-    commit.setNamespaceName(namespace.getNamespaceName());
-    commit.setChangeSets(new ConfigChangeContentBuilder().deleteItem(entity).build());
-    commit.setDataChangeCreatedBy(operator);
-    commit.setDataChangeLastModifiedBy(operator);
-    commitService.save(commit);
+    commitService.createCommit(namespace.getAppId(), namespace.getClusterName(), namespace.getNamespaceName(),
+        new ConfigChangeContentBuilder().deleteItem(entity).build(), operator
+    );
+
   }
 
   @GetMapping("/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items")

apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java

@@ -0,0 +1,165 @@
+/*
+ * Copyright 2023 Apollo 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 com.ctrip.framework.apollo.adminservice.controller;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.ctrip.framework.apollo.biz.entity.Commit;
+import com.ctrip.framework.apollo.biz.repository.CommitRepository;
+import com.ctrip.framework.apollo.biz.repository.ItemRepository;
+import com.ctrip.framework.apollo.common.dto.AppDTO;
+import com.ctrip.framework.apollo.common.dto.ClusterDTO;
+import com.ctrip.framework.apollo.common.dto.ItemDTO;
+import com.ctrip.framework.apollo.common.dto.NamespaceDTO;
+import java.util.List;
+import java.util.Objects;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.data.domain.Pageable;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.test.context.jdbc.Sql;
+import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
+import org.springframework.web.client.RestTemplate;
+
+/**
+ * @author kl (http://kailing.pub)
+ * @since 2023/3/21
+ */
+public class ItemControllerTest extends AbstractControllerTest {
+
+  @Autowired
+  private CommitRepository commitRepository;
+
+  @Autowired
+  private ItemRepository itemRepository;
+
+  @Test
+  @Sql(scripts = "/controller/test-itemset.sql", executionPhase = ExecutionPhase.BEFORE_TEST_METHOD)
+  @Sql(scripts = "/controller/cleanup.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)
+  public void testCreate(){
+    String appId = "someAppId";
+    AppDTO app = restTemplate.getForObject(appBaseUrl(), AppDTO.class, appId);
+    assert app != null;
+    ClusterDTO cluster = restTemplate.getForObject(clusterBaseUrl(), ClusterDTO.class, app.getAppId(), "default");
+    assert cluster != null;
+    NamespaceDTO namespace = restTemplate.getForObject(namespaceBaseUrl(),
+        NamespaceDTO.class, app.getAppId(), cluster.getName(), "application");
+
+    RestTemplate createdTemplate = (new TestRestTemplate()).getRestTemplate();
+    createdTemplate.setMessageConverters(restTemplate.getMessageConverters());
+
+    String itemKey = "test-key";
+    String itemValue = "test-value";
+    ItemDTO item = new ItemDTO(itemKey, itemValue, "", 1);
+    assert namespace != null;
+    item.setNamespaceId(namespace.getId());
+    item.setDataChangeLastModifiedBy("apollo");
+
+    ResponseEntity<ItemDTO> response = createdTemplate.postForEntity(itemBaseUrl(),
+        item, ItemDTO.class, app.getAppId(), cluster.getName(), namespace.getNamespaceName());
+    Assert.assertEquals(HttpStatus.OK, response.getStatusCode());
+    Assertions.assertEquals(itemKey, Objects.requireNonNull(response.getBody()).getKey());
+
+    List<Commit> commitList = commitRepository.findByAppIdAndClusterNameAndNamespaceNameOrderByIdDesc(app.getAppId(), cluster.getName(), namespace.getNamespaceName(),
+        Pageable.ofSize(10));
+    Assert.assertEquals(1, commitList.size());
+
+    Commit commit = commitList.get(0);
+    Assert.assertTrue(commit.getChangeSets().contains(itemKey));
+    Assert.assertTrue(commit.getChangeSets().contains(itemValue));
+  }
+
+  @Test
+  @Sql(scripts = "/controller/test-itemset.sql", executionPhase = ExecutionPhase.BEFORE_TEST_METHOD)
+  @Sql(scripts = "/controller/cleanup.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)
+  public void testUpdate(){
+    this.testCreate();
+
+    String appId = "someAppId";
+    AppDTO app = restTemplate.getForObject(appBaseUrl(), AppDTO.class, appId);
+    assert app != null;
+    ClusterDTO cluster = restTemplate.getForObject(clusterBaseUrl(), ClusterDTO.class, app.getAppId(), "default");
+    assert cluster != null;
+    NamespaceDTO namespace = restTemplate.getForObject(namespaceBaseUrl(),
+        NamespaceDTO.class, app.getAppId(), cluster.getName(), "application");
+
+    RestTemplate updateTemplate = (new TestRestTemplate()).getRestTemplate();
+    updateTemplate.setMessageConverters(restTemplate.getMessageConverters());
+
+    String itemKey = "test-key";
+    String itemValue = "test-value-updated";
+
+    long itemId = itemRepository.findByKey(itemKey, Pageable.ofSize(1))
+        .getContent()
+        .get(0)
+        .getId();
+    ItemDTO item = new ItemDTO(itemKey, itemValue, "", 1);
+    item.setDataChangeLastModifiedBy("apollo");
+
+    String updateUrl = url(  "/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/items/{itemId}");
+    assert namespace != null;
+    updateTemplate.put(updateUrl, item, app.getAppId(), cluster.getName(), namespace.getNamespaceName(), itemId);
+
+    itemRepository.findById(itemId).ifPresent(item1 -> {
+      assertThat(item1.getValue()).isEqualTo(itemValue);
+      assertThat(item1.getKey()).isEqualTo(itemKey);
+    });
+
+    List<Commit> commitList = commitRepository.findByAppIdAndClusterNameAndNamespaceNameOrderByIdDesc(app.getAppId(), cluster.getName(), namespace.getNamespaceName(),
+        Pageable.ofSize(10));
+    assertThat(commitList).hasSize(2);
+  }
+
+  @Test
+  @Sql(scripts = "/controller/test-itemset.sql", executionPhase = ExecutionPhase.BEFORE_TEST_METHOD)
+  @Sql(scripts = "/controller/cleanup.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)
+  public void testDelete(){
+    this.testCreate();
+
+    String appId = "someAppId";
+    AppDTO app = restTemplate.getForObject(appBaseUrl(), AppDTO.class, appId);
+    assert app != null;
+    ClusterDTO cluster = restTemplate.getForObject(clusterBaseUrl(), ClusterDTO.class, app.getAppId(), "default");
+    assert cluster != null;
+    NamespaceDTO namespace = restTemplate.getForObject(namespaceBaseUrl(),
+        NamespaceDTO.class, app.getAppId(), cluster.getName(), "application");
+
+    RestTemplate delTemplate = (new TestRestTemplate()).getRestTemplate();
+    delTemplate.setMessageConverters(restTemplate.getMessageConverters());
+
+    String itemKey = "test-key";
+
+    long itemId = itemRepository.findByKey(itemKey, Pageable.ofSize(1))
+        .getContent()
+        .get(0)
+        .getId();
+
+    String deleteUrl = url(  "/items/{itemId}?operator=apollo");
+    delTemplate.delete(deleteUrl, itemId);
+    assertThat(itemRepository.findById(itemId).isPresent())
+        .isFalse();
+
+    assert namespace != null;
+    List<Commit> commitList = commitRepository.findByAppIdAndClusterNameAndNamespaceNameOrderByIdDesc(app.getAppId(), cluster.getName(), namespace.getNamespaceName(),
+        Pageable.ofSize(10));
+    assertThat(commitList).hasSize(2);
+  }
+}

apollo-adminservice/src/test/resources/controller/cleanup.sql

@@ -20,4 +20,5 @@ DELETE FROM "Cluster";
 DELETE FROM "App";
 DELETE FROM "NamespaceLock";
 DELETE FROM "ServerConfig";
+DELETE FROM "Commit";
 

apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/CommitService.java

@@ -35,9 +35,18 @@ public CommitService(final CommitRepository commitRepository) {
   }
 
   @Transactional
-  public Commit save(Commit commit){
+  public void createCommit(String appId, String clusterName, String namespaceName, String configChangeContent,
+      String operator) {
+
+    Commit commit = new Commit();
     commit.setId(0);//protection
-    return commitRepository.save(commit);
+    commit.setAppId(appId);
+    commit.setClusterName(clusterName);
+    commit.setNamespaceName(namespaceName);
+    commit.setChangeSets(configChangeContent);
+    commit.setDataChangeCreatedBy(operator);
+    commit.setDataChangeLastModifiedBy(operator);
+    commitRepository.save(commit);
   }
 
   public List<Commit> find(String appId, String clusterName, String namespaceName, Pageable page){

apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemSetService.java

@@ -17,7 +17,6 @@
 package com.ctrip.framework.apollo.biz.service;
 
 import com.ctrip.framework.apollo.biz.entity.Audit;
-import com.ctrip.framework.apollo.biz.entity.Commit;
 import com.ctrip.framework.apollo.biz.entity.Item;
 import com.ctrip.framework.apollo.biz.entity.Namespace;
 import com.ctrip.framework.apollo.biz.utils.ConfigChangeContentBuilder;
@@ -84,7 +83,7 @@ public ItemChangeSets updateSet(String appId, String clusterName,
     }
 
     if (configChangeContentBuilder.hasContent()){
-      createCommit(appId, clusterName, namespaceName, configChangeContentBuilder.build(),
+      commitService.createCommit(appId, clusterName, namespaceName, configChangeContentBuilder.build(),
                    changeSet.getDataChangeLastModifiedBy());
     }
 
@@ -147,17 +146,4 @@ private void doCreateItems(List<ItemDTO> toCreateItems, Namespace namespace, Str
     }
   }
 
-  private void createCommit(String appId, String clusterName, String namespaceName, String configChangeContent,
-                            String operator) {
-
-    Commit commit = new Commit();
-    commit.setAppId(appId);
-    commit.setClusterName(clusterName);
-    commit.setNamespaceName(namespaceName);
-    commit.setChangeSets(configChangeContent);
-    commit.setDataChangeCreatedBy(operator);
-    commit.setDataChangeLastModifiedBy(operator);
-    commitService.save(commit);
-  }
-
 }