Skip to content

Commit f360fec

Browse files
janeisklarjaneisklar
committed
Fixed FasterXML#2729 conflicting getters/setters when not respecting capitalization
1 parent b83ab88 commit f360fec

File tree

5 files changed

+186
-8
lines changed

5 files changed

+186
-8
lines changed

‎src/main/java/com/fasterxml/jackson/databind/MapperFeature.java

+14
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,20 @@ public enum MapperFeature implements ConfigFeature
162162
*/
163163
REQUIRE_SETTERS_FOR_GETTERS(false),
164164

165+
/**
166+
* Feature that determines whether property accessors are required to be
167+
* capitalized, i.e. whether "getx()" should be regarded a valid getter for
168+
* property x when the feature is disabled. Whereas when enabled only
169+
* "getX()" represents a proper getter for property x.
170+
* Disabling it might produce issues with scala property accessors for
171+
* properties that start with "is", "get" or "set", e.g. the setter method
172+
* "settlement(string)" might be confused with a setter for the property
173+
* "tlement" rather than the property "settlement".
174+
* *<p>
175+
* Feature is enabled by default.
176+
*/
177+
REQUIRE_CAPITALIZED_PROPERTY_ACCESSOR_NAME(true),
178+
165179
/**
166180
* Feature that determines whether member fields declared as 'final' may
167181
* be auto-detected to be used mutators (used to change value of the logical

‎src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java

+10-5
Original file line numberDiff line numberDiff line change
@@ -580,11 +580,10 @@ protected int _getterPriority(AnnotatedMethod m)
580580
{
581581
final String name = m.getName();
582582
// [databind#238]: Also, regular getters have precedence over "is-getters"
583-
if (name.startsWith("get") && name.length() > 3) {
584-
// should we check capitalization?
583+
if (validateAccessorPrefix(name, "get")) {
585584
return 1;
586585
}
587-
if (name.startsWith("is") && name.length() > 2) {
586+
if (validateAccessorPrefix(name, "is")) {
588587
return 2;
589588
}
590589
return 3;
@@ -593,13 +592,19 @@ protected int _getterPriority(AnnotatedMethod m)
593592
protected int _setterPriority(AnnotatedMethod m)
594593
{
595594
final String name = m.getName();
596-
if (name.startsWith("set") && name.length() > 3) {
597-
// should we check capitalization?
595+
if (validateAccessorPrefix(name, "set")) {
598596
return 1;
599597
}
600598
return 2;
601599
}
602600

601+
private boolean validateAccessorPrefix(String name, String prefix)
602+
{
603+
final int prefixLength = prefix.length();
604+
return name.startsWith(prefix) && name.length() > prefixLength
605+
&& (!_config.isEnabled(MapperFeature.REQUIRE_CAPITALIZED_PROPERTY_ACCESSOR_NAME) || Character.isUpperCase(name.charAt(prefixLength)));
606+
}
607+
603608
/*
604609
/**********************************************************
605610
/* Implementations of refinement accessors

‎src/test/java/com/fasterxml/jackson/databind/cfg/DeserializationConfigTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public void testFeatureDefaults()
2222
assertTrue(cfg.isEnabled(MapperFeature.AUTO_DETECT_CREATORS));
2323
assertTrue(cfg.isEnabled(MapperFeature.USE_GETTERS_AS_SETTERS));
2424
assertTrue(cfg.isEnabled(MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS));
25+
assertTrue(cfg.isEnabled(MapperFeature.REQUIRE_CAPITALIZED_PROPERTY_ACCESSOR_NAME));
2526

2627
assertFalse(cfg.isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS));
2728
assertFalse(cfg.isEnabled(DeserializationFeature.USE_BIG_INTEGER_FOR_INTS));

‎src/test/java/com/fasterxml/jackson/databind/introspect/PropertyNameSetterGetterConflictTest.java

+151
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
package com.fasterxml.jackson.databind.introspect;
2+
3+
import com.fasterxml.jackson.databind.BaseMapTest;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
6+
import java.lang.reflect.Field;
7+
8+
// [databind#2729]
9+
public class PropertyNameSetterGetterConflictTest extends BaseMapTest {
10+
11+
private final ObjectMapper MAPPER = mapperWithScalaModule();
12+
13+
// Should work with setters named exactly like the property
14+
static class Issue2729BeanWithFieldNameSetterGetter {
15+
private String value;
16+
17+
public void value(String v) { value = v; }
18+
public String value() { return value; }
19+
}
20+
21+
// Should prefer java bean naming convention over property names
22+
static class Issue2729BeanWithoutCaseSensitivityBean {
23+
private String value;
24+
25+
public void value(String v) { throw new Error("Should not get called"); }
26+
public String value() { throw new Error("Should not get called"); }
27+
public void setValue(String v) { value = v; }
28+
public String getValue() { return value; }
29+
}
30+
31+
// Should prefer java bean naming convention over property names while respecting case-sensitivity
32+
static class Issue2729WithCaseSensitivityBean {
33+
private String settlementDate;
34+
private String getaways;
35+
private Boolean island;
36+
37+
public void settlementDate(String v) { throw new Error("Should not get called"); }
38+
public String settlementDate() { throw new Error("Should not get called"); }
39+
public void setSettlementDate(String v) { settlementDate = v; }
40+
public String getSettlementDate() { return settlementDate; }
41+
42+
public void getaways(String v) { throw new Error("Should not get called"); }
43+
public String getaways() { throw new Error("Should not get called"); }
44+
public void setGetaways(String v) { getaways = v; }
45+
public String getGetaways() { return getaways; }
46+
47+
public void island(Boolean v) { throw new Error("Should not get called"); }
48+
public Boolean island() { throw new Error("Should not get called"); }
49+
public void setIsland(Boolean v) { island = v; }
50+
public Boolean isIsland() { return island; }
51+
}
52+
53+
/*
54+
/**********************************************************
55+
/* Test methods
56+
/**********************************************************
57+
*/
58+
59+
public void testSetterPriorityForFieldNameSetter() throws Exception
60+
{
61+
Issue2729BeanWithFieldNameSetterGetter bean = MAPPER.readValue(aposToQuotes("{'value':'42'}"),
62+
Issue2729BeanWithFieldNameSetterGetter.class);
63+
assertEquals("42", bean.value);
64+
}
65+
66+
public void testSetterPriorityForJavaBeanNamingConvention() throws Exception
67+
{
68+
Issue2729BeanWithoutCaseSensitivityBean bean = MAPPER.readValue(aposToQuotes("{'value':'42'}"),
69+
Issue2729BeanWithoutCaseSensitivityBean.class);
70+
assertEquals("42", bean.value);
71+
}
72+
73+
public void testSetterPriorityForJavaBeanNamingConventionWhileRespectingCaseSensitivity() throws Exception
74+
{
75+
final Issue2729WithCaseSensitivityBean bean = MAPPER.readValue(aposToQuotes("{'settlementDate':'42'}"),
76+
Issue2729WithCaseSensitivityBean.class);
77+
assertEquals("42", bean.settlementDate);
78+
}
79+
80+
public void testSetterPriorityForJavaBeanNamingConventionWhileRespectingCaseSensitivity2() throws Exception
81+
{
82+
final Issue2729WithCaseSensitivityBean bean = MAPPER.readValue(aposToQuotes("{'island':true}"),
83+
Issue2729WithCaseSensitivityBean.class);
84+
assertEquals(Boolean.TRUE, bean.island);
85+
}
86+
87+
public void testGetterPriorityForFieldNameSetter() throws Exception
88+
{
89+
final Issue2729BeanWithFieldNameSetterGetter bean = new Issue2729BeanWithFieldNameSetterGetter();
90+
bean.value("42");
91+
assertEquals(aposToQuotes("{'value':'42'}"), MAPPER.writeValueAsString(bean));
92+
}
93+
94+
public void testGetterPriorityForJavaBeanNamingConvention() throws Exception
95+
{
96+
97+
final Issue2729BeanWithoutCaseSensitivityBean bean = new Issue2729BeanWithoutCaseSensitivityBean();
98+
bean.setValue("42");
99+
assertEquals(aposToQuotes("{'value':'42'}"), MAPPER.writeValueAsString(bean));
100+
}
101+
102+
public void testGetterPriorityForJavaBeanNamingConventionWhileRespectingCaseSensitivity() throws Exception
103+
{
104+
final Issue2729WithCaseSensitivityBean bean = new Issue2729WithCaseSensitivityBean();
105+
bean.setGetaways("42");
106+
assertEquals(aposToQuotes("{'settlementDate':null,'getaways':'42','island':null}"), MAPPER.writeValueAsString(bean));
107+
}
108+
109+
public void testGetterPriorityForJavaBeanNamingConventionWhileRespectingCaseSensitivity2() throws Exception
110+
{
111+
final Issue2729WithCaseSensitivityBean bean = new Issue2729WithCaseSensitivityBean();
112+
bean.setIsland(true);
113+
assertEquals(aposToQuotes("{'settlementDate':null,'getaways':null,'island':true}"), MAPPER.writeValueAsString(bean));
114+
}
115+
116+
/*
117+
/**********************************************************
118+
/* Helper methods
119+
/**********************************************************
120+
*/
121+
122+
private ObjectMapper mapperWithScalaModule()
123+
{
124+
ObjectMapper m = new ObjectMapper();
125+
m.setAnnotationIntrospector(new ScalaLikeAnnotationIntrospector());
126+
return m;
127+
}
128+
129+
static class ScalaLikeAnnotationIntrospector extends JacksonAnnotationIntrospector
130+
{
131+
private static final long serialVersionUID = 1L;
132+
133+
@Override
134+
public String findImplicitPropertyName(AnnotatedMember member) {
135+
if (member instanceof AnnotatedMethod) {
136+
return hasCorrespondingProperty(((AnnotatedMethod) member)) ? member.getName() : null;
137+
}
138+
return null;
139+
}
140+
141+
private static boolean hasCorrespondingProperty(AnnotatedMethod method) {
142+
final String name = method.getName();
143+
for (Field f : method.getDeclaringClass().getDeclaredFields()) {
144+
if (name.equals(f.getName())) {
145+
return true;
146+
}
147+
}
148+
return false;
149+
}
150+
}
151+
}

