the reason feedback panel is harmless is that although its component structure changes, it contains no components that mutate the model. i'm getting the sense that there may actually be an automatic way of determining wantPageCloned(). a starting point for thinking about that is this: problem containers are containers (whose model modelChangedStructure() OR whose component shape changes) AND which contain model mutators whose muting components overlap either change in shape. in some wierd way, i get the sense that this is a geometrical intersection problem that may not be best solved in an ad hoc fashion.
i think we're onto something, but that we should wait and not try to jam this into 1.0 at the last second.
this is a very slippery problem and i suspect we will find an even better solution that we can't quite see yet because we haven't toyed around with it enough. i'd like to keep working on the experimental branch for a few weeks and just ship 1.0 RC1 as-is.
Eelco Hillenius wrote:
There's obviously something wrong with the SF mail server, as I get this reply before Jonathan's mail.
Ok, let's loose the factory then. You can count on having a request to support custom cloning, but let's defer that until we have more experience with this and until a request actually comes...
I explained in another email - which only just arrived - that it is a good idea to have the
wantPageCloned() in MarkupContainer instead of in Page. In one request, at most one copy of the page will be created, regardless of how many containers have their structure changed. And you want to be able to have a component *not* trigger the copying even though it's structure is changed. An example of this is FeedbackPanel, that has structural changes every request. But as FeedbackPanel has no components nested that can trigger server round-trips, it is harmless, and should thus not trigger a copy.
Eelco
Gili wrote:
Instead of having to explain yourself over again why this is a bad idea why not add a Wiki entry to this end? Frankly, I am oblivious as to why it would necessarily be good or bad.
Secondly, have the stable package as wicket.* and the experimental one as wicket.experimental, wicket.incubator, wicket.unstable or whatever and make it clear that anything under that package is deemed to be unstable so there are no guarantees in terms of API or overall stability.
I like the java.net's approach to this.
Gili
On Tue, 01 Mar 2005 13:42:21 -0800, Jonathan Locke wrote:
i wonder if it's even a good idea to have a page copying factory. is anyone really going to implement this? it would be a small nightmare not only in terms of the user but in terms of our support of the feature. i think we ought to consider the feature as a simple on/off kind of thing and wait for someone to go do all this on an experimental branch and see how it works out. i'm still really sceptical that it would be a usable way of doing things. another thing that worries me is that there's the possibility that exposing this factory extension point will cause a whole cascade of other problems when someone finally goes to try copying pages. and i DO NOT want to end up trying to support Java cloning in wicket. that would make me /really/ unhappy. in fact, i don't even want to have to explain over and over why it's a bad idea. suggesting to people that they can copy pages will only cause them to try to do it. i'm -1 on the factory, but cautiously +1 on the feature (i think it should be simple, optional and default to on) if we poke around more and all the questions we bring up seem to have good answers...
now that i've been thinking about this a bit... i'm wondering if it's a good idea to have wantsPageCloned() (savePageCopyOnStructureChange()). what possible reason could anyone have for turning this off in a subclass if the feature is turned on globally? can't we use the rendering count and various mutator methods in MarkupContainer to fully detect when structural changes happen and just make it all automatic?
also, shouldn't add-after-rendering trigger cloning?
there's still some work to do in structuring this experiment as a feature. but looks good so far!
one more thing... this cloning business gave me an RFE idea... we should have an option setClusterTesting(boolean) on ApplicationSettings. if it's true, wicket could use this cloning code to serialize and deserialize each page it renders just to be sure that works. that way you catch serialization problems earlier in the cycle. btw, i just added Objects.clone(Object) to wicket.util.lang. this should be globally useful and setClusterTesting() is a second application for the same code.
-------- Original Message --------
Subject: Re: [Wicket-develop] Copy on replace implemented...
Date: Tue, 01 Mar 2005 12:55:26 -0800
From: Jonathan Locke <[EMAIL PROTECTED]>
To: [email protected]
References: <[EMAIL PROTECTED]> <[EMAIL PROTECTED]>
very interesting!!
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?).
some issues to think about:
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?
in terms of hibernate, jdo and other frameworks, is it going to be a problem to clone models like this?
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?
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.
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?
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?
the fix to choicelist should definitely go in the main branch if it hasn't already. has someone done this?
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?
jon
Eelco Hillenius wrote:
The branch is named TRY_REPLACE_AND_COPY.
I refatored it a bit and introduced IPageCopyFactory, so that people can implement the copying strategy they want. Also, the copying is not done when the page is marked stale, and I added protected method MarkupContainer.savePageCopyOnStructureChange that can be overriden when you don't want copying to happen at all. For example this is the case with FeebackPanel that removes all components on each render, but as they are read only components (no inputs, no links etc) anyway, we don't want the copying happen here.
Please everyone, take a look at the branch and try it out. We (Johan, Maurice and I) think we finally have a complete solution for the backbutton problem (indeed for the application Maurice and I are working on, the problems are gone now).
Note that these problems were not only there for the replace method, which you can still argue about, but also about the removeAll method. Sure, you could mark a page stale when you did a removeAll, but I had a thousand times that our analists forgot not to use the back button as we instructed them :) and they ended up with a stale data page without understanding why.
Eelco
Johan Compagner wrote:
Hi,
in the branch we just checked in we implemented a first cut of what we think works great
and exactly what it should do when using replace (or later on also with add or remove).
In the MarkupContainer.replace() method we now do this first:
Page page = (Page)findParent(Page.class);
if(page != null && page.getRendering() > 0)
{
page.visitChildren(Component.class, new IVisitor()
{
public Object component(final Component component)
{
component.detachModel();
return CONTINUE_TRAVERSAL;
}
});
// first make a copy of the complete page..
try
{
ByteArrayOutputStream out = new ByteArrayOutputStream ();
ObjectOutputStream oout = new ObjectOutputStream (out);
oout.writeObject (page);
ObjectInputStream in = new ObjectInputStream ( new ByteArrayInputStream (out.toByteArray ()));
Page copy = (Page)in.readObject ();
getSession().addPage(page);
getSession().setPage(copy);
page.clearRendering();
}
catch (Exception e)
{
log.error("Renderer", e);
}
}
So we first test if this page already rendered, if this is the case then normally the replace couldn't happen.
But what we do then is create a copy.
That creating of a copy now happens with Serialization. But we could implement a Factory so that a developer can choose what ever he wants.
Like Copy Constructor, or even Clone...
Then when the copy is created that copy is being set as it was the current page for the cache (session.setPage)
and the current page itself is being added to the session and the rendering count is cleared so that it looks like a completely new page.
So if you selected another tab and then go back with the back button. the page can still be found with the exact same state (the one of the copy)
By testing this, we found some serialization errors in wicket!
Some like Page.session that is not transient (it should be and getSession() of page should test for null and if null then do Session.get()) are easy to fix.
But others and those map on the ImageResouce/IResouce (like UrlResource) are harder. Those are all not serializeable and
currently we are searching for the best way to make those serializeable . Any ideas?
But the how it works now is really great. It is completely transparent for the developer.
And can also be used for ListView.removeAll(). Then exactly the same should happen. right before the remove all make a copy of the current page
and use that copy for the cache so that a back button will map on the right listview..
If the usage of serialization is to slow or eats to much resources then a developer should be able to override this behavior and make use
of its own implementation by using for example a copy constructor.. This is ofcourse faster..But the developer has then much more work todo.
We already timed the serialization approache and it is only slow the very first time you serialize a page (1.5s) but all the following calls are 50-60ms
for very complex pages.
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
_______________________________________________
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
-------------------------------------------------------
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
-------------------------------------------------------
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
------------------------------------------------------- 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
------------------------------------------------------- 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
