Fix: API tool reports fictitious changes to execution environment

The API tool only checks the Bundle-RequiredExecutionEnvironment header
when comparing the baseline with the workspace bundle. If the EE of the
baseline bundle is described via the Require-Capability header, an empty
string is compared to the workspace EE, which is treated as an
incompatibility.

This also adds a simple test case where the execution environment of two
bundles is checked. One bundle uses the  Require-Capability, the other
one the Bundle-RequiredExecutionEnvironment header.

Resolves #1301

apitools/org.eclipse.pde.api.tools.tests/src/org/eclipse/pde/api/tools/builder/tests/compatibility/ProjectTypeContainerTests.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2018 IBM Corporation and others.
+ * Copyright (c) 2009, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.pde.api.tools.builder.tests.compatibility;
 
+import static org.junit.Assert.assertArrayEquals;
+
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -66,13 +68,13 @@ protected int getDefaultProblemId() {
 	}
 
 	/**
-	 * Returns the type container associated with the "bundle.a" project in the
+	 * Returns the type container associated with the given project in the
 	 * workspace.
 	 */
-	protected IApiTypeContainer getTypeContainer() throws CoreException {
+	protected IApiTypeContainer getTypeContainer(String projectName) throws CoreException {
 		IApiBaseline baseline = ApiBaselineManager.getManager().getWorkspaceBaseline();
 		assertNotNull("Missing workspace baseline", baseline); //$NON-NLS-1$
-		IApiComponent component = baseline.getApiComponent(getEnv().getProject("bundle.a")); //$NON-NLS-1$
+		IApiComponent component = baseline.getApiComponent(getEnv().getProject(projectName));
 		assertNotNull("Missing API component", component); //$NON-NLS-1$
 		IApiTypeContainer[] containers = component.getApiTypeContainers();
 		assertEquals("Wrong number of API type containers", 1, containers.length); //$NON-NLS-1$
@@ -154,11 +156,26 @@ protected Set<String> collectAllTypeNames() throws CoreException {
 		return names;
 	}
 
+	/**
+	 * Tests whether the execution environment can be extracted from both the
+	 * {@code Bundle-RequiredExecutionEnvironment} and the
+	 * {@code Require-Capability} header.
+	 */
+	public void testExecutionEnvironment() throws CoreException {
+		IApiTypeContainer bundleA = getTypeContainer("bundle.a"); //$NON-NLS-1$
+		assertArrayEquals("Unable to find BREE for bundle using 'Bundle-RequiredExecutionEvironment'", //$NON-NLS-1$
+				new String[] { "J2SE-1.5" }, bundleA.getApiComponent().getExecutionEnvironments()); //$NON-NLS-1$
+
+		IApiTypeContainer bundleB = getTypeContainer("bundle.b"); //$NON-NLS-1$
+		assertArrayEquals("Unable to find BREE for bundle using 'Require-Capability'", //$NON-NLS-1$
+				new String[] { "JavaSE-17" }, bundleB.getApiComponent().getExecutionEnvironments()); //$NON-NLS-1$
+	}
+
 	/**
 	 * Tests all packages are returned.
 	 */
 	public void testPackageNames() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		assertEquals("Should be a project type container", IApiTypeContainer.FOLDER, container.getContainerType()); //$NON-NLS-1$
 
 		// build expected list
@@ -184,7 +201,7 @@ public void testPackageNames() throws CoreException {
 	 * Test type lookup.
 	 */
 	public void testFindType() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		IApiTypeRoot root = container.findTypeRoot("a.classes.fields.AddPrivateField"); //$NON-NLS-1$
 		assertNotNull("Unable to find type 'a.classes.fields.AddPrivateField'", root); //$NON-NLS-1$
 		IApiType structure = root.getStructure();
@@ -195,7 +212,7 @@ public void testFindType() throws CoreException {
 	 * Test that type lookup fails for a type that is not in the project.
 	 */
 	public void testMissingType() throws CoreException {
-		IApiTypeContainer container = getTypeContainer();
+		IApiTypeContainer container = getTypeContainer("bundle.a"); //$NON-NLS-1$
 		IApiTypeRoot root = container.findTypeRoot("some.bogus.Type"); //$NON-NLS-1$
 		assertNull("Should not be able to find type 'some.bogus.Type'", root); //$NON-NLS-1$
 	}
@@ -218,7 +235,7 @@ public void visit(String packageName, IApiTypeRoot typeroot) {
 				typeNames.add(typeroot.getTypeName());
 			}
 		};
-		getTypeContainer().accept(visitor);
+		getTypeContainer("bundle.a").accept(visitor); //$NON-NLS-1$
 
 		// validate type names
 		Set<String> set = collectAllTypeNames();

apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.classpath

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
+	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
+	<classpathentry kind="src" path="src"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>

apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/.project

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>bundle.b</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.jdt.core.javabuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.ManifestBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.SchemaBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.pde.PluginNature</nature>
+		<nature>org.eclipse.jdt.core.javanature</nature>
+	</natures>
+</projectDescription>

apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/META-INF/MANIFEST.MF

@@ -0,0 +1,7 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: API Tools Tests Plug-in B
+Bundle-SymbolicName: bundle.b
+Bundle-Version: 1.0.0
+Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=17))"
+Export-Package: test

apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/build.properties

@@ -0,0 +1,4 @@
+source.. = src/
+output.. = bin/
+bin.includes = META-INF/,\
+               .

apitools/org.eclipse.pde.api.tools.tests/test-builder/baseline/bundle.b/src/test/Stub.java

@@ -0,0 +1,2 @@
+package test;
+public class Stub {}

apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/model/BundleComponent.java

@@ -29,7 +29,9 @@
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Stream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
@@ -43,9 +45,11 @@
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.Status;
+import org.eclipse.osgi.internal.framework.FilterImpl;
 import org.eclipse.osgi.service.resolver.BundleDescription;
 import org.eclipse.osgi.service.resolver.BundleSpecification;
 import org.eclipse.osgi.service.resolver.ExportPackageDescription;
+import org.eclipse.osgi.service.resolver.GenericSpecification;
 import org.eclipse.osgi.service.resolver.HostSpecification;
 import org.eclipse.osgi.service.resolver.ResolverError;
 import org.eclipse.osgi.service.resolver.State;
@@ -81,6 +85,7 @@
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
 import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.Version;
 import org.osgi.framework.namespace.ExecutionEnvironmentNamespace;
 import org.xml.sax.InputSource;
@@ -91,6 +96,7 @@
  *
  * @since 1.0.0
  */
+@SuppressWarnings("restriction") // org.eclipse.osgi.internal.framework.FilterImpl
 public class BundleComponent extends Component {
 
 	private static final String[] NO_EXECUTION_ENRIONMENTS = new String[0];
@@ -164,7 +170,7 @@ public class BundleComponent extends Component {
 	 * @see #getExecutionEnvironments()
 	 * @see #hasDeclaredRequiredEE(Map)
 	 */
-	private volatile boolean fHasDeclaredRequiredEE;
+	private volatile String[] fdeclaredRequiredEE;
 
 	/**
 	 * Constructs a new API component from the specified location in the file
@@ -306,7 +312,7 @@ protected void init() throws CoreException {
 				BundleDescription bundleDescription = getBundleDescription(manifest, fLocation, fBundleId);
 				fSymbolicName = bundleDescription.getSymbolicName();
 				fVersion = bundleDescription.getVersion();
-				fHasDeclaredRequiredEE = hasDeclaredRequiredEE(manifest);
+				fdeclaredRequiredEE = getDeclaredRequiredEE(bundleDescription);
 				setName(manifest.get(Constants.BUNDLE_NAME));
 				fBundleDescription = bundleDescription;
 			} catch (BundleException e) {
@@ -883,7 +889,7 @@ private static InputStream loadFixedBundleApiDescription(String bundleAndVersion
 	public String[] getExecutionEnvironments() throws CoreException {
 		// Return the EE from the description only if explicitly specified in the
 		// manifest.
-		return fHasDeclaredRequiredEE ? getBundleDescription().getExecutionEnvironments() : NO_EXECUTION_ENRIONMENTS;
+		return fdeclaredRequiredEE;
 	}
 
 	@Override
@@ -1199,27 +1205,57 @@ protected void baselineDisposed(IApiBaseline baseline) throws CoreException {
 				NLS.bind(Messages.BundleApiComponent_baseline_disposed, getName(), baseline.getName()), disposeSource));
 	}
 
-	/*
-	 * This is a copy of
-	 * org.eclipse.pde.internal.core.MinimalState.hasDeclaredRequiredEE(Map<String,
-	 * String>). PDE ends up adding a synthetic EE to the manifest when that method
-	 * returns false, and that ends up in the bundle description such that we cannot
-	 * determine whether the EE was actually present or was synthesized. So we
-	 * repeat that computation here
+	/**
+	 * The required execution environments are either declared via the filter
+	 * attribute of the generic capabilities or directly via the (deprecated)
+	 * header.
+	 *
+	 * @see {@link #getEEFromFilter(String)}
+	 * @see Constants#BUNDLE_REQUIREDEXECUTIONENVIRONMENT
+	 * @see Constants#REQUIRE_CAPABILITY
 	 */
-	@SuppressWarnings("deprecation")
-	private boolean hasDeclaredRequiredEE(Map<String, String> manifest) {
-		if (manifest.containsKey(Constants.BUNDLE_REQUIREDEXECUTIONENVIRONMENT)) {
-			return true;
+	private static String[] getDeclaredRequiredEE(BundleDescription bundleDescription) {
+		// Bundle-RequiredExecutionEnvironment
+		String[] requiredEE = bundleDescription.getExecutionEnvironments();
+		if (requiredEE.length > 0) {
+			return requiredEE;
+		}
+		// Require-Capability
+		GenericSpecification[] genericRequires = bundleDescription.getGenericRequires();
+		if (genericRequires == null) {
+			return NO_EXECUTION_ENRIONMENTS;
+		}
+		return Stream.of(genericRequires) //
+				.map(GenericSpecification::getMatchingFilter) //
+				.map(BundleComponent::getEEFromFilter) //
+				.filter(Objects::nonNull) //
+				.toArray(String[]::new);
+	}
+
+	/**
+	 * Extracts the execution environment of the given filter specification using
+	 * the environment namespace (e.g. JavaSE) and version attribute (e.g. 17). Both
+	 * values are joined with a '-'. If the filter doesn't contain either value or
+	 * if the filter is otherwise invalid, {@code null} is returned.
+	 *
+	 * @see ExecutionEnvironmentNamespace#EXECUTION_ENVIRONMENT_NAMESPACE
+	 * @see ExecutionEnvironmentNamespace#CAPABILITY_VERSION_ATTRIBUTE
+	 */
+	private static String getEEFromFilter(String filterSpec) {
+		if (filterSpec == null) {
+			return null;
 		}
 		try {
-			String capability = manifest.get(Constants.REQUIRE_CAPABILITY);
-			ManifestElement[] header = ManifestElement.parseHeader(Constants.REQUIRE_CAPABILITY, capability);
-			return header != null && Arrays.stream(header).map(ManifestElement::getValue)
-					.anyMatch(ExecutionEnvironmentNamespace.EXECUTION_ENVIRONMENT_NAMESPACE::equals);
-		} catch (BundleException e) {
-			return false; // ignore
+			FilterImpl filter = FilterImpl.newInstance(filterSpec);
+			String ee = filter.getPrimaryKeyValue(ExecutionEnvironmentNamespace.EXECUTION_ENVIRONMENT_NAMESPACE);
+			String version = filter.getPrimaryKeyValue(ExecutionEnvironmentNamespace.CAPABILITY_VERSION_ATTRIBUTE);
+			if (ee != null && version != null) {
+				return ee + '-' + version;
+			}
+		} catch (InvalidSyntaxException e) {
+			// Already validated by createOSGiRequires(...)
 		}
+		return null;
 	}
 
 }