Skip to content

Fix #5152: support "iPhone" style properties -- add MapperFeature.FIX_FIELD_NAME_CASE_MISMATCH #5187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: 2.x
Choose a base branch
from
2 changes: 2 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Project: jackson-databind
serialization
#5151: Add new exception type, `MissingInjectValueException`, to be used
for failed `@JacksonInject`
#5152: Support "iPhone" style capitalized properties (add
`MapperFeature.FIX_FIELD_NAME_CASE_MISMATCH`)
#5179: Add "current token" info into `MismatchedInputException`
#5192: Record types are broken on Android when using R8
(reported by @HelloOO7)
Expand Down
28 changes: 22 additions & 6 deletions src/main/java/com/fasterxml/jackson/databind/MapperFeature.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,17 @@ public enum MapperFeature implements ConfigFeature
*/
INFER_CREATOR_FROM_CONSTRUCTOR_PROPERTIES(true),

/**
* Feature that when enabled will allow getters with is-Prefix also for
* non-boolean return types; if disabled only methods that return
* {@code boolean} or {@code Boolean} qualify as "is getters".
* <p>
* Feature is disabled by default for backwards compatibility.
*
* @since 2.14
*/
ALLOW_IS_GETTERS_FOR_NON_BOOLEAN(false),

/**
* Feature that determines whether nominal property type of {@link Void} is
* allowed for Getter methods to indicate {@code null} valued pseudo-property
Expand Down Expand Up @@ -541,15 +552,20 @@ public enum MapperFeature implements ConfigFeature
ALLOW_EXPLICIT_PROPERTY_RENAMING(false),

/**
* Feature that when enabled will allow getters with is-Prefix also for
* non-boolean return types; if disabled only methods that return
* {@code boolean} or {@code Boolean} qualify as "is getters".
* Feature that can be enabled to solve problem where an upper-case letter in
* the first 2 characters of Java field name (like {@code "IPhone"} or {@code "iPhone"})
* prevents match with property name derived from accessors (setter like
* {@code getIPhone()} becomes {@code "iphone"}).
* If enabled, additional checking is done with case-insensitive comparison (for
* cases of the first or second letter of Field name being upper-case) to merge
* accessors. If disabled, no special processing is done.
* <p>
* Feature is disabled by default for backwards compatibility.
* Feature is disabled by default in 2.x for backwards-compatibility.
* It will be enabled by default in 3.0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabled in 3.0 ? Let's add documentation label then

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, need a follow-up issue for defaults changing after this gets merged.

*
* @since 2.14
* @since 2.20
*/
ALLOW_IS_GETTERS_FOR_NON_BOOLEAN(false),
FIX_FIELD_NAME_CASE_MISMATCH(false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is... Would the name imply we would fix any case difference issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. So maybe should still consider name to use...

In a way it is not necessarily limited to first two, but it only checks cases where Field name has first and/or second character upper case. Since those are ones where name-mangling can produce differences. But comparison itself consider whole implicit names.
So bit tricky to explain.

Alternatively could change logic itself to actually consider all cases of "lone" (unmatched) Fields, either with at least one upper-case letter, or possibly just any (since Getter/Setter name could have upper-case even without Field)


/*
/******************************************************
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -433,20 +433,23 @@ protected void collectAll()
_potentialCreators = new PotentialCreators();

// First: gather basic accessors
LinkedHashMap<String, POJOPropertyBuilder> props = new LinkedHashMap<String, POJOPropertyBuilder>();
LinkedHashMap<String, POJOPropertyBuilder> props = new LinkedHashMap<>();

// 14-Nov-2024, tatu: Previously skipped checking fields for Records; with 2.18+ won't
// (see [databind#3628], [databind#3895], [databind#3992], [databind#4626])
_addFields(props); // note: populates _fieldRenameMappings

_addMethods(props);
// 25-Jan-2016, tatu: Avoid introspecting (constructor-)creators for non-static
// inner classes, see [databind#1502]
// 14-Nov-2024, tatu: Similarly need Creators for Records too (2.18+)
if (!_classDef.isNonStaticInnerClass()) {
_addCreators(props);
}

// 11-Jun-2025, tatu: [databind#5152] May need to "fix" mis-matching leading case
// wrt Fields vs Accessors
if (_config.isEnabled(MapperFeature.FIX_FIELD_NAME_CASE_MISMATCH)) {
_fixLeadingFieldNameCase(props);
}
// Remove ignored properties, first; this MUST precede annotation merging
// since logic relies on knowing exactly which accessor has which annotation
_removeUnwantedProperties(props);
Expand Down Expand Up @@ -547,10 +550,9 @@ private Map<String, POJOPropertyBuilder> _putAnyGettersInTheEnd(
protected void _addFields(Map<String, POJOPropertyBuilder> props)
{
final AnnotationIntrospector ai = _annotationIntrospector;
/* 28-Mar-2013, tatu: For deserialization we may also want to remove
* final fields, as often they won't make very good mutators...
* (although, maybe surprisingly, JVM _can_ force setting of such fields!)
*/
// 28-Mar-2013, tatu: For deserialization we may also want to remove
// final fields, as often they won't make very good mutators...
// (although, maybe surprisingly, JVM _can_ force setting of such fields!)
final boolean pruneFinalFields = !_forSerialization && !_config.isEnabled(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS);
final boolean transientAsIgnoral = _config.isEnabled(MapperFeature.PROPAGATE_TRANSIENT_MARKER);

Expand Down Expand Up @@ -1318,6 +1320,97 @@ private String _checkRenameByField(String implName) {
return implName;
}

/*
/**********************************************************
/* Internal methods; merging/fixing case-differences
/**********************************************************
*/

// @since 2.20
protected void _fixLeadingFieldNameCase(Map<String, POJOPropertyBuilder> props)
{
// 11-Jun-2025, tatu: [databind#5152] May need to "fix" mis-matching leading case
// wrt Fields vs Accessors

// First: find possible candidates where:
//
// 1. Property only has Field
// 2. Field does NOT have explicit name (renaming)
// 3. Implicit name has upper-case for first and/or second character

Map<String, POJOPropertyBuilder> fieldsToCheck = null;
for (Map.Entry<String, POJOPropertyBuilder> entry : props.entrySet()) {
POJOPropertyBuilder prop = entry.getValue();

// First: (1) and (2)
if (!prop.hasFieldAndNothingElse()
|| prop.isExplicitlyNamed()) {
continue;
}
// Second: (3)
if (!_firstOrSecondCharUpperCase(entry.getKey())) {
continue;
}
if (fieldsToCheck == null) {
fieldsToCheck = new HashMap<>();
}
fieldsToCheck.put(entry.getKey(), prop);
}
/*// DEBUGGING
if (fieldsToCheck == null) {
System.err.println("_fixLeadingCase, candidates -> null; props -> "+props.keySet());
} else {
System.err.println("_fixLeadingCase, candidates -> "+fieldsToCheck);
}
*/

if (fieldsToCheck == null) {
return;
}

for (Map.Entry<String, POJOPropertyBuilder> fieldEntry : fieldsToCheck.entrySet()) {
Iterator<Map.Entry<String, POJOPropertyBuilder>> it = props.entrySet().iterator();
final POJOPropertyBuilder fieldProp = fieldEntry.getValue();
final String fieldName = fieldEntry.getKey();

while (it.hasNext()) {
Map.Entry<String, POJOPropertyBuilder> propEntry = it.next();
final POJOPropertyBuilder prop = propEntry.getValue();

// Skip anything that has Field (can't merge)
if (prop == fieldProp || prop.hasField()) {
continue;
}
if (fieldName.equalsIgnoreCase(propEntry.getKey())) {
// Remove non-Field property; add its accessors to Field one
it.remove();
fieldProp.addAll(prop);
// Should we continue with possible other accessors?
// For now assume only one merge needed/desired
break;
}
}
}
}

// @since 2.20
private boolean _firstOrSecondCharUpperCase(String name) {
switch (name.length()) {
case 0:
return false;
default:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this neccessary to place default statement in the middle?

Unless strongly required, Could switch to if else statement

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could move 0 as last, making default first entry. Not sure if that's any better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah not a blocker. We can go on

if (!Character.isLowerCase(name.charAt(1))) {
return true;
}
// fall through
case 1:
if (!Character.isLowerCase(name.charAt(0))) {
return true;
}
return false;
}
}

/*
/**********************************************************
/* Internal methods; removing ignored properties
Expand Down Expand Up @@ -1420,6 +1513,7 @@ protected void _renameProperties(Map<String, POJOPropertyBuilder> props)
// With renaming need to do in phases: first, find properties to rename
Iterator<Map.Entry<String,POJOPropertyBuilder>> it = props.entrySet().iterator();
LinkedList<POJOPropertyBuilder> renamed = null;

while (it.hasNext()) {
Map.Entry<String, POJOPropertyBuilder> entry = it.next();
POJOPropertyBuilder prop = entry.getValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ public Class<?> getRawPrimaryType() {
@Override
public boolean hasField() { return _fields != null; }

// @since 2.20 additional accessor
public boolean hasFieldAndNothingElse() {
return (_fields != null)
&& ((_getters == null) && (_setters == null) && (_ctorParameters == null));
}

@Override
public boolean hasConstructorParameter() { return _ctorParameters != null; }

Expand Down Expand Up @@ -416,9 +422,8 @@ public AnnotatedMethod getGetter()
}
// But if multiple, verify that they do not conflict...
for (; next != null; next = next.next) {
/* [JACKSON-255] Allow masking, i.e. do not report exception if one
* is in super-class from the other
*/
// Allow masking, i.e. do not report exception if one
// is in super-class from the other
Class<?> currClass = curr.value.getDeclaringClass();
Class<?> nextClass = next.value.getDeclaringClass();
if (currClass != nextClass) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package com.fasterxml.jackson.databind.tofix;
package com.fasterxml.jackson.databind.misc;

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.testutil.DatabindTestUtil;
import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected;

import static org.junit.jupiter.api.Assertions.*;

Expand Down Expand Up @@ -73,10 +71,27 @@ public void setPhone(String value) {
}
}

