Re: wicket 6.0 and automatic model detachment
I'm -0 to add it as enabled by default IDetachListener. There is no big demand for this. At least I haven't seen many mails/tickets explaining problems with detaching recently. Additionally since there is no exact contract for the number of calls of Component#detach() per request cycle I'd like to avoid any code that may make things slower. Currently Page#detach() is called at least twice per request in 1.5.x On Sun, Apr 8, 2012 at 7:30 PM, Igor Vaynberg igor.vaynb...@gmail.com wrote: s/where this is enabled/where this is disabled/ -igor On Sun, Apr 8, 2012 at 8:51 AM, Igor Vaynberg igor.vaynb...@gmail.com wrote: like i said, this only makes sense as a core features. because components implemented with this enabled will not work properly in an application where this is enabled. this is an all-or-nothing feature. -igor On Sun, Apr 8, 2012 at 3:15 AM, Johan Compagner jcompag...@gmail.com wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg igor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
Re: wicket 6.0 and automatic model detachment
A bit late, but I didn't have time to reply this weekend. I think a feature like this is realy nice, but only if it works in all cases, all the time. You must be able to rely on Wicket to detach your IDetachable no mather where it is in your component hierarchy, also when it's not directly referenceble from a component. This means you will have to use reflection to visit all fields and all fields of fields, etc, which would probably give quite some overhead. The reason why I'm against adding something that can only detach models in components is that I think people well get confused when to detach and when not to detach. For example, a single model will get detached, but a list of models will not. Detaching models already is a major problem for many people, adding helper magic will only make it worse. Adding this as a feature does not make any sense, because it destroy the possibility of using it in any component that might be reused later on in a different application. In short: -1 for adding something that can only detach detachables directly referenced from a component. Best regards, Emond On Friday 06 April 2012 09:42:42 Igor Vaynberg wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
On Fri, Apr 6, 2012 at 18:42, Igor Vaynberg igor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor I'm neither for nor against, but people don't seem convinced (yet?), if the decision is no, I think it would still be of great benefit to instead of detaching, log a warning and enable it in DEVELOPMENT mode only. I know at least 3 people that wrote something like this (myself included). Which also hints that model detaching in an actual problem and it might be a good idea to handle this more automatically.
Re: wicket 6.0 and automatic model detachment
i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg igor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
Once this is introduced you can't turn it off. You might use some components which depend on their models being detached externally, without you even being aware of it. Sven On 04/08/2012 12:15 PM, Johan Compagner wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynbergigor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meiers...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
The user would have to turn it off via settings. Sent from tablet device. Please excuse typos and brevity. On Apr 8, 2012 6:28 AM, Sven Meier s...@meiers.net wrote: Once this is introduced you can't turn it off. You might use some components which depend on their models being detached externally, without you even being aware of it. Sven On 04/08/2012 12:15 PM, Johan Compagner wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynbergigor.vaynberg@gmail.**comigor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meiers...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynberg@gmail.**comigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
Oh, sorry I didn't understand what you meant. Yeah that is a nasty scenario. I would much rather implement this as an aspect if it were me. Wicket should have a library of useful aspects like this. Sent from tablet device. Please excuse typos and brevity. On Apr 8, 2012 10:03 AM, James Carman jcar...@carmanconsulting.com wrote: The user would have to turn it off via settings. Sent from tablet device. Please excuse typos and brevity. On Apr 8, 2012 6:28 AM, Sven Meier s...@meiers.net wrote: Once this is introduced you can't turn it off. You might use some components which depend on their models being detached externally, without you even being aware of it. Sven On 04/08/2012 12:15 PM, Johan Compagner wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynbergigor.vaynberg@gmail.**comigor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meiers...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynberg@gmail.**comigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
like i said, this only makes sense as a core features. because components implemented with this enabled will not work properly in an application where this is enabled. this is an all-or-nothing feature. -igor On Sun, Apr 8, 2012 at 3:15 AM, Johan Compagner jcompag...@gmail.com wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg igor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
I think it should be an optional add-on (if included in the library at all), so that core/extensions components won't rely on it, and application developers may use it if they want the convenience. It is just a convenience, after all. If it is enabled by default, it may cause really weird, hard-to-trace bugs in applications. If so, it will be hard to upgrade from previous versions (it's much, much worse than a method rename, because the IDE won't warn you in any way). And, what will I do if I don't want some IModel attribute to be detached? I declare an array of one element, just to 'hide' it from the detacher? Besides, AFAIK, Wicket's use of reflection has been pretty limited, and in very predictable forms. If at least you had to annotate these attributes to enable it, it wouldn't be so bad, but this feels like is magic. This is the kind of thing that makes Seam and CDI good for demos and awful for real applications. Convenient, saves two lines of code, and you never know what is going on. On Sun, Apr 8, 2012 at 12:51 PM, Igor Vaynberg igor.vaynb...@gmail.com wrote: like i said, this only makes sense as a core features. because components implemented with this enabled will not work properly in an application where this is enabled. this is an all-or-nothing feature. -igor On Sun, Apr 8, 2012 at 3:15 AM, Johan Compagner jcompag...@gmail.com wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg igor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
s/where this is enabled/where this is disabled/ -igor On Sun, Apr 8, 2012 at 8:51 AM, Igor Vaynberg igor.vaynb...@gmail.com wrote: like i said, this only makes sense as a core features. because components implemented with this enabled will not work properly in an application where this is enabled. this is an all-or-nothing feature. -igor On Sun, Apr 8, 2012 at 3:15 AM, Johan Compagner jcompag...@gmail.com wrote: i think it would be fine to have something like this, and enabled by default but only to have an option to turn it off On Sat, Apr 7, 2012 at 22:30, Igor Vaynberg igor.vaynb...@gmail.com wrote: -1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
-1 on adding it if its not enabled by default. its a trivial class thats only about 40-50 lines of real code. adding it into extensions and not using it will just add to code rot because i doubt many people will go out looking for something like this since most of them wont even know that its possible to do this. -igor On Fri, Apr 6, 2012 at 11:28 PM, Sven Meier s...@meiers.net wrote: The listener won't be set in IFrameworkSettings by default, right? IMHO it's better located in extensions then. Sven On 04/07/2012 01:37 AM, James Carman wrote: Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynbergigor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
At first glance I'm torn about this, to be honest. On the one hand,
yes, not having to do that would be neat. On the other hand, that makes
models even more magic to new users, and not obvious what happens.
Especially if you create a model in e.g. your constructor and just
reference it a method of an anonymous class, that would not be detected
by this.
MyComponent() {
IModel foo = createSomeModel();
add(new Link() { onClick() {
// use foo model
}});
}
This model would not be detected by reflection, as far as I can tell.
So some models would be magically detached, others wouldn't. That might
lead to more confusion than just telling people Well just detach it!.
So far we've been getting along with a simple rule: Detach the model
yourself or pass it on to someone else. Easy to remember and works
reliably. This is what I've been teaching too.
I guess I'd have to see it in practice to make up my mind really. Is it
possible to offer it as an extension or something in wicketstuff, at
least for now?
Carl-Eric
www.wicketbuch.de
On Fri, 6 Apr 2012 09:42:42 -0700
Igor Vaynberg igor.vaynb...@gmail.com wrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
I wish there were a way to apply this strategy to the whole object graph (not just fields). I use local final LoadableDetachableModels on occasion, and it'd be nice for those to magically detach too. I'm +0. Just got Calc-Eric's message. Looks like we're on the same page. On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg igor.vaynb...@gmail.comwrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
Re: wicket 6.0 and automatic model detachment
Sorry to double-post, but I realized it's trivial to visit the object graph
since local final variables show up as fields in the inner class. This
following snippet prints detaching. So what do you think about
generalizing the automatic detach to all objects, not just components?
final IModel model = new IModel() {
@Override
public Object getObject() {
return null;
}
@Override
public void setObject(Object object) {
}
@Override
public void detach() {
System.out.println(detaching);
}
};
Object object = new Object() {
public void method() {
System.out.println(model.getObject());
}
};
for (Field f : object.getClass().getDeclaredFields()) {
System.out.println(f.getName());
if (IModel.class.isAssignableFrom(f.getType())) {
f.setAccessible(true);
((IModel)f.get(object)).detach();
}
}
On Fri, Apr 6, 2012 at 11:26 AM, Dan Retzlaff dretzl...@gmail.com wrote:
I wish there were a way to apply this strategy to the whole object graph
(not just fields). I use local final LoadableDetachableModels on occasion,
and it'd be nice for those to magically detach too. I'm +0.
Just got Calc-Eric's message. Looks like we're on the same page.
On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg igor.vaynb...@gmail.comwrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
actually all variables referenced in such manner become fields of the
anonymous class and are accessible via reflection.
-igor
On Fri, Apr 6, 2012 at 10:23 AM, Carl-Eric Menzel cmen...@wicketbuch.de wrote:
At first glance I'm torn about this, to be honest. On the one hand,
yes, not having to do that would be neat. On the other hand, that makes
models even more magic to new users, and not obvious what happens.
Especially if you create a model in e.g. your constructor and just
reference it a method of an anonymous class, that would not be detected
by this.
MyComponent() {
IModel foo = createSomeModel();
add(new Link() { onClick() {
// use foo model
}});
}
This model would not be detected by reflection, as far as I can tell.
So some models would be magically detached, others wouldn't. That might
lead to more confusion than just telling people Well just detach it!.
So far we've been getting along with a simple rule: Detach the model
yourself or pass it on to someone else. Easy to remember and works
reliably. This is what I've been teaching too.
I guess I'd have to see it in practice to make up my mind really. Is it
possible to offer it as an extension or something in wicketstuff, at
least for now?
Carl-Eric
www.wicketbuch.de
On Fri, 6 Apr 2012 09:42:42 -0700
Igor Vaynberg igor.vaynb...@gmail.com wrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
its easy to enable detaching of any IDetachable field rather then only
IModel. this code can also walk behaviors, but i dont think it should
go beyond that. walking the entire object graph will be too expensive.
right now it performs a lot of caching which makes it work well.
-igor
On Fri, Apr 6, 2012 at 10:48 AM, Dan Retzlaff dretzl...@gmail.com wrote:
Sorry to double-post, but I realized it's trivial to visit the object graph
since local final variables show up as fields in the inner class. This
following snippet prints detaching. So what do you think about
generalizing the automatic detach to all objects, not just components?
final IModel model = new IModel() {
@Override
public Object getObject() {
return null;
}
@Override
public void setObject(Object object) {
}
@Override
public void detach() {
System.out.println(detaching);
}
};
Object object = new Object() {
public void method() {
System.out.println(model.getObject());
}
};
for (Field f : object.getClass().getDeclaredFields()) {
System.out.println(f.getName());
if (IModel.class.isAssignableFrom(f.getType())) {
f.setAccessible(true);
((IModel)f.get(object)).detach();
}
}
On Fri, Apr 6, 2012 at 11:26 AM, Dan Retzlaff dretzl...@gmail.com wrote:
I wish there were a way to apply this strategy to the whole object graph
(not just fields). I use local final LoadableDetachableModels on occasion,
and it'd be nice for those to magically detach too. I'm +0.
Just got Calc-Eric's message. Looks like we're on the same page.
On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg
igor.vaynb...@gmail.comwrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
+1 Having IDetachable items detached by default makes sense
On Fri, Apr 6, 2012 at 10:56 AM, Igor Vaynberg igor.vaynb...@gmail.com wrote:
its easy to enable detaching of any IDetachable field rather then only
IModel. this code can also walk behaviors, but i dont think it should
go beyond that. walking the entire object graph will be too expensive.
right now it performs a lot of caching which makes it work well.
-igor
On Fri, Apr 6, 2012 at 10:48 AM, Dan Retzlaff dretzl...@gmail.com wrote:
Sorry to double-post, but I realized it's trivial to visit the object graph
since local final variables show up as fields in the inner class. This
following snippet prints detaching. So what do you think about
generalizing the automatic detach to all objects, not just components?
final IModel model = new IModel() {
@Override
public Object getObject() {
return null;
}
@Override
public void setObject(Object object) {
}
@Override
public void detach() {
System.out.println(detaching);
}
};
Object object = new Object() {
public void method() {
System.out.println(model.getObject());
}
};
for (Field f : object.getClass().getDeclaredFields()) {
System.out.println(f.getName());
if (IModel.class.isAssignableFrom(f.getType())) {
f.setAccessible(true);
((IModel)f.get(object)).detach();
}
}
On Fri, Apr 6, 2012 at 11:26 AM, Dan Retzlaff dretzl...@gmail.com wrote:
I wish there were a way to apply this strategy to the whole object graph
(not just fields). I use local final LoadableDetachableModels on occasion,
and it'd be nice for those to magically detach too. I'm +0.
Just got Calc-Eric's message. Looks like we're on the same page.
On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg
igor.vaynb...@gmail.comwrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
-1, what if I don't want this feature? this makes models even more complex
to understand. I would rather check, whether detachables aren't detached an
log it. but I don't like the idea of taking actions directly.
regards
martin
Am 06.04.2012 20:01 schrieb Ryan McKinley ryan...@gmail.com:
+1 Having IDetachable items detached by default makes sense
On Fri, Apr 6, 2012 at 10:56 AM, Igor Vaynberg igor.vaynb...@gmail.com
wrote:
its easy to enable detaching of any IDetachable field rather then only
IModel. this code can also walk behaviors, but i dont think it should
go beyond that. walking the entire object graph will be too expensive.
right now it performs a lot of caching which makes it work well.
-igor
On Fri, Apr 6, 2012 at 10:48 AM, Dan Retzlaff dretzl...@gmail.com
wrote:
Sorry to double-post, but I realized it's trivial to visit the object
graph
since local final variables show up as fields in the inner class. This
following snippet prints detaching. So what do you think about
generalizing the automatic detach to all objects, not just components?
final IModel model = new IModel() {
@Override
public Object getObject() {
return null;
}
@Override
public void setObject(Object object) {
}
@Override
public void detach() {
System.out.println(detaching);
}
};
Object object = new Object() {
public void method() {
System.out.println(model.getObject());
}
};
for (Field f : object.getClass().getDeclaredFields()) {
System.out.println(f.getName());
if (IModel.class.isAssignableFrom(f.getType())) {
f.setAccessible(true);
((IModel)f.get(object)).detach();
}
}
On Fri, Apr 6, 2012 at 11:26 AM, Dan Retzlaff dretzl...@gmail.com
wrote:
I wish there were a way to apply this strategy to the whole object
graph
(not just fields). I use local final LoadableDetachableModels on
occasion,
and it'd be nice for those to magically detach too. I'm +0.
Just got Calc-Eric's message. Looks like we're on the same page.
On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg
igor.vaynb...@gmail.comwrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
Yeah, IDetachable is better. Full graph traversal is only incrementally
worse that that required for serialization. It also happens after the
request.
A negative consideration: after making a change that requires components to
redetermine their models' cached states, it's nice to rely on direct calls
to Component#detach(). This proposal encourages a lazy style whereby
calling detach may not actually detach. Looking through core's and
extensions' detach() invocations, most appear unnecessary under this
proposal but I'm glad they're there for this reason.
On Fri, Apr 6, 2012 at 11:56 AM, Igor Vaynberg igor.vaynb...@gmail.comwrote:
its easy to enable detaching of any IDetachable field rather then only
IModel. this code can also walk behaviors, but i dont think it should
go beyond that. walking the entire object graph will be too expensive.
right now it performs a lot of caching which makes it work well.
-igor
On Fri, Apr 6, 2012 at 10:48 AM, Dan Retzlaff dretzl...@gmail.com wrote:
Sorry to double-post, but I realized it's trivial to visit the object
graph
since local final variables show up as fields in the inner class. This
following snippet prints detaching. So what do you think about
generalizing the automatic detach to all objects, not just components?
final IModel model = new IModel() {
@Override
public Object getObject() {
return null;
}
@Override
public void setObject(Object object) {
}
@Override
public void detach() {
System.out.println(detaching);
}
};
Object object = new Object() {
public void method() {
System.out.println(model.getObject());
}
};
for (Field f : object.getClass().getDeclaredFields()) {
System.out.println(f.getName());
if (IModel.class.isAssignableFrom(f.getType())) {
f.setAccessible(true);
((IModel)f.get(object)).detach();
}
}
On Fri, Apr 6, 2012 at 11:26 AM, Dan Retzlaff dretzl...@gmail.com
wrote:
I wish there were a way to apply this strategy to the whole object graph
(not just fields). I use local final LoadableDetachableModels on
occasion,
and it'd be nice for those to magically detach too. I'm +0.
Just got Calc-Eric's message. Looks like we're on the same page.
On Fri, Apr 6, 2012 at 10:42 AM, Igor Vaynberg igor.vaynb...@gmail.com
wrote:
i wrote a IDetachListener that automatically detaches any IModel
fields found on components. is this something we would be interested
in for core? its been running in production for a while without any
noticeable overhead and its nice not to have to implemenet onDetach()
all the time just to forward it to secondary models. the only downside
is that once we introduce this feature we can never remote it because
doing so will break code.
thoughts?
-igor
Re: wicket 6.0 and automatic model detachment
On Fri, 6 Apr 2012 10:48:35 -0700 Igor Vaynberg igor.vaynb...@gmail.com wrote: actually all variables referenced in such manner become fields of the anonymous class and are accessible via reflection. I wasn't sure about that so before I posted my reply I did an admittedly naive test. The variable I tried did at least not show up in Class#getDeclaredFields. I agree it makes sense that it should be there somewhere (that's how anon classes work) - but where does it show up? Carl-Eric
Re: wicket 6.0 and automatic model detachment
Add the listener to core and if folks want to use it they can. You could have a component instantiation listener add the detach listener to the components. Another option would be an aspect. On Apr 6, 2012 12:43 PM, Igor Vaynberg igor.vaynb...@gmail.com wrote: i wrote a IDetachListener that automatically detaches any IModel fields found on components. is this something we would be interested in for core? its been running in production for a while without any noticeable overhead and its nice not to have to implemenet onDetach() all the time just to forward it to secondary models. the only downside is that once we introduce this feature we can never remote it because doing so will break code. thoughts? -igor
