Simplify Installable-Unit handling in IUBundleContainer

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/IUBundleContainer.java

@@ -22,20 +22,19 @@
 import java.io.StringWriter;
 import java.net.URI;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.IntStream;
-import java.util.stream.Stream;
 
 import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
@@ -120,21 +119,14 @@ public class IUBundleContainer extends AbstractBundleContainer {
 	 */
 	public static final int FOLLOW_REPOSITORY_REFERENCES = 1 << 4;
 
-	/**
-	 * IU identifiers.
-	 */
-	private String[] fIds;
-
-	/**
-	 * IU versions
-	 */
-	private Version[] fVersions;
+	/** The list of id+version of all root units declared in this container. */
+	private final Set<IVersionedId> fIUs;
 
 	/**
 	 * Cached IU's referenced by this bundle container, or <code>null</code> if not
 	 * resolved.
 	 */
-	private IInstallableUnit[] fUnits;
+	private List<IInstallableUnit> fUnits = List.of();
 
 	/**
 	 * Repositories to consider, empty if default.
@@ -161,10 +153,8 @@ public class IUBundleContainer extends AbstractBundleContainer {
 	/**
 	 * Constructs a installable unit bundle container for the specified units.
 	 *
-	 * @param ids
-	 *            IU identifiers
-	 * @param versions
-	 *            IU versions
+	 * @param units
+	 *            the units composed of their identifier and version
 	 * @param repositories
 	 *            metadata repositories used to search for IU's or
 	 *            <code>null</code> for default set
@@ -176,11 +166,10 @@ public class IUBundleContainer extends AbstractBundleContainer {
 	 *            {@link IUBundleContainer#INCLUDE_CONFIGURE_PHASE},
 	 *            {@link IUBundleContainer#FOLLOW_REPOSITORY_REFERENCES}
 	 */
-	IUBundleContainer(String[] ids, Version[] versions, URI[] repositories, int resolutionFlags) {
-		fIds = ids;
+	IUBundleContainer(List<IVersionedId> units, List<URI> repositories, int resolutionFlags) {
+		fIUs = new LinkedHashSet<>(units);
 		fFlags = resolutionFlags;
-		fVersions = versions;
-		fRepos = repositories == null ? List.of() : List.of(repositories);
+		fRepos = List.copyOf(repositories);
 	}
 
 	@Override
@@ -277,15 +266,15 @@ private void cacheIUs() throws CoreException {
 		IProfile profile = fSynchronizer.getProfile();
 		List<IInstallableUnit> result = new ArrayList<>();
 		MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories, null);
-		for (int i = 0; i < fIds.length; i++) {
-			IQuery<IInstallableUnit> query = QueryUtil.createIUQuery(fIds[i], fVersions[i]);
-			addQueryResult(profile, query, i, result, status);
+		for (IVersionedId unit : fIUs) {
+			IQuery<IInstallableUnit> query = QueryUtil.createIUQuery(unit);
+			addQueryResult(profile, query, unit, result, status);
 		}
 		if (!status.isOK()) {
 			fResolutionStatus = status;
 			throw new CoreException(status);
 		}
-		fUnits = result.toArray(new IInstallableUnit[result.size()]);
+		fUnits = List.copyOf(result);
 	}
 
 	/**
@@ -300,7 +289,7 @@ private void cacheBundles(ITargetDefinition target) throws CoreException {
 		boolean onlyStrict = !fSynchronizer.getIncludeAllRequired();
 		IProfile metadata = fSynchronizer.getProfile();
 		PermissiveSlicer slicer = new PermissiveSlicer(metadata, new HashMap<>(), true, false, true, onlyStrict, false);
-		IQueryable<IInstallableUnit> slice = slicer.slice(Arrays.asList(fUnits), new NullProgressMonitor());
+		IQueryable<IInstallableUnit> slice = slicer.slice(fUnits, new NullProgressMonitor());
 
 		if (slicer.getStatus().getSeverity() == IStatus.ERROR) {
 			// If the slicer has an error, report it instead of returning an empty set
@@ -377,30 +366,29 @@ public synchronized IUBundleContainer update(Set<String> toUpdate, IProgressMoni
 		IQueryable<IInstallableUnit> source = P2TargetUtils.getQueryableMetadata(fRepos, isFollowRepositoryReferences(),
 				progress.split(30));
 		boolean updated = false;
-		String[] updateIDs = fIds.clone();
-		Version[] updateVersions = fVersions.clone();
-		SubMonitor loopProgress = progress.split(70).setWorkRemaining(updateIDs.length);
-		for (int i = 0; i < updateIDs.length; i++) {
-			String id = updateIDs[i];
-			if (!toUpdate.isEmpty() && !toUpdate.contains(id)) {
+		List<IVersionedId> updatedUnits = new ArrayList<>(fIUs);
+		SubMonitor loopProgress = progress.split(70).setWorkRemaining(updatedUnits.size());
+		for (int i = 0; i < updatedUnits.size(); i++) {
+			IVersionedId unit = updatedUnits.get(i);
+			if (!toUpdate.isEmpty() && !toUpdate.contains(unit.getId())) {
 				continue;
 			}
-			IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(id));
+			IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(unit.getId()));
 			Optional<IInstallableUnit> queryResult = queryFirst(source, query, loopProgress.split(1));
 			Version updatedVersion = queryResult.map(IInstallableUnit::getVersion)
 					// bail if the feature is no longer available.
-					.orElseThrow(() -> new CoreException(Status.error(NLS.bind(Messages.IUBundleContainer_1, id))));
+					.orElseThrow(() -> new CoreException(Status.error(NLS.bind(Messages.IUBundleContainer_1, unit))));
 			// if the version is different from the spec (up or down), record the change.
-			if (!updatedVersion.equals(updateVersions[i])) {
+			if (!updatedVersion.equals(unit.getVersion())) {
 				updated = true;
 				// if the spec was not specific (e.g., 0.0.0) the target def itself has changed.
-				if (!updateVersions[i].equals(Version.emptyVersion)) {
-					updateVersions[i] = updatedVersion;
+				if (!unit.getVersion().equals(Version.emptyVersion)) {
+					updatedUnits.set(i, new VersionedId(unit.getId(), updatedVersion));
 				}
 			}
 		}
 		if (updated) {
-			return new IUBundleContainer(updateIDs, updateVersions, fRepos.toArray(URI[]::new), fFlags);
+			return new IUBundleContainer(updatedUnits, fRepos, fFlags);
 		}
 		return null;
 	}
@@ -466,7 +454,7 @@ private void generateBundle(IInstallableUnit unit, IFileArtifactRepository repo,
 	@Override
 	public int hashCode() {
 		return Objects.hash(getIncludeAllRequired(), getIncludeAllEnvironments(), getIncludeSource(),
-				getIncludeConfigurePhase(), Arrays.asList(fIds), Arrays.asList(fVersions), fRepos);
+				getIncludeConfigurePhase(), fIUs, fRepos);
 	}
 
 	@Override
@@ -479,8 +467,7 @@ && getIncludeAllRequired() == other.getIncludeAllRequired()
 				&& getIncludeAllEnvironments() == other.getIncludeAllEnvironments()
 				&& getIncludeSource() == other.getIncludeSource()
 				&& getIncludeConfigurePhase() == other.getIncludeConfigurePhase() //
-				&& Arrays.equals(fIds, other.fIds) //
-				&& Arrays.equals(fVersions, other.fVersions) //
+				&& fIUs.equals(other.fIUs) //
 				&& fRepos.equals(other.fRepos);
 	}
 
@@ -500,17 +487,7 @@ public List<URI> getRepositories() {
 	 * @param unit unit to remove from the list of root IUs
 	 */
 	public synchronized void removeInstallableUnit(IInstallableUnit unit) {
-		List<String> newIds = new ArrayList<>(fIds.length);
-		List<Version> newVersions = new ArrayList<>(fIds.length);
-		for (int i = 0; i < fIds.length; i++) {
-			if (!fIds[i].equals(unit.getId()) || !fVersions[i].equals(unit.getVersion())) {
-				newIds.add(fIds[i]);
-				newVersions.add(fVersions[i]);
-			}
-		}
-		fIds = newIds.toArray(new String[newIds.size()]);
-		fVersions = newVersions.toArray(new Version[newVersions.size()]);
-
+		fIUs.remove(new VersionedId(unit.getId(), unit.getVersion()));
 		// Need to mark the container as unresolved
 		clearResolutionStatus();
 	}
@@ -597,28 +574,12 @@ public boolean isFollowRepositoryReferences() {
 	 * @exception CoreException if unable to retrieve IU's
 	 */
 	public List<IInstallableUnit> getInstallableUnits() throws CoreException {
-		if (fUnits == null) {
-			return List.of();
-		}
-		return Arrays.asList(fUnits);
+		return fUnits;
 	}
 
-	/**
-	 * Returns installable unit identifiers.
-	 *
-	 * @return IU id's
-	 */
-	String[] getIds() {
-		return fIds;
-	}
-
-	/**
-	 * Returns installable unit versions.
-	 *
-	 * @return IU versions
-	 */
-	Version[] getVersions() {
-		return fVersions;
+	/** Returns the declared installable unit identifiers and versions. */
+	Collection<IVersionedId> getUnits() {
+		return Collections.unmodifiableSet(fIUs);
 	}
 
 	/**
@@ -701,10 +662,7 @@ public String serialize() {
 			containerElement.appendChild(repo);
 		}
 		// Generate a predictable order of the elements
-		String[] ids = getIds();
-		Version[] versions = getVersions();
-		Stream<VersionedId> ius = IntStream.range(0, fIds.length).mapToObj(i -> new VersionedId(ids[i], versions[i]));
-		ius.sorted(BY_ID_THEN_VERSION).forEach(iu -> {
+		fIUs.stream().sorted(BY_ID_THEN_VERSION).forEach(iu -> {
 			Element unit = document.createElement(TargetDefinitionPersistenceHelper.INSTALLABLE_UNIT);
 			unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_ID, iu.getId());
 			unit.setAttribute(TargetDefinitionPersistenceHelper.ATTR_VERSION, iu.getVersion().toString());
@@ -729,10 +687,10 @@ IInstallableUnit[] getRootIUs(IProgressMonitor monitor) throws CoreException {
 				isFollowRepositoryReferences(), monitor);
 		MultiStatus status = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.IUBundleContainer_ProblemsLoadingRepositories, null);
 		List<IInstallableUnit> result = new ArrayList<>();
-		for (int j = 0; j < fIds.length; j++) {
+		for (IVersionedId iu : fIUs) {
 			// For versions such as 0.0.0, the IU query may return multiple IUs, so we check which is the latest version
-			IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(fIds[j], fVersions[j]));
-			addQueryResult(repos, query, j, result, status);
+			IQuery<IInstallableUnit> query = QueryUtil.createLatestQuery(QueryUtil.createIUQuery(iu));
+			addQueryResult(repos, query, iu, result, status);
 		}
 		if (!status.isOK()) {
 			fResolutionStatus = status;
@@ -741,11 +699,11 @@ IInstallableUnit[] getRootIUs(IProgressMonitor monitor) throws CoreException {
 		return result.toArray(new IInstallableUnit[0]);
 	}
 
-	private void addQueryResult(IQueryable<IInstallableUnit> queryable, IQuery<IInstallableUnit> query, int iuIndex,
+	private void addQueryResult(IQueryable<IInstallableUnit> queryable, IQuery<IInstallableUnit> query, IVersionedId iu,
 			List<IInstallableUnit> result, MultiStatus status) {
 		Optional<IInstallableUnit> queryResult = queryFirst(queryable, query, null);
 		if (queryResult.isEmpty()) {
-			status.add(Status.error(NLS.bind(Messages.IUBundleContainer_1, fIds[iuIndex] + " " + fVersions[iuIndex]))); //$NON-NLS-1$
+			status.add(Status.error(NLS.bind(Messages.IUBundleContainer_1, iu)));
 		} else {
 			result.add(queryResult.get());
 		}

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2023 EclipseSource Inc. and others.
+ * Copyright (c) 2010, 2024 EclipseSource Inc. and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -66,6 +66,7 @@
 import org.eclipse.equinox.p2.metadata.IInstallableUnit;
 import org.eclipse.equinox.p2.metadata.IProvidedCapability;
 import org.eclipse.equinox.p2.metadata.IRequirement;
+import org.eclipse.equinox.p2.metadata.IVersionedId;
 import org.eclipse.equinox.p2.metadata.MetadataFactory;
 import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription;
 import org.eclipse.equinox.p2.metadata.Version;
@@ -539,11 +540,9 @@ private boolean checkProfile(ITargetDefinition target, final IProfile profile) {
 		}
 		for (ITargetLocation container : containers) {
 			if (container instanceof IUBundleContainer bc) {
-				String[] ids = bc.getIds();
-				Version[] versions = bc.getVersions();
-				for (int j = 0; j < versions.length; j++) {
+				for (IVersionedId iu : bc.getUnits()) {
 					// if there is something in a container but not in the profile, recreate
-					if (!installedIUs.remove(new NameVersionDescriptor(ids[j], versions[j].toString()))) {
+					if (!installedIUs.remove(new NameVersionDescriptor(iu.getId(), iu.getVersion().toString()))) {
 						return false;
 					}
 				}

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2020 IBM Corporation and others.
+ * Copyright (c) 2008, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -24,6 +24,7 @@
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -35,6 +36,7 @@
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
 import org.eclipse.core.resources.IFile;
@@ -59,7 +61,8 @@
 import org.eclipse.e4.core.services.events.IEventBroker;
 import org.eclipse.equinox.frameworkadmin.BundleInfo;
 import org.eclipse.equinox.p2.metadata.IInstallableUnit;
-import org.eclipse.equinox.p2.metadata.Version;
+import org.eclipse.equinox.p2.metadata.IVersionedId;
+import org.eclipse.equinox.p2.metadata.VersionedId;
 import org.eclipse.osgi.service.datalocation.Location;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.pde.core.plugin.IPluginModelBase;
@@ -643,22 +646,22 @@ public IStatus compareWithTargetPlatform(ITargetDefinition target) throws CoreEx
 
 	@Override
 	public ITargetLocation newIULocation(IInstallableUnit[] units, URI[] repositories, int resolutionFlags) {
-		String[] fIds = new String[units.length];
-		Version[] fVersions = new Version[units.length];
-		for (int i = 0; i < units.length; i++) {
-			fIds[i] = units[i].getId();
-			fVersions[i] = units[i].getVersion();
-		}
-		return new IUBundleContainer(fIds, fVersions, repositories, resolutionFlags);
+		Stream<IVersionedId> ius = Arrays.stream(units).map(iu -> new VersionedId(iu.getId(), iu.getVersion()));
+		return createIUBundleContainer(ius, repositories, resolutionFlags);
 	}
 
 	@Override
 	public ITargetLocation newIULocation(String[] unitIds, String[] versions, URI[] repositories, int resolutionFlags) {
-		Version[] fVersions = new Version[versions.length];
-		for (int i = 0; i < versions.length; i++) {
-			fVersions[i] = Version.create(versions[i]);
+		if (unitIds.length != versions.length) {
+			throw new IllegalArgumentException("Units and versions must have the same length"); //$NON-NLS-1$
 		}
-		return new IUBundleContainer(unitIds, fVersions, repositories, resolutionFlags);
+		Stream<IVersionedId> ius = IntStream.range(0, unitIds.length)
+				.mapToObj(i -> new VersionedId(unitIds[i], versions[i]));
+		return createIUBundleContainer(ius, repositories, resolutionFlags);
+	}
+
+	private ITargetLocation createIUBundleContainer(Stream<IVersionedId> ius, URI[] repos, int resolutionFlags) {
+		return new IUBundleContainer(ius.toList(), repos == null ? List.of() : List.of(repos), resolutionFlags);
 	}
 
 }