WICKET-5215 Better exception message when Page instantiation fails in DefaultPageFactory
authorMartin Tzvetanov Grigorov <mgrigorov@apache.org>
Mon, 10 Jun 2013 11:36:58 +0000 (14:36 +0300)
committerMartin Tzvetanov Grigorov <mgrigorov@apache.org>
Mon, 10 Jun 2013 11:36:58 +0000 (14:36 +0300)
wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java
wicket-core/src/test/java/org/apache/wicket/request/resource/UrlResourceReferenceTest.java
wicket-core/src/test/java/org/apache/wicket/session/DefaultPageFactoryTest.java

index 5d8d29f..6e713e5 100644 (file)
@@ -18,6 +18,7 @@ package org.apache.wicket.session;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Modifier;
 import java.util.concurrent.ConcurrentMap;
 
 import org.apache.wicket.IPageFactory;
@@ -61,7 +62,7 @@ public final class DefaultPageFactory implements IPageFactory
                {
                        // throw an exception in case default constructor is missing
                        // => improved error message
-                       Constructor<C> constructor = pageClass.getConstructor((Class<?>[])null);
+                       Constructor<C> constructor = pageClass.getDeclaredConstructor((Class<?>[]) null);
 
                        return processPage(newPage(constructor, null), null);
                }
@@ -124,7 +125,7 @@ public final class DefaultPageFactory implements IPageFactory
                        try
                        {
                                // Try to find the constructor
-                               constructor = pageClass.getConstructor(new Class[] { argumentType });
+                               constructor = pageClass.getDeclaredConstructor(new Class[] { argumentType });
 
                                // Store it in the cache
                                Constructor<C> tmpConstructor = (Constructor<C>) constructorForClass.putIfAbsent(pageClass, constructor);
@@ -210,18 +211,22 @@ public final class DefaultPageFactory implements IPageFactory
 
        private String createDescription(final Constructor<?> constructor, final Object argument)
        {
-               String msg;
+               String msg = "Can't instantiate page using constructor '" + constructor + "'";
                if (argument != null)
                {
-                       msg = "Can't instantiate page using constructor '" + constructor + "' and argument '" +
-                               argument;
+                       msg += " and argument '" + argument + "'";
+               }
+
+               if (constructor != null && Modifier.isPrivate(constructor.getModifiers()))
+               {
+                       msg += ". This constructor is private!";
                }
                else
                {
-                       msg = "Can't instantiate page using constructor '" + constructor;
+                       msg += ". There is no such constructor!";
                }
 
-               return msg + "'. Might be it doesn't exist, may be it is not visible (public).";
+               return msg;
        }
 
        @Override
@@ -232,7 +237,7 @@ public final class DefaultPageFactory implements IPageFactory
                {
                        try
                        {
-                               if (pageClass.getConstructor(new Class[] { }) != null)
+                               if (pageClass.getDeclaredConstructor(new Class[] { }) != null)
                                {
                                        bookmarkable = Boolean.TRUE;
                                }
@@ -241,7 +246,7 @@ public final class DefaultPageFactory implements IPageFactory
                        {
                                try
                                {
-                                       if (pageClass.getConstructor(new Class[] { PageParameters.class }) != null)
+                                       if (pageClass.getDeclaredConstructor(new Class[] { PageParameters.class }) != null)
                                        {
                                                bookmarkable = Boolean.TRUE;
                                        }
index 3405b22..a231e20 100644 (file)
@@ -90,7 +90,7 @@ public class UrlResourceReferenceTest extends WicketTestCase
        /**
         * A test page for #contextRelativeUrl()
         */
-       private static class TestPage extends WebPage implements IMarkupResourceStreamProvider
+       public static class TestPage extends WebPage implements IMarkupResourceStreamProvider
        {
                @Override
                public void renderHead(IHeaderResponse response)
index ae8e52a..78b2751 100644 (file)
@@ -23,11 +23,13 @@ import org.apache.wicket.markup.html.WebPage;
 import org.apache.wicket.request.flow.ResetResponseException;
 import org.apache.wicket.request.handler.EmptyRequestHandler;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 
 /**
- * Default page facotry tests
+ * Default page factory tests
  * 
  * @author ivaynberg
  */
@@ -126,11 +128,60 @@ public class DefaultPageFactoryTest extends WicketTestCase
                }
        }
 
+       public static class PrivateDefaultConstructorPage extends WebPage
+       {
+               private PrivateDefaultConstructorPage()
+               {}
+       }
+
+       public static class PrivateConstructorWithParametersPage extends WebPage
+       {
+               private PrivateConstructorWithParametersPage(PageParameters parameters)
+               {
+                       super(parameters);
+               }
+       }
+
+       public static class NonDefaultConstructorPage extends WebPage
+       {
+               public NonDefaultConstructorPage(String aa) {
+                       super();
+               }
+       }
 
        final private IPageFactory pageFactory = new DefaultPageFactory();
 
+       @Rule
+       public ExpectedException expectedException = ExpectedException.none();
+
+       @Test
+       public void privateConstructor() {
+               expectedException.expect(WicketRuntimeException.class);
+               expectedException.expectMessage("Can't instantiate page using constructor 'private org.apache.wicket.session.DefaultPageFactoryTest$PrivateDefaultConstructorPage()'. This constructor is private!");
+
+               pageFactory.newPage(PrivateDefaultConstructorPage.class);
+       }
+
+       @Test
+       public void privateConstructorWithParameters() {
+               expectedException.expect(WicketRuntimeException.class);
+               expectedException.expectMessage("Can't instantiate page using constructor 'private org.apache.wicket.session.DefaultPageFactoryTest$PrivateConstructorWithParametersPage(org.apache.wicket.request.mapper.parameter.PageParameters)' and argument 'key=[value]'. This constructor is private!");
+
+               PageParameters parameters = new PageParameters();
+               parameters.add("key", "value");
+               pageFactory.newPage(PrivateConstructorWithParametersPage.class, parameters);
+       }
+
+       @Test
+       public void nonDefaultConstructor() {
+               expectedException.expect(WicketRuntimeException.class);
+               expectedException.expectMessage("Unable to create page from class org.apache.wicket.session.DefaultPageFactoryTest$NonDefaultConstructorPage. Class does not have a visible default constructor.");
+
+               pageFactory.newPage(NonDefaultConstructorPage.class);
+       }
+
        /**
-        * Verifies page factory bubbles AbortAndRespondException
+        * Verifies page factory bubbles ResetResponseException
         */
        @Test
        public void abortAndRespondContract()