Wow, thx, found other typo in the commentary article as well, should be ok now.
Yep, I would rather to keep both as public. We need BookmarkablePage with a public constructor receiving a PageParameters or no parameters in order to be tested as bookmarkable. The majority of pages we have in the tests are public classes, even the inner ones. The problem is that if you downgrade their visibility, the framework will not be able to create instances for them or to test their bookmarkable aspect. Also it's all ok they show up in some IDE took since they can easily be useful in some other test. Pedro Santos On Thu, Jan 10, 2013 at 7:29 AM, Martin Grigorov <mgrigo...@apache.org>wrote: > On Thu, Jan 10, 2013 at 12:58 AM, <pe...@apache.org> wrote: > > > Updated Branches: > > refs/heads/sandbox/bookmarkable-callback-url fba8bddd9 -> 1ec0b8b36 > > > > > > WICKET-4932: testing if the page is bookmarkable before to encode the > > callback url for a component inside it > > > > Project: http://git-wip-us.apache.org/repos/asf/wicket/repo > > Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/1ec0b8b3 > > Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/1ec0b8b3 > > Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/1ec0b8b3 > > > > Branch: refs/heads/sandbox/bookmarkable-callback-url > > Commit: 1ec0b8b3637feb19da4f87b485e96bfc7ce4e992 > > Parents: fba8bdd > > Author: Pedro Santos <pe...@apache.com> > > Authored: Wed Jan 9 20:58:40 2013 -0200 > > Committer: Pedro Santos <pe...@apache.com> > > Committed: Wed Jan 9 20:58:40 2013 -0200 > > > > ---------------------------------------------------------------------- > > .../request/mapper/AbstractBookmarkableMapper.java | 2 +- > > .../mapper/AbstractBookmarkableMapperTest.java | 70 > ++++++++++++++- > > .../request/mapper/BookmarkableMapperTest.java | 6 ++ > > .../wicket/request/mapper/PackageMapperTest.java | 6 ++ > > 4 files changed, 80 insertions(+), 4 deletions(-) > > ---------------------------------------------------------------------- > > > > > > > > > http://git-wip-us.apache.org/repos/asf/wicket/blob/1ec0b8b3/wicket-core/src/main/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapper.java > > ---------------------------------------------------------------------- > > diff --git > > > a/wicket-core/src/main/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapper.java > > > b/wicket-core/src/main/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapper.java > > index 47ac13a..7e164dd 100644 > > --- > > > a/wicket-core/src/main/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapper.java > > +++ > > > b/wicket-core/src/main/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapper.java > > @@ -333,7 +333,7 @@ public abstract class AbstractBookmarkableMapper > > extends AbstractComponentMapper > > > > protected boolean checkPageClass(Class<? extends > IRequestablePage> > > pageClass) > > { > > - return true; > > + return > > Application.get().getPageFactory().isBookmarkable(pageClass); > > } > > > > public Url mapHandler(IRequestHandler requestHandler) > > > > > > > http://git-wip-us.apache.org/repos/asf/wicket/blob/1ec0b8b3/wicket-core/src/test/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapperTest.java > > ---------------------------------------------------------------------- > > diff --git > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapperTest.java > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapperTest.java > > index 693adec..705f14c 100644 > > --- > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapperTest.java > > +++ > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/AbstractBookmarkableMapperTest.java > > @@ -17,13 +17,22 @@ package org.apache.wicket.request.mapper; > > * limitations under the License. > > */ > > > > +import static org.hamcrest.CoreMatchers.notNullValue; > > +import static org.hamcrest.CoreMatchers.nullValue; > > + > > import org.apache.wicket.MockPage; > > import org.apache.wicket.WicketTestCase; > > +import org.apache.wicket.markup.html.link.ILinkListener; > > import org.apache.wicket.protocol.http.PageExpiredException; > > import org.apache.wicket.request.Request; > > import org.apache.wicket.request.Url; > > +import org.apache.wicket.request.component.IRequestableComponent; > > +import > > > org.apache.wicket.request.handler.BookmarkableListenerInterfaceRequestHandler; > > +import > org.apache.wicket.request.handler.ListenerInterfaceRequestHandler; > > +import org.apache.wicket.request.handler.PageAndComponentProvider; > > import org.apache.wicket.request.mapper.info.PageInfo; > > import org.junit.Assert; > > +import org.junit.Before; > > import org.junit.Test; > > > > /** > > @@ -34,7 +43,14 @@ public class AbstractBookmarkableMapperTest extends > > WicketTestCase > > > > private static final int NOT_RENDERED_COUNT = 2; > > private static final int EXPIRED_ID = 2; > > + private AbstractBookmarkableMapperStub mapper; > > > > + /** */ > > + @Before > > + public void initialize() > > + { > > + mapper = new AbstractBookmarkableMapperStub(); > > + } > > > > /** > > * <a href="https://issues.apache.org/jira/browse/WICKET-4932 > > ">WICKET-4932</a> > > @@ -43,12 +59,54 @@ public class AbstractBookmarkableMapperTest extends > > WicketTestCase > > public void > > itFailsToProcessAnExpiredPageIfShouldNotRecreateMountedPagesAfterExpiry() > > { > > > > > tester.getApplication().getPageSettings().setRecreateMountedPagesAfterExpiry(false); > > - AbstractBookmarkableMapperStub mapper = new > > AbstractBookmarkableMapperStub(); > > mapper.processHybrid(new PageInfo(EXPIRED_ID), > > MockPage.class, null, NOT_RENDERED_COUNT); > > Assert.fail("it shouldn't process expired pages if the > app > > was flagged to not recreated mounted pages after expiry"); > > } > > > > - /** only a stub since we are testing an abstract class */ > > + /** */ > > + @Test > > + public void > > itDoenstEndodesBookmarkableInfoForCallbacksInNonBookmarkablePages() > > > > There is a typo in the method name (endodes) > > > > + { > > + assertThat(mapper.mapHandler(anInterfaceHandlerFor(new > > NonBookmarkablePage(1))), > > + nullValue()); > > + assertThat(mapper.mapHandler(anInterfaceHandlerFor(new > > BookmarkablePage())), notNullValue()); > > + } > > + > > + private ListenerInterfaceRequestHandler > > anInterfaceHandlerFor(MockPage page) > > + { > > + IRequestableComponent component = page.get("bar:foo"); > > + return new ListenerInterfaceRequestHandler(new > > PageAndComponentProvider(page, component), > > + ILinkListener.INTERFACE); > > + } > > + > > + /** > > + * An non bookmarkable page since there's no default constructor > > + */ > > + public static class NonBookmarkablePage extends MockPage > > > > private ? > I prefer private because otherwise the IDE suggests this class in places > where I don't wont it (like other tests). > > > > + { > > + /** */ > > + private static final long serialVersionUID = 1L; > > + > > + /** > > + * @param aMandatoryParameter > > + */ > > + public NonBookmarkablePage(int aMandatoryParameter) > > + { > > + } > > + } > > + > > + /** > > + * An bookmarkable page since there's a default constructor > > + */ > > + public static class BookmarkablePage extends MockPage > > > > private ? > > > > + { > > + /** */ > > + private static final long serialVersionUID = 1L; > > + } > > + > > + /** > > + * only a stub since we are testing an abstract class > > + */ > > private static class AbstractBookmarkableMapperStub extends > > AbstractBookmarkableMapper > > { > > > > @@ -61,7 +119,7 @@ public class AbstractBookmarkableMapperTest extends > > WicketTestCase > > @Override > > protected Url buildUrl(UrlInfo info) > > { > > - return null; > > + return new Url(); > > } > > > > @Override > > @@ -76,6 +134,12 @@ public class AbstractBookmarkableMapperTest extends > > WicketTestCase > > return 0; > > } > > > > + @Override > > + protected Url > > mapBookmarkableHandler(BookmarkableListenerInterfaceRequestHandler > handler) > > + { > > + return super.mapBookmarkableHandler(handler); > > + } > > + > > } > > > > } > > > > > > > http://git-wip-us.apache.org/repos/asf/wicket/blob/1ec0b8b3/wicket-core/src/test/java/org/apache/wicket/request/mapper/BookmarkableMapperTest.java > > ---------------------------------------------------------------------- > > diff --git > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/BookmarkableMapperTest.java > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/BookmarkableMapperTest.java > > index 628ae04..c3b013d 100644 > > --- > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/BookmarkableMapperTest.java > > +++ > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/BookmarkableMapperTest.java > > @@ -48,6 +48,12 @@ public class BookmarkableMapperTest extends > > AbstractMapperTest > > { > > return context; > > } > > + > > + @Override > > + protected boolean checkPageClass(java.lang.Class<? > extends > > IRequestablePage> pageClass) > > + { > > + return true; > > + } > > }; > > > > private static final String PAGE_CLASS_NAME = > > MockPage.class.getName(); > > > > > > > http://git-wip-us.apache.org/repos/asf/wicket/blob/1ec0b8b3/wicket-core/src/test/java/org/apache/wicket/request/mapper/PackageMapperTest.java > > ---------------------------------------------------------------------- > > diff --git > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/PackageMapperTest.java > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/PackageMapperTest.java > > index fa8e6c8..67bff94 100644 > > --- > > > a/wicket-core/src/test/java/org/apache/wicket/request/mapper/PackageMapperTest.java > > +++ > > > b/wicket-core/src/test/java/org/apache/wicket/request/mapper/PackageMapperTest.java > > @@ -52,6 +52,12 @@ public class PackageMapperTest extends > > AbstractMapperTest > > { > > return context; > > } > > + > > + @Override > > + protected boolean checkPageClass(java.lang.Class<? > extends > > IRequestablePage> pageClass) > > + { > > + return true; > > + } > > }; > > > > private static final String PAGE_CLASS_NAME = > > MockPage.class.getSimpleName(); > > > > > > > -- > Martin Grigorov > jWeekend > Training, Consulting, Development > http://jWeekend.com <http://jweekend.com/> >
