From cf282dd15ce391eed832063aea956a8d55521278 Mon Sep 17 00:00:00 2001 From: liamfallon Date: Fri, 7 Sep 2018 13:36:41 +0100 Subject: Remove changing of access on par fields Parameter handling refactored to remove changing of access on fields in parameters, new implementation requires getters to be defined for all fields. Note: This change causes a knock on into distribution Change-Id: I172f5d9310caf92d6ea825ff93292019c00a47c3 Issue-ID: POLICY-1095 Signed-off-by: liamfallon --- common-parameters/pom.xml | 4 ++ .../common/parameters/GroupValidationResult.java | 75 ++++++++++++++++------ .../policy/common/parameters/TestValidation.java | 13 ++++ .../common/parameters/TestValidationErrors.java | 26 ++++++++ .../testclasses/ParameterGroupMissingGetter.java | 56 ++++++++++++++++ .../testclasses/ParameterGroupPrivateGetter.java | 60 +++++++++++++++++ ...ParameterGroupWithParameterGroupCollection.java | 2 +- .../parameters/testclasses/TestParametersL00.java | 40 +++++++++++- .../parameters/testclasses/TestParametersL10.java | 25 +++++++- .../testclasses/TestParametersLGeneric.java | 8 +++ .../TestJsonYamlValidationResult.txt | 4 +- .../TestParametersL0_0_OK.txt | 4 +- integrity-audit/pom.xml | 1 - pom.xml | 70 +++++++++++--------- utils/pom.xml | 1 - 15 files changed, 330 insertions(+), 59 deletions(-) create mode 100644 common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupMissingGetter.java create mode 100644 common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupPrivateGetter.java diff --git a/common-parameters/pom.xml b/common-parameters/pom.xml index 87b2d3a5..3b7ae7f5 100644 --- a/common-parameters/pom.xml +++ b/common-parameters/pom.xml @@ -31,6 +31,10 @@ [${project.parent.artifactId}] module provides common property and parameter handling the ONAP Policy Framework + + org.apache.commons + commons-lang3 + junit junit diff --git a/common-parameters/src/main/java/org/onap/policy/common/parameters/GroupValidationResult.java b/common-parameters/src/main/java/org/onap/policy/common/parameters/GroupValidationResult.java index fc2f6ca5..94d9ad49 100644 --- a/common-parameters/src/main/java/org/onap/policy/common/parameters/GroupValidationResult.java +++ b/common-parameters/src/main/java/org/onap/policy/common/parameters/GroupValidationResult.java @@ -21,11 +21,16 @@ package org.onap.policy.common.parameters; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; +import org.apache.commons.lang3.StringUtils; + /** * This class holds the result of the validation of a parameter group. */ @@ -60,18 +65,13 @@ public class GroupValidationResult implements ValidationResult { continue; } - // Make the field accessible - boolean savedAccessibilityValue = field.isAccessible(); - field.setAccessible(true); - - try { - // Set the validation result - validationResultMap.put(field.getName(), getValidationResult(field, parameterGroup)); - } catch (IllegalArgumentException | IllegalAccessException e) { - throw new ParameterRuntimeException("could not get value of parameter \"" + field.getName() + "\"", e); - } finally { - field.setAccessible(savedAccessibilityValue); + // Exclude static fields + if (Modifier.isStatic(field.getModifiers())) { + continue; } + + // Set the validation result + validationResultMap.put(field.getName(), getValidationResult(field, parameterGroup)); } } @@ -81,13 +81,12 @@ public class GroupValidationResult implements ValidationResult { * @param field The parameter field * @param ParameterGroup The parameter group containing the field * @return the validation result - * @throws IllegalAccessException on accessing private fields + * @throws Exception on accessing private fields */ - private ValidationResult getValidationResult(final Field field, final ParameterGroup parameterGroup) - throws IllegalAccessException { + private ValidationResult getValidationResult(final Field field, final ParameterGroup parameterGroup) { final String fieldName = field.getName(); final Class fieldType = field.getType(); - final Object fieldObject = field.get(parameterGroup); + final Object fieldObject = getObjectField(parameterGroup, field); // Nested parameter groups are allowed if (ParameterGroup.class.isAssignableFrom(fieldType)) { @@ -110,6 +109,47 @@ public class GroupValidationResult implements ValidationResult { return new ParameterValidationResult(field, fieldObject); } + /** + * Get the value of a field in an object using a getter found with reflection + * + * @param targetObject The object on which to read the field value + * @param fieldName The name of the field + * @return The field value + */ + private Object getObjectField(final Object targetObject, final Field field) { + String getterMethodName; + + // Check for Boolean fields, the convention for boolean getters is that they start with "is" + // If the field name already starts with "is" then the getter has the field name otherwise + // the field name is prepended with "is" + if (boolean.class.equals(field.getType())) { + if (field.getName().startsWith("is")) { + getterMethodName = field.getName(); + } else { + getterMethodName = "is" + StringUtils.capitalize(field.getName()); + } + } else { + getterMethodName = "get" + StringUtils.capitalize(field.getName()); + } + + // Look up the getter method for the field + Method getterMethod; + try { + getterMethod = targetObject.getClass().getMethod(getterMethodName, (Class[]) null); + } catch (NoSuchMethodException | SecurityException e) { + throw new ParameterRuntimeException("could not get getter method for parameter \"" + field.getName() + "\"", + e); + } + + // Invoke the getter + try { + return getterMethod.invoke(targetObject, (Object[]) null); + } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { + throw new ParameterRuntimeException("error calling getter method for parameter \"" + field.getName() + "\"", + e); + } + } + /** * Check if this field is a map of parameter groups indexed by string keys. * @@ -327,13 +367,12 @@ public class GroupValidationResult implements ValidationResult { validationResultBuilder.append(initialIndentation); validationResultBuilder.append("parameter group \""); - + if (parameterGroup != null) { validationResultBuilder.append(parameterGroup.getName()); validationResultBuilder.append("\" type \""); validationResultBuilder.append(parameterGroup.getClass().getCanonicalName()); - } - else { + } else { validationResultBuilder.append("UNDEFINED"); } validationResultBuilder.append("\" "); diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidation.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidation.java index fe750b2c..fb08d325 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidation.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidation.java @@ -31,6 +31,7 @@ import java.nio.file.Paths; import org.junit.Test; import org.onap.policy.common.parameters.testclasses.TestParametersL00; +import org.onap.policy.common.parameters.testclasses.TestParametersL10; public class TestValidation { @Test @@ -182,4 +183,16 @@ public class TestValidation { assertTrue(validationResult.isValid()); assertEquals(null, validationResult.getResult()); } + + @Test + public void testValidationEmptySubGroup() throws IOException { + TestParametersL10 l10Parameters = new TestParametersL10("l10Parameters"); + + l10Parameters.setL10LGenericNested0(null); + + GroupValidationResult validationResult = l10Parameters.validate(); + assertTrue(validationResult.isValid()); + + assertTrue(validationResult.getResult("", "", true).contains("UNDEFINED")); + } } diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidationErrors.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidationErrors.java index a489cc3f..2c1e2f18 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidationErrors.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidationErrors.java @@ -26,6 +26,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.junit.Test; +import org.onap.policy.common.parameters.testclasses.ParameterGroupMissingGetter; +import org.onap.policy.common.parameters.testclasses.ParameterGroupPrivateGetter; import org.onap.policy.common.parameters.testclasses.ParameterGroupWithArray; import org.onap.policy.common.parameters.testclasses.ParameterGroupWithCollection; import org.onap.policy.common.parameters.testclasses.ParameterGroupWithIllegalMapKey; @@ -112,4 +114,28 @@ public class TestValidationErrors { + "map \"intMap\" is not a parameter group", e.getMessage()); } } + + @Test + public void testMissingGetter() { + ParameterGroupMissingGetter badGetterName = new ParameterGroupMissingGetter("BGN"); + try { + badGetterName.isValid(); + fail("test should throw an exception"); + } catch (ParameterRuntimeException e) { + assertEquals("could not get getter method for parameter \"value\"", e.getMessage()); + } + + } + + @Test + public void testPrivateGetter() { + ParameterGroupPrivateGetter privateGetter = new ParameterGroupPrivateGetter("privateGetter"); + try { + privateGetter.isValid(); + fail("test should throw an exception"); + } catch (ParameterRuntimeException e) { + assertEquals("could not get getter method for parameter \"value\"", e.getMessage()); + } + + } } diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupMissingGetter.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupMissingGetter.java new file mode 100644 index 00000000..e05eea3f --- /dev/null +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupMissingGetter.java @@ -0,0 +1,56 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2018 Ericsson. 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. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.policy.common.parameters.testclasses; + +import org.onap.policy.common.parameters.GroupValidationResult; +import org.onap.policy.common.parameters.ParameterGroup; + +public class ParameterGroupMissingGetter implements ParameterGroup { + private String name; + private String value; + + public ParameterGroupMissingGetter(final String name) { + this.name = name; + } + + public String getTheValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + @Override + public String getName() { + return name; + } + + @Override + public void setName(final String name) { + this.name = name; + } + + @Override + public GroupValidationResult validate() { + return new GroupValidationResult(this); + } +} diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupPrivateGetter.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupPrivateGetter.java new file mode 100644 index 00000000..78a7c157 --- /dev/null +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupPrivateGetter.java @@ -0,0 +1,60 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2018 Ericsson. 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. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.policy.common.parameters.testclasses; + +import org.onap.policy.common.parameters.GroupValidationResult; +import org.onap.policy.common.parameters.ParameterGroup; + +public class ParameterGroupPrivateGetter implements ParameterGroup { + private String name; + private String value; + + public ParameterGroupPrivateGetter(final String name) { + this.name = name; + } + + public String getTheValue() { + return getValue(); + } + + private String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + @Override + public String getName() { + return name; + } + + @Override + public void setName(final String name) { + this.name = name; + } + + @Override + public GroupValidationResult validate() { + return new GroupValidationResult(this); + } +} diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupWithParameterGroupCollection.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupWithParameterGroupCollection.java index dadf7273..3966e49c 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupWithParameterGroupCollection.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupWithParameterGroupCollection.java @@ -42,7 +42,7 @@ public class ParameterGroupWithParameterGroupCollection implements ParameterGrou parameterGroupArrayList.add(new TestParametersLGeneric("Generic2")); } - public List getIntArrayList() { + public List getParameterGroupArrayList() { return parameterGroupArrayList; } diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL00.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL00.java index 6cd65603..b4a7e9c8 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL00.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL00.java @@ -30,12 +30,16 @@ import org.onap.policy.common.parameters.ParameterGroup; import org.onap.policy.common.parameters.ValidationStatus; public class TestParametersL00 implements ParameterGroup { - private String name; + private static final String A_CONSTANT = "A Constant"; + + private String name = A_CONSTANT; private int l00IntField = 0; private String l00StringField = "Legal " + this.getClass().getCanonicalName(); private TestParametersL10 l00L10Nested = new TestParametersL10("l00L10Nested"); private TestParametersLGeneric l00LGenericNested = new TestParametersLGeneric("l00LGenericNested"); private Map l00LGenericNestedMap = new LinkedHashMap<>(); + private boolean isSomeFlag; + private boolean someNonIsFlag; /** * Default constructor. @@ -43,7 +47,7 @@ public class TestParametersL00 implements ParameterGroup { public TestParametersL00() { // Default Cnstructor } - + /** * Create a test parameter group. * @@ -58,6 +62,38 @@ public class TestParametersL00 implements ParameterGroup { l00LGenericNestedMap.put(l00LGenericNestedMapVal1.getName(), l00LGenericNestedMapVal1); } + public int getL00IntField() { + return l00IntField; + } + + public String getL00StringField() { + return l00StringField; + } + + public TestParametersL10 getL00L10Nested() { + return l00L10Nested; + } + + public TestParametersLGeneric getL00LGenericNested() { + return l00LGenericNested; + } + + public Map getL00LGenericNestedMap() { + return l00LGenericNestedMap; + } + + public boolean isSomeFlag() { + return isSomeFlag; + } + + public boolean isSomeNonIsFlag() { + return someNonIsFlag; + } + + public void setSomeFlag(boolean isSomeFlag) { + this.isSomeFlag = isSomeFlag; + } + public void setName(String name) { this.name = name; } diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL10.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL10.java index 6efddac0..f63ec3f9 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL10.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL10.java @@ -58,6 +58,26 @@ public class TestParametersL10 implements ParameterGroup { l10LGenericNestedMap.put(l10LGenericNestedMapVal1.getName(), l10LGenericNestedMapVal1); } + public int getL10IntField() { + return l10IntField; + } + + public String getL10StringField() { + return l10StringField; + } + + public TestParametersLGeneric getL10LGenericNested0() { + return l10LGenericNested0; + } + + public TestParametersLGeneric getL10LGenericNested1() { + return l10LGenericNested1; + } + + public Map getL10LGenericNestedMap() { + return l10LGenericNestedMap; + } + public void setName(String name) { this.name = name; } @@ -160,8 +180,9 @@ public class TestParametersL10 implements ParameterGroup { ParameterConstants.PARAMETER_HAS_STATUS_MESSAGE + ValidationStatus.CLEAN.toString()); } - - validationResult.setResult("l10LGenericNested0", l10LGenericNested0.validate()); + if (l10LGenericNested0 != null) { + validationResult.setResult("l10LGenericNested0", l10LGenericNested0.validate()); + } validationResult.setResult("l10LGenericNested1", l10LGenericNested1.validate()); for (Entry nestedGroupEntry : l10LGenericNestedMap.entrySet()) { diff --git a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersLGeneric.java b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersLGeneric.java index 2e678da0..2d263fc7 100644 --- a/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersLGeneric.java +++ b/common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersLGeneric.java @@ -46,6 +46,14 @@ public class TestParametersLGeneric implements ParameterGroup { this.name = name; } + public int getLgenericIntField() { + return lgenericIntField; + } + + public String getLgenericStringField() { + return lgenericStringField; + } + public void setName(String name) { this.name = name; } diff --git a/common-parameters/src/test/resources/expectedValidationResults/TestJsonYamlValidationResult.txt b/common-parameters/src/test/resources/expectedValidationResults/TestJsonYamlValidationResult.txt index 103321ff..8439e0b1 100644 --- a/common-parameters/src/test/resources/expectedValidationResults/TestJsonYamlValidationResult.txt +++ b/common-parameters/src/test/resources/expectedValidationResults/TestJsonYamlValidationResult.txt @@ -36,4 +36,6 @@ parameter group "l00NameFromFile" type "org.onap.policy.common.parameters.testcl field "name" type "java.lang.String" value "L00Entry1Name" CLEAN, parameter has status CLEAN field "lgenericIntField" type "int" value "1" CLEAN, parameter has status CLEAN field "lgenericStringField" type "java.lang.String" value "L00Entry1 value from file" CLEAN, parameter has status CLEAN - + field "isSomeFlag" type "boolean" value "false" CLEAN, parameter has status CLEAN + field "someNonIsFlag" type "boolean" value "false" CLEAN, parameter has status CLEAN + \ No newline at end of file diff --git a/common-parameters/src/test/resources/expectedValidationResults/TestParametersL0_0_OK.txt b/common-parameters/src/test/resources/expectedValidationResults/TestParametersL0_0_OK.txt index 7f6d298c..2774f35c 100644 --- a/common-parameters/src/test/resources/expectedValidationResults/TestParametersL0_0_OK.txt +++ b/common-parameters/src/test/resources/expectedValidationResults/TestParametersL0_0_OK.txt @@ -36,4 +36,6 @@ parameter group "l0Parameters" type "org.onap.policy.common.parameters.testclass field "name" type "java.lang.String" value "l00LGenericNestedMapVal1" CLEAN, parameter has status CLEAN field "lgenericIntField" type "int" value "0" CLEAN, parameter has status CLEAN field "lgenericStringField" type "java.lang.String" value "Legal org.onap.policy.common.parameters.testclasses.TestParametersLGeneric" CLEAN, parameter has status CLEAN - + field "isSomeFlag" type "boolean" value "false" CLEAN, parameter has status CLEAN + field "someNonIsFlag" type "boolean" value "false" CLEAN, parameter has status CLEAN + \ No newline at end of file diff --git a/integrity-audit/pom.xml b/integrity-audit/pom.xml index 241a9a88..a45dac2c 100644 --- a/integrity-audit/pom.xml +++ b/integrity-audit/pom.xml @@ -76,7 +76,6 @@ org.apache.commons commons-lang3 - 3.4 diff --git a/pom.xml b/pom.xml index e4420771..52941288 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ org.onap.policy.parent integration 2.0.0-SNAPSHOT - + org.onap.policy.common @@ -70,6 +70,7 @@ 1.0.2 1.4.186 + 3.4 @@ -123,6 +124,11 @@ eclipselink ${eclipselink.version} + + org.apache.commons + commons-lang3 + ${commons-lang3.version} + @@ -252,41 +258,41 @@ - maven-checkstyle-plugin - - - onap-java-style - - check - - process-sources - + maven-checkstyle-plugin + + + onap-java-style + + check + + process-sources + - onap-checkstyle/onap-java-style.xml + onap-checkstyle/onap-java-style.xml - ${project.build.sourceDirectory}/src/main/java - true - true - true - - - checkstyle-suppressions.xml - true - true - warning - - - - - - org.onap.oparent - checkstyle - ${oparent.version} - compile - - + ${project.build.sourceDirectory}/src/main/java + true + true + true + + + checkstyle-suppressions.xml + true + true + warning + + + + + + org.onap.oparent + checkstyle + ${oparent.version} + compile + + diff --git a/utils/pom.xml b/utils/pom.xml index 8285a373..4b0f03ba 100644 --- a/utils/pom.xml +++ b/utils/pom.xml @@ -41,7 +41,6 @@ org.apache.commons commons-lang3 - 3.4 junit -- cgit 1.2.3-korg