> > Where is the patch? for my part it's a lot easier to join the discussion > > when the code changes in question are also readable... > > > You're right, I should have told. It's sitting here: > https://github.com/nlebas/tiles/tree/request-api
Right. still getting use to that. Give me a chance to moderate it... I'm still a newbie on git, but is it possible for you to collapse these commits regarding this discussion into one? (...or collapse them so to meet the separate points in this discussion if there's any likelihood they'll end up as separate commits). Also i notice that some of the diffs are awkward to read because of whitespace changes on every line. I've also noticed you've removed the apache license header from some files... eg https://github.com/nlebas/tiles/commit/78f6723a36cbfe607befd8621e9dc9354a954088#tiles-request/tiles-request-api/src/main/java/org/apache/tiles/request/Request.java > > Is "WebRequest" the correct name? Servlet and Portlet technologies (and > > the java ee stack) indicates web but couldn't the base request be used > > for other web requests that were simple (but not java ee), > > I'm not sure I understand your point, and I'm not sure if I've been > clear either. Providing the code should help explaining myself. Would > you prefer "JavaEERequest"? I'm thinking along the lines: is the addition of these methods dispatch(..) include(..) setContentType(..) a declaration in itself of a "web" technology being used...? I think this is well elaborated by your previous discussion with Antonio... > > Back to the difficult point, summing up (correct me if I forget something): > > > > I think the dispatch/include methods should be moved from Request to > > ApplicationContext because the decision of what processes a Request should > > be independant of the Request itself. > > You think they should stay in the Request because they should act > > consistently based on the history of the Request (and the underlying > > HttpServletRequest). > > > > But why can't we have both? I see no technical problem with it. > > And if Request.getApplicationContext(**) stays, we can even have the > > Request as a single entry point. In that discussion you were asking this functionality be move out to specific DispatchRenderers (invalidating the need for an extended request interface). But a two-solution approach seems a better idea all round. That conclusion i think actually sets the context for naming. This gives us a choice between names like IncludingRequest or DispatchingRequest, depending on whether "dispatch" or "include" is the implicit behaviour in the original Request interface? Since Tiles is a composition framework i would think that "include" is the implicit and "dispatch" is the extension that introduces the choice between forward and include. So my preference is the name DispatchRequest which would also mirror DispatcherRenderer where you were at one point proposing the functionality to go to... > >> * getHeader split into two maps, one immutable as documented, the other > >> write-only for response headers. > > The purpose of the Map interface is to access the headers easily from > expression languages (${request.header['User-Agent']}). But of course a > read-only map is enough for that purpose; ELs don't need to access the > response. Then i'd rather have the method: Addable<String> getResponseHeaders(); The concept of a write-only map seems odd to me. At the same time should we change Map<String,String> getHeaders(); to ReadOnlyEnumerationMap<String> getHeaders(); ? > >> * I'd like to add a method "checkNotModified" to the Request interface. > >> It would take an timestamp as a parameter, repesenting the actual last > >> modification time of the model. > > And that's exactly the situation I'm addressing. Then i'm not understanding. When and where would request.checkNotModified(timestamp) be called? What would be the implementation of it in ServletRequest? > Now if you feel this has no place in tiles-request, I'm ok with it :) No that's not my angle at all. It's more a desire to spa a little more before pushing changes through. i think that process is as important for us (and those developers to follow) as the final commits themselves. if that makes sense... > * After implementing and using a number of renderers, I find the method > "isRenderable" so useful it should be mandatory. Besides, it's really > easy to implement. I'd like to merge TypeDetectingRenderer into Renderer. I'm good for that. ~mck -- "Don't use Outlook. Outlook is really just a security hole with a small e-mail client attached to it." Brian Trosko | http://tech.finn.no | http://github.com/finn-no |
signature.asc
Description: This is a digitally signed message part
