From 2fd06307c4b0ee23f2c01f6b6c62292d98f87b62 Mon Sep 17 00:00:00 2001 From: Michael Buck Date: Thu, 2 Aug 2018 07:14:22 -0400 Subject: [PATCH 1/5] added methods to return non-synthetic fields from a given class and its parent(s). --- .../commons/lang3/reflect/FieldUtils.java | 100 +++++++++++++++--- .../commons/lang3/reflect/FieldUtilsTest.java | 31 ++++-- 2 files changed, 108 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java b/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java index c4ccccab74d..dcfec173581 100644 --- a/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java +++ b/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java @@ -24,7 +24,6 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; -import java.util.Collections; import java.util.List; /** @@ -38,6 +37,52 @@ */ public class FieldUtils { + // an interface that tests the specified Field and returns true if the test passes. when we move to JDK 8+ this can be replaced with java.util.function.Predicate + private interface FieldPredicate { + boolean test(Field field); + } + + // A FieldPredicate that always returns true. + private static final FieldPredicate ACCEPT_ALL_FIELD_PREDICATE = new FieldPredicate() { + @Override + public boolean test(Field field) { + return true; + } + }; + + // A FieldPredicate that returns true if the specified Field is non-synthetic. + private static final FieldPredicate NON_SYNTHETIC_FIELD_PREDICATE = new FieldPredicate() { + @Override + public boolean test(Field field) { + return !field.isSynthetic(); + } + }; + + // A FieldPredicate that returns true if the given Field is annotated with the given Annotation. + private static final class AnnotatedFieldPredicate implements FieldPredicate { + private final Class m_annotationClass; + + AnnotatedFieldPredicate(Class annotationClass) { + m_annotationClass = annotationClass; + } + + @Override + public boolean test(Field field) { + return field.isAnnotationPresent(m_annotationClass); + } + } + + // gets all fields from the given class that adhere to the given FieldPredicate + private static List getFields(final Field[] declaredFields, FieldPredicate fieldPredicate) { + List fields = new ArrayList<>(declaredFields.length); + for (Field declaredField : declaredFields) { + if (fieldPredicate.test(declaredField)) { + fields.add(declaredField); + } + } + return fields; + } + /** * {@link FieldUtils} instances should NOT be constructed in standard programming. *

@@ -205,7 +250,7 @@ public static Field[] getAllFields(final Class cls) { * * @param cls * the {@link Class} to query - * @return an array of Fields (possibly empty). + * @return a List of Fields (possibly empty). * @throws IllegalArgumentException * if the class is {@code null} * @since 3.2 @@ -213,11 +258,42 @@ public static Field[] getAllFields(final Class cls) { public static List getAllFieldsList(final Class cls) { Validate.isTrue(cls != null, "The class must not be null"); final List allFields = new ArrayList<>(); - Class currentClass = cls; - while (currentClass != null) { - final Field[] declaredFields = currentClass.getDeclaredFields(); - Collections.addAll(allFields, declaredFields); - currentClass = currentClass.getSuperclass(); + for (Class currentClass = cls; currentClass != null; currentClass = currentClass.getSuperclass()) { + allFields.addAll(getFields(currentClass.getDeclaredFields(), ACCEPT_ALL_FIELD_PREDICATE)); + } + return allFields; + } + + /** + * Gets all non-synthetic fields of the given class and its parents (if any). + * + * @param cls + * the {@link Class} to query + * @return an array of Fields (possibly empty). + * @throws IllegalArgumentException + * if the class is {@code null} + * @since 3.7 + */ + public static Field[] getAllFieldsNonSynthetic(final Class cls) { + final List allFieldsList = getAllFieldsNonSyntheticList(cls); + return allFieldsList.toArray(new Field[allFieldsList.size()]); + } + + /** + * Gets all non-synthetic fields of the given class and its parents (if any). + * + * @param cls + * the {@link Class} to query + * @return a List of Fields (possibly empty). + * @throws IllegalArgumentException + * if the class is {@code null} + * @since 3.7 + */ + public static List getAllFieldsNonSyntheticList(final Class cls) { + Validate.isTrue(cls != null, "The class must not be null"); + final List allFields = new ArrayList<>(); + for (Class currentClass = cls; currentClass != null; currentClass = currentClass.getSuperclass()) { + allFields.addAll(getFields(currentClass.getDeclaredFields(), NON_SYNTHETIC_FIELD_PREDICATE)); } return allFields; } @@ -250,15 +326,9 @@ public static Field[] getFieldsWithAnnotation(final Class cls, final Class getFieldsListWithAnnotation(final Class cls, final Class annotationCls) { + Validate.isTrue(cls != null, "The class must not be null"); Validate.isTrue(annotationCls != null, "The annotation class must not be null"); - final List allFields = getAllFieldsList(cls); - final List annotatedFields = new ArrayList<>(); - for (final Field field : allFields) { - if (field.getAnnotation(annotationCls) != null) { - annotatedFields.add(field); - } - } - return annotatedFields; + return getFields(cls.getDeclaredFields(), new AnnotatedFieldPredicate(annotationCls)); } /** diff --git a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java index f98438f93ec..f1ae87a6ecf 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java @@ -18,14 +18,7 @@ import org.apache.commons.lang3.ArrayUtils; -import org.apache.commons.lang3.reflect.testbed.Ambig; -import org.apache.commons.lang3.reflect.testbed.Annotated; -import org.apache.commons.lang3.reflect.testbed.Foo; -import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild; -import org.apache.commons.lang3.reflect.testbed.PublicChild; -import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild; -import org.apache.commons.lang3.reflect.testbed.StaticContainer; -import org.apache.commons.lang3.reflect.testbed.StaticContainerChild; +import org.apache.commons.lang3.reflect.testbed.*; import org.junit.Before; import org.junit.Test; @@ -198,6 +191,28 @@ public void testGetAllFieldsList() { } + @Test + public void testGetAllFieldNonSyntheticList() { + assertEquals(0, FieldUtils.getAllFieldsNonSyntheticList(Object.class).size()); + + final List fieldsNumber = Arrays.asList(Number.class.getDeclaredFields()); + assertEquals(fieldsNumber, FieldUtils.getAllFieldsNonSyntheticList(Number.class)); + + // at the time of this writing Integer contains a synthetic field called $assertionsDisabled + assertEquals(Integer.class.getDeclaredFields().length - 1 + Number.class.getDeclaredFields().length + Object.class.getDeclaredFields().length, FieldUtils.getAllFieldsNonSyntheticList(Integer.class).size()); + } + + @Test + public void testGetAllFieldsNonSynthetic() { + assertEquals(0, FieldUtils.getAllFieldsNonSynthetic(Object.class).length); + + final Field[] fieldsNumber = Number.class.getDeclaredFields(); + assertArrayEquals(fieldsNumber, FieldUtils.getAllFieldsNonSynthetic(Number.class)); + + // at the time of this writing Integer contains a synthetic field called $assertionsDisabled + assertEquals(FieldUtils.getAllFields(Integer.class).length - 1, FieldUtils.getAllFieldsNonSynthetic(Integer.class).length); + } + @Test public void testGetFieldsWithAnnotation() throws NoSuchFieldException { assertArrayEquals(new Field[0], FieldUtils.getFieldsWithAnnotation(Object.class, Annotated.class)); From 48b72c854e33368b9a15dbcfe9e276c789e718e4 Mon Sep 17 00:00:00 2001 From: Michael Buck Date: Thu, 2 Aug 2018 09:32:28 -0400 Subject: [PATCH 2/5] fixed checkstyle rule violation --- .../org/apache/commons/lang3/reflect/FieldUtilsTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java index f1ae87a6ecf..415bd1ad2d1 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java @@ -18,7 +18,14 @@ import org.apache.commons.lang3.ArrayUtils; -import org.apache.commons.lang3.reflect.testbed.*; +import org.apache.commons.lang3.reflect.testbed.Ambig; +import org.apache.commons.lang3.reflect.testbed.Annotated; +import org.apache.commons.lang3.reflect.testbed.Foo; +import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild; +import org.apache.commons.lang3.reflect.testbed.PublicChild; +import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild; +import org.apache.commons.lang3.reflect.testbed.StaticContainer; +import org.apache.commons.lang3.reflect.testbed.StaticContainerChild; import org.junit.Before; import org.junit.Test; From e1ed56bf46275a25dd9044927d2f6b129a5815f1 Mon Sep 17 00:00:00 2001 From: Michael Buck Date: Fri, 3 Aug 2018 13:55:52 -0400 Subject: [PATCH 3/5] fixed failing tests in multiple JVM versions. --- .../commons/lang3/reflect/FieldUtilsTest.java | 37 ++++++++++++++++--- .../lang3/reflect/testbed/FooEnum.java | 7 ++++ 2 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java diff --git a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java index 415bd1ad2d1..02230fc0f89 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java @@ -21,6 +21,7 @@ import org.apache.commons.lang3.reflect.testbed.Ambig; import org.apache.commons.lang3.reflect.testbed.Annotated; import org.apache.commons.lang3.reflect.testbed.Foo; +import org.apache.commons.lang3.reflect.testbed.FooEnum; import org.apache.commons.lang3.reflect.testbed.PrivatelyShadowedChild; import org.apache.commons.lang3.reflect.testbed.PublicChild; import org.apache.commons.lang3.reflect.testbed.PubliclyShadowedChild; @@ -199,14 +200,22 @@ public void testGetAllFieldsList() { } @Test - public void testGetAllFieldNonSyntheticList() { + public void testGetAllFieldsNonSyntheticList() { assertEquals(0, FieldUtils.getAllFieldsNonSyntheticList(Object.class).size()); final List fieldsNumber = Arrays.asList(Number.class.getDeclaredFields()); assertEquals(fieldsNumber, FieldUtils.getAllFieldsNonSyntheticList(Number.class)); - // at the time of this writing Integer contains a synthetic field called $assertionsDisabled - assertEquals(Integer.class.getDeclaredFields().length - 1 + Number.class.getDeclaredFields().length + Object.class.getDeclaredFields().length, FieldUtils.getAllFieldsNonSyntheticList(Integer.class).size()); + Field[] allFields = FieldUtils.getAllFields(FooEnum.class); + + // the JVM changes all the time. so who knows how many synthetic fields it will generate tomorrow. + // here we get the number of non-synthetic fields from the enum and use that for an assertion later on. + final int expectedNumberOfNonSyntheticFields = getNumberOfNonSyntheticFields(allFields); + + assertTrue(expectedNumberOfNonSyntheticFields > 0); + + final Field[] allFieldsNonSynthetic = FieldUtils.getAllFieldsNonSynthetic(FooEnum.class); + assertEquals(expectedNumberOfNonSyntheticFields, allFieldsNonSynthetic.length); } @Test @@ -216,8 +225,16 @@ public void testGetAllFieldsNonSynthetic() { final Field[] fieldsNumber = Number.class.getDeclaredFields(); assertArrayEquals(fieldsNumber, FieldUtils.getAllFieldsNonSynthetic(Number.class)); - // at the time of this writing Integer contains a synthetic field called $assertionsDisabled - assertEquals(FieldUtils.getAllFields(Integer.class).length - 1, FieldUtils.getAllFieldsNonSynthetic(Integer.class).length); + Field[] allFields = FieldUtils.getAllFields(FooEnum.class); + + // the JVM changes all the time. so who knows how many synthetic fields it will generate tomorrow. + // here we get the number of non-synthetic fields from the enum and use that for an assertion later on. + final int expectedNumberOfNonSyntheticFields = getNumberOfNonSyntheticFields(allFields); + + assertTrue(expectedNumberOfNonSyntheticFields > 0); + + Field[] allFieldsNonSynthetic = FieldUtils.getAllFieldsNonSynthetic(FooEnum.class); + assertEquals(expectedNumberOfNonSyntheticFields, allFieldsNonSynthetic.length); } @Test @@ -1406,4 +1423,14 @@ public void testRemoveFinalModifierAccessNotNeeded() throws Exception { assertFalse(field.isAccessible()); } + private static int getNumberOfNonSyntheticFields(Field[] fields) { + int numberOfNonSyntheticFields = 0; + for (Field field : fields) { + if (!field.isSynthetic()) { + ++numberOfNonSyntheticFields; + } + } + + return numberOfNonSyntheticFields; + } } diff --git a/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java new file mode 100644 index 00000000000..b07ed8d9045 --- /dev/null +++ b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java @@ -0,0 +1,7 @@ +package org.apache.commons.lang3.reflect.testbed; + +public enum FooEnum { + ITEM_1, + ITEM_2, + ITEM_3, +} From ce0a51e524fb8fef065016a293ea2f21fe742cf1 Mon Sep 17 00:00:00 2001 From: Michael Buck Date: Fri, 3 Aug 2018 14:01:37 -0400 Subject: [PATCH 4/5] added license header. --- .../lang3/reflect/testbed/FooEnum.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java index b07ed8d9045..7566c1353c7 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java +++ b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java @@ -1,5 +1,24 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ package org.apache.commons.lang3.reflect.testbed; +/** + * + */ public enum FooEnum { ITEM_1, ITEM_2, From 4da7eabd0a715ca8cb61b669ac6c8027e569814c Mon Sep 17 00:00:00 2001 From: "mike.buck@pb.com" Date: Fri, 3 Aug 2018 14:10:46 -0400 Subject: [PATCH 5/5] fixed code style failures. --- .../org/apache/commons/lang3/reflect/testbed/FooEnum.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java index 7566c1353c7..f1802ad7438 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java +++ b/src/test/java/org/apache/commons/lang3/reflect/testbed/FooEnum.java @@ -17,10 +17,9 @@ package org.apache.commons.lang3.reflect.testbed; /** - * */ public enum FooEnum { - ITEM_1, - ITEM_2, - ITEM_3, + ITEM_1, + ITEM_2, + ITEM_3, }