On 12/7/05, Christian Essl <[EMAIL PROTECTED]> wrote:
good, then we can at least drop this part of the discussion :)
<snip/>
Lets first agree that this is needed and we are going to implement it in wicket, and then we can discuss the gritty details like these :)
but the model will not be in the detached state. we use Object=model.getModelObject() so that attaches the model and then we clone the returned object if the model doesnt implement the IModelObjectVersionable. This is bad default behaviour because detachable models are usually pull models and so we should not version their model objects. currently this is not a problem because we clone the model and the object it links to is transient.
i guess this is really a non-issue because most of the time you do not call setmodelobject on a detachable model :) but it is def something to watch out for, imagine if the model is for a hibernate object and you start serializing it - serialize the whole object graph. we do provide a way to break out of this so maybe its not a huge deal.
Or you can implement your own Page{addPublicStateChange()} ;)
good, then we can at least drop this part of the discussion :)
>
> What may help is maybe having two interfaces IModelVersionable { Change
> getModelSnapshot() } and IModelObjectVersionable { Change
> getModelObjectSnapshot(); } to let the model control how it creates
> snapshots of objects and of itself.
>
<snip/>
I like this. You turn off versioning by returning null. Two minor remarks:
I think you should not check on the first IModel but on the deepest nested
or the first nested which is IModelObjectVersionable. Second maybe the
default (if not interface implemented) should be off, because of the
transient Detachables and my personal taste. It is a question of what is
needed more often and you have more experience there.
Lets first agree that this is needed and we are going to implement it in wicket, and then we can discuss the gritty details like these :)
But I'd say as it is it is good. I was heading in the same direction just
slower.
>
> however as you can see this would cause a paradigm shift of how
> detachable
> models work, it is no longer enough to simply declare the reference as
> transient, you need to implement IModelObjectVersionable and return
> null. so
> there are two things you need to do. maybe there is a more elegant
> solution
> that i do not see yet.
Don't realy understand this. I thought currently IDetachables should
onDetach set the reference to null at the end of the request and if they
want to be versioned they should (currently) keep the reference
non-transient otherwise it will be lost with cloning.
but the model will not be in the detached state. we use Object=model.getModelObject() so that attaches the model and then we clone the returned object if the model doesnt implement the IModelObjectVersionable. This is bad default behaviour because detachable models are usually pull models and so we should not version their model objects. currently this is not a problem because we clone the model and the object it links to is transient.
i guess this is really a non-issue because most of the time you do not call setmodelobject on a detachable model :) but it is def something to watch out for, imagine if the model is for a hibernate object and you start serializing it - serialize the whole object graph. we do provide a way to break out of this so maybe its not a huge deal.
> I also dont know about all these instanceof checks we would introduce. is
> that still a big performance concern?
>
Not sure, wheter this is only an old performance-myth. Would be intresting
how it compares to all the reflection with CompoundPropertyModel etc and
Objects.clone (). I think in this light they are fast.
Johan youve been doing a lot of profiling havent you? Last time i looked instanceof was a hotspot with 1% total runtime. So im not that worried, but maybe you have better stats.
-Igor