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 < [email protected]> 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 <[email protected]> > 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: [email protected] > For additional commands, e-mail: [email protected] > >
