Skip to content

Commit 1c7dcf8

Browse files
committed
feat:
1. solve the problem of duplicate comments and blank lines 2. reduce the data growth of the item table 3. in the same case, the size of ItemChangeSets can be reduced 4. when revoke configuration, if there is only one comment or blank line, it will not be deleted.
1 parent 31e6486 commit 1c7dcf8

File tree

4 files changed

+59
-44
lines changed

4 files changed

+59
-44
lines changed

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/txtresolver/PropertyResolver.java

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,20 @@
2121
import com.ctrip.framework.apollo.common.exception.BadRequestException;
2222
import com.ctrip.framework.apollo.common.utils.BeanUtils;
2323

24+
import com.ctrip.framework.apollo.core.utils.StringUtils;
2425
import com.google.common.base.Strings;
2526
import org.springframework.stereotype.Component;
27+
import org.springframework.util.CollectionUtils;
2628

2729
import javax.validation.constraints.NotNull;
30+
import java.util.ArrayList;
31+
import java.util.Comparator;
2832
import java.util.HashMap;
2933
import java.util.HashSet;
3034
import java.util.List;
3135
import java.util.Map;
3236
import java.util.Set;
37+
import java.util.stream.Collectors;
3338

3439
/**
3540
* normal property file resolver.
@@ -45,12 +50,21 @@ public class PropertyResolver implements ConfigTextResolver {
4550
@Override
4651
public ItemChangeSets resolve(long namespaceId, String configText, List<ItemDTO> baseItems) {
4752

48-
Map<Integer, ItemDTO> oldLineNumMapItem = BeanUtils.mapByKey("lineNum", baseItems);
4953
Map<String, ItemDTO> oldKeyMapItem = BeanUtils.mapByKey("key", baseItems);
50-
5154
//remove comment and blank item map.
5255
oldKeyMapItem.remove("");
5356

57+
// comment items
58+
List<ItemDTO> baseCommentItems = new ArrayList<>();
59+
// blank items
60+
List<ItemDTO> baseBlankItems = new ArrayList<>();
61+
if(!CollectionUtils.isEmpty(baseItems)) {
62+
63+
baseCommentItems = baseItems.stream().filter(itemDTO -> isCommentItem(itemDTO)).sorted(Comparator.comparing(ItemDTO::getLineNum)).collect(Collectors.toList());
64+
65+
baseBlankItems = baseItems.stream().filter(itemDTO -> isBlankItem(itemDTO)).sorted(Comparator.comparing(ItemDTO::getLineNum)).collect(Collectors.toList());
66+
}
67+
5468
String[] newItems = configText.split(ITEM_SEPARATOR);
5569
Set<String> repeatKeys = new HashSet<>();
5670
if (isHasRepeatKey(newItems, repeatKeys)) {
@@ -63,17 +77,25 @@ public ItemChangeSets resolve(long namespaceId, String configText, List<ItemDTO>
6377
for (String newItem : newItems) {
6478
newItem = newItem.trim();
6579
newLineNumMapItem.put(lineCounter, newItem);
66-
ItemDTO oldItemByLine = oldLineNumMapItem.get(lineCounter);
6780

6881
//comment item
6982
if (isCommentItem(newItem)) {
83+
ItemDTO oldItemDTO = null;
84+
if(!CollectionUtils.isEmpty(baseCommentItems)) {
85+
oldItemDTO = baseCommentItems.remove(0);
86+
}
7087

71-
handleCommentLine(namespaceId, oldItemByLine, newItem, lineCounter, changeSets);
88+
handleCommentLine(namespaceId, oldItemDTO, newItem, lineCounter, changeSets);
7289

7390
//blank item
7491
} else if (isBlankItem(newItem)) {
7592

76-
handleBlankLine(namespaceId, oldItemByLine, lineCounter, changeSets);
93+
ItemDTO oldItemDTO = null;
94+
if(!CollectionUtils.isEmpty(baseBlankItems)) {
95+
oldItemDTO = baseBlankItems.remove(0);
96+
}
97+
98+
handleBlankLine(namespaceId, oldItemDTO, lineCounter, changeSets);
7799

78100
//normal item
79101
} else {
@@ -83,7 +105,7 @@ public ItemChangeSets resolve(long namespaceId, String configText, List<ItemDTO>
83105
lineCounter++;
84106
}
85107

86-
deleteCommentAndBlankItem(oldLineNumMapItem, newLineNumMapItem, changeSets);
108+
deleteCommentAndBlankItem(baseCommentItems, baseBlankItems, changeSets);
87109
deleteNormalKVItem(oldKeyMapItem, changeSets);
88110

89111
return changeSets;
@@ -122,16 +144,18 @@ private String[] parseKeyValueFromItem(String item) {
122144
}
123145

124146
private void handleCommentLine(Long namespaceId, ItemDTO oldItemByLine, String newItem, int lineCounter, ItemChangeSets changeSets) {
125-
String oldComment = oldItemByLine == null ? "" : oldItemByLine.getComment();
126-
//create comment. implement update comment by delete old comment and create new comment
127-
if (!(isCommentItem(oldItemByLine) && newItem.equals(oldComment))) {
147+
if(null == oldItemByLine ){
128148
changeSets.addCreateItem(buildCommentItem(0L, namespaceId, newItem, lineCounter));
149+
}else if(!StringUtils.equals(oldItemByLine.getComment(), newItem) || lineCounter != oldItemByLine.getLineNum()) {
150+
changeSets.addUpdateItem(buildCommentItem(oldItemByLine.getId(), namespaceId, newItem, lineCounter));
129151
}
130152
}
131153

132154
private void handleBlankLine(Long namespaceId, ItemDTO oldItem, int lineCounter, ItemChangeSets changeSets) {
133-
if (!isBlankItem(oldItem)) {
155+
if(null == oldItem ){
134156
changeSets.addCreateItem(buildBlankItem(0L, namespaceId, lineCounter));
157+
}else if (lineCounter != oldItem.getLineNum()) {
158+
changeSets.addUpdateItem(buildBlankItem(oldItem.getId(), namespaceId, lineCounter));
135159
}
136160
}
137161

@@ -183,23 +207,11 @@ private void deleteNormalKVItem(Map<String, ItemDTO> baseKeyMapItem, ItemChangeS
183207
}
184208
}
185209

186-
private void deleteCommentAndBlankItem(Map<Integer, ItemDTO> oldLineNumMapItem,
187-
Map<Integer, String> newLineNumMapItem,
210+
private void deleteCommentAndBlankItem(List<ItemDTO> baseCommentItems,
211+
List<ItemDTO> baseBlankItems,
188212
ItemChangeSets changeSets) {
189-
190-
for (Map.Entry<Integer, ItemDTO> entry : oldLineNumMapItem.entrySet()) {
191-
int lineNum = entry.getKey();
192-
ItemDTO oldItem = entry.getValue();
193-
String newItem = newLineNumMapItem.get(lineNum);
194-
195-
//1. old is blank by now is not
196-
//2.old is comment by now is not exist or modified
197-
//3.old is blank by now is not exist or modified
198-
if ((isBlankItem(oldItem) && !isBlankItem(newItem))
199-
|| (isCommentItem(oldItem) || isBlankItem(oldItem)) && (newItem == null || !newItem.equals(oldItem.getComment()))) {
200-
changeSets.addDeleteItem(oldItem);
201-
}
202-
}
213+
baseCommentItems.forEach(oldItemDTO-> changeSets.addDeleteItem(oldItemDTO));
214+
baseBlankItems.forEach(oldItemDTO-> changeSets.addDeleteItem(oldItemDTO));
203215
}
204216

205217
private ItemDTO buildCommentItem(Long id, Long namespaceId, String comment, int lineNum) {

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import java.util.LinkedList;
5151
import java.util.List;
5252
import java.util.Map;
53+
import java.util.stream.Collectors;
5354

5455
@Service
5556
public class ItemService {
@@ -216,12 +217,13 @@ public void revokeItem(String appId, Env env, String clusterName, String namespa
216217
}
217218
List<ItemDTO> baseItems = itemAPI.findItems(appId, env, clusterName, namespaceName);
218219
Map<String, ItemDTO> oldKeyMapItem = BeanUtils.mapByKey("key", baseItems);
219-
Map<String, ItemDTO> deletedItemDTOs = new HashMap<>();
220+
//remove comment and blank item map.
221+
oldKeyMapItem.remove("");
220222

221223
//deleted items for comment
222-
findDeletedItems(appId, env, clusterName, namespaceName).forEach(item -> {
223-
deletedItemDTOs.put(item.getKey(),item);
224-
});
224+
Map<String, ItemDTO> deletedItemDTOs = findDeletedItems(appId, env, clusterName, namespaceName).stream()
225+
.filter(itemDTO -> !StringUtils.isEmpty(itemDTO.getKey()))
226+
.collect(Collectors.toMap(itemDTO -> itemDTO.getKey(), v -> v, (v1, v2) -> v2));
225227

226228
ItemChangeSets changeSets = new ItemChangeSets();
227229
AtomicInteger lineNum = new AtomicInteger(1);
@@ -238,7 +240,7 @@ public void revokeItem(String appId, Env env, String clusterName, String namespa
238240
oldKeyMapItem.remove(key);
239241
lineNum.set(lineNum.get() + 1);
240242
});
241-
oldKeyMapItem.forEach((key, value) -> changeSets.addDeleteItem(oldKeyMapItem.get(key)));
243+
oldKeyMapItem.forEach((key, value) -> changeSets.addDeleteItem(value));
242244
changeSets.setDataChangeLastModifiedBy(userInfoHolder.getUser().getUserId());
243245

244246
updateItems(appId, env, clusterName, namespaceName, changeSets);

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/NamespaceService.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ private NamespaceBO transformNamespace2BO(Env env, NamespaceDTO namespace, boole
309309
//latest Release
310310
ReleaseDTO latestRelease;
311311
Map<String, String> releaseItems = new HashMap<>();
312-
Map<String, ItemDTO> deletedItemDTOs = new HashMap<>();
313312
latestRelease = releaseService.loadLatestRelease(appId, env, clusterName, namespaceName);
314313
if (latestRelease != null) {
315314
releaseItems = GSON.fromJson(latestRelease.getConfigurations(), GsonType.CONFIG);
@@ -333,9 +332,9 @@ private NamespaceBO transformNamespace2BO(Env env, NamespaceDTO namespace, boole
333332

334333
if (includeDeletedItems) {
335334
//deleted items
336-
itemService.findDeletedItems(appId, env, clusterName, namespaceName).forEach(item -> {
337-
deletedItemDTOs.put(item.getKey(), item);
338-
});
335+
Map<String, ItemDTO> deletedItemDTOs = itemService.findDeletedItems(appId, env, clusterName, namespaceName).stream()
336+
.filter(itemDTO -> !StringUtils.isEmpty(itemDTO.getKey()))
337+
.collect(Collectors.toMap(itemDTO -> itemDTO.getKey(), v -> v, (v1, v2) -> v2));
339338

340339
List<ItemBO> deletedItems = parseDeletedItems(items, releaseItems, deletedItemDTOs);
341340
itemBOs.addAll(deletedItems);
@@ -385,6 +384,8 @@ private void fillAppNamespaceProperties(NamespaceBO namespace) {
385384

386385
private List<ItemBO> parseDeletedItems(List<ItemDTO> newItems, Map<String, String> releaseItems, Map<String, ItemDTO> deletedItemDTOs) {
387386
Map<String, ItemDTO> newItemMap = BeanUtils.mapByKey("key", newItems);
387+
//remove comment and blank item map.
388+
newItemMap.remove("");
388389

389390
List<ItemBO> deletedItems = new LinkedList<>();
390391
for (Map.Entry<String, String> entry : releaseItems.entrySet()) {

apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/component/txtresolver/PropertyResolverTest.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ public void testDeleteItem() {
9191
@Test
9292
public void testDeleteCommentItem() {
9393
ItemChangeSets changeSets = resolver.resolve(1, "a=b\n\nb=c", mockBaseItemWith2Key1Comment1Blank());
94-
Assert.assertEquals(2, changeSets.getDeleteItems().size());
95-
Assert.assertEquals(2, changeSets.getUpdateItems().size());
96-
Assert.assertEquals(1, changeSets.getCreateItems().size());
94+
Assert.assertEquals(1, changeSets.getDeleteItems().size());
95+
Assert.assertEquals(3, changeSets.getUpdateItems().size());
96+
Assert.assertEquals(0, changeSets.getCreateItems().size());
9797
}
9898

9999
@Test
@@ -120,17 +120,17 @@ public void testUpdateCommentItem() {
120120
+ "a=b\n"
121121
+"\n"
122122
+ "b=c", mockBaseItemWith2Key1Comment1Blank());
123-
Assert.assertEquals(1, changeSets.getDeleteItems().size());
124-
Assert.assertEquals(0, changeSets.getUpdateItems().size());
125-
Assert.assertEquals(1, changeSets.getCreateItems().size());
123+
Assert.assertEquals(0, changeSets.getDeleteItems().size());
124+
Assert.assertEquals(1, changeSets.getUpdateItems().size());
125+
Assert.assertEquals(0, changeSets.getCreateItems().size());
126126
}
127127

128128
@Test
129129
public void testAllSituation(){
130130
ItemChangeSets changeSets = resolver.resolve(1, "#ww\nd=e\nb=c\na=b\n\nq=w\n#eee", mockBaseItemWith2Key1Comment1Blank());
131-
Assert.assertEquals(2, changeSets.getDeleteItems().size());
132-
Assert.assertEquals(2, changeSets.getUpdateItems().size());
133-
Assert.assertEquals(5, changeSets.getCreateItems().size());
131+
Assert.assertEquals(0, changeSets.getDeleteItems().size());
132+
Assert.assertEquals(4, changeSets.getUpdateItems().size());
133+
Assert.assertEquals(3, changeSets.getCreateItems().size());
134134
}
135135

136136
/**

0 commit comments

Comments
 (0)