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

Support record parameter containers #42642

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Support record parameter containers #42642

merged 1 commit into from
Aug 26, 2024

Conversation

FroMage
Copy link
Member

@FroMage FroMage commented Aug 20, 2024

Also unify the parameter container detection code paths Make sure we do not turn record parameter containers into beans
For records, we store every contructor parameter in a local variable until we have enough to call the constructor via a generated static method.

I'll open an issue with the work remaining, but this has already taken me too long to do I need to get back to the other stuff for now :(

This comment has been minimized.

Copy link

github-actions bot commented Aug 20, 2024

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@FroMage
Copy link
Member Author

FroMage commented Aug 20, 2024

Naturally, the TCK had to fail…

@FroMage
Copy link
Member Author

FroMage commented Aug 20, 2024

Now this is interesting. It fails because the TCK has an Application class limiting the set of classes:

public class AppConfig extends Application {

    public java.util.Set<java.lang.Class<?>> getClasses() {
        Set<Class<?>> resources = new HashSet<Class<?>>();
        resources.add(Resource.class);
        resources.add(RuntimeExceptionMapper.class);
        resources.add(WebApplicationExceptionMapper.class);
        return resources;
    }
}

The javadoc for getClasses says:

Get a set of root resource, provider and feature classes. The default life-cycle for resource class instances is per-request. The default life-cycle for providers (registered directly or via a feature) is singleton.

And our (new unified) parameter container scanning respects this to exclude the bean params. I don't think bean params fall within the scope of "resource, provider or feature" so I think they should not be filtered out…

@geoand
Copy link
Contributor

geoand commented Aug 20, 2024

I don't think bean params fall within the scope of "resource, provider or feature" so I think they should not be filtered out

Agreed

@FroMage
Copy link
Member Author

FroMage commented Aug 20, 2024

Go back to PTO man ;)

I'm just thinking out loud.

Also unify the parameter container detection code paths
Make sure we do not turn record parameter containers into beans
For records, we store every contructor parameter in a local variable
until we have enough to call the constructor via a generated static
method.

Fixes quarkusio#19686
Copy link

quarkus-bot bot commented Aug 20, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 9206f88.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Aug 20, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9206f88.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@geoand geoand merged commit 4b8fcd3 into quarkusio:main Aug 26, 2024
49 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Aug 26, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Aug 26, 2024
@FroMage FroMage deleted the 19686 branch August 26, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Support constructor injection for RESTEasy Reactive multipart bodies
2 participants