Skip to content

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 manovotn and mkouba 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
@quarkus-bot

This comment has been minimized.

@quarkus-bot
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