problem here is

4) DataView is detached (internalDetach() is called at the end of
renderComponent()) and the cached RowCount is cleared

we shouldnt detach components right away after rendering them in ajax, but
wait until all ajax components have been rendered and then detach.

problem is people dont create jira issues for these things and they tend to
get lost.

feel free to add your thoughts to
https://issues.apache.org/jira/browse/WICKET-175

-igor



On 12/19/06, Martin Benda <[EMAIL PROTECTED]> wrote:

Hi folks!

Today I've accidentally discovered another bug involving
attaching/detaching
of compontns/models. Because I use wicket daily (and I really do not want
to
use anything else since the very day I tasted it:-)) and these bugs are
becoming show-stoppers for me, I decided to try to describe the problems
I've
discovered and propose a solution...

AFAIK there are two similar (and in my opinion critical) bugs for more
than a
month:

1) Models which are attached during an AJAX request processing are not
detached at the end of the request. This can be observerved in the
wicket-examples/ajax/Form Example example (the feedback panel is not
updated
correctly).

2) Models which are attached during the "update models" phase of a form
submission are not detached if the response is redirected to some other
page
(by calling setResponsePage() inside Form#onSubmit()).

The newly discovered problem is a little bit more complicated:

I have a DataView (with some IDataProvider) and my own NavigatorPanel
which
provides navigation functionality for the DataView. The navigation and
filtering is done through AJAX requests. The problem is that sometimes -
when
the number of data items provided by the IDataProvider changes and the
number
of rows per page is greater than the total number of rows - the DataView
renders wrong number of rows.

Here is what really happens:

--- ajax request one ---
1) DataView rendering begins
2) DataView asks the IDataProvider for the total number of rows and caches
it
so the IDataProvider#size() should be called only once per request -
subsequent calls to getRowCount() return the cached value
3) DataView rendering ends
4) DataView is detached (internalDetach() is called at the end of
renderComponent()) and the cached RowCount is cleared
5) NavigatorPanel rendering begins
6) NavigatorPanel calls IPageable#getPageCount() on DataView - this calls
getRowCount() which asks IDataProvider and caches the row count *again*
7) NavigatorPanel rendering ends
8) NavigatorPanel is detached
--- ajax request two ---
1) DataView rendering begins
2) DataView does *not* ask the IDataProvider for the current number of
rows
and uses the cached (and stale) value from the last request
3) DataView renders wrong number of rows...
...

I have looked at the wicket source code and the detach logic is IMHO quite
chaotic and scattered in many places. It would be nice and much error
prone
if all detaching happened in one place.

This is what I propose:

- remove Page.internalDetach() from all RequestTargets
- remove internalDetach() from Component#renderComponent()
- remove child.internalDetach() form MarkupContainer#internalDetach()
- add "Set<Component> attachedComponents" to the RequestCycle class
- every attached component adds itself to the attachedComponents set using
some new IRequestCycle API (probably inside Component#internalAttach())
- at the end of a request (e.g. inside RequestCycle#onEndRequest()) the
method
internalDetach() is called on all components in the attachedComponents set
- call detachModels() from Component#internalOnDetach(), not from
Page#internalOnDetach()
- remove the method Page#detachModels()

This would IMHO solve all the not-detached bugs but I realize that the
proposed changes are quite extensive...

What do you think?

Regards,
Bendis

Reply via email to