[
https://issues.apache.org/jira/browse/LANG-1677?focusedWorklogId=753429&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-753429
]
ASF GitHub Bot logged work on LANG-1677:
----------------------------------------
Author: ASF GitHub Bot
Created on: 06/Apr/22 14:16
Start Date: 06/Apr/22 14:16
Worklog Time Spent: 10m
Work Description: garydgregory commented on code in PR #838:
URL: https://github.com/apache/commons-lang/pull/838#discussion_r843999518
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -99,6 +107,34 @@ public ReflectionDiffBuilder(final T lhs, final T rhs,
final ToStringStyle style
diffBuilder = new DiffBuilder<>(lhs, rhs, style);
}
+ /**
+ * Returns the field names that should be excluded from the diff
+ * @return Returns the excludeFieldNames.
+ * @since 3.13.0
+ */
+ public String[] getExcludeFieldNames() {
+ return this.excludeFieldNames.clone();
+ }
+
+
+ /**
+ * Sets the field names to exclude.
+ *
+ * @param excludeFieldNamesParam
+ * The field names to exclude from the diff or {@code null}.
+ * @return {@code this}
+ * @since 3.13.0
+ */
+ public ReflectionDiffBuilder setExcludeFieldNames(final String...
excludeFieldNamesParam) {
+ if (excludeFieldNamesParam == null) {
+ this.excludeFieldNames = new String[0];
Review Comment:
Reuse `ArrayUtils.EMPTY_STRING_ARRAY`
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -73,6 +75,12 @@
private final Object right;
private final DiffBuilder<T> diffBuilder;
+ /**
+ * Which field names to exclude from output. Intended for fields like
{@code "password"} or {@code "lastModificationDate}.
+ * @since 3.13.0
+ */
+ protected String[] excludeFieldNames;
Review Comment:
Make `private`.
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -99,6 +107,34 @@ public ReflectionDiffBuilder(final T lhs, final T rhs,
final ToStringStyle style
diffBuilder = new DiffBuilder<>(lhs, rhs, style);
}
+ /**
+ * Returns the field names that should be excluded from the diff
+ * @return Returns the excludeFieldNames.
+ * @since 3.13.0
+ */
+ public String[] getExcludeFieldNames() {
+ return this.excludeFieldNames.clone();
+ }
+
+
+ /**
+ * Sets the field names to exclude.
+ *
+ * @param excludeFieldNamesParam
+ * The field names to exclude from the diff or {@code null}.
+ * @return {@code this}
+ * @since 3.13.0
+ */
+ public ReflectionDiffBuilder setExcludeFieldNames(final String...
excludeFieldNamesParam) {
+ if (excludeFieldNamesParam == null) {
+ this.excludeFieldNames = new String[0];
+ } else {
+ //clone and remove nulls
Review Comment:
Space after `//`
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -48,10 +48,14 @@
private final Object[] objectArrayField = {null};
private static int staticField;
private transient String transientField;
+ @DiffExclude
+ private String annotatedField = "a";
+ private String excludedField = "a";
Review Comment:
You should test a non-transient null field.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -92,7 +96,7 @@ public void test_primitive_difference() {
@Test
public void test_array_difference() {
final TypeTestClass firstObject = new TypeTestClass();
- firstObject.charArrayField = new char[] { 'c' };
+ firstObject.charArrayField = new char[]{'c'};
Review Comment:
Leave formatting alone here.
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -131,7 +167,15 @@ private boolean accept(final Field field) {
if (Modifier.isTransient(field.getModifiers())) {
return false;
}
- return !Modifier.isStatic(field.getModifiers());
+ if (Modifier.isStatic(field.getModifiers())) {
+ return false;
+ }
+ if (this.excludeFieldNames != null
Review Comment:
If you make the default value the empty array and the setter does not allow
excludeFieldNames to be null, then you do not need this check? It might be
better this way though, not sure.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -128,4 +132,43 @@ public void test_difference_in_inherited_field() {
final DiffResult list = firstObject.diff(secondObject);
assertEquals(1, list.getNumberOfDiffs());
}
+
+ @Test
+ public void test_no_differences_excluded_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.excludedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void test_no_differences_diff_exclude_annotated_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.annotatedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
+ reflectionDiffBuilder.setExcludeFieldNames(null);
+ String[] excludeFieldNames =
reflectionDiffBuilder.getExcludeFieldNames();
+ assertNotNull(excludeFieldNames);
+ assertEquals(0, excludeFieldNames.length);
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullValuesInExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
Review Comment:
Use final where you can.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -128,4 +132,43 @@ public void test_difference_in_inherited_field() {
final DiffResult list = firstObject.diff(secondObject);
assertEquals(1, list.getNumberOfDiffs());
}
+
+ @Test
+ public void test_no_differences_excluded_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.excludedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void test_no_differences_diff_exclude_annotated_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.annotatedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
Review Comment:
Use final where you can.
Issue Time Tracking
-------------------
Worklog Id: (was: 753429)
Time Spent: 2h 10m (was: 2h)
> It should be possible to exclude fields in ReflectionDiffBuilder
> ----------------------------------------------------------------
>
> Key: LANG-1677
> URL: https://issues.apache.org/jira/browse/LANG-1677
> Project: Commons Lang
> Issue Type: Wish
> Components: lang.builder.*
> Affects Versions: 3.12.0
> Reporter: Dennis Baerten
> Priority: Major
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> When using ReflectionDiffBuilder to make a diff between two object it will be
> default include all fields. As stated in the documentation static and
> transient fields are excluded.
> Using the transient modifier in combination with other frameworks ( such as
> Hibernate ) also has a side affect that those fields are not persisted.
> The use case I'm trying to solve it making a diff of an object that get's
> updated and has a LastModificationDate and LastModificationUser and thus will
> always be a field in the diff.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
