You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by wu...@apache.org on 2022/11/10 17:54:55 UTC
[ambari] branch trunk updated: AMBARI-25300: Improper Error Handling when user creates a duplicate alert (#3475)
This is an automated email from the ASF dual-hosted git repository.
wuzhiguo pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new 05c826ec1a AMBARI-25300: Improper Error Handling when user creates a duplicate alert (#3475)
05c826ec1a is described below
commit 05c826ec1a0eb2d672ad84505d9bde95a578fa09
Author: Zhiguo Wu <wu...@apache.org>
AuthorDate: Fri Nov 11 01:54:49 2022 +0800
AMBARI-25300: Improper Error Handling when user creates a duplicate alert (#3475)
---
.../internal/AlertTargetResourceProvider.java | 10 ++--
.../internal/AlertTargetResourceProviderTest.java | 64 ++++++++++++++++++++++
2 files changed, 70 insertions(+), 4 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
index be18477677..3437645541 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
@@ -335,13 +335,15 @@ public class AlertTargetResourceProvider extends
}
// if we are overwriting an existing, determine if one exists first
- AlertTargetEntity entity = null;
- if( overwriteExisting ) {
- entity = s_dao.findTargetByName(name);
- }
+ AlertTargetEntity entity = s_dao.findTargetByName(name);
if (null == entity) {
entity = new AlertTargetEntity();
+ } else {
+ if( !overwriteExisting ) {
+ throw new IllegalArgumentException(
+ "Alert targets already exists and can't be created");
+ }
}
// groups are not required on creation
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
index b20dd9f61a..1bb9f096c7 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
@@ -294,6 +294,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testCreateResources(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> targetCapture = newCapture();
m_dao.create(capture(targetCapture));
expectLastCall();
@@ -364,6 +365,7 @@ public class AlertTargetResourceProviderTest {
group2.setGroupId(2L);
group3.setGroupId(3L);
groups.addAll(Arrays.asList(group1, group2, group3));
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
expect(m_dao.findGroupsById(groupIds)).andReturn(groups).once();
Capture<AlertTargetEntity> targetCapture = EasyMock.newCapture();
@@ -431,6 +433,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testCreateGlobalTarget(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> targetCapture = EasyMock.newCapture();
m_dao.create(capture(targetCapture));
expectLastCall();
@@ -497,6 +500,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testCreateResourceWithRecipientArray(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> targetCapture = EasyMock.newCapture();
m_dao.create(capture(targetCapture));
expectLastCall();
@@ -561,6 +565,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testCreateResourceWithAlertStates(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> targetCapture = EasyMock.newCapture();
m_dao.create(capture(targetCapture));
expectLastCall();
@@ -628,6 +633,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testUpdateResources(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> entityCapture = EasyMock.newCapture();
m_dao.create(capture(entityCapture));
expectLastCall().times(1);
@@ -703,6 +709,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testUpdateResourcesWithGroups(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> entityCapture = EasyMock.newCapture();
m_dao.create(capture(entityCapture));
expectLastCall().times(1);
@@ -785,6 +792,7 @@ public class AlertTargetResourceProviderTest {
* @throws Exception
*/
private void testDeleteResources(Authentication authentication) throws Exception {
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> entityCapture = EasyMock.newCapture();
m_dao.create(capture(entityCapture));
expectLastCall().times(1);
@@ -824,6 +832,60 @@ public class AlertTargetResourceProviderTest {
verify(m_amc, m_dao);
}
+ @Test
+ public void testDuplicateDirectiveAsClusterAdministrator() throws Exception {
+ testDuplicate(TestAuthenticationFactory.createClusterAdministrator());
+ }
+
+ @Test(expected = AuthorizationException.class)
+ public void testDuplicateDirectiveAsServiceAdministrator() throws Exception {
+ testDuplicate(TestAuthenticationFactory.createServiceAdministrator());
+ }
+
+ @Test(expected = AuthorizationException.class)
+ public void testDuplicateDirectiveAsClusterUser() throws Exception {
+ testDuplicate(TestAuthenticationFactory.createClusterUser());
+ }
+
+ @Test(expected = AuthorizationException.class)
+ public void testDuplicateDirectiveAsViewUser() throws Exception {
+ testDuplicate(TestAuthenticationFactory.createViewUser(99L));
+ }
+
+ @Test
+ public void testDuplicateDirectiveAsAdministrator() throws Exception {
+ testDuplicate(TestAuthenticationFactory.createAdministrator());
+ }
+
+
+ public void testDuplicate(Authentication authentication) throws Exception{
+ // mock out returning an existing entity
+ AlertTargetEntity entity = getMockEntities().get(0);
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(entity).atLeastOnce();
+
+ replay(m_amc, m_dao);
+
+ SecurityContextHolder.getContext().setAuthentication(authentication);
+
+ AlertTargetResourceProvider provider = createProvider(m_amc);
+ Map<String, Object> requestProps = getCreationProperties();
+
+ // mock out the directive
+ Map<String, String> requestInfoProperties = new HashMap<>();
+
+ Request request = PropertyHelper.getCreateRequest(
+ Collections.singleton(requestProps), requestInfoProperties);
+
+ try {
+ provider.createResources(request);
+ Assert.fail("IllegalArgumentException expected as duplicate not allowed");
+ } catch (IllegalArgumentException e){
+ assertEquals(e.getMessage(), "Alert targets already exists and can't be created");
+ }
+
+ verify(m_amc, m_dao);
+ }
+
@Test
public void testOverwriteDirectiveAsAdministrator() throws Exception {
testOverwriteDirective(TestAuthenticationFactory.createAdministrator());
@@ -893,6 +955,7 @@ public class AlertTargetResourceProviderTest {
@Test
public void testUpdateAlertTargetsWithCustomGroups() throws Exception{
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> entityCapture = EasyMock.newCapture();
m_dao.create(capture(entityCapture));
expectLastCall().times(1);
@@ -954,6 +1017,7 @@ public class AlertTargetResourceProviderTest {
@Test
public void testUpdateAlertTargetsWithAllGroups() throws Exception{
+ expect(m_dao.findTargetByName(ALERT_TARGET_NAME)).andReturn(null).atLeastOnce();
Capture<AlertTargetEntity> entityCapture = EasyMock.newCapture();
m_dao.create(capture(entityCapture));
expectLastCall().times(1);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ambari.apache.org
For additional commands, e-mail: commits-help@ambari.apache.org