Skip to content

Commit d207c32

Browse files
authored
Merge pull request apolloconfig#561 from lepdou/lock
bugfix: do not unlock branch's lock when redo operate
2 parents 75878e1 + 0fd4847 commit d207c32

File tree

2 files changed

+78
-21
lines changed

2 files changed

+78
-21
lines changed

apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/aop/NamespaceUnlockAspect.java

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.ctrip.framework.apollo.adminservice.aop;
22

33

4+
import com.google.common.collect.MapDifference;
45
import com.google.common.collect.Maps;
56
import com.google.gson.Gson;
67

@@ -35,7 +36,6 @@
3536
* --------------------------------------------
3637
* First operate: change k1 = v2 (lock namespace)
3738
* Second operate: change k1 = v1 (unlock namespace)
38-
*
3939
*/
4040
@Aspect
4141
@Component
@@ -106,42 +106,52 @@ boolean isModified(Namespace namespace) {
106106
}
107107

108108
Map<String, String> releasedConfiguration = gson.fromJson(release.getConfigurations(), GsonType.CONFIG);
109-
Map<String, String> configurationFromItems = Maps.newHashMap();
109+
Map<String, String> configurationFromItems = generateConfigurationFromItems(namespace, items);
110+
111+
MapDifference<String, String> difference = Maps.difference(releasedConfiguration, configurationFromItems);
110112

