Index: command/src/main/java/org/apache/karaf/features/command/ListFeaturesCommand.java =================================================================== --- command/src/main/java/org/apache/karaf/features/command/ListFeaturesCommand.java (revision 1098477) +++ command/src/main/java/org/apache/karaf/features/command/ListFeaturesCommand.java (working copy) @@ -40,6 +40,10 @@ private static final String NAME = "Name"; private static final String REPOSITORY = "Repository"; private static final String DESCRIPTION = "Description"; + + private int getLength(String st) { + return (st==null) ? 0 : st.length(); + } protected void doExecute(FeaturesService admin) throws Exception { @@ -67,15 +71,15 @@ // Print column headers. int maxVersionSize = VERSION.length(); for (Feature f : features) { - maxVersionSize = Math.max(maxVersionSize, f.getVersion().length()); + maxVersionSize = Math.max(maxVersionSize, getLength(f.getVersion())); } int maxNameSize = NAME.length(); for (Feature f : features) { - maxNameSize = Math.max(maxNameSize, f.getName().length()); + maxNameSize = Math.max(maxNameSize, getLength(f.getName())); } int maxRepositorySize = REPOSITORY.length(); for (Repository repository:repositories) { - maxRepositorySize = Math.max(maxRepositorySize, repository.getName().length()); + maxRepositorySize = Math.max(maxRepositorySize, getLength(repository.getName())); } StringBuilder sb = new StringBuilder(); @@ -123,10 +127,13 @@ sb.append(" "); String name = repositories.get(0).getName(); + if (name == null) { + name = ""; + } sb.append(name); repositories.remove(0); - if (name.charAt(name.length() - 1) == '*') { + if (name.length() > 1 && name.charAt(name.length() - 1) == '*') { needsLegend = true; } Index: core/pom.xml =================================================================== --- core/pom.xml (revision 1098477) +++ core/pom.xml (working copy) @@ -77,12 +77,11 @@ easymock test - - - - - - + + org.slf4j + slf4j-jdk14 + test + Index: core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java =================================================================== --- core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java (revision 1098477) +++ core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java (working copy) @@ -334,27 +334,7 @@ } // Start all bundles for (Bundle b : state.bundles) { - // do not start fragment bundles - Dictionary d = b.getHeaders(); - String fragmentHostHeader = (String) d.get(Constants.FRAGMENT_HOST); - if (fragmentHostHeader == null || fragmentHostHeader.trim().length() == 0) { - // do not start bundles that are persistently stopped - if (state.installed.contains(b) - || (b.getState() != Bundle.STARTING && b.getState() != Bundle.ACTIVE - && getStartLevel().isBundlePersistentlyStarted(b))) { - // do no start bundles when user request it - Long bundleId = b.getBundleId(); - BundleInfo bundleInfo = state.bundleInfos.get(bundleId); - if (bundleInfo == null || bundleInfo.isStart()) { - try { - b.start(); - } catch (BundleException be) { - String msg = format("Could not start bundle %s in feature(s) %s: %s", b.getLocation(), getFeaturesContainingBundleList(b), be.getMessage()); - throw new Exception(msg, be); - } - } - } - } + startBundle(state, b); } // Clean up for batch if (!options.contains(Option.NoCleanIfFailure)) { @@ -368,45 +348,76 @@ } } } catch (Exception e) { - // cleanup on error - if (!options.contains(Option.NoCleanIfFailure)) { - // Uninstall everything - for (Bundle b : state.installed) { - try { - b.uninstall(); - } catch (Exception e2) { - // Ignore - } - } - for (Bundle b : failure.installed) { - try { - b.uninstall(); - } catch (Exception e2) { - // Ignore - } - } - } else { - // Force start of bundles so that they are flagged as persistently started - for (Bundle b : state.installed) { - try { - b.start(); - } catch (Exception e2) { - // Ignore - } - } + boolean noCleanIfFailure = options.contains(Option.NoCleanIfFailure); + cleanUpOnFailure(state, failure, noCleanIfFailure); + throw e; + } finally { + for (Feature f : features) { + callListeners(new FeatureEvent(f, FeatureEvent.EventType.FeatureInstalled, false)); } - // rethrow exception - throw e; + for (Map.Entry> e : state.features.entrySet()) { + installed.put(e.getKey(), e.getValue()); + } + saveState(); } - for (Feature f : features) { - callListeners(new FeatureEvent(f, FeatureEvent.EventType.FeatureInstalled, false)); - } - for (Map.Entry> e : state.features.entrySet()) { - installed.put(e.getKey(), e.getValue()); - } - saveState(); } + private void startBundle(InstallationState state, Bundle b) + throws Exception { + // do not start fragment bundles + Dictionary d = b.getHeaders(); + String fragmentHostHeader = (String) d.get(Constants.FRAGMENT_HOST); + if (fragmentHostHeader == null || fragmentHostHeader.trim().length() == 0) { + // do not start bundles that are persistently stopped + if (state.installed.contains(b) + || (b.getState() != Bundle.STARTING && b.getState() != Bundle.ACTIVE + && getStartLevel().isBundlePersistentlyStarted(b))) { + // do no start bundles when user request it + Long bundleId = b.getBundleId(); + BundleInfo bundleInfo = state.bundleInfos.get(bundleId); + if (bundleInfo == null || bundleInfo.isStart()) { + try { + b.start(); + } catch (BundleException be) { + String msg = format("Could not start bundle %s in feature(s) %s: %s", b.getLocation(), getFeaturesContainingBundleList(b), be.getMessage()); + throw new Exception(msg, be); + } + } + } + } + } + + private void cleanUpOnFailure(InstallationState state, + InstallationState failure, boolean noCleanIfFailure) { + // cleanup on error + if (!noCleanIfFailure) { + // Uninstall everything + for (Bundle b : state.installed) { + try { + b.uninstall(); + } catch (Exception e2) { + // Ignore + } + } + for (Bundle b : failure.installed) { + try { + b.uninstall(); + } catch (Exception e2) { + // Ignore + } + } + } else { + // Force start of bundles so that they are flagged as persistently started + for (Bundle b : state.installed) { + try { + b.start(); + } catch (Exception e2) { + // Ignore + } + } + } + } + protected static class InstallationState { final Set installed = new HashSet(); final List bundles = new ArrayList(); Index: core/src/main/java/org/apache/karaf/features/internal/FeatureValidationUtil.java =================================================================== --- core/src/main/java/org/apache/karaf/features/internal/FeatureValidationUtil.java (revision 1098477) +++ core/src/main/java/org/apache/karaf/features/internal/FeatureValidationUtil.java (working copy) @@ -27,6 +27,8 @@ import javax.xml.validation.SchemaFactory; import javax.xml.validation.Validator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.w3c.dom.Document; /** @@ -35,6 +37,7 @@ * @author ldywicki */ public class FeatureValidationUtil { + private static final Logger log = LoggerFactory.getLogger(FeatureValidationUtil.class); /** * Runs schema validation. @@ -53,7 +56,8 @@ dFactory.setNamespaceAware(true); Document doc = dFactory.newDocumentBuilder().parse(stream); - if (doc.getDocumentElement().getNamespaceURI() == null) { + if ("features".equals(doc.getDocumentElement().getNodeName()) && doc.getDocumentElement().getNamespaceURI() == null) { + log.warn("Old style feature file without namespace found (URI: {}). This format is deprecated and support for it will soon be removed", uri); return; } Index: core/src/main/java/org/apache/karaf/features/internal/model/JaxbUtil.java =================================================================== --- core/src/main/java/org/apache/karaf/features/internal/model/JaxbUtil.java (revision 1098477) +++ core/src/main/java/org/apache/karaf/features/internal/model/JaxbUtil.java (working copy) @@ -48,7 +48,7 @@ import org.xml.sax.helpers.XMLFilterImpl; /** - * @version $Rev:$ $Date:$ + * @version $Rev$ $Date$ */ public class JaxbUtil { @@ -62,15 +62,6 @@ } } - public static Features unmarshal(InputStream in, boolean validate) throws XMLStreamException, JAXBException { - XMLStreamReader xmlStream = XMLINPUT_FACTORY.createXMLStreamReader(in); - Unmarshaller unmarshaller = FEATURES_CONTEXT.createUnmarshaller(); - JAXBElement element = unmarshaller.unmarshal(xmlStream, Features.class); - Features features = element.getValue(); - return features; - - } - public static void marshal(Class type, Object object, OutputStream out) throws JAXBException { JAXBContext ctx2 = JAXBContext.newInstance(type); Marshaller marshaller = ctx2.createMarshaller(); @@ -102,15 +93,18 @@ * @throws SAXException if there is an xml problem * @throws JAXBException if the xml cannot be marshalled into a T. */ - public static T unmarshal(Class type, InputStream in, boolean validate) throws ParserConfigurationException, SAXException, JAXBException { + public static Features unmarshal(InputStream in, boolean validate) { InputSource inputSource = new InputSource(in); SAXParserFactory factory = SAXParserFactory.newInstance(); factory.setNamespaceAware(true); factory.setValidating(validate); - SAXParser parser = factory.newSAXParser(); + SAXParser parser; + try { + parser = factory.newSAXParser(); + - JAXBContext ctx = JAXBContext.newInstance(type); + JAXBContext ctx = JAXBContext.newInstance(Features.class); Unmarshaller unmarshaller = ctx.createUnmarshaller(); unmarshaller.setEventHandler(new ValidationEventHandler() { public boolean handleEvent(ValidationEvent validationEvent) { @@ -119,18 +113,32 @@ } }); - XMLFilter xmlFilter = new NoSourceFilter(parser.getXMLReader()); + XMLFilter xmlFilter = new NoSourceAndNamespaceFilter(parser.getXMLReader()); xmlFilter.setContentHandler(unmarshaller.getUnmarshallerHandler()); SAXSource source = new SAXSource(xmlFilter, inputSource); - return type.cast(unmarshaller.unmarshal(source)); + return (Features)unmarshaller.unmarshal(source); + + } catch (ParserConfigurationException e) { + throw new RuntimeException(e); + } catch (JAXBException e) { + throw new RuntimeException(e); + } catch (SAXException e) { + throw new RuntimeException(e); + } } - public static class NoSourceFilter extends XMLFilterImpl { + /** + * Provides an empty inputsource for the entity resolver. + * Converts all elements with empty namespace to the features namespace to make old feature files + * compatible to the new format + */ + public static class NoSourceAndNamespaceFilter extends XMLFilterImpl { + private static final String FEATURES_NAMESPACE = "http://karaf.apache.org/xmlns/features/v1.0.0"; private static final InputSource EMPTY_INPUT_SOURCE = new InputSource(new ByteArrayInputStream(new byte[0])); - public NoSourceFilter(XMLReader xmlReader) { + public NoSourceAndNamespaceFilter(XMLReader xmlReader) { super(xmlReader); } @@ -141,12 +149,20 @@ @Override public void startElement(String uri, String localName, String qname, Attributes atts) throws SAXException { - super.startElement("http://karaf.apache.org/xmlns/features/v1.0.0", localName, qname, atts); + if ("".equals(uri)) { + super.startElement(FEATURES_NAMESPACE, localName, qname, atts); + } else { + super.startElement(uri, localName, qname, atts); + } } @Override public void endElement(String uri, String localName, String qName) throws SAXException { - super.endElement("http://karaf.apache.org/xmlns/features/v1.0.0", localName, qName); + if ("".equals(uri)) { + super.endElement(FEATURES_NAMESPACE, localName, qName); + } else { + super.endElement(uri, localName, qName); + } } } Index: core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java =================================================================== --- core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java (revision 1098477) +++ core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java (working copy) @@ -46,11 +46,13 @@ import org.apache.karaf.features.internal.FeaturesServiceImpl; import org.easymock.EasyMock; +import org.junit.Assert; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; import org.osgi.service.log.LogService; import org.osgi.service.packageadmin.PackageAdmin; +import org.xml.sax.SAXParseException; import static org.easymock.EasyMock.*; @@ -872,7 +874,7 @@ File tmp = File.createTempFile("smx", ".feature"); PrintWriter pw = new PrintWriter(new FileWriter(tmp)); - pw.println(""); + pw.println(""); pw.println(" "); pw.println(" " + bundle1 + ""); pw.println(" " + bundle2 + ""); @@ -971,13 +973,17 @@ /** * This test checks feature service behavior with old, non namespaced descriptor. */ - public void testNoSchemaValidation() throws Exception { + public void testLoadOldFeatureFile() throws Exception { + String bundle1 = getJarUrl(LogService.class); + String bundle2 = getJarUrl(Bundle.class); + File tmp = File.createTempFile("smx", ".feature"); PrintWriter pw = new PrintWriter(new FileWriter(tmp)); pw.println(""); - pw.println(" "); - pw.println(" anotherBundle"); - pw.println(" "); + pw.println(" "); + pw.println(" " + bundle1 + ""); + pw.println(" " + bundle2 + ""); + pw.println(" "); pw.println(""); pw.close(); @@ -990,6 +996,10 @@ FeaturesServiceImpl svc = new FeaturesServiceImpl(); svc.setBundleContext(bundleContext); svc.addRepository(uri); + Feature feature = svc.getFeature("f1"); + Assert.assertNotNull("No feature named fi found", feature); + List bundles = feature.getBundles(); + Assert.assertEquals(2, bundles.size()); } private String getJarUrl(Class cl) {