390361 Test that ProductAction fails if the result may be brokenv20120928-163700

- Added test for fail fast behaviour of ProductAction in case of unknown
  inclusion filters.
- Also: For callers that don't check the returned status, the behavior
  remains as before -> the missing filter issue may then be detected
  later when the product is installed.
- Also: Improved messages.

Bug: 390361 Published products are broken if included fragments are not in context

6 files changed, 71 insertions, 12 deletions

diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/Messages.java b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/Messages.java
index 96fcf947c..c914949a0 100644
--- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/Messages.java
+++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/Messages.java
@@ -31,7 +31,7 @@ public class Messages extends NLS {
 	public static String bundlesInProductFileIgnored;
 
 	public static String message_problemPublishingProduct;
-	public static String message_includedElementNotFound;
+	public static String message_cannotDetermineFilterOnInclusion;
 
 	static {
 		// load message values from bundle file
diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/messages.properties b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/messages.properties
index 6e82053c9..4c63bca8c 100644
--- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/messages.properties
@@ -23,5 +23,5 @@ feature_parse_emptyRequires = Error parsing feature: \"require\" must include at
 
 featuresInProductFileIgnored=The features specified in the product definition are ignored; verify the value of the 'type' or 'useFeatures' attribute.   
 bundlesInProductFileIgnored=The bundles specified in the product definition are ignored; verify the value of the 'type' or 'useFeatures' attribute.
-message_includedElementNotFound=Included element not found: {0}/{1}
-message_problemPublishingProduct=Problems while publishing product {1}
+message_cannotDetermineFilterOnInclusion=Included element {0} {1} is missing. Cannot determine filter for requirement to this element.
+message_problemPublishingProduct=Problems while publishing product {0}
diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/ProductAction.java b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/ProductAction.java
index 17747259f..f0e17855c 100644
--- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/ProductAction.java
+++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/ProductAction.java
@@ -184,7 +184,11 @@ public class ProductAction extends AbstractPublisherAction {
 			if (unit != null) {
 				result.add(unit);
 			} else {
-				finalStatus.add(new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.message_includedElementNotFound, element.getId(), elementVersion)));
+				// if the bundle is platform specific we will have broken metadata due to a missing filter
+				finalStatus.add(new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.message_cannotDetermineFilterOnInclusion, element.getId(), elementVersion)));
+
+				// best effort guess for callers who choose to ignore this problem
+				result.add(new VersionedId(element.getId(), elementVersion));
 			}
 		}
 		return result;
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java
index bfc942a82..74f952940 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/AbstractProvisioningTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2011 IBM Corporation and others. All rights reserved. This
+ * Copyright (c) 2007, 2012 IBM Corporation and others. All rights reserved. This
  * program and the accompanying materials are made available under the terms of
  * the Eclipse Public License v1.0 which accompanies this distribution, and is
  * available at http://www.eclipse.org/legal/epl-v10.html
@@ -7,7 +7,6 @@
  * Contributors: IBM Corporation - initial API and implementation
  ******************************************************************************/
 package org.eclipse.equinox.p2.tests;
-
 import java.io.*;
 import java.lang.reflect.Field;
 import java.net.URI;
@@ -44,6 +43,7 @@ import org.eclipse.osgi.service.resolver.BundleDescription;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 
+
 /**
  * Base class for provisioning tests with convenience methods used by multiple tests.
  */
@@ -587,6 +587,19 @@ public abstract class AbstractProvisioningTest extends TestCase {
 		return new IRequirement[] {MetadataFactory.createRequirement(namespace, name, range, filter, false, false)};
 	}
 
