Constructor autodetection does not take naming strategy into account

[joca-bt]

Constructor autodetection (https://cowtowncoder.medium.com/jackson-2-12-most-wanted-3-5-246624e2d3d0/) does not appear to be working when used together with naming strategies which don't match the field names. See the examples below for more details.

public class Test {
    private final String fieldName;

    public Test(String fieldName) {
        this.fieldName = fieldName;
    }
}

ObjectMapper objectMapper = JsonMapper.builder()
    .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
    .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
    .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
    .build();
// Ok. Field name in JSON matches the one of the Java class so Jackson is able to find it, however, I've defined my strategy as snake case, so I was expecting fieldName to be invalid.
objectMapper.readValue("{ \"fieldName\": \"value\" }", Test.class);
// Fails. Field name doesn't match and the property naming strategy isn't being taken into consideration.
objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class);

public class Test {
    private final String fieldName;

    public Test(@JsonProperty("field_name") String fieldName) {
        this.fieldName = fieldName;
    }
}

ObjectMapper objectMapper = JsonMapper.builder()
    .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
    .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
    .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
    .build();
// Ok. However we are helping Jackson by using @JsonProperty, which defeats the purpose of annotationless deserialization.
objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class);

public class Test {
    private String fieldName;

    public Test(String fieldName) {
        this.fieldName = fieldName;
    }
    
    public void setFieldName(String fieldName) {
        this.fieldName = fieldName;
    }
}

ObjectMapper objectMapper = JsonMapper.builder()
    .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
    .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
    .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
    .build();
// Ok. Jackson is able to find the field without needing any annotation, but we need to define a setter. This is a no-go for records.
objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class);

[JooHyukKim]

Seems like problem is with detecting properties and registering PropertyNamingStrategies... 🤔
Maybe below test is more narrowed down? -- note that jackson-module-parameter-names module is assumed to exist.

    static class PointFail {
        private int xNum;
        private int yNum;

        public PointFail(int xNum, int yNum) {
            this.xNum = xNum;
            this.yNum = yNum;
        }
    }

    static class PointSuccess {
        private int xNum;
        private int yNum;

        public PointSuccess(int xNum, int yNum) {
            this.xNum = xNum;
            this.yNum = yNum;
        }

        public void setxNum(int xNum) {
            this.xNum = xNum;
        }

        public void setyNum(int yNum) {
            this.yNum = yNum;
        }
    }

    // with `jackson-module-parameter-names` module
    ObjectMapper mapper = JsonMapper.builder()
        .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
        .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
        .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
        .build();

    // Fails
    public void testPointFails() throws Exception {
        PointFail strategyBean = mapper
            .readValue(a2q("{'x_num':1, 'y_num':2}"), PointFail.class);

        assertEquals(strategyBean.xNum, 1);
        assertEquals(strategyBean.yNum, 2);
    }

    // Success, with setters OR getters
    public void testPointSuccess() throws Exception {
        PointSuccess strategyBean = mapper
            .readValue(a2q("{'x_num':1, 'y_num':2}"), PointSuccess.class);

        assertEquals(strategyBean.xNum, 1);
        assertEquals(strategyBean.yNum, 2);
    }

[cowtowncoder]

Yes, @JooHyukKim is right; this is due to issue discussed under #3719 and just one symptom.

[yihtserns]

Just a side note - about:

ObjectMapper objectMapper = JsonMapper.builder()
    .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
    .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
    .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
    .build();
// Ok. Jackson is able to find the field without needing any annotation, but we need to define a setter.
// *****This is a no-go for records.****** <---------------------
objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class);

...PropertyNamingStrategy actually never worked with Records (#2992), but that has been fixed in 2.15.0 (tested with 2.15.0-rc2), so now it can work without any special configuration, e.g.:

record Test(String fieldName) {
}

ObjectMapper objectMapper = JsonMapper.builder()
        .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
        .build();

// Prints Test[fieldName=value]
System.out.println(objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class));

[yihtserns]

Asking out of curiousity: if the field is private & it doesn't have a getter method, how is the class actually used?

[joca-bt]

The point is that it doesn't have a setter. Getter is irrelevant here.

