Bug 495789 - Re-throw exceptions from converter

Otherwise unconverted values are set to the target,
and no conversion errors are reported.

Change-Id: Iadc78000eb8162b8d967930a1a857e75257ffec1
Signed-off-by: Jens Lidestrom <jens@lidestrom.se>

4 files changed, 35 insertions, 3 deletions

diff --git a/bundles/org.eclipse.core.databinding/META-INF/MANIFEST.MF b/bundles/org.eclipse.core.databinding/META-INF/MANIFEST.MF
index 31cc5c33e70..fac4141b717 100644
--- a/bundles/org.eclipse.core.databinding/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.core.databinding/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.databinding
-Bundle-Version: 1.7.600.qualifier
+Bundle-Version: 1.7.700.qualifier
 Bundle-ClassPath: .
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
diff --git a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateStrategy.java b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateStrategy.java
index 885269cbad9..7fe5c60559e 100644
--- a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateStrategy.java
+++ b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateStrategy.java
@@ -691,6 +691,7 @@ import com.ibm.icu.text.NumberFormat;
 				return converter.convert(value);
 			} catch (Exception ex) {
 				Policy.getLog().log(new Status(IStatus.ERROR, Activator.PLUGIN_ID, ex.getMessage(), ex));
+				throw ex;
 			}
 		}
 		return (D) value;
diff --git a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateValueStrategy.java b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateValueStrategy.java
index 87aedc6709b..5eda25055fc 100644
--- a/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateValueStrategy.java
+++ b/bundles/org.eclipse.core.databinding/src/org/eclipse/core/databinding/UpdateValueStrategy.java
@@ -402,8 +402,11 @@ public class UpdateValueStrategy<S, D> extends UpdateStrategy<S, D> {
 	}
 
 	/**
-	 * Sets the converter to be invoked when converting from the source type to
-	 * the destination type.
+	 * Sets the converter to be invoked when converting from the source type to the
+	 * destination type.
+	 * <p>
+	 * If the converter throws any exceptions they are reported as validation
+	 * errors, using the exception message.
 	 *
 	 * @param converter
 	 * @return the receiver, to enable method call chaining
diff --git a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ValueBindingTest.java b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ValueBindingTest.java
index 15c16003db4..b0e7a3ea94b 100755
--- a/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ValueBindingTest.java
+++ b/tests/org.eclipse.jface.tests.databinding/src/org/eclipse/core/tests/databinding/ValueBindingTest.java
@@ -20,6 +20,7 @@ import static org.eclipse.core.databinding.UpdateValueStrategy.POLICY_NEVER;
 import static org.eclipse.core.databinding.UpdateValueStrategy.POLICY_UPDATE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -27,6 +28,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 
 import org.eclipse.core.databinding.Binding;
 import org.eclipse.core.databinding.DataBindingContext;
@@ -38,6 +40,7 @@ import org.eclipse.core.databinding.observable.value.AbstractObservableValue;
 import org.eclipse.core.databinding.observable.value.IObservableValue;
 import org.eclipse.core.databinding.observable.value.ValueDiff;
 import org.eclipse.core.databinding.observable.value.WritableValue;
+import org.eclipse.core.databinding.util.Policy;
 import org.eclipse.core.databinding.validation.IValidator;
 import org.eclipse.core.databinding.validation.ValidationStatus;
 import org.eclipse.core.internal.databinding.BindingStatus;
@@ -368,6 +371,31 @@ public class ValueBindingTest extends AbstractDefaultRealmTestCase {
 		assertEquals(model.getValue(), target.getValue());
 	}
 
+	@Test
+	public void testErrorDuringConversion() {
+		UpdateValueStrategy<String, String> modelToTarget = new UpdateValueStrategy<>();
+		modelToTarget.setConverter(IConverter.create(String.class, String.class, fromObject -> {
+			throw new IllegalArgumentException();
+		}));
+
+		Binding binding = dbc.bindValue(target, model, new UpdateValueStrategy<>(), modelToTarget);
+		CountDownLatch latch = new CountDownLatch(1);
+
+		Policy.setLog(status -> {
+			latch.countDown();
+			assertEquals(IStatus.ERROR, status.getSeverity());
+			assertTrue(status.getException() instanceof IllegalArgumentException);
+		});
+
+		model.setValue("first");
+
+		assertNull("Target not changed on conversion error", target.getValue());
+		assertEquals(0, latch.getCount());
+		assertEquals(IStatus.ERROR, binding.getValidationStatus().getValue().getCode());
+
+		Policy.setLog(null);
+	}
+
 	private void bindLoggingValue(UpdateValueStrategy<Object, String> targetToModel,
 			UpdateValueStrategy<String, Object> modelToTarget) {
 		// Set model and target to different values to ensure we get a change