diff options
author | Marcus Hoepfner | 2019-12-27 08:49:57 +0000 |
---|---|---|
committer | Lars Vogel | 2020-02-04 08:53:03 +0000 |
commit | c1e824bb2026b5a7ef58698e0bd914ad0d8d0c5a (patch) | |
tree | 72284a6cbc77d706c810eacd81c2b2b7295397b9 | |
parent | f600a80625b497bb00aeebe40eeb4a776f15823a (diff) | |
download | rt.equinox.bundles-c1e824bb2026b5a7ef58698e0bd914ad0d8d0c5a.tar.gz rt.equinox.bundles-c1e824bb2026b5a7ef58698e0bd914ad0d8d0c5a.tar.xz rt.equinox.bundles-c1e824bb2026b5a7ef58698e0bd914ad0d8d0c5a.zip |
Bug 558642: SafeRunner can return resultsI20200205-1800I20200205-0030I20200204-1800
SafeRunner has a second run method which expects a
ISafeRunnableWithResult<T>. The run method returns this result.
Also some refactoring of the handleException method has been done (boy
scout rule).
Furthermore test have been improved:
- adapt to lambda
- better test method names to recognize what was wrong
- better text messages in assertions
Change-Id: I3a648490f85101d87119335310c03774371408af
Signed-off-by: Marcus Hoepfner <marcus.hoepfner@sap.com>
3 files changed, 193 insertions, 103 deletions
diff --git a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java index 7edf55d0a..c209ab8a9 100644 --- a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java +++ b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java @@ -13,7 +13,10 @@ *******************************************************************************/ package org.eclipse.equinox.common.tests; -import org.eclipse.core.runtime.*; +import org.eclipse.core.runtime.ISafeRunnable; +import org.eclipse.core.runtime.ISafeRunnableWithResult; +import org.eclipse.core.runtime.OperationCanceledException; +import org.eclipse.core.runtime.SafeRunner; import org.eclipse.core.tests.harness.CoreTest; /** @@ -22,89 +25,36 @@ import org.eclipse.core.tests.harness.CoreTest; public class SafeRunnerTest extends CoreTest { /** - * Ensures that cancelation exceptions are handled + * Ensures that cancellation exceptions are handled */ - public void testCancel() { - boolean caught = false; + public void testOperationCanceledExceptionAreHandled() { try { - SafeRunner.run(new ISafeRunnable() { - @Override - public void handleException(Throwable exception) { - } - - @Override - public void run() throws Exception { - throw new OperationCanceledException(); - } + SafeRunner.run(() -> { + throw new OperationCanceledException(); }); } catch (OperationCanceledException e) { - caught = true; + fail("OperationCanceledException unexpectedly caught.", e); } - assertFalse("1.0", caught); } - /** - * Tests that SafeRunner catches the expected exception types. - */ - public void testCaughtExceptionTypes() { - Throwable[] failures = new Throwable[] {new AssertionError(), new LinkageError(), new RuntimeException()}; - for (int i = 0; i < failures.length; i++) { - final Throwable[] handled = new Throwable[1]; - final Throwable current = failures[i]; - try { - SafeRunner.run(new ISafeRunnable() { - @Override - public void handleException(Throwable exception) { - handled[0] = exception; - } + public void testAssertionErrorIsCaught() { + assertExceptionHandled(new AssertionError()); + } - @Override - public void run() throws Exception { - if (current instanceof Exception) { - throw (Exception) current; - } else if (current instanceof Error) { - throw (Error) current; - } - } - }); - } catch (Throwable t) { - fail("1." + i, t); - } - assertEquals("2." + i, current, handled[0]); - } + public void testLinkageErrorIsCaught() { + assertExceptionHandled(new LinkageError()); + } + public void testRuntimeExceptionIsCaught() { + assertExceptionHandled(new RuntimeException()); } - /** - * Tests that SafeRunner re-throws expected exception types. - */ - public void testThrownExceptionTypes() { - //thrown exceptions - final Throwable[] thrown = new Throwable[] {new Error(), new OutOfMemoryError()}; - for (int i = 0; i < thrown.length; i++) { - boolean caught = false; - final Throwable current = thrown[i]; - try { - SafeRunner.run(new ISafeRunnable() { - @Override - public void handleException(Throwable exception) { - } + public void testRethrowsError() { + assertExceptionRethrown(new Error()); + } - @Override - public void run() throws Exception { - if (current instanceof Exception) { - throw (Exception) current; - } else if (current instanceof Error) { - throw (Error) current; - } - } - }); - } catch (Throwable t) { - assertEquals("1." + i, current, t); - caught = true; - } - assertTrue("2." + i, caught); - } + public void testRethrowsOutOfMemoryError() { + assertExceptionRethrown(new OutOfMemoryError()); } public void testNull() { @@ -112,7 +62,7 @@ public class SafeRunnerTest extends CoreTest { SafeRunner.run(null); fail("1.0"); } catch (RuntimeException e) { - //expected + // expected } } @@ -120,7 +70,7 @@ public class SafeRunnerTest extends CoreTest { * Ensures that exceptions are propagated when the safe runner re-throws it */ public void testRethrow() { - boolean caught = false; + IllegalArgumentException caughtException = null; try { SafeRunner.run(new ISafeRunnable() { @Override @@ -136,9 +86,64 @@ public class SafeRunnerTest extends CoreTest { } }); } catch (IllegalArgumentException e) { - caught = true; + caughtException = e; + } + assertNotNull("Cathed exception expected.", caughtException); + + } + + public void testWithResult() { + assertEquals("TestRun", SafeRunner.run(() -> "TestRun")); + } + + public void testWithResultReturnsNullOnException() { + ISafeRunnableWithResult<String> code = () -> { + throw new IllegalArgumentException(); + }; + assertNull(SafeRunner.run(code)); + } + + private void assertExceptionRethrown(Throwable current) { + Throwable caughtException = null; + try { + SafeRunner.run(new ISafeRunnable() { + + @Override + public void run() throws Exception { + if (current instanceof Exception) { + throw (Exception) current; + } else if (current instanceof Error) { + throw (Error) current; + } + } + }); + } catch (Throwable t) { + caughtException = t; } - assertTrue("1.0", caught); + assertEquals("Unexpected exception.", current, caughtException); + } + private void assertExceptionHandled(Throwable throwable) { + final Throwable[] handled = new Throwable[1]; + try { + SafeRunner.run(new ISafeRunnable() { + @Override + public void handleException(Throwable exception) { + handled[0] = exception; + } + + @Override + public void run() throws Exception { + if (throwable instanceof Exception) { + throw (Exception) throwable; + } else if (throwable instanceof Error) { + throw (Error) throwable; + } + } + }); + } catch (Throwable t) { + fail("Exception unexpectedly caught.", t); + } + assertEquals("Unexpected exception.", throwable, handled[0]); } -} +}
\ No newline at end of file diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java new file mode 100644 index 000000000..1122fa052 --- /dev/null +++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java @@ -0,0 +1,48 @@ +/******************************************************************************* + * Copyright (c) 2020 Marcus Höpfner and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Marcus Höpfner - initial API and implementation + *******************************************************************************/ +package org.eclipse.core.runtime; + +/** + * Safe runnables represent blocks of code and associated exception + * handlers. They are typically used when a plug-in needs to call some + * untrusted code (e.g., code contributed by another plug-in via an + * extension). In contradiction to {@link ISafeRunnable} this runnable is able to return a result. + * <p> + * This interface can be used without OSGi running. + * </p><p> + * Clients may implement this interface. + * </p> + * @param <T> the type of the result + * @see SafeRunner#run(ISafeRunnableWithResult) + * + * @since 3.11 + */ +@FunctionalInterface +public interface ISafeRunnableWithResult<T> extends ISafeRunnable { + @Override + default void run() throws Exception { + runWithResult(); + } + + /** + * Runs this runnable and returns the result. Any exceptions thrown from this method will + * be logged by the caller and passed to this runnable's + * {@link #handleException} method. + * @return the result + * + * @exception Exception if a problem occurred while running this method + * @see SafeRunner#run(ISafeRunnable) + */ + public T runWithResult() throws Exception; +}
\ No newline at end of file diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java index 3d6861f8b..0fc230be9 100644 --- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java +++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java @@ -28,9 +28,9 @@ import org.eclipse.osgi.util.NLS; public final class SafeRunner { /** - * Runs the given runnable in a protected mode. Exceptions + * Runs the given runnable in a protected mode. Exceptions * thrown in the runnable are logged and passed to the runnable's - * exception handler. Such exceptions are not rethrown by this method. + * exception handler. Such exceptions are not rethrown by this method. * <p> * In addition to catching all {@link Exception} types, this method also catches certain {@link Error} * types that typically result from programming errors in the code being executed. @@ -48,29 +48,66 @@ public final class SafeRunner { } } - private static void handleException(ISafeRunnable code, Throwable e) { - if (!(e instanceof OperationCanceledException)) { - // try to obtain the correct plug-in id for the bundle providing the safe runnable - Activator activator = Activator.getDefault(); - String pluginId = null; - if (activator != null) - pluginId = activator.getBundleId(code); - if (pluginId == null) - pluginId = IRuntimeConstants.PI_COMMON; - String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId); - IStatus status; - if (e instanceof CoreException) { - status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e); - ((MultiStatus) status).merge(((CoreException) e).getStatus()); - } else { - status = new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e); - } - // Make sure user sees the exception: if the log is empty, log the exceptions on stderr - if (!RuntimeLog.isEmpty()) - RuntimeLog.log(status); - else - e.printStackTrace(); + /** + * Runs the given runnable in a protected mode and returns the result given by the runnable. Exceptions + * thrown in the runnable are logged and passed to the runnable's + * exception handler. Such exceptions are not rethrown by this method, instead null is returned. + * <p> + * In addition to catching all {@link Exception} types, this method also catches certain {@link Error} + * types that typically result from programming errors in the code being executed. + * Severe errors that are not generally safe to catch are not caught by this method. + * </p> + * @param <T> the result type + * + * @param code the runnable to run + * @return the result + * + * @since 3.11 + */ + public static <T> T run(ISafeRunnableWithResult<T> code) { + Assert.isNotNull(code); + try { + return code.runWithResult(); + } catch (Exception | LinkageError | AssertionError e) { + handleException(code, e); + return null; + } + } + + private static void handleException(ISafeRunnable code, Throwable exception) { + if (!(exception instanceof OperationCanceledException)) { + String pluginId = getBundleIdOfSafeRunnable(code); + IStatus status = convertToStatus(exception, pluginId); + makeSureUserSeesException(exception, status); + } + code.handleException(exception); + } + + private static void makeSureUserSeesException(Throwable exception, IStatus status) { + if (RuntimeLog.isEmpty()) { + exception.printStackTrace(); + } else { + RuntimeLog.log(status); + } + } + + private static String getBundleIdOfSafeRunnable(ISafeRunnable code) { + Activator activator = Activator.getDefault(); + String pluginId = null; + if (activator != null) + pluginId = activator.getBundleId(code); + if (pluginId == null) + return IRuntimeConstants.PI_COMMON; + return pluginId; + } + + private static IStatus convertToStatus(Throwable exception, String pluginId) { + String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId); + if (exception instanceof CoreException) { + MultiStatus status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception); + status.merge(((CoreException) exception).getStatus()); + return status; } - code.handleException(e); + return new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception); } -} +}
\ No newline at end of file |