[yihtserns]

It works if there's a getter, though. Why is a setter relevant?

[yihtserns]

I mean as pointed out here, naming strategy is used when a getter exists e.g.:

public class Test {
    private final String fieldName;

    public Test(String fieldName) {
        this.fieldName = fieldName;
    }

    public String getFieldName() { // with getter
        return this.fieldName;
    }
}

ObjectMapper objectMapper = JsonMapper.builder()
    .addModule(new ParameterNamesModule(JsonCreator.Mode.PROPERTIES))
    .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
    .propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
    .build();

objectMapper.readValue("{ \"field_name\": \"value\" }", Test.class); // This works
objectMapper.readValue("{ \"fieldName\": \"value\" }", Test.class); // This also works, but that's a (different) bug

[joca-bt]

Why is a setter relevant?

If there is a setter Jackson may use that instead of the constructor.

[yihtserns]

For naming strategy to work for a "property", one of these is currently needed:

1. Visible field (e.g. public field, or private field + setVisibility(PropertyAccessor.FIELD, ANY)).
2. Getter.
3. Setter.
4. Method parameter with implicit name (e.g. with ParameterNamesModule), for a @JsonCreator(mode=PROPERTIES) annotated (valueOf) factory method.
5. Constructor parameter with implicit name (e.g. with ParameterNamesModule), for a @JsonCreator annotated constructor.
6. Constructor parameter for Record's canonical constructor.

So one of the things you can do to make naming strategy to work with:

public class Test {
    private final String fieldName;

    public Test(String fieldName) {
        this.fieldName = fieldName;
    }
}

...is to add a getter:

public class Test {
    private final String fieldName;

    public Test(String fieldName) {
        this.fieldName = fieldName;
    }

    public String getFieldName() {
        return this.fieldName;
    }
}

...which is pretty normal because how else can that private field be used?

I'm just curious about what are you doing with your JSON model class in your project/codebase, for it to have a private field without getter, resulting in you facing this issue?

[joca-bt]

This example is about #5.

Constructor parameter with implicit name (e.g. with ParameterNamesModule), for a @JsonCreator annotated constructor.

Except @JsonCreator should not be needed contrary to what you wrote. You can read more about it here https://cowtowncoder.medium.com/jackson-2-12-most-wanted-3-5-246624e2d3d0.

for it to have a private field without getter, resulting in you facing this issue?

I am not. It was not relevant for the example but I can see it being valid. Not all instance fields need/should have getters 🙃.

[yihtserns]

Constructor parameter with implicit name (e.g. with ParameterNamesModule), for a @JsonCreator annotated constructor.

Except @JsonCreator should not be needed contrary to what you wrote. You can read more about it here https://cowtowncoder.medium.com/jackson-2-12-most-wanted-3-5-246624e2d3d0.

I specifically said "one of these is currently needed"... Meaning, even if you don't have number 5, as long as you have either 1, 2, 3, 4, or 6, naming strategy will work.

I am not. It was not relevant for the example but I can see it being valid. Not all instance fields need/should have getters 🙃.

OK so you're not reporting for a practical problem (i.e. "I'm facing this issue now!"), but rather an edge-case you found while testing Jackson. I just wanted to know if I'm missing anything I should be careful of as a Jackson user, thanks!

[cowtowncoder]

As per my earlier notes, I think it is very likely this is related to the problem of "implicit" Creators being discovered too late to be properly linked to accessors (getters/setters): explicit (annotation indicated) Creators are discovered first, their arguments get matched properly with getters/setters; implicit ones are discovered at a later phase.
It is sometimes possible to force proper linking by additional annotations (obviously @JsonCreator, but also @JsonProperty on Creator parameters).

This is something that I think requires a rewrite of property discovery (see #3719), and if and when this happens can be resolved. But I do not think smaller incremental fixes are possible.

[cowtowncoder]

Work on #4515 may make this fixable for 2.18.

What would be great would be a reproduction (unit test), to help test eventual fix.

[cowtowncoder]

Now that #4515 is fixed, I think this issue is likely covered as well.
Since no unit test added, I consider this as solved; may be reopened/re-filed with a reproduction (test) if there are further issues.