svn commit: r1731141 - in /aries/trunk/subsystem: subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java

[jw...@apache.org]

Author: jwross
Date: Thu Feb 18 20:52:44 2016
New Revision: 1731141

URL: http://svn.apache.org/viewvc?rev=1731141&view=rev
Log:
[ARIES-1443] After a restart the capabilities of a subsystem have changed (seem correct) before the restart they seem wrong

This issue affects only newly installing features. The Subsystem service is registered and added to the system repository before all of its capabilities have been computed. Because
features implicitly export everything, their capabilities must be derived from their content. This is currently done examining constituents and filtering them based on the
Subsystem-Content header. It must be done this way for persisted subsystems, which are unaffected by this issue.

Initially, I tried to fix the issue by looking for features in the INSTALLED state as part of the modifiedService method of the SystemRepository because by then the consituents have
been computed and all capabilities are available. Unfortunately, this will not work for newly installing features with apache-aries-provision-dependencies:=resolve and that remain
in the INSTALLING state.

Ultimately, the issue was fixed in BasicSubsystem by adding an additional path to both getCapabilities and getRequirements. Newly installing subsystems can be distinguished from
persisted subsystems by the presence of a SubsystemResource, which the former will have while the latter will not. If a SubsystemResource is present, the capabilities and
requirements can be computed from the content contained therein. This is available as the newly installing subsystem is being added to the system repository. Persisted subsystems
continue to function as before.

Note that this should also fix ARIES-1442.

Added:
    aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java
Modified:
    aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java

Modified: aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java?rev=1731141&r1=1731140&r2=1731141&view=diff
==============================================================================
--- aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java (original)
+++ aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BasicSubsystem.java Thu Feb 18 20:52:44 2016
@@ -157,40 +157,107 @@ public class BasicSubsystem implements R
 	
 	@Override
 	public List<Capability> getCapabilities(String namespace) {
+		// First, add the capabilities from the manifest.
 		SubsystemManifest manifest = getSubsystemManifest();
 		List<Capability> result = manifest.toCapabilities(this);
-		if (namespace != null)
-			for (Iterator<Capability> i = result.iterator(); i.hasNext();)
-				if (!i.next().getNamespace().equals(namespace))
+		if (namespace != null) {
+			for (Iterator<Capability> i = result.iterator(); i.hasNext();) {
+				if (!i.next().getNamespace().equals(namespace)) {
 					i.remove();
-		// TODO Somehow, exposing the capabilities of content resources of a
-		// feature is causing an infinite regression of feature2 installations
-		// in FeatureTest.testSharedContent() under certain conditions.
-		if (isScoped() || IdentityNamespace.IDENTITY_NAMESPACE.equals(namespace))
+				}
+			}
+		}
+		if (isScoped() || IdentityNamespace.IDENTITY_NAMESPACE.equals(namespace)) {
+			// Scoped subsystems have all capabilities explicitly declared in
+			// their manifests. Also, we do not want to include the osgi.identity
+			// capabilities of content since a resource must have zero or one
+			// osgi.identity capabilities.
 			return result;
-		SubsystemContentHeader header = manifest.getSubsystemContentHeader();
-		for (Resource constituent : getConstituents())
-			if (header.contains(constituent))
-				for (Capability capability : constituent.getCapabilities(namespace))
-					result.add(new BasicCapability(capability, this));
+		}
+		// This is an unscoped subsystem that implicitly exports everything.
+		// Its capabilities must be derived from its content.
+		if (resource == null) {
+			// This is a persisted subsystem. We no longer have access to the
+			// original content. We must look at constituents that are also 
+			// content.
+			SubsystemContentHeader header = manifest.getSubsystemContentHeader();
+			for (Resource constituent : getConstituents()) {
+				if (header.contains(constituent)) {
+					for (Capability capability : constituent.getCapabilities(namespace)) {
+						result.add(new BasicCapability(capability, this));
+					}
+				}
+			}
+			return result;
+		}
+		// This is a newly installing subsystem. We therefore have access to the
+		// original content via the SubsystemResource and can derive the
+		// capabilities from there.
+		Collection<Resource> installableContent = resource.getInstallableContent();
+		Collection<Resource> sharedContent = resource.getSharedContent();
+		Collection<Resource> contents = new ArrayList<Resource>(installableContent.size() + sharedContent.size());
+		contents.addAll(installableContent);
+		contents.addAll(sharedContent);
+		for (Resource content : contents) {
+			for (Capability capability : content.getCapabilities(namespace)) {
+				if (namespace == null && IdentityNamespace.IDENTITY_NAMESPACE.equals(capability.getNamespace())) {
+					// Don't want to include the osgi.identity capabilities of
+					// content. Need a second check here in case the namespace
+					// is null.
+					continue;
+				}
+				result.add(new BasicCapability(capability, this));
+			}
+		}
 		return result;
 	}
 
 	@Override
 	public List<Requirement> getRequirements(String namespace) {
+		// First, add the requirements from the manifest.
 		SubsystemManifest manifest = getSubsystemManifest();
 		List<Requirement> result = manifest.toRequirements(this);
-		if (namespace != null)
-			for (Iterator<Requirement> i = result.iterator(); i.hasNext();)
-				if (!i.next().getNamespace().equals(namespace))
+		if (namespace != null) {
+			for (Iterator<Requirement> i = result.iterator(); i.hasNext();) {
+				if (!i.next().getNamespace().equals(namespace)) {
 					i.remove();
-		if (isScoped())
+				}
+			}
+		}
+		if (isScoped()) {
+			// Scoped subsystems have all requirements explicitly declared in
+			// their manifests.
+			return result;
+		}
+		// This is an unscoped subsystem that implicitly imports everything.
+		// Its requirements must be derived from its content.
+		if (resource == null) {
+			// This is a persisted subsystem. We no longer have access to the
+			// original content. We must look at constituents that are also 
+			// content.
+			SubsystemContentHeader header = manifest.getSubsystemContentHeader();
+			for (Resource constituent : getConstituents()) {
+				if (header.contains(constituent)) {
+					for (Requirement requirement : constituent.getRequirements(namespace)) {
+						result.add(new BasicRequirement(requirement, this));
+					}
+				}
+			}
 			return result;
-		SubsystemContentHeader header = manifest.getSubsystemContentHeader();
-		for (Resource constituent : getConstituents())
-			if (header.contains(constituent))
-				for (Requirement requirement : constituent.getRequirements(namespace))
-					result.add(new BasicRequirement(requirement, this));
+		}
+		// This is a newly installing subsystem. We therefore have access to the
+		// original content via the SubsystemResource and can derive the
+		// requirements from there.
+		Collection<Resource> installableContent = resource.getInstallableContent();
+		Collection<Resource> sharedContent = resource.getSharedContent();
+		Collection<Resource> contents = new ArrayList<Resource>(installableContent.size() + sharedContent.size());
+		contents.addAll(installableContent);
+		contents.addAll(sharedContent);
+		for (Resource content : contents) {
+			for (Requirement requirement : content.getRequirements(namespace)) {
+				result.add(new BasicRequirement(requirement, this));
+			}
+		}
 		return result;
 	}
 	