‎src/test/java/com/fasterxml/jackson/databind/introspect/TestPropertyConflicts.java

+10-3
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,23 @@ public String getStr() {
8888
/**********************************************************
8989
*/
9090

91-
public void testFailWithDupProps() throws Exception
91+
public void testFailWithDupPropsWhenAccessCapitalizationIsTurnedOff() throws Exception
9292
{
9393
BeanWithConflict bean = new BeanWithConflict();
9494
try {
95-
String json = objectWriter().writeValueAsString(bean);
95+
String json = jsonMapperBuilder().configure(MapperFeature.REQUIRE_CAPITALIZED_PROPERTY_ACCESSOR_NAME, false).build().writeValueAsString(bean);
9696
fail("Should have failed due to conflicting accessor definitions; got JSON = "+json);
9797
} catch (JsonProcessingException e) {
9898
verifyException(e, "Conflicting getter definitions");
9999
}
100-
}
100+
}
101+
102+
public void testDoesNotFailWithDupPropsWhenAccessCapitalizationIsTurnedOn() throws Exception
103+
{
104+
BeanWithConflict bean = new BeanWithConflict();
105+
String json = jsonMapperBuilder().configure(MapperFeature.REQUIRE_CAPITALIZED_PROPERTY_ACCESSOR_NAME, true).build().writeValueAsString(bean);
106+
assertEquals(aposToQuotes("{'x':3}"), json);
107+
}
101108

102109
// [databind#238]: ok to have getter, "isGetter"
103110
public void testRegularAndIsGetter() throws Exception

0 commit comments

Comments
 (0)