diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginReference.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginReference.java index 34d440d469b..525eb3bf143 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginReference.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/plugin/PluginReference.java @@ -41,11 +41,14 @@ public String getId() { } public IPlugin getPlugin() { - if (fPlugin == null && fId != null) { - IPluginModelBase model = findModel(); - fPlugin = model instanceof IPluginModel i ? i.getPlugin() : null; + // Only an explicitly assigned plug-in is cached. Looking the id up on + // every call keeps isResolved() in sync with the current target + // platform, which the error decorations in the editor rely on. + if (fPlugin != null || fId == null) { + return fPlugin; } - return fPlugin; + IPluginModelBase model = findModel(); + return model instanceof IPluginModel i ? i.getPlugin() : null; } protected IPluginModelBase findModel() { diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java index 17129fd3cd4..5b0ddf26898 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/AllPDECoreTests.java @@ -8,6 +8,7 @@ @SelectClasses({ // DependencyManagerTest.class, // DependencyLoopFinderTest.class, // + StaleDependencyResolutionTest.class, // WorkspaceModelManagerTest.class, // WorkspaceProductModelManagerTest.class, // }) diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/StaleDependencyResolutionTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/StaleDependencyResolutionTest.java new file mode 100644 index 00000000000..ebcee7f4513 --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/StaleDependencyResolutionTest.java @@ -0,0 +1,174 @@ +/******************************************************************************* + * Copyright (c) 2026 Lars Vogel and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Lars Vogel - initial API and implementation + *******************************************************************************/ +package org.eclipse.pde.core.tests.internal; + +import static java.util.Map.entry; +import static org.eclipse.pde.ui.tests.util.TargetPlatformUtil.bundle; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.osgi.framework.Constants.EXPORT_PACKAGE; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.eclipse.core.runtime.CoreException; +import org.eclipse.jface.text.Document; +import org.eclipse.osgi.service.resolver.State; +import org.eclipse.osgi.service.resolver.StateDelta; +import org.eclipse.pde.core.plugin.PluginRegistry; +import org.eclipse.pde.core.target.NameVersionDescriptor; +import org.eclipse.pde.internal.core.IStateDeltaListener; +import org.eclipse.pde.internal.core.PDECore; +import org.eclipse.pde.internal.core.PluginModelDelta; +import org.eclipse.pde.internal.core.PluginModelManager; +import org.eclipse.pde.internal.core.plugin.PluginReference; +import org.eclipse.pde.internal.core.text.bundle.BundleModel; +import org.eclipse.pde.internal.core.text.bundle.ImportPackageHeader; +import org.eclipse.pde.internal.core.text.bundle.ImportPackageObject; +import org.eclipse.pde.ui.tests.util.ProjectUtils; +import org.eclipse.pde.ui.tests.util.TargetPlatformUtil; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.TestRule; +import org.osgi.framework.Constants; + +/** + * Guards the Dependencies tab of the manifest editor against stale error + * decorations. The decorations are computed by {@code PDELabelProvider} from + * {@code isResolved()}, so they are only correct if the resolution result is + * recomputed and if the editor is notified that the target platform changed. + */ +public class StaleDependencyResolutionTest { + + @ClassRule + public static final TestRule RESTORE_TARGET_DEFINITION = TargetPlatformUtil.RESTORE_CURRENT_TARGET_DEFINITION_AFTER; + @ClassRule + public static final TestRule CLEAR_WORKSPACE = ProjectUtils.DELETE_ALL_WORKSPACE_PROJECTS_BEFORE_AND_AFTER; + + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + private Path targetWithBundleA; + private Path targetWithoutBundleA; + + @Before + public void setUp() throws IOException { + targetWithBundleA = folder.newFolder("targetWithBundleA").toPath(); + targetWithoutBundleA = folder.newFolder("targetWithoutBundleA").toPath(); + // ensure the PluginModelManager is initialized before listening to it + PluginModelManager.getInstance().getState(); + } + + /** + * A target reload never fires a {@link PluginModelDelta}, so the dependency + * sections cannot rely on plug-in model listeners alone. They also listen to + * {@link IStateDeltaListener#stateChanged}, which is the only notification a + * reload produces. + */ + @Test + public void testTargetReloadNotifiesStateListeners() throws Exception { + setTargetPlatform(targetWithBundleA, bundle("bundle.a", "1.0.0")); + + List states = new CopyOnWriteArrayList<>(); + IStateDeltaListener listener = new IStateDeltaListener() { + @Override + public void stateResolved(StateDelta delta) { + // not the notification the sections depend on for a reload + } + + @Override + public void stateChanged(State newState) { + states.add(newState); + } + }; + PluginModelManager manager = PDECore.getDefault().getModelManager(); + manager.addStateDeltaListener(listener); + try { + setTargetPlatform(targetWithoutBundleA, bundle("bundle.b", "1.0.0")); + + assertNull("precondition: bundle.a must be gone from the target", + PluginRegistry.findModel("bundle.a")); + assertFalse("target reload did not notify the state listeners", states.isEmpty()); + } finally { + manager.removeStateDeltaListener(listener); + } + } + + /** + * {@link PluginReference} must not memoize a looked-up plug-in. A required + * bundle that disappears from the target has to report itself as unresolved + * so the error decoration appears. + */ + @Test + public void testResolutionIsRecomputedWhenBundleLeavesTarget() throws Exception { + setTargetPlatform(targetWithBundleA, bundle("bundle.a", "1.0.0")); + + PluginReference reference = new PluginReference("bundle.a"); + assertTrue("precondition: bundle.a must resolve while it is in the target", reference.isResolved()); + + setTargetPlatform(targetWithoutBundleA, bundle("bundle.b", "1.0.0")); + + assertNull("precondition: bundle.a must be gone from the target", PluginRegistry.findModel("bundle.a")); + assertFalse("resolution result was not recomputed after bundle.a left the target", reference.isResolved()); + } + + /** + * The counterpart for imported packages. {@code ImportPackageObject} queries + * the live {@code PDEState} on every call, so its result follows a target + * change without any caching to invalidate. + */ + @Test + public void testImportedPackageResolutionFollowsTarget() throws Exception { + setTargetPlatform(targetWithBundleA, bundle("bundle.a", "1.0.0", entry(EXPORT_PACKAGE, "bundle.a.pack"))); + + ImportPackageObject importedPackage = importedPackage("bundle.a.pack"); + assertNotNull("precondition: bundle.a must be in the target", PluginRegistry.findModel("bundle.a")); + assertTrue("bundle.a.pack must resolve while its exporter is in the target", importedPackage.isResolved()); + + setTargetPlatform(targetWithoutBundleA, bundle("bundle.b", "1.0.0")); + + assertFalse("bundle.a.pack must not resolve after its exporter left the target", + importedPackage.isResolved()); + } + + private static ImportPackageObject importedPackage(String packageName) throws CoreException { + Document document = new Document(); + document.set(""" + Manifest-Version: 1.0 + Bundle-ManifestVersion: 2 + Bundle-SymbolicName: bundle.importer + Bundle-Version: 1.0.0 + Import-Package: %s + """.formatted(packageName)); + BundleModel model = new BundleModel(document, false); + model.load(); + ImportPackageHeader header = (ImportPackageHeader) model.getBundle() + .getManifestHeader(Constants.IMPORT_PACKAGE); + return header.getPackage(packageName); + } + + @SafeVarargs + private static void setTargetPlatform(Path jarDirectory, + Map.Entry>... bundles) throws Exception { + TargetPlatformUtil.setDummyBundlesAsTarget(Map.ofEntries(bundles), List.of(), jarDirectory); + } +} diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ImportPackageSection.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ImportPackageSection.java index 64019a4be10..4b0e6e1a98d 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ImportPackageSection.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/ImportPackageSection.java @@ -49,6 +49,7 @@ import org.eclipse.osgi.service.resolver.ExportPackageDescription; import org.eclipse.osgi.service.resolver.HostSpecification; import org.eclipse.osgi.service.resolver.State; +import org.eclipse.osgi.service.resolver.StateDelta; import org.eclipse.pde.core.IBaseModel; import org.eclipse.pde.core.IModel; import org.eclipse.pde.core.IModelChangedEvent; @@ -58,6 +59,11 @@ import org.eclipse.pde.core.target.NameVersionDescriptor; import org.eclipse.pde.internal.core.ClasspathUtilCore; import org.eclipse.pde.internal.core.ICoreConstants; +import org.eclipse.pde.internal.core.IPluginModelListener; +import org.eclipse.pde.internal.core.IStateDeltaListener; +import org.eclipse.pde.internal.core.PDECore; +import org.eclipse.pde.internal.core.PluginModelDelta; +import org.eclipse.pde.internal.core.PluginModelManager; import org.eclipse.pde.internal.core.TargetPlatformHelper; import org.eclipse.pde.internal.core.WorkspaceModelManager; import org.eclipse.pde.internal.core.bundle.BundlePluginBase; @@ -92,6 +98,7 @@ import org.eclipse.swt.graphics.Image; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Table; import org.eclipse.ui.IWorkingSet; @@ -105,7 +112,7 @@ import org.osgi.framework.Version; import org.osgi.resource.Resource; -public class ImportPackageSection extends TableSection { +public class ImportPackageSection extends TableSection implements IPluginModelListener, IStateDeltaListener { private static final int ADD_INDEX = 0; private static final int REMOVE_INDEX = 1; @@ -265,10 +272,49 @@ public int compare(Viewer viewer, Object e1, Object e2) { IBundleModel model = getBundleModel(); fPackageViewer.setInput(model); model.addModelChangedListener(this); - section.addDisposeListener(e -> model.removeModelChangedListener(ImportPackageSection.this)); + PluginModelManager modelManager = PDECore.getDefault().getModelManager(); + modelManager.addPluginModelListener(this); + modelManager.addStateDeltaListener(this); + section.addDisposeListener(e -> { + model.removeModelChangedListener(ImportPackageSection.this); + modelManager.removePluginModelListener(ImportPackageSection.this); + modelManager.removeStateDeltaListener(ImportPackageSection.this); + }); updateButtons(); } + @Override + public void modelsChanged(PluginModelDelta delta) { + refreshPackages(); + } + + @Override + public void stateResolved(StateDelta delta) { + // already covered by modelsChanged, which is fired for the same batch + } + + @Override + public void stateChanged(State newState) { + // a target reload replaces the state without firing a PluginModelDelta + refreshPackages(); + } + + /** + * Repaints the table so the resolution decorations match the current target + * platform. The imported packages themselves are unchanged, only their + * resolution status is recomputed. + */ + private void refreshPackages() { + Control control = fPackageViewer.getControl(); + if (!control.isDisposed()) { + control.getDisplay().asyncExec(() -> { + if (!control.isDisposed()) { + fPackageViewer.refresh(); + } + }); + } + } + @Override public boolean doGlobalAction(String actionId) { diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/RequiresSection.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/RequiresSection.java index 42f948db0f5..17f5320528f 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/RequiresSection.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/plugin/RequiresSection.java @@ -41,6 +41,8 @@ import org.eclipse.jface.viewers.TableViewer; import org.eclipse.jface.viewers.ViewerDropAdapter; import org.eclipse.jface.window.Window; +import org.eclipse.osgi.service.resolver.State; +import org.eclipse.osgi.service.resolver.StateDelta; import org.eclipse.pde.core.IModel; import org.eclipse.pde.core.IModelChangedEvent; import org.eclipse.pde.core.plugin.IPlugin; @@ -51,6 +53,7 @@ import org.eclipse.pde.core.plugin.IPluginModelFactory; import org.eclipse.pde.core.plugin.PluginRegistry; import org.eclipse.pde.internal.core.IPluginModelListener; +import org.eclipse.pde.internal.core.IStateDeltaListener; import org.eclipse.pde.internal.core.PDECore; import org.eclipse.pde.internal.core.PluginModelDelta; import org.eclipse.pde.internal.core.bundle.BundlePluginBase; @@ -91,7 +94,8 @@ import org.eclipse.ui.forms.widgets.Section; import org.eclipse.ui.progress.UIJob; -public class RequiresSection extends TableSection implements IPluginModelListener, IPropertyChangeListener { +public class RequiresSection extends TableSection + implements IPluginModelListener, IStateDeltaListener, IPropertyChangeListener { private static final int ADD_INDEX = 0; private static final int REMOVE_INDEX = 1; @@ -262,6 +266,7 @@ public void dispose() { model.removeModelChangedListener(this); } PDECore.getDefault().getModelManager().removePluginModelListener(this); + PDECore.getDefault().getModelManager().removeStateDeltaListener(this); super.dispose(); } @@ -546,6 +551,7 @@ private void addSystemBundle(java.util.List list) { public void initialize() { PDECore.getDefault().getModelManager().addPluginModelListener(this); + PDECore.getDefault().getModelManager().addStateDeltaListener(this); if (getPage().getModel() instanceof IPluginModelBase model) { fImportViewer.setInput(model.getPluginBase()); updateButtons(); @@ -672,6 +678,21 @@ public void modelChanged(final IModelChangedEvent event) { @Override public void modelsChanged(PluginModelDelta delta) { + refreshImports(); + } + + @Override + public void stateResolved(StateDelta delta) { + // already covered by modelsChanged, which is fired for the same batch + } + + @Override + public void stateChanged(State newState) { + // a target reload replaces the state without firing a PluginModelDelta + refreshImports(); + } + + private void refreshImports() { fImports = null; final Control control = fImportViewer.getControl(); if (!control.isDisposed()) {