Added: aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java?rev=1731141&view=auto
==============================================================================
--- aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java (added)
+++ aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/defect/Aries1443Test.java Thu Feb 18 20:52:44 2016
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.aries.subsystem.itests.defect;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.aries.subsystem.core.archive.AriesProvisionDependenciesDirective;
+import org.apache.aries.subsystem.core.internal.Activator;
+import org.apache.aries.subsystem.core.internal.SystemRepository;
+import org.apache.aries.subsystem.itests.SubsystemTest;
+import org.apache.aries.subsystem.itests.util.BundleArchiveBuilder;
+import org.apache.aries.subsystem.itests.util.SubsystemArchiveBuilder;
+import org.apache.aries.subsystem.itests.util.TestRequirement;
+import org.junit.Test;
+import org.osgi.resource.Capability;
+import org.osgi.resource.Requirement;
+import org.osgi.service.subsystem.Subsystem;
+import org.osgi.service.subsystem.SubsystemConstants;
+
+public class Aries1443Test extends SubsystemTest {
+	@Test
+    public void testProvisionDependenciesInstall() throws Exception {
+		test(AriesProvisionDependenciesDirective.VALUE_INSTALL);
+    }
+	
+	@Test
+    public void testProvisionDependenciesResolve() throws Exception {
+		test(AriesProvisionDependenciesDirective.VALUE_RESOLVE);
+    }
+	
+	private void test(String apacheAriesProvisionDependencies) throws Exception {
+		Subsystem root = getRootSubsystem();
+		Subsystem composite = installSubsystem(
+				root,
+				"composite", 
+				new SubsystemArchiveBuilder()
+						.symbolicName("composite")
+						.type(SubsystemConstants.SUBSYSTEM_TYPE_COMPOSITE 
+								+ ';' 
+								+ AriesProvisionDependenciesDirective.NAME
+								+ ":="
+								+ apacheAriesProvisionDependencies)
+						.importPackage("org.osgi.framework")
+						.build(),
+				AriesProvisionDependenciesDirective.VALUE_INSTALL.equals(apacheAriesProvisionDependencies)
+		);
+		uninstallableSubsystems.add(composite);
+		startSubsystem(composite, AriesProvisionDependenciesDirective.VALUE_INSTALL.equals(apacheAriesProvisionDependencies));
+		stoppableSubsystems.add(composite);
+		installSubsystem(
+				composite,
+				"feature", 
+				new SubsystemArchiveBuilder()
+						.symbolicName("feature")
+						.type(SubsystemConstants.SUBSYSTEM_TYPE_FEATURE
+								+ ';'
+								+ AriesProvisionDependenciesDirective.NAME
+								+ ":="
+								+ apacheAriesProvisionDependencies)
+						.bundle(
+								"a", 
+								new BundleArchiveBuilder()
+										.symbolicName("a")
+										.exportPackage("a")
+								.build())
+						.build(),
+				AriesProvisionDependenciesDirective.VALUE_INSTALL.equals(apacheAriesProvisionDependencies)
+		);
+		SystemRepository repository = Activator.getInstance().getSystemRepository();
+		Requirement requirement = new TestRequirement.Builder()
+				.namespace("osgi.wiring.package")
+				.directive("filter", "(osgi.wiring.package=a)")
+				.build();
+		Map<Requirement, Collection<Capability>> providers = repository.findProviders(
+				Collections.singleton(requirement));
+		Collection<Capability> capabilities = providers.get(requirement);
+		assertEquals("Missing provider", 2, capabilities.size());
+	}
+}