From 5bde2cf8b4b66188e3cd933d4b88ce6a01b02b37 Mon Sep 17 00:00:00 2001 From: "A. J. David Bosschaert" Date: Wed, 31 May 2017 16:44:33 +0000 Subject: [PATCH] FELIX-5644 Repository#getURI() is no longer unique in case of XML-based repositories Patch applied on behalf of cfiehe with many thanks. This closes https://github.com/apache/felix/pull/109 git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1797072 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/DataModelHelperImpl.java | 23 +++++++++++------- .../bundlerepository/impl/PullParser.java | 14 +++++------ .../impl/RepositoryParser.java | 3 ++- .../impl/SpecXMLPullParser.java | 24 +++++++++---------- .../bundlerepository/impl/StaxParser.java | 8 +++---- .../impl/OSGiRepositoryXMLTest.java | 20 +++++++++------- .../impl/RepositoryAdminTest.java | 17 +++++++++++++ 7 files changed, 68 insertions(+), 41 deletions(-) diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/DataModelHelperImpl.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/DataModelHelperImpl.java index 7d575b657e2..5d845b75cf8 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/DataModelHelperImpl.java +++ b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/DataModelHelperImpl.java @@ -91,7 +91,6 @@ public Filter filter(String filter) public Repository repository(final URL url) throws Exception { InputStream is = null; - BufferedReader br = null; try { @@ -132,13 +131,19 @@ else if (url.getPath().endsWith(".gz")) if (is != null) { - String repostr = url.toExternalForm(); - if (repostr.endsWith("zip")) { - repostr = "jar:".concat(repostr).concat("!/"); - } else if (repostr.endsWith(".xml")) { - repostr = repostr.substring(0, repostr.lastIndexOf('/')+1); + String repositoryUri = url.toExternalForm(); + String baseUri; + if (repositoryUri.endsWith(".zip")) { + baseUri = new StringBuilder("jar:").append(repositoryUri).append("!/").toString(); + } else if (repositoryUri.endsWith(".xml")) { + baseUri = repositoryUri.substring(0, repositoryUri.lastIndexOf('/') + 1); + } else { + baseUri = repositoryUri; } - return repository(is, repostr); + RepositoryImpl repository = repository(is, URI.create(baseUri)); + repository.setURI(repositoryUri); + + return repository; } else { @@ -162,10 +167,10 @@ else if (url.getPath().endsWith(".gz")) } } - public RepositoryImpl repository(InputStream is, String uri) throws Exception + public RepositoryImpl repository(InputStream is, URI baseURI) throws Exception { RepositoryParser parser = RepositoryParser.getParser(); - RepositoryImpl repository = parser.parseRepository(is, uri); + RepositoryImpl repository = parser.parseRepository(is, baseURI); return repository; } diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/PullParser.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/PullParser.java index a5c6b257c1d..d91b2ec0dd3 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/PullParser.java +++ b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/PullParser.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.Reader; +import java.net.URI; import org.kxml2.io.KXmlParser; import org.xmlpull.v1.XmlPullParser; @@ -36,7 +37,7 @@ public PullParser() { } - public RepositoryImpl parseRepository(InputStream is, String repositoryURI) throws Exception + public RepositoryImpl parseRepository(InputStream is, URI baseUri) throws Exception { XmlPullParser reader = new KXmlParser(); @@ -51,16 +52,15 @@ public RepositoryImpl parseRepository(InputStream is, String repositoryURI) thro } RepositoryImpl repo; - if ("http://www.osgi.org/xmlns/repository/v1.0.0".equals(reader.getNamespace())) + if ("http://www.osgi.org/xmlns/repository/v1.0.0".equals(reader.getNamespace())) { // TODO there are a bunch of other methods here that create a parser, should they be updated too? // at the very least they should be made namespace-aware too, so that parsing is the same no matter // how its initiated. - return SpecXMLPullParser.parse(reader, repositoryURI); - else + return SpecXMLPullParser.parse(reader, baseUri); + } else { // We're parsing the old - repo = parse(reader); - repo.setURI(repositoryURI); - return repo; + return parse(reader); + } } public RepositoryImpl parseRepository(Reader r) throws Exception diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/RepositoryParser.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/RepositoryParser.java index 407f74f373c..3fb28cc6898 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/RepositoryParser.java +++ b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/RepositoryParser.java @@ -20,6 +20,7 @@ import java.io.InputStream; import java.io.Reader; +import java.net.URI; public abstract class RepositoryParser { @@ -71,7 +72,7 @@ public static RepositoryParser getParser() } - public abstract RepositoryImpl parseRepository(InputStream is, String repositoryURI) throws Exception; + public abstract RepositoryImpl parseRepository(InputStream is, URI baseUri) throws Exception; public abstract RepositoryImpl parseRepository(Reader r) throws Exception; diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/SpecXMLPullParser.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/SpecXMLPullParser.java index 1aa1831667e..0cfa78f5189 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/SpecXMLPullParser.java +++ b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/SpecXMLPullParser.java @@ -49,10 +49,9 @@ public class SpecXMLPullParser private static final String REQUIREMENT = "requirement"; private static final String RESOURCE = "resource"; - public static RepositoryImpl parse(XmlPullParser reader, String repositoryURI) throws Exception + public static RepositoryImpl parse(XmlPullParser reader, URI baseUri) throws Exception { RepositoryImpl repository = new RepositoryImpl(); - repository.setURI(repositoryURI); for (int i = 0, ac = reader.getAttributeCount(); i < ac; i++) { String name = reader.getAttributeName(i); @@ -73,7 +72,7 @@ else if (INCREMENT.equals(name)) } else if (RESOURCE.equals(element)) { - Resource resource = parseResource(reader, repositoryURI); + Resource resource = parseResource(reader, baseUri); repository.addResource(resource); } else @@ -86,7 +85,7 @@ else if (RESOURCE.equals(element)) return repository; } - private static Resource parseResource(XmlPullParser reader, String repositoryURI) throws Exception + private static Resource parseResource(XmlPullParser reader, URI baseUri) throws Exception { ResourceImpl resource = new ResourceImpl(); try @@ -97,7 +96,7 @@ private static Resource parseResource(XmlPullParser reader, String repositoryURI String element = reader.getName(); if (CAPABILITY.equals(element)) { - Capability capability = parseCapability(reader, resource, repositoryURI); + Capability capability = parseCapability(reader, resource, baseUri); if (capability != null) resource.addCapability(capability); } @@ -123,7 +122,7 @@ else if (REQUIREMENT.equals(element)) } } - private static Capability parseCapability(XmlPullParser reader, ResourceImpl resource, String repositoryURI) throws Exception + private static Capability parseCapability(XmlPullParser reader, ResourceImpl resource, URI baseUri) throws Exception { String namespace = reader.getAttributeValue(null, NAMESPACE); if (IdentityNamespace.IDENTITY_NAMESPACE.equals(namespace)) @@ -135,7 +134,7 @@ private static Capability parseCapability(XmlPullParser reader, ResourceImpl res { if (resource.getURI() == null) { - parseContentNamespace(reader, resource, repositoryURI); + parseContentNamespace(reader, resource, baseUri); return null; } // if the URI is already set, this is a second osgi.content capability. @@ -189,11 +188,10 @@ private static void parseIdentityNamespace(XmlPullParser reader, ResourceImpl re } } - private static void parseContentNamespace(XmlPullParser reader, ResourceImpl resource, String repositoryURI) throws Exception + private static void parseContentNamespace(XmlPullParser reader, ResourceImpl resource, URI baseUri) throws Exception { Map attributes = new HashMap(); parseAttributesDirectives(reader, attributes, new HashMap(), CAPABILITY); - URI repo = URI.create(repositoryURI); for (Map.Entry entry : attributes.entrySet()) { @@ -202,9 +200,11 @@ private static void parseContentNamespace(XmlPullParser reader, ResourceImpl res continue; else if (ContentNamespace.CAPABILITY_URL_ATTRIBUTE.equals(entry.getKey())) { String value = (String) entry.getValue(); - URI uri = URI.create(value); - URI resourceURI = uri.isAbsolute()? uri:URI.create(repo.toString().concat(value)); - resource.put(Resource.URI, resourceURI); + URI resourceUri = URI.create(value); + if (!resourceUri.isAbsolute()) { + resourceUri = URI.create(new StringBuilder(baseUri.toString()).append(value).toString()); + } + resource.put(Resource.URI, resourceUri); } else resource.put(entry.getKey(), entry.getValue()); diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/StaxParser.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/StaxParser.java index e040bb704c1..7f4a5f912e8 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/StaxParser.java +++ b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/StaxParser.java @@ -20,6 +20,8 @@ import java.io.InputStream; import java.io.Reader; +import java.net.URI; + import javax.xml.stream.Location; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamConstants; @@ -69,7 +71,7 @@ protected static boolean setProperty(XMLInputFactory factory, String name, boole return false; } - public RepositoryImpl parseRepository(InputStream is, String repositoryURI) throws Exception + public RepositoryImpl parseRepository(InputStream is, URI baseUri) throws Exception { XMLStreamReader reader = getFactory().createXMLStreamReader(is); int event = reader.nextTag(); @@ -77,9 +79,7 @@ public RepositoryImpl parseRepository(InputStream is, String repositoryURI) thro { throw new Exception("Expected element 'repository' at the root of the document"); } - RepositoryImpl repo = parseRepository(reader); - repo.setURI(repositoryURI); - return repo; + return parseRepository(reader); } public RepositoryImpl parseRepository(Reader r) throws Exception diff --git a/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/OSGiRepositoryXMLTest.java b/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/OSGiRepositoryXMLTest.java index 1181649f4e3..b732fdc2acf 100644 --- a/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/OSGiRepositoryXMLTest.java +++ b/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/OSGiRepositoryXMLTest.java @@ -71,7 +71,7 @@ public void testIdentityCapability() throws Exception { public void testIdentityCapabilityWithRelativePath() throws Exception { URL url = getClass().getResource("/spec_repository.xml"); RepositoryAdminImpl repoAdmin = createRepositoryAdmin(); - repoAdmin.addRepository(url); + RepositoryImpl repository = (RepositoryImpl) repoAdmin.addRepository(url); Resolver resolver = repoAdmin.resolver(); @@ -86,15 +86,17 @@ public void testIdentityCapabilityWithRelativePath() throws Exception { org.apache.felix.bundlerepository.Resource[] resources = resolver.getAddedResources(); assertNotNull(resources[0]); - String repostr = url.toExternalForm().substring(0, url.toExternalForm().lastIndexOf('/')+1); - assertEquals(repostr+"repo_files/test_file_6.jar", resources[0].getURI()); - + + String repositoryUri = repository.getURI(); + String baseUri = repositoryUri.substring(0, repositoryUri.lastIndexOf('/') + 1); + String resourceUri = new StringBuilder(baseUri).append("repo_files/test_file_6.jar").toString(); + assertEquals(resourceUri, resources[0].getURI()); } public void testIdentityCapabilityForZipWithRelativePath() throws Exception { URL url = getClass().getResource("/spec_repository.zip"); RepositoryAdminImpl repoAdmin = createRepositoryAdmin(); - repoAdmin.addRepository(url); + RepositoryImpl repository = (RepositoryImpl) repoAdmin.addRepository(url); Resolver resolver = repoAdmin.resolver(); @@ -109,9 +111,11 @@ public void testIdentityCapabilityForZipWithRelativePath() throws Exception { org.apache.felix.bundlerepository.Resource[] resources = resolver.getAddedResources(); assertNotNull(resources[0]); - String repostr = url.toExternalForm().substring(0, url.toExternalForm().lastIndexOf('/')+1); - assertEquals("jar:"+ repostr+"spec_repository.zip!/repo_files/test_file_6.jar", resources[0].getURI()); - + + String repositoryUri = repository.getURI(); + String baseUri = new StringBuilder("jar:").append(repositoryUri).append("!/").toString(); + String resourceUri = new StringBuilder(baseUri).append("repo_files/test_file_6.jar").toString(); + assertEquals(resourceUri, resources[0].getURI()); } diff --git a/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/RepositoryAdminTest.java b/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/RepositoryAdminTest.java index df6f0997d13..68133a092df 100644 --- a/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/RepositoryAdminTest.java +++ b/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/RepositoryAdminTest.java @@ -24,6 +24,7 @@ import junit.framework.TestCase; +import org.apache.felix.bundlerepository.Repository; import org.apache.felix.bundlerepository.Resource; import org.apache.felix.utils.filter.FilterImpl; import org.apache.felix.utils.log.Logger; @@ -55,6 +56,22 @@ public void testResourceFilterOnCapabilities() throws Exception assertNotNull(resources); assertEquals(1, resources.length); } + + public void testRemoveRepository() throws Exception { + URL url = getClass().getResource("/repo_for_resolvertest.xml"); + + RepositoryAdminImpl repoAdmin = createRepositoryAdmin(); + Repository repository = repoAdmin.addRepository(url); + assertNotNull(repository); + + String repositoryUri = repository.getURI(); + assertNotNull(repositoryUri); + + assertTrue(repoAdmin.removeRepository(repositoryUri)); + for (Repository repo : repoAdmin.listRepositories()) { + assertNotSame(repositoryUri, repo.getURI()); + } + } private RepositoryAdminImpl createRepositoryAdmin() throws Exception {