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

Reply via email to