I think this would be not such a big problem as long as addStateChange() was public. A Model which needs versionioning of its internal state would just take any component in the construtor and add the Change to it on modification. In the end the changes will all endup in the page.

The Problem Igor and me were talking about is more about POJOs behind the IModel which need to get versioned but do not know anything about wicket-versioning. For such POJOs you have to version somehow from the outside before you make a change. Currently you do this with modelChanging() but I think this is broken.

Ie a FormComponent (or the IModel which does the change) must have a way to indicate that it is now going to change the underlying POJO and that the POJO should be saved for change.

To do this properly IMO component must have a way to decide
a) wheter the POJO should be versioned at all,
b) wheter a sourounding IModel (ie VersionablePOJOModel) takes care of the versioning and the component just indicates that the pojo was changed
c) if not b) how to version it (clone() or something better).

Out of my head: Mybe we should make for such POJOs a cglib proxy which does the versioning an setters - and keep the model versioionng (except setModel()) out of Component. If a cglib proxy does not do it than the user has to implenet a versionaware object.

Christian


On Wed, 7 Dec 2005 22:40:03 +0100, Johan Compagner <[EMAIL PROTECTED]> wrote:

The problem with this is (i looked further how IVersionable should work)
is that in a Model doesn't have to be ONE single object is the object to
version.
For exampe the HibernateObjectModel has these:

    private boolean unproxy;
    private final Class objectClass;
    private final boolean createNewObjectWhenIdIsNull;
    private NullIdHandler nullIdHandler;


And then i don't count the things of its super class PersistentObjectModel:

    private IModel idModel;
    private ISelectObjectAction selectObjectAction;

but maybe a model should just do all its stuff itself?
So if one of these properties change for example the selectObjectAction then
the model will add a Change() object to the version manager.
The problem is that a model doesn't know the component/page where it is
bound on so it can't call addStateChange()

johan


On 12/7/05, Igor Vaynberg <[EMAIL PROTECTED]> wrote:

heh, excuse me while i pull my head out of my ass. maybe i should stop
staying up till 1am every day working on this stuff and go to bed earlier :)

i was under the impression that we handle setmodelobject differently in
that a call to that would only clone the object the model is pointing to and not the model itself. guess i was wrong there. i think this is something we
should definetely consider adding.

in its simplest form it can have two more callbacks modelObjectChanging()
and modelObjectChanged() that do for the model object what we do for the
model. because in fact it is the model object and not the model that change. in fact this is exactly what the ISort/FilterStateLocator accomplish, the
locator is not cloned so it can be safely shared, but the object it is
pointing to is cloned. this can also be more efficient as we will be cloning
less info for some models.

does this sound like a good idea? and then while we are at it we can
consider the IVersionable and add that in.

-Igor



