2011/10/24 Nicolas LE BAS <[email protected]> > I've been playing a bit with the Tiles Request API (awaiting github :))
It seems that the Infra team made a mistake in synchronizing SVN to GIT (they put only a branch, strange) so you have to wait a bit more. > * getApplicationContext is only used as way to access the Application > scope. I'm afraid there may be two solutions to a single purpose here. I > understand why Renderers need to know the ApplicationContext, but perhaps it > should be provided to them as separate object during initialization, and the > application scope should be accessible only through the Request interface to > avoid confusion. > So you want to remove ApplicationContext.getApplicationScope()? If yes notice that this way, all the objects that need a connection to the application scope won't be able to do it at startup-time, because they need a request. Sincerely I am -1 for it. > * isResponseCommitted, dispatch and include: those are a pain to implement. > These concepts are unrelated to the request we are currently processing: > they belong to the ApplicationContext (like getRequestDispatcher belongs to > the ServletContext). Besides, I'm not sure why we need "dispatch" and > "isResponseCommitted" from a theoretical point of view. Forwarding a request > is a controller behaviour, not something a renderer should attempt. It's > only used by Tiles as an optimization of "include" for the servlet > environment, and optimizations should not be exposed in the API imho. > I think that this doubt is due to the fact that you are changing the scope of the Request API. What I had in mind is to abstract requests in requests-based technologies (Servlets and Portlets) and have a common API with related view technologies. Probably the right thing to do is to create an extended interface so to have: * a base Request containing context attributes and a bunch of more methods (you're doing a great analysis so I'll leave it to you :-D ) * an extended Request for Java EE requests. This way, specific requests for Velocity+Servlets and Freemarker+Servlets should wrap an instance of Request of the second type. WDYT? > * setContentType is a controller concept, a renderer should not attempt to > change the content type. Besides, Tiles is not using it. > I agree, but anyway I will move it to the request of the second type, see above. > * getOutputStream would be useful if we wanted to render binary data, but > none of our current technologies actually support it. Perhaps a future > addition? > This was just to have a resemblance to Servlet/Portlet response. To move to the request of the second type, IMHO. > * getRequestObjects seems only here for historical purpose; it is not used. > +1 to remove it. > > The response headers are missing, too (except for ContentType), and that's > a good thing, too. They're a concern of the controller, not the renderer, > and because they're not there, the API helps to avoid mistakes. > Well, no. The Request.getHeader* methods return a map that can write to response headers. See this class (setValue method): http://svn.eu.apache.org/repos/asf/tiles/framework/trunk/tiles-request/tiles-request-servlet/src/main/java/org/apache/tiles/request/servlet/extractor/HeaderExtractor.java Overall it's a good API and we can do much by extending Tiles 3 that we > could not do with Tiles 2. IMHO it still needs improvement, but perhaps that > can be postponed for a while. > It's never been released yet so you can still play with it without fear of breaking anything. It's just my opinion anyway, but I hope this can help in improving tiles > further. > Absolutely! Thanks Antonio
