[
https://issues.apache.org/jira/browse/MYFACES-2730?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12881302#action_12881302
]
Leonardo Uribe commented on MYFACES-2730:
-----------------------------------------
Hi Jakob
@delete svn files: It is good to know there are different opinions about it,
but a discussion about that point only lead to a non sense "bizantine war". My
suggestion about do not delete svn files is more a personal programming
practice than a rule of thumb. According to Murphy's law, it is more often that
you delete code that you really need that the opposite. In this case, the svn
history of those files are important, as I'll show below.
Did you notice the part I said: "...MYFACES-2730-2.patch has an alternative I'm
working on...."? The code I sended is not complete on purpose, because what I
wanted is create a discussion about what is the community position and show an
alternate way to do it and possible advantages/disadvantages and
suggestions/tomatoes.
The reason I'm doing this issue is because I'm working to get a release of
myfaces core 2.0.1 out and the code committed here causes a problem, because I
can't start a release over a code that it is not tested. Please note I reverted
the code but I created a patch with the committed changes, to take advantage of
those changes and do not lose job. It is not possible to propose an alternative
and see the differences without revert the code.
I saw the stuff related to ServletExternalContextImplBase, and I still maintain
my position about it. That class only has sense if we want to override methods
in StartupServletExternalContextImpl to throw exceptions, but the patch I
sended does not suppose that, instead it tries to let as many code as possible
working.
Now, some comments related to the solution I'm working on. At first, this is
how FacesInitializer should looks like:
public interface FacesInitializer
{
void initFaces(ServletContext servletContext);
void destroyFaces(ServletContext servletContext);
FacesContext initStartupFacesContext(ServletContext servletContext);
void destroyStartupFacesContext(FacesContext facesContext);
FacesContext initShutdownFacesContext(ServletContext servletContext);
void destroyShutdownFacesContext(FacesContext facesContext);
}
We need to add methods that initialize and destroy the FacesContext instance
and implement them on AbstractFacesInitializer. In that way we provide a way to
override the startup FacesContext/ExternalContext/ExceptionHandler.
Let's remember the history of why we need a startup FacesContext, checking the
svn log. This is the list of related myfaces issues:
MYFACES-2358 System event system not working (to publish system events we need
Application object)
MYFACES-2434 dummy request/response classes for system event listeners will
break with Servlet 3.0 (note orchestra try to resolve EL Expressions on startup
time)
MYFACES-2520 UnsupportedOperationException when launching Trinidad 2 w/
MyFaces2 in Jetty (note the call to
ServletExternalContextImpl.getRequestContentType)
In few words, there is a chance that we end throwing exceptions on init time.
When the code lookup a Factory, that triggers the creation of objects in other
libraries that could think this is a real request. For example, the call to
ServletExternalContextImpl.getRequestContentType. It seems orchestra try to
resolve EL expressions, on FacesContext init, but with the new code we don't
have this problem anymore.
Note that StartupFacesContextImpl.setViewRoot() is called from init code, but
to be strict that method should throw an UnsupportedOperationException. We need
a ViewRoot to publish init/destroy events.
So, at this point we have three options:
- Throw UnsupportedOperationException "aggresively", and live with the
possibility of find myfaces bugs on xxx or yyy lib by that reason, that could
be prevented using dummy methods.
- Do not throw any UnsupportedOperationException, and instead implements dummy
methods.
- Just throw UnsupportedOperationException when definitively does not have
sense to use it, that means, methods that do something like
ExternalContext.dispatch() or ExternalContext.redirect(), but be nice and
return null, false or 0 on methods like ExternalContext.getRequestContentType()
and so on.
Note that based on previous tests done, RI uses dummy methods and do not throw
UnsupportedOperationException.
I'll check FacesContext and ExternalContext implementation to analyze in which
cases we should throw UnsupportedOperationException and propose a patch to be
discussed.
".....That's why I object committing any of your patches...."
I'll propose as many alternatives as I can imagine until find one that fits,
but I reserve myself the right of change of opinion based on proper
argumentation. My only interest is solve as many issues as possible.
best regards,
Leonardo Uribe
> FacesContext not available on application startup
> -------------------------------------------------
>
> Key: MYFACES-2730
> URL: https://issues.apache.org/jira/browse/MYFACES-2730
> Project: MyFaces Core
> Issue Type: Bug
> Components: JSR-127, JSR-252, JSR-314
> Affects Versions: 1.1.8, 1.2.9, 2.0.0
> Reporter: Nick Belaevski
> Assignee: Leonardo Uribe
> Fix For: 2.0.2-SNAPSHOT
>
> Attachments: MYFACES-2730-1.patch, MYFACES-2730-2.patch,
> MYFACES-2730-revert.patch
>
>
> If custom ResourceHandler calls FacesContext.getCurrentInstance() in
> constructor to read init parameters, null value is returned. This affects
> latest MyFaces 2.0.0-SNAPSHOT. Mojarra 2.0 provides InitFacesContext in this
> case.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
