tiles-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nicolas LE BAS <m...@nlebas.net>
Subject Re: Tiles Request API again
Date Fri, 09 Dec 2011 17:39:01 GMT
On 11-12-09 10:13 AM, mck wrote:
>>> 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).

Basically the branch is all about it. I suppose the first 3 commits 
could be collapsed into a big one, although the thought process is more 
obvious this way.

> 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

hmm... Checkstyle should have caught that kind of mistake, I'll look 
into it. Obviously there's something wrong with my working environment, 
I'll fix it.

>>> 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"?
> [...]
> So my preference is the name DispatchRequest which would also mirror
> DispatcherRenderer where you were at one point proposing the
> functionality to go to...

I like that.

>>>> * 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.

It is odd indeed. Returning Addable is fine with me.

> At the same time should we change
>      Map<String,String>  getHeaders();
> to
>      ReadOnlyEnumerationMap<String>  getHeaders();
> ?

ReadOnlyEnumerationMap looks like an implementation detail to me, since 
it is a map implemented by the o.a.t.r.attribute package.

I agree a ReadOnlyMap interface without the "put" and "putAll" methods 
would be best if the java language provided it, but it doesn't. The 
example provided by the language is "Collections.unmodifiableMap()", and 
I believe it should not surprise the users if we behave the same.

>>>> * 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?

 From the "glue" code between controller and view that typically creates 
the Request instance, like TilesDispatchServlet for instance. I used to 
call it from a Spring View among other places, now I tend to favor some 
kind of renderer wrapper for reusability.

Of course it requires the controller to provide the actual timestamp of 
the last modification as a reference.

And of course spring MVC implements that method, so I could have the 
controller do it in the situations where I use spring MVC, but... well, 
I'm just not always in a web context.

 From a long term point of view, we could imagine using wrappers or 
interceptors around definitions or renderers. These would be a 
generalization of both ViewPreparer and PublisherRenderer, and then a 
preparer could get the timestamp and check it against the request.
We could also imagine a "CachingRenderer" with a 
"CachingRequestWrapper", caching HTML fragments for performance (ehcache 
has a solution to do it using ServletFilters, but... let's just say I 
think tiles can do a better job of it).


View raw message