From 7b5f7f8627d6e3cc267b4f7eefeb73d2efb31f96 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Thu, 11 Jul 2019 10:10:17 -0400 Subject: Fix checkstyle issues in common/gson Also deleted the checkstyle suppression file. Change-Id: I6d310af32d6d4be9633a8f88745447f40a566af7 Issue-ID: POLICY-1074 Signed-off-by: Jim Hahn --- gson/checkstyle-suppressions.xml | 39 ----------- gson/pom.xml | 6 +- .../onap/policy/common/gson/internal/Adapter.java | 33 ++++++++-- .../policy/common/gson/internal/ClassWalker.java | 8 ++- .../policy/common/gson/internal/AdapterTest.java | 76 ++++++++++++++++------ .../common/gson/internal/ClassWalkerTest.java | 13 +++- 6 files changed, 107 insertions(+), 68 deletions(-) delete mode 100644 gson/checkstyle-suppressions.xml diff --git a/gson/checkstyle-suppressions.xml b/gson/checkstyle-suppressions.xml deleted file mode 100644 index 0705e0e6..00000000 --- a/gson/checkstyle-suppressions.xml +++ /dev/null @@ -1,39 +0,0 @@ - - - - - - - - - - - diff --git a/gson/pom.xml b/gson/pom.xml index 1c630ca8..45ea110f 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -56,6 +56,11 @@ assertj-core test + + org.powermock + powermock-api-mockito + test + junit junit @@ -120,7 +125,6 @@ true - ${project.basedir}/checkstyle-suppressions.xml true true warning diff --git a/gson/src/main/java/org/onap/policy/common/gson/internal/Adapter.java b/gson/src/main/java/org/onap/policy/common/gson/internal/Adapter.java index c7b3bc98..174b4912 100644 --- a/gson/src/main/java/org/onap/policy/common/gson/internal/Adapter.java +++ b/gson/src/main/java/org/onap/policy/common/gson/internal/Adapter.java @@ -41,6 +41,11 @@ public class Adapter { */ private static final Pattern VALID_NAME_PAT = Pattern.compile("[a-zA-Z_]\\w*"); + /** + * Factory to access objects. Overridden by junit tests. + */ + private static Factory factory = new Factory(); + /** * Name of the property within the json structure containing the item. */ @@ -157,7 +162,7 @@ public class Adapter { * @return {@code true} if the field is managed by the walker, {@code false} otherwise */ public static boolean isManaged(Field field) { - return VALID_NAME_PAT.matcher(field.getName()).matches(); + return VALID_NAME_PAT.matcher(factory.getName(field)).matches(); } /** @@ -168,7 +173,7 @@ public class Adapter { * otherwise */ public static boolean isManaged(Method method) { - return VALID_NAME_PAT.matcher(method.getName()).matches(); + return VALID_NAME_PAT.matcher(factory.getName(method)).matches(); } /** @@ -185,7 +190,7 @@ public class Adapter { } // no name provided - use it as is - return (isManaged(field) ? field.getName() : null); + return (isManaged(field) ? factory.getName(field) : null); } /** @@ -204,7 +209,7 @@ public class Adapter { return null; } - String name = method.getName(); + String name = factory.getName(method); if (name.startsWith("get")) { return name.substring(3); @@ -238,7 +243,7 @@ public class Adapter { return null; } - String name = method.getName(); + String name = factory.getName(method); if (name.startsWith("set")) { return name.substring(3); @@ -284,7 +289,7 @@ public class Adapter { * @return the field fully qualified name */ public static String getQualifiedName(Field field) { - return (field.getDeclaringClass().getName() + "." + field.getName()); + return (field.getDeclaringClass().getName() + "." + factory.getName(field)); } /** @@ -294,7 +299,7 @@ public class Adapter { * @return the method's fully qualified name */ public static String getQualifiedName(Method method) { - return (method.getDeclaringClass().getName() + "." + method.getName()); + return (method.getDeclaringClass().getName() + "." + factory.getName(method)); } /** @@ -340,4 +345,18 @@ public class Adapter { return conv; } } + + /** + * Factory used to access various objects. + */ + public static class Factory { + + public String getName(Field field) { + return field.getName(); + } + + public String getName(Method method) { + return method.getName(); + } + } } diff --git a/gson/src/main/java/org/onap/policy/common/gson/internal/ClassWalker.java b/gson/src/main/java/org/onap/policy/common/gson/internal/ClassWalker.java index 89e4f89d..ef4eaae3 100644 --- a/gson/src/main/java/org/onap/policy/common/gson/internal/ClassWalker.java +++ b/gson/src/main/java/org/onap/policy/common/gson/internal/ClassWalker.java @@ -241,7 +241,7 @@ public class ClassWalker { return; } - String name = Adapter.detmPropName(field); + String name = detmPropName(field); if (name == null) { // invalid name return; @@ -386,4 +386,10 @@ public class ClassWalker { private String getFqdn(Method method) { return (method.getDeclaringClass().getName() + "." + method.getName()); } + + // these may be overridden by junit tests + + protected String detmPropName(Field field) { + return Adapter.detmPropName(field); + } } diff --git a/gson/src/test/java/org/onap/policy/common/gson/internal/AdapterTest.java b/gson/src/test/java/org/onap/policy/common/gson/internal/AdapterTest.java index 33160007..fc98017e 100644 --- a/gson/src/test/java/org/onap/policy/common/gson/internal/AdapterTest.java +++ b/gson/src/test/java/org/onap/policy/common/gson/internal/AdapterTest.java @@ -23,6 +23,9 @@ package org.onap.policy.common.gson.internal; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -31,24 +34,34 @@ import com.google.gson.JsonPrimitive; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.List; +import org.junit.After; +import org.junit.BeforeClass; import org.junit.Test; import org.onap.policy.common.gson.JacksonExclusionStrategy; import org.onap.policy.common.gson.annotation.GsonJsonProperty; +import org.onap.policy.common.gson.internal.Adapter.Factory; import org.onap.policy.common.gson.internal.DataAdapterFactory.Data; import org.onap.policy.common.gson.internal.DataAdapterFactory.DerivedData; +import org.powermock.reflect.Whitebox; public class AdapterTest { + private static final String GET_INVALID_NAME = "get$InvalidName"; + private static final String SET_INVALID_NAME = "set$InvalidName"; private static final String EMPTY_ALIAS = "emptyAlias"; private static final String GET_VALUE = ".getValue"; private static final String GET_VALUE_NAME = "getValue"; + private static final String SET_VALUE_NAME = "setValue"; private static final String VALUE_NAME = "value"; private static final String MY_NAME = AdapterTest.class.getName(); + private static final String FACTORY_FIELD = "factory"; private static DataAdapterFactory dataAdapter = new DataAdapterFactory(); private static Gson gson = new GsonBuilder().registerTypeAdapterFactory(dataAdapter) .setExclusionStrategies(new JacksonExclusionStrategy()).create(); + private static Factory saveFactory; + /* * The remaining fields are just used within the tests. */ @@ -64,26 +77,44 @@ public class AdapterTest { protected String unaliased; - protected String $invalidFieldName; - private List listField; private Data dataField; + @BeforeClass + public static void setUpBeforeClass() { + saveFactory = Whitebox.getInternalState(Adapter.class, FACTORY_FIELD); + } + + @After + public void tearDown() { + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, saveFactory); + } @Test public void testIsManagedField() { assertTrue(Adapter.isManaged(field(VALUE_NAME))); - assertFalse(Adapter.isManaged(field("$invalidFieldName"))); + // return an invalid field name + Factory factory = mock(Factory.class); + when(factory.getName(any(Field.class))).thenReturn("$invalidFieldName"); + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, factory); + assertFalse(Adapter.isManaged(field(VALUE_NAME))); } @Test public void testIsManagedMethod() { assertTrue(Adapter.isManaged(mget(GET_VALUE_NAME))); - assertFalse(Adapter.isManaged(mget("get$InvalidName"))); - assertFalse(Adapter.isManaged(mset("set$InvalidName"))); + // return an invalid method name + Factory factory = mock(Factory.class); + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, factory); + + when(factory.getName(any(Method.class))).thenReturn(GET_INVALID_NAME); + assertFalse(Adapter.isManaged(mget(GET_VALUE_NAME))); + + when(factory.getName(any(Method.class))).thenReturn(SET_INVALID_NAME); + assertFalse(Adapter.isManaged(mset(SET_VALUE_NAME))); } @Test @@ -178,7 +209,7 @@ public class AdapterTest { // test setter - adapter = new Adapter(gson, mset("setValue"), String.class); + adapter = new Adapter(gson, mset(SET_VALUE_NAME), String.class); assertEquals(VALUE_NAME, adapter.getPropName()); assertEquals(MY_NAME + ".setValue", adapter.getFullName()); @@ -205,7 +236,12 @@ public class AdapterTest { assertEquals(EMPTY_ALIAS, Adapter.detmPropName(field(EMPTY_ALIAS))); assertEquals("name-with-alias", Adapter.detmPropName(field("nameWithAlias"))); assertEquals("unaliased", Adapter.detmPropName(field("unaliased"))); - assertEquals(null, Adapter.detmPropName(field("$invalidFieldName"))); + + // return an invalid field name + Factory factory = mock(Factory.class); + when(factory.getName(any(Field.class))).thenReturn("$invalidFieldName"); + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, factory); + assertEquals(null, Adapter.detmPropName(field(VALUE_NAME))); } @Test @@ -218,7 +254,13 @@ public class AdapterTest { assertEquals(null, Adapter.detmGetterPropName(mget("isString"))); assertEquals(null, Adapter.detmGetterPropName(mget("noGet"))); assertEquals(null, Adapter.detmGetterPropName(mget("get"))); - assertEquals(null, Adapter.detmGetterPropName(mget("get$InvalidName"))); + + // return an invalid method name + Factory factory = mock(Factory.class); + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, factory); + + when(factory.getName(any(Method.class))).thenReturn(GET_INVALID_NAME); + assertEquals(null, Adapter.detmGetterPropName(mget(GET_VALUE_NAME))); } @Test @@ -228,7 +270,13 @@ public class AdapterTest { assertEquals("plain", Adapter.detmSetterPropName(mset("setPlain"))); assertEquals(null, Adapter.detmSetterPropName(mset("noSet"))); assertEquals(null, Adapter.detmSetterPropName(mset("set"))); - assertEquals(null, Adapter.detmSetterPropName(mset("set$InvalidName"))); + + // return an invalid method name + Factory factory = mock(Factory.class); + Whitebox.setInternalState(Adapter.class, FACTORY_FIELD, factory); + + when(factory.getName(any(Method.class))).thenReturn(SET_INVALID_NAME); + assertEquals(null, Adapter.detmSetterPropName(mset(SET_VALUE_NAME))); } @Test @@ -335,11 +383,6 @@ public class AdapterTest { return ""; } - // name has a bogus character - protected String get$InvalidName() { - return ""; - } - protected void setValue(String text) { // do nothing @@ -371,11 +414,6 @@ public class AdapterTest { // do nothing } - // name has a bogus character - protected void set$InvalidName(String text) { - // do nothing - } - // returns a list protected List getMyList() { return listField; diff --git a/gson/src/test/java/org/onap/policy/common/gson/internal/ClassWalkerTest.java b/gson/src/test/java/org/onap/policy/common/gson/internal/ClassWalkerTest.java index 6af4ae4f..a1350643 100644 --- a/gson/src/test/java/org/onap/policy/common/gson/internal/ClassWalkerTest.java +++ b/gson/src/test/java/org/onap/policy/common/gson/internal/ClassWalkerTest.java @@ -46,6 +46,7 @@ import org.onap.policy.common.gson.annotation.GsonJsonProperty; public class ClassWalkerTest { private static final String SET_OVERRIDE = ".setOverride"; + private static final String INVALID_FIELD_NAME = "invalidFieldName"; private MyWalker walker; @@ -205,6 +206,15 @@ public class ClassWalkerTest { super.examine(method); } + + @Override + protected String detmPropName(Field field) { + if (INVALID_FIELD_NAME.equals(field.getName())) { + return null; + } + + return super.detmPropName(field); + } } protected static interface Intfc1 { @@ -223,7 +233,8 @@ public class ClassWalkerTest { private int id; public String value; - public String invalid$fieldName; + // this is not actually invalid, but will be treated as if it were + public String invalidFieldName; @GsonJsonProperty("exposed") private String exposedField; -- cgit 1.2.3-korg