You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by sy...@apache.org on 2005/02/01 16:43:45 UTC
svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src:
deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/
java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/
java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Author: sylvain
Date: Tue Feb 1 07:43:40 2005
New Revision: 149408
URL: http://svn.apache.org/viewcvs?view=rev&rev=149408
Log:
more fixes of path-through with deeply nested cocoon: sources
Modified:
cocoon/branches/BRANCH_2_1_X/src/deprecated/java/org/apache/cocoon/components/source/SitemapSource.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/CocoonComponentManager.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/source/impl/SitemapSource.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/ConcreteTreeProcessor.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/TreeProcessor.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/AbstractEnvironment.java
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/wrapper/EnvironmentWrapper.java
cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/explain-test.xml
cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/sitemap.xmap
Modified: cocoon/branches/BRANCH_2_1_X/src/deprecated/java/org/apache/cocoon/components/source/SitemapSource.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/deprecated/java/org/apache/cocoon/components/source/SitemapSource.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/deprecated/java/org/apache/cocoon/components/source/SitemapSource.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/deprecated/java/org/apache/cocoon/components/source/SitemapSource.java Tue Feb 1 07:43:40 2005
@@ -133,7 +133,7 @@
} else if (uri.startsWith("/", position)) {
position ++;
this.prefix = null;
- this.processor = CocoonComponentManager.getCurrentProcessor();
+ this.processor = CocoonComponentManager.getActiveProcessor();
} else {
throw new ProcessingException("Malformed cocoon URI.");
}
@@ -243,7 +243,7 @@
this.processKey = CocoonComponentManager.startProcessing(this.environment);
this.environment.setURI(this.prefix, this.uri);
this.processingPipeline = this.processor.buildPipeline(this.environment);
- this.pipelineProcessor = CocoonComponentManager.getLastProcessor(this.environment);
+ this.pipelineProcessor = CocoonComponentManager.getActiveProcessor();
String redirectURL = this.environment.getRedirectURL();
if (redirectURL != null) {
if (redirectURL.indexOf(":") == -1) {
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/CocoonComponentManager.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/CocoonComponentManager.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/CocoonComponentManager.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/CocoonComponentManager.java Tue Feb 1 07:43:40 2005
@@ -58,6 +58,9 @@
/** The key used to store the current process environment */
private static final String PROCESS_KEY = CocoonComponentManager.class.getName();
+
+ /** The environment attribute used to keep track of the actual environment in which the pipeline was built. */
+ private static final String PROCESSOR_ATTR = "CocoonComponentManager.processor";
/** The environment information */
private static InheritableThreadLocal environmentStack = new CloningInheritableThreadLocal();
@@ -120,7 +123,7 @@
stack.push(new Object[] {env, processor, manager, new Integer(stack.getOffset())});
stack.setOffset(stack.size()-1);
- env.setAttribute("CocoonComponentManager.processor", processor);
+ env.setAttribute(PROCESSOR_ATTR, processor);
}
/**
@@ -128,9 +131,20 @@
* It's the counterpart to {@link #enterEnvironment(Environment, ComponentManager, Processor)}.
*/
public static void leaveEnvironment() {
+ leaveEnvironment(false);
+ }
+
+ /**
+ * This hook must be called by the sitemap each time a sitemap is left.
+ * It's the counterpart to {@link #enterEnvironment(Environment, ComponentManager, Processor)}.
+ *
+ * @param success indicates if the request was successfully handled by the environment that's being left
+ */
+ public static void leaveEnvironment(boolean success) {
final EnvironmentStack stack = (EnvironmentStack)environmentStack.get();
final Object[] objs = (Object[])stack.pop();
stack.setOffset(((Integer)objs[3]).intValue());
+
if ( stack.isEmpty() ) {
final Environment env = (Environment)objs[0];
final Map globalComponents = (Map)env.getAttribute(GlobalRequestLifecycleComponent.class.getName());
@@ -144,6 +158,11 @@
}
}
env.removeAttribute(GlobalRequestLifecycleComponent.class.getName());
+ } else {
+ if (!success) {
+ // Restore the current environment as being the active one
+ getCurrentEnvironment().setAttribute(PROCESSOR_ATTR, getCurrentProcessor());
+ }
}
}
@@ -242,10 +261,10 @@
}
/**
- * Return the processor that is actually processing the request
+ * Return the processor that has actually processed the request
*/
- public static Processor getLastProcessor(Environment env) {
- return (Processor) env.getAttribute("CocoonComponentManager.processor");
+ public static Processor getActiveProcessor() {
+ return (Processor) getCurrentEnvironment().getAttribute(PROCESSOR_ATTR);
}
/**
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/source/impl/SitemapSource.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/source/impl/SitemapSource.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/source/impl/SitemapSource.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/source/impl/SitemapSource.java Tue Feb 1 07:43:40 2005
@@ -140,12 +140,12 @@
String prefix;
if (uri.startsWith("//", position)) {
position += 2;
- this.processor = CocoonComponentManager.getCurrentProcessor().getRootProcessor();
+ this.processor = CocoonComponentManager.getActiveProcessor().getRootProcessor();
prefix = ""; // start at the root
} else if (uri.startsWith("/", position)) {
position ++;
prefix = null;
- this.processor = CocoonComponentManager.getCurrentProcessor();
+ this.processor = CocoonComponentManager.getActiveProcessor();
} else {
throw new MalformedURLException("Malformed cocoon URI: " + uri);
}
@@ -330,7 +330,7 @@
try {
this.processKey = CocoonComponentManager.startProcessing(this.environment);
this.processingPipeline = this.processor.buildPipeline(this.environment);
- this.pipelineProcessor = CocoonComponentManager.getLastProcessor(this.environment);
+ this.pipelineProcessor = CocoonComponentManager.getActiveProcessor();
String redirectURL = this.environment.getRedirectURL();
if (redirectURL == null) {
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/ConcreteTreeProcessor.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/ConcreteTreeProcessor.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/ConcreteTreeProcessor.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/ConcreteTreeProcessor.java Tue Feb 1 07:43:40 2005
@@ -235,14 +235,15 @@
setupLogger(redirector);
context.setRedirector(redirector);
- objectModel.put(ProcessingNode.OBJECT_SOURCE_RESOLVER, environment);
+ objectModel.put(ProcessingNode.OBJECT_SOURCE_RESOLVER, environment);
+ boolean success = false;
try {
- boolean success = this.rootNode.invoke(environment, context);
+ success = this.rootNode.invoke(environment, context);
return success;
} finally {
- CocoonComponentManager.leaveEnvironment();
+ CocoonComponentManager.leaveEnvironment(success);
// Restore old redirector and resolver
context.setRedirector(oldRedirector);
objectModel.put(ProcessingNode.OBJECT_SOURCE_RESOLVER, oldResolver);
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/TreeProcessor.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/TreeProcessor.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/TreeProcessor.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/treeprocessor/TreeProcessor.java Tue Feb 1 07:43:40 2005
@@ -328,7 +328,7 @@
newProcessor.setProcessorData(builder.getSitemapComponentManager(), root, builder.getDisposableNodes());
} finally {
- CocoonComponentManager.leaveEnvironment();
+ CocoonComponentManager.leaveEnvironment(false);
this.builderSelector.release(builder);
}
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/AbstractEnvironment.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/AbstractEnvironment.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/AbstractEnvironment.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/AbstractEnvironment.java Tue Feb 1 07:43:40 2005
@@ -447,6 +447,10 @@
public void setAttribute(String name, Object value) {
this.attributes.put(name, value);
}
+
+ protected boolean hasAttribute(String name) {
+ return this.attributes.containsKey(name);
+ }
public void removeAttribute(String name) {
this.attributes.remove(name);
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/wrapper/EnvironmentWrapper.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/wrapper/EnvironmentWrapper.java?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/wrapper/EnvironmentWrapper.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/environment/wrapper/EnvironmentWrapper.java Tue Feb 1 07:43:40 2005
@@ -427,21 +427,22 @@
public Object getAttribute(String name)
{
Object value = super.getAttribute(name);
- if (value == null)
+
+ // get it from the wrapped env only if it's not defined here with a null value
+ if (value == null && !hasAttribute(name)) {
value = this.environment.getAttribute(name);
+ }
return value;
}
/**
- * 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);
}
/**
Modified: cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/explain-test.xml
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/explain-test.xml?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/explain-test.xml (original)
+++ cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/explain-test.xml Tue Feb 1 07:43:40 2005
@@ -29,8 +29,10 @@
<para>
Test links:
<ul>
- <li><link href="welcome">Correct page</link></li>
- <li><link href="welcome-cocoon">Broken page</link></li>
+ <li><link href="welcome">Direct pipeline</link></li>
+ <li><link href="welcome1">One "cocoon:" URL</link></li>
+ <li><link href="welcome2">Two chained "cocoon:" URLs</link></li>
+ <li><link href="welcome3">Three chained "cocoon:" URLs</link></li>
</ul>
</para>
</content>
Modified: cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/sitemap.xmap
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/sitemap.xmap?view=diff&r1=149407&r2=149408
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/sitemap.xmap (original)
+++ cocoon/branches/BRANCH_2_1_X/src/webapp/samples/test/pass-through/sitemap.xmap Tue Feb 1 07:43:40 2005
@@ -8,18 +8,24 @@
<map:match pattern="welcome">
<map:generate src="welcome.xml"/>
- <map:serialize type="xhtml"/>
+ <map:serialize/>
+ </map:match>
+
+ <map:match pattern="welcome1">
+ <map:generate src="cocoon:/welcome"/>
+ <map:serialize/>
</map:match>
- <map:match pattern="welcome-cocoon">
- <map:generate src="cocoon:/welcome.ccn"/>
+ <map:match pattern="welcome2">
+ <map:generate src="cocoon:/welcome1"/>
<map:serialize/>
</map:match>
- <map:match pattern="welcome.ccn">
- <map:generate src="welcome.xml"/>
- <map:serialize type="xhtml"/>
+ <map:match pattern="welcome3">
+ <map:generate src="cocoon:/welcome2"/>
+ <map:serialize/>
</map:match>
+
</map:pipeline>
</map:pipelines>
</map:sitemap>
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Sylvain Wallez <sy...@apache.org>.
Carsten Ziegeler wrote:
> Sylvain Wallez wrote:
<snip/>
>> Also, if I don't succeed in fixing 2.1, I promise to roll back the
>> current changes.
>>
> Deal :) (Just as a reminder, I'm sure you would have done anyway:
> Please also add the important changes to 2.2 later on, like the change
> in the removeAttribute() method)
Sure!
> Good luck :)
Thanks :-)
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Carsten Ziegeler <cz...@apache.org>.
Sylvain Wallez wrote:
>
> 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 ;-)
>
:) True, your project might really be a good test case.
>
> 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.
>
Perhaps ;) I'm currently not sure if the pass-through is working 100% in
2.2, but it should be much easier to fix - at least that's my hope...
> Also, if I don't succeed in fixing 2.1, I promise to roll back the
> current changes.
>
Deal :) (Just as a reminder, I'm sure you would have done anyway: Please
also add the important changes to 2.2 later on, like the change in the
removeAttribute() method)
Good luck :)
Carsten
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Carsten Ziegeler <cz...@apache.org>.
Sylvain Wallez wrote:
>
> Fixed. Weird combinations of "cocoon:" and pass-through mounts now work
> correctly in 2.1. I'll do the same test in 2.2.
>
Great :) If there are problems, let's see if they can be fixed in a
different way than in 2.1.
Carsten
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Sylvain Wallez <sy...@apache.org>.
Sylvain Wallez wrote:
> 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 ;-)
> Also, if I don't succeed in fixing 2.1, I promise to roll back the
> current changes.
Fixed. Weird combinations of "cocoon:" and pass-through mounts now work
correctly in 2.1. I'll do the same test in 2.2.
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Sylvain Wallez <sy...@apache.org>.
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 }
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Carsten Ziegeler <cz...@apache.org>.
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?
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 :(
But hopefully everything will run smoothly after your changes and noone
is affected by the incompatible changes.
Carsten
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Sylvain Wallez <sy...@apache.org>.
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...
Sylvain
--
Sylvain Wallez Anyware Technologies
http://www.apache.org/~sylvain http://www.anyware-tech.com
{ XML, Java, Cocoon, OpenSource }*{ Training, Consulting, Projects }
Re: svn commit: r149408 - in cocoon/branches/BRANCH_2_1_X/src: deprecated/java/org/apache/cocoon/components/source/
java/org/apache/cocoon/components/ java/org/apache/cocoon/components/source/impl/
java/org/apache/cocoon/components/treeprocessor/ java/org/apache/cocoon/environment/
java/org/apache/cocoon/environment/wrapper/ webapp/samples/test/pass-through/
Posted by Carsten Ziegeler <cz...@apache.org>.
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.
Carsten