Deserialization of record fails on constructor parameter ordering

[Giovds]

I was testing if I could use record deserialization in my project and I found that it fails to match the correct parameter in the record constructor. I seems to follow alphabetic order. I'm not sure if this can differ in other JDK implementations. I'm using azul-21.

The ValueReaderLocator#_resolveBeanForDeser() gets the parameters from the record, which seem to be returned in alphabetic order. This ordering will then be used when iterating over that list and thus using the incorrect constructor index when creating the BeanPropertyReader here. If all types match it will continue to parse and will return incorrect values. Those are eventually read in the BeanReader based on this indexing as shown in the test provided. If they don't match it results in a type mismatch exception.

I have added a test below to reproduce the issue, but you can recreate it with the Cow examples as well by renaming the fields.

record FoundDependency(String id, String g, String a, String v, String timestamp) {}

public void testOrderOfParamsFails() throws Exception {
    final var 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", "1718267050000");
    final var actual = jsonHandler.beanFrom(FoundDependency.class, input);
    assertEquals(expected, actual);
}

[cowtowncoder]

Ok sounds like a flaw -- thank you for reporting this.

[cowtowncoder]

cc @TomaszGaweda looks like there's some work left to do with Record deserialization.

[Giovds]

I think I know how to fix this. I'd be happy to provide a PR later this week, if that is ok.

[cowtowncoder]

That would be great!

…

On Sun, Oct 13, 2024 at 12:22 AM Giovanni van der Schelde < ***@***.***> wrote: I think I know how to fix this. I'd be happy to provide a PR later this week, if that is ok. — Reply to this email directly, view it on GitHub <#167 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANOGLVQPCSGL35I3WSP3LZ3INS5AVCNFSM6AAAAABPNFWLYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBYHA2TSMJSHA> . You are receiving this because you commented.Message ID: ***@***.***>

[Giovds]

@cowtowncoder @TomaszGaweda I think the approach I propose in the PR should solve the issue. If there are any concerns please let me know.

I'm a bit hesitant on whether or not BeanPropertyReader should be using a index if it isn't a record, so perhaps that should be a different model to keep the concerns separate. I think it is only used for reading the index when reading to a Record.