Christopher Turner wrote:
I'm not sure that I 100% follow this conversation and even why we need to copy pages but to me the whole concept of copying/cloning anything seems highly risky. Whenever I've come across this approach before it has ALWAYS been down to poor design elsewhere in the software that forces the copy/clone workaround. I think we should do everything we can to find the reason why we need to do this and then solve the underlying problem. This just feels bad to me.agree. i especially don't want to jam this feature into 1.0. i think we're only beginning to discover the issues at hand.
My primary concern is that the page copy also has the effect of copying the model objects and I think every model object is slightly different in terms of its copy semantics. Also, because the models are the main extension point where users will be providing their own implementation I think it puts a lot of burden on them to consider these copy type semantics.exaaaaaactly. and this was the reason that i was kindof testy over IM talking about this. it's a really scary and risky idea which we need to dig into a lot more if we're going to do it.
Obviously for simple models that just wrap a native type (such as a String) or which are our own models (such as PropertyModel) then there is no problem. However, as soon as you move into the realm of custom models then it just gets messy. For example, consider a 'shopping basket'. I may implement this as a detachable model and store the basket contents in the database. Fine, no problem. However I might also implement this as an in memory object that stays in the session (both are 100% valid design decisions and we need to be able to support both). In the first case copying the model is not a problem as the underlying object will always be reattached each time. However, in the second case it is an issue as I will be sharing the same business object (ShoppingBasket) across multiple pages. I'm happy for the Wicket model to be copied/cloned, but I don't want this to happen to my shopping basket object.yup.
the approach of just declaring that everything is always a transient/detachable hibernate object doesn't seem reasonable to me.
I'm also concerned that copying/cloning type approaches will make it more difficult to solve some of our clustering issues come 1.1. I'm not sure what the problems might be or how they may manifest themselves but general experience tells me that copy/clone decisions always come back to bite you at some time in the future.definitely!
i'd like to keep the discussion going on this for 1.1, but the problem is not at all straightforward or simple. and even though the change in the experimental tree is small in terms of lines-of-code, it has HUGE ramifications for the whole future of wicket.
jon
Regards, Chris
>
>
>
> >> i think 50ms on a 1GHz+ machine is actually pretty slow... you can
> >> seek and read on a disk in that time. also, serialization
> (which is
> >> really the only way to clone something easily in java) is
> notoriously
> >> thorny. but this approach may still be worthwhile (btw,
> thanks for
> >> finding those serialization problems! i think the good
> changes there
> >> ought to be checked into the main branch... has someone
> done this?).
> >
> > Not yet. But if it was me ALL the changes should go into the main
> > branch. how or what is used by default wicket settings is another
> > quiestion ofcourse.
>
> well, i think /some/ feature like this should go in, i'm just
> not sure
> if this is the exact implementation we want yet... still thinking...
>
> >
> >>
> >> are there model problems with this? if the user
> inncocently creates
> >> BookEditPage, when this copy happens, the new page will no
> longer be
> >> editing the same book model. we'd at least need to document this
> >> behavior. furthermore, if the copying doesn't /always/ happen (it
> >> depends on what components they use), things could feel a bit
> >> inconsistent to the user. sometimes it's the same page
> and sometimes
> >> its a copy. is there some simple thing we can tell people here?
> >
> >
> > Not true. It will edit the exact same book. Because those things
> > should be detachable anyway so In the end they are editing
> the exact
> > same thing.
> > And if you really have to do it by hand then i know exactly what
> > happens..
> > The developer will constuct an exact same page and components (very
> > cumbersum) and then also edit that same book (not by using the same
> > model but by using the same id in the model)
> > In the end the result is equal only how to get there is so
> much more
> > work.
>
> but what if it's not detachable? do you think should we stop
> supporting
> regular models and only have AbstractDetachableModel?
>
> >>
> >> in terms of hibernate, jdo and other frameworks, is it
> going to be a
> >> problem to clone models like this?
> >
> >
> > No why? Because those should use detachable models.
> > Please remeber!! We use serialization that would also be used but
> > clustering or by the webcontainer to dump the session on disk!!!
> > So that should work then this should work also! There is no
> difference.
>
> obviously. but again, what if the user has a regular old bean that's
> not detachable?
>
> >> also, detaching a model like the code below could be
> really expensive
> >> because reattaching might involve a database query, for example. > >> should we even detach the model? if we don't is hibernate
> going to
> >> be happy? i suspect the answer is that we don't need to do the
> >> detach, actually. isn't hibernate designed to deal with this?
> >
> >
> > We need to detach. There is no other way. Because only deattached
> > models are ready for serialization.
>
> actually, you /can/ serialize non-detachable models.
>
> > But again. If wicket doesn't provide this then i as developer has to
> > do this by hand myself. So anyplace where i call:
> Container.replace()
> > or Container.removeAll()
> > i would make a new page and make a complete new component structure
> > with models. (that are in default deattached states)
> > So yes the end result is the same.
> >
> > i don't find stale pages acceptable in anycase. Can never happen.
> > Should never happen. back buttons should always work work
> (2 deep not
> > much more)
> > I currently never call invalidateModel..(now called modelChanged)
> > Becaus that makes the page stale.. unacceptable.
>
> but stale pages /do/ happen. if you delete the only row from
> a list of
> fields and then hit back, the model is now an empty list. > what is that
> previous page supposed to do when you hit back, edit the field and
> submit your change? there's no entry in the model to edit. > the page is
> stale because the model changed.
>
> one way you could fix this would be by not having detachable models! > then each page has its own independent model. when you hit
> back, you'd
> go back to a page editing the model that was attached to that
> page. i'm
> not suggesting this is the right thing, but it would "solve"
> the problem.
>
> detachable models are essentially shared because they are
> backed by some
> store like a db. if you have detachable models, you have stale pages
> because the state is shared! when one page changes the
> state, the other
> pages have to go stale. this is what tapestry does and it's what we
> have in wicket now.
>
> like it or not, stale pages are a fact of life if you're in a
> multi-user
> environment or you have shared models like detachable models.
>
> > We should really strive for 100% back button prove in every
> possible
> > way.
>
> agree that back button should work as much as possible. but it can't
> always work because the structure of the old page may no longer match
> the structure of the model it's editing. at some point, you have to
> give up and show the user an error.
>
> > By the way i find modelChanged behaviour very strange by the way. In
> > my swing experience this will not 'destroy' the components
> > but let the components know that they should reaquire the model for
> > its data because the model's data is changed.
> > I would prefer to rename that method to what it really does:
> > setPageState(); (or something like that)
>
> what the component /does/ on modelChanged is an
> implementation detail of
> the component. the method is named based on is use not its
> implementation. i think it's aptly named.
>
> >> another issue is that detaching the model could cause it to change
> >> unexpectedly when its reattached. but that's a problem with our
> >> detachable model idea in general. wicket sessions are only
> >> guaranteed to cluster accurately if the attach/detach is somehow
> >> isolated from database changes.
> >
> >
> > how change unexcepectedly?
> > that shouldn't happen:
> > model.deatach()
> > model.atach()
> > model should be completely equal then right before detach().
>
> some other thread/session can modify the database. i'm just pointing
> out that detach() opens a window of opportunity for these
> errors to come
> up. i think it's a problem regardless though since we always
> detach()
> at the end of rendering so session sync can be lighter
> weight. so it's
> a problem, but not anything we can fix generically.
>
> >> i think a better name for savePageCopyOnStructureChange() might be
> >> wantPageCloned(). also, i don't understand why
> >> savePageCopyOnStructureChange() is in MarkupContainer. i
> think the
> >> implementation of this is going to have to be at the page level
> >> because there may actually be /more than one component/ on
> the page
> >> (for example, two tab components or some complex dynamic page or
> >> wizard) that want to trigger cloning at the same time. i'd think
> >> we'd need to ask every component on the page if it wants the page
> >> copied and then do it just once. so maybe it should be boolean
> >> Component.wantPageCloned() and the logic for
> onStructureChange would
> >> then migrate to Page, which makes some good sense doesn't it?
> >
> > It will only happen once in a request.. Never anymore.
> > So if somehow 2 times in a same request a tabpanel is reset or
> > removeALL is called then only the first time the 'clone' is
> happening.
>
> right. because of the render count reset, right?
>
> > Of course we could drop down the wantPageCloned to Page
> that is just
> > fine.
>
> but isn't it the component that wants the page cloned?
>
> > Also the implementation in onstructurechange could go to
> the page yes
> > (most calls are done on page any way....)
>
> ok. although i missed the part where the render count was reset to
> prevent future clonings. but it does feel like it belongs up there
> anyway....
>
> >> clearRendering() could use a comment since the reason for its
> >> existence is fairly involved. also, doesn't the container
> version of
> >> clearRendering need to recurse using visitChildren?
> >
> >
> > it is recurief it sets all component rendering to 0 and if
> a component
> > is a container then all its childs are also called ..
> >
> ah right. sorry, i read it wrong somehow.
>
> > finally, maybe we should consider keeping the current
> stale-detection
> > functionality via an ApplicationSetting and have the option
> default to
> > this (hopefully) idiot-proof transparent copying thing... that way
> > things work more often, but users who would rather not pay
> the cost of
> > serialization OR deal with pagefactory could get the current
> > functionality. i think this might be worth saving. we may
> get a lot
> > of criticism over the performance of cloning via serialization.
> >
> >> thoughts? replies?
> >>
> > one thing more.
> > I am searching for a better serialization for use in the
> same JVM. It
> > must be much faster if the ObjectOutputstream knows that it doesn't
> > have to store any class meta data ect.
> > that it only have to dump the instance data nothing more.
> > Anybody knows a bit optimized java serialization implementation?
>
> i'm not sure i understand. what's an example of class meta
> data that's
> being written out by serialization that you could omit? what
> would you
> write out in its place?
>
> i think serialization is pretty much going to be greviously slow. if
> there was a way to make it significantly faster, wouldn't
> someone have
> done that already?
>
> >
> > johan
> >
> >
> >
> > -------------------------------------------------------
> > SF email is sponsored by - The IT Product Guide
> > Read honest & candid reviews on hundreds of IT Products from real
> > users. Discover which products truly live up to the hype. Start
> > reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click <http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click>
> > _______________________________________________
> > Wicket-develop mailing list [email protected]
> > https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
>
>
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from
> real users. Discover which products truly live up to the
> hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396 <http://ads.osdn.com/?ad_id=6595&alloc_id=14396>> &op=click
>
> _______________________________________________
>
> Wicket-develop mailing list [email protected]
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
>
>
------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ Wicket-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/wicket-develop
