(was: Re: [magnolia-dev] [magn olia-svn] [12148] MAGNOLIA-1698
Magnolia now works with forward and error dispatchers
On Nov 1, 2007, at 17:03 , Fabrizio Giustina wrote:
On 11/1/07, Grégory Joseph <[email protected]> wrote:
I just have one question, about the change in ContentTypeFilter[1].
Why do we resetAggregationState there? Isn't the ContextFilter used
just before ? It seems odd and out-of-place to do this in the filter
responsible for selecting/setting the content type. Thoughts?
Well, actually that filter was already responsible for creating the
AggregationState instance (the line above my change), and the reset is
needed just before that. (I know, looks odd to me too that
ContentFilter is used for instantiating the AggregationState, but
that's we have now...)
The fact that it creates it there is a side-effect, no a controlled
behavior. (ie it's created lazily, so if any other filter needs it
before the ContentTypeFilter, it will get it)
I suppose it's needed when doing a forward, since we're going down
the chain filter again but the AggState is already there. Why not do
it in the ContextFilter?
Digression:
Or, maybe better, we could do this in some of context related class
or init method ? I was about to suggest we could also do it in the
initAs.. methods of MgnlContext, and then remembered that the AnonCtx
was replaced by a WebContext at some point; then I was surprised to
see that the loginFilter (which inits the WebContext) is placed
*after* the ContentTypeFilter; and then finally I realized that the
AggState was a request attribute, so it doesn't get lost when
replacing the context... seems a bit obscure to me. Am also surprised
that we do request.getAttribute() to retrieve it, but we store it
with (thisContext.)setAttribute.
In any case, our new recruit, Andriy, is working closely with Philipp
on some refactorings and changes of the Context API, so I suppose we
can expect improvements and cleanup there in the near future.
(responding separately about code style)
Cheers,
greg
----------------------------------------------------------------
for list details see
http://documentation.magnolia.info/docs/en/editor/stayupdated.html
----------------------------------------------------------------
