Skip to content

Commit

Permalink
Ignore static methods when finding property accessors. Fixes #1458
Browse files Browse the repository at this point in the history
Previously, when looking for property accessors, Introspection would consider
a static method with a name in the required form as being an accessor for a
property. Where such a static method existed, this was likely to result in the
wrong value being considered as the static method is unlikely to return the
correct value for an instance's property.

This commit updates Introspection so that static methods are ignored when
looking for property accessors.
  • Loading branch information
wilkinsona authored and joel-costigliola committed Mar 15, 2019
1 parent b600f5b commit 8d83c41
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 7 deletions.
Expand Up @@ -34,7 +34,7 @@ public final class Introspection {

/**
* Returns the getter {@link Method} for a property matching the given name in the given object.
*
*
* @param propertyName the given property name.
* @param target the given object.
* @return the getter {@code Method} for a property matching the given name in the given object.
Expand Down Expand Up @@ -77,14 +77,19 @@ private static Method findGetter(String propertyName, Object target) {
String capitalized = propertyName.substring(0, 1).toUpperCase(ENGLISH) + propertyName.substring(1);
// try to find getProperty
Method getter = findMethod("get" + capitalized, target);
if (getter != null) return getter;
if (isValidGetter(getter)) return getter;
if (bareNamePropertyMethods) {
// try to find bare name property
getter = findMethod(propertyName, target);
if (getter != null) return getter;
if (isValidGetter(getter)) return getter;
}
// try to find isProperty for boolean properties
return findMethod("is" + capitalized, target);
Method isAccessor = findMethod("is" + capitalized, target);
return isValidGetter(isAccessor) ? isAccessor : null;
}

private static boolean isValidGetter(Method method) {
return method != null && !Modifier.isStatic(method.getModifiers());
}

private static Method findMethod(String name, Object target) {
Expand Down
Expand Up @@ -112,17 +112,20 @@ public void should_throw_error_when_no_property_nor_public_field_match_given_nam

@Test
public void should_throw_exception_when_given_property_or_field_name_is_null() {
assertThatIllegalArgumentException().isThrownBy(() -> propertyOrFieldSupport.getValueOf(null, yoda)).withMessage("The name of the property/field to read should not be null");
assertThatIllegalArgumentException().isThrownBy(() -> propertyOrFieldSupport.getValueOf(null, yoda))
.withMessage("The name of the property/field to read should not be null");
}

@Test
public void should_throw_exception_when_given_name_is_empty() {
assertThatIllegalArgumentException().isThrownBy(() -> propertyOrFieldSupport.getValueOf("", yoda)).withMessage("The name of the property/field to read should not be empty");
assertThatIllegalArgumentException().isThrownBy(() -> propertyOrFieldSupport.getValueOf("", yoda))
.withMessage("The name of the property/field to read should not be empty");
}

@Test
public void should_throw_exception_if_property_cannot_be_extracted_due_to_runtime_exception_during_property_access() {
assertThatExceptionOfType(IntrospectionError.class).isThrownBy(() -> propertyOrFieldSupport.getValueOf("adult", brokenEmployee()));
assertThatExceptionOfType(IntrospectionError.class).isThrownBy(() -> propertyOrFieldSupport.getValueOf("adult",
brokenEmployee()));
}

@Test
Expand Down Expand Up @@ -151,6 +154,27 @@ public void should_extract_single_value_from_maps_by_key() {
assertThat(maps).extracting("bad key").containsExactly(null, null);
}

@Test
public void should_extract_field_value_if_only_static_getter_matches_name() {
Object value = propertyOrFieldSupport.getValueOf("city", new StaticPropertyEmployee());

assertThat(value).isEqualTo("New York");
}

@Test
public void should_extract_field_value_if_only_static_is_method_matches_name() {
Object value = propertyOrFieldSupport.getValueOf("tall", new StaticBooleanPropertyEmployee());

assertThat(value).isEqualTo(false);
}

@Test
public void should_extract_field_value_if_only_static_bare_method_matches_name() {
Object value = propertyOrFieldSupport.getValueOf("city", new StaticBarePropertyEmployee());

assertThat(value).isEqualTo("New York");
}

private Employee employeeWithBrokenName(String name) {
return new Employee(1L, new Name(name), 0) {
@Override
Expand Down Expand Up @@ -178,4 +202,24 @@ public boolean isAdult() {
};
}

static class StaticPropertyEmployee extends Employee {
public static String getCity() {
return "London";
}
}

static class StaticBarePropertyEmployee extends Employee {
public static String city() {
return "London";
}
}

static class StaticBooleanPropertyEmployee extends Employee {
boolean tall = false;

public static boolean isTall() {
return true;
}
}

}

0 comments on commit 8d83c41

Please sign in to comment.