From 47dd63e6b2aea24548281cd364048b618701b61f Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 26 Sep 2014 10:42:38 +0200 Subject: [PATCH] Don't use instanceof in a parse(IQ|PacketExtension) Instead of using instanceof *every time* we parse an IQ or PacketExtension, we now check the type of the Provider only once when adding it. --- .../smack/provider/ProviderManager.java | 78 +++++++++++++------ .../smack/util/PacketParserUtils.java | 42 +++++----- 2 files changed, 76 insertions(+), 44 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java b/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java index 5ecd76b39..9d3437f08 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/provider/ProviderManager.java @@ -17,13 +17,15 @@ package org.jivesoftware.smack.provider; -import java.util.Collection; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.jivesoftware.smack.SmackConfiguration; import org.jivesoftware.smack.packet.IQ; +import org.jivesoftware.smack.packet.PacketExtension; import org.jxmpp.util.XmppStringUtils; /** @@ -107,8 +109,10 @@ import org.jxmpp.util.XmppStringUtils; */ public final class ProviderManager { - private static final Map extensionProviders = new ConcurrentHashMap(); - private static final Map iqProviders = new ConcurrentHashMap(); + private static final Map extensionProviders = new ConcurrentHashMap(); + private static final Map iqProviders = new ConcurrentHashMap(); + private static final Map> extensionIntrospectionProviders = new ConcurrentHashMap>(); + private static final Map> iqIntrospectionProviders = new ConcurrentHashMap>(); private static final Map streamFeatureProviders = new ConcurrentHashMap(); static { @@ -122,13 +126,13 @@ public final class ProviderManager { public static void addLoader(ProviderLoader loader) { if (loader.getIQProviderInfo() != null) { for (IQProviderInfo info : loader.getIQProviderInfo()) { - iqProviders.put(getKey(info.getElementName(), info.getNamespace()), info.getProvider()); + addIQProvider(info.getElementName(), info.getNamespace(), info.getProvider()); } } if (loader.getExtensionProviderInfo() != null) { for (ExtensionProviderInfo info : loader.getExtensionProviderInfo()) { - extensionProviders.put(getKey(info.getElementName(), info.getNamespace()), info.getProvider()); + addExtensionProvider(info.getElementName(), info.getNamespace(), info.getProvider()); } } } @@ -153,11 +157,16 @@ public final class ProviderManager { * @param namespace the XML namespace. * @return the IQ provider. */ - public static Object getIQProvider(String elementName, String namespace) { + public static IQProvider getIQProvider(String elementName, String namespace) { String key = getKey(elementName, namespace); return iqProviders.get(key); } + public static Class getIQIntrospectionProvider(String elementName, String namespace) { + String key = getKey(elementName, namespace); + return iqIntrospectionProviders.get(key); + } + /** * Returns an unmodifiable collection of all IQProvider instances. Each object * in the collection will either be an IQProvider instance, or a Class object @@ -165,8 +174,11 @@ public final class ProviderManager { * * @return all IQProvider instances. */ - public static Collection getIQProviders() { - return Collections.unmodifiableCollection(iqProviders.values()); + public static List getIQProviders() { + List providers = new ArrayList(iqProviders.size() + iqIntrospectionProviders.size()); + providers.addAll(iqProviders.values()); + providers.addAll(iqIntrospectionProviders.values()); + return Collections.unmodifiableList(providers); } /** @@ -181,14 +193,16 @@ public final class ProviderManager { public static void addIQProvider(String elementName, String namespace, Object provider) { - if (!(provider instanceof IQProvider || (provider instanceof Class && - IQ.class.isAssignableFrom((Class)provider)))) - { + // First remove existing providers + String key = removeIQProvider(elementName, namespace); + if (provider instanceof IQProvider) { + iqProviders.put(key, (IQProvider) provider); + } else if (provider instanceof Class && IQ.class.isAssignableFrom((Class) provider)) { + iqIntrospectionProviders.put(key, (Class) provider); + } else { throw new IllegalArgumentException("Provider must be an IQProvider " + - "or a Class instance sublcassing IQ."); + "or a Class instance sublcassing IQ."); } - String key = getKey(elementName, namespace); - iqProviders.put(key, provider); } /** @@ -198,10 +212,13 @@ public final class ProviderManager { * * @param elementName the XML element name. * @param namespace the XML namespace. + * @return the key of the removed IQ Provider */ - public static void removeIQProvider(String elementName, String namespace) { + public static String removeIQProvider(String elementName, String namespace) { String key = getKey(elementName, namespace); iqProviders.remove(key); + iqIntrospectionProviders.remove(key); + return key; } /** @@ -223,11 +240,16 @@ public final class ProviderManager { * @param namespace namespace associated with extension provider. * @return the extenion provider. */ - public static Object getExtensionProvider(String elementName, String namespace) { + public static PacketExtensionProvider getExtensionProvider(String elementName, String namespace) { String key = getKey(elementName, namespace); return extensionProviders.get(key); } + public static Class getExtensionIntrospectionProvider(String elementName, String namespace) { + String key = getKey(elementName, namespace); + return extensionIntrospectionProviders.get(key); + } + /** * Adds an extension provider with the specified element name and name space. The provider * will override any providers loaded through the classpath. The provider must be either @@ -240,12 +262,16 @@ public final class ProviderManager { public static void addExtensionProvider(String elementName, String namespace, Object provider) { - if (!(provider instanceof PacketExtensionProvider || provider instanceof Class)) { + // First remove existing providers + String key = removeExtensionProvider(elementName, namespace); + if (provider instanceof PacketExtensionProvider) { + extensionProviders.put(key, (PacketExtensionProvider) provider); + } else if (provider instanceof Class && PacketExtension.class.isAssignableFrom((Class) provider)) { + extensionIntrospectionProviders.put(key, (Class) provider); + } else { throw new IllegalArgumentException("Provider must be a PacketExtensionProvider " + - "or a Class instance."); + "or a Class instance."); } - String key = getKey(elementName, namespace); - extensionProviders.put(key, provider); } /** @@ -255,10 +281,13 @@ public final class ProviderManager { * * @param elementName the XML element name. * @param namespace the XML namespace. + * @return the key of the removed packet extension provider */ - public static void removeExtensionProvider(String elementName, String namespace) { + public static String removeExtensionProvider(String elementName, String namespace) { String key = getKey(elementName, namespace); extensionProviders.remove(key); + extensionIntrospectionProviders.remove(key); + return key; } /** @@ -268,8 +297,11 @@ public final class ProviderManager { * * @return all PacketExtensionProvider instances. */ - public static Collection getExtensionProviders() { - return Collections.unmodifiableCollection(extensionProviders.values()); + public static List getExtensionProviders() { + List providers = new ArrayList(extensionProviders.size() + extensionIntrospectionProviders.size()); + providers.addAll(extensionProviders.values()); + providers.addAll(extensionIntrospectionProviders.values()); + return Collections.unmodifiableList(providers); } public static StreamFeatureProvider getStreamFeatureProvider(String elementName, String namespace) { diff --git a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java index be4cf3c05..bb1d81752 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/util/PacketParserUtils.java @@ -543,23 +543,25 @@ public class PacketParserUtils { // Otherwise, see if there is a registered provider for // this element name and namespace. else { - Object provider = ProviderManager.getIQProvider(elementName, namespace); + IQProvider provider = ProviderManager.getIQProvider(elementName, namespace); if (provider != null) { - if (provider instanceof IQProvider) { - iqPacket = ((IQProvider)provider).parseIQ(parser); + iqPacket =provider.parseIQ(parser); + } else { + Class introspectionProvider = ProviderManager.getIQIntrospectionProvider( + elementName, namespace); + if (introspectionProvider != null) { + iqPacket = (IQ) PacketParserUtils.parseWithIntrospection( + elementName, introspectionProvider, + parser); } - else if (provider instanceof Class) { - iqPacket = (IQ)PacketParserUtils.parseWithIntrospection(elementName, - (Class)provider, parser); + // Only handle unknown IQs of type result. Types of 'get' and 'set' which are not understood + // have to be answered with an IQ error response. See the code a few lines below + else if (IQ.Type.result == type){ + // No Provider found for the IQ stanza, parse it to an UnparsedIQ instance + // so that the content of the IQ can be examined later on + iqPacket = new UnparsedResultIQ(parseContent(parser)); } } - // Only handle unknown IQs of type result. Types of 'get' and 'set' which are not understood - // have to be answered with an IQ error response. See the code a few lines below - else if (IQ.Type.result == type){ - // No Provider found for the IQ stanza, parse it to an UnparsedIQ instance - // so that the content of the IQ can be examined later on - iqPacket = new UnparsedResultIQ(parseContent(parser)); - } } } else if (eventType == XmlPullParser.END_TAG) { @@ -885,15 +887,13 @@ public class PacketParserUtils { public static PacketExtension parsePacketExtension(String elementName, String namespace, XmlPullParser parser) throws Exception { // See if a provider is registered to handle the extension. - Object provider = ProviderManager.getExtensionProvider(elementName, namespace); + PacketExtensionProvider provider = ProviderManager.getExtensionProvider(elementName, namespace); if (provider != null) { - if (provider instanceof PacketExtensionProvider) { - return ((PacketExtensionProvider)provider).parseExtension(parser); - } - else if (provider instanceof Class) { - return (PacketExtension)parseWithIntrospection( - elementName, (Class)provider, parser); - } + return provider.parseExtension(parser); + } + Class introspectionProvider = ProviderManager.getExtensionIntrospectionProvider(elementName, namespace); + if (introspectionProvider != null) { + return (PacketExtension)parseWithIntrospection(elementName, introspectionProvider, parser); } // No providers registered, so use a default extension. DefaultPacketExtension extension = new DefaultPacketExtension(elementName, namespace);