You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ju...@apache.org on 2017/03/31 19:14:49 UTC

svn commit: r1789734 - in /sling/trunk/bundles/extensions/models: impl/src/main/java/org/apache/sling/models/impl/ impl/src/test/java/org/apache/sling/models/impl/ integration-tests/

Author: justin
Date: Fri Mar 31 19:14:48 2017
New Revision: 1789734

URL: http://svn.apache.org/viewvc?rev=1789734&view=rev
Log:
SLING-6764 - Guard against reflection failures when registering Sling Model classes

Modified:
    sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
    sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
    sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
    sling/trunk/bundles/extensions/models/integration-tests/pom.xml

Modified: sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java?rev=1789734&r1=1789733&r2=1789734&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java (original)
+++ sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/AdapterImplementations.java Fri Mar 31 19:14:48 2017
@@ -102,38 +102,52 @@ final class AdapterImplementations {
     }
     
     /** Add implementation mapping for the given model class (implementation is the model class itself).
-     * Only used for testing purposes. Use {@link #add(Class, Class)} in case you want to register a different implementation.
+     * Only used for testing purposes. Use {@link #addAll(Class, Class...)} in case you want to register a different implementation.
      * @param modelClasses the model classes to register
      */
     protected void addClassesAsAdapterAndImplementation(Class<?>... modelClasses) {
         for (Class<?> modelClass : modelClasses) {
-            add(modelClass, modelClass);
+            addAll(modelClass, modelClass);
         }
     }
     
     /**
-     * Add implementation mapping for the given adapter type.
-     * @param adapterType Adapter type
+     * Add implementation mapping for the given adapter types.
      * @param implType Implementation type
+     * @param adapterTypes Adapter types
+     * @result true if adapters were successfully added
      */
     @SuppressWarnings("unchecked")
