From: k.kazak Date: Thu, 18 Apr 2019 12:46:20 +0000 (+0200) Subject: Fix Sonar Bugs NullPointers X-Git-Tag: 3.2.0~324^2 X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;ds=sidebyside;h=7f6fd40106f158273b74816e86614eb883f313ed;p=portal.git Fix Sonar Bugs NullPointers Fix Sonar bugs - potential NullPointerException Remove unused code Add/Update Tests for updated methods Change-Id: Iaf28a0ee5f6507da1caf4b497e2d73bf35bc0b58 Issue-ID: PORTAL-556 Signed-off-by: k.kazak --- diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsController.java index b5bd02bc..789a4097 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/AppsController.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -37,13 +39,13 @@ */ package org.onap.portalapp.portal.controller; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; import java.util.List; import java.util.Set; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import org.onap.portalapp.controller.EPRestrictedBaseController; import org.onap.portalapp.portal.domain.AdminUserApplications; import org.onap.portalapp.portal.domain.AppIdAndNameTransportModel; @@ -80,9 +82,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; - @RestController @EnableAspectJAutoProxy @EPAuditLog @@ -711,8 +710,9 @@ public class AppsController extends EPRestrictedBaseController { try { String oldvaluesAsJson = new ObjectMapper().writeValueAsString(oldEPApp); String newvaluesAsJson = new ObjectMapper().writeValueAsString(modifiedOnboardingApp); - logger.info(EELFLoggerDelegate.auditLogger, "/portalApi/onboardingApps, old values ="+oldvaluesAsJson); - logger.info(EELFLoggerDelegate.auditLogger, "/portalApi/onboardingApps, loginId="+user.getLoginId()+", new values ="+newvaluesAsJson); + logger.info(EELFLoggerDelegate.auditLogger, "/portalApi/onboardingApps, old values ={}", oldvaluesAsJson); + logger.info(EELFLoggerDelegate.auditLogger, "/portalApi/onboardingApps, loginId={}, new values ={}", + user != null ? user.getLoginId() : "", newvaluesAsJson); } catch (JsonProcessingException e) { logger.error(EELFLoggerDelegate.errorLogger, "putOnboardingApps failed", e); } diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/RoleManageController.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/RoleManageController.java index c6849cd8..c8e22d39 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/RoleManageController.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/controller/RoleManageController.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -694,7 +696,7 @@ public class RoleManageController extends EPRestrictedBaseController { externalAccessRolesService.syncApplicationRolesWithEcompDB(app); } else { logger.info(EELFLoggerDelegate.auditLogger, - "RoleManageController.syncRoles, Unauthorized user:" + user.getOrgUserId()); + "RoleManageController.syncRoles, Unauthorized user:{}", user != null ? user.getOrgUserId() : ""); EcompPortalUtils.setBadPermissions(user, response, "createAdmin"); return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Unauthorized User", "Failure"); } @@ -713,10 +715,10 @@ public class RoleManageController extends EPRestrictedBaseController { EPApp app = appService.getApp(appId); if (isAuthorizedUser(user, app)) { fieldsValidation(app); - externalAccessRolesService.syncRoleFunctionFromExternalAccessSystem(app);; + externalAccessRolesService.syncRoleFunctionFromExternalAccessSystem(app); } else { logger.info(EELFLoggerDelegate.auditLogger, - "RoleManageController.syncFunctions, Unauthorized user:" + user.getOrgUserId()); + "RoleManageController.syncFunctions, Unauthorized user:{}", user != null ? user.getOrgUserId() : ""); EcompPortalUtils.setBadPermissions(user, response, "createAdmin"); return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Unauthorized User", "Failure"); } @@ -804,7 +806,7 @@ public class RoleManageController extends EPRestrictedBaseController { logExterlaAuthRoleFunctionActivity(code , user, app, activityCode); } else { logger.info(EELFLoggerDelegate.auditLogger, - "RoleManageController.syncRoles, Unauthorized user:" + user.getOrgUserId()); + "RoleManageController.syncRoles, Unauthorized user:{}", user != null ? user.getOrgUserId() : ""); EcompPortalUtils.setBadPermissions(user, response, "createAdmin"); return new PortalRestResponse<>(PortalRestStatusEnum.ERROR, "Unauthorized User", "Failure"); } diff --git a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/AdminRolesServiceImpl.java b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/AdminRolesServiceImpl.java index c8e04f4f..7099eda5 100644 --- a/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/AdminRolesServiceImpl.java +++ b/ecomp-portal-BE-common/src/main/java/org/onap/portalapp/portal/service/AdminRolesServiceImpl.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -143,9 +145,9 @@ public class AdminRolesServiceImpl implements AdminRolesService { try { userList = dataAccessService.executeNamedQuery("getEPUserByOrgUserId", userParams, null); } catch (Exception e) { - logger.error(EELFLoggerDelegate.errorLogger, "getEPUserByOrgUserId failed", e); + logger.error(EELFLoggerDelegate.errorLogger, "getEPUserByOrgUserId failed", e); } - + HashMap appsUserAdmin = new HashMap(); if (userList!= null && userList.size() > 0) { EPUser user = userList.get(0); @@ -265,7 +267,7 @@ public class AdminRolesServiceImpl implements AdminRolesService { // Add user admin role for list of centralized applications in external system addAdminRoleInExternalSystem(user, localSession, newAppsWhereUserIsAdmin); result = true; - } + } } catch (Exception e) { EPLogUtil.logEcompError(logger, EPAppMessagesEnum.BeDaoSystemError, e); logger.error(EELFLoggerDelegate.errorLogger, @@ -444,12 +446,14 @@ public class AdminRolesServiceImpl implements AdminRolesService { public boolean isAccountAdmin(EPUser user) { try { - EPUser currentUser = user != null - ? (EPUser) dataAccessService.getDomainObject(EPUser.class, user.getId(), null) - : null; - + if (user == null) { + return false; + } + + EPUser currentUser = (EPUser) dataAccessService.getDomainObject(EPUser.class, user.getId(), null); + final Map userParams = new HashMap<>(); - userParams.put("userId", user.getId()); + userParams.put("userId", user.getId()); logger.debug(EELFLoggerDelegate.debugLogger, "Is account admin for user {}", user.getId()); List userAdminApps = new ArrayList<>(); @@ -460,7 +464,7 @@ public class AdminRolesServiceImpl implements AdminRolesService { if (currentUser != null && currentUser.getId() != null) { for (EPUserApp userApp : currentUser.getEPUserApps()) { - + if (userApp.getRole().getId().equals(ACCOUNT_ADMIN_ROLE_ID)||(userAdminApps.size()>1)) { logger.debug(EELFLoggerDelegate.debugLogger, "Is account admin for userAdminApps() - for user {}, found Id {}", user.getOrgUserId(), userApp.getRole().getId()); // Account Administrator sees only the applications @@ -476,19 +480,14 @@ public class AdminRolesServiceImpl implements AdminRolesService { } return false; } - - + + public boolean isRoleAdmin(EPUser user) { try { logger.debug(EELFLoggerDelegate.debugLogger, "Checking if user has isRoleAdmin access"); - EPUser currentUser = user != null - ? (EPUser) dataAccessService.getDomainObject(EPUser.class, user.getId(), null) - : null; final Map userParams = new HashMap<>(); - userParams.put("userId", user.getId()); - List roleFunctionSet = new ArrayList<>(); - + userParams.put("userId", user.getId()); List getRoleFuncListOfUser = dataAccessService.executeNamedQuery("getRoleFunctionsOfUserforAlltheApplications", userParams, null); logger.debug(EELFLoggerDelegate.debugLogger, "Checking if user has isRoleAdmin access :: getRoleFuncListOfUser" , getRoleFuncListOfUser); Set getRoleFuncListOfPortalSet = new HashSet<>(getRoleFuncListOfUser); @@ -497,15 +496,10 @@ public class AdminRolesServiceImpl implements AdminRolesService { roleFunSet = getRoleFuncListOfPortalSet.stream().filter(x -> x.contains("|")).collect(Collectors.toSet()); if (roleFunSet.size() > 0) for (String roleFunction : roleFunSet) { - //String roleFun = EcompPortalUtils.getFunctionCode(roleFunction); - String roleFun = EcompPortalUtils.getFunctionCode(roleFunction); String type = externalAccessRolesService.getFunctionCodeType(roleFunction); - //getRoleFuncListOfPortalSet.remove(roleFunction); getRoleFuncListOfPortalSet1.add(type); } - - - + for (String rolefunc : getRoleFuncListOfPortalSet1) { logger.debug(EELFLoggerDelegate.debugLogger, "Checking if user has approver rolefunction" , rolefunc); if (rolefunc.equalsIgnoreCase(TYPE_APPROVER)) { @@ -516,8 +510,7 @@ public class AdminRolesServiceImpl implements AdminRolesService { } } - - + } catch (Exception e) { EPLogUtil.logEcompError(logger, EPAppMessagesEnum.BeDaoSystemError, e); logger.error(EELFLoggerDelegate.errorLogger, "Exception occurred while executing isRoleAdmin operation", @@ -568,14 +561,14 @@ public class AdminRolesServiceImpl implements AdminRolesService { Boolean isApplicationAccountAdmin=false; try { final Map userParams = new HashMap<>(); - userParams.put("userId", user.getId()); + userParams.put("userId", user.getId()); logger.debug(EELFLoggerDelegate.debugLogger, "Is account admin for user {}", user.getId()); List userAdminApps = new ArrayList<>(); userAdminApps =dataAccessService.executeNamedQuery("getAdminAppsForTheUser", userParams, null); if(userAdminApps.size()>=1){ isApplicationAccountAdmin=userAdminApps.contains((int) (long) app.getId()); logger.debug(EELFLoggerDelegate.debugLogger, "Is account admin for user is true{} ,appId {}", user.getId(),app.getId()); - } + } } catch (Exception e) { EPLogUtil.logEcompError(logger, EPAppMessagesEnum.BeDaoSystemError, e); logger.error(EELFLoggerDelegate.errorLogger, diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerTest.java index ff37fd50..59b5a8ed 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/AppsControllerTest.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -733,6 +735,7 @@ public class AppsControllerTest extends MockitoTestSuite{ expectedFieldValidator.setErrorCode(null); Mockito.when(adminRolesService.isSuperAdmin(user)).thenReturn(true); Mockito.when(appService.modifyOnboardingApp(OnboardingApp, user)).thenReturn(expectedFieldValidator); + Mockito.when(mockedResponse.getStatus()).thenReturn(200); FieldsValidator actualFieldValidator = appsController.putOnboardingApp(mockedRequest, OnboardingApp, mockedResponse); assertEquals(expectedFieldValidator, actualFieldValidator); @@ -758,6 +761,13 @@ public class AppsControllerTest extends MockitoTestSuite{ Mockito.when(appService.modifyOnboardingApp(OnboardingApp, user)).thenThrow(nullPointerException); assertNull(appsController.putOnboardingApp(mockedRequest, OnboardingApp, mockedResponse)); } + + @Test + public void putOnboardingAppNullUserTest() { + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenThrow(nullPointerException); + Mockito.when(mockedResponse.getStatus()).thenReturn(200); + assertNull(appsController.putOnboardingApp(mockedRequest, new OnboardingApp(), mockedResponse)); + } @Test public void postOnboardingAppTest() { diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/RoleManageControllerTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/RoleManageControllerTest.java index ff9fcffc..8bfa39c3 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/RoleManageControllerTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/controller/RoleManageControllerTest.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -76,6 +78,7 @@ import org.onap.portalapp.portal.domain.EPApp; import org.onap.portalapp.portal.domain.EPUser; import org.onap.portalapp.portal.ecomp.model.PortalRestResponse; import org.onap.portalapp.portal.ecomp.model.PortalRestStatusEnum; +import org.onap.portalapp.portal.ecomp.model.UploadRoleFunctionExtSystem; import org.onap.portalapp.portal.framework.MockitoTestSuite; import org.onap.portalapp.portal.service.AdminRolesService; import org.onap.portalapp.portal.service.EPAppService; @@ -474,6 +477,9 @@ public class RoleManageControllerTest { @Test public void syncRolesException() throws Exception { + EPUser user = mockUser.mockEPUser(); + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user); + Mockito.when(adminRolesService.isAccountAdminOfApplication(user, null)).thenReturn(true); Mockito.when(appService.getAppDetailByAppName("test")).thenThrow(nullPointerException); PortalRestResponse actual = roleManageController.syncRoles(mockedRequest, mockedResponse, 1l); PortalRestResponse portalRestResponse = new PortalRestResponse<>(); @@ -483,6 +489,18 @@ public class RoleManageControllerTest { assertEquals(portalRestResponse, actual); } + @Test + public void syncRolesUserNullException() throws Exception { + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(null); + Mockito.when(appService.getAppDetailByAppName("test")).thenThrow(nullPointerException); + PortalRestResponse actual = roleManageController.syncRoles(mockedRequest, mockedResponse, 1l); + PortalRestResponse portalRestResponse = new PortalRestResponse<>(); + portalRestResponse.setMessage("Unauthorized User"); + portalRestResponse.setResponse("Failure"); + portalRestResponse.setStatus(PortalRestStatusEnum.ERROR); + assertEquals(portalRestResponse, actual); + } + @Test public void syncRolesFunctionsTest() throws Exception { PowerMockito.mockStatic(EPUserUtils.class); @@ -510,7 +528,10 @@ public class RoleManageControllerTest { @Test public void syncRolesFunctionsException() throws Exception { - Mockito.when(appService.getAppDetailByAppName("test")).thenThrow(nullPointerException); + EPUser user = mockUser.mockEPUser(); + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(user); + Mockito.when(adminRolesService.isAccountAdminOfApplication(user, null)).thenReturn(true); + Mockito.when(appService.getAppDetailByAppName("test")).thenThrow(nullPointerException); PortalRestResponse actual = roleManageController.syncFunctions(mockedRequest, mockedResponse, 1l); PortalRestResponse portalRestResponse = new PortalRestResponse<>(); portalRestResponse.setMessage(null); @@ -519,6 +540,18 @@ public class RoleManageControllerTest { assertEquals(portalRestResponse, actual); } + @Test + public void syncRolesFunctionsUserNullException() throws Exception { + Mockito.when(EPUserUtils.getUserSession(mockedRequest)).thenReturn(null); + Mockito.when(appService.getAppDetailByAppName("test")).thenThrow(nullPointerException); + PortalRestResponse actual = roleManageController.syncFunctions(mockedRequest, mockedResponse, 1l); + PortalRestResponse portalRestResponse = new PortalRestResponse<>(); + portalRestResponse.setMessage("Unauthorized User"); + portalRestResponse.setResponse("Failure"); + portalRestResponse.setStatus(PortalRestStatusEnum.ERROR); + assertEquals(portalRestResponse, actual); + } + @Test public void addChildRoleTest() throws Exception { ModelAndView modelandView = new ModelAndView("login.htm"); @@ -890,6 +923,16 @@ public class RoleManageControllerTest { List actual = roleManageController.getCentralizedAppRoles(mockedRequest, mockedResponse, user.getOrgUserId()); assertNull(actual); } + + @Test + public void bulkUploadRoleFuncUserNullTest() { + UploadRoleFunctionExtSystem data = Mockito.mock(UploadRoleFunctionExtSystem.class); + Mockito.when(appService.getApp(127L)).thenReturn(null); + PortalRestResponse response = roleManageController.bulkUploadRoleFunc(mockedRequest, mockedResponse, data, 127L); + assertEquals(PortalRestStatusEnum.ERROR, response.getStatus()); + assertEquals("Unauthorized User", response.getMessage()); + assertEquals("Failure", response.getResponse()); + } public CentralV2RoleFunction mockCentralRoleFunction() { CentralV2RoleFunction roleFunction = new CentralV2RoleFunction(); diff --git a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/AdminRolesServiceImplTest.java b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/AdminRolesServiceImplTest.java index 15f021c5..f14d3fd9 100644 --- a/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/AdminRolesServiceImplTest.java +++ b/ecomp-portal-BE-common/src/test/java/org/onap/portalapp/portal/service/AdminRolesServiceImplTest.java @@ -4,6 +4,8 @@ * =================================================================== * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. * =================================================================== + * Modifications Copyright (c) 2019 Samsung + * =================================================================== * * Unless otherwise specified, all software contained herein is licensed * under the Apache License, Version 2.0 (the "License"); @@ -93,6 +95,9 @@ public class AdminRolesServiceImplTest { @Mock DataAccessService dataAccessService = new DataAccessServiceImpl(); + @Mock + ExternalAccessRolesService externalAccessRolesService; + @Mock EPAppCommonServiceImpl epAppCommonServiceImpl = new EPAppCommonServiceImpl(); @@ -370,6 +375,25 @@ public class AdminRolesServiceImplTest { assertFalse(actual); } + @Test + public void isAccountAdminUserNull() { + boolean actual = adminRolesServiceImpl.isAccountAdmin(null); + assertFalse(actual); + } + + @Test + public void isRoleAdminTest() { + EPUser user = mockUser.mockEPUser(); + List roles = new ArrayList<>(); + roles.add("approver\\|"); + Mockito.when(dataAccessService.executeNamedQuery( + Matchers.eq("getRoleFunctionsOfUserforAlltheApplications"), Matchers.any(), Matchers.any())) + .thenReturn(roles); + Mockito.when(externalAccessRolesService.getFunctionCodeType(Matchers.anyString())).thenReturn("approver"); + boolean actual = adminRolesServiceImpl.isRoleAdmin(user); + assertTrue(actual); + } + @Test public void isUserTest() { EPUser user = mockUser.mockEPUser();