From: Prasanth Pallamreddy Date: Wed, 30 Oct 2013 19:34:03 +0000 (-0700) Subject: Handle conflicting types in BundleScanner X-Git-Tag: jenkins-controller-bulk-release-prepare-only-2-1~517 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=9845fed57093178ff55d3c8c13b7f218feabd458 Handle conflicting types in BundleScanner - Change precedence order for JAXB types - Add a validation check to report conflicting types - Add exclude capability to explicitly exclude types Change-Id: I734be4ea72100d59ddc02d26072abad2dc5b0e9e Signed-off-by: Prasanth Pallamreddy --- diff --git a/opendaylight/northbound/bundlescanner/api/src/main/java/org/opendaylight/controller/northbound/bundlescanner/IBundleScanService.java b/opendaylight/northbound/bundlescanner/api/src/main/java/org/opendaylight/controller/northbound/bundlescanner/IBundleScanService.java index 5cb0b63f1c..dde205acd6 100644 --- a/opendaylight/northbound/bundlescanner/api/src/main/java/org/opendaylight/controller/northbound/bundlescanner/IBundleScanService.java +++ b/opendaylight/northbound/bundlescanner/api/src/main/java/org/opendaylight/controller/northbound/bundlescanner/IBundleScanService.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.northbound.bundlescanner; import java.util.List; +import java.util.Set; import org.osgi.framework.BundleContext; @@ -30,5 +31,6 @@ public interface IBundleScanService { public List> getAnnotatedClasses( BundleContext context, String[] annotations, + Set excludes, boolean includeDependentBundleClasses); } diff --git a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleInfo.java b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleInfo.java index 4e94c5f845..9cb1cb52ed 100644 --- a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleInfo.java +++ b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleInfo.java @@ -62,14 +62,14 @@ import org.slf4j.LoggerFactory; return bundle.getBundleId(); } - public List> getAnnotatedClasses(Pattern pattern) { + public List> getAnnotatedClasses(Pattern pattern, Set excludes) { List result = new ArrayList(); for (Map.Entry> entry : annotatedClasses.entrySet()) { if (matches(pattern, entry.getValue())) { result.add(entry.getKey()); } } - return BundleScanner.loadClasses(result, bundle); + return BundleScanner.loadClasses(result, bundle, excludes); } private boolean matches(Pattern pattern, Set values) { @@ -87,16 +87,18 @@ import org.slf4j.LoggerFactory; * @param allbundles - all bundles * @param pattern - annotation pattern to match * @param initBundle - the bundle which initiated this call + * @param excludes - set of class names to be excluded * * @return list of annotated classes matching the pattern */ public List> getAnnotatedClasses( Collection allbundles, - Pattern pattern, Bundle initBundle) + Pattern pattern, Bundle initBundle, + Set excludes) { - List> classes = getAnnotatedClasses(pattern); + List> classes = getAnnotatedClasses(pattern, excludes); processAnnotatedClassesInternal(this, allbundles, pattern, - new HashSet(), classes, initBundle); + new HashSet(), classes, initBundle, excludes); return classes; } @@ -125,17 +127,18 @@ import org.slf4j.LoggerFactory; Pattern pattern, Collection visited, List> classes, - Bundle initBundle) + Bundle initBundle, Set excludes) { for (BundleInfo other : bundlesToScan) { if (other.getId() == target.getId()) continue; if (target.isDependantOn(other)) { if (!visited.contains(other)) { classes.addAll(BundleScanner.loadClasses( - other.getExportedAnnotatedClasses(pattern), initBundle)); + other.getExportedAnnotatedClasses(pattern), + initBundle, excludes)); visited.add(other); processAnnotatedClassesInternal(other, bundlesToScan, - pattern, visited, classes, initBundle); + pattern, visited, classes, initBundle, excludes); } } } diff --git a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanServiceImpl.java b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanServiceImpl.java index ad6d8e9a17..751b597e78 100644 --- a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanServiceImpl.java +++ b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanServiceImpl.java @@ -1,6 +1,7 @@ package org.opendaylight.controller.northbound.bundlescanner.internal; import java.util.List; +import java.util.Set; import org.opendaylight.controller.northbound.bundlescanner.IBundleScanService; import org.osgi.framework.BundleContext; @@ -13,10 +14,11 @@ public class BundleScanServiceImpl implements IBundleScanService { @Override public List> getAnnotatedClasses(BundleContext context, String[] annotations, + Set excludes, boolean includeDependentBundleClasses) { return BundleScanner.getInstance().getAnnotatedClasses( - context, annotations, includeDependentBundleClasses); + context, annotations, excludes, includeDependentBundleClasses); } } diff --git a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanner.java b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanner.java index a5a2073a61..0c64082059 100644 --- a/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanner.java +++ b/opendaylight/northbound/bundlescanner/implementation/src/main/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScanner.java @@ -22,6 +22,8 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; +import javax.xml.bind.annotation.XmlRootElement; + import org.objectweb.asm.AnnotationVisitor; import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassVisitor; @@ -66,6 +68,7 @@ import org.slf4j.LoggerFactory; public List> getAnnotatedClasses(BundleContext context, String[] annotations, + Set excludes, boolean includeDependentBundleClasses) { BundleInfo info = bundleAnnotations.get(context.getBundle().getBundleId()); @@ -74,9 +77,14 @@ import org.slf4j.LoggerFactory; List> result = null; if (includeDependentBundleClasses) { result = info.getAnnotatedClasses(bundleAnnotations.values(), - pattern, context.getBundle()); + pattern, context.getBundle(), excludes); + // reverse the list to give precedence to the types loaded from the + // initiating bundle + Collections.reverse(result); + // validate for conflicts only when searching dependencies + validate(result); } else { - result = info.getAnnotatedClasses(pattern); + result = info.getAnnotatedClasses(pattern, excludes); } LOGGER.debug("Annotated classes detected: {} matching: {}", result, pattern); return result; @@ -240,12 +248,13 @@ import org.slf4j.LoggerFactory; public static List> loadClasses( Collection annotatedClasses, - Bundle initBundle) + Bundle initBundle, Set excludes) { List> result = new ArrayList>(); StringBuilder errors = new StringBuilder(); for (String name : annotatedClasses) { try { + if (excludes != null && excludes.contains(name)) continue; result.add(initBundle.loadClass(name)); } catch (ClassNotFoundException e) { errors.append(name).append(", "); @@ -281,4 +290,31 @@ import org.slf4j.LoggerFactory; return Pattern.compile(regex.toString()); } + private void validate(List> classes) { + if (classes == null || classes.size() == 0) return; + Map names = new HashMap(); + StringBuilder conflictsMsg = new StringBuilder(); + for (Class c : classes) { + XmlRootElement root = (XmlRootElement) c.getAnnotation(XmlRootElement.class); + if (root == null) continue; + String rootName = root.name(); + if ("##default".equals(rootName)) { + String clsName = c.getSimpleName(); + rootName = Character.toLowerCase(clsName.charAt(0)) + clsName.substring(1); + } + String other = names.get(rootName); + if (other != null && !other.equals(c.getName())) { + conflictsMsg.append(System.lineSeparator()) + .append("[").append(rootName).append(":") + .append(c.getName()).append(",").append(other) + .append("]"); + } else { + names.put(rootName, c.getName()); + } + } + if (conflictsMsg.length() > 0) { + LOGGER.info("JAXB type conflicts detected : {}", conflictsMsg.toString()); + } + } + } diff --git a/opendaylight/northbound/bundlescanner/implementation/src/test/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScannerTest.java b/opendaylight/northbound/bundlescanner/implementation/src/test/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScannerTest.java index 670ba9eae1..38d361c824 100644 --- a/opendaylight/northbound/bundlescanner/implementation/src/test/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScannerTest.java +++ b/opendaylight/northbound/bundlescanner/implementation/src/test/java/org/opendaylight/controller/northbound/bundlescanner/internal/BundleScannerTest.java @@ -11,8 +11,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Dictionary; import java.util.Enumeration; +import java.util.HashSet; import java.util.Hashtable; import java.util.List; +import java.util.Set; import java.util.regex.Pattern; import org.junit.AfterClass; @@ -63,18 +65,18 @@ public class BundleScannerTest { public void testBundleEvents() throws Exception { MockBundle newBundle = new TestMockBundle("misc", "", "bundle_misc"); assertTrue(bundleScanner.getAnnotatedClasses( - newBundle.getBundleContext(), null, false).size() == 0); + newBundle.getBundleContext(), null, null, false).size() == 0); BundleEvent event = new BundleEvent(BundleEvent.RESOLVED, newBundle); bundleScanner.bundleChanged(event); assertTrue(bundleScanner.getAnnotatedClasses( - newBundle.getBundleContext(), null, false).size() == 1); + newBundle.getBundleContext(), null, null, false).size() == 1); } @Test public void testAnnotatedClassesWithDependencies() throws Exception { for (Bundle bundle : bundles) { List> classes = bundleScanner.getAnnotatedClasses( - bundle.getBundleContext(), null, true); + bundle.getBundleContext(), null, null, true); String name = bundle.getSymbolicName(); System.out.println("name:" + name + " classes:" + classes.size()); if ("misc".equals(name)) { @@ -94,7 +96,7 @@ public class BundleScannerTest { Bundle bundle = findBundle("sub1"); String[] annos = { "javax.xml.bind.annotation.XmlTransient" }; List> classes = bundleScanner.getAnnotatedClasses( - bundle.getBundleContext(), annos, true); + bundle.getBundleContext(), annos, null, true); assertTrue(classes.size() == 1); } @@ -103,7 +105,7 @@ public class BundleScannerTest { Bundle bundle = findBundle("sub1"); String[] annos = { "javax.xml.bind.annotation.*" }; List> classes = bundleScanner.getAnnotatedClasses( - bundle.getBundleContext(), annos, true); + bundle.getBundleContext(), annos, null, true); assertTrue(classes.size() == 6); } @@ -112,7 +114,7 @@ public class BundleScannerTest { Bundle bundle = findBundle("sub1"); String[] annos = { "non.existent.pkg" }; List> classes = bundleScanner.getAnnotatedClasses( - bundle.getBundleContext(), annos, true); + bundle.getBundleContext(), annos, null, true); assertTrue(classes.size() == 0); } @@ -129,6 +131,17 @@ public class BundleScannerTest { assertFalse(pattern.matcher("Ljavax/servlet/FOO;").find()); } + @Test + public void testExclude() { + Set excludes = new HashSet(); + excludes.add("bundle_base.Animal"); + Bundle bundle = findBundle("sub1"); + String[] annos = { "javax.xml.bind.annotation.*" }; + List> classes = bundleScanner.getAnnotatedClasses( + bundle.getBundleContext(), annos, excludes, true); + assertTrue(classes.size() == 5); + } + private static Bundle findBundle(String symName) { for (Bundle bundle : bundles) { if (bundle.getSymbolicName().equals(symName)) return bundle; diff --git a/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/NorthboundApplication.java b/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/NorthboundApplication.java index 5b8219126b..e97a562620 100644 --- a/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/NorthboundApplication.java +++ b/opendaylight/northbound/commons/src/main/java/org/opendaylight/controller/northbound/commons/NorthboundApplication.java @@ -39,7 +39,23 @@ import org.slf4j.LoggerFactory; @SuppressWarnings("unchecked") public class NorthboundApplication extends Application { public static final String JAXRS_RESOURCES_MANIFEST_NAME = "Jaxrs-Resources"; + public static final String JAXRS_EXCLUDES_MANIFEST_NAME = "Jaxrs-Exclude-Types"; private static final Logger LOGGER = LoggerFactory.getLogger(NorthboundApplication.class); + private final Set _singletons; + + public NorthboundApplication() { + _singletons = new HashSet(); + _singletons.add(new ContextResolver() { + JAXBContext jaxbContext = newJAXBContext(); + @Override + public JAXBContext getContext(Class type) { + return jaxbContext; + } + + } ); + _singletons.add(getJsonProvider()); + _singletons.add(new JacksonJsonProcessingExceptionMapper()); + } //////////////////////////////////////////////////////////////// // Application overrides @@ -47,17 +63,7 @@ public class NorthboundApplication extends Application { @Override public Set getSingletons() { - Set singletons = new HashSet(); - singletons.add(new ContextResolver() { - @Override - public JAXBContext getContext(Class type) { - return newJAXBContext(); - } - - } ); - singletons.add(getJsonProvider()); - singletons.add(new JacksonJsonProcessingExceptionMapper()); - return singletons; + return _singletons; } @Override @@ -102,6 +108,7 @@ public class NorthboundApplication extends Application { try { List> cls = svc.getAnnotatedClasses(ctx, new String[] { XmlRootElement.class.getPackage().getName() }, + parseManifestEntry(ctx, JAXRS_EXCLUDES_MANIFEST_NAME), true); return JAXBContext.newInstance(cls.toArray(new Class[cls.size()])); } catch (JAXBException je) { @@ -118,29 +125,22 @@ public class NorthboundApplication extends Application { try { IBundleScanService svc = lookupBundleScanner(ctx); result.addAll(svc.getAnnotatedClasses(ctx, - new String[] { javax.ws.rs.Path.class.getName() }, false)); + new String[] { javax.ws.rs.Path.class.getName() }, + null, false)); } catch (ServiceException se) { recordException = se; LOGGER.debug("Error finding JAXRS resource annotated classes in " + "bundle: {} error: {}.", bundleName, se.getMessage()); // the bundle scan service cannot be lookedup. Lets attempt to // lookup the resources from the bundle manifest header - Dictionary headers = ctx.getBundle().getHeaders(); - String header = headers.get(JAXRS_RESOURCES_MANIFEST_NAME); - if (header != null) { - for (String s : header.split(",")) { - s = s.trim(); - if (s.length() > 0) { - try { - result.add(ctx.getBundle().loadClass(s)); - } catch (ClassNotFoundException cnfe) { - LOGGER.error("Cannot load class: {} in bundle: {} " + - "defined as MANIFEST JAX-RS resource", s, bundleName, cnfe); - } - } + for (String c : parseManifestEntry(ctx, JAXRS_RESOURCES_MANIFEST_NAME)) { + try { + result.add(ctx.getBundle().loadClass(c)); + } catch (ClassNotFoundException cnfe) { + LOGGER.error("Cannot load class: {} in bundle: {} " + + "defined as MANIFEST JAX-RS resource", c, bundleName, cnfe); } } - } if (result.size() == 0) { @@ -154,4 +154,19 @@ public class NorthboundApplication extends Application { return result; } + private final Set parseManifestEntry(BundleContext ctx, String name) { + Set result = new HashSet(); + Dictionary headers = ctx.getBundle().getHeaders(); + String header = headers.get(name); + if (header != null) { + for (String s : header.split(",")) { + s = s.trim(); + if (s.length() > 0) { + result.add(s); + } + } + } + return result; + } + }