Bug 533827 - Infinite loop in SettingToControlMapperImpl
Based on the fix provided by Remi (https://git.eclipse.org/r/#/c/121455/
).
This includes tests for the infinitive loop as well as the missing
removal of the parent from all setting.
Change-Id: I78609be11cf5dd389977279ceb455f7c0d65188a
Signed-off-by: Eugen Neufeld <eneufeld@eclipsesource.com>
diff --git a/bundles/org.eclipse.emfforms.core.services/src/org/eclipse/emfforms/internal/core/services/controlmapper/SettingToControlMapperImpl.java b/bundles/org.eclipse.emfforms.core.services/src/org/eclipse/emfforms/internal/core/services/controlmapper/SettingToControlMapperImpl.java
index 6ea3e5b..c21dccd 100644
--- a/bundles/org.eclipse.emfforms.core.services/src/org/eclipse/emfforms/internal/core/services/controlmapper/SettingToControlMapperImpl.java
+++ b/bundles/org.eclipse.emfforms.core.services/src/org/eclipse/emfforms/internal/core/services/controlmapper/SettingToControlMapperImpl.java
@@ -373,18 +373,20 @@
* @param controlToRemove The {@link VControl} that will be removed from this setting to control mapper
*/
private void vControlParentsRemoved(EMFFormsViewContext childContext, VControl controlToRemove) {
- VElement parentElement = contextParentMap.get(childContext);
- while (parentElement != null) {
- for (final UniqueSetting setting : controlToSettingMap.get(controlToRemove)) {
- settingToControlMap.get(setting).remove(parentElement);
- final EMFFormsViewContext context = controlContextMap.get(parentElement);
- if (context == null) {
- parentElement = null;
- break;
- }
- parentElement = contextParentMap.get(context);
- }
+ if (childContext == null) {
+ return;
}
+ final VElement parentElement = contextParentMap.get(childContext);
+ if (parentElement == null) {
+ return;
+ }
+ // remove the mapping of each setting of the removed control to the parent element
+ for (final UniqueSetting setting : controlToSettingMap.get(controlToRemove)) {
+ settingToControlMap.get(setting).remove(parentElement);
+ }
+ // Then compute the parent of the parent element
+ final EMFFormsViewContext parentContext = controlContextMap.get(parentElement);
+ vControlParentsRemoved(parentContext, controlToRemove);
}
/**
diff --git a/tests/org.eclipse.emfforms.core.services.tests/src/org/eclipse/emfforms/internal/core/services/scoped/SettingToControlMapper_ITest.java b/tests/org.eclipse.emfforms.core.services.tests/src/org/eclipse/emfforms/internal/core/services/scoped/SettingToControlMapper_ITest.java
index 246c24a..848a5af 100644
--- a/tests/org.eclipse.emfforms.core.services.tests/src/org/eclipse/emfforms/internal/core/services/scoped/SettingToControlMapper_ITest.java
+++ b/tests/org.eclipse.emfforms.core.services.tests/src/org/eclipse/emfforms/internal/core/services/scoped/SettingToControlMapper_ITest.java
@@ -13,6 +13,10 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.same;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import java.text.MessageFormat;
import java.util.ArrayList;
@@ -39,6 +43,8 @@
import org.eclipse.emfforms.spi.core.services.mappingprovider.EMFFormsMappingProviderManager;
import org.junit.Before;
import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
public class SettingToControlMapper_ITest {
@@ -49,6 +55,7 @@
private EObject domainObject;
private VControl control;
private FakeViewContext context;
+ private EMFFormsMappingProviderManager mappingManager;
@Before
public void setup() {
@@ -58,17 +65,25 @@
view.getChildren().add(control);
domainObject = EcoreUtil.create(EcorePackage.eINSTANCE.getEClass());
context = new FakeViewContext(domainObject, view);
- mapper = new SettingToControlMapperImpl(
- new EMFFormsMappingProviderManager() {
+
+ mappingManager = mock(EMFFormsMappingProviderManager.class);
+ when(mappingManager.getAllSettingsFor(any(VDomainModelReference.class), any(EObject.class)))
+ .then(new Answer<Set<UniqueSetting>>() {
+
@Override
- public Set<UniqueSetting> getAllSettingsFor(VDomainModelReference domainModelReference,
- EObject domainObject) {
+ public Set<UniqueSetting> answer(InvocationOnMock invocation) throws Throwable {
final Set<UniqueSetting> settings = new LinkedHashSet<UniqueSetting>();
+ final Object[] arguments = invocation.getArguments();
settings.add(
- UniqueSetting.createSetting(domainObject, EcorePackage.eINSTANCE.getEClass_EAttributes()));
+ UniqueSetting.createSetting((EObject) arguments[1],
+ EcorePackage.eINSTANCE.getEClass_EAttributes()));
+ settings.add(
+ UniqueSetting.createSetting((EObject) arguments[1],
+ EcorePackage.eINSTANCE.getEClass_EReferences()));
return settings;
}
- }, context);
+ });
+ mapper = new SettingToControlMapperImpl(mappingManager, context);
}
@Test
@@ -154,27 +169,31 @@
final FakeViewContext childContext = new FakeViewContext(childDomainObject, childView);
context.addChildContext(control, childContext);
+ final UniqueSetting childSettingAttributes = UniqueSetting.createSetting(childDomainObject,
+ EcorePackage.eINSTANCE.getEClass_EAttributes());
+ final UniqueSetting childSettingReferences = UniqueSetting.createSetting(childDomainObject,
+ EcorePackage.eINSTANCE.getEClass_EReferences());
+ assertEquals(2, mapper.getControlsFor(childSettingAttributes).size());
+ assertEquals(2, mapper.getControlsFor(childSettingReferences).size());
+
// Verification that add control and add child context work is done by other test cases.
mapper.childContextDisposed(childContext);
- final UniqueSetting setting = UniqueSetting.createSetting(childDomainObject,
- EcorePackage.eINSTANCE.getEClass_EAttributes());
-
// Verify that the mapping for the child setting is empty
final Set<UniqueSetting> settingsForControl = mapper.getSettingsForControl(childControl);
assertEquals(0, settingsForControl.size());
// Verify that after disposing the child context, the mapping from setting to parent control is also removed
- final Set<VElement> controlsForSetting = mapper.getControlsFor(setting);
- assertEquals(0, controlsForSetting.size());
+ assertEquals(0, mapper.getControlsFor(childSettingAttributes).size());
+ assertEquals(0, mapper.getControlsFor(childSettingReferences).size());
final UniqueSetting parentSetting = UniqueSetting.createSetting(domainObject,
EcorePackage.eINSTANCE.getEClass_EAttributes());
// Verify that disposing a child context does not clear the mapping of the parent's control
final Set<UniqueSetting> settingsForParentControl = mapper.getSettingsForControl(control);
- assertEquals(1, settingsForParentControl.size());
+ assertEquals(2, settingsForParentControl.size());
assertTrue(settingsForParentControl.contains(parentSetting));
// Verify that disposing a child context does not clear the mapping of the parent's setting
@@ -242,4 +261,40 @@
// half a millisecond should be more then enough time
assertTrue(MessageFormat.format("Duration was {0}.", durationYs), durationYs < 500); //$NON-NLS-1$
}
+
+ @Test
+ public void childContextDisposed_533827_InfiniteLoop() {
+ // Add parent control to later verify that it is not illegally removed during child context disposal
+ mapper.vControlAdded(control);
+
+ // Setup and add child context with one control and one setting
+ final VView childView = VViewFactory.eINSTANCE.createView();
+ final VControl childControl = VViewFactory.eINSTANCE.createControl();
+ childControl.setDomainModelReference(EcorePackage.eINSTANCE.getEClass_EAttributes());
+ childView.getChildren().add(childControl);
+ final EClass childDomainObject = EcoreFactory.eINSTANCE.createEClass();
+ childDomainObject.setName("child"); //$NON-NLS-1$
+ final Set<UniqueSetting> uniqueSettings = Collections.emptySet();
+ when(mappingManager.getAllSettingsFor(same(childControl.getDomainModelReference()), any(EObject.class)))
+ .thenReturn(uniqueSettings);
+ final FakeViewContext childContext = new FakeViewContext(childDomainObject, childView);
+ context.addChildContext(control, childContext);
+
+ // Verification that add control and add child context work is done by other test cases.
+ // mapper.vControlRemoved(childControl);
+ mapper.childContextDisposed(childContext);
+
+ final UniqueSetting parentSetting = UniqueSetting.createSetting(domainObject,
+ EcorePackage.eINSTANCE.getEClass_EAttributes());
+
+ // Verify that disposing a child context does not clear the mapping of the parent's control
+ final Set<UniqueSetting> settingsForParentControl = mapper.getSettingsForControl(control);
+ assertEquals(2, settingsForParentControl.size());
+ assertTrue(settingsForParentControl.contains(parentSetting));
+
+ // Verify that disposing a child context does not clear the mapping of the parent's setting
+ final Set<VElement> controlsForParentSetting = mapper.getControlsFor(parentSetting);
+ assertEquals(1, controlsForParentSetting.size());
+ assertTrue(controlsForParentSetting.contains(control));
+ }
}
