Fix: Concurrency issues with AbstractDeclarativeValidator

3 files changed, 214 insertions, 72 deletions

diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/AbstractDeclarativeValidatorTest.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/AbstractDeclarativeValidatorTest.java
index 9bf1083..9eb2806 100644
--- a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/AbstractDeclarativeValidatorTest.java
+++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/AbstractDeclarativeValidatorTest.java
@@ -7,83 +7,82 @@
  *******************************************************************************/
 package org.eclipse.xtext.validator;
 
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
 
 import junit.framework.TestCase;
 
-import org.eclipse.emf.common.util.DiagnosticChain;
 import org.eclipse.emf.ecore.EClass;
-import org.eclipse.emf.ecore.EObject;
-import org.eclipse.emf.ecore.EStructuralFeature;
 import org.eclipse.emf.ecore.EcorePackage;
+import org.eclipse.xtext.validator.ValidationTestHelper.TestChain;
 
 /**
  * @author Sven Efftinge - Initial contribution and API
- * 
+ *
  */
 public class AbstractDeclarativeValidatorTest extends TestCase {
 
+	private ValidationTestHelper helper;
+
+	@Override
+	protected void setUp() throws Exception {
+		super.setUp();
+		this.helper = new ValidationTestHelper();
+	}
+
+	@Override
+	protected void tearDown() throws Exception {
+		this.helper = null;
+		super.tearDown();
+	}
+
 	public void testSimpleDispatch() throws Exception {
-		AbstractDeclarativeValidator test = new TestValidator();
-		TestChain chain = chain();
+		AbstractDeclarativeValidator test = helper.validator();
+		TestChain chain = helper.chain();
 		test.validate(EcorePackage.eINSTANCE.getEClass(), chain, null);
-		assertMatch(chain, 1, 3);
+		helper.assertMatch(chain, 1, 3);
 	}
 
 	public void testDeeperHierarchyWithOverwrittenJavaMethods() throws Exception {
-		AbstractDeclarativeValidator test = new TestValidator() {
+		AbstractDeclarativeValidator test = new ValidationTestHelper.TestValidator() {
 			@Override
 			@Check
 			public void foo(Object x) {
 				error("fooObject", 5);
 			}
 		};
-		TestChain chain = chain();
+		TestChain chain = helper.chain();
 		test.validate(EcorePackage.eINSTANCE.getEClass(), chain, null);
-		assertMatch(chain, 1, 5);
+		helper.assertMatch(chain, 1, 5);
 	}
 
 	@SuppressWarnings("serial")
 	public void testSkipExpensive() throws Exception {
-		AbstractDeclarativeValidator test = new TestValidator() {
+		AbstractDeclarativeValidator test = new ValidationTestHelper.TestValidator() {
 			@SuppressWarnings("unused")
 			@Check(CheckType.EXPENSIVE)
 			public void bar(Object x) {
 				error("fooObject", 27);
 			}
 		};
-		TestChain chain = chain();
+		TestChain chain = helper.chain();
 		test.validate(EcorePackage.eINSTANCE.getEClass(), chain, new HashMap<Object, Object>() {
 			{
 				put(CheckMode.KEY, CheckMode.NORMAL_AND_FAST);
 			}
 		});
-		assertMatch(chain, 1, 3);
+		helper.assertMatch(chain, 1, 3);
 
-		chain = chain();
+		chain = helper.chain();
 		test.validate(EcorePackage.eINSTANCE.getEClass(), chain, new HashMap<Object, Object>() {
 			{
 				put(CheckMode.KEY, CheckMode.ALL);
 			}
 		});
-		assertMatch(chain, 1, 3, 27);
+		helper.assertMatch(chain, 1, 3, 27);
 
-		chain = chain();
+		chain = helper.chain();
 		test.validate(EcorePackage.eINSTANCE.getEClass(), chain, null);
-		assertMatch(chain, 1, 3, 27);
-	}
-
-	private void assertMatch(TestChain chain, Integer... expectedFeatureIds) {
-		assertEquals(expectedFeatureIds.length, chain.integers.size());
-		List<Integer> asList = Arrays.asList(expectedFeatureIds);
-		assertTrue("expected : " + asList + " , but was " + chain.integers, chain.integers.containsAll(asList));
-	}
-
-	private TestChain chain() {
-		return new TestChain();
+		helper.assertMatch(chain, 1, 3, 27);
 	}
 
 	public void testGuard() throws Exception {
@@ -95,15 +94,15 @@ public class AbstractDeclarativeValidatorTest extends TestCase {
 			}
 		};
 
-		TestChain diagnostics = new TestChain();
+		TestChain diagnostics = helper.chain();
 		validator.validate(EcorePackage.eINSTANCE.getEClass(), diagnostics, null);
-		assertEquals(0, diagnostics.integers.size());
+		assertTrue(diagnostics.toString(), diagnostics.isEmpty());
 	}
 
 	@SuppressWarnings("serial")
 	public void testCheckModeSettedProperly() throws Exception {
-		AbstractDeclarativeValidator test = new TestValidator();
-		TestChain chain = chain();
+		AbstractDeclarativeValidator test = helper.validator();
+		TestChain chain = helper.chain();
 		try {
 			test.validate(EcorePackage.eINSTANCE.getEClass(), chain, new HashMap<Object, Object>() {
 				{
@@ -117,42 +116,4 @@ public class AbstractDeclarativeValidatorTest extends TestCase {
 		fail("CheckMode with wrong type, should throw an IllegalArgumentException");
 	}
 
-	@ComposedChecks(validators = { ExternalValidator.class })
-	private static class TestValidator extends AbstractDeclarativeValidator {
-		@Check
-		protected void foo(EStructuralFeature x) {
-			error("fooInteger", 2);
-		}
-
-		@Check
-		public void foo(Object x) {
-			error("fooObject", 3);
-		}
-	}
-
-	@ComposedChecks(validators = { TestValidator.class })
-	private static class ExternalValidator extends AbstractDeclarativeValidator {
-		@SuppressWarnings("unused")
-		@Check
-		private void foo(EClass x) {
-			error("fooString", 1);
-		}
-	}
-
-	private final class TestChain implements DiagnosticChain {
-		public List<Integer> integers = new ArrayList<Integer>();
-
-		public void add(org.eclipse.emf.common.util.Diagnostic diagnostic) {
-			assertTrue(diagnostic.getData().get(0) instanceof EObject);
-			integers.add((Integer) diagnostic.getData().get(1));
-		}
-
-		public void addAll(org.eclipse.emf.common.util.Diagnostic diagnostic) {
-			throw new UnsupportedOperationException();
-		}
-
-		public void merge(org.eclipse.emf.common.util.Diagnostic diagnostic) {
-			throw new UnsupportedOperationException();
-		}
-	}
 }
diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ConcurrentValidationTest.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ConcurrentValidationTest.java
new file mode 100644
index 0000000..9beea42
--- a/dev/null
+++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ConcurrentValidationTest.java
@@ -0,0 +1,93 @@
+/*******************************************************************************
+ * Copyright (c) 2009 itemis AG (http://www.itemis.eu) 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
+package org.eclipse.xtext.validator;
+
+import org.eclipse.emf.ecore.EStructuralFeature;
+import org.eclipse.emf.ecore.EcorePackage;
+import org.eclipse.xtext.junit.AbstractXtextTests;
+import org.eclipse.xtext.validator.ValidationTestHelper.TestChain;
+
+/**
+ * @author Sebastian Zarnekow - Initial contribution and API
+ */
+public class ConcurrentValidationTest extends AbstractXtextTests {
+
+	private ValidationTestHelper helper;
+
+	@Override
+	protected void tearDown() throws Exception {
+		helper = null;
+		super.tearDown();
+	}
+
+	@Override
+	protected void setUp() throws Exception {
+		super.setUp();
+		helper = new ValidationTestHelper();
+	}
+
+	public void testConcurrentValidation() {
+		AbstractDeclarativeValidator validator = new ValidationTestHelper.TestValidator() {
+			@Override
+			protected void foo(EStructuralFeature x) {
+				try {
+					Thread.sleep(50);
+					super.foo(x);
+					Thread.sleep(50);
+				}
+				catch (InterruptedException e) {
+					//
+				}
+			}
+
+			@Override
+			public void foo(Object x) {
+				try {
+					Thread.sleep(50);
+					super.foo(x);
+					Thread.sleep(50);
+				}
+				catch (InterruptedException e) {
+					//
+				}
+			}
+		};
+		PoorMansValidationJob runnable = new PoorMansValidationJob(validator);
+		for(int i = 0; i < 100; i++) {
+			new Thread(runnable).start();
+		}
+		try {
+			Thread.sleep(1000);
+		}
+		catch (InterruptedException e) {
+			//
+		}
+		assertNull(runnable.lastEx);
+	}
+
+	private class PoorMansValidationJob implements Runnable {
+
+		private final AbstractDeclarativeValidator validator;
+		private IllegalStateException lastEx;
+
+		private PoorMansValidationJob(AbstractDeclarativeValidator validator) {
+			this.validator = validator;
+		}
+
+		public void run() {
+			try {
+				TestChain diagnostics = helper.chain();
+				validator.validate(EcorePackage.eINSTANCE.getEClass(), diagnostics, null);
+				helper.assertMatch(diagnostics, 1, 3);
+			} catch(IllegalStateException e) {
+				lastEx = e;
+			}
+		}
+	}
+
+}
diff --git a/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ValidationTestHelper.java b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ValidationTestHelper.java
new file mode 100644
index 0000000..eb0f4e5
--- a/dev/null
+++ b/tests/org.eclipse.xtext.generator.tests/src/org/eclipse/xtext/validator/ValidationTestHelper.java
@@ -0,0 +1,88 @@
+/*******************************************************************************
+ * Copyright (c) 2009 itemis AG (http://www.itemis.eu) 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
+package org.eclipse.xtext.validator;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import junit.framework.Assert;
+
+import org.eclipse.emf.common.util.DiagnosticChain;
+import org.eclipse.emf.ecore.EClass;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.emf.ecore.EStructuralFeature;
+
+/**
+ * @author Sebastian Zarnekow - Initial contribution and API
+ */
+public class ValidationTestHelper {
+
+	public void assertMatch(TestChain chain, Integer... expectedFeatureIds) {
+		Assert.assertEquals(expectedFeatureIds.length, chain.integers.size());
+		List<Integer> asList = Arrays.asList(expectedFeatureIds);
+		Assert.assertTrue("expected : " + asList + " , but was " + chain.integers, asList.containsAll(chain.integers));
+	}
+
+	public TestChain chain() {
+		return new TestChain();
+	}
+
+	public TestValidator validator() {
+		return new TestValidator();
+	}
+
+	public final class TestChain implements DiagnosticChain {
+		private final List<Integer> integers = new ArrayList<Integer>();
+
+		public boolean isEmpty() {
+			return integers.isEmpty();
+		}
+
+		public void add(org.eclipse.emf.common.util.Diagnostic diagnostic) {
+			Assert.assertTrue(diagnostic.getData().get(0) instanceof EObject);
+			integers.add((Integer) diagnostic.getData().get(1));
+		}
+
+		public void addAll(org.eclipse.emf.common.util.Diagnostic diagnostic) {
+			throw new UnsupportedOperationException();
+		}
+
+		public void merge(org.eclipse.emf.common.util.Diagnostic diagnostic) {
+			throw new UnsupportedOperationException();
+		}
+
+		@Override
+		public String toString() {
+			return integers.toString();
+		}
+	}
+
+	@ComposedChecks(validators = { ExternalValidator.class })
+	public static class TestValidator extends AbstractDeclarativeValidator {
+		@Check
+		protected void foo(EStructuralFeature x) {
+			error("fooInteger", 2);
+		}
+
+		@Check
+		public void foo(Object x) {
+			error("fooObject", 3);
+		}
+	}
+
+	@ComposedChecks(validators = { TestValidator.class })
+	private static class ExternalValidator extends AbstractDeclarativeValidator {
+		@SuppressWarnings("unused")
+		@Check
+		private void foo(EClass x) {
+			error("fooString", 1);
+		}
+	}
+
+}