Skip to content
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

Exception when deserialization of private record with default constructor #4175

Closed
1 task done
janpachol opened this issue Oct 25, 2023 · 7 comments · Fixed by #4178
Closed
1 task done

Exception when deserialization of private record with default constructor #4175

janpachol opened this issue Oct 25, 2023 · 7 comments · Fixed by #4178
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue Record Issue related to JDK17 java.lang.Record support
Milestone

Comments

@janpachol
Copy link

janpachol commented Oct 25, 2023

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

When deserializing a record, jackson fails with

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of ... (no Creators, like default constructor, exist): cannot deserialize from Object value (no delegate- or property-based Creator)

It looks like jackson does not see default constructor.

Version Information

2.16.0-rc1

Reproduction

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

class TestObjectTest {

    @Test
    public void shouldDeserialize() throws JsonProcessingException {
        TestObject testObject = new ObjectMapper().readValue("{\"text\":\"anything\"}", TestObject.class);
        Assertions.assertEquals("anything", testObject.text());
    }

    private record TestObject(String text) {}
}

When I add compact constructor with @JsonCreator to TestObject, it works correctly:

    private record TestObject(String text) {
        @JsonCreator
        private TestObject {
        }
    }

Additional context

@janpachol janpachol added the to-evaluate Issue that has been received but not yet evaluated label Oct 25, 2023
@janpachol janpachol changed the title Exception when deserialization of record Exception when deserialization of record with default constructor Oct 25, 2023
@pjfanning
Copy link
Member

@janpachol we have tests with records like this. I think that this is most likely down to the record being private as opposed to the fact that jackson can't handle public record TestObject(String text) {}.

@pjfanning
Copy link
Member

I created #4176 as a reproducible test case.

@janpachol
Copy link
Author

Oh, you're correct, but it looks problem is not in private record, but in private constructor (when record is private, it looks that compilation creates also private constructor):

This doesn't work:

    public record TestObject(String text) {
        private TestObject {
        }
    }

This works:

    private record TestObject(String text) {
        public TestObject {
        }
    }

@cowtowncoder cowtowncoder changed the title Exception when deserialization of record with default constructor Exception when deserialization of private record with default constructor Oct 25, 2023
@cowtowncoder cowtowncoder added the Record Issue related to JDK17 java.lang.Record support label Oct 25, 2023
@cowtowncoder cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue and removed to-evaluate Issue that has been received but not yet evaluated labels Oct 25, 2023
@cowtowncoder
Copy link
Member

Yes, it makes sense the issue is private constructor via Record being private (in the original case).
Given it is regression from 2.15, would be great to fix.

@JooHyukKim
Copy link
Member

Created a PR for #4178. The issue seems to link back to the fix for #3906 via 75d0c46 where sort of force setting of record constructor visibility.

@cowtowncoder
Copy link
Member

Cool. While I have am bit concerned about change causing some other regression, there isn't much that can be done without reproduction of such problem.

@JooHyukKim
Copy link
Member

JooHyukKim commented Oct 26, 2023

Cool. While I have am bit concerned about change causing some other regression, there isn't much that can be done without reproduction of such problem.

Wrt regression, we may already have some tests against regression via 228bc0e --though indirectly covers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue Record Issue related to JDK17 java.lang.Record support
Projects
None yet
4 participants