Wicket tab panels detaching too early

[Neil Curzon <ne...@gmail.com>]

Hi all,

Our Wicket 1.4 project (currently 1.4.3) uses tabs on some pages to display
linked information. For example, an Account may have a User. On the Account
page, there would be a User tab in this case.  The User is a PropertyModel
on a LoadableDetachableModel for the Account (which grabs from the DB).

We notice that whenever one tab accesses any part of the model in its
constructor, we get two queries to the database to display the page. Some
digging revealed the cause:

Tabbed panel does the following (simplified).

1. Instantiate the new Panel being switched to (which causes the
LoadableDetachableModel to load() as the constructor uses it)
2. call addOrReplace with this new Panel. This causes the old Panel to be
removed and detach() to be called on it. Unfortunately, the other tab also
had a PropertyModel on the same Account object. This means that the
LoadableDetachableModel that's already queried the db within this request
will detach()

Later, the rendering causes the LoadableDetachableModel to load() again.

It could be argued that we shouldn't be accessing the model directly in the
constructor but instead setting PropertyModels on its attributes to be
displayed at render time. I have managed to fix almost all of our pages to
do this*, but the problem is how fragile this is. It's difficult to write a
test case that verifies that no page needlessly causes two loads() in any
LodableDetachableModel, and the consequence of such a mistake would be no
less than doubling the load on the database.

I suggest that TabbedPanel should instead remove any old tab before
instantiating the new tab so models won't be unexpectedly detached before
rendering even happens.  I would be willing to supply a JIRA and patch
unless somebody out there a knows better way to do all of this :)

Thanks,
Neil

* My main obstacle has been PageParameters links which don't seem to be able
to take a Model as an argument. Is there a way to work around this?

Re: Wicket tab panels detaching too early

[Neil Curzon <ne...@gmail.com>]

I do understand and appreciate the need to detatch the models. It's just
that if the detached happened a few lines earlier, before the new tab is
instantiated, it would be a lot less error prone with respect to
accidentally causing more db load.

Current mechanism:

1. Instantiate new tab (causes model to load)
2. replace old tab with new tab (detach chains to model that the new tab is
also connected to)
3. render. This causes the above model to reload, twice in the same request

Proposed mechanism

1. Remove current tab if it's already there
2. Instantiate new tab (model loads)
3. Render. Model is already loaded, so no second load.

I think this order is a little less error prone.

On Mon, Jan 25, 2010 at 2:52 PM, Martijn Dashorst <
martijn.dashorst@gmail.com> wrote:

> Wicket needs to detach the replaced panel because it is no longer
> attached to the component hierarchy. If we didn't detach at that time,
> hell [c/w]ould break loose.
>
> We assume with loadable detachable models that people use proper
> caching such that the database won't be hit hard. E.G. if you use
> Hibernate, loading the object for the second time during the request
> would retrieve it from the first level cache (hibernate's Session
> object). If the session is no longer available we assume it is
> retrieved from the second level cache.
>
> In short: detaching the replaced panel is done on purpose.
>
> Martijn
>
> On Mon, Jan 25, 2010 at 7:52 PM, Neil Curzon <ne...@gmail.com>
> wrote:
> > Hi all,
> >
> > Our Wicket 1.4 project (currently 1.4.3) uses tabs on some pages to
> display
> > linked information. For example, an Account may have a User. On the
> Account
> > page, there would be a User tab in this case.  The User is a
> PropertyModel
> > on a LoadableDetachableModel for the Account (which grabs from the DB).
> >
> > We notice that whenever one tab accesses any part of the model in its
> > constructor, we get two queries to the database to display the page. Some
> > digging revealed the cause:
> >
> > Tabbed panel does the following (simplified).
> >
> > 1. Instantiate the new Panel being switched to (which causes the
> > LoadableDetachableModel to load() as the constructor uses it)
> > 2. call addOrReplace with this new Panel. This causes the old Panel to be
> > removed and detach() to be called on it. Unfortunately, the other tab
> also
> > had a PropertyModel on the same Account object. This means that the
> > LoadableDetachableModel that's already queried the db within this request
> > will detach()
> >
> > Later, the rendering causes the LoadableDetachableModel to load() again.
> >
> > It could be argued that we shouldn't be accessing the model directly in
> the
> > constructor but instead setting PropertyModels on its attributes to be
> > displayed at render time. I have managed to fix almost all of our pages
> to
> > do this*, but the problem is how fragile this is. It's difficult to write
> a
> > test case that verifies that no page needlessly causes two loads() in any
> > LodableDetachableModel, and the consequence of such a mistake would be no
> > less than doubling the load on the database.
> >
> > I suggest that TabbedPanel should instead remove any old tab before
> > instantiating the new tab so models won't be unexpectedly detached before
> > rendering even happens.  I would be willing to supply a JIRA and patch
> > unless somebody out there a knows better way to do all of this :)
> >
> > Thanks,
> > Neil
> >
> > * My main obstacle has been PageParameters links which don't seem to be
> able
> > to take a Model as an argument. Is there a way to work around this?
> >
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
> Apache Wicket 1.4 increases type safety for web applications
> Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.4.4
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>

