You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "volosied (via GitHub)" <gi...@apache.org> on 2023/10/03 17:53:17 UTC
[PR] Spec Issue 1584 - Rely on CDI Class Scanning [myfaces]
volosied opened a new pull request, #615:
URL: https://github.com/apache/myfaces/pull/615
For `https://github.com/jakartaee/faces/issues/1584`
Since CDI scanning is default now, I removed the `USE_CDI_FOR_ANNOTATION_SCANNING` configuration.
Question: Should we still have fallbacks 2 & 3?
TODO: Create JIRA if one doesn't exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1756085115
As for removing the Annotation Provider, does the quarkus extension need it? I see there's a QuarkusAnnotationProvider.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec Issue 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1745622796
Alternatively, we keep the config and switch it to false for the failng `FacesConfiguratorDefaultValidatorsTestCase` tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1757000840
yep, i see
then we should move the CDI logic to DefaultAnnotationProvider and remove the old stuff
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1757968800
I can take care of it If the Mojarra Team give their +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1746303611
if the spec switches to to this behavior, we should completely remove the AnnotationProvider SPI IMO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on code in PR #615:
URL: https://github.com/apache/myfaces/pull/615#discussion_r1354298046
##########
impl/src/main/java/org/apache/myfaces/config/annotation/DefaultAnnotationProvider.java:
##########
@@ -128,21 +128,23 @@ public DefaultAnnotationProvider()
@Override
public Map<Class<? extends Annotation>, Set<Class<?>>> getAnnotatedClasses(ExternalContext ctx)
{
- if (MyfacesConfig.getCurrentInstance(ctx).isUseCdiForAnnotationScanning())
+ //1. Use CDI
+ BeanManager beanManager = CDIUtils.getBeanManager(ctx);
+ if(beanManager != null) // shouldnt be null, but needed for our mock junit tests
Review Comment:
yep
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1757845070
Could you be more specific -- I don't quite follow which parts you mean?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec Issue 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1745618978
Down to 6 failures:
```
[ERROR] Errors:
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testBeanValidationNotAvailable:213 » NullPointer
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testDefaultBeanValidatorDisabled:186 » NullPointer
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testDefaultBeanValidatorDisabledButPresentInFacesConfig:153 » NullPointer
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testDefaultValidatorsClearedByLatterConfigFileWithEmptyElement:251 » NullPointer
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testDefaultValidatorsNotClearedByLatterConfigFileWithNoElement:292 » NullPointer
[ERROR] FacesConfiguratorDefaultValidatorsTestCase.testDefaultValidatorsOverwrittenByLatterConfigFile:333 » NullPointer
```
I'm not sure how to enable CDI in the `AbstractJsfConfigurableMockTestCase` super class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faces 5.0: Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko commented on PR #615:
URL: https://github.com/apache/myfaces/pull/615#issuecomment-1798502155
i will close this for now and completly remove old scanning after we aligned on the spec change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Faces 5.0: Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "tandraschko (via GitHub)" <gi...@apache.org>.
tandraschko closed pull request #615: Faces 5.0: Spec 1584 - Rely on CDI Class Scanning
URL: https://github.com/apache/myfaces/pull/615
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spec 1584 - Rely on CDI Class Scanning [myfaces]
Posted by "volosied (via GitHub)" <gi...@apache.org>.
volosied commented on code in PR #615:
URL: https://github.com/apache/myfaces/pull/615#discussion_r1353137293
##########
impl/src/main/java/org/apache/myfaces/config/annotation/DefaultAnnotationProvider.java:
##########
@@ -128,21 +128,23 @@ public DefaultAnnotationProvider()
@Override
public Map<Class<? extends Annotation>, Set<Class<?>>> getAnnotatedClasses(ExternalContext ctx)
{
- if (MyfacesConfig.getCurrentInstance(ctx).isUseCdiForAnnotationScanning())
+ //1. Use CDI
+ BeanManager beanManager = CDIUtils.getBeanManager(ctx);
+ if(beanManager != null) // shouldnt be null, but needed for our mock junit tests
Review Comment:
@tandraschko Is it okay if I add a null check here? Otherwise, we'd have to handle the failing `mock` junit tests.
- skip them
- find some way to enable CDI ( looks tedious as we'll need to create lot of mock objects to replicate the CDIAnnotationDelegateInjectionProvider class)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@myfaces.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org