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 <liam.fallon@ericsson.com>

12 files changed, 292 insertions, 25 deletions

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 @@
     <description>[${project.parent.artifactId}] module provides common property and parameter handling the ONAP Policy Framework</description>
 
     <dependencies>
+         <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
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)) {
@@ -111,6 +110,47 @@ public class GroupValidationResult implements ValidationResult {
     }
 
     /**
+     * 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.
      * 
      * @param fieldName the name of the collection field.
@@ -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<ParameterGroup> getIntArrayList() {
+    public List<ParameterGroup> 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<String, TestParametersLGeneric> 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<String, TestParametersLGeneric> 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<String, TestParametersLGeneric> 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<String, TestParametersLGeneric> 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