Re: Wicket tab panels detaching too early

[Martijn Dashorst <ma...@gmail.com>]

Wicket needs to detach the replaced panel because it is no longer
attached to the component hierarchy. If we didn't detach at that time,
hell [c/w]ould break loose.

We assume with loadable detachable models that people use proper
caching such that the database won't be hit hard. E.G. if you use
Hibernate, loading the object for the second time during the request
would retrieve it from the first level cache (hibernate's Session
object). If the session is no longer available we assume it is
retrieved from the second level cache.

In short: detaching the replaced panel is done on purpose.

Martijn

On Mon, Jan 25, 2010 at 7:52 PM, Neil Curzon <ne...@gmail.com> wrote:
> Hi all,
>
> Our Wicket 1.4 project (currently 1.4.3) uses tabs on some pages to display
> linked information. For example, an Account may have a User. On the Account
> page, there would be a User tab in this case.  The User is a PropertyModel
> on a LoadableDetachableModel for the Account (which grabs from the DB).
>
> We notice that whenever one tab accesses any part of the model in its
> constructor, we get two queries to the database to display the page. Some
> digging revealed the cause:
>
> Tabbed panel does the following (simplified).
>
> 1. Instantiate the new Panel being switched to (which causes the
> LoadableDetachableModel to load() as the constructor uses it)
> 2. call addOrReplace with this new Panel. This causes the old Panel to be
> removed and detach() to be called on it. Unfortunately, the other tab also
> had a PropertyModel on the same Account object. This means that the
> LoadableDetachableModel that's already queried the db within this request
> will detach()
>
> Later, the rendering causes the LoadableDetachableModel to load() again.
>
> It could be argued that we shouldn't be accessing the model directly in the
> constructor but instead setting PropertyModels on its attributes to be
> displayed at render time. I have managed to fix almost all of our pages to
> do this*, but the problem is how fragile this is. It's difficult to write a
> test case that verifies that no page needlessly causes two loads() in any
> LodableDetachableModel, and the consequence of such a mistake would be no
> less than doubling the load on the database.
>
> I suggest that TabbedPanel should instead remove any old tab before
> instantiating the new tab so models won't be unexpectedly detached before
> rendering even happens.  I would be willing to supply a JIRA and patch
> unless somebody out there a knows better way to do all of this :)
>
> Thanks,
> Neil
>
> * My main obstacle has been PageParameters links which don't seem to be able
> to take a Model as an argument. Is there a way to work around this?
>



-- 
Become a Wicket expert, learn from the best: http://wicketinaction.com
Apache Wicket 1.4 increases type safety for web applications
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.4.4

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org