I did a little more research before changing my code and found other
problematic cases which, I think, deserve mention.
Methodology: ran the regex "get(?:model)?object\(\)" on wicket 1.5
source and inspected the results.
Some results:
1-Component#setDefaultModelObject(final Object) makes this call
"getModelComparator().compare(this, object)" which does
getDefaultModelObject()
2-FormComponent#updateCollectionModel(FormComponent<Collection<S>>)
calls formComponent.getModelObject()
3-ListMultipleChoice#convertChoiceIdsToChoices and
AbstractSingleSelectChoice#convertChoiceIdToChoice use
AbstractChoice#getChoices() which calls choices.getObject()
4-FormComponent.MessageSource#getLabel() calls fc.getLabel().getObject()
and is used when building an error message
[1] and [2] relate to FormComponent model update which can clearly
trigger the load of LDMs which can be out of date after form processing.
[3] is for input conversion and can also leave some out of date LDMs
attached after form processing.
For [4], imagine a case where the label (a custom model) of a form
component references the name of a person (property model) which is
fetched from a LDM of a person. Granted, usually when there is an error
message for a form component, the whole form does nothing. However, as
AjaxFallbackButton shows, there can still be some processing happening
in onError(AjaxRequestTarget target, Form<?> form).
I haven't found this pattern in Wicket's source, but consider also
validators. They can also need to access some LDMs for validation and I
don't think this is bad practice. The fix for this is to require
validators to explicitly detach the models they use for validation, but
I would argue that it wasn't historically necessary and is easy to forget
All this amounts to a lot of stuff to detach manually and I probably
missed other cases as well. Also, this affects stateful and stateless
pages since it is related to form processing.
Regards,
Bertrand
On 28/10/2011 5:04 PM, Bertrand Guay-Paquet wrote:
Thanks for clearing that up. I didn't know it was considered bad
practice to access _any_ model's object before rendering. In that
case, my points are moot (but I have some code I must change...)
I wasn't only trying to build an impossible example btw :) It was
pretty close to what I intended on doing and was wondering how to
solve these issues when Maarten started the thread.
Cheers,
Bertrand
On 28/10/2011 4:42 PM, Igor Vaynberg wrote:
i suppose you can contrive an example that breaks any situation :)
in this case you are making a lot of assumptions: the two panels are
not sharing a model even though they are both referencing the same
person, the show panel loads the model prematurely instead of at
render time. these two things are considered bad practice anyways.
if you wanted to solves this in a neat way you can use events.
updatepersonpanel can send out a PersonUpdated event and
ShowPersonPanel can listen for it and detach its model.
-igor
On Fri, Oct 28, 2011 at 1:15 PM, Bertrand Guay-Paquet
<ber...@step.polymtl.ca> wrote:
On 28/10/2011 12:49 PM, Igor Vaynberg wrote:
i think you are blowing this way out of proportion.
I hope so :)
this only affects components that (A) modify their own model but do
not update it and *also* (B) have their model loaded for some reason
before they update it.
This is not the type of situation I meant. What you describe is a clear
situation where the component doing the update can easily handle
detaching
its model for a refresh.
With a business layer and DTOs, doing the update correctly on the
wicket
model directly is impossible without replicating the business logic
in the
web client. That's why I say that the model can be detached and
refreshed
above and not just updated.
We could also argue that event handlers accessing model objects (see my
issue 1) should systematically detach all accessed models after their
processing to avoid stale models.
However, Issue 2 still remains. Here is a more detailed example to
illustrate it:
class PersonDto {
private int id;
// other fields and methods
}
class ShowPersonPanel extends Panel {
private int personId;
// other fields
MyPanel (IModel<PersonDto> model) {
personId = model.getObject().getId(); // triggers load of LDM
// Pass model to other components + other stuff
}
}
class UpdatePersonPanel extends Panel {
// adds a component of type StatelessAjaxUpdatePersonLink
}
class StatelessAjaxUpdatePersonLink extends Link {
private IModel<PersonDto> personModel;
onClick(AjaxRequestTarget target) {
updateSomePersonAttribute(personModel);
personModel.detach();
target.add(this);
}
}
Consider a stateless page with a ShowPersonPanel and an
UpdatePersonPanel.
Further, when the StatelessAjaxUpdatePersonLink is clicked, the
ShowPersonPanel is added to the request target by other code.
Sequence of events for stateless ajax request:
1-New page instance created with ShowPersonPanel and
UpdatePersonPanel, not
sharing the same IModel<PersonDto> instance for some reason. Note
that the
constructor of ShowPersonPanel triggers the load of its PersonDto LDM.
2-Ajax link listener is invoked which updates the persistent Person and
detaches the now stale model
3-ShowPersonPanel is added to the ajax target (through events perhaps)
4-ShowPersonPanel and UpdatePersonPanel are rendered.
ShowPersonPanel has
stale LDM and UpdatePersonPanel has fresh LDM.
I don't see how to manage this without either A) a blanket detach()
of all
models or B) some complicated panel and model inter-dependency
management.
Note that this sequence of events is not the same for stateful
pages. The
stateful pages do not construct a new page instance before invoking
the link
listener so no outside LDMs are loaded.
Regards,
Bertrand
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org