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

Bug Report: CDI tries to instantiate interface (Jersey 2.30) #4847

Merged
merged 2 commits into from
Sep 14, 2021

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Sep 3, 2021

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@senivam
Copy link
Contributor

senivam commented Sep 6, 2021

isn't it better to put FINE/FINER level output?

@jbescos
Copy link
Member Author

jbescos commented Sep 6, 2021

isn't it better to put FINE/FINER level output?

I thought it is too much to move from WARNING to FINE, but I am okay doing that.

Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@senivam senivam left a comment

Choose a reason for hiding this comment

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

actually 1 change is required:

the main IF here checks if resource is acceptable. Whis includes:

 ((c.getModifiers() & Modifier.ABSTRACT) != 0
                         || c.isPrimitive()
                         || c.isAnnotation()
                         || c.isInterface()
                         || c.isLocalClass()
                         || (c.isMemberClass() && (c.getModifiers() & Modifier.STATIC) == 0));

however as per the issue description this warning output is not applicable only to interface.
So, it would be appropriate to write

LOGGER.log( clazz.isInterface() ? Level.FINE : Level.WARNING,
                    LocalizationMessages.CDI_NON_INSTANTIABLE_COMPONENT(clazz));

@jbescos
Copy link
Member Author

jbescos commented Sep 7, 2021

actually 1 change is required:

the main IF here checks if resource is acceptable. Whis includes:

 ((c.getModifiers() & Modifier.ABSTRACT) != 0
                         || c.isPrimitive()
                         || c.isAnnotation()
                         || c.isInterface()
                         || c.isLocalClass()
                         || (c.isMemberClass() && (c.getModifiers() & Modifier.STATIC) == 0));

however as per the issue description this warning output is not applicable only to interface.
So, it would be appropriate to write

LOGGER.log( clazz.isInterface() ? Level.FINE : Level.WARNING,
                    LocalizationMessages.CDI_NON_INSTANTIABLE_COMPONENT(clazz));

Correct, nice catch.

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@senivam senivam merged commit e5b7905 into eclipse-ee4j:master Sep 14, 2021
@senivam senivam added this to the 2.36 milestone Sep 14, 2021
@mkarg
Copy link
Member

mkarg commented Oct 11, 2021

Thanks a lot for having this fixed! 👍 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants