aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPaul Elder2013-07-15 12:04:50 (EDT)
committerGerrit Code Review @ Eclipse.org2013-07-18 13:41:01 (EDT)
commitf2fbd43ff1b20deea217bffbf4d31ab5e2fff506 (patch)
treebeb74c8f9d43ebf3bf69e6b1b7ed3ae361e18633
parentb9759603472fa79e196346ff68ad96a48c71567d (diff)
downloadeclipse.platform.ui-f2fbd43ff1b20deea217bffbf4d31ab5e2fff506.zip
eclipse.platform.ui-f2fbd43ff1b20deea217bffbf4d31ab5e2fff506.tar.gz
eclipse.platform.ui-f2fbd43ff1b20deea217bffbf4d31ab5e2fff506.tar.bz2
Bug 412681: IExecutionListener.preExecute receives ExecutionEventrefs/changes/64/14564/2
incompatible with HandlerUtil methods JUnit test changes illustrate the problem. Fix reverts to passing an ExpressionContext to Command.executeWithChecks() and Command.setEnabled() with-in HandlerServiceImpl. In HandlerServiceHandler, the static context is then retrieved from HandlerServiceImpl's stack (and cross checked with the paired execution context. Change-Id: Ie323caf3852e585b4e48f090a6c29a4871023342
-rw-r--r--bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java33
-rw-r--r--bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceImpl.java5
-rw-r--r--tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/commands/CommandExecutionTest.java28
3 files changed, 41 insertions, 25 deletions
diff --git a/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java b/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java
index ffb2674..ca6253f 100644
--- a/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java
+++ b/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceHandler.java
@@ -42,7 +42,7 @@ public class HandlerServiceHandler extends AbstractHandler {
public boolean isEnabled() {
ExecutionContexts contexts = HandlerServiceImpl.peek();
// setEnabled(contexts);
- IEclipseContext executionContext = getExecutionContext(contexts);
+ IEclipseContext executionContext = contexts.context; // getExecutionContext(contexts);
if (executionContext == null) {
return super.isEnabled();
}
@@ -51,7 +51,7 @@ public class HandlerServiceHandler extends AbstractHandler {
setBaseEnabled(false);
return super.isEnabled();
}
- IEclipseContext staticContext = getStaticContext(contexts);
+ IEclipseContext staticContext = contexts.staticContext; // getStaticContext(contexts);
Boolean result = (Boolean) ContextInjectionFactory.invoke(handler, CanExecute.class,
executionContext, staticContext, Boolean.TRUE);
setBaseEnabled(result.booleanValue());
@@ -74,7 +74,7 @@ public class HandlerServiceHandler extends AbstractHandler {
if (handler == null) {
return;
}
- IEclipseContext staticContext = getStaticContext(evaluationContext);
+ IEclipseContext staticContext = getStaticContext(executionContext);
if (staticContext == null) {
staticContext = EclipseContextFactory.create();
createContext = true;
@@ -86,32 +86,21 @@ public class HandlerServiceHandler extends AbstractHandler {
}
}
- IEclipseContext getStaticContext(Object evalObj) {
- if (evalObj instanceof ExecutionContexts) {
- return ((ExecutionContexts) evalObj).staticContext;
- }
- if (evalObj instanceof IEclipseContext) {
- return (IEclipseContext) ((IEclipseContext) evalObj)
- .get(HandlerServiceImpl.STATIC_CONTEXT);
- }
- if (evalObj instanceof ExpressionContext) {
- return (IEclipseContext) (((ExpressionContext) evalObj).eclipseContext)
- .get(HandlerServiceImpl.STATIC_CONTEXT);
- }
- if (evalObj instanceof IEvaluationContext) {
- return getStaticContext(((IEvaluationContext) evalObj).getParent());
- }
+ private IEclipseContext getStaticContext(IEclipseContext executionContext) {
final ExecutionContexts pair = HandlerServiceImpl.peek();
if (pair != null) {
+ if (pair.context != executionContext) {
+ // log this
+ }
return pair.staticContext;
}
return null;
}
protected IEclipseContext getExecutionContext(Object evalObj) {
- if (evalObj instanceof ExecutionContexts) {
- return ((ExecutionContexts) evalObj).context;
- }
+ // if (evalObj instanceof ExecutionContexts) {
+ // return ((ExecutionContexts) evalObj).context;
+ // }
if (evalObj instanceof IEclipseContext) {
return (IEclipseContext) evalObj;
}
@@ -159,7 +148,7 @@ public class HandlerServiceHandler extends AbstractHandler {
new NotHandledException(FAILED_TO_FIND_HANDLER_DURING_EXECUTION));
}
- IEclipseContext staticContext = getStaticContext(event.getApplicationContext());
+ IEclipseContext staticContext = getStaticContext(executionContext);
Object handler = HandlerServiceImpl.lookUpHandler(executionContext, commandId);
if (handler == null) {
return null;
diff --git a/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceImpl.java b/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceImpl.java
index 1385205..61cab22 100644
--- a/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceImpl.java
+++ b/bundles/org.eclipse.e4.core.commands/src/org/eclipse/e4/core/commands/internal/HandlerServiceImpl.java
@@ -26,6 +26,7 @@ import org.eclipse.core.commands.ParameterValueConversionException;
import org.eclipse.core.commands.ParameterizedCommand;
import org.eclipse.core.commands.common.NotDefinedException;
import org.eclipse.e4.core.commands.EHandlerService;
+import org.eclipse.e4.core.commands.ExpressionContext;
import org.eclipse.e4.core.contexts.ContextFunction;
import org.eclipse.e4.core.contexts.EclipseContextFactory;
import org.eclipse.e4.core.contexts.IEclipseContext;
@@ -170,7 +171,7 @@ public class HandlerServiceImpl implements EHandlerService {
push(executionContext, staticContext);
try {
Command cmd = command.getCommand();
- cmd.setEnabled(peek());
+ cmd.setEnabled(new ExpressionContext(peek().context));
return cmd.isEnabled();
} finally {
pop();
@@ -210,7 +211,7 @@ public class HandlerServiceImpl implements EHandlerService {
push(executionContext, staticContext);
try {
// Command cmd = command.getCommand();
- return command.executeWithChecks(null, peek());
+ return command.executeWithChecks(null, new ExpressionContext(peek().context));
} catch (ExecutionException e) {
staticContext.set(HANDLER_EXCEPTION, e);
} catch (NotDefinedException e) {
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/commands/CommandExecutionTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/commands/CommandExecutionTest.java
index 2d22b69..5ad100d 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/commands/CommandExecutionTest.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/commands/CommandExecutionTest.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2012 IBM Corporation and others.
+ * Copyright (c) 2012, 2013 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -16,13 +16,16 @@ import java.util.ArrayList;
import org.eclipse.core.commands.Command;
import org.eclipse.core.commands.ExecutionEvent;
import org.eclipse.core.commands.ExecutionException;
+import org.eclipse.core.commands.IExecutionListener;
import org.eclipse.core.commands.IExecutionListenerWithChecks;
import org.eclipse.core.commands.NotEnabledException;
import org.eclipse.core.commands.NotHandledException;
import org.eclipse.core.commands.common.NotDefinedException;
import org.eclipse.ui.IPageLayout;
import org.eclipse.ui.IWorkbenchCommandConstants;
+import org.eclipse.ui.IWorkbenchWindow;
import org.eclipse.ui.commands.ICommandService;
+import org.eclipse.ui.handlers.HandlerUtil;
import org.eclipse.ui.handlers.IHandlerService;
import org.eclipse.ui.tests.harness.util.UITestCase;
@@ -53,9 +56,12 @@ public class CommandExecutionTest extends UITestCase {
private static class EL implements IExecutionListenerWithChecks {
ArrayList<Pair> methods = new ArrayList<Pair>();
+ IWorkbenchWindow wbw;
public void preExecute(String commandId, ExecutionEvent event) {
methods.add(new Pair("preExecute", event));
+ // ensure HandlerUtil has proper access. See bug 412681.
+ wbw = HandlerUtil.getActiveWorkbenchWindow(event);
}
public void postExecuteSuccess(String commandId, Object returnValue) {
@@ -110,6 +116,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notEnabled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandExecute() throws Exception {
@@ -131,6 +138,18 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notEnabled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
+ }
+
+ /**
+ * Verify that {@link IExecutionListener#preExecute(String, ExecutionEvent)} has
+ * received an event compatible with {@link HandlerUtil} methods.
+ * @param listener
+ */
+ private void verifyHandlerUtilAccessDuringPreExecute(EL listener) {
+ assertNotNull(
+ "HandlerUtil.getActiveWorkbenchWindow() returned null during ICommandListener.preExecute().",
+ listener.wbw);
}
public void testCommandListenerExecute() throws Exception {
@@ -152,6 +171,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notEnabled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandServiceExecuteRefresh() throws Exception {
@@ -170,6 +190,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notHandled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandExecuteRefresh() throws Exception {
@@ -191,6 +212,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notHandled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandListenerExecuteRefresh() throws Exception {
@@ -212,6 +234,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "notHandled" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandServiceExecuteClosePart() throws Exception {
@@ -232,6 +255,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "postExecuteSuccess" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandExecuteClosePart() throws Exception {
@@ -255,6 +279,7 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "postExecuteSuccess" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
public void testCommandListenerExecuteClosePart() throws Exception {
@@ -278,5 +303,6 @@ public class CommandExecutionTest extends UITestCase {
System.out.println(listener.methods);
String[] calls = { "preExecute", "postExecuteSuccess" };
compare(calls, listener.methods);
+ verifyHandlerUtilAccessDuringPreExecute(listener);
}
}