incubator-adffaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam Winer" <awi...@gmail.com>
Subject Re: Re: [PORTAL] Changes and API review?
Date Fri, 15 Dec 2006 20:46:07 GMT
On 12/15/06, Scott O'Bryan <darkarena@gmail.com> wrote:
> Further investigation:
>
> PortletExternalContext can be removed.  There is a possibility that it
> will need to be added again for JSR-301 but that depends on interfaces
> which aren't defined yet so I'm cool on getting rid of it till we need it.
>
> The ServletExternalContext is used in the ResourceServlet (which is in
> the API package) and the TrinidadFilterImpl (which is in the Impl
> package).  Should I do some research on how to move this to the Impl
> package and still allow it to be called from ResourceServlet?  I know
> we're still discussing the need to use a custom FacesContext, but if it
> turns out we should keep it, we're going to need to do something for the
> ExternalContext.
>
> One more point about the TrinidadFacesContext.  It replaces the
> PseudoFacesContext that is already in Trinidad.  The PseudoFacesContext
> didn't implement the base FacesContext stuff, but most of the external
> context was already handled in that class.  The TrinidadFacesContext is
> an enhanced version of the PseudoFacesContext only it's not as hokey
> because it's a full impl instead of a partial impl AND it can be used
> from the resources.

PseudoFacesContext served only one purpose - provide enough of
a bootstrap for FacesContext so that we could have one *before*
the real FacesContext got instantiated.  It's also not a public API.

-- Adam

>
> Finally, I looked and there is no reason that ExternalContextUtils
> cannot be put into the impl package.  It's mostly used for setup of the
> RequestContext and some other logic which happens outside of the Faces
> scope.  So good call.
>
> Scott
>
> Scott O'Bryan wrote:
> >
> >> All of the classes in webapp/wrappers and context/external:
> >> - Why aren't these in impl?
> >> - I don't understand at all why we could or should be implementing
> >>   ServletExternalContext...  that's provided by the impl.  And
> >>   PortletExternalContext should be provided by the bridge,
> >>   or the impl as well if it supports portlets.  What am I missing?
> >>   I suspect these come from adding TrinidadFacesContext, so...
> > These are valid questions and I went back and forth on this myself.
> > The main issue is that the Configurators rely on someone being able to
> > override the ExternalContext.  And while a decorator may be sufficient
> > for the overriding part, it's kind of silly (in my opinion) to force
> > everyone to re-implement the pieces of the external context (such as,
> > say, the RequestParameterMap) which is why those are public.  As for
> > the ServletExternalContext and the PortletExternalContext, if you look
> > at the API for configurators again, they require us to supply an
> > external context.  In order to maintain compatibility with the servlet
> > usecases that previous versions of Trinidad supported, we essentially
> > need to construct a valid ExternalContext within the filter (Sevlet or
> > future Portlet) and supply it to the configurators.  It's better to
> > provide implementations of these rather then rely on having to make
> > them yourself.
> >
> > Now that being said, I moved the external context classes over to
> > public as a while.  While I believe we need the decorators and some of
> > the map objects public, we can probably move the full ExternalContext
> > implementations in impl (or in the case of the Portal one which was
> > provided for completeness, remove it since I don't think it's used
> > anywhere currently (I'll check).
> >> - TrinidadFacesContext:  why can't you just use the regular
> >>   FacesContextFactory, as we're doing today?  Almost any
> >>   solution is better than duplicating large amounts of impl code.
> > This is a very good question and one that I thought a lot about.
> > First off, this class is used within the resource servlets.  Faces
> > itself is designed with the idea of allowing renderkits to extend the
> > framework as they need to.  In theory (and I know the reality is
> > somewhat different), but someone could add two renderkits to a
> > particular web-app and use them.  The problem is that the
> > FacesContextFactory takes all of these entensions into account when
> > returning the context.  From a renderkit perspective, this is good
> > because you hopefully have any functionality that your renderkit
> > provides, somewhere in the Chain of FacesContexts.  The problem,
> > however, is that our Resource servlets belong to Trinidad only.  This
> > means that there is no reason for us to have to go through all the
> > external-context wrapping that other renderkits might tack on through
> > their own custom factories.  This adds bloat when there really doesn't
> > need to be.
> >
> > From a Portal perspectivethis is even more important.  Trinidad, up
> > until this project, has been able to limit exposure to the "wrapping"
> > problem by relying on the fact that the filters are run only before
> > and after a call to the "faces servlet" essentially.  Being that there
> > is no equivalent in the Portal environment, modifications have had to
> > be made to allow Configurators to be run through a custom
> > FacesContextFactory.  This means that even within our own renderkit,
> > things like skinning, file upload handling, and RequestContext
> > initialization (as well as the running of additional services) all
> > happen when these external resources retrieve the FacesContext from
> > the factory.
> > As far as I'm aware, there is no sure-fire way in Faces to say, "give
> > me a FacesContext, but ignore all other FacesContext wrappers from
> > everyone else".  If there was a way to get the "default" FacesContext
> > then I would agree with you.
> >>
> >> ExternalContextUtils:
> >> -  To what extent does this really need to be in API?
> >> -  In particular, I'd rather not expose any of the methods
> >>  that are getting added to ExternalContext in 1.2:
> >>    - getRequestCharacterEncoding()
> >>    - getRequestContentType()
> >>  ... but in general, I'd rather not expose anything here
> >>  as a public API unless absolutely necessary.
> > LOL Yeah, let me check what the exposure is.  When I wrote the
> > utility, I was using it from something inside of the API package, but
> > I think that this can be solved by using RequestContext.  Everything
> > else that needs to use this should be inside impl.  I'll take a look
> > at why it's still public and get back to you.
> >> - A Coding surprise:  you may not call
> >>   request.getClass().getMethod().  Doesn't work, sadly, because
> >>   the defining class might be package-private.  You have to
> >>   get the API directly from ServletRequest.class, PortletRequest.class,
> >>   etc.
> > Good point.  Since I'm going to have to so a series of instanceof's
> > however, I may as well just cast them.  I was hoping to make this
> > faster rather then bogging the system down with casts.  But in light
> > of this I suppose I'll just have to bite the bullet and do it.  Thanks
> > for taking a look at this Adam.  I look forward to your reply.
> >
> > Scott
> >
>
>

Mime
View raw message