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

Turn a class annotated with @Startup but no scope into a bean #7058

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Feb 7, 2020

- add @ApplicationScope automatically
- resolves quarkusio#7048
@boring-cyborg boring-cyborg bot added area/arc Issue related to ARC (dependency injection) area/documentation labels Feb 7, 2020
@mkouba mkouba added this to the 1.3.0 milestone Feb 7, 2020
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.

Weird, I tried this and it didn't work for me. I must have made some subtle mistake :)

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.

Ah, I see you beat me to this PR. Was planning on sending it but got distracted by other issues...
I left one comment, otherwise it looks good as there is no black magic in this PR :)


@Override
public void transform(TransformationContext context) {
if (context.isClass() && !BuiltinScope.isDeclaredOn(context.getTarget().asClass())) {
Copy link
Contributor

@manovotn manovotn Feb 7, 2020

Choose a reason for hiding this comment

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

I know it's somewhat far-fetched but should we care about non-built in scopes? Users can have custom ones, or there is Transactional one already present. Both cases will pass this check and try to add yet another scope on the bean (which should later on blow up).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. Although in this case I doubt that a @Startup bean would be annotated with @Transactional or a custom scope. Let's create a separate issue to handle this corretly. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, the problem is that annotation transformers are registered before the final set of scopes is known...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created #7076

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) area/documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow the standalone use of @Startup
3 participants