From: Pamela Dragosh Date: Wed, 31 Jan 2018 13:15:42 +0000 (-0500) Subject: Adding code coverage reduce duplicate lines X-Git-Tag: v1.2.0~174 X-Git-Url: https://gerrit.onap.org/r/gitweb?p=policy%2Fengine.git;a=commitdiff_plain;h=9301fed7c290f52208922f780fdfe4b5a880b5aa Adding code coverage reduce duplicate lines Removed some duplicate code across some of the classes. For CheckPDP, got the code coverage above 80%. Issue-ID: POLICY-482 Change-Id: I41495cf9f92e8fd248350bf33f5a183c876f38f2 Signed-off-by: Pamela Dragosh --- diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/CheckPDP.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/CheckPDP.java index 49eb80802..643320496 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/CheckPDP.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/CheckPDP.java @@ -44,6 +44,18 @@ import org.onap.policy.xacml.api.XACMLErrorConstants; import com.att.research.xacml.util.XACMLProperties; +/** + * What is not good about this class is that once a value has been set for pdpProperties path + * you cannot change it. That may be ok for a highly controlled production environment in which + * nothing changes, but not a very good implementation. + * + * The reset() method has been added to assist with the above problem in order to + * acquire >80% JUnit code coverage. + * + * This static class doesn't really check a PDP, it simply loads a properties file and tried + * to ensure that a valid URL exists for a PDP along with user/password. + * + */ public class CheckPDP { private static Path pdpPath = null; private static Long oldModified = null; @@ -57,6 +69,12 @@ public class CheckPDP { public static Map getPdpMap() { return pdpMap; } + + private static void reset() { + pdpPath = null; + oldModified = null; + pdpMap = null; + } public static boolean validateID(String id) { // ReadFile @@ -66,6 +84,9 @@ public class CheckPDP { LOGGER.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + e); return false; } + if (pdpMap == null) { + return false; + } // Check ID return pdpMap.containsKey(id); } @@ -83,15 +104,12 @@ public class CheckPDP { } if (pdpPath == null) { pdpPath = Paths.get(pdpFile); - if (!pdpPath.toFile().exists()) { + if (!pdpPath.toString().endsWith(".properties") || !pdpPath.toFile().exists()) { LOGGER.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "File doesn't exist in the specified Path : " + pdpPath.toString()); - - } - if (pdpPath.toString().endsWith(".properties")) { - readProps(); - } else { - LOGGER.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "Not a .properties file " + pdpFile); + CheckPDP.reset(); + return; } + readProps(); } // Check if File is updated recently else { @@ -120,13 +138,14 @@ public class CheckPDP { for (String propKey : sorted) { loadPDPProperties(propKey, pdpProp); } - if (pdpMap == null || pdpMap.isEmpty()) { - LOGGER.debug(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "Cannot Proceed without PDP_URLs"); - } in.close(); } catch (IOException e) { LOGGER.error(XACMLErrorConstants.ERROR_SYSTEM_ERROR + e); } + if (pdpMap == null || pdpMap.isEmpty()) { + LOGGER.debug(XACMLErrorConstants.ERROR_SYSTEM_ERROR + "Cannot Proceed without PDP_URLs"); + CheckPDP.reset(); + } } private static void loadPDPProperties(String propKey, Properties pdpProp){ diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyManagerServlet.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyManagerServlet.java index 3be7de30a..54a14cb8d 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyManagerServlet.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/admin/PolicyManagerServlet.java @@ -68,7 +68,6 @@ import org.onap.policy.common.logging.flexlogger.Logger; import org.onap.policy.components.HumanPolicyComponent; import org.onap.policy.controller.PolicyController; import org.onap.policy.controller.PolicyExportAndImportController; -import org.onap.policy.model.Roles; import org.onap.policy.rest.XACMLRest; import org.onap.policy.rest.XACMLRestProperties; import org.onap.policy.rest.adapter.PolicyRestAdapter; @@ -79,6 +78,7 @@ import org.onap.policy.rest.jpa.PolicyEntity; import org.onap.policy.rest.jpa.PolicyVersion; import org.onap.policy.rest.jpa.UserInfo; import org.onap.policy.utils.PolicyUtils; +import org.onap.policy.utils.UserUtils.Pair; import org.onap.policy.xacml.api.XACMLErrorConstants; import org.onap.policy.xacml.util.XACMLPolicyScanner; import org.onap.portalsdk.core.web.support.UserUtils; @@ -326,24 +326,10 @@ public class PolicyManagerServlet extends HttpServlet { try { //Get the Login Id of the User from Request String userId = UserUtils.getUserSession(request).getOrgUserId(); - //Check if the Role and Scope Size are Null get the values from db. List userRoles = controller.getRoles(userId); - roles = new ArrayList<>(); - scopes = new HashSet<>(); - for(Object role: userRoles){ - Roles userRole = (Roles) role; - roles.add(userRole.getRole()); - if(userRole.getScope() != null){ - if(userRole.getScope().contains(",")){ - String[] multipleScopes = userRole.getScope().split(","); - for(int i =0; i < multipleScopes.length; i++){ - scopes.add(multipleScopes[i]); - } - }else{ - scopes.add(userRole.getScope()); - } - } - } + Pair, List> pair = org.onap.policy.utils.UserUtils.checkRoleAndScope(userRoles); + roles = pair.u; + scopes = pair.t; if (roles.contains(ADMIN) || roles.contains(EDITOR) || roles.contains(GUEST) ) { if(scopes.isEmpty()){ return error("No Scopes has been Assigned to the User. Please, Contact Super-Admin"); @@ -572,24 +558,10 @@ public class PolicyManagerServlet extends HttpServlet { //Get the Login Id of the User from Request String testUserID = getTestUserId(); String userId = testUserID != null ? testUserID : UserUtils.getUserSession(request).getOrgUserId(); - //Check if the Role and Scope Size are Null get the values from db. List userRoles = controller.getRoles(userId); - roles = new ArrayList<>(); - scopes = new HashSet<>(); - for(Object role: userRoles){ - Roles userRole = (Roles) role; - roles.add(userRole.getRole()); - if(userRole.getScope() != null){ - if(userRole.getScope().contains(",")){ - String[] multipleScopes = userRole.getScope().split(","); - for(int i =0; i < multipleScopes.length; i++){ - scopes.add(multipleScopes[i]); - } - }else{ - scopes.add(userRole.getScope()); - } - } - } + Pair, List> pair = org.onap.policy.utils.UserUtils.checkRoleAndScope(userRoles); + roles = pair.u; + scopes = pair.t; List resultList = new ArrayList<>(); boolean onlyFolders = params.getBoolean("onlyFolders"); diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java index e2db78c6c..ebbbd2f63 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/CreateDcaeMicroServiceController.java @@ -892,52 +892,28 @@ public class CreateDcaeMicroServiceController extends RestrictedBaseController { } node.put(key.substring(key.indexOf('.')+1), value); } - }else if(node.size()!=0){ - if(nodeKey.contains("@")){ - if(arryKey==null){ - arryKey = nodeKey.substring(0,nodeKey.indexOf('@')); - } - if(nodeKey.endsWith("@0")){ - isArray = true; - jsonArray = new JSONArray(); - } - if(jsonArray != null && arryKey.equals(nodeKey.substring(0,nodeKey.indexOf('@')))){ - jsonArray.put(decodeContent(node)); - } - jsonResult.put(arryKey, jsonArray); - jsonArray = new JSONArray(); - arryKey = nodeKey.substring(0,nodeKey.indexOf('@')); - }else{ - isArray = false; - jsonResult.put(nodeKey, decodeContent(node)); - } - node = nodeFactory.objectNode(); - if(key.contains("@")){ - isArray = true; - if(key.endsWith("@0")|| jsonArray==null){ + }else { + if(node.size()!=0){ + if(nodeKey.contains("@")){ + if(arryKey==null){ + arryKey = nodeKey.substring(0,nodeKey.indexOf('@')); + } + if(nodeKey.endsWith("@0")){ + isArray = true; + jsonArray = new JSONArray(); + } + if(jsonArray != null && arryKey.equals(nodeKey.substring(0,nodeKey.indexOf('@')))){ + jsonArray.put(decodeContent(node)); + } + jsonResult.put(arryKey, jsonArray); jsonArray = new JSONArray(); - } - }else if(!key.contains("@")){ - isArray = false; - } - if(isArray){ - if(oldValue==null){ - oldValue = key.substring(0,key.indexOf('@')); - } - if(oldValue!=prevKey){ - oldValue = key.substring(0,key.indexOf('@')); - } - if(oldValue.equals(key.substring(0,key.indexOf('@')))){ - jsonArray.put(value); + arryKey = nodeKey.substring(0,nodeKey.indexOf('@')); }else{ - jsonResult.put(oldValue, jsonArray); - jsonArray = new JSONArray(); + isArray = false; + jsonResult.put(nodeKey, decodeContent(node)); } - oldValue = key.substring(0,key.indexOf('@')); - }else{ - jsonResult.put(key, value); + node = nodeFactory.objectNode(); } - }else{ if(key.contains("@")){ isArray = true; if(key.endsWith("@0")|| jsonArray==null){ diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PDPController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PDPController.java index 1dce3a6b0..7966af15b 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PDPController.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PDPController.java @@ -38,7 +38,7 @@ import org.onap.policy.admin.RESTfulPAPEngine; import org.onap.policy.common.logging.flexlogger.FlexLogger; import org.onap.policy.common.logging.flexlogger.Logger; import org.onap.policy.model.PDPGroupContainer; -import org.onap.policy.model.Roles; +import org.onap.policy.utils.UserUtils.Pair; import org.onap.policy.xacml.api.XACMLErrorConstants; import org.onap.policy.xacml.api.pap.OnapPDPGroup; import org.onap.policy.xacml.api.pap.PAPPolicyEngine; @@ -92,22 +92,10 @@ public class PDPController extends RestrictedBaseController { List roles; String userId = isJunit() ? "Test" : UserUtils.getUserSession(request).getOrgUserId(); List userRoles = controller.getRoles(userId); - roles = new ArrayList<>(); - scopes = new HashSet<>(); - for(Object role: userRoles){ - Roles userRole = (Roles) role; - roles.add(userRole.getRole()); - if(userRole.getScope() != null){ - if(userRole.getScope().contains(",")){ - String[] multipleScopes = userRole.getScope().split(","); - for(int i =0; i < multipleScopes.length; i++){ - scopes.add(multipleScopes[i]); - } - }else{ - scopes.add(userRole.getScope()); - } - } - } + Pair, List> pair = org.onap.policy.utils.UserUtils.checkRoleAndScope(userRoles); + roles = pair.u; + scopes = pair.t; + if(!junit&& controller.getPapEngine()==null){ setPAPEngine(request); } diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyExportAndImportController.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyExportAndImportController.java index 7618989ef..5365af18c 100644 --- a/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyExportAndImportController.java +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/controller/PolicyExportAndImportController.java @@ -29,7 +29,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -49,7 +48,6 @@ import org.apache.poi.ss.usermodel.Workbook; import org.json.JSONObject; import org.onap.policy.common.logging.flexlogger.FlexLogger; import org.onap.policy.common.logging.flexlogger.Logger; -import org.onap.policy.model.Roles; import org.onap.policy.rest.adapter.PolicyExportAdapter; import org.onap.policy.rest.dao.CommonClassDao; import org.onap.policy.rest.jpa.ActionBodyEntity; @@ -58,6 +56,7 @@ import org.onap.policy.rest.jpa.PolicyEditorScopes; import org.onap.policy.rest.jpa.PolicyEntity; import org.onap.policy.rest.jpa.PolicyVersion; import org.onap.policy.rest.jpa.UserInfo; +import org.onap.policy.utils.UserUtils.Pair; import org.onap.policy.xacml.api.XACMLErrorConstants; import org.onap.portalsdk.core.controller.RestrictedBaseController; import org.onap.portalsdk.core.web.support.UserUtils; @@ -217,24 +216,11 @@ public class PolicyExportAndImportController extends RestrictedBaseController { String userId = UserUtils.getUserSession(request).getOrgUserId(); UserInfo userInfo = (UserInfo) commonClassDao.getEntityItem(UserInfo.class, "userLoginId", userId); - //Check if the Role and Scope Size are Null get the values from db. List userRoles = controller.getRoles(userId); - roles = new ArrayList<>(); - scopes = new HashSet<>(); - for(Object role: userRoles){ - Roles userRole = (Roles) role; - roles.add(userRole.getRole()); - if(userRole.getScope() != null){ - if(userRole.getScope().contains(",")){ - String[] multipleScopes = userRole.getScope().split(","); - for(int i =0; i < multipleScopes.length; i++){ - scopes.add(multipleScopes[i]); - } - }else{ - scopes.add(userRole.getScope()); - } - } - } + Pair, List> pair = org.onap.policy.utils.UserUtils.checkRoleAndScope(userRoles); + roles = pair.u; + scopes = pair.t; + FileInputStream excelFile = new FileInputStream(new File(file)); workbook = new HSSFWorkbook(excelFile); Sheet datatypeSheet = workbook.getSheetAt(0); diff --git a/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/UserUtils.java b/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/UserUtils.java new file mode 100644 index 000000000..a34983a5f --- /dev/null +++ b/POLICY-SDK-APP/src/main/java/org/onap/policy/utils/UserUtils.java @@ -0,0 +1,70 @@ +/*- + * ============LICENSE_START======================================================= + * ONAP Policy Engine + * ================================================================================ + * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ============LICENSE_END========================================================= + */ +package org.onap.policy.utils; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.onap.policy.controller.PolicyController; +import org.onap.policy.model.Roles; + +public final class UserUtils { + + private UserUtils () { + // Empty Constructor + } + + public static class Pair { + public final T t; + public final U u; + + public Pair (T t, U u) { + this.t = t; + this.u = u; + } + } + + public static Pair, List> checkRoleAndScope(List userRoles) { + Set scopes; + List roles; + //Check if the Role and Scope Size are Null get the values from db. +// List userRoles = ; + roles = new ArrayList<>(); + scopes = new HashSet<>(); + for(Object role: userRoles){ + Roles userRole = (Roles) role; + roles.add(userRole.getRole()); + if(userRole.getScope() != null){ + if(userRole.getScope().contains(",")){ + String[] multipleScopes = userRole.getScope().split(","); + for(int i =0; i < multipleScopes.length; i++){ + scopes.add(multipleScopes[i]); + } + }else{ + scopes.add(userRole.getScope()); + } + } + } + return new Pair<>(scopes, roles); + } + +} diff --git a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/CheckPDPTest.java b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/CheckPDPTest.java index 47358a8f2..e97e89eae 100644 --- a/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/CheckPDPTest.java +++ b/POLICY-SDK-APP/src/test/java/org/onap/policy/admin/CheckPDPTest.java @@ -19,19 +19,54 @@ */ package org.onap.policy.admin; -import static org.junit.Assert.*; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; +import org.junit.FixMethodOrder; import org.junit.Test; +import org.junit.runners.MethodSorters; +@FixMethodOrder(MethodSorters.NAME_ASCENDING) public class CheckPDPTest { @Test - public final void testCheckPDP() { + public final void test1NoPropertySet() { + try { + System.clearProperty("xacml.rest.pdp.idfile"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "idonotexist.properties"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "doesnothaveproperties.atall"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "testbad.properties"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "empty.properties"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "testnotenoughvalues.properties"); + assertFalse(CheckPDP.validateID("http://localhost:8082/pdp/")); + + assertNull(CheckPDP.getPdpMap()); + assertNull(CheckPDP.getEncoding("http://localhost:8082/pdp/")); + + } catch (Exception e) { + fail("Error occured in CheckPDP test"); + } + } + + @Test + public final void test2CheckPDP() { try { System.setProperty("xacml.rest.pdp.idfile", new File(".").getCanonicalPath() + File.separator + "src"+ File.separator + "test" + File.separator + "resources" + File.separator + "test.properties"); - CheckPDP.validateID("http://localhost:8082/pdp/"); + assertTrue(CheckPDP.validateID("http://localhost:8082/pdp/")); assertTrue(CheckPDP.getPdpMap().containsKey("http://localhost:8082/pdp/")); assertTrue(CheckPDP.getEncoding("http://localhost:8082/pdp/").equals("dGVzdHBkcDphbHBoYTQ1Ng==")); } catch (Exception e) { diff --git a/POLICY-SDK-APP/src/test/resources/empty.properties b/POLICY-SDK-APP/src/test/resources/empty.properties new file mode 100644 index 000000000..f6a6a6062 --- /dev/null +++ b/POLICY-SDK-APP/src/test/resources/empty.properties @@ -0,0 +1,21 @@ +### +# ============LICENSE_START======================================================= +# ONAP Policy Engine +# ================================================================================ +# Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= +### + +# Purposefully empty \ No newline at end of file diff --git a/POLICY-SDK-APP/src/test/resources/test.properties b/POLICY-SDK-APP/src/test/resources/test.properties index e882dfd20..c3c6e9ec8 100644 --- a/POLICY-SDK-APP/src/test/resources/test.properties +++ b/POLICY-SDK-APP/src/test/resources/test.properties @@ -19,3 +19,4 @@ ### PDP_URL=http://localhost:8082/pdp/, testpdp, alpha456;http://localhost:8081/pdp/, testpdp, alpha456 +WHAT= diff --git a/POLICY-SDK-APP/src/test/resources/testbad.properties b/POLICY-SDK-APP/src/test/resources/testbad.properties new file mode 100644 index 000000000..384af8ab4 --- /dev/null +++ b/POLICY-SDK-APP/src/test/resources/testbad.properties @@ -0,0 +1,21 @@ +### +# ============LICENSE_START======================================================= +# ONAP Policy Engine +# ================================================================================ +# Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= +### + +BADPDP_URL=http://localhost:8082/pdp/, testpdp, alpha456;http://localhost:8081/pdp/, testpdp, alpha456 diff --git a/POLICY-SDK-APP/src/test/resources/testnotenoughvalues.properties b/POLICY-SDK-APP/src/test/resources/testnotenoughvalues.properties new file mode 100644 index 000000000..5823a5d68 --- /dev/null +++ b/POLICY-SDK-APP/src/test/resources/testnotenoughvalues.properties @@ -0,0 +1,21 @@ +### +# ============LICENSE_START======================================================= +# ONAP Policy Engine +# ================================================================================ +# Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= +### + +PDP_URL=http://localhost:8082/pdp/, testpdp;http://localhost:8081/pdp/, alpha456