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

DefaultScalaModule breaks deserialization of java classes #644

Closed
kudrevatykh opened this issue Aug 16, 2023 · 13 comments
Closed

DefaultScalaModule breaks deserialization of java classes #644

kudrevatykh opened this issue Aug 16, 2023 · 13 comments

Comments

@kudrevatykh
Copy link

I found problem that adding DefaultScalaModule to object mapper breaks Deserialization of classes that successfuly deserialized without scala module

package test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.scala.DefaultScalaModule;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class ScalaModuleTest {


    public record M(String name, String domain) {
        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        public static M fromString(String uri) {
            String[] parts = uri.split("@");
            if (parts.length > 2 || parts.length == 0) {
                throw new RuntimeException(String.format("Invalid SIP URI: %s", uri));
            }

            String[] usernameParts = parts[0].split("\\.");
            return new M(usernameParts[usernameParts.length - 1], parts.length == 2 ? parts[1] : null);
        }

        @Override
        @JsonValue
        public String toString() {
            return String.format("%s@%s", name, domain);
        }
    }

    public record Obj(M val) {

    }

    @Test
    public void testScalaModule() throws JsonProcessingException {
        var json = "{\"val\": \"a@b\"}";
        var mapper = new ObjectMapper().registerModule(new DefaultScalaModule());
        var val = mapper.readValue(json, Obj.class);
        Assertions.assertEquals(new M("a", "b"), val.val);

    }
}
@pjfanning
Copy link
Member

Please always provide the version of the jar you are using. A fix went in a while ago to stop this module accidentally affecting how Java classes are handled.

@kudrevatykh
Copy link
Author

Sorry, I tried with 2.13-2.14.1 and 2.13-2.15.2 with same results

@kudrevatykh
Copy link
Author

@pjfanning can you tell commit or pull request where this issue was fixed? I can't find anything like this since 2.15.2 released.

@pjfanning
Copy link
Member

The support for checking if classes are Scala classes and trying not to affect how Java classes are handled was added around v2.12.0 but has been adjusted a few times. When people raise issues, there is no guessing if they are using 10 year copies of the code or something up to date.

Are you sure that your deserialization works if you don't add DefaultScalaModule? If you look at Jackson-Databind, support for Java Records has been very problematic. A big change was made in jackson-databind (v2.14.0, I think) that fixed a lot of issues but that broke a load more. Could you try not changing the jackson version and testing if registering DefaultScalaModule affects your deserialization? Your test in the description registers the DefaultScalaModule in its only test case so that proves nothing.

@kudrevatykh
Copy link
Author

As I write earlier I tried latest stable version of jackson (2.15.2) this code still doesn't work. Also I tried to remove records completely from code and it's still doesn't work. Removing DefaultScalaModule from ObjectMapper fixes test.

@pjfanning
Copy link
Member

I'm failing to reproduce this issue. I have a PR with a test that passes. #645

@pjfanning
Copy link
Member

The changes ran for me locally but are failing to compile on the CI run. I'll look into those later.

@pjfanning
Copy link
Member

pjfanning commented Aug 16, 2023

@kudrevatykh which Scala version are using? It seems that older Scala versions cannot deal with Java records.

Could you trying using Scala 2.13.11? Scala 3.x may not yet support Java records either.

@kudrevatykh
Copy link
Author

I used 2.13.8, updating to 2.13.11 doesn't help too

@kudrevatykh
Copy link
Author

I commited full example without records and with latest jackson and scala https://github.com/kudrevatykh/scala-jackson-test
It fails when scala module added and succeds without it

@pjfanning
Copy link
Member

I would recommend not using the DefaultScalaModule in your case. There are no patch releases planned in the near future so even if this module is changed, it could be a while till the next release. This issue is not a high priority for me.

@pjfanning
Copy link
Member

#646 fixes the issue. I have published a 2.16.0-SNAPSHOT jar with the fix.

@kudrevatykh
Copy link
Author

Great news, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants