From: Leif Åstrand Date: Tue, 1 Aug 2017 08:00:21 +0000 (+0300) Subject: Unify basic instance creation and related error reporting (#9704) X-Git-Tag: 8.1.1~10 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=921cd76504ed3045492b69285b8100fecb842abd;p=vaadin-framework.git Unify basic instance creation and related error reporting (#9704) --- diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java index ae467be123..43f893e33f 100644 --- a/server/src/main/java/com/vaadin/data/Binder.java +++ b/server/src/main/java/com/vaadin/data/Binder.java @@ -2261,12 +2261,10 @@ public class Binder implements Serializable { private HasValue makeFieldInstance( Class> fieldClass) { try { - return fieldClass.newInstance(); - } catch (InstantiationException | IllegalAccessException e) { - throw new IllegalStateException( - String.format("Couldn't create an '%s' type instance", - fieldClass.getName()), - e); + return ReflectTools.createInstance(fieldClass); + } catch (IllegalArgumentException e) { + // Rethrow as the exception type declared for bindInstanceFields + throw new IllegalStateException(e); } } diff --git a/server/src/main/java/com/vaadin/navigator/Navigator.java b/server/src/main/java/com/vaadin/navigator/Navigator.java index 23d6cc5f91..6b18cd4000 100644 --- a/server/src/main/java/com/vaadin/navigator/Navigator.java +++ b/server/src/main/java/com/vaadin/navigator/Navigator.java @@ -34,6 +34,7 @@ import com.vaadin.ui.ComponentContainer; import com.vaadin.ui.CssLayout; import com.vaadin.ui.SingleComponentContainer; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; /** * A navigator utility that allows switching of views in a part of an @@ -315,15 +316,7 @@ public class Navigator implements Serializable { @Override public View getView(String viewName) { if (this.viewName.equals(viewName)) { - try { - View view = viewClass.newInstance(); - return view; - } catch (InstantiationException | IllegalAccessException e) { - // TODO error handling - throw new RuntimeException(e); - } - // TODO error handling - + return ReflectTools.createInstance(viewClass); } return null; } @@ -1061,11 +1054,7 @@ public class Navigator implements Serializable { setErrorProvider(new ViewProvider() { @Override public View getView(String viewName) { - try { - return viewClass.newInstance(); - } catch (Exception e) { - throw new RuntimeException(e); - } + return ReflectTools.createInstance(viewClass); } @Override diff --git a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java index 3bce462522..fc05f67860 100644 --- a/server/src/main/java/com/vaadin/server/AbstractClientConnector.java +++ b/server/src/main/java/com/vaadin/server/AbstractClientConnector.java @@ -45,6 +45,7 @@ import com.vaadin.ui.Component.Event; import com.vaadin.ui.HasComponents; import com.vaadin.ui.LegacyComponent; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; import elemental.json.JsonObject; import elemental.json.JsonValue; @@ -295,7 +296,7 @@ public abstract class AbstractClientConnector */ protected SharedState createState() { try { - return getStateType().newInstance(); + return ReflectTools.createInstance(getStateType()); } catch (Exception e) { throw new RuntimeException("Error creating state of type " + getStateType().getName() + " for " + getClass().getName(), diff --git a/server/src/main/java/com/vaadin/server/BootstrapHandler.java b/server/src/main/java/com/vaadin/server/BootstrapHandler.java index 01fd2bf880..13a4852164 100644 --- a/server/src/main/java/com/vaadin/server/BootstrapHandler.java +++ b/server/src/main/java/com/vaadin/server/BootstrapHandler.java @@ -53,6 +53,7 @@ import com.vaadin.shared.communication.PushMode; import com.vaadin.ui.Dependency; import com.vaadin.ui.Dependency.Type; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; import elemental.json.Json; import elemental.json.JsonException; @@ -262,7 +263,7 @@ public abstract class BootstrapHandler extends SynchronizedRequestHandler { /** * The URI resolver used in the bootstrap process. - * + * * @since 8.1 */ protected static class BootstrapUriResolver extends VaadinUriResolver { @@ -541,7 +542,8 @@ public abstract class BootstrapHandler extends SynchronizedRequestHandler { Class viewportGeneratorClass = viewportGeneratorClassAnnotation .value(); try { - viewportContent = viewportGeneratorClass.newInstance() + viewportContent = ReflectTools + .createInstance(viewportGeneratorClass) .getViewport(context.getRequest()); } catch (Exception e) { throw new RuntimeException( diff --git a/server/src/main/java/com/vaadin/server/JsonCodec.java b/server/src/main/java/com/vaadin/server/JsonCodec.java index 9c105d505e..a7f94de468 100644 --- a/server/src/main/java/com/vaadin/server/JsonCodec.java +++ b/server/src/main/java/com/vaadin/server/JsonCodec.java @@ -46,6 +46,7 @@ import com.vaadin.shared.JsonConstants; import com.vaadin.shared.communication.UidlValue; import com.vaadin.ui.Component; import com.vaadin.ui.ConnectorTracker; +import com.vaadin.util.ReflectTools; import elemental.json.Json; import elemental.json.JsonArray; @@ -611,7 +612,7 @@ public class JsonCodec implements Serializable { Class targetClass = getClassForType(targetType); try { - Object decodedObject = targetClass.newInstance(); + Object decodedObject = ReflectTools.createInstance(targetClass); for (BeanProperty property : getProperties(targetClass)) { String fieldName = property.getName(); diff --git a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java index f3a56a2c53..4524442531 100644 --- a/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java +++ b/server/src/main/java/com/vaadin/server/LegacyCommunicationManager.java @@ -39,6 +39,7 @@ import com.vaadin.ui.ConnectorTracker; import com.vaadin.ui.HasComponents; import com.vaadin.ui.SelectiveRenderer; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; import elemental.json.JsonObject; import elemental.json.JsonValue; @@ -124,7 +125,7 @@ public class LegacyCommunicationManager implements Serializable { } try { - SharedState referenceState = stateType.newInstance(); + SharedState referenceState = ReflectTools.createInstance(stateType); EncodeResult encodeResult = JsonCodec.encode(referenceState, null, stateType, null); return encodeResult.getEncodedValue(); diff --git a/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java b/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java index 9f22905664..831239096a 100644 --- a/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java +++ b/server/src/main/java/com/vaadin/server/LegacyVaadinPortlet.java @@ -20,6 +20,8 @@ import javax.portlet.PortletConfig; import javax.portlet.PortletException; import javax.portlet.PortletRequest; +import com.vaadin.util.ReflectTools; + public class LegacyVaadinPortlet extends VaadinPortlet { private static final LegacyApplicationUIProvider provider = new LegacyApplicationUIProvider() { @@ -70,7 +72,7 @@ public class LegacyVaadinPortlet extends VaadinPortlet { throws PortletException { try { Class applicationClass = getApplicationClass(); - return applicationClass.newInstance(); + return ReflectTools.createInstance(applicationClass); } catch (Exception e) { throw new PortletException(e); } diff --git a/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java b/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java index 40f76ebd1a..70174fdc32 100644 --- a/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java +++ b/server/src/main/java/com/vaadin/server/LegacyVaadinServlet.java @@ -20,6 +20,8 @@ import javax.servlet.ServletConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; +import com.vaadin.util.ReflectTools; + public class LegacyVaadinServlet extends VaadinServlet { private static final UIProvider provider = new LegacyApplicationUIProvider() { @@ -69,7 +71,7 @@ public class LegacyVaadinServlet extends VaadinServlet { throws ServletException { try { Class applicationClass = getApplicationClass(); - return applicationClass.newInstance(); + return ReflectTools.createInstance(applicationClass); } catch (Exception e) { throw new ServletException(e); } diff --git a/server/src/main/java/com/vaadin/server/ServletPortletHelper.java b/server/src/main/java/com/vaadin/server/ServletPortletHelper.java index bf7b1c8452..a679e97d2e 100644 --- a/server/src/main/java/com/vaadin/server/ServletPortletHelper.java +++ b/server/src/main/java/com/vaadin/server/ServletPortletHelper.java @@ -22,6 +22,7 @@ import java.util.Properties; import com.vaadin.shared.ApplicationConstants; import com.vaadin.ui.Component; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; /** * Contains helper methods shared by {@link VaadinServlet} and @@ -176,17 +177,13 @@ public class ServletPortletHelper implements Serializable { Class providerClass = classLoader.loadClass(uiProviderProperty); Class subclass = providerClass .asSubclass(UIProvider.class); - return subclass.newInstance(); + return ReflectTools.createInstance(subclass); } catch (ClassNotFoundException e) { throw new ServiceException( "Could not load UIProvider class " + uiProviderProperty, e); } catch (ClassCastException e) { throw new ServiceException("UIProvider class " + uiProviderProperty + " does not extend UIProvider", e); - } catch (InstantiationException | IllegalAccessException e) { - throw new ServiceException( - "Could not instantiate UIProvider " + uiProviderProperty, - e); } } diff --git a/server/src/main/java/com/vaadin/server/UIProvider.java b/server/src/main/java/com/vaadin/server/UIProvider.java index a652b423ff..11d5d43109 100644 --- a/server/src/main/java/com/vaadin/server/UIProvider.java +++ b/server/src/main/java/com/vaadin/server/UIProvider.java @@ -31,6 +31,7 @@ import com.vaadin.annotations.Widgetset; import com.vaadin.shared.communication.PushMode; import com.vaadin.shared.ui.ui.Transport; import com.vaadin.ui.UI; +import com.vaadin.util.ReflectTools; public abstract class UIProvider implements Serializable { @@ -40,13 +41,7 @@ public abstract class UIProvider implements Serializable { public abstract Class getUIClass(UIClassSelectionEvent event); public UI createInstance(UICreateEvent event) { - try { - return event.getUIClass().newInstance(); - } catch (InstantiationException e) { - throw new RuntimeException("Could not instantiate UI class", e); - } catch (IllegalAccessException e) { - throw new RuntimeException("Could not access UI class", e); - } + return ReflectTools.createInstance(event.getUIClass()); } /** diff --git a/server/src/main/java/com/vaadin/ui/declarative/Design.java b/server/src/main/java/com/vaadin/ui/declarative/Design.java index b051c68a90..78288737a0 100644 --- a/server/src/main/java/com/vaadin/ui/declarative/Design.java +++ b/server/src/main/java/com/vaadin/ui/declarative/Design.java @@ -44,6 +44,7 @@ import com.vaadin.ui.Composite; import com.vaadin.ui.CustomComponent; import com.vaadin.ui.declarative.DesignContext.ComponentCreatedEvent; import com.vaadin.ui.declarative.DesignContext.ComponentCreationListener; +import com.vaadin.util.ReflectTools; /** * Design is used for reading a component hierarchy from an html string or input @@ -175,7 +176,7 @@ public class Design implements Serializable { + " which is not a Vaadin Component class"; try { - return componentClass.newInstance(); + return ReflectTools.createInstance(componentClass); } catch (Exception e) { throw new DesignException( "Could not create component " + fullyQualifiedClassName, diff --git a/server/src/main/java/com/vaadin/util/ReflectTools.java b/server/src/main/java/com/vaadin/util/ReflectTools.java index 8ff8ddcbb0..4728c76c47 100644 --- a/server/src/main/java/com/vaadin/util/ReflectTools.java +++ b/server/src/main/java/com/vaadin/util/ReflectTools.java @@ -20,6 +20,7 @@ import java.beans.PropertyDescriptor; import java.io.Serializable; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; /** * An util class with helpers for reflection operations. Used internally by @@ -29,6 +30,14 @@ import java.lang.reflect.Method; * @since 6.2 */ public class ReflectTools implements Serializable { + + static final String CREATE_INSTANCE_FAILED = "Unable to create an instance of {0}. Make sure it has a no-arg constructor"; + static final String CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS = "Unable to create an instance of {0}. Make sure the class is static if it is a nested class."; + static final String CREATE_INSTANCE_FAILED_ACCESS_EXCEPTION = "Unable to create an instance of {0}. Make sure the class is public and that is has a public no-arg constructor."; + static final String CREATE_INSTANCE_FAILED_NO_PUBLIC_NOARG_CONSTRUCTOR = "Unable to create an instance of {0}. Make sure the class has a public no-arg constructor."; + static final String CREATE_INSTANCE_FAILED_LOCAL_CLASS = "Cannot instantiate local class '%s'. Move class declaration outside the method."; + static final String CREATE_INSTANCE_FAILED_CONSTRUCTOR_THREW_EXCEPTION = "Unable to create an instance of {0}. The constructor threw an exception."; + /** * Locates the method in the given class. Returns null if the method is not * found. Throws an ExceptionInInitializerError if there is a problem @@ -249,4 +258,67 @@ public class ReflectTools implements Serializable { return currentClass; } + + /** + * Creates a instance of the given class with a no-arg constructor. + *

+ * Catches all exceptions which might occur and wraps them in a + * {@link IllegalArgumentException} with a descriptive error message hinting + * of what might be wrong with the class that could not be instantiated. + * + * @param cls + * the class to instantiate + * @return an instance of the class + * @since 8.1.1 + */ + public static T createInstance(Class cls) { + checkClassAccessibility(cls); + try { + return cls.getConstructor().newInstance(); + } catch (NoSuchMethodException e) { + throw new IllegalArgumentException(String.format( + CREATE_INSTANCE_FAILED_NO_PUBLIC_NOARG_CONSTRUCTOR, + cls.getName()), e); + } catch (InstantiationException e) { + if (cls.isMemberClass() && !Modifier.isStatic(cls.getModifiers())) { + throw new IllegalArgumentException(String.format( + CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS, + cls.getName()), e); + } else { + throw new IllegalArgumentException( + String.format(CREATE_INSTANCE_FAILED, cls.getName()), + e); + } + } catch (IllegalAccessException e) { + throw new IllegalArgumentException(String.format( + CREATE_INSTANCE_FAILED_ACCESS_EXCEPTION, cls.getName()), e); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + String.format(CREATE_INSTANCE_FAILED, cls.getName()), e); + } catch (InvocationTargetException e) { + throw new IllegalArgumentException(String.format( + CREATE_INSTANCE_FAILED_CONSTRUCTOR_THREW_EXCEPTION, + cls.getName()), e); + } + } + + /** + * Makes a check whether the provided class is externally accessible for + * instantiation (e.g. it's not inner class (nested and not static) and is + * not a local class). + * + * @param cls + * type to check + */ + private static void checkClassAccessibility(Class cls) { + if (cls.isMemberClass() && !Modifier.isStatic(cls.getModifiers())) { + throw new IllegalArgumentException(String.format( + CREATE_INSTANCE_FAILED_FOR_NON_STATIC_MEMBER_CLASS, + cls.getName())); + } else if (cls.isLocalClass()) { + throw new IllegalArgumentException(String + .format(CREATE_INSTANCE_FAILED_LOCAL_CLASS, cls.getName())); + } + } + }