From ccecdf216fd1f113fbd76734eed8a452cfde5dff Mon Sep 17 00:00:00 2001 From: glen3b Date: Wed, 14 May 2014 16:06:26 -0700 Subject: [PATCH 1/6] Throw ClassNotFoundException when appropriate Currently, the ClassSource returned by ClassSource.fromMap will return null if the Class cannot be found (as that is the behavior of maps). However, other ClassSources throw a ClassNotFoundException if the class cannot be loaded. This commit changes the behavior of ClassSource.fromMap to throw a ClassNotFoundException if the class was not found in the map (or was mapped to null). This commit also changes the method to interpret a null map as an empty map. --- .../java/com/comphenix/protocol/utility/ClassSource.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index f2277c77..5832fa5e 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -47,7 +47,12 @@ public abstract class ClassSource { return new ClassSource() { @Override public Class loadClass(String canonicalName) throws ClassNotFoundException { - return map.get(canonicalName); + Class loaded = map == null ? null : map.get(canonicalName); + if(loaded == null){ + // Throw the appropriate exception if we can't load the class + throw new ClassNotFoundException("The specified class could not be found by this ClassLoader."); + } + return loaded; } }; } From 324e6de28483dc9f9298f4e77697562dfb39745c Mon Sep 17 00:00:00 2001 From: Glen Husman Date: Wed, 14 May 2014 16:26:51 -0700 Subject: [PATCH 2/6] Document new fromMap behavior --- .../main/java/com/comphenix/protocol/utility/ClassSource.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index 5832fa5e..8b76b5af 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -40,6 +40,7 @@ public abstract class ClassSource { /** * Construct a class source from a mapping of canonical names and the corresponding classes. + * If the map is null, it will be interpreted as an empty map. If the map does not contain a Class with the specified name, or that string maps to NULL explicitly, a {@link ClassNotFoundException} will be thrown. * @param map - map of class names and classes. * @return The class source. */ From e0449b2db6c910ffcc933fb5754db01280480a9a Mon Sep 17 00:00:00 2001 From: Glen Husman Date: Wed, 14 May 2014 16:40:22 -0700 Subject: [PATCH 3/6] Add ClassSource.attemptLoadFrom method This method is an alternative to chaining retry calls --- .../protocol/utility/ClassSource.java | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index 8b76b5af..25b7d95c 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -14,7 +14,7 @@ public abstract class ClassSource { public static ClassSource fromClassLoader() { return fromClassLoader(ClassSource.class.getClassLoader()); } - + /** * Construct a class source from the default class loader and package. * @param packageName - the package that is prepended to every lookup. @@ -23,7 +23,7 @@ public abstract class ClassSource { public static ClassSource fromPackage(String packageName) { return fromClassLoader().usingPackage(packageName); } - + /** * Construct a class source from the given class loader. * @param loader - the class loader. @@ -57,7 +57,20 @@ public abstract class ClassSource { } }; } - + + /** + * Retrieve a class source that will attempt lookups in each of the given sources in the order they are in the array, and return the first value that is found. + * @param sources - the class sources. + * @return A new class source. + */ + public static ClassSource attemptLoadFrom(final ClassSource... sources) { + ClassSource source = sources[0]; + for(int i = 1; i < sources.length; i++){ + source = source.retry(sources[i]); + } + return source; + } + /** * Retrieve a class source that will retry failed lookups in the given source. * @param other - the other class source. @@ -75,7 +88,7 @@ public abstract class ClassSource { } }; } - + /** * Retrieve a class source that prepends a specific package name to every lookup. * @param packageName - the package name to prepend. @@ -99,7 +112,7 @@ public abstract class ClassSource { protected static String append(String a, String b) { boolean left = a.endsWith("."); boolean right = b.endsWith("."); - + // Only add a dot if necessary if (left && right) return a.substring(0, a.length() - 1) + b; @@ -108,7 +121,7 @@ public abstract class ClassSource { else return a + "." + b; } - + /** * Retrieve a class by name. * @param canonicalName - the full canonical name of the class. From 95087a5b9f0a1c88f78c7ce8fe8cc622566f13c7 Mon Sep 17 00:00:00 2001 From: Glen Husman Date: Wed, 14 May 2014 16:51:56 -0700 Subject: [PATCH 4/6] attemptLoadFrom now ignores null values properly --- .../java/com/comphenix/protocol/utility/ClassSource.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index 25b7d95c..fd28262a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -60,13 +60,15 @@ public abstract class ClassSource { /** * Retrieve a class source that will attempt lookups in each of the given sources in the order they are in the array, and return the first value that is found. + * If the sources array is empty, null, or composed of all null elements, the returned ClassSource will be null. Null elements in the array are ignored. * @param sources - the class sources. * @return A new class source. */ public static ClassSource attemptLoadFrom(final ClassSource... sources) { - ClassSource source = sources[0]; + ClassSource source = sources != null && sources.length >= 1 ? sources[0] : null; for(int i = 1; i < sources.length; i++){ - source = source.retry(sources[i]); + source = sources[i] == null ? source : + source == null ? sources[i] : source.retry(sources[i]); } return source; } @@ -125,7 +127,7 @@ public abstract class ClassSource { /** * Retrieve a class by name. * @param canonicalName - the full canonical name of the class. - * @return The corresponding class + * @return The corresponding class. If the class is not found, NULL should not be returned, instead a {@code ClassNotFoundException} exception should be thrown. * @throws ClassNotFoundException If the class could not be found. */ public abstract Class loadClass(String canonicalName) throws ClassNotFoundException; From 0f5be7f1f1fc097a535744f946068f0a26816d99 Mon Sep 17 00:00:00 2001 From: Glen Husman Date: Sat, 17 May 2014 12:23:59 -0700 Subject: [PATCH 5/6] Add ClassSource.empty, ClassSource.attemptLoadFrom will also not tolerate nulls --- .../protocol/utility/ClassSource.java | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index fd28262a..a801a269 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -1,5 +1,6 @@ package com.comphenix.protocol.utility; +import java.util.Collections; import java.util.Map; /** @@ -57,18 +58,32 @@ public abstract class ClassSource { } }; } + + /** + * @return A ClassLoader which will never successfully load a class. + */ + public static ClassSource empty(){ + return fromMap(Collections.>emptyMap()); + } /** * Retrieve a class source that will attempt lookups in each of the given sources in the order they are in the array, and return the first value that is found. - * If the sources array is empty, null, or composed of all null elements, the returned ClassSource will be null. Null elements in the array are ignored. + * If the sources array is null or composed of any null elements, an exception will be thrown. * @param sources - the class sources. * @return A new class source. */ public static ClassSource attemptLoadFrom(final ClassSource... sources) { - ClassSource source = sources != null && sources.length >= 1 ? sources[0] : null; + if(sources.length == 0){ // Throws NPE if sources is null, which is what we want + return ClassSource.empty(); + } + + ClassSource source = sources[0]; for(int i = 1; i < sources.length; i++){ - source = sources[i] == null ? source : - source == null ? sources[i] : source.retry(sources[i]); + if(source == null || sources[i] == null){ + throw new IllegalArgumentException("Null values are not permitted as ClassSources."); + } + + source = source.retry(sources[i]); } return source; } From 1ee68d4e091f6989f0f123310e7b2a789b9aa76c Mon Sep 17 00:00:00 2001 From: Glen Husman Date: Sat, 17 May 2014 20:00:40 -0700 Subject: [PATCH 6/6] attemptLoadFrom will throw IllegalArgumentException for arrays of length 1 with a null element --- .../java/com/comphenix/protocol/utility/ClassSource.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java index a801a269..293e0e2a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -77,13 +77,13 @@ public abstract class ClassSource { return ClassSource.empty(); } - ClassSource source = sources[0]; - for(int i = 1; i < sources.length; i++){ - if(source == null || sources[i] == null){ + ClassSource source = null; + for(int i = 0; i < sources.length; i++){ + if(sources[i] == null){ throw new IllegalArgumentException("Null values are not permitted as ClassSources."); } - source = source.retry(sources[i]); + source = source == null ? sources[i] : source.retry(sources[i]); } return source; }