Re: broken CDI handling in MyFacesContainerInitializer

2018-04-13 Thread Mark Struberg
It seems this only happens with MaFaces-2.2.x and is fixed in 2.3.0!
I'll update the ticket to reflect this info.Do we want to backport this change 
or just move forward?
LieGrue,strub
 

On Friday, 13 April 2018, 17:17:17 CEST, Mark Struberg  
wrote:  
 
 Hi Leo!

The point is that we do bean initialisation in FacesInitializer. But we 
actually should do it in the CDI Extensions.
When we startup the FacesInitializer it is _not_ guaranteed that the CDI 
container is already started!
And even we get our hand at the BeanManager then this is not a guarantee that 
the BeanManager is fully booted.
In any case we must not invoke getBeans() etc on this BeanManager before 
AfterDeploymentValidation got fired.

I gonna fix that. It is already working fine locally but I want to do some 
further test and also let it run in TomEE to ensure I didn't trash anything.

LieGrue,
strub


> Am 13.04.2018 um 16:21 schrieb Leonardo Uribe :
> 
> Hi
> 
> It looks like a chicken-egg problem. But if CDI is present, it should run 
> before MyFaces, so BeanManager should be available on MyFaces startup, never 
> the opposite. After all, it is the bean container and the code was not 
> designed for the opposite. I don't know if something changed. 
> 
> As far as I can remember there is no CDI initialization in the startup, but 
> the BeanManager reference is required at that point since after that part, 
> other features are enabled/disabled based on the reference.
> 
> But in 2.3.0 MyFaces is no longer able to run without CDI (deprecation of 
> Managed Beans). 
> 
> In my opinion the container should set the ordering: first CDI then MyFaces, 
> not the opposite. It doesn't look like something to be solved in MyFaces side.
> 
> regards,
> 
> Leonardo Uribe
> 
> 2018-04-13 3:04 GMT-05:00 Thomas Andraschko :
> Puh, stupid problem
> 
> I think we must move the "CDI-init stuff" into a extension, but the leave 
> everything else as it is as MF can still be used without CDI.
> As the ServletContextInitializer runs before the CDI Extensions, the 
> StartupFacesContext could be available, also in the extension?
> 
> 
> 
> 2018-04-13 9:54 GMT+02:00 Mark Struberg :
> Hi folks!
> 
> I've figured that we blow up pretty nasty when using latest MyFaces on Tomcat 
> with any CDI container (OWB or Weld).
> That's because you must not use BeanManager#getBeans before 
> AfterDeplyomentValidation gets fired. 
> 
> I think the whole handling should ONLY be done via a CDI Extension!
> In which way it will automatically get picked up and will initialise Flows 
> perfectly fine.
> 
> The only problem to solve is how to make the FacesContext available from 
> within the CDI Extension.
> The ticket is tracked as MYFACES-4224.
> Feedback welcome.
> 
> 
> LieGrue,
> strub
> 
> 
  

Re: broken CDI handling in MyFacesContainerInitializer

2018-04-13 Thread Mark Struberg
Hi Leo!

The point is that we do bean initialisation in FacesInitializer. But we 
actually should do it in the CDI Extensions.
When we startup the FacesInitializer it is _not_ guaranteed that the CDI 
container is already started!
And even we get our hand at the BeanManager then this is not a guarantee that 
the BeanManager is fully booted.
In any case we must not invoke getBeans() etc on this BeanManager before 
AfterDeploymentValidation got fired.

I gonna fix that. It is already working fine locally but I want to do some 
further test and also let it run in TomEE to ensure I didn't trash anything.

LieGrue,
strub


> Am 13.04.2018 um 16:21 schrieb Leonardo Uribe :
> 
> Hi
> 
> It looks like a chicken-egg problem. But if CDI is present, it should run 
> before MyFaces, so BeanManager should be available on MyFaces startup, never 
> the opposite. After all, it is the bean container and the code was not 
> designed for the opposite. I don't know if something changed. 
> 
> As far as I can remember there is no CDI initialization in the startup, but 
> the BeanManager reference is required at that point since after that part, 
> other features are enabled/disabled based on the reference.
> 
> But in 2.3.0 MyFaces is no longer able to run without CDI (deprecation of 
> Managed Beans). 
> 
> In my opinion the container should set the ordering: first CDI then MyFaces, 
> not the opposite. It doesn't look like something to be solved in MyFaces side.
> 
> regards,
> 
> Leonardo Uribe
> 
> 2018-04-13 3:04 GMT-05:00 Thomas Andraschko :
> Puh, stupid problem
> 
> I think we must move the "CDI-init stuff" into a extension, but the leave 
> everything else as it is as MF can still be used without CDI.
> As the ServletContextInitializer runs before the CDI Extensions, the 
> StartupFacesContext could be available, also in the extension?
> 
> 
> 
> 2018-04-13 9:54 GMT+02:00 Mark Struberg :
> Hi folks!
> 
> I've figured that we blow up pretty nasty when using latest MyFaces on Tomcat 
> with any CDI container (OWB or Weld).
> That's because you must not use BeanManager#getBeans before 
> AfterDeplyomentValidation gets fired. 
> 
> I think the whole handling should ONLY be done via a CDI Extension!
> In which way it will automatically get picked up and will initialise Flows 
> perfectly fine.
> 
> The only problem to solve is how to make the FacesContext available from 
> within the CDI Extension.
> The ticket is tracked as MYFACES-4224.
> Feedback welcome.
> 
> 
> LieGrue,
> strub
> 
> 



Re: broken CDI handling in MyFacesContainerInitializer

2018-04-13 Thread Leonardo Uribe
Hi

It looks like a chicken-egg problem. But if CDI is present, it should run
before MyFaces, so BeanManager should be available on MyFaces startup,
never the opposite. After all, it is the bean container and the code was
not designed for the opposite. I don't know if something changed.

As far as I can remember there is no CDI initialization in the startup, but
the BeanManager reference is required at that point since after that part,
other features are enabled/disabled based on the reference.

But in 2.3.0 MyFaces is no longer able to run without CDI (deprecation of
Managed Beans).

In my opinion the container should set the ordering: first CDI then
MyFaces, not the opposite. It doesn't look like something to be solved in
MyFaces side.

regards,

Leonardo Uribe

2018-04-13 3:04 GMT-05:00 Thomas Andraschko :

> Puh, stupid problem
>
> I think we must move the "CDI-init stuff" into a extension, but the leave
> everything else as it is as MF can still be used without CDI.
> As the ServletContextInitializer runs before the CDI Extensions, the
> StartupFacesContext could be available, also in the extension?
>
>
>
> 2018-04-13 9:54 GMT+02:00 Mark Struberg :
>
>> Hi folks!
>>
>> I've figured that we blow up pretty nasty when using latest MyFaces on
>> Tomcat with any CDI container (OWB or Weld).
>> That's because you must not use BeanManager#getBeans before
>> AfterDeplyomentValidation gets fired.
>>
>> I think the whole handling should ONLY be done via a CDI Extension!
>> In which way it will automatically get picked up and will initialise
>> Flows perfectly fine.
>>
>> The only problem to solve is how to make the FacesContext available from
>> within the CDI Extension.
>> The ticket is tracked as MYFACES-4224.
>> Feedback welcome.
>>
>>
>> LieGrue,
>> strub
>
>
>


Re: broken CDI handling in MyFacesContainerInitializer

2018-04-13 Thread Thomas Andraschko
Puh, stupid problem

I think we must move the "CDI-init stuff" into a extension, but the leave
everything else as it is as MF can still be used without CDI.
As the ServletContextInitializer runs before the CDI Extensions, the
StartupFacesContext could be available, also in the extension?



2018-04-13 9:54 GMT+02:00 Mark Struberg :

> Hi folks!
>
> I've figured that we blow up pretty nasty when using latest MyFaces on
> Tomcat with any CDI container (OWB or Weld).
> That's because you must not use BeanManager#getBeans before
> AfterDeplyomentValidation gets fired.
>
> I think the whole handling should ONLY be done via a CDI Extension!
> In which way it will automatically get picked up and will initialise Flows
> perfectly fine.
>
> The only problem to solve is how to make the FacesContext available from
> within the CDI Extension.
> The ticket is tracked as MYFACES-4224.
> Feedback welcome.
>
>
> LieGrue,
> strub