diff --git a/bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF index 79b52fd9e9a..be8da055481 100644 --- a/bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.e4.ui.workbench/META-INF/MANIFEST.MF @@ -1,7 +1,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-SymbolicName: org.eclipse.e4.ui.workbench;singleton:=true -Bundle-Version: 1.18.300.qualifier +Bundle-Version: 1.18.400.qualifier Bundle-Name: %pluginName Bundle-Vendor: %providerName Bundle-Localization: plugin diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java index 581e1243588..3941fa5af01 100644 --- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java +++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelAssembler.java @@ -775,7 +775,20 @@ public void resolveImports(List imports, List { + if (element != null && !feature.getEType().isInstance(element)) { + // An import that resolves to a different-typed element (e.g. two + // elements sharing an elementId) is a contribution error, not a + // reason to abort assembling the whole application model. + warn("Skipping import for feature {} on {}: resolved element {} is not assignable to the feature type", //$NON-NLS-1$ + feature.getName(), target, element.getElementId()); + return; + } if (feature.isMany()) { + if (element == null) { + warn("Skipping unresolved import for many-valued feature {} on {}", //$NON-NLS-1$ + feature.getName(), target); + return; + } error(""" Replacing in {}. Feature={}. diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java index 7b658aab90d..c11f5b41c5b 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java @@ -17,6 +17,7 @@ package org.eclipse.e4.ui.tests.workbench; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import jakarta.annotation.PostConstruct; @@ -44,6 +45,7 @@ import org.eclipse.e4.ui.internal.workbench.swt.E4Application; import org.eclipse.e4.ui.model.application.MApplication; import org.eclipse.e4.ui.model.application.MApplicationElement; +import org.eclipse.e4.ui.model.application.commands.MCommand; import org.eclipse.e4.ui.model.application.impl.ApplicationFactoryImpl; import org.eclipse.e4.ui.model.application.ui.MUIElement; import org.eclipse.e4.ui.model.application.ui.advanced.MArea; @@ -340,6 +342,54 @@ public void testImports_noImportElementId() throws Exception { assertEquals("Could not resolve import for null", logMessages.poll()); } + /** + * Tests that an import resolving to an element of an incompatible type (two + * elements sharing an elementId) is skipped with a warning instead of aborting + * model assembly with a {@link ClassCastException}. See + * issue + * 4148. + */ + @Test + public void testImports_typeIncompatibleElement() throws Exception { + List imports = new ArrayList<>(); + List addedElements = new ArrayList<>(); + + final String sharedElementId = "testImports_typeIncompatible_id"; + + // The fragment imports a window with the shared id ... + MTrimmedWindow importWindow = modelService.createModelElement(MTrimmedWindow.class); + importWindow.setElementId(sharedElementId); + MModelFragments fragment = MFragmentFactory.INSTANCE.createModelFragments(); + fragment.getImports().add(importWindow); + imports.add(importWindow); + + // ... but the application only contains a command (a different type) that + // shares that id, so the id-based lookup resolves to a type-incompatible + // element for the MUIElement-typed placeholder reference. + MCommand collidingCommand = modelService.createModelElement(MCommand.class); + collidingCommand.setElementId(sharedElementId); + application.getCommands().add(collidingCommand); + + MPlaceholder placeholder = modelService.createModelElement(MPlaceholder.class); + placeholder.setRef(importWindow); + addedElements.add(placeholder); + + CountDownLatch countDownLatch = new CountDownLatch(1); + this.logListener.countDownLatch = countDownLatch; + + // Must not throw: the incompatible resolution is skipped, the reference is + // left untouched, and every other fragment still applies. + assembler.resolveImports(imports, addedElements); + + assertNotEquals(collidingCommand, placeholder.getRef()); + assertEquals(importWindow, placeholder.getRef()); + + boolean completed = countDownLatch.await(COUNTDOWN_TIMEOUT, TimeUnit.MILLISECONDS); + assertTrue(completed, "Timeout - no event received"); + assertEquals(1, logMessages.size()); + assertTrue(logMessages.poll().startsWith("Skipping import for feature")); + } + /** * Make sure that all fragments and imports are resolved before the * post-processors are run. For reference, see