Skip to content
Snippets Groups Projects
Commit 343b2922 authored by Vincent Massol's avatar Vincent Massol
Browse files

XWIKI-12119: The VelocityContext located in the ExecutionContext is not clean/copied

parent 403278e3
No related branches found
No related tags found
No related merge requests found
...@@ -20,17 +20,18 @@ ...@@ -20,17 +20,18 @@
package org.xwiki.mail.internal.thread.context; package org.xwiki.mail.internal.thread.context;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton; import javax.inject.Singleton;
import javax.script.ScriptContext;
import org.apache.commons.lang3.exception.CloneFailedException; import org.apache.commons.lang3.exception.CloneFailedException;
import org.apache.velocity.VelocityContext;
import org.xwiki.component.annotation.Component; import org.xwiki.component.annotation.Component;
import org.xwiki.context.Execution; import org.xwiki.context.Execution;
import org.xwiki.context.ExecutionContext; import org.xwiki.context.ExecutionContext;
import org.xwiki.context.ExecutionContextInitializer;
import org.xwiki.context.ExecutionContextManager; import org.xwiki.context.ExecutionContextManager;
import org.xwiki.script.ScriptContextManager; import org.xwiki.script.ScriptContextManager;
import org.xwiki.velocity.VelocityManager; import org.xwiki.velocity.VelocityManager;
import org.xwiki.velocity.internal.VelocityExecutionContextInitializer;
import com.xpn.xwiki.XWikiContext; import com.xpn.xwiki.XWikiContext;
...@@ -53,7 +54,7 @@ public class ExecutionContextCopier implements Copier<ExecutionContext> ...@@ -53,7 +54,7 @@ public class ExecutionContextCopier implements Copier<ExecutionContext>
private ScriptContextManager scriptContextManager; private ScriptContextManager scriptContextManager;
@Inject @Inject
private VelocityManager velociyManager; private VelocityManager velocityManager;
@Inject @Inject
private Execution execution; private Execution execution;
...@@ -61,6 +62,10 @@ public class ExecutionContextCopier implements Copier<ExecutionContext> ...@@ -61,6 +62,10 @@ public class ExecutionContextCopier implements Copier<ExecutionContext>
@Inject @Inject
private Copier<XWikiContext> xwikiContextCloner; private Copier<XWikiContext> xwikiContextCloner;
@Inject
@Named("velocity")
private ExecutionContextInitializer velocityExecutionContextInitializer;
@Override @Override
public ExecutionContext copy(ExecutionContext originalExecutionContext) throws CloneFailedException public ExecutionContext copy(ExecutionContext originalExecutionContext) throws CloneFailedException
{ {
...@@ -74,26 +79,13 @@ public ExecutionContext copy(ExecutionContext originalExecutionContext) throws C ...@@ -74,26 +79,13 @@ public ExecutionContext copy(ExecutionContext originalExecutionContext) throws C
XWikiContext clonedXWikiContext = xwikiContextCloner.copy(xwikiContext); XWikiContext clonedXWikiContext = xwikiContextCloner.copy(xwikiContext);
clonedExecutionContext.setProperty(XWikiContext.EXECUTIONCONTEXT_KEY, clonedXWikiContext); clonedExecutionContext.setProperty(XWikiContext.EXECUTIONCONTEXT_KEY, clonedXWikiContext);
// We have just set the clonedXWikiContext. We can now use our clonedExecutionContext as base for the // VelocityContext
// initialization of the ScriptContext and the VelocityContext by pushing it in the Execution. // Reset the VelocityContext from the EC by removing it and calling the Velocity ECInitializer which is
try { // normally called by the execution of the ECInitializers by ECManager.clone(). This ensures a clean new
this.execution.pushContext(clonedExecutionContext); // VC is created. It'll get filled when VelocityContextManager.getVelocityContext() is called by whatever
// code need the VC.
// The managers will also run the initializers on the returned contexts, thus using the XWikiContext clonedExecutionContext.removeProperty(VelocityExecutionContextInitializer.VELOCITY_CONTEXT_ID);
// from the execution. this.velocityExecutionContextInitializer.initialize(clonedExecutionContext);
ScriptContext reInitializedScriptContext = scriptContextManager.getScriptContext();
// Note: scriptContextManager.getScriptContext() is already called by
// velociyManager.getVelocityContext() internally, so we could avoid running it twice just for the sake
// of code readability.
VelocityContext reInitializedVelocityContext = velociyManager.getVelocityContext();
// Note2: we could even argue that velociyManager.getVelocityContext() will eventually be called by a
// client willing to use it, since it should generally not be used directly from the ExecutionContext,
// but it should be always used through the VelocityManager. Could this entire inner try-catch block be
// removed and deemed not needed? Would it be safe enough?
} finally {
// Remember to pop it, since we don`t want it to replace our current ExecutionContext.
this.execution.popContext();
}
return clonedExecutionContext; return clonedExecutionContext;
} catch (Exception e) { } catch (Exception e) {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment