Author: timothyjward
Date: Wed Feb 20 09:16:04 2019
New Revision: 1853936

URL: http://svn.apache.org/viewvc?rev=1853936&view=rev
Log:
FELIX-6063 Avoid using getDeclaredXXX() in the converter

Modified:
    
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/ConvertingImpl.java
    
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/DTOUtil.java
    
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/Util.java

Modified: 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/ConvertingImpl.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/ConvertingImpl.java?rev=1853936&r1=1853935&r2=1853936&view=diff
==============================================================================
--- 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/ConvertingImpl.java
 (original)
+++ 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/ConvertingImpl.java
 Wed Feb 20 09:16:04 2019
@@ -385,28 +385,24 @@ class ConvertingImpl extends AbstractSpe
 
                                Field f = null;
                                try {
-                                       f = 
targetAsCls.getDeclaredField(fieldName);
+                                       f = targetAsCls.getField(fieldName);
                                } catch (NoSuchFieldException e) {
-                                       try {
-                                               f = 
targetAsCls.getField(fieldName);
-                                       } catch (NoSuchFieldException | 
NullPointerException e1) {
-                                               // There is no field with this 
name
-                                               if (keysIgnoreCase) {
-                                                       // If enabled, try 
again but now ignore case
-                                                       for (Field fs : 
targetAsCls.getDeclaredFields()) {
-                                                               if 
(fs.getName().equalsIgnoreCase(fieldName)) {
-                                                                       f = fs;
-                                                                       break;
-                                                               }
+                                       // There is no field with this name
+                                       if (keysIgnoreCase) {
+                                               // If enabled, try again but 
now ignore case
+                                               for (Field fs : 
targetAsCls.getFields()) {
+                                                       if 
(fs.getName().equalsIgnoreCase(fieldName)) {
+                                                               f = fs;
+                                                               break;
                                                        }
+                                               }
 
-                                                       if (f == null) {
-                                                               for (Field fs : 
targetAsCls.getFields()) {
-                                                                       if 
(fs.getName()
-                                                                               
        .equalsIgnoreCase(fieldName)) {
-                                                                               
f = fs;
-                                                                               
break;
-                                                                       }
+                                               if (f == null) {
+                                                       for (Field fs : 
targetAsCls.getFields()) {
+                                                               if (fs.getName()
+                                                                               
.equalsIgnoreCase(fieldName)) {
+                                                                       f = fs;
+                                                                       break;
                                                                }
                                                        }
                                                }
@@ -523,12 +519,10 @@ class ConvertingImpl extends AbstractSpe
 
        private List<String> getNames(Class< ? > cls) {
                List<String> names = new ArrayList<>();
-               for (Field field : cls.getDeclaredFields()) {
+               for (Field field : cls.getFields()) {
                        int modifiers = field.getModifiers();
                        if (Modifier.isStatic(modifiers))
                                continue;
-                       if (!Modifier.isPublic(modifiers))
-                               continue;
 
                        String name = field.getName();
                        if (!names.contains(name))
@@ -930,17 +924,11 @@ class ConvertingImpl extends AbstractSpe
                return null;
        }
 
-       private boolean isMarkerAnnotation(Class< ? > annClass) {
-               for (Method m : annClass.getDeclaredMethods()) {
-                       try {
-                               if (Annotation.class
-                                               .getMethod(m.getName(), 
m.getParameterTypes())
-                                               .getReturnType()
-                                               .equals(m.getReturnType()))
-                                       // this is a base annotation method
-                                       continue;
-                       } catch (Exception ex) {
-                               // Method not found, not a marker annotation
+       private static boolean isMarkerAnnotation(Class< ? > annClass) {
+               for (Method m : annClass.getMethods()) {
+                       if (m.getDeclaringClass() != annClass) {
+                               // this is a base annotation or object method
+                               continue;
                        }
                        return false;
                }
@@ -950,8 +938,9 @@ class ConvertingImpl extends AbstractSpe
        @SuppressWarnings("unchecked")
        private <T> T tryStandardMethods() {
                try {
-                       Method m = targetAsClass.getDeclaredMethod("valueOf", 
String.class);
-                       if (m != null) {
+                       // Section 707.4.2.3 and 707.4.2.5 require valueOf to 
be public and static
+                       Method m = targetAsClass.getMethod("valueOf", 
String.class);
+                       if (m != null && Modifier.isStatic(m.getModifiers())) {
                                return (T) m.invoke(null, object.toString());
                        }
                } catch (Exception e) {
@@ -1009,7 +998,8 @@ class ConvertingImpl extends AbstractSpe
                Set<String> invokedMethods = new HashSet<>();
 
                Map result = new HashMap();
-               for (Method md : sourceCls.getDeclaredMethods()) {
+               // Bean accessors must be public
+               for (Method md : sourceCls.getMethods()) {
                        handleBeanMethod(obj, md, invokedMethods, result);
                }
 
@@ -1021,28 +1011,31 @@ class ConvertingImpl extends AbstractSpe
                Set<String> handledFields = new HashSet<>();
 
                Map result = new HashMap();
-               // Do we need 'declaredfields'? We only need to look at the 
public
-               // ones...
-               for (Field f : obj.getClass().getDeclaredFields()) {
-                       handleDTOField(obj, f, handledFields, result, ic);
-               }
+               // We only use public fields for mapping a DTO
                for (Field f : obj.getClass().getFields()) {
                        handleDTOField(obj, f, handledFields, result, ic);
                }
                return result;
        }
 
-       @SuppressWarnings("rawtypes")
+       @SuppressWarnings({"unchecked","rawtypes"})
        private static Map createMapFromInterface(Object obj, Class< ? > 
srcCls) {
                Map result = new HashMap();
 
-               for (Class i : getInterfaces(srcCls)) {
-                       for (Method md : i.getMethods()) {
-                               handleInterfaceMethod(obj, i, md, new 
HashSet<String>(),
-                                               result);
+               if(Annotation.class.isAssignableFrom(srcCls) && 
isMarkerAnnotation(((Annotation)obj).annotationType())) {
+                       // We special case this if the source is a marker 
annotation because we will end up with no
+                       // interface methods otherwise
+                       
result.put(Util.getMarkerAnnotationKey(((Annotation)obj).annotationType(), 
obj), Boolean.TRUE);
+                       return result;
+               } else {
+                       for (Class i : getInterfaces(srcCls)) {
+                               for (Method md : i.getMethods()) {
+                                       handleInterfaceMethod(obj, i, md, new 
HashSet<String>(),
+                                                       result);
+                               }
+                               if (result.size() > 0)
+                                       return result;
                        }
-                       if (result.size() > 0)
-                               return result;
                }
                throw new ConversionException("Cannot be converted to map: " + 
obj);
        }
@@ -1099,10 +1092,14 @@ class ConvertingImpl extends AbstractSpe
                        return Collections.emptySet();
 
                Set<Class< ? >> interfaces = getInterfaces0(cls);
-               for (Iterator<Class< ? >> it = interfaces.iterator(); 
it.hasNext();) {
+               outer: for (Iterator<Class< ? >> it = interfaces.iterator(); 
it.hasNext();) {
                        Class< ? > intf = it.next();
-                       if (intf.getDeclaredMethods().length == 0)
-                               it.remove();
+                       for (Method method : intf.getMethods()) {
+                               if(method.getDeclaringClass() == intf) {
+                                       continue outer;
+                               }
+                       }
+                       it.remove();
                }
 
                interfaces.removeAll(NO_MAP_VIEW_TYPES);
@@ -1217,9 +1214,8 @@ class ConvertingImpl extends AbstractSpe
 
        private boolean hasGetProperties(Class< ? > cls) {
                try {
-                       Method m = cls.getDeclaredMethod("getProperties");
-                       if (m == null)
-                               m = cls.getMethod("getProperties");
+                       // Section 707.4.4.4.8 says getProperties must be public
+                       Method m = cls.getMethod("getProperties");
                        return m != null;
                } catch (Exception e) {
                        return false;
@@ -1228,9 +1224,8 @@ class ConvertingImpl extends AbstractSpe
 
        private Map< ? , ? > getPropertiesDelegate(Object obj, Class< ? > cls) {
                try {
-                       Method m = cls.getDeclaredMethod("getProperties");
-                       if (m == null)
-                               m = cls.getMethod("getProperties");
+                       // Section 707.4.4.4.8 says getProperties must be public
+                       Method m = cls.getMethod("getProperties");
 
                        return converter.convert(m.invoke(obj)).to(Map.class);
                } catch (Exception e) {
@@ -1262,7 +1257,7 @@ class ConvertingImpl extends AbstractSpe
                Set<Method> setters = new HashSet<>();
                while (!Object.class.equals(cls)) {
                        Set<Method> methods = new HashSet<>();
-                       methods.addAll(Arrays.asList(cls.getDeclaredMethods()));
+                       // Only public methods can be Java Bean setters
                        methods.addAll(Arrays.asList(cls.getMethods()));
                        for (Method md : methods) {
                                if (md.getParameterTypes().length != 1)

Modified: 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/DTOUtil.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/DTOUtil.java?rev=1853936&r1=1853935&r2=1853936&view=diff
==============================================================================
--- 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/DTOUtil.java
 (original)
+++ 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/DTOUtil.java
 Wed Feb 20 09:16:04 2019
@@ -30,14 +30,9 @@ class DTOUtil {
 
        static boolean isDTOType(Class< ? > cls) {
                try {
-                       cls.getDeclaredConstructor();
+                       cls.getConstructor();
                } catch (NoSuchMethodException | SecurityException e) {
-                       // No zero-arg constructor, not a DTO
-                       return false;
-               }
-
-               if (cls.getDeclaredMethods().length > 0) {
-                       // should not have any methods
+                       // No public zero-arg constructor, not a DTO
                        return false;
                }
 

Modified: 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/Util.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/Util.java?rev=1853936&r1=1853935&r2=1853936&view=diff
==============================================================================
--- 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/Util.java 
(original)
+++ 
felix/trunk/converter/converter/src/main/java/org/osgi/util/converter/Util.java 
Wed Feb 20 09:16:04 2019
@@ -81,7 +81,8 @@ class Util {
 
        static Map<String,Method> getBeanKeys(Class< ? > beanClass) {
                Map<String,Method> keys = new LinkedHashMap<>();
-               for (Method md : beanClass.getDeclaredMethods()) {
+               // Bean methods must be public and can be on parent classes
+               for (Method md : beanClass.getMethods()) {
                        String key = getBeanKey(md);
                        if (key != null && !keys.containsKey(key)) {
                                keys.put(key, md);
@@ -202,7 +203,13 @@ class Util {
                        return null;
 
                boolean valueFound = false;
-               for (Method md : ann.getDeclaredMethods()) {
+               // All annotation methods must be public
+               for (Method md : ann.getMethods()) {
+                       if(md.getDeclaringClass() != ann) {
+                               // Ignore Object methods and Annotation methods
+                               continue;
+                       }
+                       
                        if ("value".equals(md.getName())) {
                                valueFound = true;
                                continue;
@@ -317,10 +324,20 @@ class Util {
 
        static String getPrefix(Class< ? > cls) {
                try {
-                       Field prefixField = cls.getDeclaredField("PREFIX_");
-                       if (prefixField.getType().equals(String.class)) {
-                               if ((prefixField.getModifiers() & 
(Modifier.PUBLIC
-                                               | Modifier.FINAL | 
Modifier.STATIC)) > 0) {
+                       // We can use getField as the PREFIX must be public 
(see spec erratum)
+                       Field prefixField = cls.getField("PREFIX_");
+                       if (prefixField.getDeclaringClass() == cls &&
+                                       
prefixField.getType().equals(String.class)) {
+                               int modifiers = prefixField.getModifiers();
+                               // We need to be final *and* static
+                               if (Modifier.isFinal(modifiers) &&
+                                       Modifier.isStatic(modifiers)) {
+                                       
+                                       if(!prefixField.isAccessible()) {
+                                               // Should we log that we have 
to do this?
+                                               prefixField.setAccessible(true);
+                                       }
+                                       
                                        return (String) prefixField.get(null);
                                }
                        }


Reply via email to