Skip to content

Commit 0236660

Browse files
committed
Add regression test for type-incompatible fragment import
ModelAssembler.resolveImports resolves fragment <imports> by elementId only, ignoring the element type, and eSets whatever it finds onto the target reference. When a fragment imports an element whose id collides with a different-typed element in the model, the resolved element is not assignable to the reference and model assembly aborts with a ClassCastException. This test reproduces the crash by importing a window whose id matches an unrelated command that already exists in the application model. See #4148
1 parent a217bfc commit 0236660

1 file changed

Lines changed: 50 additions & 0 deletions

File tree

tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/ModelAssemblerTests.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.eclipse.e4.ui.tests.workbench;
1818

1919
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
2021
import static org.junit.jupiter.api.Assertions.assertTrue;
2122

2223
import jakarta.annotation.PostConstruct;
@@ -44,6 +45,7 @@
4445
import org.eclipse.e4.ui.internal.workbench.swt.E4Application;
4546
import org.eclipse.e4.ui.model.application.MApplication;
4647
import org.eclipse.e4.ui.model.application.MApplicationElement;
48+
import org.eclipse.e4.ui.model.application.commands.MCommand;
4749
import org.eclipse.e4.ui.model.application.impl.ApplicationFactoryImpl;
4850
import org.eclipse.e4.ui.model.application.ui.MUIElement;
4951
import org.eclipse.e4.ui.model.application.ui.advanced.MArea;
@@ -340,6 +342,54 @@ public void testImports_noImportElementId() throws Exception {
340342
assertEquals("Could not resolve import for null", logMessages.poll());
341343
}
342344

345+
/**
346+
* Tests that an import resolving to an element of an incompatible type (two
347+
* elements sharing an elementId) is skipped with a warning instead of aborting
348+
* model assembly with a {@link ClassCastException}. See
349+
* <a href="https://github.com/eclipse-platform/eclipse.platform.ui/issues/4148">issue
350+
* 4148</a>.
351+
*/
352+
@Test
353+
public void testImports_typeIncompatibleElement() throws Exception {
354+
List<MApplicationElement> imports = new ArrayList<>();
355+
List<MApplicationElement> addedElements = new ArrayList<>();
356+
357+
final String sharedElementId = "testImports_typeIncompatible_id";
358+
359+
// The fragment imports a window with the shared id ...
360+
MTrimmedWindow importWindow = modelService.createModelElement(MTrimmedWindow.class);
361+
importWindow.setElementId(sharedElementId);
362+
MModelFragments fragment = MFragmentFactory.INSTANCE.createModelFragments();
363+
fragment.getImports().add(importWindow);
364+
imports.add(importWindow);
365+
366+
// ... but the application only contains a command (a different type) that
367+
// shares that id, so the id-based lookup resolves to a type-incompatible
368+
// element for the MUIElement-typed placeholder reference.
369+
MCommand collidingCommand = modelService.createModelElement(MCommand.class);
370+
collidingCommand.setElementId(sharedElementId);
371+
application.getCommands().add(collidingCommand);
372+
373+
MPlaceholder placeholder = modelService.createModelElement(MPlaceholder.class);
374+
placeholder.setRef(importWindow);
375+
addedElements.add(placeholder);
376+
377+
CountDownLatch countDownLatch = new CountDownLatch(1);
378+
this.logListener.countDownLatch = countDownLatch;
379+
380+
// Must not throw: the incompatible resolution is skipped, the reference is
381+
// left untouched, and every other fragment still applies.
382+
assembler.resolveImports(imports, addedElements);
383+
384+
assertNotEquals(collidingCommand, placeholder.getRef());
385+
assertEquals(importWindow, placeholder.getRef());
386+
387+
boolean completed = countDownLatch.await(COUNTDOWN_TIMEOUT, TimeUnit.MILLISECONDS);
388+
assertTrue(completed, "Timeout - no event received");
389+
assertEquals(1, logMessages.size());
390+
assertTrue(logMessages.poll().startsWith("Skipping import for feature"));
391+
}
392+
343393
/**
344394
* Make sure that all fragments and imports are resolved before the
345395
* post-processors are run. For reference, see

0 commit comments

Comments
 (0)