-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
feat: add determine appid+cluster namespace num limit logic #5228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
4d29c26
abb9a20
2195d79
4ae8bee
a946ad1
e562606
cded46c
2ccd935
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| package com.ctrip.framework.apollo.biz.service; | ||
|
|
||
| import com.ctrip.framework.apollo.biz.AbstractIntegrationTest; | ||
| import com.ctrip.framework.apollo.biz.config.BizConfig; | ||
| import com.ctrip.framework.apollo.biz.entity.Cluster; | ||
| import com.ctrip.framework.apollo.biz.entity.Commit; | ||
| import com.ctrip.framework.apollo.biz.entity.InstanceConfig; | ||
|
|
@@ -25,23 +26,31 @@ | |
| import com.ctrip.framework.apollo.biz.entity.Release; | ||
| import com.ctrip.framework.apollo.biz.entity.ReleaseHistory; | ||
| import com.ctrip.framework.apollo.biz.repository.InstanceConfigRepository; | ||
| import com.ctrip.framework.apollo.biz.repository.NamespaceRepository; | ||
| import com.ctrip.framework.apollo.common.entity.AppNamespace; | ||
|
|
||
| import com.ctrip.framework.apollo.common.exception.ServiceException; | ||
| import java.text.ParseException; | ||
| import java.text.SimpleDateFormat; | ||
| import java.util.Arrays; | ||
| import java.util.Date; | ||
| import java.util.HashSet; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.boot.test.mock.mockito.MockBean; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.PageRequest; | ||
| import org.springframework.test.context.jdbc.Sql; | ||
|
|
||
| import java.util.List; | ||
| import org.springframework.test.util.ReflectionTestUtils; | ||
|
|
||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertNotNull; | ||
| import static org.junit.Assert.assertNull; | ||
| import static org.junit.Assert.assertTrue; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| public class NamespaceServiceIntegrationTest extends AbstractIntegrationTest { | ||
|
|
||
|
|
@@ -62,6 +71,11 @@ public class NamespaceServiceIntegrationTest extends AbstractIntegrationTest { | |
| private ReleaseHistoryService releaseHistoryService; | ||
| @Autowired | ||
| private InstanceConfigRepository instanceConfigRepository; | ||
| @Autowired | ||
| private NamespaceRepository namespaceRepository; | ||
|
|
||
| @MockBean | ||
| private BizConfig bizConfig; | ||
|
|
||
| private String testApp = "testApp"; | ||
| private String testCluster = "default"; | ||
|
|
@@ -134,4 +148,85 @@ public void testGetCommitsByModifiedTime() throws ParseException { | |
| } | ||
|
|
||
|
|
||
| @Test | ||
| @Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||
| @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) | ||
| public void testNamespaceNumLimit() { | ||
|
|
||
| when(bizConfig.isNamespaceNumLimitEnabled()).thenReturn(true); | ||
| when(bizConfig.namespaceNumLimit()).thenReturn(2); | ||
|
|
||
| Namespace namespace = new Namespace(); | ||
| namespace.setAppId(testApp); | ||
| namespace.setClusterName(testCluster); | ||
| namespace.setNamespaceName("demo-namespace"); | ||
| namespaceService.save(namespace); | ||
|
|
||
| try { | ||
| Namespace namespace2 = new Namespace(); | ||
| namespace2.setAppId(testApp); | ||
| namespace2.setClusterName(testCluster); | ||
| namespace2.setNamespaceName("demo-namespace2"); | ||
| namespaceService.save(namespace2); | ||
|
|
||
| Assert.fail(); | ||
| } catch (Exception e) { | ||
| Assert.assertTrue(e instanceof ServiceException); | ||
| } | ||
|
|
||
| int nowCount = namespaceRepository.countByAppIdAndClusterName(testApp, testCluster); | ||
| Assert.assertEquals(2, nowCount); | ||
|
|
||
| } | ||
|
Comment on lines
+151
to
+180
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve exception handling in testNamespaceNumLimit The test effectively verifies the namespace number limit functionality. However, the exception handling can be improved for better clarity and specificity. Consider using JUnit's @Test
@Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
@Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
public void testNamespaceNumLimit() {
when(bizConfig.isNamespaceNumLimitEnabled()).thenReturn(true);
when(bizConfig.namespaceNumLimit()).thenReturn(2);
Namespace namespace = new Namespace();
namespace.setAppId(testApp);
namespace.setClusterName(testCluster);
namespace.setNamespaceName("demo-namespace");
namespaceService.save(namespace);
Namespace namespace2 = new Namespace();
namespace2.setAppId(testApp);
namespace2.setClusterName(testCluster);
namespace2.setNamespaceName("demo-namespace2");
assertThrows(ServiceException.class, () -> namespaceService.save(namespace2));
int nowCount = namespaceRepository.countByAppIdAndClusterName(testApp, testCluster);
Assert.assertEquals(2, nowCount);
}This refactoring improves readability and makes the test's intention clearer. |
||
|
|
||
| @Test | ||
| @Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||
| @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) | ||
| public void testNamespaceNumLimitFalse() { | ||
|
|
||
| when(bizConfig.namespaceNumLimit()).thenReturn(2); | ||
|
|
||
| Namespace namespace = new Namespace(); | ||
| namespace.setAppId(testApp); | ||
| namespace.setClusterName(testCluster); | ||
| namespace.setNamespaceName("demo-namespace"); | ||
| namespaceService.save(namespace); | ||
|
|
||
| Namespace namespace2 = new Namespace(); | ||
| namespace2.setAppId(testApp); | ||
| namespace2.setClusterName(testCluster); | ||
| namespace2.setNamespaceName("demo-namespace2"); | ||
| namespaceService.save(namespace2); | ||
|
|
||
| int nowCount = namespaceRepository.countByAppIdAndClusterName(testApp, testCluster); | ||
| Assert.assertEquals(3, nowCount); | ||
|
|
||
| } | ||
youngzil marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+182
to
+204
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve test setup in testNamespaceNumLimitFalse The test effectively verifies the behavior when the namespace limit is not enforced. However, it could be improved by explicitly setting Consider adding an explicit mock for @Test
@Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
@Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
public void testNamespaceNumLimitFalse() {
when(bizConfig.isNamespaceNumLimitEnabled()).thenReturn(false);
when(bizConfig.namespaceNumLimit()).thenReturn(2);
Namespace namespace = new Namespace();
namespace.setAppId(testApp);
namespace.setClusterName(testCluster);
namespace.setNamespaceName("demo-namespace");
namespaceService.save(namespace);
Namespace namespace2 = new Namespace();
namespace2.setAppId(testApp);
namespace2.setClusterName(testCluster);
namespace2.setNamespaceName("demo-namespace2");
namespaceService.save(namespace2);
int nowCount = namespaceRepository.countByAppIdAndClusterName(testApp, testCluster);
Assert.assertEquals(3, nowCount);
}This change makes it explicit that the namespace limit is disabled, improving the test's readability and intent. |
||
|
|
||
| @Test | ||
| @Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||
| @Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD) | ||
| public void testNamespaceNumLimitWhite() { | ||
|
|
||
| when(bizConfig.isNamespaceNumLimitEnabled()).thenReturn(true); | ||
| when(bizConfig.namespaceNumLimit()).thenReturn(2); | ||
| when(bizConfig.namespaceNumLimitWhite()).thenReturn(new HashSet<>(Arrays.asList(testApp))); | ||
|
|
||
| Namespace namespace = new Namespace(); | ||
| namespace.setAppId(testApp); | ||
| namespace.setClusterName(testCluster); | ||
| namespace.setNamespaceName("demo-namespace"); | ||
| namespaceService.save(namespace); | ||
|
|
||
| Namespace namespace2 = new Namespace(); | ||
| namespace2.setAppId(testApp); | ||
| namespace2.setClusterName(testCluster); | ||
| namespace2.setNamespaceName("demo-namespace2"); | ||
| namespaceService.save(namespace2); | ||
|
|
||
| int nowCount = namespaceRepository.countByAppIdAndClusterName(testApp, testCluster); | ||
| Assert.assertEquals(3, nowCount); | ||
|
|
||
| } | ||
youngzil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.