-    public void add(Class<?> adapterType, Class<?> implType) {
-        String key = adapterType.getName();
-        if (adapterType == implType) {
-            modelClasses.put(key, new ModelClass(implType, sortedStaticInjectAnnotationProcessorFactories));
+    boolean addAll(Class<?> implType, Class<?>... adapterTypes) {
+        ModelClass<?> modelClass = null;
+        try {
+            modelClass = new ModelClass(implType, sortedStaticInjectAnnotationProcessorFactories);
+        } catch (Exception e) {
+            log.warn("Unable to reflect on " + implType.getName(), e);
+            return false;
+        } catch (NoClassDefFoundError e) {
+            log.warn("Unable to reflect on " + implType.getName(), e);
+            return false;
         }
-        else {
-            // although we already use a ConcurrentMap synchronize explicitly because we apply non-atomic operations on it
-            synchronized (adapterImplementations) {
-                ConcurrentNavigableMap<String,ModelClass<?>> implementations = adapterImplementations.get(key);
-                if (implementations == null) {
-                    // to have a consistent ordering independent of bundle loading use a ConcurrentSkipListMap that sorts by class name
-                    implementations = new ConcurrentSkipListMap<String,ModelClass<?>>();
-                    adapterImplementations.put(key, implementations);
+
+        for (Class<?> adapterType : adapterTypes) {
+            String key = adapterType.getName();
+            if (adapterType == implType) {
+                modelClasses.put(key, modelClass);
+            } else {
+                // although we already use a ConcurrentMap synchronize explicitly because we apply non-atomic operations on it
+                synchronized (adapterImplementations) {
+                    ConcurrentNavigableMap<String, ModelClass<?>> implementations = adapterImplementations.get(key);
+                    if (implementations == null) {
+                        // to have a consistent ordering independent of bundle loading use a ConcurrentSkipListMap that sorts by class name
+                        implementations = new ConcurrentSkipListMap<String, ModelClass<?>>();
+                        adapterImplementations.put(key, implementations);
+                    }
+                    implementations.put(implType.getName(), modelClass);
                 }
-                implementations.put(implType.getName(), new ModelClass(implType, sortedStaticInjectAnnotationProcessorFactories));
             }
         }
+        return true;
     }
     
     /**

Modified: sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java?rev=1789734&r1=1789733&r2=1789734&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java (original)
+++ sling/trunk/bundles/extensions/models/impl/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java Fri Mar 31 19:14:48 2017
@@ -149,34 +149,33 @@ public class ModelPackageBundleListener
                 }
                 // register adapter only if given adapters are valid
                 if (validateAdapterClasses(implType, adapterTypes)) {
-                    for (Class<?> adapterType : adapterTypes) {
-                        adapterImplementations.add(adapterType, implType);
-                    }
-                    ServiceRegistration reg = registerAdapterFactory(adapterTypes, annotation.adaptables(), implType, annotation.condition());
-                    regs.add(reg);
+                    if (adapterImplementations.addAll(implType, adapterTypes)) {
+                        ServiceRegistration reg = registerAdapterFactory(adapterTypes, annotation.adaptables(), implType, annotation.condition());
+                        regs.add(reg);
 
-                    String[] resourceTypes = annotation.resourceType();
-                    for (String resourceType : resourceTypes) {
-                        if (StringUtils.isNotEmpty(resourceType)) {
-                            for (Class<?> adaptable : annotation.adaptables()) {
-                                adapterImplementations.registerModelToResourceType(bundle, resourceType, adaptable, implType);
-                                ExportServlet.ExportedObjectAccessor accessor = null;
-                                if (adaptable == Resource.class) {
-                                    accessor = new ExportServlet.ResourceAccessor(implType);
-                                } else if (adaptable == SlingHttpServletRequest.class) {
-                                    accessor = new ExportServlet.RequestAccessor(implType);
-                                }
-                                Exporter exporterAnnotation = implType.getAnnotation(Exporter.class);
-                                if (exporterAnnotation != null) {
-                                    registerExporter(bundle, implType, resourceType, exporterAnnotation, regs, accessor);
-                                }
-                                Exporters exportersAnnotation = implType.getAnnotation(Exporters.class);
-                                if (exportersAnnotation != null) {
-                                    for (Exporter ann : exportersAnnotation.value()) {
-                                        registerExporter(bundle, implType, resourceType, ann, regs, accessor);
+                        String[] resourceTypes = annotation.resourceType();
+                        for (String resourceType : resourceTypes) {
+                            if (StringUtils.isNotEmpty(resourceType)) {
+                                for (Class<?> adaptable : annotation.adaptables()) {
+                                    adapterImplementations.registerModelToResourceType(bundle, resourceType, adaptable, implType);
+                                    ExportServlet.ExportedObjectAccessor accessor = null;
+                                    if (adaptable == Resource.class) {
+                                        accessor = new ExportServlet.ResourceAccessor(implType);
+                                    } else if (adaptable == SlingHttpServletRequest.class) {
+                                        accessor = new ExportServlet.RequestAccessor(implType);
+                                    }
+                                    Exporter exporterAnnotation = implType.getAnnotation(Exporter.class);
+                                    if (exporterAnnotation != null) {
+                                        registerExporter(bundle, implType, resourceType, exporterAnnotation, regs, accessor);
+                                    }
+                                    Exporters exportersAnnotation = implType.getAnnotation(Exporters.class);
+                                    if (exportersAnnotation != null) {
+                                        for (Exporter ann : exportersAnnotation.value()) {
+                                            registerExporter(bundle, implType, resourceType, ann, regs, accessor);
+                                        }
                                     }
-                                }
 
+                                }
                             }
                         }
                     }

Modified: sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java?rev=1789734&r1=1789733&r2=1789734&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java (original)
+++ sling/trunk/bundles/extensions/models/impl/src/test/java/org/apache/sling/models/impl/AdapterImplementationsTest.java Fri Mar 31 19:14:48 2017
@@ -74,7 +74,7 @@ public class AdapterImplementationsTest
     
     @Test
     public void testSingleMapping() {
-        underTest.add(SAMPLE_ADAPTER, String.class);
+        underTest.addAll(String.class, SAMPLE_ADAPTER);
         
         assertEquals(String.class, underTest.lookup(SAMPLE_ADAPTER, SAMPLE_ADAPTABLE).getType());
         
@@ -85,9 +85,9 @@ public class AdapterImplementationsTest
 
     @Test
     public void testMultipleMappings() {
-        underTest.add(SAMPLE_ADAPTER, String.class);
-        underTest.add(SAMPLE_ADAPTER, Integer.class);
-        underTest.add(SAMPLE_ADAPTER, Long.class);
+        underTest.addAll(String.class, SAMPLE_ADAPTER);
+        underTest.addAll(Integer.class, SAMPLE_ADAPTER);
+        underTest.addAll(Long.class, SAMPLE_ADAPTER);
         
         assertEquals(Integer.class, underTest.lookup(SAMPLE_ADAPTER, SAMPLE_ADAPTABLE).getType());
         
@@ -103,9 +103,9 @@ public class AdapterImplementationsTest
     
     @Test
     public void testRemoveAll() {
-        underTest.add(SAMPLE_ADAPTER, String.class);
-        underTest.add(SAMPLE_ADAPTER, Integer.class);
-        underTest.add(SAMPLE_ADAPTER, Long.class);
+        underTest.addAll(String.class, SAMPLE_ADAPTER);
+        underTest.addAll(Integer.class, SAMPLE_ADAPTER);
+        underTest.addAll(Long.class, SAMPLE_ADAPTER);
         
         underTest.removeAll();
         
@@ -120,16 +120,16 @@ public class AdapterImplementationsTest
             new FirstImplementationPicker()
         ));
 
-        underTest.add(SAMPLE_ADAPTER, String.class);
-        underTest.add(SAMPLE_ADAPTER, Integer.class);
-        underTest.add(SAMPLE_ADAPTER, Long.class);
+        underTest.addAll(String.class, SAMPLE_ADAPTER);
+        underTest.addAll(Integer.class, SAMPLE_ADAPTER);
+        underTest.addAll(Long.class, SAMPLE_ADAPTER);
         
         assertEquals(String.class, underTest.lookup(SAMPLE_ADAPTER, SAMPLE_ADAPTABLE).getType());
     }
     
     @Test
     public void testSimpleModel() {
-        underTest.add(SAMPLE_ADAPTER, SAMPLE_ADAPTER);
+        underTest.addAll(SAMPLE_ADAPTER, SAMPLE_ADAPTER);
         
         assertEquals(SAMPLE_ADAPTER, underTest.lookup(SAMPLE_ADAPTER, SAMPLE_ADAPTABLE).getType());
     }

Modified: sling/trunk/bundles/extensions/models/integration-tests/pom.xml
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/extensions/models/integration-tests/pom.xml?rev=1789734&r1=1789733&r2=1789734&view=diff
==============================================================================
--- sling/trunk/bundles/extensions/models/integration-tests/pom.xml (original)
+++ sling/trunk/bundles/extensions/models/integration-tests/pom.xml Fri Mar 31 19:14:48 2017
@@ -130,6 +130,10 @@
                         </Sling-Model-Classes>
                         <Sling-Test-Regexp>.*Test</Sling-Test-Regexp>
                         <Export-Package>org.apache.sling.models.it</Export-Package>
+                        <Import-Package>
+                            org.apache.commons.beanutils;resolution:=optional,
+                            *
+                        </Import-Package>
                     </instructions>
                 </configuration>
             </plugin>