On Thu, Jan 10, 2013 at 7:39 PM, Pedro Santos <pedros...@gmail.com> wrote:
> 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. > Yes, this is a good reason. > > Also it's all ok they show up in some IDE took since they can easily > be useful in some other test. > In this case I think the class should not be inner. > > 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/> > > > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com <http://jweekend.com/>
