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

ArC: add validation of synthetic bean types #46794

Merged
merged 1 commit into from
Mar 17, 2025

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Mar 13, 2025

So far, bean types of synthetic beans were not validated, so it was possible to create synthetic beans with illegal bean types. This commit fixes that. If a synthetic bean has an illegal type, definition error occurs.

Fixes #46771

So far, bean types of synthetic beans were not validated, so it was possible
to create synthetic beans with illegal bean types. This commit fixes that.
If a synthetic bean has an illegal type, definition error occurs.
@Ladicek Ladicek added the area/arc Issue related to ARC (dependency injection) label Mar 13, 2025
@Ladicek Ladicek requested review from mkouba and manovotn March 13, 2025 12:10
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 13, 2025

One question I have is: in case of declared beans, illegal bean types don't cause a definition error, they are just removed from the set of bean types. It seems to me that in case of synthetic beans, it is better to fail immediately, because the types are not discovered by the container, they are added by the user, so they probably want to know they're doing something wrong. But it is not consistent with other kinds of beans.

@mkouba
Copy link
Contributor

mkouba commented Mar 13, 2025

One question I have is: in case of declared beans, illegal bean types don't cause a definition error, they are just removed from the set of bean types. It seems to me that in case of synthetic beans, it is better to fail immediately, because the types are not discovered by the container, they are added by the user, so they probably want to know they're doing something wrong.

I do agree.

But it is not consistent with other kinds of beans.

It's not the only inconsistency between synthetic and regular beans 🤷.

@mkouba mkouba added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport labels Mar 13, 2025

This comment has been minimized.

Copy link

quarkus-bot bot commented Mar 17, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 37e8e28.

✅ 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.

@geoand geoand merged commit 24804a3 into quarkusio:main Mar 17, 2025
59 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 17, 2025
@quarkus-bot quarkus-bot bot added this to the 3.22 - main milestone Mar 17, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Mar 17, 2025
Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

It seems to me that in case of synthetic beans, it is better to fail immediately, because the types are not discovered by the container, they are added by the user, so they probably want to know they're doing something wrong.

Definitely agree, thanks for the PR and tests 👍

@Ladicek Ladicek deleted the arc-validate-synth-bean-types branch March 18, 2025 15:00
@gsmet gsmet modified the milestones: 3.22 - main, 3.21.0 Mar 18, 2025
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/enhancement New feature or request triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArC: validate bean types added to a synthetic bean
5 participants