Carsten Ziegeler wrote:
> Sylvain Wallez wrote:
>
>> Carsten Ziegeler wrote:
>>
>>> sylvain@apache.org wrote:
>>>
>>>> Author: sylvain
>>>> Date: Tue Feb 1 07:43:40 2005
>>>> New Revision: 149408
>>>>
>>>> /**
>>>> - * Remove attribute from the current instance, as well as from
>>>> the
>>>> - * wrapped environment.
>>>> + * Remove attribute from the current instance.
>>>> *
>>>> * @param name a <code>String</code> value
>>>> */
>>>> public void removeAttribute(String name) {
>>>> super.removeAttribute(name);
>>>> - this.environment.removeAttribute(name);
>>>> }
>>>>
>>>
>>>
>>>
>>> Hmm, isn't this an incompatible change? Ok, I always thought that
>>> the implementation was not the way one would expect, but we
>>> shouldn't imho introduce such changes during maintenance releases.
>>> I'm fine with changing this for 2.2.
>>
>>
>>
>>
>> I tracked down all uses of removeAttribute in the code base (there
>> are really few):
>> - ActionSetNode.call : the previous implementation could lead to bugs
>> when an action-set was calling a cocoon: that itself uses an action-set
>> - MountNode.invoke : same bug if a pass-through calls a cocoon: that
>> has a pass-through
>> - CocoonComponentManager.leaveEnvironment : called only when env
>> stack is empty, meaning we aren't in a wrapped environment
>>
>> So this change actually fixes some bugs ;-)
>>
>> Now my changes in pass-through handling aren't finished, as I found
>> some cases where it breaks. Working on it...
>>
> I really value your work, but you are changing a lot of things and I'm
> really wondering what site effects this may cause. The internal
> environment handling has always been a very fragile part of Cocoon.
> Remember all the problems with internal redirects, redirects from flow
> and so on, so I'm really questioning if it's worth to go through this
> just because you need this pass-through feature in 2.1.x?
Well, we have based some important parts of our project based on the
assumption that it worked, and well... I need to make it work now :-/
Now if I find all these bugs in 2.1, I guess that also because our
project makes a good test case for this fragile thing. So if our project
runs, it's very likely that everything is ok. I'm also building some
test cases in webapp/samples/tests/pass-through along with my bugfix
work. We'll see how 2.2 behaves ;-)
> I think it would have been better if you had made an internal fork and
> not commit this into 2.1.x and once 2.2 is stable you could forget
> about your fork and use that. But apparently it's too late, I guess :(
Mmmh... yes...
> But hopefully everything will run smoothly after your changes and
> noone is affected by the incompatible changes.
Hopefully :-) And in the meantime, this allows me to better understand
the internals of the environment management, which was still a dark area
for me. I know this has changed a lot in 2.2, but this knowledge won't
be lost and will allow me to more easily understand the reasons that led
to the 2.2 refactoring.
Also, if I don't succeed in fixing 2.1, I promise to roll back the
current changes.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
|