You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by dm...@apache.org on 2019/06/11 13:05:49 UTC
[ambari] branch branch-2.7 updated: AMBARI-25300. Improper Error
Handling when user creates a duplicate alert (dlysnichenko) (#3005)
This is an automated email from the ASF dual-hosted git repository.
dmitriusan pushed a commit to branch branch-2.7
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/branch-2.7 by this push:
new a2514a5 AMBARI-25300. Improper Error Handling when user creates a duplicate alert (dlysnichenko) (#3005)
a2514a5 is described below
commit a2514a5bac549f0d853f03b5eac6a5a4ae185cb9
Author: Lisnichenko Dmitro <dl...@hortonworks.com>
AuthorDate: Tue Jun 11 16:05:41 2019 +0300
AMBARI-25300. Improper Error Handling when user creates a duplicate alert (dlysnichenko) (#3005)
---
.../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 dc0fd57..6489ecf 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 efedd2b..c921806 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);
@@ -825,6 +833,60 @@ public class AlertTargetResourceProviderTest {
}
@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);