FELIX-2169: Improve dev:show-tree performance and avoid NPE when no matching export is found
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@918963 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/karaf/assembly/src/main/descriptors/unix-bin.xml b/karaf/assembly/src/main/descriptors/unix-bin.xml
index bd22dee..d148c48 100644
--- a/karaf/assembly/src/main/descriptors/unix-bin.xml
+++ b/karaf/assembly/src/main/descriptors/unix-bin.xml
@@ -170,6 +170,15 @@
<include>org.ops4j.pax.url:pax-url-wrap</include>
</includes>
</dependencySet>
+ <dependencySet>
+ <outputDirectory>/system</outputDirectory>
+ <unpack>false</unpack>
+ <useProjectArtifact>false</useProjectArtifact>
+ <outputFileNameMapping>org/apache/felix/karaf/${artifact.artifactId}/${artifact.baseVersion}/${artifact.artifactId}-${artifact.baseVersion}${dashClassifier?}.${artifact.extension}</outputFileNameMapping>
+ <includes>
+ <include>org.apache.felix.karaf:org.apache.felix.karaf.commons</include>
+ </includes>
+ </dependencySet>
<dependencySet>
<outputDirectory>/system</outputDirectory>
<unpack>false</unpack>
diff --git a/karaf/assembly/src/main/descriptors/windows-bin.xml b/karaf/assembly/src/main/descriptors/windows-bin.xml
index aac4cd8..995eed1 100644
--- a/karaf/assembly/src/main/descriptors/windows-bin.xml
+++ b/karaf/assembly/src/main/descriptors/windows-bin.xml
@@ -168,6 +168,15 @@
<useProjectArtifact>false</useProjectArtifact>
<outputFileNameMapping>org/apache/felix/karaf/${artifact.artifactId}/${artifact.baseVersion}/${artifact.artifactId}-${artifact.baseVersion}${dashClassifier?}.${artifact.extension}</outputFileNameMapping>
<includes>
+ <include>org.apache.felix.karaf:org.apache.felix.karaf.commons</include>
+ </includes>
+ </dependencySet>
+ <dependencySet>
+ <outputDirectory>/system</outputDirectory>
+ <unpack>false</unpack>
+ <useProjectArtifact>false</useProjectArtifact>
+ <outputFileNameMapping>org/apache/felix/karaf/${artifact.artifactId}/${artifact.baseVersion}/${artifact.artifactId}-${artifact.baseVersion}${dashClassifier?}.${artifact.extension}</outputFileNameMapping>
+ <includes>
<include>org.apache.felix.karaf:org.apache.felix.karaf.management</include>
</includes>
</dependencySet>
diff --git a/karaf/assembly/src/main/filtered-resources/etc/startup.properties b/karaf/assembly/src/main/filtered-resources/etc/startup.properties
index 9addee2..63c3cd0 100644
--- a/karaf/assembly/src/main/filtered-resources/etc/startup.properties
+++ b/karaf/assembly/src/main/filtered-resources/etc/startup.properties
@@ -36,6 +36,7 @@
#
org/apache/aries/blueprint/org.apache.aries.blueprint/${aries.blueprint.version}/org.apache.aries.blueprint-${aries.blueprint.version}.jar=20
+org/apache/felix/karaf/org.apache.felix.karaf.commons/${pom.version}/org.apache.felix.karaf.commons-${pom.version}.jar=30
org/apache/felix/gogo/org.apache.felix.gogo.runtime/${felix.gogo.version}/org.apache.felix.gogo.runtime-${felix.gogo.version}.jar=30
org/apache/felix/karaf/shell/org.apache.felix.karaf.shell.console/${pom.version}/org.apache.felix.karaf.shell.console-${pom.version}.jar=30
org/apache/felix/karaf/deployer/org.apache.felix.karaf.deployer.spring/${pom.version}/org.apache.felix.karaf.deployer.spring-${pom.version}.jar=30
diff --git a/karaf/commons/pom.xml b/karaf/commons/pom.xml
new file mode 100644
index 0000000..f491e69
--- /dev/null
+++ b/karaf/commons/pom.xml
@@ -0,0 +1,70 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+
+ <!--
+
+ 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.
+ -->
+
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <artifactId>karaf</artifactId>
+ <groupId>org.apache.felix.karaf</groupId>
+ <version>1.5.0-SNAPSHOT</version>
+ </parent>
+
+ <groupId>org.apache.felix.karaf</groupId>
+ <artifactId>org.apache.felix.karaf.commons</artifactId>
+ <version>1.5.0-SNAPSHOT</version>
+ <packaging>bundle</packaging>
+ <name>Apache Felix Karaf :: Commons</name>
+
+ <properties>
+ <appendedResourcesDirectory>${basedir}/../etc/appended-resources</appendedResourcesDirectory>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.felix</groupId>
+ <artifactId>org.osgi.core</artifactId>
+ <scope>provided</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.felix</groupId>
+ <artifactId>maven-bundle-plugin</artifactId>
+ <configuration>
+ <instructions>
+ <Bundle-SymbolicName>${pom.artifactId}</Bundle-SymbolicName>
+ <Export-Package>${pom.artifactId}*;version=${project.version}</Export-Package>
+ <Import-Package>
+ !${pom.artifactId}*,
+ *
+ </Import-Package>
+ <_versionpolicy>${bnd.version.policy}</_versionpolicy>
+ </instructions>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+
+</project>
\ No newline at end of file
diff --git a/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/VersionRange.java b/karaf/commons/src/main/java/org/apache/felix/karaf/commons/osgi/VersionRange.java
similarity index 98%
rename from karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/VersionRange.java
rename to karaf/commons/src/main/java/org/apache/felix/karaf/commons/osgi/VersionRange.java
index f600b8f..47045de 100644
--- a/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/VersionRange.java
+++ b/karaf/commons/src/main/java/org/apache/felix/karaf/commons/osgi/VersionRange.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.felix.karaf.features.internal;
+package org.apache.felix.karaf.commons.osgi;
import org.osgi.framework.Version;
diff --git a/karaf/features/core/pom.xml b/karaf/features/core/pom.xml
index aa3789a..58c19ea 100644
--- a/karaf/features/core/pom.xml
+++ b/karaf/features/core/pom.xml
@@ -55,6 +55,11 @@
</dependency>
<dependency>
+ <groupId>org.apache.felix.karaf</groupId>
+ <artifactId>org.apache.felix.karaf.commons</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.apache.felix.karaf.shell</groupId>
<artifactId>org.apache.felix.karaf.shell.console</artifactId>
</dependency>
diff --git a/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/FeaturesServiceImpl.java b/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/FeaturesServiceImpl.java
index 2e0fe64..5bbf000 100644
--- a/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/FeaturesServiceImpl.java
+++ b/karaf/features/core/src/main/java/org/apache/felix/karaf/features/internal/FeaturesServiceImpl.java
@@ -40,6 +40,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.felix.karaf.commons.osgi.VersionRange;
import org.apache.felix.karaf.features.FeaturesService;
import org.apache.felix.karaf.features.Feature;
import org.apache.felix.karaf.features.Repository;
diff --git a/karaf/pom.xml b/karaf/pom.xml
index c4deca7..88eeb5c 100644
--- a/karaf/pom.xml
+++ b/karaf/pom.xml
@@ -35,6 +35,7 @@
<modules>
<module>main</module>
+ <module>commons</module>
<module>features</module>
<module>admin</module>
<module>deployer</module>
@@ -171,6 +172,11 @@
<version>${pom.version}</version>
</dependency>
<dependency>
+ <groupId>org.apache.felix.karaf</groupId>
+ <artifactId>org.apache.felix.karaf.commons</artifactId>
+ <version>${pom.version}</version>
+ </dependency>
+ <dependency>
<groupId>org.apache.felix.karaf.deployer</groupId>
<artifactId>org.apache.felix.karaf.deployer.spring</artifactId>
<version>${pom.version}</version>
diff --git a/karaf/shell/dev/pom.xml b/karaf/shell/dev/pom.xml
index 0c49603..406df26 100644
--- a/karaf/shell/dev/pom.xml
+++ b/karaf/shell/dev/pom.xml
@@ -37,6 +37,11 @@
</dependency>
<dependency>
+ <groupId>org.apache.felix.karaf</groupId>
+ <artifactId>org.apache.felix.karaf.commons</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.ops4j.pax.url</groupId>
<artifactId>pax-url-wrap</artifactId>
</dependency>
diff --git a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/ShowBundleTree.java b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/ShowBundleTree.java
index aa5e210..c461274 100644
--- a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/ShowBundleTree.java
+++ b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/ShowBundleTree.java
@@ -48,16 +48,19 @@
private static final Logger LOGGER = LoggerFactory.getLogger(ShowBundleTree.class);
- // a cache of all exported packages
- private ExportedPackage[] allExportedPackages;
+ private Tree<Bundle> tree;
@Override
protected void doExecute(Bundle bundle) throws Exception {
+ long start = System.currentTimeMillis();
// let's do the real work here
printHeader(bundle);
- Tree<Bundle> tree = createTree(bundle);
+ tree = new Tree<Bundle>(bundle);
+ createTree(bundle);
printTree(tree);
printDuplicatePackages(tree);
+ LOGGER.debug(format("Dependency tree calculated in %d ms",
+ System.currentTimeMillis() - start));
}
/*
@@ -119,44 +122,69 @@
/*
* Creates the bundle tree
*/
- protected Tree<Bundle> createTree(Bundle bundle) {
- Tree<Bundle> tree = new Tree<Bundle>(bundle);
- Set<Bundle> trail = new HashSet<Bundle>();
+ protected void createTree(Bundle bundle) {
if (bundle.getState() >= Bundle.RESOLVED) {
- createNode(tree, trail);
+ createNode(tree);
} else {
- for (Import i : Import.parse(String.valueOf(bundle.getHeaders().get("Import-Package")))) {
- for (ExportedPackage ep : getPackageAdmin().getExportedPackages(i.getPackage())) {
- if (ep.getVersion().compareTo(i.getVersion()) >= 0) {
- if (!bundle.equals(ep.getExportingBundle())) {
- Node child = tree.addChild(ep.getExportingBundle());
- System.out.printf("- using %s to resolve import %s%n", ep.getExportingBundle(), i);
- createNode(child, trail);
- }
+ createNodesForImports(tree, bundle);
+ }
+ }
+
+ /*
+ * Creates nodes for the imports of the bundle (instead of reporting wiring information
+ */
+ private void createNodesForImports(Node node, Bundle bundle) {
+ for (Import i : Import.parse(String.valueOf(bundle.getHeaders().get("Import-Package")),
+ String.valueOf(bundle.getHeaders().get("Export-Package")))) {
+ createNodeForImport(node, bundle, i);
+ }
+ }
+
+ /*
+ * Create a child node for a given import (by finding a matching export in the currently installed bundles)
+ */
+ private void createNodeForImport(Node node, Bundle bundle, Import i) {
+ ExportedPackage[] exporters = getPackageAdmin().getExportedPackages(i.getPackage());
+ boolean foundMatch = false;
+ if (exporters != null) {
+ for (ExportedPackage ep : exporters) {
+ if (i.getVersion().isInRange(ep.getVersion())) {
+ if (bundle.equals(ep.getExportingBundle())) {
+ foundMatch = true;
+ } else {
+ Node child = node.addChild(ep.getExportingBundle());
+ System.out.printf("- import %s: resolved using %s%n", i, ep.getExportingBundle());
+ foundMatch = true;
+ createNode(child);
}
}
}
}
- return tree;
+ if (!foundMatch) {
+ System.out.printf("- import %s: WARNING - unable to find matching export%n", i);
+ }
}
/*
* Creates a node in the bundle tree
*/
- private void createNode(Node<Bundle> node, Set<Bundle> trail) {
+ private void createNode(Node<Bundle> node) {
Bundle bundle = node.getValue();
Collection<Bundle> exporters = new HashSet<Bundle>();
exporters.addAll(getWiredBundles(bundle).values());
for (Bundle exporter : exporters) {
- if (trail.contains(exporter)) {
- LOGGER.debug(format("Skipping %s because it already exists in the current tree branch", exporter));
+ if (node.hasAncestor(exporter)) {
+ LOGGER.debug(format("Skipping %s (already exists in the current branch)", exporter));
} else {
- trail.add(exporter);
- Node child = node.addChild(exporter);
+ boolean existing = tree.flatten().contains(exporter);
LOGGER.debug(format("Adding %s as a dependency for %s", exporter, bundle));
- createNode(child, trail);
- trail.remove(exporter);
+ Node child = node.addChild(exporter);
+ if (existing) {
+ LOGGER.debug(format("Skipping children of %s (already exists in another branch)", exporter));
+ } else {
+ createNode(child);
+ }
}
}
}
diff --git a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Import.java b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Import.java
index bea30f1..3215707 100644
--- a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Import.java
+++ b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Import.java
@@ -19,7 +19,7 @@
import java.util.LinkedList;
import java.util.List;
-import org.osgi.framework.Version;
+import org.apache.felix.karaf.commons.osgi.VersionRange;
/**
* Simple class to model an OSGi Import-Package
@@ -27,7 +27,7 @@
public class Import {
private final String packageName;
- private final Version version;
+ private final VersionRange version;
private final String value;
/**
@@ -38,28 +38,24 @@
protected Import(String value) {
super();
this.value = value;
- if (value.contains(";")) {
- this.packageName = value.split(";")[0];
- } else {
- this.packageName = value;
- }
+ this.packageName = extractPackageName(value);
if (value.contains("version=")) {
this.version = extractVersion(value);
} else {
- this.version = Version.emptyVersion;
+ this.version = VersionRange.infiniteRange;
}
}
/*
* Extract the version from the string
*/
- private Version extractVersion(String value) {
+ private VersionRange extractVersion(String value) {
int begin = value.indexOf("version=") + 8;
int end = value.indexOf(";", begin);
if (end < 0) {
- return Version.parseVersion(unquote(value.substring(begin)));
+ return VersionRange.parse(unquote(value.substring(begin)));
} else {
- return Version.parseVersion(unquote(value.substring(begin, end)));
+ return VersionRange.parse(unquote(value.substring(begin, end)));
}
}
@@ -74,7 +70,7 @@
return packageName;
}
- public Version getVersion() {
+ public VersionRange getVersion() {
return version;
}
@@ -89,9 +85,67 @@
*/
public static List<Import> parse(String value) {
LinkedList<Import> imports = new LinkedList<Import>();
- for (String imp : value.split(",")) {
+ for (String imp : split(value)) {
imports.add(new Import(imp));
}
return imports;
}
+
+ /**
+ * Parse the value of an Import-Package META-INF header and return
+ * a list of Import instances, filtering out packages that are in the
+ * Export-Package META-INF header
+ *
+ * @param importValue the value of the Import-Package header
+ * @param exportValue the value of the Export-Package header
+ */
+ public static List<Import> parse(String importValue, String exportValue) {
+ LinkedList<String> exports = new LinkedList<String>();
+ for (String exp : split(exportValue)) {
+ exports.add(extractPackageName(exp));
+ }
+ LinkedList<Import> imports = new LinkedList<Import>();
+ for (Import imp : parse(importValue)) {
+ if (!exports.contains(imp.getPackage())) {
+ imports.add(imp);
+ }
+ }
+ return imports;
+ }
+
+ /*
+ * Extract the package name from the value
+ * e.g. org.apache.felix.karaf;version="1.x" -> org.apache.felix.karaf
+ */
+ private static String extractPackageName(String value) {
+ if (value.contains(";")) {
+ return value.split(";")[0];
+ } else {
+ return value;
+ }
+ }
+
+ /*
+ * Counts the number of quotes in a String value
+ */
+ private static int quotes(String value) {
+ return value.replaceAll("[^\"]", "").length();
+ }
+
+ /*
+ * Split the OSGi headers on the , symbol
+ */
+ private static List<String> split(String value) {
+ List<String> result = new LinkedList<String>();
+ String[] elements = value.split(",");
+ for (int i = 0; i < elements.length ; i++) {
+ if (quotes(elements[i]) % 2 == 1) {
+ // we probably split a version range, so joining it again with the next element
+ result.add(elements[i] + "," + elements[++i]);
+ } else {
+ result.add(elements[i]);
+ }
+ }
+ return result;
+ }
}
diff --git a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Node.java b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Node.java
index 1d509fa..b266fe4 100644
--- a/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Node.java
+++ b/karaf/shell/dev/src/main/java/org/apache/felix/karaf/shell/dev/util/Node.java
@@ -93,6 +93,20 @@
return result;
}
+ /**
+ * Check if the node has an ancestor that represents the given value
+ *
+ * @param value the node value
+ * @return <code>true</code> it there's an ancestor that represents the value
+ */
+ public boolean hasAncestor(T value) {
+ if (parent == null) {
+ return false;
+ } else {
+ return value.equals(parent.value) || parent.hasAncestor(value);
+ }
+ }
+
/*
* Write this node to the PrintWriter. It should be indented one step
* further for every element in the indents array. If an element in the
diff --git a/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/ImportTest.java b/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/ImportTest.java
index cd9543a..8a2aa14 100644
--- a/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/ImportTest.java
+++ b/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/ImportTest.java
@@ -20,8 +20,9 @@
import static junit.framework.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+
+import org.apache.felix.karaf.commons.osgi.VersionRange;
import org.junit.Test;
-import org.osgi.framework.Version;
/**
* Test cases for {@link org.apache.felix.karaf.shell.dev.util.Import}
@@ -38,7 +39,7 @@
public void createWithPackageNameAndVersion() {
Import i = new Import("org.wip.bar;version=\"2.0.0\"");
assertEquals("org.wip.bar", i.getPackage());
- assertEquals(new Version("2.0.0"), i.getVersion());
+ assertEquals(VersionRange.parse("2.0.0"), i.getVersion());
}
@Test
@@ -49,4 +50,22 @@
assertEquals("org.wip.bar", imports.get(0).getPackage());
assertEquals("org.wip.foo", imports.get(1).getPackage());
}
+
+ @Test
+ public void createListOfImportsWithVersionRanges() {
+ List<Import> imports =
+ Import.parse("javax.activation;version=\"[0.0,2)\",javax.annotation;version=\"[0.0,2)\"");
+ assertNotNull(imports);
+ assertEquals(2, imports.size());
+ assertEquals("javax.activation", imports.get(0).getPackage());
+ assertEquals("javax.annotation", imports.get(1).getPackage());
+ }
+
+ @Test
+ public void createListOfImportsWithExports() {
+ List<Import> imports = Import.parse("org.wip.bar;version=\"2.0.0\",org.wip.foo", "org.wip.bar;version=\"2.0.0\"");
+ assertNotNull(imports);
+ assertEquals(1, imports.size());
+ assertEquals("org.wip.foo", imports.get(0).getPackage());
+ }
}
diff --git a/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/TreeTest.java b/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/TreeTest.java
index d05b519..c11e2c3 100644
--- a/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/TreeTest.java
+++ b/karaf/shell/dev/src/test/java/org/apache/felix/karaf/shell/dev/util/TreeTest.java
@@ -24,6 +24,7 @@
import java.util.Set;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
@@ -108,6 +109,19 @@
assertTrue(elements.contains("grandchild"));
}
+ @Test
+ public void hasAncestor() throws IOException {
+ Tree<String> tree = new Tree<String>("root");
+ Node<String> child1 = tree.addChild("child1");
+ child1.addChild("grandchild");
+ Node child2 = tree.addChild("child2");
+ Node node = child2.addChild("grandchild2");
+
+ assertTrue(node.hasAncestor("child2"));
+ assertTrue(node.hasAncestor("root"));
+ assertFalse(node.hasAncestor("child1"));
+ }
+
private BufferedReader read(Tree<String> tree) {
StringWriter writer = new StringWriter();
tree.write(new PrintWriter(writer));