You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by cm...@wicketbuch.de on 2010/12/01 22:24:59 UTC
[bug?] onInitialize() for Pages?
Hi,
we just ran into an interesting problem. We are using inheritance a lot
in our pages, but at one point we need something in the super page that
depends on the constructor of the sub page. Just like with regular
components, I thought I'd just use onInitialize() for that.
Unfortunately, that didn't work:
at
com....MySubPage.onInitialize(...)
at org.apache.wicket.Component.fireInitialize(Component.java:4050) at
org.apache.wicket.MarkupContainer.initialize(MarkupContainer.java:413)
at org.apache.wicket.Page.componentAdded(Page.java:1589) at
org.apache.wicket.MarkupContainer.addedComponent(MarkupContainer.java:979)
at org.apache.wicket.MarkupContainer.add(MarkupContainer.java:142) at
com....MySuperPage.<init>(...)
See the quickstart at
https://github.com/duesenklipper/wicket-oninitialize
or just do this in the HomePage of any Wicket app:
private final boolean constructorDone;
public HomePage(final PageParameters parameters) {
add(new Label("message", "blah"));
this.constructorDone = true;
}
@Override
protected void onInitialize() {
super.onInitialize();
if (!constructorDone) {
throw new IllegalStateException();
}
}
As soon as I add() the first component to the page in the super
constructor, the page's initialize() is called, which fires
onInitialize. At this time, my onInitialize can't do anything, since
not even the super constructor has been run completely!
Should this really happen this way? I.e., should the Page be
initialized at this point, or should the page's onInitialize call be
deferred later? Or is onInitialize not the right spot to do this for a
page - what would the right way be then?
Thanks
Carl-Eric
www.wicketbuch.de
Re: [bug?] onInitialize() for Pages?
Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Hm. Given what Igor said, I could come up with a different patch that
delays the entire onInitialize thing instead of creating the new
onPageInitialize event.
I don't understand what you mean by "a useless hotspot". onInitialize
doesn't get called recursively on parent components, only on super
classes.
Currently the contract of onInitialize is that it is called at some
point before onConfigure/onBeforeRender. The main use case I see for it is to do things that require the page hierarchy *and/or* that
depend on the constructors having run completely. So I think Page's
onInitialize should not be final, but rather that as Igor said, the
whole initialization step could be done before onConfigure. It's past
1AM now already, but I'll provide a patch for this tomorrow so you can
have a look.
Carl-Eric
www.wicketbuch.de
On Thu, 2 Dec 2010 17:07:10 -0200
Pedro Santos <pe...@gmail.com> wrote:
> yes, delaying onInitialize until the constructor ends for pages isn't
> the goal.
>
> On Thu, Dec 2, 2010 at 5:03 PM, Pedro Santos <pe...@gmail.com>
> wrote:
>
> > Page will always be the root node at the components tree, we don't
> > need an useless hotspot on the framework, that is why I think the
> > page should have an final onInitialize.
> >
> >
> > On Thu, Dec 2, 2010 at 4:53 PM, Igor Vaynberg
> > <ig...@gmail.com>wrote:
> >
> >> No. the contract is that the parent is initialized before the
> >> children.
> >>
> >> Initialization of the entire hierarchy can be delayed until right
> >> before configure is called.
> >>
> >> Igor
> >>
> >> On Dec 2, 2010 10:00 AM, "Carl-Eric Menzel" <cm...@wicketbuch.de>
> >> wrote:
> >>
> >> Okay, I decided should just go ahead and put my money where my
> >> mouth is :)
> >>
> >> Delaying onInitialize for pages isn't really feasible, I think. So
> >> I created a patch that would make onInitialize final in Page (as
> >> Pedro said) and introduces a new event onPageInitialize that is
> >> only called on pages. It honors the same contract as onInitialize,
> >> i.e. it is called only once per page object, at some point before
> >> onBeforeRender.
> >>
> >> See https://issues.apache.org/jira/browse/WICKET-3218 for details.
> >>
> >>
> >> Carl-Eric
> >> www.wicketbuch.de
> >> On Thu, 2 Dec 2010 13:50:30 +0100
> >>
> >> Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
> >>
> >> >
> >>
> >> > > Make onInitialize final on pages is an good idea. It is
> >> > > designed to children have a guaranty...
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
>
>
Re: [bug?] onInitialize() for Pages?
Posted by Pedro Santos <pe...@gmail.com>.
yes, delaying onInitialize until the constructor ends for pages isn't the
goal.
On Thu, Dec 2, 2010 at 5:03 PM, Pedro Santos <pe...@gmail.com> wrote:
> Page will always be the root node at the components tree, we don't need an
> useless hotspot on the framework, that is why I think the page should have
> an final onInitialize.
>
>
> On Thu, Dec 2, 2010 at 4:53 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> No. the contract is that the parent is initialized before the children.
>>
>> Initialization of the entire hierarchy can be delayed until right before
>> configure is called.
>>
>> Igor
>>
>> On Dec 2, 2010 10:00 AM, "Carl-Eric Menzel" <cm...@wicketbuch.de>
>> wrote:
>>
>> Okay, I decided should just go ahead and put my money where my mouth
>> is :)
>>
>> Delaying onInitialize for pages isn't really feasible, I think. So I
>> created a patch that would make onInitialize final in Page (as Pedro
>> said) and introduces a new event onPageInitialize that is only called
>> on pages. It honors the same contract as onInitialize, i.e. it is
>> called only once per page object, at some point before onBeforeRender.
>>
>> See https://issues.apache.org/jira/browse/WICKET-3218 for details.
>>
>>
>> Carl-Eric
>> www.wicketbuch.de
>> On Thu, 2 Dec 2010 13:50:30 +0100
>>
>> Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
>>
>> >
>>
>> > > Make onInitialize final on pages is an good idea. It is designed to
>> > > children have a guaranty...
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
--
Pedro Henrique Oliveira dos Santos
Re: [bug?] onInitialize() for Pages?
Posted by Pedro Santos <pe...@gmail.com>.
Page will always be the root node at the components tree, we don't need an
useless hotspot on the framework, that is why I think the page should have
an final onInitialize.
On Thu, Dec 2, 2010 at 4:53 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> No. the contract is that the parent is initialized before the children.
>
> Initialization of the entire hierarchy can be delayed until right before
> configure is called.
>
> Igor
>
> On Dec 2, 2010 10:00 AM, "Carl-Eric Menzel" <cm...@wicketbuch.de> wrote:
>
> Okay, I decided should just go ahead and put my money where my mouth
> is :)
>
> Delaying onInitialize for pages isn't really feasible, I think. So I
> created a patch that would make onInitialize final in Page (as Pedro
> said) and introduces a new event onPageInitialize that is only called
> on pages. It honors the same contract as onInitialize, i.e. it is
> called only once per page object, at some point before onBeforeRender.
>
> See https://issues.apache.org/jira/browse/WICKET-3218 for details.
>
>
> Carl-Eric
> www.wicketbuch.de
> On Thu, 2 Dec 2010 13:50:30 +0100
>
> Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
>
> >
>
> > > Make onInitialize final on pages is an good idea. It is designed to
> > > children have a guaranty...
>
--
Pedro Henrique Oliveira dos Santos
Re: [bug?] onInitialize() for Pages?
Posted by Igor Vaynberg <ig...@gmail.com>.
No. the contract is that the parent is initialized before the children.
Initialization of the entire hierarchy can be delayed until right before
configure is called.
Igor
On Dec 2, 2010 10:00 AM, "Carl-Eric Menzel" <cm...@wicketbuch.de> wrote:
Okay, I decided should just go ahead and put my money where my mouth
is :)
Delaying onInitialize for pages isn't really feasible, I think. So I
created a patch that would make onInitialize final in Page (as Pedro
said) and introduces a new event onPageInitialize that is only called
on pages. It honors the same contract as onInitialize, i.e. it is
called only once per page object, at some point before onBeforeRender.
See https://issues.apache.org/jira/browse/WICKET-3218 for details.
Carl-Eric
www.wicketbuch.de
On Thu, 2 Dec 2010 13:50:30 +0100
Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
>
> > Make onInitialize final on pages is an good idea. It is designed to
> > children have a guaranty...
Re: [bug?] onInitialize() for Pages?
Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Okay, I decided should just go ahead and put my money where my mouth
is :)
Delaying onInitialize for pages isn't really feasible, I think. So I
created a patch that would make onInitialize final in Page (as Pedro
said) and introduces a new event onPageInitialize that is only called
on pages. It honors the same contract as onInitialize, i.e. it is
called only once per page object, at some point before onBeforeRender.
See https://issues.apache.org/jira/browse/WICKET-3218 for details.
Carl-Eric
www.wicketbuch.de
On Thu, 2 Dec 2010 13:50:30 +0100
Carl-Eric Menzel <cm...@wicketbuch.de> wrote:
>
> > Make onInitialize final on pages is an good idea. It is designed to
> > children have a guaranty that the path to page exists, not to
> > guaranty that an parent has all its children on the hierarchy
> > already. So it is fairly more useful to children than for parent
> > components. I just don't know if that change implies in rename the
> > onInitialize to notifyHierarchy or similar.
>
> While I think making onInitialize final for Pages is a *valid*
> solution, I think it is not really useful. I still can have the same
> problem in Pages that onInitialize is trying to solve for Components:
> That some things can only be done once *all* constructors have run.
> With Components, I can use onInitialize. I would really like to be
> able to do the same for Pages.
>
> Carl-Eric
> www.wicketbuch.de
Re: [bug?] onInitialize() for Pages?
Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
> Make onInitialize final on pages is an good idea. It is designed to
> children have a guaranty that the path to page exists, not to
> guaranty that an parent has all its children on the hierarchy
> already. So it is fairly more useful to children than for parent
> components. I just don't know if that change implies in rename the
> onInitialize to notifyHierarchy or similar.
While I think making onInitialize final for Pages is a *valid*
solution, I think it is not really useful. I still can have the same
problem in Pages that onInitialize is trying to solve for Components:
That some things can only be done once *all* constructors have run.
With Components, I can use onInitialize. I would really like to be able
to do the same for Pages.
Carl-Eric
www.wicketbuch.de
Re: [bug?] onInitialize() for Pages?
Posted by Pedro Santos <pe...@gmail.com>.
On Thu, Dec 2, 2010 at 8:39 AM, Carl-Eric Menzel <cm...@wicketbuch.de>wrote:
> Hi Pedro,
>
> thanks for your reply.
>
> I find the current behavior of onInitialize in Pages *very* surprising
> and unexpected. It doesn't say anywhere in the documentation that using
> onInitialize prohibits use of constructors.
>
> Also, this only seems to affect pages. In all other components it seems
> to work fine, which makes this behavior rather inconsistent.
>
> Also, this quickstart is of course somewhat simplified. My real-world
> issue is that I am extending from a chain of superclasses that all do
> something in their constructors. It's not realistic to switch the
> entire hierarchy from constructors to onInitialize.
>
> Also again ;-), right now onInitialize for Pages is broken. For all
> other components, onInitialize is called *after* all constructors have
> run. Only for Pages this is done in the middle of the constructor.
>
> I think either Page should have a final onInitialize implementation to
> really prohibit onInitialize being used at all, or there should be a
> special case for Page causing onInitialize to be called just before
> onBeforeRender.
Make onInitialize final on pages is an good idea. It is designed to children
have a guaranty that the path to page exists, not to guaranty that an parent
has all its children on the hierarchy already. So it is fairly more useful
to children than for parent components. I just don't know if that change
implies in rename the onInitialize to notifyHierarchy or similar.
> This would be consistent with Java object construction
> and also consistent with the contract of onInitialize, which simply
> says "it is called sometime before onBeforeRender()".
>
> Would that be an acceptable solution? Want me to provide a patch?
>
> Thanks
> Carl-Eric
> www.wicketbuch.de
>
> On Wed, 1 Dec 2010 20:31:35 -0200
> Pedro Santos <pe...@gmail.com> wrote:
>
> > Hi Carl, you are mixing two different initializations strategies,
> > that is why you are finding it weird. If you are using an overwritten
> > onInitialize to initialize your component, do all your initialization
> > there, even add your component's children.
> >
> > For example, your quickstart works fine as:
> > @Override
> > protected void onInitialize() {
> > super.onInitialize();
> > add(new Label("message",
> > "If you see this message wicket is properly
> > configured and running"));
> > if (!constructorDone) {
> > throw new IllegalStateException();
> > }
> > }
> >
> > On Wed, Dec 1, 2010 at 7:24 PM, <cm...@wicketbuch.de> wrote:
> >
> > > Hi,
> > >
> > > we just ran into an interesting problem. We are using inheritance a
> > > lot in our pages, but at one point we need something in the super
> > > page that depends on the constructor of the sub page. Just like
> > > with regular components, I thought I'd just use onInitialize() for
> > > that.
> > >
> > > Unfortunately, that didn't work:
> > >
> > > at
> > > com....MySubPage.onInitialize(...)
> > > at org.apache.wicket.Component.fireInitialize(Component.java:4050)
> > > at
> > > org.apache.wicket.MarkupContainer.initialize(MarkupContainer.java:413)
> > > at org.apache.wicket.Page.componentAdded(Page.java:1589) at
> > >
> org.apache.wicket.MarkupContainer.addedComponent(MarkupContainer.java:979)
> > > at org.apache.wicket.MarkupContainer.add(MarkupContainer.java:142)
> > > at com....MySuperPage.<init>(...)
> > >
> > > See the quickstart at
> > > https://github.com/duesenklipper/wicket-oninitialize
> > >
> > > or just do this in the HomePage of any Wicket app:
> > >
> > > private final boolean constructorDone;
> > > public HomePage(final PageParameters parameters) {
> > > add(new Label("message", "blah"));
> > >
> > > this.constructorDone = true;
> > > }
> > >
> > > @Override
> > > protected void onInitialize() {
> > > super.onInitialize();
> > > if (!constructorDone) {
> > > throw new IllegalStateException();
> > > }
> > > }
> > >
> > > As soon as I add() the first component to the page in the super
> > > constructor, the page's initialize() is called, which fires
> > > onInitialize. At this time, my onInitialize can't do anything, since
> > > not even the super constructor has been run completely!
> > >
> > > Should this really happen this way? I.e., should the Page be
> > > initialized at this point, or should the page's onInitialize call be
> > > deferred later? Or is onInitialize not the right spot to do this
> > > for a page - what would the right way be then?
> > >
> > > Thanks
> > > Carl-Eric
> > > www.wicketbuch.de
> > >
> >
> >
> >
>
>
--
Pedro Henrique Oliveira dos Santos
Re: [bug?] onInitialize() for Pages?
Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Hi Pedro,
thanks for your reply.
I find the current behavior of onInitialize in Pages *very* surprising
and unexpected. It doesn't say anywhere in the documentation that using
onInitialize prohibits use of constructors.
Also, this only seems to affect pages. In all other components it seems
to work fine, which makes this behavior rather inconsistent.
Also, this quickstart is of course somewhat simplified. My real-world
issue is that I am extending from a chain of superclasses that all do
something in their constructors. It's not realistic to switch the
entire hierarchy from constructors to onInitialize.
Also again ;-), right now onInitialize for Pages is broken. For all
other components, onInitialize is called *after* all constructors have
run. Only for Pages this is done in the middle of the constructor.
I think either Page should have a final onInitialize implementation to
really prohibit onInitialize being used at all, or there should be a
special case for Page causing onInitialize to be called just before
onBeforeRender. This would be consistent with Java object construction
and also consistent with the contract of onInitialize, which simply
says "it is called sometime before onBeforeRender()".
Would that be an acceptable solution? Want me to provide a patch?
Thanks
Carl-Eric
www.wicketbuch.de
On Wed, 1 Dec 2010 20:31:35 -0200
Pedro Santos <pe...@gmail.com> wrote:
> Hi Carl, you are mixing two different initializations strategies,
> that is why you are finding it weird. If you are using an overwritten
> onInitialize to initialize your component, do all your initialization
> there, even add your component's children.
>
> For example, your quickstart works fine as:
> @Override
> protected void onInitialize() {
> super.onInitialize();
> add(new Label("message",
> "If you see this message wicket is properly
> configured and running"));
> if (!constructorDone) {
> throw new IllegalStateException();
> }
> }
>
> On Wed, Dec 1, 2010 at 7:24 PM, <cm...@wicketbuch.de> wrote:
>
> > Hi,
> >
> > we just ran into an interesting problem. We are using inheritance a
> > lot in our pages, but at one point we need something in the super
> > page that depends on the constructor of the sub page. Just like
> > with regular components, I thought I'd just use onInitialize() for
> > that.
> >
> > Unfortunately, that didn't work:
> >
> > at
> > com....MySubPage.onInitialize(...)
> > at org.apache.wicket.Component.fireInitialize(Component.java:4050)
> > at
> > org.apache.wicket.MarkupContainer.initialize(MarkupContainer.java:413)
> > at org.apache.wicket.Page.componentAdded(Page.java:1589) at
> > org.apache.wicket.MarkupContainer.addedComponent(MarkupContainer.java:979)
> > at org.apache.wicket.MarkupContainer.add(MarkupContainer.java:142)
> > at com....MySuperPage.<init>(...)
> >
> > See the quickstart at
> > https://github.com/duesenklipper/wicket-oninitialize
> >
> > or just do this in the HomePage of any Wicket app:
> >
> > private final boolean constructorDone;
> > public HomePage(final PageParameters parameters) {
> > add(new Label("message", "blah"));
> >
> > this.constructorDone = true;
> > }
> >
> > @Override
> > protected void onInitialize() {
> > super.onInitialize();
> > if (!constructorDone) {
> > throw new IllegalStateException();
> > }
> > }
> >
> > As soon as I add() the first component to the page in the super
> > constructor, the page's initialize() is called, which fires
> > onInitialize. At this time, my onInitialize can't do anything, since
> > not even the super constructor has been run completely!
> >
> > Should this really happen this way? I.e., should the Page be
> > initialized at this point, or should the page's onInitialize call be
> > deferred later? Or is onInitialize not the right spot to do this
> > for a page - what would the right way be then?
> >
> > Thanks
> > Carl-Eric
> > www.wicketbuch.de
> >
>
>
>
Re: [bug?] onInitialize() for Pages?
Posted by Pedro Santos <pe...@gmail.com>.
Hi Carl, you are mixing two different initializations strategies, that is
why you are finding it weird. If you are using an overwritten onInitialize
to initialize your component, do all your initialization there, even add
your component's children.
For example, your quickstart works fine as:
@Override
protected void onInitialize() {
super.onInitialize();
add(new Label("message",
"If you see this message wicket is properly configured and
running"));
if (!constructorDone) {
throw new IllegalStateException();
}
}
On Wed, Dec 1, 2010 at 7:24 PM, <cm...@wicketbuch.de> wrote:
> Hi,
>
> we just ran into an interesting problem. We are using inheritance a lot
> in our pages, but at one point we need something in the super page that
> depends on the constructor of the sub page. Just like with regular
> components, I thought I'd just use onInitialize() for that.
>
> Unfortunately, that didn't work:
>
> at
> com....MySubPage.onInitialize(...)
> at org.apache.wicket.Component.fireInitialize(Component.java:4050) at
> org.apache.wicket.MarkupContainer.initialize(MarkupContainer.java:413)
> at org.apache.wicket.Page.componentAdded(Page.java:1589) at
> org.apache.wicket.MarkupContainer.addedComponent(MarkupContainer.java:979)
> at org.apache.wicket.MarkupContainer.add(MarkupContainer.java:142) at
> com....MySuperPage.<init>(...)
>
> See the quickstart at
> https://github.com/duesenklipper/wicket-oninitialize
>
> or just do this in the HomePage of any Wicket app:
>
> private final boolean constructorDone;
> public HomePage(final PageParameters parameters) {
> add(new Label("message", "blah"));
>
> this.constructorDone = true;
> }
>
> @Override
> protected void onInitialize() {
> super.onInitialize();
> if (!constructorDone) {
> throw new IllegalStateException();
> }
> }
>
> As soon as I add() the first component to the page in the super
> constructor, the page's initialize() is called, which fires
> onInitialize. At this time, my onInitialize can't do anything, since
> not even the super constructor has been run completely!
>
> Should this really happen this way? I.e., should the Page be
> initialized at this point, or should the page's onInitialize call be
> deferred later? Or is onInitialize not the right spot to do this for a
> page - what would the right way be then?
>
> Thanks
> Carl-Eric
> www.wicketbuch.de
>
--
Pedro Henrique Oliveira dos Santos