Remove changing of access on par fields 93/65193/2
authorliamfallon <liam.fallon@ericsson.com>
Fri, 7 Sep 2018 12:36:41 +0000 (13:36 +0100)
committerliamfallon <liam.fallon@ericsson.com>
Fri, 7 Sep 2018 12:52:35 +0000 (13:52 +0100)
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>
15 files changed:
common-parameters/pom.xml
common-parameters/src/main/java/org/onap/policy/common/parameters/GroupValidationResult.java
common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidation.java
common-parameters/src/test/java/org/onap/policy/common/parameters/TestValidationErrors.java
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupMissingGetter.java [new file with mode: 0644]
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupPrivateGetter.java [new file with mode: 0644]
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/ParameterGroupWithParameterGroupCollection.java
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL00.java
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersL10.java
common-parameters/src/test/java/org/onap/policy/common/parameters/testclasses/TestParametersLGeneric.java
common-parameters/src/test/resources/expectedValidationResults/TestJsonYamlValidationResult.txt
common-parameters/src/test/resources/expectedValidationResults/TestParametersL0_0_OK.txt
integrity-audit/pom.xml
pom.xml
utils/pom.xml

index 87b2d3a..3b7ae7f 100644 (file)
     <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>
index fc2f6ca..94d9ad4 100644 (file)
 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("\" ");
index fe750b2..fb08d32 100644 (file)
@@ -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"));
+    }
 }
index a489cc3..2c1e2f1 100644 (file)
@@ -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 (file)
index 0000000..e05eea3
--- /dev/null
@@ -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 (file)
index 0000000..78a7c15
--- /dev/null
@@ -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);
+    }
+}
index dadf727..3966e49 100644 (file)
@@ -42,7 +42,7 @@ public class ParameterGroupWithParameterGroupCollection implements ParameterGrou
         parameterGroupArrayList.add(new TestParametersLGeneric("Generic2"));
     }
 
-    public List<ParameterGroup> getIntArrayList() {
+    public List<ParameterGroup> getParameterGroupArrayList() {
         return parameterGroupArrayList;
     }
     
index 6cd6560..b4a7e9c 100644 (file)
@@ -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;
     }
index 6efddac..f63ec3f 100644 (file)
@@ -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()) {
index 2e678da..2d263fc 100644 (file)
@@ -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;
     }
index 103321f..8439e0b 100644 (file)
@@ -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
index 7f6d298..2774f35 100644 (file)
@@ -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
index 241a9a8..a45dac2 100644 (file)
@@ -76,7 +76,6 @@
                <dependency>
                         <groupId>org.apache.commons</groupId>
                         <artifactId>commons-lang3</artifactId>
-                        <version>3.4</version>
                </dependency>
        </dependencies>
 
diff --git a/pom.xml b/pom.xml
index e442077..5294128 100644 (file)
--- a/pom.xml
+++ b/pom.xml
@@ -26,7 +26,7 @@
         <groupId>org.onap.policy.parent</groupId>
         <artifactId>integration</artifactId>
         <version>2.0.0-SNAPSHOT</version>
-        <relativePath/>
+        <relativePath />
     </parent>
 
     <groupId>org.onap.policy.common</groupId>
@@ -70,6 +70,7 @@
         <!-- Project common dependency versions -->
         <javax.persistence.api.version>1.0.2</javax.persistence.api.version>
         <h2.version>1.4.186</h2.version>
+        <commons-lang3.version>3.4</commons-lang3.version>
     </properties>
 
     <modules>
                 <artifactId>eclipselink</artifactId>
                 <version>${eclipselink.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.apache.commons</groupId>
+                <artifactId>commons-lang3</artifactId>
+                <version>${commons-lang3.version}</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
 
                 </dependencies>
             </plugin>
             <plugin>
-              <artifactId>maven-checkstyle-plugin</artifactId>
-              <executions>
-                <execution>
-                  <id>onap-java-style</id>
-                  <goals>
-                    <goal>check</goal>
-                  </goals>
-                  <phase>process-sources</phase>
-                  <configuration>
+                <artifactId>maven-checkstyle-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>onap-java-style</id>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                        <phase>process-sources</phase>
+                        <configuration>
                     <!-- Use Google Java Style Guide:
                     https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
                     with minor changes -->
-                    <configLocation>onap-checkstyle/onap-java-style.xml</configLocation>
+                            <configLocation>onap-checkstyle/onap-java-style.xml</configLocation>
                     <!-- <sourceDirectory> is needed so that checkstyle ignores the generated sources directory -->
-                    <sourceDirectory>${project.build.sourceDirectory}/src/main/java</sourceDirectory>
-                    <includeResources>true</includeResources>
-                    <includeTestSourceDirectory>true</includeTestSourceDirectory>
-                    <includeTestResources>true</includeTestResources>
-                    <excludes>
-                    </excludes>
-                    <suppressionsLocation>checkstyle-suppressions.xml</suppressionsLocation>
-                    <consoleOutput>true</consoleOutput>
-                    <failOnViolation>true</failOnViolation>
-                    <violationSeverity>warning</violationSeverity>
-                  </configuration>
-                </execution>
-              </executions>
-              <dependencies>
-                <dependency>
-                  <groupId>org.onap.oparent</groupId>
-                  <artifactId>checkstyle</artifactId>
-                  <version>${oparent.version}</version>
-                  <scope>compile</scope>
-                </dependency>
-              </dependencies>
+                            <sourceDirectory>${project.build.sourceDirectory}/src/main/java</sourceDirectory>
+                            <includeResources>true</includeResources>
+                            <includeTestSourceDirectory>true</includeTestSourceDirectory>
+                            <includeTestResources>true</includeTestResources>
+                            <excludes>
+                            </excludes>
+                            <suppressionsLocation>checkstyle-suppressions.xml</suppressionsLocation>
+                            <consoleOutput>true</consoleOutput>
+                            <failOnViolation>true</failOnViolation>
+                            <violationSeverity>warning</violationSeverity>
+                        </configuration>
+                    </execution>
+                </executions>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.onap.oparent</groupId>
+                        <artifactId>checkstyle</artifactId>
+                        <version>${oparent.version}</version>
+                        <scope>compile</scope>
+                    </dependency>
+                </dependencies>
             </plugin>
         </plugins>
     </build>
index 8285a37..4b0f03b 100644 (file)
@@ -41,7 +41,6 @@
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-lang3</artifactId>
-            <version>3.4</version>
         </dependency>
         <dependency>
             <groupId>junit</groupId>