diff --git a/CHANGES.md b/CHANGES.md index e78980af6bb..4bcbc9b589f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -22,5 +22,6 @@ Apollo 2.5.0 * [optimize: Implement unified permission verification logic and Optimize the implementation of permission verification](https://github.com/apolloconfig/apollo/pull/5456) * [CI: Add code and header formatter by spotless plugin](https://github.com/apolloconfig/apollo/pull/5485) * [Fix: Operate the AccessKey multiple times within one second](https://github.com/apolloconfig/apollo/pull/5490) +* [Bugfix: Prevent accidental cache deletion when recreating AppNamespace with the same name and appid](https://github.com/apolloconfig/apollo/issues/5502) ------------------ All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/16?closed=1) diff --git a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java index 70e9fc5661a..c164fbba4dc 100644 --- a/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java +++ b/apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCache.java @@ -37,10 +37,7 @@ import org.springframework.stereotype.Service; import org.springframework.util.CollectionUtils; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -261,7 +258,13 @@ private void handleDeletedAppNamespaces(Set deletedIds) { if (deleted == null) { continue; } - appNamespaceCache.remove(assembleAppNamespaceKey(deleted)); + + String appNamespaceKey = assembleAppNamespaceKey(deleted); + AppNamespace appNamespace = appNamespaceCache.get(appNamespaceKey); + if (appNamespace != null && Objects.equals(appNamespace.getId(), deletedId)) { + appNamespaceCache.remove(appNamespaceKey); + } + if (deleted.isPublic()) { AppNamespace publicAppNamespace = publicAppNamespaceCache.get(deleted.getName()); // in case there is some dirty data, e.g. public namespace deleted in some app and now diff --git a/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCacheTest.java b/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCacheTest.java index 9334d600228..ada0d05ed5c 100644 --- a/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCacheTest.java +++ b/apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/service/AppNamespaceServiceWithCacheTest.java @@ -38,6 +38,7 @@ import static org.awaitility.Awaitility.await; import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.when; /** @@ -311,4 +312,54 @@ private AppNamespace assembleAppNamespace(long id, String appId, String name, bo appNamespace.setDataChangeLastModifiedTime(new Date()); return appNamespace; } + + /** + * fix issue #5502 + */ + @Test + public void doubleCheckForAppNamespaceCache() throws Exception { + String appId = "app1"; + String namespaceName = "name1"; + + AppNamespace oldNs = assembleAppNamespace(1, appId, namespaceName, false); + + AppNamespace newNs = assembleAppNamespace(2, appId, namespaceName, false); + + when(appNamespaceRepository.findFirst500ByIdGreaterThanOrderByIdAsc(0)) + .thenReturn(Lists.newArrayList(oldNs)); + + invokePrivateMethod("scanNewAppNamespaces"); + + AppNamespace cached = + appNamespaceServiceWithCache.findByAppIdAndNamespace(appId, namespaceName); + assertNotNull(cached); + assertEquals(1, cached.getId()); + + when(appNamespaceRepository.findFirst500ByIdGreaterThanOrderByIdAsc(1)) + .thenReturn(Lists.newArrayList(newNs)); + + invokePrivateMethod("scanNewAppNamespaces"); + + cached = appNamespaceServiceWithCache.findByAppIdAndNamespace(appId, namespaceName); + assertNotNull(cached); + assertEquals(2, cached.getId()); + + when(appNamespaceRepository.findAllById(anyList())) + .thenAnswer(invocation -> Lists.newArrayList(newNs)); + + invokePrivateMethod("updateAndDeleteCache"); + + cached = appNamespaceServiceWithCache.findByAppIdAndNamespace(appId, namespaceName); + + assertNotNull("new cache is accidentally deleted", cached); + assertEquals(2, cached.getId()); + } + + private void invokePrivateMethod(String methodName) throws Exception { + java.lang.reflect.Method method = + AppNamespaceServiceWithCache.class.getDeclaredMethod(methodName); + method.setAccessible(true); + method.invoke(appNamespaceServiceWithCache); + } + }