Mailing-List: contact dev-help@tiles.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@tiles.apache.org
Received-SPF: pass (athena.apache.org: domain of mail@nlebas.net designates
 87.98.221.115 as permitted sender)
Message-ID: <4EE247B5.6060208@nlebas.net>
Date: Fri, 09 Dec 2011 12:39:01 -0500
From: Nicolas LE BAS <mail@nlebas.net>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
 rv:1.9.2.23) Gecko/20110921 Thunderbird/3.1.15
MIME-Version: 1.0
To: dev@tiles.apache.org
Subject: Re: Tiles Request API again
References: <4EDA7CC3.9010706@nlebas.net>
	 <1323335118.8073.1589.camel@localhost>  <4EE0FEB6.6000005@nlebas.net>
 <1323443638.29872.0.camel@localhost>
In-Reply-To: <1323443638.29872.0.camel@localhost>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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

Nick.