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) @@ -33,7 +33,7 @@ boolean installed; private static final String STATE = "State"; - private static final String INSTALLED = "installed "; + private static final String INSTALLED = "installed"; private static final String UNINSTALLED = "uninstalled"; private static final String VERSION = "Version"; @@ -42,20 +42,29 @@ private static final String DESCRIPTION = "Description"; protected void doExecute(FeaturesService admin) throws Exception { + List lines = new ArrayList(); + boolean needsLegend = false; // Get the feature data to print. - List features = new ArrayList(); - List repositories = new ArrayList(); for (Repository r : Arrays.asList(admin.listRepositories())) { for (Feature f : r.getFeatures()) { if (installed && !admin.isInstalled(f)) { + // Filter out not installed features if we only want to see the installed ones continue; } - features.add(f); - repositories.add(r); + InfoLine line = new InfoLine(); + line.state = admin.isInstalled(f) ? INSTALLED : UNINSTALLED; + line.version = getSafeString(f.getVersion()); + line.name = getSafeString(f.getName()); + line.repoName = getSafeString(r.getName()); + line.description = getSafeString(f.getDescription()); + if (isInstalledViaDeployDir(line.repoName)) { + needsLegend = true; + } + lines.add(line); } } - if (features.size() == 0) { + if (lines.size() == 0) { if (installed) { System.out.println("No features installed."); } else { @@ -64,90 +73,44 @@ return; } - // Print column headers. + // Determine size of columns int maxVersionSize = VERSION.length(); - for (Feature f : features) { - maxVersionSize = Math.max(maxVersionSize, f.getVersion().length()); - } int maxNameSize = NAME.length(); - for (Feature f : features) { - maxNameSize = Math.max(maxNameSize, f.getName().length()); - } int maxRepositorySize = REPOSITORY.length(); - for (Repository repository:repositories) { - maxRepositorySize = Math.max(maxRepositorySize, repository.getName().length()); + for (InfoLine line : lines) { + maxVersionSize = Math.max(maxVersionSize, line.version.length()); + maxNameSize = Math.max(maxNameSize, line.name.length()); + maxRepositorySize = Math.max(maxRepositorySize, line.repoName.length()); } - StringBuilder sb = new StringBuilder(); - sb.append(STATE).append(" ").append(VERSION).append(" "); - for (int i = VERSION.length(); i < maxVersionSize; i++) { - sb.append(" "); + // Print feature info in columns + String formatHeader = "%-13s %-17s %-" + maxNameSize + "s %-" + maxRepositorySize + "s %s"; + String formatLine = "[%-11s] [%-15s] %-" + maxNameSize + "s %-" + maxRepositorySize + "s %s"; + System.out.println(String.format(formatHeader, STATE, VERSION, NAME, REPOSITORY, DESCRIPTION)); + for (InfoLine line : lines) { + System.out.println(String.format(formatLine, line.state, line.version, line.name, line.repoName, line.description)); } - sb.append(NAME).append(" "); - for (int i = NAME.length(); i < maxNameSize; i++) { - sb.append(" "); - } - sb.append(REPOSITORY).append(" "); - for (int i = REPOSITORY.length(); i < maxRepositorySize; i++) { - sb.append(" "); - } - sb.append(DESCRIPTION); - System.out.println(sb.toString()); - - // Print the feature data. - boolean needsLegend = false; - for (Feature f : features) { - - sb.setLength(0); - sb.append("["); - if (admin.isInstalled(f)) { - sb.append(INSTALLED); - } else { - sb.append(UNINSTALLED); - } - - sb.append("] ["); - String str = f.getVersion(); - sb.append(str); - for (int i = str.length(); i < maxVersionSize; i++) { - sb.append(" "); - } - sb.append("] "); - - str = f.getName(); - sb.append(str); - for (int i = str.length(); i < maxNameSize; i++) { - sb.append(" "); - } - - sb.append(" "); - String name = repositories.get(0).getName(); - sb.append(name); - repositories.remove(0); - - if (name.charAt(name.length() - 1) == '*') { - needsLegend = true; - } - - for (int i = name.length(); i < maxRepositorySize; i++) { - sb.append(" "); - } - - sb.append(" "); - String description = ""; - if(f.getDescription() != null) { - description = f.getDescription(); - } - sb.append(description); - - System.out.println(sb.toString()); - } - if (needsLegend) { System.out.println("* Installed via deploy directory"); } } + private String getSafeString(String st) { + return st == null ? "" : st; + } + + private boolean isInstalledViaDeployDir(String st) { + return (st == null || st.length() <= 1) ? false : (st.charAt(st.length() - 1) == '*'); + } + + private class InfoLine { + String state; + String version; + String name; + String repoName; + String description; + } + } 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) {