Hmm, I forgot something until I went to implement this approach. . .
It will be nearly impossible to avoid accessing the
TilesApplicationContext in order to create the TilesRequestContext
unless we change a couple of things. The reason is that we currently
provide access to factories through the TilesUtil - which in turn looks
them up in the TilesApplicationContext (previously TilesContext).
This allows the factory instantiation and configuration to be hidden
from the client (external application and/or tag) and done only once for
the life of the application.
In other words, if you'd like to create the TilesRequestContext from the
factory directly, you'll have to do this:
TilesApplicationContext ctx = . . .
TilesRequestContext requestCtx = TilesUtil.createRequestContext(ctx,
req, res);
or this:
TilesApplicationContext ctx = . . .
TilesContextFactory factory = TilesUtil.getContextFactory(ctx);
TilesRequestContext requestCtx = factory.createRequestContext(ctx, req,
res);
One way around this is to cache the TilesApplicationContext within the
TilesUtilImpl. This would allow must simpler invocations such as:
TilesRequestContext context = TilesUtil.createRequestContext(req, res)
The one negative to this approach is that it will eliminate the ability
to support multiple contexts (when tiles is packaged in a common
classloader). The TilesUtil currently appears to be implemented in a
way which suggests that the original intent was to support multiple
contexts. That said, the support is already only partial since Tiles
utilizes several static accessors and instances such as TilesUtilImpl
will be shared across applications.
The second approach that would solve this issue would be to refactor the
codebase to eliminate the prevalent use of static methods. Instead, all
tiles functionality could be configured and encapsulated into a self
contained "container" which would be cached and retrieved when needed.
In this scenario, the configuration servlet, filter, or listener, would
create the container and provide access to it from a publicly available
place (perhaps the underlying context). Whenever tiles were needed, the
client would retrieve the container and invoke it. Services like the
TilesUtil would be provided by the container, not statically accessed.
I think this latter approach provides the best of both worlds and IMHO
will make things a little simpler. I think it will also give us a
little more flexibility down the road as well.
Interested in your thoughts . . .perhaps there's something here that I'm
missing!
David
David H. DeWolf wrote:
Greg Reddin wrote:
On Oct 18, 2006, at 6:52 AM, David H. DeWolf wrote:
1) TilesContext is refactored into two different contexts -
TilesContext and TilesRequestContext. Only one instance of the
TilesContext should exist within the application and it will be used
to provide application scoped functions (e.g. getResource() or
getInitParameters()) . The TilesRequestContext will provide
request/response specific operations (e.g. include() or
getParameters()).
At first I didn't like the separation, but now I do after looking at
your patch. It makes sense that you would put request-based stuff
into a separate class from application-scoped stuff. I have two small
concerns:
1) The name TilesContext: Perhaps we should change it to
TilesApplicationContext.
Makes sense to me. I'll rename it.
2) How will Tiles be used from other frameworks? Here's an example
from Shale. Shale provides a TilesViewHandler with the following code
(the code is similar to what is used in other frameworks like
Struts2's TilesResult class):
ExternalContext externalContext = FacesContext.getCurrentInstance()
.getExternalContext();
Object request = externalContext.getRequest();
Object context = externalContext.getContext();
ComponentDefinition tile = null;
try {
TilesContext tilesContext =
TilesContextFactory.getInstance(context, request);
tile = TilesUtil.getDefinition(name, tilesContext);
} catch (NoSuchDefinitionException nsex) {
log.error("Couldn't find Tiles definition.", nsex);
} catch (DefinitionsFactoryException dex) {
log.error("Tiles error", dex);
}
return tile;
After your refactoring the code above will look like this:
ExternalContext externalContext = FacesContext.getCurrentInstance()
.getExternalContext();
Object request = externalContext.getRequest();
Object context = externalContext.getContext();
ComponentDefinition tile = null;
try {
TilesContextFactory contextFactory = new
BasicTilesContextFactory();
TilesContext tilesContext =
contextFactory.getInstance(context);
TilesRequestContext tilesRequestContext =
tilesContext.createRequestContext(request, response);
tile = TilesUtil.getDefinition(name, tilesRequestContext);
} catch (NoSuchDefinitionException nsex) {
log.error("Couldn't find Tiles definition.", nsex);
} catch (DefinitionsFactoryException dex) {
log.error("Tiles error", dex);
}
return tile;
The only reason I have to create a TilesContext above is because I
need a TilesRequestContext. What if the factory provided methods to
create either the TilesApplicationContext or the TilesRequestContext
instead of the TilesContext being required to create the
TilesRequestContext?
I'm fine with that. My original thought was that since the request
context has to be retrieved from various locations in the app (tags), it
may be easier to avoid propagating the availability of the factory by
providing access through the application context. The ramification of
this is that it's a little awkward in the way you described.
After looking through stuff again, I think I agree with you. I'll go
ahead and provide access to the factory through the TilesUtil. The
implementation will be modeled after the UrlDefinitionsFactory. This
consistency itself is enough of an argument for this approach.
Thanks for the great feedback. . .
David
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