On 12/7/05, Christian Essl < [EMAIL PROTECTED]> wrote:
>
> Right your locators do not have this problem. But why don't have a plain
>
> IFilterState{getFilterBean()} and if it implements IVersionable call
> before a change beforeUpdateSave(this)?
>
> For general IModels I think I am right. If I understand right (please
> correct and forgive ignorance) I think you have to indicate to wicket
> that
> the POJO has changed by calling Component.modelChanging(). This than
> calls
> Page.componentModelChanging()->
> UndoPageVersiongManager.componentModelChanging()->
> ChangeList.componentModelChanging()->new
> ModelChange() and this does clone the IModel in the constructor and
> resets
> the IModel on undo(). (It checks for CompundPropertyModels which is not
> nice as such in a very general method)
>
> How else shold Wicket know that my Pojo *is going to change* to record
> the
> current state. And in face of the different IModels going around I
> wouldn't know how to make this different.
>
> Let the user call the beforeChangeSave() method directly on an IModel
> which needs it and the Component should just not care (except making
> addStateChange() public and replacing modelChanging check IVersionalbe
> and
> call it for backwards-comp). It is not more work and a better separation > of concerns IMO. And than have a POJOModel which extends Model and does
> the cloning if you need versioning.
>
>
> Christian
>
> On Wed, 7 Dec 2005 00:43:31 -0800, Igor Vaynberg <
> [EMAIL PROTECTED]>
> wrote:
>
> > I think you miss the point. IModel itself is never serialized as part
> of
> > the
> > change, only the object it is pointing to is. So you are free to share
> an
> > imodel between two components and you will not end up with two
> underlying
> > copies when undoing changes.
> >
> > This is why ISortStateLocator also has a setter (just like imodel). It
> > itself is never serialized as part of the change, but the object it
> > points
> > to is. When the change is restored the setter on the locator is called
> to
> > restore the object.
> >
> > here is a snippet from the SortStateChange ( it is an internal class
> so
> > the
> > ref to ISortStateLocator stateLocator  comes form the parent class):
> > private final class SortStateChange extends Change
> >     {
> >         private final ISortState old = (ISortState)Objects.clone(
> > stateLocator.getSortState());
> >
> >         public void undo()
> >         {
> >             stateLocator.setSortState(old);
> >         }
> > }
> >
> > The locator is never serialized as part of the change, the change
> > retains a
> > reference to it. It is the underlying state object that is cloned, and
> > when
> > the change is undone it is reset on the locator.
> >
> > Eventually the ISortStateLocator is serialized when the page is
> > replicated,
> > but then it is only serialized once and all other references link to
> the
> > same serialized instance, so when it deserializes it does not get
> > duplicated
> > like you suggest.
> >
> > -Igor
> >
> >
> > On 12/6/05, Christian Essl <[EMAIL PROTECTED]> wrote:
> >>
> >> I think they are wicket-specific, because where outside of wicket
> would
> >> you need a wicket interface?
> >> And if you implement it yourself you will have to think of
> versioning,
> >> otherwise you will have to think of versioning everytime you just use
> >> it.
> >>
> >> For now I'd say have for POJO IFilterLocator a
> >> rememberVersion(IStateRecorder) method (but name it different) Than
> it
> >> is
> >> explicit what is going on and it is easier to use. (The same could be
>
> >> done
> >> for ISortState). (Again IFilterLocator should take a Serializable and
> >> maybe Ojbects.clone() could check if the cloneable implements
> Cloneable)
> >>
> >> Generally I think IModels which need versioning and the underlying
> >> object
> >> does not do it itself (which POJOs) should implement an interface and
> >> this
> >> should be as:
> >>
> >> IPOJOVersionable{
> >>   rememberVersion(IStateRecorder recorder);
> >> }
> >>
> >> The user than has to call this method on the Model.
> >>
> >> I think this is better because:
> >> a) it is more explicit. which ordinary user - like me knows - what is
>
> >> going on in modelChanging()
> >> b) currently only the model gets cloned which for pojos only has an
> >> effect
> >> if you use a model with a non transient reference
> >> c) you can not share the same underlying POJO instance between
> different
> >> components because after the roleback you will end with two instances
> >> (transparently)
> >> d) I often use more than one POJO ie in a Form so if I need
> versioning I
> >> want to version them both without the need of an extra component for
> >> registering the Pojo.
> >>
> >> Christian
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> On Tue, 6 Dec 2005 15:36:55 -0800, Igor Vaynberg
> >> <[EMAIL PROTECTED]>
> >> wrote:
> >>
> >> > I guess the question is do we want to infact make it something
> >> > wicket-specific. right now its just an ordinary pojo you can use
> >> anywhere
> >> > you like. but if we roll the versioning logic into it, it really
> does
> >> > become
> >> > locked to wicket. that is why i struggled to find a way to keep
> >> > versioning
> >> > out of it - it allows users to work with simple pojos.
> >> >
> >> > by keeping it an pojo there is another advantage - it is easier to
> >> expand
> >> > on. For example IFilterLocator works with ordinary objects that are
>
> >> used
> >> > as
> >> > a model for the filter form. everytime you write a filter model
> >> > (normally a
> >> > simple bean) do you want to deal with versioning everytime or would
>
> >> you
> >> > rather let the filter toolbar do it for you?
> >> >
> >> > by keeping the versioning out of models we shift the versioning
> into
> >> > toolbars. most users will use toolbars and not create them so i
> think
> >> its
> >> > better to keep stuff like that out and let the users who create
> >> toolbars
> >> > know that they should version the model they are provided if they
> want
> >> > the
> >> > backbutton to work - but that is not any different then creating
> any
> >> > other
> >> > custom component that modifies data - so this isnt even anything
> >> special
> >> > about toolbars.
> >> >
> >> > -Igor
> >> >
> >> >
> >> >
> >> > On 12/6/05, Christian Essl < [EMAIL PROTECTED]> wrote:
> >> >>
> >> >> On Tue, 6 Dec 2005 12:25:23 -0800, Igor Vaynberg
> >> >> < [EMAIL PROTECTED]>
> >> >> wrote:
> >> >>
> >> >> > So to summarize your idea it would be like this
> >> >> >
> >> >> > class SingleSortState
> >> >> >   IChangeRecorder recorder;
> >> >> >   SortState(IChangeRecorder) {...}
> >> >> >
> >> >> >   setPropertyState(...) {
> >> >> >       recorder.addChange( new Change() {...} )
> >> >> >    }
> >> >> >
> >> >> > OrderByLink { onclick () { getState().setProperty(...); } }
> >> >> >
> >> >> > this has the same net affect as
> >> >> >
> >> >> > OrderByLink { onclick() { addstatechange(new Change() { ...
> >> >> > clone(getState()); }); getState().setProperty(...); } }
> >> >> >
> >> >>
> >> >> Right.
> >> >>
> >> >> >
> >> >> > Your way shifts a lot more responsibility into the model because
> it
> >> >> > needs to
> >> >> > know how to version itself, and most models in wicket need to do
> >> that
> >> >> to
> >> >> > support the back button. That means you can no longer user your
> >> pojo
> >> >> > models
> >> >> > as wicket models. for example i can no longer reuse my Person
> pojo
> >> >> from
> >> >> > the
> >> >> > domain layer because it has no concept of versioning. am i
> totally
> >> off
> >> >> > here?
> >> >> > Thats the advantage of doing it on component side - your models
> can
> >> >> stay
> >> >> > dumb pojos and the component takes care of versioning them by
> using
> >> >> the
> >> >> > only
> >> >> > fool-proof way it can - cloning.
> >> >> >
> >> >> > -Igor
> >> >>
> >> >> You are right I definately went too far. ModelChanging triggers a
> >> >> versioning clone - did not know that. Propably I did not see it
> >> because
> >> >> I
> >> >> mainly use my Pojos from the DB and I do not version them. Thanks
> for
> >> >> the
> >> >> advice I now understand versioning a bit better.
> >> >>
> >> >> But back to the SortState: I think for such Wicket only models it
> is
> >> >> still
> >> >> more convinient to do it directly inside the model. In the end you
> >> will
> >> >> have to 'version' it somehow and while you are right that the only
> >> entry
> >> >> is through a component there can be many entry-points. And than it
>
> >> is a
> >> >> good candidate to refactor it in one place - why not the model?
> >> >>
> >> >> Christian
> >> >>
> >> >> --
> >> >> Christian Essl
> >> >>
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> ___________________________________________________________
> >> >> Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier
>
> >> >> anmelden: http://mail.yahoo.de
> >> >>
> >> >>
> >> >>
> >> >> -------------------------------------------------------
> >> >> This SF.net email is sponsored by: Splunk Inc. Do you grep through
> >> log
> >> >> files
> >> >> for problems?  Stop!  Download the new AJAX search engine that
> makes
> >> >> searching your log files as easy as surfing the  web.  DOWNLOAD
> >> SPLUNK!
> >> >> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> >> >> _______________________________________________
> >> >> Wicket-user mailing list
> >> >> Wicket-user@lists.sourceforge.net
> >> >> https://lists.sourceforge.net/lists/listinfo/wicket-user
> >> >>
> >>
> >>
> >>
> >> --
> >> Christian Essl
> >>
> >>
> >>
> >>
> >>
> >> ___________________________________________________________
> >> Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier
> >> anmelden: http://mail.yahoo.de
> >>
> >>
> >>
> >> -------------------------------------------------------
> >> This SF.net email is sponsored by: Splunk Inc. Do you grep through
> log
> >> files
> >> for problems? Stop! Download the new AJAX search engine that makes
> >> searching your log files as easy as surfing the  web.  DOWNLOAD
> SPLUNK!
> >> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> >> _______________________________________________
> >> Wicket-user mailing list
> >> Wicket-user@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/wicket-user
> >>
>
>
>
> --
> Christian Essl
>
>
>
>
>
> ___________________________________________________________
> Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier
> anmelden: http://mail.yahoo.de
>
>
>
> -------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> files
> for problems?  Stop!  Download the new AJAX search engine that makes
> searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
> http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> _______________________________________________
> Wicket-user mailing list
> Wicket-user@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/wicket-user
>





--
Christian Essl
        

        
                
___________________________________________________________ Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
Wicket-user mailing list
Wicket-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-user

Reply via email to