Not sure, they are closely related to its outer class and their code next to each other exposes their fundamental difference giving it readability. It's your call.
Pedro Santos On Fri, Jan 11, 2013 at 6:28 AM, Martin Grigorov <[email protected]>wrote: > On Thu, Jan 10, 2013 at 7:39 PM, Pedro Santos <[email protected]> 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 <[email protected] > > >wrote: > > > > > On Thu, Jan 10, 2013 at 12:58 AM, <[email protected]> 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 <[email protected]> > > > > Authored: Wed Jan 9 20:58:40 2013 -0200 > > > > Committer: Pedro Santos <[email protected]> > > > > 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/> >