113+
return !difference.areEqual();
114+
115+
}
116+
117+
private boolean hasNormalItems(List<Item> items) {
111118
for (Item item : items) {
112-
String key = item.getKey();
113-
if (StringUtils.isBlank(key)) {
114-
continue;
115-
}
116-
//added
117-
if (releasedConfiguration.get(key) == null) {
119+
if (!StringUtils.isEmpty(item.getKey())) {
118120
return true;
119121
}
120-
configurationFromItems.put(key, item.getValue());
121122
}
122123

123-
for (Map.Entry<String, String> entry : releasedConfiguration.entrySet()) {
124-
String key = entry.getKey();
125-
String value = entry.getValue();
124+
return false;
125+
}
126+
127+
private Map<String, String> generateConfigurationFromItems(Namespace namespace, List<Item> namespaceItems) {
126128

127-
//deleted or modified
128-
if (!Objects.equals(configurationFromItems.get(key), value)) {
129-
return true;
130-
}
129+
Map<String, String> configurationFromItems = Maps.newHashMap();
131130

131+
Namespace parentNamespace = namespaceService.findParentNamespace(namespace);
132+
//parent namespace
133+
if (parentNamespace == null) {
134+
generateMapFromItems(namespaceItems, configurationFromItems);
135+
} else {//child namespace
136+
List<Item> parentItems = itemService.findItems(parentNamespace.getId());
137+
138+
generateMapFromItems(parentItems, configurationFromItems);
139+
generateMapFromItems(namespaceItems, configurationFromItems);
132140
}
133141

134-
return false;
142+
return configurationFromItems;
135143
}
136144

137-
private boolean hasNormalItems(List<Item> items) {
145+
private Map<String, String> generateMapFromItems(List<Item> items, Map<String, String> configurationFromItems) {
138146
for (Item item : items) {
139-
if (!StringUtils.isEmpty(item.getKey())) {
140-
return true;
147+
String key = item.getKey();
148+
if (StringUtils.isBlank(key)) {
149+
continue;
141150
}
151+
configurationFromItems.put(key, item.getValue());
142152
}
143153

144-
return false;
154+
return configurationFromItems;
145155
}
146156

147157
}

apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/aop/NamespaceUnlockAspectTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.ctrip.framework.apollo.biz.entity.Namespace;
55
import com.ctrip.framework.apollo.biz.entity.Release;
66
import com.ctrip.framework.apollo.biz.service.ItemService;
7+
import com.ctrip.framework.apollo.biz.service.NamespaceService;
78
import com.ctrip.framework.apollo.biz.service.ReleaseService;
89

910
import org.junit.Assert;
@@ -26,6 +27,8 @@ public class NamespaceUnlockAspectTest {
2627
private ReleaseService releaseService;
2728
@Mock
2829
private ItemService itemService;
30+
@Mock
31+
private NamespaceService namespaceService;
2932

3033
@InjectMocks
3134
private NamespaceUnlockAspect namespaceUnlockAspect;
@@ -55,6 +58,7 @@ public void testNamespaceAddItem() {
5558

5659
when(releaseService.findLatestActiveRelease(namespace)).thenReturn(release);
5760
when(itemService.findItems(namespaceId)).thenReturn(items);
61+
when(namespaceService.findParentNamespace(namespace)).thenReturn(null);
5862

5963
boolean isModified = namespaceUnlockAspect.isModified(namespace);
6064

@@ -71,6 +75,7 @@ public void testNamespaceModifyItem() {
7175

7276
when(releaseService.findLatestActiveRelease(namespace)).thenReturn(release);
7377
when(itemService.findItems(namespaceId)).thenReturn(items);
78+
when(namespaceService.findParentNamespace(namespace)).thenReturn(null);
7479

7580
boolean isModified = namespaceUnlockAspect.isModified(namespace);
7681

@@ -87,12 +92,54 @@ public void testNamespaceDeleteItem() {
8792

8893
when(releaseService.findLatestActiveRelease(namespace)).thenReturn(release);
8994
when(itemService.findItems(namespaceId)).thenReturn(items);
95+
when(namespaceService.findParentNamespace(namespace)).thenReturn(null);
9096

9197
boolean isModified = namespaceUnlockAspect.isModified(namespace);
9298

9399
Assert.assertTrue(isModified);
94100
}
95101

102+
@Test
103+
public void testChildNamespaceModified() {
104+
long childNamespaceId = 1, parentNamespaceId = 2;
105+
Namespace childNamespace = createNamespace(childNamespaceId);
106+
Namespace parentNamespace = createNamespace(childNamespaceId);
107+
108+
Release childRelease = createRelease("{\"k1\":\"v1\", \"k2\":\"v2\"}");
109+
List<Item> childItems = Arrays.asList(createItem("k1", "v3"));
110+
List<Item> parentItems = Arrays.asList(createItem("k1", "v1"), createItem("k2", "v2"));
111+
112+
when(releaseService.findLatestActiveRelease(childNamespace)).thenReturn(childRelease);
113+
when(itemService.findItems(childNamespaceId)).thenReturn(childItems);
114+
when(itemService.findItems(parentNamespaceId)).thenReturn(parentItems);
115+
when(namespaceService.findParentNamespace(parentNamespace)).thenReturn(parentNamespace);
116+
117+
boolean isModified = namespaceUnlockAspect.isModified(parentNamespace);
118+
119+
Assert.assertTrue(isModified);
120+
}
121+
122+
@Test
123+
public void testChildNamespaceNotModified() {
124+
long childNamespaceId = 1, parentNamespaceId = 2;
125+
Namespace childNamespace = createNamespace(childNamespaceId);
126+
Namespace parentNamespace = createNamespace(childNamespaceId);
127+
128+
Release childRelease = createRelease("{\"k1\":\"v1\", \"k2\":\"v2\"}");
129+
List<Item> childItems = Arrays.asList(createItem("k1", "v1"));
130+
List<Item> parentItems = Arrays.asList(createItem("k2", "v2"));
131+
132+
when(releaseService.findLatestActiveRelease(childNamespace)).thenReturn(childRelease);
133+
when(itemService.findItems(childNamespaceId)).thenReturn(childItems);
134+
when(itemService.findItems(parentNamespaceId)).thenReturn(parentItems);
135+
when(namespaceService.findParentNamespace(parentNamespace)).thenReturn(parentNamespace);
136+
137+
boolean isModified = namespaceUnlockAspect.isModified(parentNamespace);
138+
139+
Assert.assertTrue(isModified);
140+
}
141+
142+
96143
private Namespace createNamespace(long namespaceId) {
97144
Namespace namespace = new Namespace();
98145
namespace.setId(namespaceId);

0 commit comments

Comments
 (0)