// [databind#2696]
static class OAuthTokenBean {
protected String oAuthToken;

public OAuthTokenBean(String t) {
oAuthToken = t;
}

public String getOAuthToken() {
return this.oAuthToken;
}

public void setOAuthToken(String oAuthToken) {
this.oAuthToken = oAuthToken;
}
}

private final ObjectMapper MAPPER = jsonMapperBuilder()
.enable(MapperFeature.FIX_FIELD_NAME_CASE_MISMATCH)
.build();

@JacksonTestFailureExpected
@Test
public void testIPhoneStyleProperty() throws Exception {
// Test with iPhone style property
Expand Down Expand Up @@ -104,21 +119,19 @@ public void testRegularPojoProperty() throws Exception {
}

// [databind#2835]: "dLogHeader" property
@JacksonTestFailureExpected
@Test
public void testDLogHeaderStyleProperty() throws Exception {
// Test with DLogHeader style property
String json = "{\"dLogHeader\":\"Debug Log Header\"}";
String json = "{\"DLogHeader\":\"Debug Log Header\"}";
DLogHeaderBean result = MAPPER.readValue(json, DLogHeaderBean.class);
assertNotNull(result);
assertEquals("Debug Log Header", result.getDLogHeader());

// Test serialization
String serialized = MAPPER.writeValueAsString(result);
assertEquals("{\"dLogHeader\":\"Debug Log Header\"}", serialized);
assertEquals("{\"DLogHeader\":\"Debug Log Header\"}", serialized);
}

@JacksonTestFailureExpected
@Test
public void testKBSBroadCastingStyleProperty() throws Exception {
// Test with KBSBroadCasting style property
Expand All @@ -132,7 +145,6 @@ public void testKBSBroadCastingStyleProperty() throws Exception {
assertEquals("{\"KBSBroadCasting\":\"Korean Broadcasting System\"}", serialized);
}

@JacksonTestFailureExpected
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make this test case valid then we may want to change JavaDoc for our new feature because it says first or second character are only supported

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's... bit tricky to explain logic. Basically only cases where name-mangling should produce mismatches are if name starts with one or more upper-case (non-lower case) letters -- like "KBSBroadcasting" -- or starts with lower-case but is immediately followed by one or more upper-case letters -- like "iPhone". But in either case, it can be a sequence of more than one upper-case letters that need to be considered in case-insensitive manner.

So logic is:

  1. Check if and only if:
    • Field name has first and/or second letter upper-case; and that name does not yet match any other accessor
  2. If checking, see if Field name matches, case excluded to ANY other accessor sets (that do not have Field matched)
  3. If check matches, combine Field and non-Field accessors, using Field name as-is (not mangled getter/setter name)

@Test
public void testPhoneStyleProperty() throws Exception {
// Test with Phone style property
Expand All @@ -146,4 +158,10 @@ public void testPhoneStyleProperty() throws Exception {
assertEquals("{\"Phone\":\"iPhone 15\"}", serialized);
}

}
// [databind#2696]
@Test
public void testOAuthProperty() throws Exception {
assertEquals(a2q("{'oAuthToken':'123'}"),
MAPPER.writeValueAsString(new OAuthTokenBean("123")));
}
}