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

Record deserialisation with instance methods fails #172

Closed
Giovds opened this issue Oct 29, 2024 · 6 comments
Closed

Record deserialisation with instance methods fails #172

Giovds opened this issue Oct 29, 2024 · 6 comments
Labels
2.18 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Milestone

Comments

@Giovds
Copy link

Giovds commented Oct 29, 2024

The deserialisation now works for plain records, as fixed with #167.
However, I've slightly changed my implementation of my record to move some logic for calculating the time into a record instance method. Deserialisation with instance methods fails, as it tries to access a field that does not exist.

The final List<POJODefinition.Prop> rawProps = beanDef.getProperties(); contains a dateTime property which is declared in the record as instance method. When you get the FoundDependency.class declared fields it is not there, as it is a derived value and not a constructor parameter. It does however work (as expected) for static instance methods.

Test case slightly modified to replicate the issue:

public class RecordDeser167Test extends TestCase
{
    record FoundDependency(String id, String g, String a, String v, long timestamp) {
        public LocalDate getDateTime() {
            return Instant.ofEpochMilli(timestamp).atZone(java.time.ZoneId.systemDefault()).toLocalDate();
        }
    }

    // [jackson-jr#167]
    public void testRecordDeserOrder167() throws Exception
    {
        final String input = """
            {
              "id": "org.apache.maven:maven-core:3.9.8",
              "g": "org.apache.maven",
              "a": "maven-core",
              "v": "3.9.8",
              "p": "jar",
              "timestamp": 1718267050000,
              "ec": [
                "-cyclonedx.json",
                "-sources.jar",
                "-cyclonedx.xml",
                ".pom",
                "-javadoc.jar",
                ".jar"
              ],
              "tags": [
                "core",
                "maven",
                "classes"
              ]
            }
            """;
        final var expected = new FoundDependency("org.apache.maven:maven-core:3.9.8", "org.apache.maven", "maven-core", "3.9.8", 1718267050000L);
        final var actual = JSON.std.beanFrom(FoundDependency.class, input);
        assertEquals(expected, actual);
    }
}
@cowtowncoder
Copy link
Member

Thank you for reporting this @Giovds. Definitely challenging to have minimal-change implementation for Record deserialization... it is bit fragile to say the least.

@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Oct 30, 2024
cowtowncoder added a commit that referenced this issue Nov 5, 2024
@cowtowncoder cowtowncoder added this to the 2.18.2 milestone Nov 5, 2024
@cowtowncoder
Copy link
Member

Fixed in 2.18 branch for 2.18.2; should be testable with 2.18.2-SNAPSHOT (added a test but just in case).

@Giovds
Copy link
Author

Giovds commented Nov 7, 2024

Awesome work! I’ll give it a swing once I’m able to 💪

@cowtowncoder
Copy link
Member

Ok: due to code being bit... minimalistic.. I wouldn't be surprised if you couldn't find other problems still. Eagerly awaiting bug reports :)

@Giovds
Copy link
Author

Giovds commented Nov 21, 2024

Somewhat late response. I've been able to test it on my repository. It seems to work as expected. 👍

@cowtowncoder
Copy link
Member

@Giovds better late than never :)

Thank you for confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

2 participants