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

A normal scoped bean implemented as Java record cannot be serialised to json #33810

Closed
JackyAnn opened this issue Jun 4, 2023 · 11 comments · Fixed by #33882
Closed

A normal scoped bean implemented as Java record cannot be serialised to json #33810

JackyAnn opened this issue Jun 4, 2023 · 11 comments · Fixed by #33882
Labels
area/arc Issue related to ARC (dependency injection) kind/bug Something isn't working
Milestone

Comments

@JackyAnn
Copy link

JackyAnn commented Jun 4, 2023

Describe the bug

  /**
   * 
   * Can not be serialised
   * @return
   */
  @Produces
  @ApplicationScoped
  public ApplicationRecord create1(){
    return new ApplicationRecord("ApplicationRecord");
  }

  @Produces
  @Singleton
  public SingletonRecord create2(){
    return new SingletonRecord("SingletonRecord");
  }

  public record ApplicationRecord(String name){

  }

  public record SingletonRecord(String name){

  }
@QuarkusTest
public class GreetingResource1Test {
    @Inject
    private JsonConfigCustomizer.ApplicationRecord applicationRecord;
    @Inject
    private JsonConfigCustomizer.SingletonRecord singletonRecord;
    @Inject
    private Jsonb jsonb;

    @Test
    public void testHelloEndpoint() {
        /**
         * print
         * {}
         */
        System.out.println("---"+jsonb.toJson(applicationRecord));
        
        /**
         * print
         * {
         *     "name": "SingletonRecord"
         * }
         */
        System.out.println("---"+jsonb.toJson(singletonRecord));
    }




}

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

Darwin jackydeMacBook-Pro.local 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:51:50 PDT 2023; root:xnu-8796.121.2~5/RELEASE_X86_64 x86_64

Output of java -version

java version "20" 2023-03-21 Java(TM) SE Runtime Environment (build 20+36-2344) Java HotSpot(TM) 64-Bit Server VM (build 20+36-2344, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.1.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@JackyAnn JackyAnn added the kind/bug Something isn't working label Jun 4, 2023
@JackyAnn
Copy link
Author

JackyAnn commented Jun 4, 2023

@mkouba
Copy link
Contributor

mkouba commented Jun 5, 2023

@JackyAnn thanks for the rerport. Could you be more specific? What exactly does not work? It seems that you're trying to serialize an @ApplicationScoped CDI bean represented by a Java record to JSON using jsonb, right?

@mkouba mkouba changed the title beans proxied by client proxy cannot be serialised A normal scoped bean implemented as Java record cannot be serialised to json Jun 5, 2023
@mkouba mkouba added area/arc Issue related to ARC (dependency injection) and removed triage/needs-triage labels Jun 5, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 5, 2023

/cc @Ladicek (arc), @manovotn (arc)

@Ladicek
Copy link
Contributor

Ladicek commented Jun 5, 2023

At first glance, I don't think it really matters that the bean is a record -- the cause here is that it is normal scoped. That is, you're trying to serialize the client proxy. Could you please try jsonb.toJson(ClientProxy.unwrap(applicationRecord)) and report back?

@JackyAnn
Copy link
Author

JackyAnn commented Jun 5, 2023

@Ladicek
I've tried jsonb.toJson(ClientProxy.unwrap(applicationRecord)) and it still doesn't serialise properly.image

@Ladicek
Copy link
Contributor

Ladicek commented Jun 5, 2023

OK, that sounds weird. I'll take a proper look.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 7, 2023

I have reproduced this locally with my own code and there's one really frustrating thing. If I declare

public class Producers {
    @Produces
    @ApplicationScoped
    public App app() {
        return new App("Application");
    }

    @Produces
    @Singleton
    public Sing sing() {
        return new Sing("Singleton");
    }

    public record App(String name) {
    }

    public record Sing(String name) {
    }
}

then Producers.App.class.isRecord() returns false for me (while Producers.Sing.class.isRecord() is true as expected). I have no idea why.

I'm using OpenJDK 17.0.7.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 7, 2023

Ahh I think I know! We do a bytecode transformation on App which removes the final flag from the class and adds a zero-parameter constructor. In my JDK source code, Class.isRecord() looks like this:

public boolean isRecord() {
    // this superclass and final modifier check is not strictly necessary
    // they are intrinsified and serve as a fast-path check
    return getSuperclass() == java.lang.Record.class &&
            (this.getModifiers() & Modifier.FINAL) != 0 &&
            isRecord0();
}

The class doesn't have the final flag, therefore Class.isRecord() returns false, and that causes Yasson to not use the special case for treating record component accessor methods as getters. Phew!

@Ladicek
Copy link
Contributor

Ladicek commented Jun 7, 2023

I think we probably shouldn't remove the final flag from records. That would prevent using records as normal scoped beans, which is probably a good idea anyway.

@manovotn
Copy link
Contributor

manovotn commented Jun 7, 2023

That would prevent using records as normal scoped beans, which is probably a good idea anyway.

Yea, that sounds fishy anyway.
I was thinking about records and CDI anyway - now that next CDI version is on JDK 17+ (actually, it's likely 21), we should perhaps add a note about this into the spec? I can create an issue

@mkouba
Copy link
Contributor

mkouba commented Jun 7, 2023

I think we probably shouldn't remove the final flag from records. That would prevent using records as normal scoped beans, which is probably a good idea anyway.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants