myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Leonardo Uribe (JIRA)" <...@myfaces.apache.org>
Subject [jira] [Commented] (MYFACES-4047) @PreDestroy method not invokved on javax.faces.bean.ViewScoped beans on Session invalidation
Date Thu, 19 May 2016 03:01:12 GMT

    [ https://issues.apache.org/jira/browse/MYFACES-4047?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15290362#comment-15290362
] 

Leonardo Uribe commented on MYFACES-4047:
-----------------------------------------

The patch has some observations we need to discuss. Let me tell you the whole story, I know
it could be repetitive somehow but it is important to understand how solve this properly.

In JSF 2.0, javax.faces.event.PreDestroyViewMapEvent was added with this description in the
javadoc:

"... This event must be published by a call to Application.publishEvent(javax.faces.context.FacesContext,
java.lang.Class, java.lang.Object) when the clear method is called on the map returned from
UIViewRoot.getViewMap(). ..."

The idea in that time, was to provide an event that can be used to handle @PreDestroy annotations
for view scope. 

But the implementation was not perfect. JSF 2.0 spec section 5.4.1 says "... in the case of
a managed bean placed in view scope, methods annotated with @PreDestroy must only be called
when the view scope is destroyed. See the javadoc for FacesContext.setViewRoot(). ..."

The key point is FacesContext.setViewRoot(...) is usually called from NavigationHandler.handleNavigation(...),
mostly because this is the point where you can be 100% sure the view is discarded. So, deep
inside setViewRoot(...) there is a call that invokes viewMap.clear() and the implementation
of ViewMap (in that time it was located in UIViewRoot) publish PreDestroyViewMapEvent and
since there is a listener (ManagedBeanDestroyer) subscribed to that event, the managed beans
on view scope were destroyed.

But in JSF 2.2 things changed, because it was specified. The related issue is:

https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-905

But in JSF 2.2 section 5.4 there is a line that says: "... The annotations in the package
javax.faces.bean will be deprecated in a version of the JSF specification after 2.2. Therefore,
developers are strongly recommended avoid using those annotations and instead use the ones
from Java EE 6. ..."

So, in MyFaces the code was let as it was in JSF 2.0/2.1 and focus on CDI implementation of
view scope, so PreDestroy works there on session expiration.

But looking now on some comments in JAVASERVERFACES_SPEC_PUBLIC-905, it looks the behavior
was implemented in the RI for both cases.

Now, the problem with the patch proposed is that it does not follow the convention, publish
PreDestroyViewMapEvent and then let the listener attached to that event to their job. Instead,
it calls directly the listener. 

There is a class called org.apache.myfaces.view.ViewScopeProxyMap that works as a "middle
man" class that allows to switch between implementations (default or CDI). There you can see
this lines on clear() implementation:

{code:java}
        FacesContext facesContext = FacesContext.getCurrentInstance();
        facesContext.getApplication().publishEvent(facesContext, 
                PreDestroyViewMapEvent.class, facesContext.getViewRoot());
{code}

Instead call LifecycleProvider.destroyInstance(...), try use this call. The problem is onSessionDestroyed()
could be called from locations where there is no valid FacesContext instance (ex: session
invalidation), so in this case, if there is no FacesContext instance, you need to create a
custom context in the same way as in ViewScopeBeanHolder is done (but it is a bit tricky because
you need a reference to servletContext, and the problem is how to get it, maybe pass it as
a param from DefaultViewScopeProviderFactory).

> @PreDestroy method not invokved on javax.faces.bean.ViewScoped beans on Session invalidation
> --------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-4047
>                 URL: https://issues.apache.org/jira/browse/MYFACES-4047
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-344
>    Affects Versions: 2.2.10
>            Reporter: Paul Nicolucci
>            Priority: Minor
>         Attachments: myfaces-4047.patch
>
>
> Consider the following scenario:
> Bean1:
> @Named
> @ViewScoped (javax.faces.view.ViewScoped)
> Bean2:
> @ManagedBean
> @ViewScoped (javax.faces.bean.ViewScoped)
> When the session is invalidated the org.apache.myfaces.cdi.impl.CDIManagedBeanHandlerImpl.onSessionDestroyed()
method is invoked. Here, MyFaces destroys all of the javax.faces.view.ViewScoped @Named beans
and @PreDestroy is invoked on these beans. However, the @ManagedBean /ViewScoped bean does
not have its @PreDestroy invoked.
> I believe this is because the assumption was made that if an Application is CDI enabled
it will contain all CDI @Named beans and not any @ManagedBeans. There is also the DefaultViewScopeHandler
which is not yet implemented for "onSessionDestroyed" that would be used in the case when
CDI is not enabled, that is when only @ManagedBeans are contained in the application.
> I believe it is possible to update the CDIManagedBeanHandlerImpl to be aware of the ViewScoped
ManagedBeans and destroy them onSessionDestroy first by just getting the viewMap and iterating
over the entries as we do when the PreDestroyViewMapEvent is processed.
> I'd like to use this issue to resolve the problem in CDIManagedBeanHandlerImpl as well
as implement onSessionDestroy in DefaultViewScopeHandler.java



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message