+	public static IRequirement createIURequirement(String name, VersionRange range) {
+		return MetadataFactory.createRequirement(IInstallableUnit.NAMESPACE_IU_ID, name, range, null, false, false);
+	}
+
+	public static IRequirement createIURequirement(String name, VersionRange range, String filter) {
+		return MetadataFactory.createRequirement(IInstallableUnit.NAMESPACE_IU_ID, name, range, InstallableUnit.parseFilter(filter), false, false);
+	}
+
+	public static VersionRange createStrictVersionRange(String version) {
+		Version parsedVersion = Version.create(version);
+		return new VersionRange(parsedVersion, true, parsedVersion, true);
+	}
+
 	public static boolean delete(File file) {
 		if (!file.exists())
 			return true;
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/ProductActionTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/ProductActionTest.java
index 49eb38bc6..55fec1996 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/ProductActionTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/publisher/actions/ProductActionTest.java
@@ -12,8 +12,7 @@
 package org.eclipse.equinox.p2.tests.publisher.actions;
 
 import static org.easymock.EasyMock.createNiceMock;
-import static org.eclipse.equinox.p2.tests.publisher.actions.StatusMatchers.okStatus;
-import static org.eclipse.equinox.p2.tests.publisher.actions.StatusMatchers.statusWithMessageWhich;
+import static org.eclipse.equinox.p2.tests.publisher.actions.StatusMatchers.*;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.junit.Assert.assertThat;
@@ -313,10 +312,29 @@ public class ProductActionTest extends ActionTest {
 		assertFalse("2.6", update.isUpdateOf(createIU("com.other", Version.createOSGi(4, 1, 0))));
 	}
 
+	public void testFiltersOfInclusions() throws Exception {
+		ProductFile productFile = new ProductFile(TestData.getFile("ProductActionTest", "productIncludingFragments.product").toString());
+		addContextIU("generalbundle", "1.0.1");
+		addContextIU("fragment.win", "1.0.2", WIN_FILTER);
+		// no fragment.linux in the context
+
+		IStatus status = performProductActionAndReturnStatus(productFile);
+
+		IInstallableUnit productIU = getUniquePublishedIU("productIncludingFragments.uid");
+		assertThat(productIU.getRequirements(), hasItem(createIURequirement("generalbundle", createStrictVersionRange("1.0.1"))));
+		assertThat(productIU.getRequirements(), hasItem(createIURequirement("fragment.win", createStrictVersionRange("1.0.2"), WIN_FILTER)));
+
+		// this is bug 390361: the Linux fragment is required without filter, so the product cannot be installed for Windows ...
+		assertThat(productIU.getRequirements(), hasItem(createIURequirement("fragment.linux", ANY_VERSION)));
+
+		// ... therefore the action shall report an error
+		assertThat(status, is(errorStatus()));
+		assertThat(Arrays.asList(status.getChildren()), hasItem(statusWithMessageWhich(containsString("Included element fragment.linux 0.0.0 is missing"))));
+	}
+
 	public void testMessageForProductWithIgnoredContent() throws Exception {
 		ProductFile productFile = new ProductFile(TestData.getFile("ProductActionTest", "mixedContentIgnored.product").toString());
-		testAction = new ProductAction(source, productFile, flavorArg, executablesFeatureLocation);
-		IStatus status = testAction.perform(publisherInfo, publisherResult, null);
+		IStatus status = performProductActionAndReturnStatus(productFile);
 
 		// expect a warning about redundant, ignored content in product file -> requested in bug 325611
 		assertThat(Arrays.asList(status.getChildren()), hasItem(statusWithMessageWhich(containsString("are ignored"))));
@@ -324,11 +342,15 @@ public class ProductActionTest extends ActionTest {
 	}
 
 	private void performProductAction(ProductFile productFile) {
-		testAction = new ProductAction(source, productFile, flavorArg, executablesFeatureLocation);
-		IStatus status = testAction.perform(publisherInfo, publisherResult, null);
+		IStatus status = performProductActionAndReturnStatus(productFile);
 		assertThat(status, is(okStatus()));
 	}
 
+	private IStatus performProductActionAndReturnStatus(ProductFile productFile) {
+		testAction = new ProductAction(source, productFile, flavorArg, executablesFeatureLocation);
+		return testAction.perform(publisherInfo, publisherResult, null);
+	}
+
 	private void setConfiguration(String configSpec) {
 		((PublisherInfo) publisherInfo).setConfigurations(new String[] {configSpec});
 	}
diff --git a/bundles/org.eclipse.equinox.p2.tests/testData/ProductActionTest/productIncludingFragments.product b/bundles/org.eclipse.equinox.p2.tests/testData/ProductActionTest/productIncludingFragments.product
new file mode 100644
index 000000000..57da4285b
--- /dev/null
+++ b/bundles/org.eclipse.equinox.p2.tests/testData/ProductActionTest/productIncludingFragments.product
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<?pde version="3.5"?>
+
+<product uid="productIncludingFragments.uid" useFeatures="false" includeLaunchers="false">
+
+   <configIni use="default">
+   </configIni>
+
+   <launcherArgs>
+      <vmArgsMac>-XstartOnFirstThread -Dorg.eclipse.swt.internal.carbon.smallFonts</vmArgsMac>
+   </launcherArgs>
+
+   <plugins>
+      <plugin id="generalbundle"/>
+      <plugin id="fragment.win"/>
+      <plugin id="fragment.linux"/>
+   </plugins>
+
+
+</product>