Mailing-List: contact dev-help@myfaces.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "MyFaces Development" <dev@myfaces.apache.org>
Date: Thu, 19 May 2016 03:01:12 +0000 (UTC)
From: "Leonardo Uribe (JIRA)" <dev@myfaces.apache.org>
To: dev@myfaces.apache.org
Message-ID: <JIRA.12970776.1463580906000.231775.1463626872888@Atlassian.JIRA>
In-Reply-To: <JIRA.12970776.1463580906000@Atlassian.JIRA>
References: <JIRA.12970776.1463580906000@Atlassian.JIRA> <JIRA.12970776.1463580906837@arcas>
Subject: [jira] [Commented] (MYFACES-4047) @PreDestroy method not invokved
 on javax.faces.bean.ViewScoped beans on Session invalidation
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Thu, 19 May 2016 03:01:25 -0000


    [ 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)