You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jeremy Thomerson <je...@wickettraining.com> on 2010/11/24 04:22:54 UTC
[vote] release Wicket 1.4.14
This vote is to release wicket 1.4.14. This is a bugfix release on the
1.4.x (stable) branch.
Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
Please test the release and offer your vote:
[ ] Yes, release
[ ] No, don't release it (and please tell us why)
--
Jeremy Thomerson
http://wickettraining.com
Need a CMS for Wicket? Use Brix! http://brixcms.org
Re: [vote] release Wicket 1.4.14
Posted by Jan Kriesten <kr...@mail.footprint.de>.
> [X] Yes, release
> [ ] No, don't release it (and please tell us why)
Best regards, --- Jan.
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
if it works for you keep it. all we did was give you a better/safer alternative.
-igor
On Mon, Nov 29, 2010 at 12:59 PM, James Carman
<ja...@carmanconsulting.com> wrote:
> I am glad we have something new that's better, but going from "do
> this" to "this is evil" is the troubling part. A lot of us have a lot
> of code that is based on the previous advice. Now declaring that code
> is "evil" is kind of scary, especially in the "middle" of a major
> version. If something is evil, then we should probably try to avoid
> it, so what do we do, go clean up all of our existing code (and
> re-test it)?
>
> On Mon, Nov 29, 2010 at 3:52 PM, Igor Vaynberg <ig...@gmail.com> wrote:
>> how so? we added something new that we think will work better.
>>
>> -igor
>>
>> On Mon, Nov 29, 2010 at 12:45 PM, James Carman
>> <ja...@carmanconsulting.com> wrote:
>>> On Mon, Nov 29, 2010 at 1:13 PM, Eelco Hillenius
>>> <ee...@gmail.com> wrote:
>>>> Niether is evil. It has potential pitfalls, which you should just be
>>>> aware of. We use such overrides all over the place and never have
>>>> problems with them either. :-) Avoiding it is safer, but also more
>>>> verbose (in 1.3.x at least).
>>>>
>>>
>>> Well, in the past, the "canned" answer was "override
>>> isEnabled/isVisible." Changing that paradigm and doing a complete 180
>>> is troubling.
>>>
>>
>
Re: overriding isVisible bad?
Posted by James Carman <ja...@carmanconsulting.com>.
I am glad we have something new that's better, but going from "do
this" to "this is evil" is the troubling part. A lot of us have a lot
of code that is based on the previous advice. Now declaring that code
is "evil" is kind of scary, especially in the "middle" of a major
version. If something is evil, then we should probably try to avoid
it, so what do we do, go clean up all of our existing code (and
re-test it)?
On Mon, Nov 29, 2010 at 3:52 PM, Igor Vaynberg <ig...@gmail.com> wrote:
> how so? we added something new that we think will work better.
>
> -igor
>
> On Mon, Nov 29, 2010 at 12:45 PM, James Carman
> <ja...@carmanconsulting.com> wrote:
>> On Mon, Nov 29, 2010 at 1:13 PM, Eelco Hillenius
>> <ee...@gmail.com> wrote:
>>> Niether is evil. It has potential pitfalls, which you should just be
>>> aware of. We use such overrides all over the place and never have
>>> problems with them either. :-) Avoiding it is safer, but also more
>>> verbose (in 1.3.x at least).
>>>
>>
>> Well, in the past, the "canned" answer was "override
>> isEnabled/isVisible." Changing that paradigm and doing a complete 180
>> is troubling.
>>
>
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
how so? we added something new that we think will work better.
-igor
On Mon, Nov 29, 2010 at 12:45 PM, James Carman
<ja...@carmanconsulting.com> wrote:
> On Mon, Nov 29, 2010 at 1:13 PM, Eelco Hillenius
> <ee...@gmail.com> wrote:
>> Niether is evil. It has potential pitfalls, which you should just be
>> aware of. We use such overrides all over the place and never have
>> problems with them either. :-) Avoiding it is safer, but also more
>> verbose (in 1.3.x at least).
>>
>
> Well, in the past, the "canned" answer was "override
> isEnabled/isVisible." Changing that paradigm and doing a complete 180
> is troubling.
>
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
> Well, in the past, the "canned" answer was "override
> isEnabled/isVisible." Changing that paradigm and doing a complete 180
> is troubling.
I don't think that's the case though. We've had many discussions on
this list (and in private even), and we've always felt uneasy about
supporting two rather than one way of achieving the same thing.
Ultimately we decided to support both, but there have been several
times where we were near making isVisible final.
Eelco
Re: overriding isVisible bad?
Posted by James Carman <ja...@carmanconsulting.com>.
On Mon, Nov 29, 2010 at 1:13 PM, Eelco Hillenius
<ee...@gmail.com> wrote:
> Niether is evil. It has potential pitfalls, which you should just be
> aware of. We use such overrides all over the place and never have
> problems with them either. :-) Avoiding it is safer, but also more
> verbose (in 1.3.x at least).
>
Well, in the past, the "canned" answer was "override
isEnabled/isVisible." Changing that paradigm and doing a complete 180
is troubling.
Re: overriding isVisible bad?
Posted by Emond Papegaaij <em...@topicus.nl>.
On of the main problems I see with onConfigure() is that is a general
'configuration' method. In our code, we ofter create an anonymous inner-class,
overriding the isVisible() method. These methods often look like 'return
super.isVisible() && someOtherCondition;'. This is a lot more difficult to
implement in a onConfigure() method, especially when that method also
configures a lot of other things. The only way to make that work, is by
starting with a call to super.onConfigure(), and including super.isVisible()
in the call to setVisible(). In this situation, I think overriding isVisible()
is much cleaner.
Emond
On Monday 29 November 2010 19:56:39 Igor Vaynberg wrote:
> ive run into plenty of weird problems with overrides, but maybe
> because this was in a high concurrency app where data changed
> frequently. the problems arise from the fact that the value returned
> from isvisible() can change while we are doing traversals, etc.
>
> eg we run a traversal for all visible components and put them in a
> list. later we iterate over the list and try to render these
> components. the render function also checks their visibility and if
> they are no longer visible it throws an exception.
>
> if isvisible() override depends on some external factor like the
> database there is a small window where the value can change and now
> you can have a weird exception: such as "tried to invoke a listener on
> a component that is not visible or not enabled". these are very
> intermittent and damn near impossible to reproduce.
>
> another problem is performance. isvisible() is called multiple times
> during the request and if it depends on the database it can be a
> performance problem. in fact a couple of users have complained about
> this on the list in the past. at least now we have an easy solution
> for them - use onconfigure().
>
> so as of right now the developers have two choices: override
> isvisible() and potentially suffer the consequences. or, override
> onconfigure() and set visibility there in a more deterministic
> fashion.
>
> -igor
>
>
>
> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>
> <ee...@gmail.com> wrote:
> > To expand, unless I'm missing something (new?), things are really only
> > problematic when both the mutable value and the override are mixed. In
> > a way, I think that using the override is 'more pure', as it's a
> > simple function that is executed when needed, whereas mutable state
> > can be harder to deal with when trying to figure out how it got to be
> > in that state. So, sorry Igor, but we disagree on this one.
> >
> > Eelco
> >
> >
> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >
> > <ee...@gmail.com> wrote:
> >> Niether is evil. It has potential pitfalls, which you should just be
> >> aware of. We use such overrides all over the place and never have
> >> problems with them either. :-) Avoiding it is safer, but also more
> >> verbose (in 1.3.x at least).
> >>
> >> Eelco
> >>
> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com>
wrote:
> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
> >>>> Hi Douglas,
> >>>>
> >>>> WICKET-3171 describes a problematic case, where visibility of a
> >>>> component changes while its form is being processed.
> >>>> In our projects we're overriding isVisible() where appropriate and
> >>>> never encountered a similar problem.
> >>>>
> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
> >>>> isEnabled() going to be declared evil too? ;)
> >>>
> >>> yes
> >>>
> >>> -igor
> >>>
> >>>> Sven
> >>>>
> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >>>>> Can you explain why? We have done this all over the place.
> >>>>>
> >>>>> D/
> >>>>>
> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >>>>> > The recommended way since a few 1.4 releases is to override
> >>>>> > onConfigure() and call setVisible(true|false) depending on your
> >>>>> > conditions.
> >>>>> >
> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >>>>> >
> >>>>> > douglas@douglasferguson.us> wrote:
> >>>>> >> Igor posted a comment to this bug saying that overriding
> >>>>> >> isVisible() is "evil"
> >>>>> >>
> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >>>>> >>
> >>>>> >> I was surprised by this and am curious to hear more.
> >>>>> >>
> >>>>> >> D/
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
afair this is already explained in the javadoc of onconfigure
label { onconfigure() { link.onconfigure(); setvisible(link.isvisible()); }}
-igor
On Mon, Dec 6, 2010 at 7:35 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> Igor,
>
> I agree that there are places that using onConfigure / setVisible may be
> better than overriding isVisible. However, I often have the following type
> of scenario, and wonder how you would suggest doing it without overriding
> isVisible:
>
> Situation: one component depends on the visibility of another (sibling)
> component for its visibility. if using onConfigure / setVisible, wouldn't
> there be an order-of-operations problem?
>
> Example:
> final Link next = new Link("next") {
> onConfigure() { setVisible(someCalculatedState); }
> }
>
> Label nextLabel = new Label("nextLabel") {
> onConfigure() { setVisible(next.isVisible()); }
> }
>
> I could do that before by overriding isVisible (rather than onConfigure) for
> both. Then, when nextLabel#isVisible called next#isVisible to calculate its
> state, it was guaranteed that next would return the proper info. But now,
> you could have the following situation:
>
> Render 1:
> both are visible
>
> Render 2:
> something has changed and now "someCalculatedState" will be false
> nextLabel is configured first and sets visible to true since next has not
> yet reconfigured based on the new state
> next is configured, and sets visibility to false based on the new state
>
> I suppose the best way to accomplish this with the onConfigure/setVisible
> method is to do onConfigure() { setVisible(calculateSomeState()); }.... the
> problem is that it makes it more difficult to create generic components this
> way (a label that takes another component as constructor arg and uses that
> component to determine its visibility).
>
> Thoughts?
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> *Need a CMS for Wicket? Use Brix! http://brixcms.org*
>
Re: overriding isVisible bad?
Posted by Jeremy Thomerson <je...@wickettraining.com>.
Igor,
I agree that there are places that using onConfigure / setVisible may be
better than overriding isVisible. However, I often have the following type
of scenario, and wonder how you would suggest doing it without overriding
isVisible:
Situation: one component depends on the visibility of another (sibling)
component for its visibility. if using onConfigure / setVisible, wouldn't
there be an order-of-operations problem?
Example:
final Link next = new Link("next") {
onConfigure() { setVisible(someCalculatedState); }
}
Label nextLabel = new Label("nextLabel") {
onConfigure() { setVisible(next.isVisible()); }
}
I could do that before by overriding isVisible (rather than onConfigure) for
both. Then, when nextLabel#isVisible called next#isVisible to calculate its
state, it was guaranteed that next would return the proper info. But now,
you could have the following situation:
Render 1:
both are visible
Render 2:
something has changed and now "someCalculatedState" will be false
nextLabel is configured first and sets visible to true since next has not
yet reconfigured based on the new state
next is configured, and sets visibility to false based on the new state
I suppose the best way to accomplish this with the onConfigure/setVisible
method is to do onConfigure() { setVisible(calculateSomeState()); }.... the
problem is that it makes it more difficult to create generic components this
way (a label that takes another component as constructor arg and uses that
component to determine its visibility).
Thoughts?
--
Jeremy Thomerson
http://wickettraining.com
*Need a CMS for Wicket? Use Brix! http://brixcms.org*
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
> huh? that doesnt make any sense. the callbacks like onconfigure simply
> give you checkpoints for calculating and caching visibility rather
> then calculating every time.
I wasn't arguing against onConfigure (which is a fine trade-off) but
saw an example of where relying on just setVisible would be
problematic.
Now the problem is that we have three ways of achieving the same
thing. And yet another overridable method. It's probably too late in
the game, but I would be more in favor of a delegation mechanism (e.g.
interfaces for the functions of isVisible, isEnabled and whatever else
makes sense) and a way to set those functions. So you'd do things more
through composition. Might even consider making working with such
functions a generic capability that also supports detaching (which
then can also be used to let the component manage detachment of
multiple models if there's more than one), etc. Would be a significant
break though, so I'm not expecting a lot of cheering :-)
Eelco
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
huh? that doesnt make any sense. the callbacks like onconfigure simply
give you checkpoints for calculating and caching visibility rather
then calculating every time.
-igor
On Fri, Dec 3, 2010 at 5:12 PM, Eelco Hillenius
<ee...@gmail.com> wrote:
> That's actually also an example of why I prefer overriding isVisible:
> I can have that button and other widgets (maybe completely unrelated)
> widgets that depend on a particular state (like whether a record
> exists), and a function (override of isVisible) will always yield the
> correct result. In contrast, relying on the internal component state
> (set/isVisible) puts the responsibility of updating the relevant
> components on the handler (or worse, it may have to be triggered from
> the business layer).
>
> Eelco
>
> On Fri, Dec 3, 2010 at 10:46 AM, Igor Vaynberg <ig...@gmail.com> wrote:
>> ldm also doesnt always work nicely. suppose you have a delete button
>> that is only visible if the record exists. the page renders, the user
>> clicks the button. the onclick handler invoked, and isvisible is
>> checked - at which point it is true. the record is deleted in the
>> onclick handler, the page is rerendered - delete button is still
>> visible because the ldm has cached the visibility value from before
>> onclick handler is invoked. to make it work correctly the ldm would
>> have to be detached manually after onclick handler.
>>
>> -igor
>>
>> On Thu, Dec 2, 2010 at 6:36 PM, Clint Checketts <ch...@gmail.com> wrote:
>>> Yesterday a friend following this thread pointed out that we should rethink
>>> our overriding of onVisible and use onConfigure. I've used
>>> LoadabledDetachableModels to cache the value used in my isVisible/isEnabled
>>> overriding so changing values mid request aren't a problem. That is its
>>> whole purpose. Also calling .detach() on that model isn't hacky, that is its
>>> design.
>>>
>>> While I appreciate having onConfigure as an option it seems like overriding
>>> isVisible is still the cleaner and clearer way. Folks just need to follow
>>> the rule that expensive calls should be contained in an LDM.
>>>
>>> Am I stuck in the past in holding this view?
>>>
>>> -Clint
>>>
>>> On Thu, Dec 2, 2010 at 4:03 AM, Martin Makundi <
>>> martin.makundi@koodaripalvelut.com> wrote:
>>>
>>>> What about using onconfigure to replace loadabledetachablemodel ? We
>>>> have had some trouble with loadabledetachablemodels when their state
>>>> is frozen before a dependent model has been initialized (for example)
>>>> and we need to call model.detach() from within our code, which seems
>>>> bit hacky.
>>>>
>>>> Initializing also models at a specific known requestcycle moment might
>>>> be beneficial. Ofcourse it is not so straightforward as with
>>>> enable/visible state.
>>>>
>>>> **
>>>> Martin
>>>>
>>>> 2010/12/1 Igor Vaynberg <ig...@gmail.com>:
>>>> > i would be happy if that was good enough. in the past it hasnt been,
>>>> > thats why we have the current solution. maybe we can try it again in
>>>> > 1.5 and see what happens.
>>>> >
>>>> > -igor
>>>> >
>>>> > On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com>
>>>> wrote:
>>>> >> I have a different point of view, the HTTP imposes us some limitations,
>>>> we
>>>> >> will hardly have an good synchronization between the component state on
>>>> >> browser and server using only HTTP conversation. So it is mandatory the
>>>> >> service layer to respect the described security restriction.
>>>> >>
>>>> >> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>>>> >wrote:
>>>> >>
>>>> >>> an easy example is security.
>>>> >>>
>>>> >>> a user views a page that allows them to delete another user
>>>> >>> meanwhile their permissions are tweaked and they can no longer delete
>>>> >>> other users
>>>> >>> half an hour later the user clicks the delete button - this should
>>>> >>> fail, but wont if we are using last-rendered state.
>>>> >>>
>>>> >>> -igor
>>>> >>>
>>>> >>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
>>>> >>> wrote:
>>>> >>> > I need to look better on which core components are relying on an
>>>> updated
>>>> >>> > visibility/enabled state at the process event time, and why the last
>>>> >>> > rendered state wouldn't be enough to them to work nicely.
>>>> >>> >
>>>> >>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <
>>>> igor.vaynberg@gmail.com
>>>> >>> >wrote:
>>>> >>> >
>>>> >>> >> currently we only invoke configure before the render. this would
>>>> mean
>>>> >>> >> we would have to invoke it before processing a listener, clearing
>>>> the
>>>> >>> >> cache, and then invoking it again before render. i wonder if that is
>>>> >>> >> enough places to invoke it....
>>>> >>> >>
>>>> >>> >> -igor
>>>> >>> >>
>>>> >>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
>>>> >>> wrote:
>>>> >>> >> > If user click an link, it will change the value of some property
>>>> at
>>>> >>> the
>>>> >>> >> > process_event request cycle step. Then the processor will go to
>>>> the
>>>> >>> >> respond
>>>> >>> >> > step, will invoke every component before render method which will
>>>> end
>>>> >>> up
>>>> >>> >> > invoking the Component#configure and updating the
>>>> visibility/enabled
>>>> >>> >> state
>>>> >>> >> > (even if it changes, we are able to work with the updated state).
>>>> So
>>>> >>> when
>>>> >>> >> > the this component has the opportunity to render it self, it will
>>>> be
>>>> >>> >> aware
>>>> >>> >> > its update state.
>>>> >>> >> >
>>>> >>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
>>>> >>> igor.vaynberg@gmail.com
>>>> >>> >> >wrote:
>>>> >>> >> >
>>>> >>> >> >> there are other places that should be checked though. for example
>>>> >>> >> >> before we invoke a listener on the component we should check
>>>> again to
>>>> >>> >> >> make sure that visibility hasnt changed. eg if visibility depends
>>>> on
>>>> >>> >> >> some property of the user clicking the link that changed between
>>>> >>> >> >> render and clicking the link.
>>>> >>> >> >>
>>>> >>> >> >> -igor
>>>> >>> >> >>
>>>> >>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <
>>>> pedrosans@gmail.com>
>>>> >>> >> wrote:
>>>> >>> >> >> > An implementation idea:
>>>> >>> >> >> >
>>>> >>> >> >> > Component {
>>>> >>> >> >> > public final void configure()
>>>> >>> >> >> > {
>>>> >>> >> >> > if (!getFlag(FLAG_CONFIGURED))
>>>> >>> >> >> > {
>>>> >>> >> >> > setVisible_NoClientCode(isVisible()); //we only
>>>> check
>>>> >>> the
>>>> >>> >> user
>>>> >>> >> >> > isVisible in here
>>>> >>> >> >> > onConfigure();
>>>> >>> >> >> > setFlag(FLAG_CONFIGURED, true);
>>>> >>> >> >> > }
>>>> >>> >> >> > }
>>>> >>> >> >> > }
>>>> >>> >> >> >
>>>> >>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>>>> >>> >> igor.vaynberg@gmail.com
>>>> >>> >> >> >wrote:
>>>> >>> >> >> >
>>>> >>> >> >> >> so how is it different if they can still override something
>>>> that
>>>> >>> >> needs
>>>> >>> >> >> >> to be checked all the time?
>>>> >>> >> >> >>
>>>> >>> >> >> >> -igor
>>>> >>> >> >> >>
>>>> >>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
>>>> >>> pedrosans@gmail.com>
>>>> >>> >> >> wrote:
>>>> >>> >> >> >> > I understand the concern about possible isVisible
>>>> >>> implementations
>>>> >>> >> like
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>>>> >>> >> component
>>>> >>> >> >> >> being
>>>> >>> >> >> >> > rendered at 09:59:59
>>>> >>> >> >> >> > isVisible(return dao.list().size() > 0);// performance
>>>> issues
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > But maybe we can have the best from both approaches. This is
>>>> an
>>>> >>> >> >> >> copy/paste
>>>> >>> >> >> >> > from java.awt.Component:
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > public boolean isVisible() {
>>>> >>> >> >> >> > return isVisible_NoClientCode();
>>>> >>> >> >> >> > }
>>>> >>> >> >> >> > final boolean isVisible_NoClientCode() {
>>>> >>> >> >> >> > return visible;
>>>> >>> >> >> >> > }
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > There are some points in the awt framework were the
>>>> isVisible
>>>> >>> >> method
>>>> >>> >> >> is
>>>> >>> >> >> >> not
>>>> >>> >> >> >> > used in benefit of isVisible_NoClientCode
>>>> >>> >> >> >> > I'm in favor of create an final isVisible/Enabled version
>>>> and
>>>> >>> >> change
>>>> >>> >> >> the
>>>> >>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
>>>> >>> provide
>>>> >>> >> >> their
>>>> >>> >> >> >> > isVisible/Enable implementations that will serve to feed the
>>>> >>> core
>>>> >>> >> >> >> component
>>>> >>> >> >> >> > state.
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>>>> >>> >> >> igor.vaynberg@gmail.com
>>>> >>> >> >> >> >wrote:
>>>> >>> >> >> >> >
>>>> >>> >> >> >> >> ive run into plenty of weird problems with overrides, but
>>>> maybe
>>>> >>> >> >> >> >> because this was in a high concurrency app where data
>>>> changed
>>>> >>> >> >> >> >> frequently. the problems arise from the fact that the value
>>>> >>> >> returned
>>>> >>> >> >> >> >> from isvisible() can change while we are doing traversals,
>>>> etc.
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> eg we run a traversal for all visible components and put
>>>> them
>>>> >>> in a
>>>> >>> >> >> >> >> list. later we iterate over the list and try to render
>>>> these
>>>> >>> >> >> >> >> components. the render function also checks their
>>>> visibility
>>>> >>> and
>>>> >>> >> if
>>>> >>> >> >> >> >> they are no longer visible it throws an exception.
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> if isvisible() override depends on some external factor
>>>> like
>>>> >>> the
>>>> >>> >> >> >> >> database there is a small window where the value can change
>>>> and
>>>> >>> >> now
>>>> >>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
>>>> >>> >> listener
>>>> >>> >> >> on
>>>> >>> >> >> >> >> a component that is not visible or not enabled". these are
>>>> very
>>>> >>> >> >> >> >> intermittent and damn near impossible to reproduce.
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> another problem is performance. isvisible() is called
>>>> multiple
>>>> >>> >> times
>>>> >>> >> >> >> >> during the request and if it depends on the database it can
>>>> be
>>>> >>> a
>>>> >>> >> >> >> >> performance problem. in fact a couple of users have
>>>> complained
>>>> >>> >> about
>>>> >>> >> >> >> >> this on the list in the past. at least now we have an easy
>>>> >>> >> solution
>>>> >>> >> >> >> >> for them - use onconfigure().
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> so as of right now the developers have two choices:
>>>> override
>>>> >>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
>>>> >>> override
>>>> >>> >> >> >> >> onconfigure() and set visibility there in a more
>>>> deterministic
>>>> >>> >> >> >> >> fashion.
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> -igor
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>>>> >>> >> >> >> >> <ee...@gmail.com> wrote:
>>>> >>> >> >> >> >> > To expand, unless I'm missing something (new?), things
>>>> are
>>>> >>> >> really
>>>> >>> >> >> only
>>>> >>> >> >> >> >> > problematic when both the mutable value and the override
>>>> are
>>>> >>> >> mixed.
>>>> >>> >> >> In
>>>> >>> >> >> >> >> > a way, I think that using the override is 'more pure', as
>>>> >>> it's a
>>>> >>> >> >> >> >> > simple function that is executed when needed, whereas
>>>> mutable
>>>> >>> >> state
>>>> >>> >> >> >> >> > can be harder to deal with when trying to figure out how
>>>> it
>>>> >>> got
>>>> >>> >> to
>>>> >>> >> >> be
>>>> >>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this
>>>> one.
>>>> >>> >> >> >> >> >
>>>> >>> >> >> >> >> > Eelco
>>>> >>> >> >> >> >> >
>>>> >>> >> >> >> >> >
>>>> >>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>>>> >>> >> >> >> >> > <ee...@gmail.com> wrote:
>>>> >>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you
>>>> should
>>>> >>> >> just
>>>> >>> >> >> be
>>>> >>> >> >> >> >> >> aware of. We use such overrides all over the place and
>>>> never
>>>> >>> >> have
>>>> >>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
>>>> >>> also
>>>> >>> >> more
>>>> >>> >> >> >> >> >> verbose (in 1.3.x at least).
>>>> >>> >> >> >> >> >>
>>>> >>> >> >> >> >> >> Eelco
>>>> >>> >> >> >> >> >>
>>>> >>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>>>> >>> >> >> >> igor.vaynberg@gmail.com>
>>>> >>> >> >> >> >> wrote:
>>>> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>>>> >>> sven@meiers.net>
>>>> >>> >> >> >> wrote:
>>>> >>> >> >> >> >> >>>> Hi Douglas,
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where
>>>> visibility
>>>> >>> of
>>>> >>> >> a
>>>> >>> >> >> >> >> >>>> component changes while its form is being processed.
>>>> >>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
>>>> >>> >> appropriate
>>>> >>> >> >> and
>>>> >>> >> >> >> >> never
>>>> >>> >> >> >> >> >>>> encountered a similar problem.
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's
>>>> next,
>>>> >>> is
>>>> >>> >> >> >> overriding
>>>> >>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>>>> >>> >> >> >> >> >>>
>>>> >>> >> >> >> >> >>> yes
>>>> >>> >> >> >> >> >>>
>>>> >>> >> >> >> >> >>> -igor
>>>> >>> >> >> >> >> >>>
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>> Sven
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson
>>>> wrote:
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
>>>> >>> place.
>>>> >>> >> >> >> >> >>>>>
>>>> >>> >> >> >> >> >>>>> D/
>>>> >>> >> >> >> >> >>>>>
>>>> >>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>>> >>> >> >> >> >> >>>>>
>>>> >>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>>>> >>> >> override
>>>> >>> >> >> >> >> onConfigure()
>>>> >>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>>>> >>> >> conditions.
>>>> >>> >> >> >> >> >>>>> >
>>>> >>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>>> >>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>>>> >>> >> >> >> >> >>>>> >
>>>> >>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
>>>> >>> overriding
>>>> >>> >> >> >> >> isVisible() is
>>>> >>> >> >> >> >> >>>>> >> "evil"
>>>> >>> >> >> >> >> >>>>> >>
>>>> >>> >> >> >> >> >>>>> >>
>>>> >>> https://issues.apache.org/jira/browse/WICKET-3171
>>>> >>> >> >> >> >> >>>>> >>
>>>> >>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear
>>>> more.
>>>> >>> >> >> >> >> >>>>> >>
>>>> >>> >> >> >> >> >>>>> >> D/
>>>> >>> >> >> >> >> >>>>>
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>>
>>>> >>> >> >> >> >> >>>
>>>> >>> >> >> >> >> >>
>>>> >>> >> >> >> >> >
>>>> >>> >> >> >> >>
>>>> >>> >> >> >> >
>>>> >>> >> >> >> >
>>>> >>> >> >> >> >
>>>> >>> >> >> >> > --
>>>> >>> >> >> >> > Pedro Henrique Oliveira dos Santos
>>>> >>> >> >> >> >
>>>> >>> >> >> >>
>>>> >>> >> >> >
>>>> >>> >> >> >
>>>> >>> >> >> >
>>>> >>> >> >> > --
>>>> >>> >> >> > Pedro Henrique Oliveira dos Santos
>>>> >>> >> >> >
>>>> >>> >> >>
>>>> >>> >> >
>>>> >>> >> >
>>>> >>> >> >
>>>> >>> >> > --
>>>> >>> >> > Pedro Henrique Oliveira dos Santos
>>>> >>> >> >
>>>> >>> >>
>>>> >>> >
>>>> >>> >
>>>> >>> >
>>>> >>> > --
>>>> >>> > Pedro Henrique Oliveira dos Santos
>>>> >>> >
>>>> >>>
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> Pedro Henrique Oliveira dos Santos
>>>> >>
>>>> >
>>>>
>>>
>>
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
Component can continue having the public isVisible/Enabled method always
returning its correct state avaliable to use. Just the core request code
will use an safe version that can be: isVisible/EnabledOnRequest. I really
value an request cycle protected against volatile component states.
I like the idea of using composition. We can refactor all the Component
state code to an StateManager object. Its main goal can be:
- maintain the components state: all its flags, the data object and
respective API
- define how close the "visible/enabledOnRequest" state will be to the user
logic: we can define the check points, by default will be only the
onConfigure. Perhaps user will can even override the SateManager, coding
specific component logic (can solve some security restrictions).
On Fri, Dec 3, 2010 at 11:12 PM, Eelco Hillenius
<ee...@gmail.com>wrote:
> That's actually also an example of why I prefer overriding isVisible:
> I can have that button and other widgets (maybe completely unrelated)
> widgets that depend on a particular state (like whether a record
> exists), and a function (override of isVisible) will always yield the
> correct result. In contrast, relying on the internal component state
> (set/isVisible) puts the responsibility of updating the relevant
> components on the handler (or worse, it may have to be triggered from
> the business layer).
>
> Eelco
>
> On Fri, Dec 3, 2010 at 10:46 AM, Igor Vaynberg <ig...@gmail.com>
> wrote:
> > ldm also doesnt always work nicely. suppose you have a delete button
> > that is only visible if the record exists. the page renders, the user
> > clicks the button. the onclick handler invoked, and isvisible is
> > checked - at which point it is true. the record is deleted in the
> > onclick handler, the page is rerendered - delete button is still
> > visible because the ldm has cached the visibility value from before
> > onclick handler is invoked. to make it work correctly the ldm would
> > have to be detached manually after onclick handler.
> >
> > -igor
> >
> > On Thu, Dec 2, 2010 at 6:36 PM, Clint Checketts <ch...@gmail.com>
> wrote:
> >> Yesterday a friend following this thread pointed out that we should
> rethink
> >> our overriding of onVisible and use onConfigure. I've used
> >> LoadabledDetachableModels to cache the value used in my
> isVisible/isEnabled
> >> overriding so changing values mid request aren't a problem. That is its
> >> whole purpose. Also calling .detach() on that model isn't hacky, that is
> its
> >> design.
> >>
> >> While I appreciate having onConfigure as an option it seems like
> overriding
> >> isVisible is still the cleaner and clearer way. Folks just need to
> follow
> >> the rule that expensive calls should be contained in an LDM.
> >>
> >> Am I stuck in the past in holding this view?
> >>
> >> -Clint
> >>
> >> On Thu, Dec 2, 2010 at 4:03 AM, Martin Makundi <
> >> martin.makundi@koodaripalvelut.com> wrote:
> >>
> >>> What about using onconfigure to replace loadabledetachablemodel ? We
> >>> have had some trouble with loadabledetachablemodels when their state
> >>> is frozen before a dependent model has been initialized (for example)
> >>> and we need to call model.detach() from within our code, which seems
> >>> bit hacky.
> >>>
> >>> Initializing also models at a specific known requestcycle moment might
> >>> be beneficial. Ofcourse it is not so straightforward as with
> >>> enable/visible state.
> >>>
> >>> **
> >>> Martin
> >>>
> >>> 2010/12/1 Igor Vaynberg <ig...@gmail.com>:
> >>> > i would be happy if that was good enough. in the past it hasnt been,
> >>> > thats why we have the current solution. maybe we can try it again in
> >>> > 1.5 and see what happens.
> >>> >
> >>> > -igor
> >>> >
> >>> > On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com>
> >>> wrote:
> >>> >> I have a different point of view, the HTTP imposes us some
> limitations,
> >>> we
> >>> >> will hardly have an good synchronization between the component state
> on
> >>> >> browser and server using only HTTP conversation. So it is mandatory
> the
> >>> >> service layer to respect the described security restriction.
> >>> >>
> >>> >> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> >>> >wrote:
> >>> >>
> >>> >>> an easy example is security.
> >>> >>>
> >>> >>> a user views a page that allows them to delete another user
> >>> >>> meanwhile their permissions are tweaked and they can no longer
> delete
> >>> >>> other users
> >>> >>> half an hour later the user clicks the delete button - this should
> >>> >>> fail, but wont if we are using last-rendered state.
> >>> >>>
> >>> >>> -igor
> >>> >>>
> >>> >>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <
> pedrosans@gmail.com>
> >>> >>> wrote:
> >>> >>> > I need to look better on which core components are relying on an
> >>> updated
> >>> >>> > visibility/enabled state at the process event time, and why the
> last
> >>> >>> > rendered state wouldn't be enough to them to work nicely.
> >>> >>> >
> >>> >>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <
> >>> igor.vaynberg@gmail.com
> >>> >>> >wrote:
> >>> >>> >
> >>> >>> >> currently we only invoke configure before the render. this would
> >>> mean
> >>> >>> >> we would have to invoke it before processing a listener,
> clearing
> >>> the
> >>> >>> >> cache, and then invoking it again before render. i wonder if
> that is
> >>> >>> >> enough places to invoke it....
> >>> >>> >>
> >>> >>> >> -igor
> >>> >>> >>
> >>> >>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <
> pedrosans@gmail.com>
> >>> >>> wrote:
> >>> >>> >> > If user click an link, it will change the value of some
> property
> >>> at
> >>> >>> the
> >>> >>> >> > process_event request cycle step. Then the processor will go
> to
> >>> the
> >>> >>> >> respond
> >>> >>> >> > step, will invoke every component before render method which
> will
> >>> end
> >>> >>> up
> >>> >>> >> > invoking the Component#configure and updating the
> >>> visibility/enabled
> >>> >>> >> state
> >>> >>> >> > (even if it changes, we are able to work with the updated
> state).
> >>> So
> >>> >>> when
> >>> >>> >> > the this component has the opportunity to render it self, it
> will
> >>> be
> >>> >>> >> aware
> >>> >>> >> > its update state.
> >>> >>> >> >
> >>> >>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
> >>> >>> igor.vaynberg@gmail.com
> >>> >>> >> >wrote:
> >>> >>> >> >
> >>> >>> >> >> there are other places that should be checked though. for
> example
> >>> >>> >> >> before we invoke a listener on the component we should check
> >>> again to
> >>> >>> >> >> make sure that visibility hasnt changed. eg if visibility
> depends
> >>> on
> >>> >>> >> >> some property of the user clicking the link that changed
> between
> >>> >>> >> >> render and clicking the link.
> >>> >>> >> >>
> >>> >>> >> >> -igor
> >>> >>> >> >>
> >>> >>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <
> >>> pedrosans@gmail.com>
> >>> >>> >> wrote:
> >>> >>> >> >> > An implementation idea:
> >>> >>> >> >> >
> >>> >>> >> >> > Component {
> >>> >>> >> >> > public final void configure()
> >>> >>> >> >> > {
> >>> >>> >> >> > if (!getFlag(FLAG_CONFIGURED))
> >>> >>> >> >> > {
> >>> >>> >> >> > setVisible_NoClientCode(isVisible()); //we only
> >>> check
> >>> >>> the
> >>> >>> >> user
> >>> >>> >> >> > isVisible in here
> >>> >>> >> >> > onConfigure();
> >>> >>> >> >> > setFlag(FLAG_CONFIGURED, true);
> >>> >>> >> >> > }
> >>> >>> >> >> > }
> >>> >>> >> >> > }
> >>> >>> >> >> >
> >>> >>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
> >>> >>> >> igor.vaynberg@gmail.com
> >>> >>> >> >> >wrote:
> >>> >>> >> >> >
> >>> >>> >> >> >> so how is it different if they can still override
> something
> >>> that
> >>> >>> >> needs
> >>> >>> >> >> >> to be checked all the time?
> >>> >>> >> >> >>
> >>> >>> >> >> >> -igor
> >>> >>> >> >> >>
> >>> >>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
> >>> >>> pedrosans@gmail.com>
> >>> >>> >> >> wrote:
> >>> >>> >> >> >> > I understand the concern about possible isVisible
> >>> >>> implementations
> >>> >>> >> like
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine
> this
> >>> >>> >> component
> >>> >>> >> >> >> being
> >>> >>> >> >> >> > rendered at 09:59:59
> >>> >>> >> >> >> > isVisible(return dao.list().size() > 0);// performance
> >>> issues
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > But maybe we can have the best from both approaches.
> This is
> >>> an
> >>> >>> >> >> >> copy/paste
> >>> >>> >> >> >> > from java.awt.Component:
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > public boolean isVisible() {
> >>> >>> >> >> >> > return isVisible_NoClientCode();
> >>> >>> >> >> >> > }
> >>> >>> >> >> >> > final boolean isVisible_NoClientCode() {
> >>> >>> >> >> >> > return visible;
> >>> >>> >> >> >> > }
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > There are some points in the awt framework were the
> >>> isVisible
> >>> >>> >> method
> >>> >>> >> >> is
> >>> >>> >> >> >> not
> >>> >>> >> >> >> > used in benefit of isVisible_NoClientCode
> >>> >>> >> >> >> > I'm in favor of create an final isVisible/Enabled
> version
> >>> and
> >>> >>> >> change
> >>> >>> >> >> the
> >>> >>> >> >> >> > Wicket core to use it. Also maintain the hotspot to
> users
> >>> >>> provide
> >>> >>> >> >> their
> >>> >>> >> >> >> > isVisible/Enable implementations that will serve to feed
> the
> >>> >>> core
> >>> >>> >> >> >> component
> >>> >>> >> >> >> > state.
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> >>> >>> >> >> igor.vaynberg@gmail.com
> >>> >>> >> >> >> >wrote:
> >>> >>> >> >> >> >
> >>> >>> >> >> >> >> ive run into plenty of weird problems with overrides,
> but
> >>> maybe
> >>> >>> >> >> >> >> because this was in a high concurrency app where data
> >>> changed
> >>> >>> >> >> >> >> frequently. the problems arise from the fact that the
> value
> >>> >>> >> returned
> >>> >>> >> >> >> >> from isvisible() can change while we are doing
> traversals,
> >>> etc.
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> eg we run a traversal for all visible components and
> put
> >>> them
> >>> >>> in a
> >>> >>> >> >> >> >> list. later we iterate over the list and try to render
> >>> these
> >>> >>> >> >> >> >> components. the render function also checks their
> >>> visibility
> >>> >>> and
> >>> >>> >> if
> >>> >>> >> >> >> >> they are no longer visible it throws an exception.
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> if isvisible() override depends on some external factor
> >>> like
> >>> >>> the
> >>> >>> >> >> >> >> database there is a small window where the value can
> change
> >>> and
> >>> >>> >> now
> >>> >>> >> >> >> >> you can have a weird exception: such as "tried to
> invoke a
> >>> >>> >> listener
> >>> >>> >> >> on
> >>> >>> >> >> >> >> a component that is not visible or not enabled". these
> are
> >>> very
> >>> >>> >> >> >> >> intermittent and damn near impossible to reproduce.
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> another problem is performance. isvisible() is called
> >>> multiple
> >>> >>> >> times
> >>> >>> >> >> >> >> during the request and if it depends on the database it
> can
> >>> be
> >>> >>> a
> >>> >>> >> >> >> >> performance problem. in fact a couple of users have
> >>> complained
> >>> >>> >> about
> >>> >>> >> >> >> >> this on the list in the past. at least now we have an
> easy
> >>> >>> >> solution
> >>> >>> >> >> >> >> for them - use onconfigure().
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> so as of right now the developers have two choices:
> >>> override
> >>> >>> >> >> >> >> isvisible() and potentially suffer the consequences.
> or,
> >>> >>> override
> >>> >>> >> >> >> >> onconfigure() and set visibility there in a more
> >>> deterministic
> >>> >>> >> >> >> >> fashion.
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> -igor
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >>> >>> >> >> >> >> <ee...@gmail.com> wrote:
> >>> >>> >> >> >> >> > To expand, unless I'm missing something (new?),
> things
> >>> are
> >>> >>> >> really
> >>> >>> >> >> only
> >>> >>> >> >> >> >> > problematic when both the mutable value and the
> override
> >>> are
> >>> >>> >> mixed.
> >>> >>> >> >> In
> >>> >>> >> >> >> >> > a way, I think that using the override is 'more
> pure', as
> >>> >>> it's a
> >>> >>> >> >> >> >> > simple function that is executed when needed, whereas
> >>> mutable
> >>> >>> >> state
> >>> >>> >> >> >> >> > can be harder to deal with when trying to figure out
> how
> >>> it
> >>> >>> got
> >>> >>> >> to
> >>> >>> >> >> be
> >>> >>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on
> this
> >>> one.
> >>> >>> >> >> >> >> >
> >>> >>> >> >> >> >> > Eelco
> >>> >>> >> >> >> >> >
> >>> >>> >> >> >> >> >
> >>> >>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >>> >>> >> >> >> >> > <ee...@gmail.com> wrote:
> >>> >>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which
> you
> >>> should
> >>> >>> >> just
> >>> >>> >> >> be
> >>> >>> >> >> >> >> >> aware of. We use such overrides all over the place
> and
> >>> never
> >>> >>> >> have
> >>> >>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer,
> but
> >>> >>> also
> >>> >>> >> more
> >>> >>> >> >> >> >> >> verbose (in 1.3.x at least).
> >>> >>> >> >> >> >> >>
> >>> >>> >> >> >> >> >> Eelco
> >>> >>> >> >> >> >> >>
> >>> >>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >>> >>> >> >> >> igor.vaynberg@gmail.com>
> >>> >>> >> >> >> >> wrote:
> >>> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
> >>> >>> sven@meiers.net>
> >>> >>> >> >> >> wrote:
> >>> >>> >> >> >> >> >>>> Hi Douglas,
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where
> >>> visibility
> >>> >>> of
> >>> >>> >> a
> >>> >>> >> >> >> >> >>>> component changes while its form is being
> processed.
> >>> >>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
> >>> >>> >> appropriate
> >>> >>> >> >> and
> >>> >>> >> >> >> >> never
> >>> >>> >> >> >> >> >>>> encountered a similar problem.
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's
> >>> next,
> >>> >>> is
> >>> >>> >> >> >> overriding
> >>> >>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >>> >>> >> >> >> >> >>>
> >>> >>> >> >> >> >> >>> yes
> >>> >>> >> >> >> >> >>>
> >>> >>> >> >> >> >> >>> -igor
> >>> >>> >> >> >> >> >>>
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>> Sven
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas
> Ferguson
> >>> wrote:
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>>> Can you explain why? We have done this all over
> the
> >>> >>> place.
> >>> >>> >> >> >> >> >>>>>
> >>> >>> >> >> >> >> >>>>> D/
> >>> >>> >> >> >> >> >>>>>
> >>> >>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov
> wrote:
> >>> >>> >> >> >> >> >>>>>
> >>> >>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is
> to
> >>> >>> >> override
> >>> >>> >> >> >> >> onConfigure()
> >>> >>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on
> your
> >>> >>> >> conditions.
> >>> >>> >> >> >> >> >>>>> >
> >>> >>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas
> Ferguson <
> >>> >>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
> >>> >>> >> >> >> >> >>>>> >
> >>> >>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
> >>> >>> overriding
> >>> >>> >> >> >> >> isVisible() is
> >>> >>> >> >> >> >> >>>>> >> "evil"
> >>> >>> >> >> >> >> >>>>> >>
> >>> >>> >> >> >> >> >>>>> >>
> >>> >>> https://issues.apache.org/jira/browse/WICKET-3171
> >>> >>> >> >> >> >> >>>>> >>
> >>> >>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear
> >>> more.
> >>> >>> >> >> >> >> >>>>> >>
> >>> >>> >> >> >> >> >>>>> >> D/
> >>> >>> >> >> >> >> >>>>>
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>>
> >>> >>> >> >> >> >> >>>
> >>> >>> >> >> >> >> >>
> >>> >>> >> >> >> >> >
> >>> >>> >> >> >> >>
> >>> >>> >> >> >> >
> >>> >>> >> >> >> >
> >>> >>> >> >> >> >
> >>> >>> >> >> >> > --
> >>> >>> >> >> >> > Pedro Henrique Oliveira dos Santos
> >>> >>> >> >> >> >
> >>> >>> >> >> >>
> >>> >>> >> >> >
> >>> >>> >> >> >
> >>> >>> >> >> >
> >>> >>> >> >> > --
> >>> >>> >> >> > Pedro Henrique Oliveira dos Santos
> >>> >>> >> >> >
> >>> >>> >> >>
> >>> >>> >> >
> >>> >>> >> >
> >>> >>> >> >
> >>> >>> >> > --
> >>> >>> >> > Pedro Henrique Oliveira dos Santos
> >>> >>> >> >
> >>> >>> >>
> >>> >>> >
> >>> >>> >
> >>> >>> >
> >>> >>> > --
> >>> >>> > Pedro Henrique Oliveira dos Santos
> >>> >>> >
> >>> >>>
> >>> >>
> >>> >>
> >>> >>
> >>> >> --
> >>> >> Pedro Henrique Oliveira dos Santos
> >>> >>
> >>> >
> >>>
> >>
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
That's actually also an example of why I prefer overriding isVisible:
I can have that button and other widgets (maybe completely unrelated)
widgets that depend on a particular state (like whether a record
exists), and a function (override of isVisible) will always yield the
correct result. In contrast, relying on the internal component state
(set/isVisible) puts the responsibility of updating the relevant
components on the handler (or worse, it may have to be triggered from
the business layer).
Eelco
On Fri, Dec 3, 2010 at 10:46 AM, Igor Vaynberg <ig...@gmail.com> wrote:
> ldm also doesnt always work nicely. suppose you have a delete button
> that is only visible if the record exists. the page renders, the user
> clicks the button. the onclick handler invoked, and isvisible is
> checked - at which point it is true. the record is deleted in the
> onclick handler, the page is rerendered - delete button is still
> visible because the ldm has cached the visibility value from before
> onclick handler is invoked. to make it work correctly the ldm would
> have to be detached manually after onclick handler.
>
> -igor
>
> On Thu, Dec 2, 2010 at 6:36 PM, Clint Checketts <ch...@gmail.com> wrote:
>> Yesterday a friend following this thread pointed out that we should rethink
>> our overriding of onVisible and use onConfigure. I've used
>> LoadabledDetachableModels to cache the value used in my isVisible/isEnabled
>> overriding so changing values mid request aren't a problem. That is its
>> whole purpose. Also calling .detach() on that model isn't hacky, that is its
>> design.
>>
>> While I appreciate having onConfigure as an option it seems like overriding
>> isVisible is still the cleaner and clearer way. Folks just need to follow
>> the rule that expensive calls should be contained in an LDM.
>>
>> Am I stuck in the past in holding this view?
>>
>> -Clint
>>
>> On Thu, Dec 2, 2010 at 4:03 AM, Martin Makundi <
>> martin.makundi@koodaripalvelut.com> wrote:
>>
>>> What about using onconfigure to replace loadabledetachablemodel ? We
>>> have had some trouble with loadabledetachablemodels when their state
>>> is frozen before a dependent model has been initialized (for example)
>>> and we need to call model.detach() from within our code, which seems
>>> bit hacky.
>>>
>>> Initializing also models at a specific known requestcycle moment might
>>> be beneficial. Ofcourse it is not so straightforward as with
>>> enable/visible state.
>>>
>>> **
>>> Martin
>>>
>>> 2010/12/1 Igor Vaynberg <ig...@gmail.com>:
>>> > i would be happy if that was good enough. in the past it hasnt been,
>>> > thats why we have the current solution. maybe we can try it again in
>>> > 1.5 and see what happens.
>>> >
>>> > -igor
>>> >
>>> > On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com>
>>> wrote:
>>> >> I have a different point of view, the HTTP imposes us some limitations,
>>> we
>>> >> will hardly have an good synchronization between the component state on
>>> >> browser and server using only HTTP conversation. So it is mandatory the
>>> >> service layer to respect the described security restriction.
>>> >>
>>> >> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>>> >wrote:
>>> >>
>>> >>> an easy example is security.
>>> >>>
>>> >>> a user views a page that allows them to delete another user
>>> >>> meanwhile their permissions are tweaked and they can no longer delete
>>> >>> other users
>>> >>> half an hour later the user clicks the delete button - this should
>>> >>> fail, but wont if we are using last-rendered state.
>>> >>>
>>> >>> -igor
>>> >>>
>>> >>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
>>> >>> wrote:
>>> >>> > I need to look better on which core components are relying on an
>>> updated
>>> >>> > visibility/enabled state at the process event time, and why the last
>>> >>> > rendered state wouldn't be enough to them to work nicely.
>>> >>> >
>>> >>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <
>>> igor.vaynberg@gmail.com
>>> >>> >wrote:
>>> >>> >
>>> >>> >> currently we only invoke configure before the render. this would
>>> mean
>>> >>> >> we would have to invoke it before processing a listener, clearing
>>> the
>>> >>> >> cache, and then invoking it again before render. i wonder if that is
>>> >>> >> enough places to invoke it....
>>> >>> >>
>>> >>> >> -igor
>>> >>> >>
>>> >>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
>>> >>> wrote:
>>> >>> >> > If user click an link, it will change the value of some property
>>> at
>>> >>> the
>>> >>> >> > process_event request cycle step. Then the processor will go to
>>> the
>>> >>> >> respond
>>> >>> >> > step, will invoke every component before render method which will
>>> end
>>> >>> up
>>> >>> >> > invoking the Component#configure and updating the
>>> visibility/enabled
>>> >>> >> state
>>> >>> >> > (even if it changes, we are able to work with the updated state).
>>> So
>>> >>> when
>>> >>> >> > the this component has the opportunity to render it self, it will
>>> be
>>> >>> >> aware
>>> >>> >> > its update state.
>>> >>> >> >
>>> >>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
>>> >>> igor.vaynberg@gmail.com
>>> >>> >> >wrote:
>>> >>> >> >
>>> >>> >> >> there are other places that should be checked though. for example
>>> >>> >> >> before we invoke a listener on the component we should check
>>> again to
>>> >>> >> >> make sure that visibility hasnt changed. eg if visibility depends
>>> on
>>> >>> >> >> some property of the user clicking the link that changed between
>>> >>> >> >> render and clicking the link.
>>> >>> >> >>
>>> >>> >> >> -igor
>>> >>> >> >>
>>> >>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <
>>> pedrosans@gmail.com>
>>> >>> >> wrote:
>>> >>> >> >> > An implementation idea:
>>> >>> >> >> >
>>> >>> >> >> > Component {
>>> >>> >> >> > public final void configure()
>>> >>> >> >> > {
>>> >>> >> >> > if (!getFlag(FLAG_CONFIGURED))
>>> >>> >> >> > {
>>> >>> >> >> > setVisible_NoClientCode(isVisible()); //we only
>>> check
>>> >>> the
>>> >>> >> user
>>> >>> >> >> > isVisible in here
>>> >>> >> >> > onConfigure();
>>> >>> >> >> > setFlag(FLAG_CONFIGURED, true);
>>> >>> >> >> > }
>>> >>> >> >> > }
>>> >>> >> >> > }
>>> >>> >> >> >
>>> >>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>>> >>> >> igor.vaynberg@gmail.com
>>> >>> >> >> >wrote:
>>> >>> >> >> >
>>> >>> >> >> >> so how is it different if they can still override something
>>> that
>>> >>> >> needs
>>> >>> >> >> >> to be checked all the time?
>>> >>> >> >> >>
>>> >>> >> >> >> -igor
>>> >>> >> >> >>
>>> >>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
>>> >>> pedrosans@gmail.com>
>>> >>> >> >> wrote:
>>> >>> >> >> >> > I understand the concern about possible isVisible
>>> >>> implementations
>>> >>> >> like
>>> >>> >> >> >> >
>>> >>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>>> >>> >> component
>>> >>> >> >> >> being
>>> >>> >> >> >> > rendered at 09:59:59
>>> >>> >> >> >> > isVisible(return dao.list().size() > 0);// performance
>>> issues
>>> >>> >> >> >> >
>>> >>> >> >> >> > But maybe we can have the best from both approaches. This is
>>> an
>>> >>> >> >> >> copy/paste
>>> >>> >> >> >> > from java.awt.Component:
>>> >>> >> >> >> >
>>> >>> >> >> >> > public boolean isVisible() {
>>> >>> >> >> >> > return isVisible_NoClientCode();
>>> >>> >> >> >> > }
>>> >>> >> >> >> > final boolean isVisible_NoClientCode() {
>>> >>> >> >> >> > return visible;
>>> >>> >> >> >> > }
>>> >>> >> >> >> >
>>> >>> >> >> >> > There are some points in the awt framework were the
>>> isVisible
>>> >>> >> method
>>> >>> >> >> is
>>> >>> >> >> >> not
>>> >>> >> >> >> > used in benefit of isVisible_NoClientCode
>>> >>> >> >> >> > I'm in favor of create an final isVisible/Enabled version
>>> and
>>> >>> >> change
>>> >>> >> >> the
>>> >>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
>>> >>> provide
>>> >>> >> >> their
>>> >>> >> >> >> > isVisible/Enable implementations that will serve to feed the
>>> >>> core
>>> >>> >> >> >> component
>>> >>> >> >> >> > state.
>>> >>> >> >> >> >
>>> >>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>>> >>> >> >> igor.vaynberg@gmail.com
>>> >>> >> >> >> >wrote:
>>> >>> >> >> >> >
>>> >>> >> >> >> >> ive run into plenty of weird problems with overrides, but
>>> maybe
>>> >>> >> >> >> >> because this was in a high concurrency app where data
>>> changed
>>> >>> >> >> >> >> frequently. the problems arise from the fact that the value
>>> >>> >> returned
>>> >>> >> >> >> >> from isvisible() can change while we are doing traversals,
>>> etc.
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> eg we run a traversal for all visible components and put
>>> them
>>> >>> in a
>>> >>> >> >> >> >> list. later we iterate over the list and try to render
>>> these
>>> >>> >> >> >> >> components. the render function also checks their
>>> visibility
>>> >>> and
>>> >>> >> if
>>> >>> >> >> >> >> they are no longer visible it throws an exception.
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> if isvisible() override depends on some external factor
>>> like
>>> >>> the
>>> >>> >> >> >> >> database there is a small window where the value can change
>>> and
>>> >>> >> now
>>> >>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
>>> >>> >> listener
>>> >>> >> >> on
>>> >>> >> >> >> >> a component that is not visible or not enabled". these are
>>> very
>>> >>> >> >> >> >> intermittent and damn near impossible to reproduce.
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> another problem is performance. isvisible() is called
>>> multiple
>>> >>> >> times
>>> >>> >> >> >> >> during the request and if it depends on the database it can
>>> be
>>> >>> a
>>> >>> >> >> >> >> performance problem. in fact a couple of users have
>>> complained
>>> >>> >> about
>>> >>> >> >> >> >> this on the list in the past. at least now we have an easy
>>> >>> >> solution
>>> >>> >> >> >> >> for them - use onconfigure().
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> so as of right now the developers have two choices:
>>> override
>>> >>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
>>> >>> override
>>> >>> >> >> >> >> onconfigure() and set visibility there in a more
>>> deterministic
>>> >>> >> >> >> >> fashion.
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> -igor
>>> >>> >> >> >> >>
>>> >>> >> >> >> >>
>>> >>> >> >> >> >>
>>> >>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>>> >>> >> >> >> >> <ee...@gmail.com> wrote:
>>> >>> >> >> >> >> > To expand, unless I'm missing something (new?), things
>>> are
>>> >>> >> really
>>> >>> >> >> only
>>> >>> >> >> >> >> > problematic when both the mutable value and the override
>>> are
>>> >>> >> mixed.
>>> >>> >> >> In
>>> >>> >> >> >> >> > a way, I think that using the override is 'more pure', as
>>> >>> it's a
>>> >>> >> >> >> >> > simple function that is executed when needed, whereas
>>> mutable
>>> >>> >> state
>>> >>> >> >> >> >> > can be harder to deal with when trying to figure out how
>>> it
>>> >>> got
>>> >>> >> to
>>> >>> >> >> be
>>> >>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this
>>> one.
>>> >>> >> >> >> >> >
>>> >>> >> >> >> >> > Eelco
>>> >>> >> >> >> >> >
>>> >>> >> >> >> >> >
>>> >>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>>> >>> >> >> >> >> > <ee...@gmail.com> wrote:
>>> >>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you
>>> should
>>> >>> >> just
>>> >>> >> >> be
>>> >>> >> >> >> >> >> aware of. We use such overrides all over the place and
>>> never
>>> >>> >> have
>>> >>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
>>> >>> also
>>> >>> >> more
>>> >>> >> >> >> >> >> verbose (in 1.3.x at least).
>>> >>> >> >> >> >> >>
>>> >>> >> >> >> >> >> Eelco
>>> >>> >> >> >> >> >>
>>> >>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>>> >>> >> >> >> igor.vaynberg@gmail.com>
>>> >>> >> >> >> >> wrote:
>>> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>>> >>> sven@meiers.net>
>>> >>> >> >> >> wrote:
>>> >>> >> >> >> >> >>>> Hi Douglas,
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where
>>> visibility
>>> >>> of
>>> >>> >> a
>>> >>> >> >> >> >> >>>> component changes while its form is being processed.
>>> >>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
>>> >>> >> appropriate
>>> >>> >> >> and
>>> >>> >> >> >> >> never
>>> >>> >> >> >> >> >>>> encountered a similar problem.
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's
>>> next,
>>> >>> is
>>> >>> >> >> >> overriding
>>> >>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>>> >>> >> >> >> >> >>>
>>> >>> >> >> >> >> >>> yes
>>> >>> >> >> >> >> >>>
>>> >>> >> >> >> >> >>> -igor
>>> >>> >> >> >> >> >>>
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>> Sven
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson
>>> wrote:
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
>>> >>> place.
>>> >>> >> >> >> >> >>>>>
>>> >>> >> >> >> >> >>>>> D/
>>> >>> >> >> >> >> >>>>>
>>> >>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>> >>> >> >> >> >> >>>>>
>>> >>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>>> >>> >> override
>>> >>> >> >> >> >> onConfigure()
>>> >>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>>> >>> >> conditions.
>>> >>> >> >> >> >> >>>>> >
>>> >>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>> >>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>>> >>> >> >> >> >> >>>>> >
>>> >>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
>>> >>> overriding
>>> >>> >> >> >> >> isVisible() is
>>> >>> >> >> >> >> >>>>> >> "evil"
>>> >>> >> >> >> >> >>>>> >>
>>> >>> >> >> >> >> >>>>> >>
>>> >>> https://issues.apache.org/jira/browse/WICKET-3171
>>> >>> >> >> >> >> >>>>> >>
>>> >>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear
>>> more.
>>> >>> >> >> >> >> >>>>> >>
>>> >>> >> >> >> >> >>>>> >> D/
>>> >>> >> >> >> >> >>>>>
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>>
>>> >>> >> >> >> >> >>>
>>> >>> >> >> >> >> >>
>>> >>> >> >> >> >> >
>>> >>> >> >> >> >>
>>> >>> >> >> >> >
>>> >>> >> >> >> >
>>> >>> >> >> >> >
>>> >>> >> >> >> > --
>>> >>> >> >> >> > Pedro Henrique Oliveira dos Santos
>>> >>> >> >> >> >
>>> >>> >> >> >>
>>> >>> >> >> >
>>> >>> >> >> >
>>> >>> >> >> >
>>> >>> >> >> > --
>>> >>> >> >> > Pedro Henrique Oliveira dos Santos
>>> >>> >> >> >
>>> >>> >> >>
>>> >>> >> >
>>> >>> >> >
>>> >>> >> >
>>> >>> >> > --
>>> >>> >> > Pedro Henrique Oliveira dos Santos
>>> >>> >> >
>>> >>> >>
>>> >>> >
>>> >>> >
>>> >>> >
>>> >>> > --
>>> >>> > Pedro Henrique Oliveira dos Santos
>>> >>> >
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Pedro Henrique Oliveira dos Santos
>>> >>
>>> >
>>>
>>
>
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
ldm also doesnt always work nicely. suppose you have a delete button
that is only visible if the record exists. the page renders, the user
clicks the button. the onclick handler invoked, and isvisible is
checked - at which point it is true. the record is deleted in the
onclick handler, the page is rerendered - delete button is still
visible because the ldm has cached the visibility value from before
onclick handler is invoked. to make it work correctly the ldm would
have to be detached manually after onclick handler.
-igor
On Thu, Dec 2, 2010 at 6:36 PM, Clint Checketts <ch...@gmail.com> wrote:
> Yesterday a friend following this thread pointed out that we should rethink
> our overriding of onVisible and use onConfigure. I've used
> LoadabledDetachableModels to cache the value used in my isVisible/isEnabled
> overriding so changing values mid request aren't a problem. That is its
> whole purpose. Also calling .detach() on that model isn't hacky, that is its
> design.
>
> While I appreciate having onConfigure as an option it seems like overriding
> isVisible is still the cleaner and clearer way. Folks just need to follow
> the rule that expensive calls should be contained in an LDM.
>
> Am I stuck in the past in holding this view?
>
> -Clint
>
> On Thu, Dec 2, 2010 at 4:03 AM, Martin Makundi <
> martin.makundi@koodaripalvelut.com> wrote:
>
>> What about using onconfigure to replace loadabledetachablemodel ? We
>> have had some trouble with loadabledetachablemodels when their state
>> is frozen before a dependent model has been initialized (for example)
>> and we need to call model.detach() from within our code, which seems
>> bit hacky.
>>
>> Initializing also models at a specific known requestcycle moment might
>> be beneficial. Ofcourse it is not so straightforward as with
>> enable/visible state.
>>
>> **
>> Martin
>>
>> 2010/12/1 Igor Vaynberg <ig...@gmail.com>:
>> > i would be happy if that was good enough. in the past it hasnt been,
>> > thats why we have the current solution. maybe we can try it again in
>> > 1.5 and see what happens.
>> >
>> > -igor
>> >
>> > On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com>
>> wrote:
>> >> I have a different point of view, the HTTP imposes us some limitations,
>> we
>> >> will hardly have an good synchronization between the component state on
>> >> browser and server using only HTTP conversation. So it is mandatory the
>> >> service layer to respect the described security restriction.
>> >>
>> >> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>> >wrote:
>> >>
>> >>> an easy example is security.
>> >>>
>> >>> a user views a page that allows them to delete another user
>> >>> meanwhile their permissions are tweaked and they can no longer delete
>> >>> other users
>> >>> half an hour later the user clicks the delete button - this should
>> >>> fail, but wont if we are using last-rendered state.
>> >>>
>> >>> -igor
>> >>>
>> >>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
>> >>> wrote:
>> >>> > I need to look better on which core components are relying on an
>> updated
>> >>> > visibility/enabled state at the process event time, and why the last
>> >>> > rendered state wouldn't be enough to them to work nicely.
>> >>> >
>> >>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <
>> igor.vaynberg@gmail.com
>> >>> >wrote:
>> >>> >
>> >>> >> currently we only invoke configure before the render. this would
>> mean
>> >>> >> we would have to invoke it before processing a listener, clearing
>> the
>> >>> >> cache, and then invoking it again before render. i wonder if that is
>> >>> >> enough places to invoke it....
>> >>> >>
>> >>> >> -igor
>> >>> >>
>> >>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
>> >>> wrote:
>> >>> >> > If user click an link, it will change the value of some property
>> at
>> >>> the
>> >>> >> > process_event request cycle step. Then the processor will go to
>> the
>> >>> >> respond
>> >>> >> > step, will invoke every component before render method which will
>> end
>> >>> up
>> >>> >> > invoking the Component#configure and updating the
>> visibility/enabled
>> >>> >> state
>> >>> >> > (even if it changes, we are able to work with the updated state).
>> So
>> >>> when
>> >>> >> > the this component has the opportunity to render it self, it will
>> be
>> >>> >> aware
>> >>> >> > its update state.
>> >>> >> >
>> >>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
>> >>> igor.vaynberg@gmail.com
>> >>> >> >wrote:
>> >>> >> >
>> >>> >> >> there are other places that should be checked though. for example
>> >>> >> >> before we invoke a listener on the component we should check
>> again to
>> >>> >> >> make sure that visibility hasnt changed. eg if visibility depends
>> on
>> >>> >> >> some property of the user clicking the link that changed between
>> >>> >> >> render and clicking the link.
>> >>> >> >>
>> >>> >> >> -igor
>> >>> >> >>
>> >>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <
>> pedrosans@gmail.com>
>> >>> >> wrote:
>> >>> >> >> > An implementation idea:
>> >>> >> >> >
>> >>> >> >> > Component {
>> >>> >> >> > public final void configure()
>> >>> >> >> > {
>> >>> >> >> > if (!getFlag(FLAG_CONFIGURED))
>> >>> >> >> > {
>> >>> >> >> > setVisible_NoClientCode(isVisible()); //we only
>> check
>> >>> the
>> >>> >> user
>> >>> >> >> > isVisible in here
>> >>> >> >> > onConfigure();
>> >>> >> >> > setFlag(FLAG_CONFIGURED, true);
>> >>> >> >> > }
>> >>> >> >> > }
>> >>> >> >> > }
>> >>> >> >> >
>> >>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>> >>> >> igor.vaynberg@gmail.com
>> >>> >> >> >wrote:
>> >>> >> >> >
>> >>> >> >> >> so how is it different if they can still override something
>> that
>> >>> >> needs
>> >>> >> >> >> to be checked all the time?
>> >>> >> >> >>
>> >>> >> >> >> -igor
>> >>> >> >> >>
>> >>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
>> >>> pedrosans@gmail.com>
>> >>> >> >> wrote:
>> >>> >> >> >> > I understand the concern about possible isVisible
>> >>> implementations
>> >>> >> like
>> >>> >> >> >> >
>> >>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>> >>> >> component
>> >>> >> >> >> being
>> >>> >> >> >> > rendered at 09:59:59
>> >>> >> >> >> > isVisible(return dao.list().size() > 0);// performance
>> issues
>> >>> >> >> >> >
>> >>> >> >> >> > But maybe we can have the best from both approaches. This is
>> an
>> >>> >> >> >> copy/paste
>> >>> >> >> >> > from java.awt.Component:
>> >>> >> >> >> >
>> >>> >> >> >> > public boolean isVisible() {
>> >>> >> >> >> > return isVisible_NoClientCode();
>> >>> >> >> >> > }
>> >>> >> >> >> > final boolean isVisible_NoClientCode() {
>> >>> >> >> >> > return visible;
>> >>> >> >> >> > }
>> >>> >> >> >> >
>> >>> >> >> >> > There are some points in the awt framework were the
>> isVisible
>> >>> >> method
>> >>> >> >> is
>> >>> >> >> >> not
>> >>> >> >> >> > used in benefit of isVisible_NoClientCode
>> >>> >> >> >> > I'm in favor of create an final isVisible/Enabled version
>> and
>> >>> >> change
>> >>> >> >> the
>> >>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
>> >>> provide
>> >>> >> >> their
>> >>> >> >> >> > isVisible/Enable implementations that will serve to feed the
>> >>> core
>> >>> >> >> >> component
>> >>> >> >> >> > state.
>> >>> >> >> >> >
>> >>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>> >>> >> >> igor.vaynberg@gmail.com
>> >>> >> >> >> >wrote:
>> >>> >> >> >> >
>> >>> >> >> >> >> ive run into plenty of weird problems with overrides, but
>> maybe
>> >>> >> >> >> >> because this was in a high concurrency app where data
>> changed
>> >>> >> >> >> >> frequently. the problems arise from the fact that the value
>> >>> >> returned
>> >>> >> >> >> >> from isvisible() can change while we are doing traversals,
>> etc.
>> >>> >> >> >> >>
>> >>> >> >> >> >> eg we run a traversal for all visible components and put
>> them
>> >>> in a
>> >>> >> >> >> >> list. later we iterate over the list and try to render
>> these
>> >>> >> >> >> >> components. the render function also checks their
>> visibility
>> >>> and
>> >>> >> if
>> >>> >> >> >> >> they are no longer visible it throws an exception.
>> >>> >> >> >> >>
>> >>> >> >> >> >> if isvisible() override depends on some external factor
>> like
>> >>> the
>> >>> >> >> >> >> database there is a small window where the value can change
>> and
>> >>> >> now
>> >>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
>> >>> >> listener
>> >>> >> >> on
>> >>> >> >> >> >> a component that is not visible or not enabled". these are
>> very
>> >>> >> >> >> >> intermittent and damn near impossible to reproduce.
>> >>> >> >> >> >>
>> >>> >> >> >> >> another problem is performance. isvisible() is called
>> multiple
>> >>> >> times
>> >>> >> >> >> >> during the request and if it depends on the database it can
>> be
>> >>> a
>> >>> >> >> >> >> performance problem. in fact a couple of users have
>> complained
>> >>> >> about
>> >>> >> >> >> >> this on the list in the past. at least now we have an easy
>> >>> >> solution
>> >>> >> >> >> >> for them - use onconfigure().
>> >>> >> >> >> >>
>> >>> >> >> >> >> so as of right now the developers have two choices:
>> override
>> >>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
>> >>> override
>> >>> >> >> >> >> onconfigure() and set visibility there in a more
>> deterministic
>> >>> >> >> >> >> fashion.
>> >>> >> >> >> >>
>> >>> >> >> >> >> -igor
>> >>> >> >> >> >>
>> >>> >> >> >> >>
>> >>> >> >> >> >>
>> >>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> >>> >> >> >> >> <ee...@gmail.com> wrote:
>> >>> >> >> >> >> > To expand, unless I'm missing something (new?), things
>> are
>> >>> >> really
>> >>> >> >> only
>> >>> >> >> >> >> > problematic when both the mutable value and the override
>> are
>> >>> >> mixed.
>> >>> >> >> In
>> >>> >> >> >> >> > a way, I think that using the override is 'more pure', as
>> >>> it's a
>> >>> >> >> >> >> > simple function that is executed when needed, whereas
>> mutable
>> >>> >> state
>> >>> >> >> >> >> > can be harder to deal with when trying to figure out how
>> it
>> >>> got
>> >>> >> to
>> >>> >> >> be
>> >>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this
>> one.
>> >>> >> >> >> >> >
>> >>> >> >> >> >> > Eelco
>> >>> >> >> >> >> >
>> >>> >> >> >> >> >
>> >>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> >>> >> >> >> >> > <ee...@gmail.com> wrote:
>> >>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you
>> should
>> >>> >> just
>> >>> >> >> be
>> >>> >> >> >> >> >> aware of. We use such overrides all over the place and
>> never
>> >>> >> have
>> >>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
>> >>> also
>> >>> >> more
>> >>> >> >> >> >> >> verbose (in 1.3.x at least).
>> >>> >> >> >> >> >>
>> >>> >> >> >> >> >> Eelco
>> >>> >> >> >> >> >>
>> >>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>> >>> >> >> >> igor.vaynberg@gmail.com>
>> >>> >> >> >> >> wrote:
>> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>> >>> sven@meiers.net>
>> >>> >> >> >> wrote:
>> >>> >> >> >> >> >>>> Hi Douglas,
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where
>> visibility
>> >>> of
>> >>> >> a
>> >>> >> >> >> >> >>>> component changes while its form is being processed.
>> >>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
>> >>> >> appropriate
>> >>> >> >> and
>> >>> >> >> >> >> never
>> >>> >> >> >> >> >>>> encountered a similar problem.
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's
>> next,
>> >>> is
>> >>> >> >> >> overriding
>> >>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>> >>> >> >> >> >> >>>
>> >>> >> >> >> >> >>> yes
>> >>> >> >> >> >> >>>
>> >>> >> >> >> >> >>> -igor
>> >>> >> >> >> >> >>>
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>> Sven
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson
>> wrote:
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
>> >>> place.
>> >>> >> >> >> >> >>>>>
>> >>> >> >> >> >> >>>>> D/
>> >>> >> >> >> >> >>>>>
>> >>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >>> >> >> >> >> >>>>>
>> >>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>> >>> >> override
>> >>> >> >> >> >> onConfigure()
>> >>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>> >>> >> conditions.
>> >>> >> >> >> >> >>>>> >
>> >>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>> >>> >> >> >> >> >>>>> >
>> >>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
>> >>> overriding
>> >>> >> >> >> >> isVisible() is
>> >>> >> >> >> >> >>>>> >> "evil"
>> >>> >> >> >> >> >>>>> >>
>> >>> >> >> >> >> >>>>> >>
>> >>> https://issues.apache.org/jira/browse/WICKET-3171
>> >>> >> >> >> >> >>>>> >>
>> >>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear
>> more.
>> >>> >> >> >> >> >>>>> >>
>> >>> >> >> >> >> >>>>> >> D/
>> >>> >> >> >> >> >>>>>
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>>
>> >>> >> >> >> >> >>>
>> >>> >> >> >> >> >>
>> >>> >> >> >> >> >
>> >>> >> >> >> >>
>> >>> >> >> >> >
>> >>> >> >> >> >
>> >>> >> >> >> >
>> >>> >> >> >> > --
>> >>> >> >> >> > Pedro Henrique Oliveira dos Santos
>> >>> >> >> >> >
>> >>> >> >> >>
>> >>> >> >> >
>> >>> >> >> >
>> >>> >> >> >
>> >>> >> >> > --
>> >>> >> >> > Pedro Henrique Oliveira dos Santos
>> >>> >> >> >
>> >>> >> >>
>> >>> >> >
>> >>> >> >
>> >>> >> >
>> >>> >> > --
>> >>> >> > Pedro Henrique Oliveira dos Santos
>> >>> >> >
>> >>> >>
>> >>> >
>> >>> >
>> >>> >
>> >>> > --
>> >>> > Pedro Henrique Oliveira dos Santos
>> >>> >
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> Pedro Henrique Oliveira dos Santos
>> >>
>> >
>>
>
Re: overriding isVisible bad?
Posted by James Carman <ja...@carmanconsulting.com>.
On Thu, Dec 2, 2010 at 9:36 PM, Clint Checketts <ch...@gmail.com> wrote:
>
> While I appreciate having onConfigure as an option it seems like overriding
> isVisible is still the cleaner and clearer way. Folks just need to follow
> the rule that expensive calls should be contained in an LDM.
>
The expense isn't the problem. It's the dynamic nature of the value
returned that causes issues, IIUC.
Re: overriding isVisible bad?
Posted by Clint Checketts <ch...@gmail.com>.
Yesterday a friend following this thread pointed out that we should rethink
our overriding of onVisible and use onConfigure. I've used
LoadabledDetachableModels to cache the value used in my isVisible/isEnabled
overriding so changing values mid request aren't a problem. That is its
whole purpose. Also calling .detach() on that model isn't hacky, that is its
design.
While I appreciate having onConfigure as an option it seems like overriding
isVisible is still the cleaner and clearer way. Folks just need to follow
the rule that expensive calls should be contained in an LDM.
Am I stuck in the past in holding this view?
-Clint
On Thu, Dec 2, 2010 at 4:03 AM, Martin Makundi <
martin.makundi@koodaripalvelut.com> wrote:
> What about using onconfigure to replace loadabledetachablemodel ? We
> have had some trouble with loadabledetachablemodels when their state
> is frozen before a dependent model has been initialized (for example)
> and we need to call model.detach() from within our code, which seems
> bit hacky.
>
> Initializing also models at a specific known requestcycle moment might
> be beneficial. Ofcourse it is not so straightforward as with
> enable/visible state.
>
> **
> Martin
>
> 2010/12/1 Igor Vaynberg <ig...@gmail.com>:
> > i would be happy if that was good enough. in the past it hasnt been,
> > thats why we have the current solution. maybe we can try it again in
> > 1.5 and see what happens.
> >
> > -igor
> >
> > On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >> I have a different point of view, the HTTP imposes us some limitations,
> we
> >> will hardly have an good synchronization between the component state on
> >> browser and server using only HTTP conversation. So it is mandatory the
> >> service layer to respect the described security restriction.
> >>
> >> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >>
> >>> an easy example is security.
> >>>
> >>> a user views a page that allows them to delete another user
> >>> meanwhile their permissions are tweaked and they can no longer delete
> >>> other users
> >>> half an hour later the user clicks the delete button - this should
> >>> fail, but wont if we are using last-rendered state.
> >>>
> >>> -igor
> >>>
> >>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
> >>> wrote:
> >>> > I need to look better on which core components are relying on an
> updated
> >>> > visibility/enabled state at the process event time, and why the last
> >>> > rendered state wouldn't be enough to them to work nicely.
> >>> >
> >>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> >>> >wrote:
> >>> >
> >>> >> currently we only invoke configure before the render. this would
> mean
> >>> >> we would have to invoke it before processing a listener, clearing
> the
> >>> >> cache, and then invoking it again before render. i wonder if that is
> >>> >> enough places to invoke it....
> >>> >>
> >>> >> -igor
> >>> >>
> >>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
> >>> wrote:
> >>> >> > If user click an link, it will change the value of some property
> at
> >>> the
> >>> >> > process_event request cycle step. Then the processor will go to
> the
> >>> >> respond
> >>> >> > step, will invoke every component before render method which will
> end
> >>> up
> >>> >> > invoking the Component#configure and updating the
> visibility/enabled
> >>> >> state
> >>> >> > (even if it changes, we are able to work with the updated state).
> So
> >>> when
> >>> >> > the this component has the opportunity to render it self, it will
> be
> >>> >> aware
> >>> >> > its update state.
> >>> >> >
> >>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
> >>> igor.vaynberg@gmail.com
> >>> >> >wrote:
> >>> >> >
> >>> >> >> there are other places that should be checked though. for example
> >>> >> >> before we invoke a listener on the component we should check
> again to
> >>> >> >> make sure that visibility hasnt changed. eg if visibility depends
> on
> >>> >> >> some property of the user clicking the link that changed between
> >>> >> >> render and clicking the link.
> >>> >> >>
> >>> >> >> -igor
> >>> >> >>
> >>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <
> pedrosans@gmail.com>
> >>> >> wrote:
> >>> >> >> > An implementation idea:
> >>> >> >> >
> >>> >> >> > Component {
> >>> >> >> > public final void configure()
> >>> >> >> > {
> >>> >> >> > if (!getFlag(FLAG_CONFIGURED))
> >>> >> >> > {
> >>> >> >> > setVisible_NoClientCode(isVisible()); //we only
> check
> >>> the
> >>> >> user
> >>> >> >> > isVisible in here
> >>> >> >> > onConfigure();
> >>> >> >> > setFlag(FLAG_CONFIGURED, true);
> >>> >> >> > }
> >>> >> >> > }
> >>> >> >> > }
> >>> >> >> >
> >>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
> >>> >> igor.vaynberg@gmail.com
> >>> >> >> >wrote:
> >>> >> >> >
> >>> >> >> >> so how is it different if they can still override something
> that
> >>> >> needs
> >>> >> >> >> to be checked all the time?
> >>> >> >> >>
> >>> >> >> >> -igor
> >>> >> >> >>
> >>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
> >>> pedrosans@gmail.com>
> >>> >> >> wrote:
> >>> >> >> >> > I understand the concern about possible isVisible
> >>> implementations
> >>> >> like
> >>> >> >> >> >
> >>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
> >>> >> component
> >>> >> >> >> being
> >>> >> >> >> > rendered at 09:59:59
> >>> >> >> >> > isVisible(return dao.list().size() > 0);// performance
> issues
> >>> >> >> >> >
> >>> >> >> >> > But maybe we can have the best from both approaches. This is
> an
> >>> >> >> >> copy/paste
> >>> >> >> >> > from java.awt.Component:
> >>> >> >> >> >
> >>> >> >> >> > public boolean isVisible() {
> >>> >> >> >> > return isVisible_NoClientCode();
> >>> >> >> >> > }
> >>> >> >> >> > final boolean isVisible_NoClientCode() {
> >>> >> >> >> > return visible;
> >>> >> >> >> > }
> >>> >> >> >> >
> >>> >> >> >> > There are some points in the awt framework were the
> isVisible
> >>> >> method
> >>> >> >> is
> >>> >> >> >> not
> >>> >> >> >> > used in benefit of isVisible_NoClientCode
> >>> >> >> >> > I'm in favor of create an final isVisible/Enabled version
> and
> >>> >> change
> >>> >> >> the
> >>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
> >>> provide
> >>> >> >> their
> >>> >> >> >> > isVisible/Enable implementations that will serve to feed the
> >>> core
> >>> >> >> >> component
> >>> >> >> >> > state.
> >>> >> >> >> >
> >>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> >>> >> >> igor.vaynberg@gmail.com
> >>> >> >> >> >wrote:
> >>> >> >> >> >
> >>> >> >> >> >> ive run into plenty of weird problems with overrides, but
> maybe
> >>> >> >> >> >> because this was in a high concurrency app where data
> changed
> >>> >> >> >> >> frequently. the problems arise from the fact that the value
> >>> >> returned
> >>> >> >> >> >> from isvisible() can change while we are doing traversals,
> etc.
> >>> >> >> >> >>
> >>> >> >> >> >> eg we run a traversal for all visible components and put
> them
> >>> in a
> >>> >> >> >> >> list. later we iterate over the list and try to render
> these
> >>> >> >> >> >> components. the render function also checks their
> visibility
> >>> and
> >>> >> if
> >>> >> >> >> >> they are no longer visible it throws an exception.
> >>> >> >> >> >>
> >>> >> >> >> >> if isvisible() override depends on some external factor
> like
> >>> the
> >>> >> >> >> >> database there is a small window where the value can change
> and
> >>> >> now
> >>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
> >>> >> listener
> >>> >> >> on
> >>> >> >> >> >> a component that is not visible or not enabled". these are
> very
> >>> >> >> >> >> intermittent and damn near impossible to reproduce.
> >>> >> >> >> >>
> >>> >> >> >> >> another problem is performance. isvisible() is called
> multiple
> >>> >> times
> >>> >> >> >> >> during the request and if it depends on the database it can
> be
> >>> a
> >>> >> >> >> >> performance problem. in fact a couple of users have
> complained
> >>> >> about
> >>> >> >> >> >> this on the list in the past. at least now we have an easy
> >>> >> solution
> >>> >> >> >> >> for them - use onconfigure().
> >>> >> >> >> >>
> >>> >> >> >> >> so as of right now the developers have two choices:
> override
> >>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
> >>> override
> >>> >> >> >> >> onconfigure() and set visibility there in a more
> deterministic
> >>> >> >> >> >> fashion.
> >>> >> >> >> >>
> >>> >> >> >> >> -igor
> >>> >> >> >> >>
> >>> >> >> >> >>
> >>> >> >> >> >>
> >>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >>> >> >> >> >> <ee...@gmail.com> wrote:
> >>> >> >> >> >> > To expand, unless I'm missing something (new?), things
> are
> >>> >> really
> >>> >> >> only
> >>> >> >> >> >> > problematic when both the mutable value and the override
> are
> >>> >> mixed.
> >>> >> >> In
> >>> >> >> >> >> > a way, I think that using the override is 'more pure', as
> >>> it's a
> >>> >> >> >> >> > simple function that is executed when needed, whereas
> mutable
> >>> >> state
> >>> >> >> >> >> > can be harder to deal with when trying to figure out how
> it
> >>> got
> >>> >> to
> >>> >> >> be
> >>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this
> one.
> >>> >> >> >> >> >
> >>> >> >> >> >> > Eelco
> >>> >> >> >> >> >
> >>> >> >> >> >> >
> >>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >>> >> >> >> >> > <ee...@gmail.com> wrote:
> >>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you
> should
> >>> >> just
> >>> >> >> be
> >>> >> >> >> >> >> aware of. We use such overrides all over the place and
> never
> >>> >> have
> >>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
> >>> also
> >>> >> more
> >>> >> >> >> >> >> verbose (in 1.3.x at least).
> >>> >> >> >> >> >>
> >>> >> >> >> >> >> Eelco
> >>> >> >> >> >> >>
> >>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >>> >> >> >> igor.vaynberg@gmail.com>
> >>> >> >> >> >> wrote:
> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
> >>> sven@meiers.net>
> >>> >> >> >> wrote:
> >>> >> >> >> >> >>>> Hi Douglas,
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where
> visibility
> >>> of
> >>> >> a
> >>> >> >> >> >> >>>> component changes while its form is being processed.
> >>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
> >>> >> appropriate
> >>> >> >> and
> >>> >> >> >> >> never
> >>> >> >> >> >> >>>> encountered a similar problem.
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's
> next,
> >>> is
> >>> >> >> >> overriding
> >>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >>> >> >> >> >> >>>
> >>> >> >> >> >> >>> yes
> >>> >> >> >> >> >>>
> >>> >> >> >> >> >>> -igor
> >>> >> >> >> >> >>>
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>> Sven
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson
> wrote:
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
> >>> place.
> >>> >> >> >> >> >>>>>
> >>> >> >> >> >> >>>>> D/
> >>> >> >> >> >> >>>>>
> >>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >>> >> >> >> >> >>>>>
> >>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
> >>> >> override
> >>> >> >> >> >> onConfigure()
> >>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
> >>> >> conditions.
> >>> >> >> >> >> >>>>> >
> >>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
> >>> >> >> >> >> >>>>> >
> >>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
> >>> overriding
> >>> >> >> >> >> isVisible() is
> >>> >> >> >> >> >>>>> >> "evil"
> >>> >> >> >> >> >>>>> >>
> >>> >> >> >> >> >>>>> >>
> >>> https://issues.apache.org/jira/browse/WICKET-3171
> >>> >> >> >> >> >>>>> >>
> >>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear
> more.
> >>> >> >> >> >> >>>>> >>
> >>> >> >> >> >> >>>>> >> D/
> >>> >> >> >> >> >>>>>
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>>
> >>> >> >> >> >> >>>
> >>> >> >> >> >> >>
> >>> >> >> >> >> >
> >>> >> >> >> >>
> >>> >> >> >> >
> >>> >> >> >> >
> >>> >> >> >> >
> >>> >> >> >> > --
> >>> >> >> >> > Pedro Henrique Oliveira dos Santos
> >>> >> >> >> >
> >>> >> >> >>
> >>> >> >> >
> >>> >> >> >
> >>> >> >> >
> >>> >> >> > --
> >>> >> >> > Pedro Henrique Oliveira dos Santos
> >>> >> >> >
> >>> >> >>
> >>> >> >
> >>> >> >
> >>> >> >
> >>> >> > --
> >>> >> > Pedro Henrique Oliveira dos Santos
> >>> >> >
> >>> >>
> >>> >
> >>> >
> >>> >
> >>> > --
> >>> > Pedro Henrique Oliveira dos Santos
> >>> >
> >>>
> >>
> >>
> >>
> >> --
> >> Pedro Henrique Oliveira dos Santos
> >>
> >
>
Re: overriding isVisible bad?
Posted by Martin Makundi <ma...@koodaripalvelut.com>.
What about using onconfigure to replace loadabledetachablemodel ? We
have had some trouble with loadabledetachablemodels when their state
is frozen before a dependent model has been initialized (for example)
and we need to call model.detach() from within our code, which seems
bit hacky.
Initializing also models at a specific known requestcycle moment might
be beneficial. Ofcourse it is not so straightforward as with
enable/visible state.
**
Martin
2010/12/1 Igor Vaynberg <ig...@gmail.com>:
> i would be happy if that was good enough. in the past it hasnt been,
> thats why we have the current solution. maybe we can try it again in
> 1.5 and see what happens.
>
> -igor
>
> On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com> wrote:
>> I have a different point of view, the HTTP imposes us some limitations, we
>> will hardly have an good synchronization between the component state on
>> browser and server using only HTTP conversation. So it is mandatory the
>> service layer to respect the described security restriction.
>>
>> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>>
>>> an easy example is security.
>>>
>>> a user views a page that allows them to delete another user
>>> meanwhile their permissions are tweaked and they can no longer delete
>>> other users
>>> half an hour later the user clicks the delete button - this should
>>> fail, but wont if we are using last-rendered state.
>>>
>>> -igor
>>>
>>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
>>> wrote:
>>> > I need to look better on which core components are relying on an updated
>>> > visibility/enabled state at the process event time, and why the last
>>> > rendered state wouldn't be enough to them to work nicely.
>>> >
>>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>>> >wrote:
>>> >
>>> >> currently we only invoke configure before the render. this would mean
>>> >> we would have to invoke it before processing a listener, clearing the
>>> >> cache, and then invoking it again before render. i wonder if that is
>>> >> enough places to invoke it....
>>> >>
>>> >> -igor
>>> >>
>>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
>>> wrote:
>>> >> > If user click an link, it will change the value of some property at
>>> the
>>> >> > process_event request cycle step. Then the processor will go to the
>>> >> respond
>>> >> > step, will invoke every component before render method which will end
>>> up
>>> >> > invoking the Component#configure and updating the visibility/enabled
>>> >> state
>>> >> > (even if it changes, we are able to work with the updated state). So
>>> when
>>> >> > the this component has the opportunity to render it self, it will be
>>> >> aware
>>> >> > its update state.
>>> >> >
>>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
>>> igor.vaynberg@gmail.com
>>> >> >wrote:
>>> >> >
>>> >> >> there are other places that should be checked though. for example
>>> >> >> before we invoke a listener on the component we should check again to
>>> >> >> make sure that visibility hasnt changed. eg if visibility depends on
>>> >> >> some property of the user clicking the link that changed between
>>> >> >> render and clicking the link.
>>> >> >>
>>> >> >> -igor
>>> >> >>
>>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com>
>>> >> wrote:
>>> >> >> > An implementation idea:
>>> >> >> >
>>> >> >> > Component {
>>> >> >> > public final void configure()
>>> >> >> > {
>>> >> >> > if (!getFlag(FLAG_CONFIGURED))
>>> >> >> > {
>>> >> >> > setVisible_NoClientCode(isVisible()); //we only check
>>> the
>>> >> user
>>> >> >> > isVisible in here
>>> >> >> > onConfigure();
>>> >> >> > setFlag(FLAG_CONFIGURED, true);
>>> >> >> > }
>>> >> >> > }
>>> >> >> > }
>>> >> >> >
>>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>>> >> igor.vaynberg@gmail.com
>>> >> >> >wrote:
>>> >> >> >
>>> >> >> >> so how is it different if they can still override something that
>>> >> needs
>>> >> >> >> to be checked all the time?
>>> >> >> >>
>>> >> >> >> -igor
>>> >> >> >>
>>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
>>> pedrosans@gmail.com>
>>> >> >> wrote:
>>> >> >> >> > I understand the concern about possible isVisible
>>> implementations
>>> >> like
>>> >> >> >> >
>>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>>> >> component
>>> >> >> >> being
>>> >> >> >> > rendered at 09:59:59
>>> >> >> >> > isVisible(return dao.list().size() > 0);// performance issues
>>> >> >> >> >
>>> >> >> >> > But maybe we can have the best from both approaches. This is an
>>> >> >> >> copy/paste
>>> >> >> >> > from java.awt.Component:
>>> >> >> >> >
>>> >> >> >> > public boolean isVisible() {
>>> >> >> >> > return isVisible_NoClientCode();
>>> >> >> >> > }
>>> >> >> >> > final boolean isVisible_NoClientCode() {
>>> >> >> >> > return visible;
>>> >> >> >> > }
>>> >> >> >> >
>>> >> >> >> > There are some points in the awt framework were the isVisible
>>> >> method
>>> >> >> is
>>> >> >> >> not
>>> >> >> >> > used in benefit of isVisible_NoClientCode
>>> >> >> >> > I'm in favor of create an final isVisible/Enabled version and
>>> >> change
>>> >> >> the
>>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
>>> provide
>>> >> >> their
>>> >> >> >> > isVisible/Enable implementations that will serve to feed the
>>> core
>>> >> >> >> component
>>> >> >> >> > state.
>>> >> >> >> >
>>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>>> >> >> igor.vaynberg@gmail.com
>>> >> >> >> >wrote:
>>> >> >> >> >
>>> >> >> >> >> ive run into plenty of weird problems with overrides, but maybe
>>> >> >> >> >> because this was in a high concurrency app where data changed
>>> >> >> >> >> frequently. the problems arise from the fact that the value
>>> >> returned
>>> >> >> >> >> from isvisible() can change while we are doing traversals, etc.
>>> >> >> >> >>
>>> >> >> >> >> eg we run a traversal for all visible components and put them
>>> in a
>>> >> >> >> >> list. later we iterate over the list and try to render these
>>> >> >> >> >> components. the render function also checks their visibility
>>> and
>>> >> if
>>> >> >> >> >> they are no longer visible it throws an exception.
>>> >> >> >> >>
>>> >> >> >> >> if isvisible() override depends on some external factor like
>>> the
>>> >> >> >> >> database there is a small window where the value can change and
>>> >> now
>>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
>>> >> listener
>>> >> >> on
>>> >> >> >> >> a component that is not visible or not enabled". these are very
>>> >> >> >> >> intermittent and damn near impossible to reproduce.
>>> >> >> >> >>
>>> >> >> >> >> another problem is performance. isvisible() is called multiple
>>> >> times
>>> >> >> >> >> during the request and if it depends on the database it can be
>>> a
>>> >> >> >> >> performance problem. in fact a couple of users have complained
>>> >> about
>>> >> >> >> >> this on the list in the past. at least now we have an easy
>>> >> solution
>>> >> >> >> >> for them - use onconfigure().
>>> >> >> >> >>
>>> >> >> >> >> so as of right now the developers have two choices: override
>>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
>>> override
>>> >> >> >> >> onconfigure() and set visibility there in a more deterministic
>>> >> >> >> >> fashion.
>>> >> >> >> >>
>>> >> >> >> >> -igor
>>> >> >> >> >>
>>> >> >> >> >>
>>> >> >> >> >>
>>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>>> >> >> >> >> <ee...@gmail.com> wrote:
>>> >> >> >> >> > To expand, unless I'm missing something (new?), things are
>>> >> really
>>> >> >> only
>>> >> >> >> >> > problematic when both the mutable value and the override are
>>> >> mixed.
>>> >> >> In
>>> >> >> >> >> > a way, I think that using the override is 'more pure', as
>>> it's a
>>> >> >> >> >> > simple function that is executed when needed, whereas mutable
>>> >> state
>>> >> >> >> >> > can be harder to deal with when trying to figure out how it
>>> got
>>> >> to
>>> >> >> be
>>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
>>> >> >> >> >> >
>>> >> >> >> >> > Eelco
>>> >> >> >> >> >
>>> >> >> >> >> >
>>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>>> >> >> >> >> > <ee...@gmail.com> wrote:
>>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
>>> >> just
>>> >> >> be
>>> >> >> >> >> >> aware of. We use such overrides all over the place and never
>>> >> have
>>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
>>> also
>>> >> more
>>> >> >> >> >> >> verbose (in 1.3.x at least).
>>> >> >> >> >> >>
>>> >> >> >> >> >> Eelco
>>> >> >> >> >> >>
>>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>>> >> >> >> igor.vaynberg@gmail.com>
>>> >> >> >> >> wrote:
>>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>>> sven@meiers.net>
>>> >> >> >> wrote:
>>> >> >> >> >> >>>> Hi Douglas,
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility
>>> of
>>> >> a
>>> >> >> >> >> >>>> component changes while its form is being processed.
>>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
>>> >> appropriate
>>> >> >> and
>>> >> >> >> >> never
>>> >> >> >> >> >>>> encountered a similar problem.
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next,
>>> is
>>> >> >> >> overriding
>>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>>> >> >> >> >> >>>
>>> >> >> >> >> >>> yes
>>> >> >> >> >> >>>
>>> >> >> >> >> >>> -igor
>>> >> >> >> >> >>>
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>> Sven
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
>>> place.
>>> >> >> >> >> >>>>>
>>> >> >> >> >> >>>>> D/
>>> >> >> >> >> >>>>>
>>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>> >> >> >> >> >>>>>
>>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>>> >> override
>>> >> >> >> >> onConfigure()
>>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>>> >> conditions.
>>> >> >> >> >> >>>>> >
>>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>>> >> >> >> >> >>>>> >
>>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
>>> overriding
>>> >> >> >> >> isVisible() is
>>> >> >> >> >> >>>>> >> "evil"
>>> >> >> >> >> >>>>> >>
>>> >> >> >> >> >>>>> >>
>>> https://issues.apache.org/jira/browse/WICKET-3171
>>> >> >> >> >> >>>>> >>
>>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
>>> >> >> >> >> >>>>> >>
>>> >> >> >> >> >>>>> >> D/
>>> >> >> >> >> >>>>>
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>>
>>> >> >> >> >> >>>
>>> >> >> >> >> >>
>>> >> >> >> >> >
>>> >> >> >> >>
>>> >> >> >> >
>>> >> >> >> >
>>> >> >> >> >
>>> >> >> >> > --
>>> >> >> >> > Pedro Henrique Oliveira dos Santos
>>> >> >> >> >
>>> >> >> >>
>>> >> >> >
>>> >> >> >
>>> >> >> >
>>> >> >> > --
>>> >> >> > Pedro Henrique Oliveira dos Santos
>>> >> >> >
>>> >> >>
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Pedro Henrique Oliveira dos Santos
>>> >> >
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Pedro Henrique Oliveira dos Santos
>>> >
>>>
>>
>>
>>
>> --
>> Pedro Henrique Oliveira dos Santos
>>
>
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
i would be happy if that was good enough. in the past it hasnt been,
thats why we have the current solution. maybe we can try it again in
1.5 and see what happens.
-igor
On Tue, Nov 30, 2010 at 11:44 AM, Pedro Santos <pe...@gmail.com> wrote:
> I have a different point of view, the HTTP imposes us some limitations, we
> will hardly have an good synchronization between the component state on
> browser and server using only HTTP conversation. So it is mandatory the
> service layer to respect the described security restriction.
>
> On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> an easy example is security.
>>
>> a user views a page that allows them to delete another user
>> meanwhile their permissions are tweaked and they can no longer delete
>> other users
>> half an hour later the user clicks the delete button - this should
>> fail, but wont if we are using last-rendered state.
>>
>> -igor
>>
>> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
>> wrote:
>> > I need to look better on which core components are relying on an updated
>> > visibility/enabled state at the process event time, and why the last
>> > rendered state wouldn't be enough to them to work nicely.
>> >
>> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>> >wrote:
>> >
>> >> currently we only invoke configure before the render. this would mean
>> >> we would have to invoke it before processing a listener, clearing the
>> >> cache, and then invoking it again before render. i wonder if that is
>> >> enough places to invoke it....
>> >>
>> >> -igor
>> >>
>> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
>> wrote:
>> >> > If user click an link, it will change the value of some property at
>> the
>> >> > process_event request cycle step. Then the processor will go to the
>> >> respond
>> >> > step, will invoke every component before render method which will end
>> up
>> >> > invoking the Component#configure and updating the visibility/enabled
>> >> state
>> >> > (even if it changes, we are able to work with the updated state). So
>> when
>> >> > the this component has the opportunity to render it self, it will be
>> >> aware
>> >> > its update state.
>> >> >
>> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
>> igor.vaynberg@gmail.com
>> >> >wrote:
>> >> >
>> >> >> there are other places that should be checked though. for example
>> >> >> before we invoke a listener on the component we should check again to
>> >> >> make sure that visibility hasnt changed. eg if visibility depends on
>> >> >> some property of the user clicking the link that changed between
>> >> >> render and clicking the link.
>> >> >>
>> >> >> -igor
>> >> >>
>> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com>
>> >> wrote:
>> >> >> > An implementation idea:
>> >> >> >
>> >> >> > Component {
>> >> >> > public final void configure()
>> >> >> > {
>> >> >> > if (!getFlag(FLAG_CONFIGURED))
>> >> >> > {
>> >> >> > setVisible_NoClientCode(isVisible()); //we only check
>> the
>> >> user
>> >> >> > isVisible in here
>> >> >> > onConfigure();
>> >> >> > setFlag(FLAG_CONFIGURED, true);
>> >> >> > }
>> >> >> > }
>> >> >> > }
>> >> >> >
>> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>> >> igor.vaynberg@gmail.com
>> >> >> >wrote:
>> >> >> >
>> >> >> >> so how is it different if they can still override something that
>> >> needs
>> >> >> >> to be checked all the time?
>> >> >> >>
>> >> >> >> -igor
>> >> >> >>
>> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
>> pedrosans@gmail.com>
>> >> >> wrote:
>> >> >> >> > I understand the concern about possible isVisible
>> implementations
>> >> like
>> >> >> >> >
>> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>> >> component
>> >> >> >> being
>> >> >> >> > rendered at 09:59:59
>> >> >> >> > isVisible(return dao.list().size() > 0);// performance issues
>> >> >> >> >
>> >> >> >> > But maybe we can have the best from both approaches. This is an
>> >> >> >> copy/paste
>> >> >> >> > from java.awt.Component:
>> >> >> >> >
>> >> >> >> > public boolean isVisible() {
>> >> >> >> > return isVisible_NoClientCode();
>> >> >> >> > }
>> >> >> >> > final boolean isVisible_NoClientCode() {
>> >> >> >> > return visible;
>> >> >> >> > }
>> >> >> >> >
>> >> >> >> > There are some points in the awt framework were the isVisible
>> >> method
>> >> >> is
>> >> >> >> not
>> >> >> >> > used in benefit of isVisible_NoClientCode
>> >> >> >> > I'm in favor of create an final isVisible/Enabled version and
>> >> change
>> >> >> the
>> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
>> provide
>> >> >> their
>> >> >> >> > isVisible/Enable implementations that will serve to feed the
>> core
>> >> >> >> component
>> >> >> >> > state.
>> >> >> >> >
>> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>> >> >> igor.vaynberg@gmail.com
>> >> >> >> >wrote:
>> >> >> >> >
>> >> >> >> >> ive run into plenty of weird problems with overrides, but maybe
>> >> >> >> >> because this was in a high concurrency app where data changed
>> >> >> >> >> frequently. the problems arise from the fact that the value
>> >> returned
>> >> >> >> >> from isvisible() can change while we are doing traversals, etc.
>> >> >> >> >>
>> >> >> >> >> eg we run a traversal for all visible components and put them
>> in a
>> >> >> >> >> list. later we iterate over the list and try to render these
>> >> >> >> >> components. the render function also checks their visibility
>> and
>> >> if
>> >> >> >> >> they are no longer visible it throws an exception.
>> >> >> >> >>
>> >> >> >> >> if isvisible() override depends on some external factor like
>> the
>> >> >> >> >> database there is a small window where the value can change and
>> >> now
>> >> >> >> >> you can have a weird exception: such as "tried to invoke a
>> >> listener
>> >> >> on
>> >> >> >> >> a component that is not visible or not enabled". these are very
>> >> >> >> >> intermittent and damn near impossible to reproduce.
>> >> >> >> >>
>> >> >> >> >> another problem is performance. isvisible() is called multiple
>> >> times
>> >> >> >> >> during the request and if it depends on the database it can be
>> a
>> >> >> >> >> performance problem. in fact a couple of users have complained
>> >> about
>> >> >> >> >> this on the list in the past. at least now we have an easy
>> >> solution
>> >> >> >> >> for them - use onconfigure().
>> >> >> >> >>
>> >> >> >> >> so as of right now the developers have two choices: override
>> >> >> >> >> isvisible() and potentially suffer the consequences. or,
>> override
>> >> >> >> >> onconfigure() and set visibility there in a more deterministic
>> >> >> >> >> fashion.
>> >> >> >> >>
>> >> >> >> >> -igor
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >>
>> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> >> >> >> >> <ee...@gmail.com> wrote:
>> >> >> >> >> > To expand, unless I'm missing something (new?), things are
>> >> really
>> >> >> only
>> >> >> >> >> > problematic when both the mutable value and the override are
>> >> mixed.
>> >> >> In
>> >> >> >> >> > a way, I think that using the override is 'more pure', as
>> it's a
>> >> >> >> >> > simple function that is executed when needed, whereas mutable
>> >> state
>> >> >> >> >> > can be harder to deal with when trying to figure out how it
>> got
>> >> to
>> >> >> be
>> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
>> >> >> >> >> >
>> >> >> >> >> > Eelco
>> >> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> >> >> >> >> > <ee...@gmail.com> wrote:
>> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
>> >> just
>> >> >> be
>> >> >> >> >> >> aware of. We use such overrides all over the place and never
>> >> have
>> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
>> also
>> >> more
>> >> >> >> >> >> verbose (in 1.3.x at least).
>> >> >> >> >> >>
>> >> >> >> >> >> Eelco
>> >> >> >> >> >>
>> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>> >> >> >> igor.vaynberg@gmail.com>
>> >> >> >> >> wrote:
>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>> sven@meiers.net>
>> >> >> >> wrote:
>> >> >> >> >> >>>> Hi Douglas,
>> >> >> >> >> >>>>
>> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility
>> of
>> >> a
>> >> >> >> >> >>>> component changes while its form is being processed.
>> >> >> >> >> >>>> In our projects we're overriding isVisible() where
>> >> appropriate
>> >> >> and
>> >> >> >> >> never
>> >> >> >> >> >>>> encountered a similar problem.
>> >> >> >> >> >>>>
>> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next,
>> is
>> >> >> >> overriding
>> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>> >> >> >> >> >>>
>> >> >> >> >> >>> yes
>> >> >> >> >> >>>
>> >> >> >> >> >>> -igor
>> >> >> >> >> >>>
>> >> >> >> >> >>>>
>> >> >> >> >> >>>> Sven
>> >> >> >> >> >>>>
>> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>> >> >> >> >> >>>>
>> >> >> >> >> >>>>> Can you explain why? We have done this all over the
>> place.
>> >> >> >> >> >>>>>
>> >> >> >> >> >>>>> D/
>> >> >> >> >> >>>>>
>> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >> >> >> >> >>>>>
>> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>> >> override
>> >> >> >> >> onConfigure()
>> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>> >> conditions.
>> >> >> >> >> >>>>> >
>> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>> >> >> >> >> >>>>> >
>> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
>> overriding
>> >> >> >> >> isVisible() is
>> >> >> >> >> >>>>> >> "evil"
>> >> >> >> >> >>>>> >>
>> >> >> >> >> >>>>> >>
>> https://issues.apache.org/jira/browse/WICKET-3171
>> >> >> >> >> >>>>> >>
>> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
>> >> >> >> >> >>>>> >>
>> >> >> >> >> >>>>> >> D/
>> >> >> >> >> >>>>>
>> >> >> >> >> >>>>
>> >> >> >> >> >>>>
>> >> >> >> >> >>>>
>> >> >> >> >> >>>
>> >> >> >> >> >>
>> >> >> >> >> >
>> >> >> >> >>
>> >> >> >> >
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > --
>> >> >> >> > Pedro Henrique Oliveira dos Santos
>> >> >> >> >
>> >> >> >>
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > --
>> >> >> > Pedro Henrique Oliveira dos Santos
>> >> >> >
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Pedro Henrique Oliveira dos Santos
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Pedro Henrique Oliveira dos Santos
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
I have a different point of view, the HTTP imposes us some limitations, we
will hardly have an good synchronization between the component state on
browser and server using only HTTP conversation. So it is mandatory the
service layer to respect the described security restriction.
On Tue, Nov 30, 2010 at 5:32 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> an easy example is security.
>
> a user views a page that allows them to delete another user
> meanwhile their permissions are tweaked and they can no longer delete
> other users
> half an hour later the user clicks the delete button - this should
> fail, but wont if we are using last-rendered state.
>
> -igor
>
> On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> > I need to look better on which core components are relying on an updated
> > visibility/enabled state at the process event time, and why the last
> > rendered state wouldn't be enough to them to work nicely.
> >
> > On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> currently we only invoke configure before the render. this would mean
> >> we would have to invoke it before processing a listener, clearing the
> >> cache, and then invoking it again before render. i wonder if that is
> >> enough places to invoke it....
> >>
> >> -igor
> >>
> >> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >> > If user click an link, it will change the value of some property at
> the
> >> > process_event request cycle step. Then the processor will go to the
> >> respond
> >> > step, will invoke every component before render method which will end
> up
> >> > invoking the Component#configure and updating the visibility/enabled
> >> state
> >> > (even if it changes, we are able to work with the updated state). So
> when
> >> > the this component has the opportunity to render it self, it will be
> >> aware
> >> > its update state.
> >> >
> >> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> >> >wrote:
> >> >
> >> >> there are other places that should be checked though. for example
> >> >> before we invoke a listener on the component we should check again to
> >> >> make sure that visibility hasnt changed. eg if visibility depends on
> >> >> some property of the user clicking the link that changed between
> >> >> render and clicking the link.
> >> >>
> >> >> -igor
> >> >>
> >> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com>
> >> wrote:
> >> >> > An implementation idea:
> >> >> >
> >> >> > Component {
> >> >> > public final void configure()
> >> >> > {
> >> >> > if (!getFlag(FLAG_CONFIGURED))
> >> >> > {
> >> >> > setVisible_NoClientCode(isVisible()); //we only check
> the
> >> user
> >> >> > isVisible in here
> >> >> > onConfigure();
> >> >> > setFlag(FLAG_CONFIGURED, true);
> >> >> > }
> >> >> > }
> >> >> > }
> >> >> >
> >> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
> >> igor.vaynberg@gmail.com
> >> >> >wrote:
> >> >> >
> >> >> >> so how is it different if they can still override something that
> >> needs
> >> >> >> to be checked all the time?
> >> >> >>
> >> >> >> -igor
> >> >> >>
> >> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <
> pedrosans@gmail.com>
> >> >> wrote:
> >> >> >> > I understand the concern about possible isVisible
> implementations
> >> like
> >> >> >> >
> >> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
> >> component
> >> >> >> being
> >> >> >> > rendered at 09:59:59
> >> >> >> > isVisible(return dao.list().size() > 0);// performance issues
> >> >> >> >
> >> >> >> > But maybe we can have the best from both approaches. This is an
> >> >> >> copy/paste
> >> >> >> > from java.awt.Component:
> >> >> >> >
> >> >> >> > public boolean isVisible() {
> >> >> >> > return isVisible_NoClientCode();
> >> >> >> > }
> >> >> >> > final boolean isVisible_NoClientCode() {
> >> >> >> > return visible;
> >> >> >> > }
> >> >> >> >
> >> >> >> > There are some points in the awt framework were the isVisible
> >> method
> >> >> is
> >> >> >> not
> >> >> >> > used in benefit of isVisible_NoClientCode
> >> >> >> > I'm in favor of create an final isVisible/Enabled version and
> >> change
> >> >> the
> >> >> >> > Wicket core to use it. Also maintain the hotspot to users
> provide
> >> >> their
> >> >> >> > isVisible/Enable implementations that will serve to feed the
> core
> >> >> >> component
> >> >> >> > state.
> >> >> >> >
> >> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> >> >> igor.vaynberg@gmail.com
> >> >> >> >wrote:
> >> >> >> >
> >> >> >> >> ive run into plenty of weird problems with overrides, but maybe
> >> >> >> >> because this was in a high concurrency app where data changed
> >> >> >> >> frequently. the problems arise from the fact that the value
> >> returned
> >> >> >> >> from isvisible() can change while we are doing traversals, etc.
> >> >> >> >>
> >> >> >> >> eg we run a traversal for all visible components and put them
> in a
> >> >> >> >> list. later we iterate over the list and try to render these
> >> >> >> >> components. the render function also checks their visibility
> and
> >> if
> >> >> >> >> they are no longer visible it throws an exception.
> >> >> >> >>
> >> >> >> >> if isvisible() override depends on some external factor like
> the
> >> >> >> >> database there is a small window where the value can change and
> >> now
> >> >> >> >> you can have a weird exception: such as "tried to invoke a
> >> listener
> >> >> on
> >> >> >> >> a component that is not visible or not enabled". these are very
> >> >> >> >> intermittent and damn near impossible to reproduce.
> >> >> >> >>
> >> >> >> >> another problem is performance. isvisible() is called multiple
> >> times
> >> >> >> >> during the request and if it depends on the database it can be
> a
> >> >> >> >> performance problem. in fact a couple of users have complained
> >> about
> >> >> >> >> this on the list in the past. at least now we have an easy
> >> solution
> >> >> >> >> for them - use onconfigure().
> >> >> >> >>
> >> >> >> >> so as of right now the developers have two choices: override
> >> >> >> >> isvisible() and potentially suffer the consequences. or,
> override
> >> >> >> >> onconfigure() and set visibility there in a more deterministic
> >> >> >> >> fashion.
> >> >> >> >>
> >> >> >> >> -igor
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> >> >> >> <ee...@gmail.com> wrote:
> >> >> >> >> > To expand, unless I'm missing something (new?), things are
> >> really
> >> >> only
> >> >> >> >> > problematic when both the mutable value and the override are
> >> mixed.
> >> >> In
> >> >> >> >> > a way, I think that using the override is 'more pure', as
> it's a
> >> >> >> >> > simple function that is executed when needed, whereas mutable
> >> state
> >> >> >> >> > can be harder to deal with when trying to figure out how it
> got
> >> to
> >> >> be
> >> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
> >> >> >> >> >
> >> >> >> >> > Eelco
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >> >> >> >> > <ee...@gmail.com> wrote:
> >> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
> >> just
> >> >> be
> >> >> >> >> >> aware of. We use such overrides all over the place and never
> >> have
> >> >> >> >> >> problems with them either. :-) Avoiding it is safer, but
> also
> >> more
> >> >> >> >> >> verbose (in 1.3.x at least).
> >> >> >> >> >>
> >> >> >> >> >> Eelco
> >> >> >> >> >>
> >> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >> >> >> igor.vaynberg@gmail.com>
> >> >> >> >> wrote:
> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
> sven@meiers.net>
> >> >> >> wrote:
> >> >> >> >> >>>> Hi Douglas,
> >> >> >> >> >>>>
> >> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility
> of
> >> a
> >> >> >> >> >>>> component changes while its form is being processed.
> >> >> >> >> >>>> In our projects we're overriding isVisible() where
> >> appropriate
> >> >> and
> >> >> >> >> never
> >> >> >> >> >>>> encountered a similar problem.
> >> >> >> >> >>>>
> >> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next,
> is
> >> >> >> overriding
> >> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >> >> >> >> >>>
> >> >> >> >> >>> yes
> >> >> >> >> >>>
> >> >> >> >> >>> -igor
> >> >> >> >> >>>
> >> >> >> >> >>>>
> >> >> >> >> >>>> Sven
> >> >> >> >> >>>>
> >> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >> >> >> >> >>>>
> >> >> >> >> >>>>> Can you explain why? We have done this all over the
> place.
> >> >> >> >> >>>>>
> >> >> >> >> >>>>> D/
> >> >> >> >> >>>>>
> >> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >> >> >> >> >>>>>
> >> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
> >> override
> >> >> >> >> onConfigure()
> >> >> >> >> >>>>> > and call setVisible(true|false) depending on your
> >> conditions.
> >> >> >> >> >>>>> >
> >> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
> >> >> >> >> >>>>> >
> >> >> >> >> >>>>> >> Igor posted a comment to this bug saying that
> overriding
> >> >> >> >> isVisible() is
> >> >> >> >> >>>>> >> "evil"
> >> >> >> >> >>>>> >>
> >> >> >> >> >>>>> >>
> https://issues.apache.org/jira/browse/WICKET-3171
> >> >> >> >> >>>>> >>
> >> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
> >> >> >> >> >>>>> >>
> >> >> >> >> >>>>> >> D/
> >> >> >> >> >>>>>
> >> >> >> >> >>>>
> >> >> >> >> >>>>
> >> >> >> >> >>>>
> >> >> >> >> >>>
> >> >> >> >> >>
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> > --
> >> >> >> > Pedro Henrique Oliveira dos Santos
> >> >> >> >
> >> >> >>
> >> >> >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Pedro Henrique Oliveira dos Santos
> >> >> >
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Pedro Henrique Oliveira dos Santos
> >> >
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
an easy example is security.
a user views a page that allows them to delete another user
meanwhile their permissions are tweaked and they can no longer delete
other users
half an hour later the user clicks the delete button - this should
fail, but wont if we are using last-rendered state.
-igor
On Tue, Nov 30, 2010 at 11:18 AM, Pedro Santos <pe...@gmail.com> wrote:
> I need to look better on which core components are relying on an updated
> visibility/enabled state at the process event time, and why the last
> rendered state wouldn't be enough to them to work nicely.
>
> On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> currently we only invoke configure before the render. this would mean
>> we would have to invoke it before processing a listener, clearing the
>> cache, and then invoking it again before render. i wonder if that is
>> enough places to invoke it....
>>
>> -igor
>>
>> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com> wrote:
>> > If user click an link, it will change the value of some property at the
>> > process_event request cycle step. Then the processor will go to the
>> respond
>> > step, will invoke every component before render method which will end up
>> > invoking the Component#configure and updating the visibility/enabled
>> state
>> > (even if it changes, we are able to work with the updated state). So when
>> > the this component has the opportunity to render it self, it will be
>> aware
>> > its update state.
>> >
>> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>> >wrote:
>> >
>> >> there are other places that should be checked though. for example
>> >> before we invoke a listener on the component we should check again to
>> >> make sure that visibility hasnt changed. eg if visibility depends on
>> >> some property of the user clicking the link that changed between
>> >> render and clicking the link.
>> >>
>> >> -igor
>> >>
>> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com>
>> wrote:
>> >> > An implementation idea:
>> >> >
>> >> > Component {
>> >> > public final void configure()
>> >> > {
>> >> > if (!getFlag(FLAG_CONFIGURED))
>> >> > {
>> >> > setVisible_NoClientCode(isVisible()); //we only check the
>> user
>> >> > isVisible in here
>> >> > onConfigure();
>> >> > setFlag(FLAG_CONFIGURED, true);
>> >> > }
>> >> > }
>> >> > }
>> >> >
>> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
>> igor.vaynberg@gmail.com
>> >> >wrote:
>> >> >
>> >> >> so how is it different if they can still override something that
>> needs
>> >> >> to be checked all the time?
>> >> >>
>> >> >> -igor
>> >> >>
>> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com>
>> >> wrote:
>> >> >> > I understand the concern about possible isVisible implementations
>> like
>> >> >> >
>> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
>> component
>> >> >> being
>> >> >> > rendered at 09:59:59
>> >> >> > isVisible(return dao.list().size() > 0);// performance issues
>> >> >> >
>> >> >> > But maybe we can have the best from both approaches. This is an
>> >> >> copy/paste
>> >> >> > from java.awt.Component:
>> >> >> >
>> >> >> > public boolean isVisible() {
>> >> >> > return isVisible_NoClientCode();
>> >> >> > }
>> >> >> > final boolean isVisible_NoClientCode() {
>> >> >> > return visible;
>> >> >> > }
>> >> >> >
>> >> >> > There are some points in the awt framework were the isVisible
>> method
>> >> is
>> >> >> not
>> >> >> > used in benefit of isVisible_NoClientCode
>> >> >> > I'm in favor of create an final isVisible/Enabled version and
>> change
>> >> the
>> >> >> > Wicket core to use it. Also maintain the hotspot to users provide
>> >> their
>> >> >> > isVisible/Enable implementations that will serve to feed the core
>> >> >> component
>> >> >> > state.
>> >> >> >
>> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>> >> igor.vaynberg@gmail.com
>> >> >> >wrote:
>> >> >> >
>> >> >> >> ive run into plenty of weird problems with overrides, but maybe
>> >> >> >> because this was in a high concurrency app where data changed
>> >> >> >> frequently. the problems arise from the fact that the value
>> returned
>> >> >> >> from isvisible() can change while we are doing traversals, etc.
>> >> >> >>
>> >> >> >> eg we run a traversal for all visible components and put them in a
>> >> >> >> list. later we iterate over the list and try to render these
>> >> >> >> components. the render function also checks their visibility and
>> if
>> >> >> >> they are no longer visible it throws an exception.
>> >> >> >>
>> >> >> >> if isvisible() override depends on some external factor like the
>> >> >> >> database there is a small window where the value can change and
>> now
>> >> >> >> you can have a weird exception: such as "tried to invoke a
>> listener
>> >> on
>> >> >> >> a component that is not visible or not enabled". these are very
>> >> >> >> intermittent and damn near impossible to reproduce.
>> >> >> >>
>> >> >> >> another problem is performance. isvisible() is called multiple
>> times
>> >> >> >> during the request and if it depends on the database it can be a
>> >> >> >> performance problem. in fact a couple of users have complained
>> about
>> >> >> >> this on the list in the past. at least now we have an easy
>> solution
>> >> >> >> for them - use onconfigure().
>> >> >> >>
>> >> >> >> so as of right now the developers have two choices: override
>> >> >> >> isvisible() and potentially suffer the consequences. or, override
>> >> >> >> onconfigure() and set visibility there in a more deterministic
>> >> >> >> fashion.
>> >> >> >>
>> >> >> >> -igor
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> >> >> >> <ee...@gmail.com> wrote:
>> >> >> >> > To expand, unless I'm missing something (new?), things are
>> really
>> >> only
>> >> >> >> > problematic when both the mutable value and the override are
>> mixed.
>> >> In
>> >> >> >> > a way, I think that using the override is 'more pure', as it's a
>> >> >> >> > simple function that is executed when needed, whereas mutable
>> state
>> >> >> >> > can be harder to deal with when trying to figure out how it got
>> to
>> >> be
>> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
>> >> >> >> >
>> >> >> >> > Eelco
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> >> >> >> > <ee...@gmail.com> wrote:
>> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
>> just
>> >> be
>> >> >> >> >> aware of. We use such overrides all over the place and never
>> have
>> >> >> >> >> problems with them either. :-) Avoiding it is safer, but also
>> more
>> >> >> >> >> verbose (in 1.3.x at least).
>> >> >> >> >>
>> >> >> >> >> Eelco
>> >> >> >> >>
>> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>> >> >> igor.vaynberg@gmail.com>
>> >> >> >> wrote:
>> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
>> >> >> wrote:
>> >> >> >> >>>> Hi Douglas,
>> >> >> >> >>>>
>> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of
>> a
>> >> >> >> >>>> component changes while its form is being processed.
>> >> >> >> >>>> In our projects we're overriding isVisible() where
>> appropriate
>> >> and
>> >> >> >> never
>> >> >> >> >>>> encountered a similar problem.
>> >> >> >> >>>>
>> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
>> >> >> overriding
>> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
>> >> >> >> >>>
>> >> >> >> >>> yes
>> >> >> >> >>>
>> >> >> >> >>> -igor
>> >> >> >> >>>
>> >> >> >> >>>>
>> >> >> >> >>>> Sven
>> >> >> >> >>>>
>> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>> >> >> >> >>>>
>> >> >> >> >>>>> Can you explain why? We have done this all over the place.
>> >> >> >> >>>>>
>> >> >> >> >>>>> D/
>> >> >> >> >>>>>
>> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >> >> >> >>>>>
>> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
>> override
>> >> >> >> onConfigure()
>> >> >> >> >>>>> > and call setVisible(true|false) depending on your
>> conditions.
>> >> >> >> >>>>> >
>> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>> >> >> >> >>>>> >
>> >> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding
>> >> >> >> isVisible() is
>> >> >> >> >>>>> >> "evil"
>> >> >> >> >>>>> >>
>> >> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>> >> >> >> >>>>> >>
>> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
>> >> >> >> >>>>> >>
>> >> >> >> >>>>> >> D/
>> >> >> >> >>>>>
>> >> >> >> >>>>
>> >> >> >> >>>>
>> >> >> >> >>>>
>> >> >> >> >>>
>> >> >> >> >>
>> >> >> >> >
>> >> >> >>
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> > --
>> >> >> > Pedro Henrique Oliveira dos Santos
>> >> >> >
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Pedro Henrique Oliveira dos Santos
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Pedro Henrique Oliveira dos Santos
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
I need to look better on which core components are relying on an updated
visibility/enabled state at the process event time, and why the last
rendered state wouldn't be enough to them to work nicely.
On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> currently we only invoke configure before the render. this would mean
> we would have to invoke it before processing a listener, clearing the
> cache, and then invoking it again before render. i wonder if that is
> enough places to invoke it....
>
> -igor
>
> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com> wrote:
> > If user click an link, it will change the value of some property at the
> > process_event request cycle step. Then the processor will go to the
> respond
> > step, will invoke every component before render method which will end up
> > invoking the Component#configure and updating the visibility/enabled
> state
> > (even if it changes, we are able to work with the updated state). So when
> > the this component has the opportunity to render it self, it will be
> aware
> > its update state.
> >
> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> there are other places that should be checked though. for example
> >> before we invoke a listener on the component we should check again to
> >> make sure that visibility hasnt changed. eg if visibility depends on
> >> some property of the user clicking the link that changed between
> >> render and clicking the link.
> >>
> >> -igor
> >>
> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >> > An implementation idea:
> >> >
> >> > Component {
> >> > public final void configure()
> >> > {
> >> > if (!getFlag(FLAG_CONFIGURED))
> >> > {
> >> > setVisible_NoClientCode(isVisible()); //we only check the
> user
> >> > isVisible in here
> >> > onConfigure();
> >> > setFlag(FLAG_CONFIGURED, true);
> >> > }
> >> > }
> >> > }
> >> >
> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> >> >wrote:
> >> >
> >> >> so how is it different if they can still override something that
> needs
> >> >> to be checked all the time?
> >> >>
> >> >> -igor
> >> >>
> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com>
> >> wrote:
> >> >> > I understand the concern about possible isVisible implementations
> like
> >> >> >
> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
> component
> >> >> being
> >> >> > rendered at 09:59:59
> >> >> > isVisible(return dao.list().size() > 0);// performance issues
> >> >> >
> >> >> > But maybe we can have the best from both approaches. This is an
> >> >> copy/paste
> >> >> > from java.awt.Component:
> >> >> >
> >> >> > public boolean isVisible() {
> >> >> > return isVisible_NoClientCode();
> >> >> > }
> >> >> > final boolean isVisible_NoClientCode() {
> >> >> > return visible;
> >> >> > }
> >> >> >
> >> >> > There are some points in the awt framework were the isVisible
> method
> >> is
> >> >> not
> >> >> > used in benefit of isVisible_NoClientCode
> >> >> > I'm in favor of create an final isVisible/Enabled version and
> change
> >> the
> >> >> > Wicket core to use it. Also maintain the hotspot to users provide
> >> their
> >> >> > isVisible/Enable implementations that will serve to feed the core
> >> >> component
> >> >> > state.
> >> >> >
> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> >> igor.vaynberg@gmail.com
> >> >> >wrote:
> >> >> >
> >> >> >> ive run into plenty of weird problems with overrides, but maybe
> >> >> >> because this was in a high concurrency app where data changed
> >> >> >> frequently. the problems arise from the fact that the value
> returned
> >> >> >> from isvisible() can change while we are doing traversals, etc.
> >> >> >>
> >> >> >> eg we run a traversal for all visible components and put them in a
> >> >> >> list. later we iterate over the list and try to render these
> >> >> >> components. the render function also checks their visibility and
> if
> >> >> >> they are no longer visible it throws an exception.
> >> >> >>
> >> >> >> if isvisible() override depends on some external factor like the
> >> >> >> database there is a small window where the value can change and
> now
> >> >> >> you can have a weird exception: such as "tried to invoke a
> listener
> >> on
> >> >> >> a component that is not visible or not enabled". these are very
> >> >> >> intermittent and damn near impossible to reproduce.
> >> >> >>
> >> >> >> another problem is performance. isvisible() is called multiple
> times
> >> >> >> during the request and if it depends on the database it can be a
> >> >> >> performance problem. in fact a couple of users have complained
> about
> >> >> >> this on the list in the past. at least now we have an easy
> solution
> >> >> >> for them - use onconfigure().
> >> >> >>
> >> >> >> so as of right now the developers have two choices: override
> >> >> >> isvisible() and potentially suffer the consequences. or, override
> >> >> >> onconfigure() and set visibility there in a more deterministic
> >> >> >> fashion.
> >> >> >>
> >> >> >> -igor
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> >> >> <ee...@gmail.com> wrote:
> >> >> >> > To expand, unless I'm missing something (new?), things are
> really
> >> only
> >> >> >> > problematic when both the mutable value and the override are
> mixed.
> >> In
> >> >> >> > a way, I think that using the override is 'more pure', as it's a
> >> >> >> > simple function that is executed when needed, whereas mutable
> state
> >> >> >> > can be harder to deal with when trying to figure out how it got
> to
> >> be
> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
> >> >> >> >
> >> >> >> > Eelco
> >> >> >> >
> >> >> >> >
> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >> >> >> > <ee...@gmail.com> wrote:
> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
> just
> >> be
> >> >> >> >> aware of. We use such overrides all over the place and never
> have
> >> >> >> >> problems with them either. :-) Avoiding it is safer, but also
> more
> >> >> >> >> verbose (in 1.3.x at least).
> >> >> >> >>
> >> >> >> >> Eelco
> >> >> >> >>
> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >> >> igor.vaynberg@gmail.com>
> >> >> >> wrote:
> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
> >> >> wrote:
> >> >> >> >>>> Hi Douglas,
> >> >> >> >>>>
> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of
> a
> >> >> >> >>>> component changes while its form is being processed.
> >> >> >> >>>> In our projects we're overriding isVisible() where
> appropriate
> >> and
> >> >> >> never
> >> >> >> >>>> encountered a similar problem.
> >> >> >> >>>>
> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
> >> >> overriding
> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >> >> >> >>>
> >> >> >> >>> yes
> >> >> >> >>>
> >> >> >> >>> -igor
> >> >> >> >>>
> >> >> >> >>>>
> >> >> >> >>>> Sven
> >> >> >> >>>>
> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >> >> >> >>>>
> >> >> >> >>>>> Can you explain why? We have done this all over the place.
> >> >> >> >>>>>
> >> >> >> >>>>> D/
> >> >> >> >>>>>
> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >> >> >> >>>>>
> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
> override
> >> >> >> onConfigure()
> >> >> >> >>>>> > and call setVisible(true|false) depending on your
> conditions.
> >> >> >> >>>>> >
> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >> >> >> >>>>> > douglas@douglasferguson.us> wrote:
> >> >> >> >>>>> >
> >> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding
> >> >> >> isVisible() is
> >> >> >> >>>>> >> "evil"
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> D/
> >> >> >> >>>>>
> >> >> >> >>>>
> >> >> >> >>>>
> >> >> >> >>>>
> >> >> >> >>>
> >> >> >> >>
> >> >> >> >
> >> >> >>
> >> >> >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Pedro Henrique Oliveira dos Santos
> >> >> >
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Pedro Henrique Oliveira dos Santos
> >> >
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
currently we only invoke configure before the render. this would mean
we would have to invoke it before processing a listener, clearing the
cache, and then invoking it again before render. i wonder if that is
enough places to invoke it....
-igor
On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pe...@gmail.com> wrote:
> If user click an link, it will change the value of some property at the
> process_event request cycle step. Then the processor will go to the respond
> step, will invoke every component before render method which will end up
> invoking the Component#configure and updating the visibility/enabled state
> (even if it changes, we are able to work with the updated state). So when
> the this component has the opportunity to render it self, it will be aware
> its update state.
>
> On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> there are other places that should be checked though. for example
>> before we invoke a listener on the component we should check again to
>> make sure that visibility hasnt changed. eg if visibility depends on
>> some property of the user clicking the link that changed between
>> render and clicking the link.
>>
>> -igor
>>
>> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com> wrote:
>> > An implementation idea:
>> >
>> > Component {
>> > public final void configure()
>> > {
>> > if (!getFlag(FLAG_CONFIGURED))
>> > {
>> > setVisible_NoClientCode(isVisible()); //we only check the user
>> > isVisible in here
>> > onConfigure();
>> > setFlag(FLAG_CONFIGURED, true);
>> > }
>> > }
>> > }
>> >
>> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>> >wrote:
>> >
>> >> so how is it different if they can still override something that needs
>> >> to be checked all the time?
>> >>
>> >> -igor
>> >>
>> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com>
>> wrote:
>> >> > I understand the concern about possible isVisible implementations like
>> >> >
>> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this component
>> >> being
>> >> > rendered at 09:59:59
>> >> > isVisible(return dao.list().size() > 0);// performance issues
>> >> >
>> >> > But maybe we can have the best from both approaches. This is an
>> >> copy/paste
>> >> > from java.awt.Component:
>> >> >
>> >> > public boolean isVisible() {
>> >> > return isVisible_NoClientCode();
>> >> > }
>> >> > final boolean isVisible_NoClientCode() {
>> >> > return visible;
>> >> > }
>> >> >
>> >> > There are some points in the awt framework were the isVisible method
>> is
>> >> not
>> >> > used in benefit of isVisible_NoClientCode
>> >> > I'm in favor of create an final isVisible/Enabled version and change
>> the
>> >> > Wicket core to use it. Also maintain the hotspot to users provide
>> their
>> >> > isVisible/Enable implementations that will serve to feed the core
>> >> component
>> >> > state.
>> >> >
>> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
>> igor.vaynberg@gmail.com
>> >> >wrote:
>> >> >
>> >> >> ive run into plenty of weird problems with overrides, but maybe
>> >> >> because this was in a high concurrency app where data changed
>> >> >> frequently. the problems arise from the fact that the value returned
>> >> >> from isvisible() can change while we are doing traversals, etc.
>> >> >>
>> >> >> eg we run a traversal for all visible components and put them in a
>> >> >> list. later we iterate over the list and try to render these
>> >> >> components. the render function also checks their visibility and if
>> >> >> they are no longer visible it throws an exception.
>> >> >>
>> >> >> if isvisible() override depends on some external factor like the
>> >> >> database there is a small window where the value can change and now
>> >> >> you can have a weird exception: such as "tried to invoke a listener
>> on
>> >> >> a component that is not visible or not enabled". these are very
>> >> >> intermittent and damn near impossible to reproduce.
>> >> >>
>> >> >> another problem is performance. isvisible() is called multiple times
>> >> >> during the request and if it depends on the database it can be a
>> >> >> performance problem. in fact a couple of users have complained about
>> >> >> this on the list in the past. at least now we have an easy solution
>> >> >> for them - use onconfigure().
>> >> >>
>> >> >> so as of right now the developers have two choices: override
>> >> >> isvisible() and potentially suffer the consequences. or, override
>> >> >> onconfigure() and set visibility there in a more deterministic
>> >> >> fashion.
>> >> >>
>> >> >> -igor
>> >> >>
>> >> >>
>> >> >>
>> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> >> >> <ee...@gmail.com> wrote:
>> >> >> > To expand, unless I'm missing something (new?), things are really
>> only
>> >> >> > problematic when both the mutable value and the override are mixed.
>> In
>> >> >> > a way, I think that using the override is 'more pure', as it's a
>> >> >> > simple function that is executed when needed, whereas mutable state
>> >> >> > can be harder to deal with when trying to figure out how it got to
>> be
>> >> >> > in that state. So, sorry Igor, but we disagree on this one.
>> >> >> >
>> >> >> > Eelco
>> >> >> >
>> >> >> >
>> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> >> >> > <ee...@gmail.com> wrote:
>> >> >> >> Niether is evil. It has potential pitfalls, which you should just
>> be
>> >> >> >> aware of. We use such overrides all over the place and never have
>> >> >> >> problems with them either. :-) Avoiding it is safer, but also more
>> >> >> >> verbose (in 1.3.x at least).
>> >> >> >>
>> >> >> >> Eelco
>> >> >> >>
>> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>> >> igor.vaynberg@gmail.com>
>> >> >> wrote:
>> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
>> >> wrote:
>> >> >> >>>> Hi Douglas,
>> >> >> >>>>
>> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of a
>> >> >> >>>> component changes while its form is being processed.
>> >> >> >>>> In our projects we're overriding isVisible() where appropriate
>> and
>> >> >> never
>> >> >> >>>> encountered a similar problem.
>> >> >> >>>>
>> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
>> >> overriding
>> >> >> >>>> isEnabled() going to be declared evil too? ;)
>> >> >> >>>
>> >> >> >>> yes
>> >> >> >>>
>> >> >> >>> -igor
>> >> >> >>>
>> >> >> >>>>
>> >> >> >>>> Sven
>> >> >> >>>>
>> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>> >> >> >>>>
>> >> >> >>>>> Can you explain why? We have done this all over the place.
>> >> >> >>>>>
>> >> >> >>>>> D/
>> >> >> >>>>>
>> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >> >> >>>>>
>> >> >> >>>>> > The recommended way since a few 1.4 releases is to override
>> >> >> onConfigure()
>> >> >> >>>>> > and call setVisible(true|false) depending on your conditions.
>> >> >> >>>>> >
>> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >> >> >>>>> > douglas@douglasferguson.us> wrote:
>> >> >> >>>>> >
>> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding
>> >> >> isVisible() is
>> >> >> >>>>> >> "evil"
>> >> >> >>>>> >>
>> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>> >> >> >>>>> >>
>> >> >> >>>>> >> I was surprised by this and am curious to hear more.
>> >> >> >>>>> >>
>> >> >> >>>>> >> D/
>> >> >> >>>>>
>> >> >> >>>>
>> >> >> >>>>
>> >> >> >>>>
>> >> >> >>>
>> >> >> >>
>> >> >> >
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Pedro Henrique Oliveira dos Santos
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Pedro Henrique Oliveira dos Santos
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
If user click an link, it will change the value of some property at the
process_event request cycle step. Then the processor will go to the respond
step, will invoke every component before render method which will end up
invoking the Component#configure and updating the visibility/enabled state
(even if it changes, we are able to work with the updated state). So when
the this component has the opportunity to render it self, it will be aware
its update state.
On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> there are other places that should be checked though. for example
> before we invoke a listener on the component we should check again to
> make sure that visibility hasnt changed. eg if visibility depends on
> some property of the user clicking the link that changed between
> render and clicking the link.
>
> -igor
>
> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com> wrote:
> > An implementation idea:
> >
> > Component {
> > public final void configure()
> > {
> > if (!getFlag(FLAG_CONFIGURED))
> > {
> > setVisible_NoClientCode(isVisible()); //we only check the user
> > isVisible in here
> > onConfigure();
> > setFlag(FLAG_CONFIGURED, true);
> > }
> > }
> > }
> >
> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> so how is it different if they can still override something that needs
> >> to be checked all the time?
> >>
> >> -igor
> >>
> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com>
> wrote:
> >> > I understand the concern about possible isVisible implementations like
> >> >
> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this component
> >> being
> >> > rendered at 09:59:59
> >> > isVisible(return dao.list().size() > 0);// performance issues
> >> >
> >> > But maybe we can have the best from both approaches. This is an
> >> copy/paste
> >> > from java.awt.Component:
> >> >
> >> > public boolean isVisible() {
> >> > return isVisible_NoClientCode();
> >> > }
> >> > final boolean isVisible_NoClientCode() {
> >> > return visible;
> >> > }
> >> >
> >> > There are some points in the awt framework were the isVisible method
> is
> >> not
> >> > used in benefit of isVisible_NoClientCode
> >> > I'm in favor of create an final isVisible/Enabled version and change
> the
> >> > Wicket core to use it. Also maintain the hotspot to users provide
> their
> >> > isVisible/Enable implementations that will serve to feed the core
> >> component
> >> > state.
> >> >
> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> igor.vaynberg@gmail.com
> >> >wrote:
> >> >
> >> >> ive run into plenty of weird problems with overrides, but maybe
> >> >> because this was in a high concurrency app where data changed
> >> >> frequently. the problems arise from the fact that the value returned
> >> >> from isvisible() can change while we are doing traversals, etc.
> >> >>
> >> >> eg we run a traversal for all visible components and put them in a
> >> >> list. later we iterate over the list and try to render these
> >> >> components. the render function also checks their visibility and if
> >> >> they are no longer visible it throws an exception.
> >> >>
> >> >> if isvisible() override depends on some external factor like the
> >> >> database there is a small window where the value can change and now
> >> >> you can have a weird exception: such as "tried to invoke a listener
> on
> >> >> a component that is not visible or not enabled". these are very
> >> >> intermittent and damn near impossible to reproduce.
> >> >>
> >> >> another problem is performance. isvisible() is called multiple times
> >> >> during the request and if it depends on the database it can be a
> >> >> performance problem. in fact a couple of users have complained about
> >> >> this on the list in the past. at least now we have an easy solution
> >> >> for them - use onconfigure().
> >> >>
> >> >> so as of right now the developers have two choices: override
> >> >> isvisible() and potentially suffer the consequences. or, override
> >> >> onconfigure() and set visibility there in a more deterministic
> >> >> fashion.
> >> >>
> >> >> -igor
> >> >>
> >> >>
> >> >>
> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> >> <ee...@gmail.com> wrote:
> >> >> > To expand, unless I'm missing something (new?), things are really
> only
> >> >> > problematic when both the mutable value and the override are mixed.
> In
> >> >> > a way, I think that using the override is 'more pure', as it's a
> >> >> > simple function that is executed when needed, whereas mutable state
> >> >> > can be harder to deal with when trying to figure out how it got to
> be
> >> >> > in that state. So, sorry Igor, but we disagree on this one.
> >> >> >
> >> >> > Eelco
> >> >> >
> >> >> >
> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >> >> > <ee...@gmail.com> wrote:
> >> >> >> Niether is evil. It has potential pitfalls, which you should just
> be
> >> >> >> aware of. We use such overrides all over the place and never have
> >> >> >> problems with them either. :-) Avoiding it is safer, but also more
> >> >> >> verbose (in 1.3.x at least).
> >> >> >>
> >> >> >> Eelco
> >> >> >>
> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >> igor.vaynberg@gmail.com>
> >> >> wrote:
> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
> >> wrote:
> >> >> >>>> Hi Douglas,
> >> >> >>>>
> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of a
> >> >> >>>> component changes while its form is being processed.
> >> >> >>>> In our projects we're overriding isVisible() where appropriate
> and
> >> >> never
> >> >> >>>> encountered a similar problem.
> >> >> >>>>
> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
> >> overriding
> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >> >> >>>
> >> >> >>> yes
> >> >> >>>
> >> >> >>> -igor
> >> >> >>>
> >> >> >>>>
> >> >> >>>> Sven
> >> >> >>>>
> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >> >> >>>>
> >> >> >>>>> Can you explain why? We have done this all over the place.
> >> >> >>>>>
> >> >> >>>>> D/
> >> >> >>>>>
> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >> >> >>>>>
> >> >> >>>>> > The recommended way since a few 1.4 releases is to override
> >> >> onConfigure()
> >> >> >>>>> > and call setVisible(true|false) depending on your conditions.
> >> >> >>>>> >
> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >> >> >>>>> > douglas@douglasferguson.us> wrote:
> >> >> >>>>> >
> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding
> >> >> isVisible() is
> >> >> >>>>> >> "evil"
> >> >> >>>>> >>
> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >> >> >>>>> >>
> >> >> >>>>> >> I was surprised by this and am curious to hear more.
> >> >> >>>>> >>
> >> >> >>>>> >> D/
> >> >> >>>>>
> >> >> >>>>
> >> >> >>>>
> >> >> >>>>
> >> >> >>>
> >> >> >>
> >> >> >
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Pedro Henrique Oliveira dos Santos
> >> >
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
there are other places that should be checked though. for example
before we invoke a listener on the component we should check again to
make sure that visibility hasnt changed. eg if visibility depends on
some property of the user clicking the link that changed between
render and clicking the link.
-igor
On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pe...@gmail.com> wrote:
> An implementation idea:
>
> Component {
> public final void configure()
> {
> if (!getFlag(FLAG_CONFIGURED))
> {
> setVisible_NoClientCode(isVisible()); //we only check the user
> isVisible in here
> onConfigure();
> setFlag(FLAG_CONFIGURED, true);
> }
> }
> }
>
> On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> so how is it different if they can still override something that needs
>> to be checked all the time?
>>
>> -igor
>>
>> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com> wrote:
>> > I understand the concern about possible isVisible implementations like
>> >
>> > isVisible(return currentlyTime < 10:00:00;) //imagine this component
>> being
>> > rendered at 09:59:59
>> > isVisible(return dao.list().size() > 0);// performance issues
>> >
>> > But maybe we can have the best from both approaches. This is an
>> copy/paste
>> > from java.awt.Component:
>> >
>> > public boolean isVisible() {
>> > return isVisible_NoClientCode();
>> > }
>> > final boolean isVisible_NoClientCode() {
>> > return visible;
>> > }
>> >
>> > There are some points in the awt framework were the isVisible method is
>> not
>> > used in benefit of isVisible_NoClientCode
>> > I'm in favor of create an final isVisible/Enabled version and change the
>> > Wicket core to use it. Also maintain the hotspot to users provide their
>> > isVisible/Enable implementations that will serve to feed the core
>> component
>> > state.
>> >
>> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <igor.vaynberg@gmail.com
>> >wrote:
>> >
>> >> ive run into plenty of weird problems with overrides, but maybe
>> >> because this was in a high concurrency app where data changed
>> >> frequently. the problems arise from the fact that the value returned
>> >> from isvisible() can change while we are doing traversals, etc.
>> >>
>> >> eg we run a traversal for all visible components and put them in a
>> >> list. later we iterate over the list and try to render these
>> >> components. the render function also checks their visibility and if
>> >> they are no longer visible it throws an exception.
>> >>
>> >> if isvisible() override depends on some external factor like the
>> >> database there is a small window where the value can change and now
>> >> you can have a weird exception: such as "tried to invoke a listener on
>> >> a component that is not visible or not enabled". these are very
>> >> intermittent and damn near impossible to reproduce.
>> >>
>> >> another problem is performance. isvisible() is called multiple times
>> >> during the request and if it depends on the database it can be a
>> >> performance problem. in fact a couple of users have complained about
>> >> this on the list in the past. at least now we have an easy solution
>> >> for them - use onconfigure().
>> >>
>> >> so as of right now the developers have two choices: override
>> >> isvisible() and potentially suffer the consequences. or, override
>> >> onconfigure() and set visibility there in a more deterministic
>> >> fashion.
>> >>
>> >> -igor
>> >>
>> >>
>> >>
>> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> >> <ee...@gmail.com> wrote:
>> >> > To expand, unless I'm missing something (new?), things are really only
>> >> > problematic when both the mutable value and the override are mixed. In
>> >> > a way, I think that using the override is 'more pure', as it's a
>> >> > simple function that is executed when needed, whereas mutable state
>> >> > can be harder to deal with when trying to figure out how it got to be
>> >> > in that state. So, sorry Igor, but we disagree on this one.
>> >> >
>> >> > Eelco
>> >> >
>> >> >
>> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> >> > <ee...@gmail.com> wrote:
>> >> >> Niether is evil. It has potential pitfalls, which you should just be
>> >> >> aware of. We use such overrides all over the place and never have
>> >> >> problems with them either. :-) Avoiding it is safer, but also more
>> >> >> verbose (in 1.3.x at least).
>> >> >>
>> >> >> Eelco
>> >> >>
>> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
>> igor.vaynberg@gmail.com>
>> >> wrote:
>> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
>> wrote:
>> >> >>>> Hi Douglas,
>> >> >>>>
>> >> >>>> WICKET-3171 describes a problematic case, where visibility of a
>> >> >>>> component changes while its form is being processed.
>> >> >>>> In our projects we're overriding isVisible() where appropriate and
>> >> never
>> >> >>>> encountered a similar problem.
>> >> >>>>
>> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
>> overriding
>> >> >>>> isEnabled() going to be declared evil too? ;)
>> >> >>>
>> >> >>> yes
>> >> >>>
>> >> >>> -igor
>> >> >>>
>> >> >>>>
>> >> >>>> Sven
>> >> >>>>
>> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>> >> >>>>
>> >> >>>>> Can you explain why? We have done this all over the place.
>> >> >>>>>
>> >> >>>>> D/
>> >> >>>>>
>> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >> >>>>>
>> >> >>>>> > The recommended way since a few 1.4 releases is to override
>> >> onConfigure()
>> >> >>>>> > and call setVisible(true|false) depending on your conditions.
>> >> >>>>> >
>> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >> >>>>> > douglas@douglasferguson.us> wrote:
>> >> >>>>> >
>> >> >>>>> >> Igor posted a comment to this bug saying that overriding
>> >> isVisible() is
>> >> >>>>> >> "evil"
>> >> >>>>> >>
>> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>> >> >>>>> >>
>> >> >>>>> >> I was surprised by this and am curious to hear more.
>> >> >>>>> >>
>> >> >>>>> >> D/
>> >> >>>>>
>> >> >>>>
>> >> >>>>
>> >> >>>>
>> >> >>>
>> >> >>
>> >> >
>> >>
>> >
>> >
>> >
>> > --
>> > Pedro Henrique Oliveira dos Santos
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
An implementation idea:
Component {
public final void configure()
{
if (!getFlag(FLAG_CONFIGURED))
{
setVisible_NoClientCode(isVisible()); //we only check the user
isVisible in here
onConfigure();
setFlag(FLAG_CONFIGURED, true);
}
}
}
On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> so how is it different if they can still override something that needs
> to be checked all the time?
>
> -igor
>
> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com> wrote:
> > I understand the concern about possible isVisible implementations like
> >
> > isVisible(return currentlyTime < 10:00:00;) //imagine this component
> being
> > rendered at 09:59:59
> > isVisible(return dao.list().size() > 0);// performance issues
> >
> > But maybe we can have the best from both approaches. This is an
> copy/paste
> > from java.awt.Component:
> >
> > public boolean isVisible() {
> > return isVisible_NoClientCode();
> > }
> > final boolean isVisible_NoClientCode() {
> > return visible;
> > }
> >
> > There are some points in the awt framework were the isVisible method is
> not
> > used in benefit of isVisible_NoClientCode
> > I'm in favor of create an final isVisible/Enabled version and change the
> > Wicket core to use it. Also maintain the hotspot to users provide their
> > isVisible/Enable implementations that will serve to feed the core
> component
> > state.
> >
> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> ive run into plenty of weird problems with overrides, but maybe
> >> because this was in a high concurrency app where data changed
> >> frequently. the problems arise from the fact that the value returned
> >> from isvisible() can change while we are doing traversals, etc.
> >>
> >> eg we run a traversal for all visible components and put them in a
> >> list. later we iterate over the list and try to render these
> >> components. the render function also checks their visibility and if
> >> they are no longer visible it throws an exception.
> >>
> >> if isvisible() override depends on some external factor like the
> >> database there is a small window where the value can change and now
> >> you can have a weird exception: such as "tried to invoke a listener on
> >> a component that is not visible or not enabled". these are very
> >> intermittent and damn near impossible to reproduce.
> >>
> >> another problem is performance. isvisible() is called multiple times
> >> during the request and if it depends on the database it can be a
> >> performance problem. in fact a couple of users have complained about
> >> this on the list in the past. at least now we have an easy solution
> >> for them - use onconfigure().
> >>
> >> so as of right now the developers have two choices: override
> >> isvisible() and potentially suffer the consequences. or, override
> >> onconfigure() and set visibility there in a more deterministic
> >> fashion.
> >>
> >> -igor
> >>
> >>
> >>
> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> <ee...@gmail.com> wrote:
> >> > To expand, unless I'm missing something (new?), things are really only
> >> > problematic when both the mutable value and the override are mixed. In
> >> > a way, I think that using the override is 'more pure', as it's a
> >> > simple function that is executed when needed, whereas mutable state
> >> > can be harder to deal with when trying to figure out how it got to be
> >> > in that state. So, sorry Igor, but we disagree on this one.
> >> >
> >> > Eelco
> >> >
> >> >
> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >> > <ee...@gmail.com> wrote:
> >> >> Niether is evil. It has potential pitfalls, which you should just be
> >> >> aware of. We use such overrides all over the place and never have
> >> >> problems with them either. :-) Avoiding it is safer, but also more
> >> >> verbose (in 1.3.x at least).
> >> >>
> >> >> Eelco
> >> >>
> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> igor.vaynberg@gmail.com>
> >> wrote:
> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net>
> wrote:
> >> >>>> Hi Douglas,
> >> >>>>
> >> >>>> WICKET-3171 describes a problematic case, where visibility of a
> >> >>>> component changes while its form is being processed.
> >> >>>> In our projects we're overriding isVisible() where appropriate and
> >> never
> >> >>>> encountered a similar problem.
> >> >>>>
> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
> overriding
> >> >>>> isEnabled() going to be declared evil too? ;)
> >> >>>
> >> >>> yes
> >> >>>
> >> >>> -igor
> >> >>>
> >> >>>>
> >> >>>> Sven
> >> >>>>
> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >> >>>>
> >> >>>>> Can you explain why? We have done this all over the place.
> >> >>>>>
> >> >>>>> D/
> >> >>>>>
> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >> >>>>>
> >> >>>>> > The recommended way since a few 1.4 releases is to override
> >> onConfigure()
> >> >>>>> > and call setVisible(true|false) depending on your conditions.
> >> >>>>> >
> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >> >>>>> > douglas@douglasferguson.us> wrote:
> >> >>>>> >
> >> >>>>> >> Igor posted a comment to this bug saying that overriding
> >> isVisible() is
> >> >>>>> >> "evil"
> >> >>>>> >>
> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >> >>>>> >>
> >> >>>>> >> I was surprised by this and am curious to hear more.
> >> >>>>> >>
> >> >>>>> >> D/
> >> >>>>>
> >> >>>>
> >> >>>>
> >> >>>>
> >> >>>
> >> >>
> >> >
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
so how is it different if they can still override something that needs
to be checked all the time?
-igor
On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pe...@gmail.com> wrote:
> I understand the concern about possible isVisible implementations like
>
> isVisible(return currentlyTime < 10:00:00;) //imagine this component being
> rendered at 09:59:59
> isVisible(return dao.list().size() > 0);// performance issues
>
> But maybe we can have the best from both approaches. This is an copy/paste
> from java.awt.Component:
>
> public boolean isVisible() {
> return isVisible_NoClientCode();
> }
> final boolean isVisible_NoClientCode() {
> return visible;
> }
>
> There are some points in the awt framework were the isVisible method is not
> used in benefit of isVisible_NoClientCode
> I'm in favor of create an final isVisible/Enabled version and change the
> Wicket core to use it. Also maintain the hotspot to users provide their
> isVisible/Enable implementations that will serve to feed the core component
> state.
>
> On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> ive run into plenty of weird problems with overrides, but maybe
>> because this was in a high concurrency app where data changed
>> frequently. the problems arise from the fact that the value returned
>> from isvisible() can change while we are doing traversals, etc.
>>
>> eg we run a traversal for all visible components and put them in a
>> list. later we iterate over the list and try to render these
>> components. the render function also checks their visibility and if
>> they are no longer visible it throws an exception.
>>
>> if isvisible() override depends on some external factor like the
>> database there is a small window where the value can change and now
>> you can have a weird exception: such as "tried to invoke a listener on
>> a component that is not visible or not enabled". these are very
>> intermittent and damn near impossible to reproduce.
>>
>> another problem is performance. isvisible() is called multiple times
>> during the request and if it depends on the database it can be a
>> performance problem. in fact a couple of users have complained about
>> this on the list in the past. at least now we have an easy solution
>> for them - use onconfigure().
>>
>> so as of right now the developers have two choices: override
>> isvisible() and potentially suffer the consequences. or, override
>> onconfigure() and set visibility there in a more deterministic
>> fashion.
>>
>> -igor
>>
>>
>>
>> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> <ee...@gmail.com> wrote:
>> > To expand, unless I'm missing something (new?), things are really only
>> > problematic when both the mutable value and the override are mixed. In
>> > a way, I think that using the override is 'more pure', as it's a
>> > simple function that is executed when needed, whereas mutable state
>> > can be harder to deal with when trying to figure out how it got to be
>> > in that state. So, sorry Igor, but we disagree on this one.
>> >
>> > Eelco
>> >
>> >
>> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>> > <ee...@gmail.com> wrote:
>> >> Niether is evil. It has potential pitfalls, which you should just be
>> >> aware of. We use such overrides all over the place and never have
>> >> problems with them either. :-) Avoiding it is safer, but also more
>> >> verbose (in 1.3.x at least).
>> >>
>> >> Eelco
>> >>
>> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com>
>> wrote:
>> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
>> >>>> Hi Douglas,
>> >>>>
>> >>>> WICKET-3171 describes a problematic case, where visibility of a
>> >>>> component changes while its form is being processed.
>> >>>> In our projects we're overriding isVisible() where appropriate and
>> never
>> >>>> encountered a similar problem.
>> >>>>
>> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
>> >>>> isEnabled() going to be declared evil too? ;)
>> >>>
>> >>> yes
>> >>>
>> >>> -igor
>> >>>
>> >>>>
>> >>>> Sven
>> >>>>
>> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>> >>>>
>> >>>>> Can you explain why? We have done this all over the place.
>> >>>>>
>> >>>>> D/
>> >>>>>
>> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>> >>>>>
>> >>>>> > The recommended way since a few 1.4 releases is to override
>> onConfigure()
>> >>>>> > and call setVisible(true|false) depending on your conditions.
>> >>>>> >
>> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> >>>>> > douglas@douglasferguson.us> wrote:
>> >>>>> >
>> >>>>> >> Igor posted a comment to this bug saying that overriding
>> isVisible() is
>> >>>>> >> "evil"
>> >>>>> >>
>> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>> >>>>> >>
>> >>>>> >> I was surprised by this and am curious to hear more.
>> >>>>> >>
>> >>>>> >> D/
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>
>> >>
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
Yes, for instance if an component contribute to the HTML header with an CSS
resource, and if this component return false to the isVisible test when the
HtmlHeaderContainer is traversing the component hierarchy requesting visible
components to contribute to header, and later return true when the
Component#render method offer the MarkupStream to it render it self at the
response: we will see the components not working as expected (rendered with
its proper CSS)
On Tue, Nov 30, 2010 at 2:30 PM, Peter Ertl <pe...@gmx.org> wrote:
> Jus to get it right ... is the following statement correct?
>
> "isVisible() will work as expected when the criteria for visibility does
> not change during render"
>
>
> Am 30.11.2010 um 17:02 schrieb Pedro Santos:
>
> > I understand the concern about possible isVisible implementations like
> >
> > isVisible(return currentlyTime < 10:00:00;) //imagine this component
> being
> > rendered at 09:59:59
> > isVisible(return dao.list().size() > 0);// performance issues
> >
> > But maybe we can have the best from both approaches. This is an
> copy/paste
> > from java.awt.Component:
> >
> > public boolean isVisible() {
> > return isVisible_NoClientCode();
> > }
> > final boolean isVisible_NoClientCode() {
> > return visible;
> > }
> >
> > There are some points in the awt framework were the isVisible method is
> not
> > used in benefit of isVisible_NoClientCode
> > I'm in favor of create an final isVisible/Enabled version and change the
> > Wicket core to use it. Also maintain the hotspot to users provide their
> > isVisible/Enable implementations that will serve to feed the core
> component
> > state.
> >
> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> ive run into plenty of weird problems with overrides, but maybe
> >> because this was in a high concurrency app where data changed
> >> frequently. the problems arise from the fact that the value returned
> >> from isvisible() can change while we are doing traversals, etc.
> >>
> >> eg we run a traversal for all visible components and put them in a
> >> list. later we iterate over the list and try to render these
> >> components. the render function also checks their visibility and if
> >> they are no longer visible it throws an exception.
> >>
> >> if isvisible() override depends on some external factor like the
> >> database there is a small window where the value can change and now
> >> you can have a weird exception: such as "tried to invoke a listener on
> >> a component that is not visible or not enabled". these are very
> >> intermittent and damn near impossible to reproduce.
> >>
> >> another problem is performance. isvisible() is called multiple times
> >> during the request and if it depends on the database it can be a
> >> performance problem. in fact a couple of users have complained about
> >> this on the list in the past. at least now we have an easy solution
> >> for them - use onconfigure().
> >>
> >> so as of right now the developers have two choices: override
> >> isvisible() and potentially suffer the consequences. or, override
> >> onconfigure() and set visibility there in a more deterministic
> >> fashion.
> >>
> >> -igor
> >>
> >>
> >>
> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> <ee...@gmail.com> wrote:
> >>> To expand, unless I'm missing something (new?), things are really only
> >>> problematic when both the mutable value and the override are mixed. In
> >>> a way, I think that using the override is 'more pure', as it's a
> >>> simple function that is executed when needed, whereas mutable state
> >>> can be harder to deal with when trying to figure out how it got to be
> >>> in that state. So, sorry Igor, but we disagree on this one.
> >>>
> >>> Eelco
> >>>
> >>>
> >>> On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >>> <ee...@gmail.com> wrote:
> >>>> Niether is evil. It has potential pitfalls, which you should just be
> >>>> aware of. We use such overrides all over the place and never have
> >>>> problems with them either. :-) Avoiding it is safer, but also more
> >>>> verbose (in 1.3.x at least).
> >>>>
> >>>> Eelco
> >>>>
> >>>> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> igor.vaynberg@gmail.com>
> >> wrote:
> >>>>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
> >>>>>> Hi Douglas,
> >>>>>>
> >>>>>> WICKET-3171 describes a problematic case, where visibility of a
> >>>>>> component changes while its form is being processed.
> >>>>>> In our projects we're overriding isVisible() where appropriate and
> >> never
> >>>>>> encountered a similar problem.
> >>>>>>
> >>>>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
> overriding
> >>>>>> isEnabled() going to be declared evil too? ;)
> >>>>>
> >>>>> yes
> >>>>>
> >>>>> -igor
> >>>>>
> >>>>>>
> >>>>>> Sven
> >>>>>>
> >>>>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >>>>>>
> >>>>>>> Can you explain why? We have done this all over the place.
> >>>>>>>
> >>>>>>> D/
> >>>>>>>
> >>>>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >>>>>>>
> >>>>>>>> The recommended way since a few 1.4 releases is to override
> >> onConfigure()
> >>>>>>>> and call setVisible(true|false) depending on your conditions.
> >>>>>>>>
> >>>>>>>> On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >>>>>>>> douglas@douglasferguson.us> wrote:
> >>>>>>>>
> >>>>>>>>> Igor posted a comment to this bug saying that overriding
> >> isVisible() is
> >>>>>>>>> "evil"
> >>>>>>>>>
> >>>>>>>>> https://issues.apache.org/jira/browse/WICKET-3171
> >>>>>>>>>
> >>>>>>>>> I was surprised by this and am curious to hear more.
> >>>>>>>>>
> >>>>>>>>> D/
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
>
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Peter Ertl <pe...@gmx.org>.
Jus to get it right ... is the following statement correct?
"isVisible() will work as expected when the criteria for visibility does not change during render"
Am 30.11.2010 um 17:02 schrieb Pedro Santos:
> I understand the concern about possible isVisible implementations like
>
> isVisible(return currentlyTime < 10:00:00;) //imagine this component being
> rendered at 09:59:59
> isVisible(return dao.list().size() > 0);// performance issues
>
> But maybe we can have the best from both approaches. This is an copy/paste
> from java.awt.Component:
>
> public boolean isVisible() {
> return isVisible_NoClientCode();
> }
> final boolean isVisible_NoClientCode() {
> return visible;
> }
>
> There are some points in the awt framework were the isVisible method is not
> used in benefit of isVisible_NoClientCode
> I'm in favor of create an final isVisible/Enabled version and change the
> Wicket core to use it. Also maintain the hotspot to users provide their
> isVisible/Enable implementations that will serve to feed the core component
> state.
>
> On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> ive run into plenty of weird problems with overrides, but maybe
>> because this was in a high concurrency app where data changed
>> frequently. the problems arise from the fact that the value returned
>> from isvisible() can change while we are doing traversals, etc.
>>
>> eg we run a traversal for all visible components and put them in a
>> list. later we iterate over the list and try to render these
>> components. the render function also checks their visibility and if
>> they are no longer visible it throws an exception.
>>
>> if isvisible() override depends on some external factor like the
>> database there is a small window where the value can change and now
>> you can have a weird exception: such as "tried to invoke a listener on
>> a component that is not visible or not enabled". these are very
>> intermittent and damn near impossible to reproduce.
>>
>> another problem is performance. isvisible() is called multiple times
>> during the request and if it depends on the database it can be a
>> performance problem. in fact a couple of users have complained about
>> this on the list in the past. at least now we have an easy solution
>> for them - use onconfigure().
>>
>> so as of right now the developers have two choices: override
>> isvisible() and potentially suffer the consequences. or, override
>> onconfigure() and set visibility there in a more deterministic
>> fashion.
>>
>> -igor
>>
>>
>>
>> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
>> <ee...@gmail.com> wrote:
>>> To expand, unless I'm missing something (new?), things are really only
>>> problematic when both the mutable value and the override are mixed. In
>>> a way, I think that using the override is 'more pure', as it's a
>>> simple function that is executed when needed, whereas mutable state
>>> can be harder to deal with when trying to figure out how it got to be
>>> in that state. So, sorry Igor, but we disagree on this one.
>>>
>>> Eelco
>>>
>>>
>>> On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
>>> <ee...@gmail.com> wrote:
>>>> Niether is evil. It has potential pitfalls, which you should just be
>>>> aware of. We use such overrides all over the place and never have
>>>> problems with them either. :-) Avoiding it is safer, but also more
>>>> verbose (in 1.3.x at least).
>>>>
>>>> Eelco
>>>>
>>>> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com>
>> wrote:
>>>>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
>>>>>> Hi Douglas,
>>>>>>
>>>>>> WICKET-3171 describes a problematic case, where visibility of a
>>>>>> component changes while its form is being processed.
>>>>>> In our projects we're overriding isVisible() where appropriate and
>> never
>>>>>> encountered a similar problem.
>>>>>>
>>>>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
>>>>>> isEnabled() going to be declared evil too? ;)
>>>>>
>>>>> yes
>>>>>
>>>>> -igor
>>>>>
>>>>>>
>>>>>> Sven
>>>>>>
>>>>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>>>>>>
>>>>>>> Can you explain why? We have done this all over the place.
>>>>>>>
>>>>>>> D/
>>>>>>>
>>>>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>>>>>>
>>>>>>>> The recommended way since a few 1.4 releases is to override
>> onConfigure()
>>>>>>>> and call setVisible(true|false) depending on your conditions.
>>>>>>>>
>>>>>>>> On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>>>>>>> douglas@douglasferguson.us> wrote:
>>>>>>>>
>>>>>>>>> Igor posted a comment to this bug saying that overriding
>> isVisible() is
>>>>>>>>> "evil"
>>>>>>>>>
>>>>>>>>> https://issues.apache.org/jira/browse/WICKET-3171
>>>>>>>>>
>>>>>>>>> I was surprised by this and am curious to hear more.
>>>>>>>>>
>>>>>>>>> D/
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Pedro Santos <pe...@gmail.com>.
I understand the concern about possible isVisible implementations like
isVisible(return currentlyTime < 10:00:00;) //imagine this component being
rendered at 09:59:59
isVisible(return dao.list().size() > 0);// performance issues
But maybe we can have the best from both approaches. This is an copy/paste
from java.awt.Component:
public boolean isVisible() {
return isVisible_NoClientCode();
}
final boolean isVisible_NoClientCode() {
return visible;
}
There are some points in the awt framework were the isVisible method is not
used in benefit of isVisible_NoClientCode
I'm in favor of create an final isVisible/Enabled version and change the
Wicket core to use it. Also maintain the hotspot to users provide their
isVisible/Enable implementations that will serve to feed the core component
state.
On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> ive run into plenty of weird problems with overrides, but maybe
> because this was in a high concurrency app where data changed
> frequently. the problems arise from the fact that the value returned
> from isvisible() can change while we are doing traversals, etc.
>
> eg we run a traversal for all visible components and put them in a
> list. later we iterate over the list and try to render these
> components. the render function also checks their visibility and if
> they are no longer visible it throws an exception.
>
> if isvisible() override depends on some external factor like the
> database there is a small window where the value can change and now
> you can have a weird exception: such as "tried to invoke a listener on
> a component that is not visible or not enabled". these are very
> intermittent and damn near impossible to reproduce.
>
> another problem is performance. isvisible() is called multiple times
> during the request and if it depends on the database it can be a
> performance problem. in fact a couple of users have complained about
> this on the list in the past. at least now we have an easy solution
> for them - use onconfigure().
>
> so as of right now the developers have two choices: override
> isvisible() and potentially suffer the consequences. or, override
> onconfigure() and set visibility there in a more deterministic
> fashion.
>
> -igor
>
>
>
> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> <ee...@gmail.com> wrote:
> > To expand, unless I'm missing something (new?), things are really only
> > problematic when both the mutable value and the override are mixed. In
> > a way, I think that using the override is 'more pure', as it's a
> > simple function that is executed when needed, whereas mutable state
> > can be harder to deal with when trying to figure out how it got to be
> > in that state. So, sorry Igor, but we disagree on this one.
> >
> > Eelco
> >
> >
> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> > <ee...@gmail.com> wrote:
> >> Niether is evil. It has potential pitfalls, which you should just be
> >> aware of. We use such overrides all over the place and never have
> >> problems with them either. :-) Avoiding it is safer, but also more
> >> verbose (in 1.3.x at least).
> >>
> >> Eelco
> >>
> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com>
> wrote:
> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
> >>>> Hi Douglas,
> >>>>
> >>>> WICKET-3171 describes a problematic case, where visibility of a
> >>>> component changes while its form is being processed.
> >>>> In our projects we're overriding isVisible() where appropriate and
> never
> >>>> encountered a similar problem.
> >>>>
> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
> >>>> isEnabled() going to be declared evil too? ;)
> >>>
> >>> yes
> >>>
> >>> -igor
> >>>
> >>>>
> >>>> Sven
> >>>>
> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >>>>
> >>>>> Can you explain why? We have done this all over the place.
> >>>>>
> >>>>> D/
> >>>>>
> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >>>>>
> >>>>> > The recommended way since a few 1.4 releases is to override
> onConfigure()
> >>>>> > and call setVisible(true|false) depending on your conditions.
> >>>>> >
> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >>>>> > douglas@douglasferguson.us> wrote:
> >>>>> >
> >>>>> >> Igor posted a comment to this bug saying that overriding
> isVisible() is
> >>>>> >> "evil"
> >>>>> >>
> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >>>>> >>
> >>>>> >> I was surprised by this and am curious to hear more.
> >>>>> >>
> >>>>> >> D/
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
> it has nothing to do with requiring a function to be set. the problem
> is that the function is free to change its mind at any moment, but we
> rely on it returning the same value during some fixed periods of time.
> if we truly want to support isvisible() we would need to cache/memoize
> the value for the period we assume it to be constant.
Yes, that's what I'm saying.
> but, defining
> these caching regions is very difficult, and maintaining them as we
> work on core we be even more difficult still.
I don't think it would be that difficult, but certainly not something
worth doing at this stage.
Eelco
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
On Tue, Nov 30, 2010 at 12:55 AM, Eelco Hillenius
<ee...@gmail.com> wrote:
> On Mon, Nov 29, 2010 at 11:51 PM, Juergen Donnerstag
> <ju...@gmail.com> wrote:
>> I'm curious. Which ideas would you steal from SiteBricks and JaxRS?
There are also many interesting ideas in Apache Sling. How it uses
OSGi for modularization, comes with a 'launchpad' and has a pretty
flexible bootstrapping mechanism and looks like a good power-to-weight
ratio.
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
On Mon, Nov 29, 2010 at 11:51 PM, Juergen Donnerstag
<ju...@gmail.com> wrote:
> I'm curious. Which ideas would you steal from SiteBricks and JaxRS?
I think SiteBricks strikes a nice balance between Wicket's and
Tapestry's (or JSF's/ ASP.NET's) programming models. I like that it
allows *some* logic in the templates - but which is checked early -
and code-behind constructs (which can save you lots of lines of code),
that it supports DI natively, and some things like how URLs are mapped
and e.g. how custom component names are defined are neat. I do have
doubts how well this would scale for complexity, which imho is the
problem with frameworks like Tapestry also. Relatively simple code is
easy enough to do and often looks more elegant (shorter at least) than
Wicket, but things can get messy quickly, especially when dealing with
deeper widget trees and projects using multiple component libraries. I
like the consistency that Wicket gives you, but I would try to make a
few common things a little bit less verbose.
Wrt JaxRs, I just like that framework. I like how easy it is (with
Jersey at least) to pass around objects that under the covers are
converted from/ to JSON, I like how easy it is to map method arguments
to request or POST parameters, etc. In short, I'd try to make better
use of annotations but keep it clean, straightforward and optional,
and I'd try to have a more DI centric approach (for instance, I really
like how you can let Jersey just pass in context objects in a method
if you need it).
Eelco
Re: overriding isVisible bad?
Posted by Juergen Donnerstag <ju...@gmail.com>.
I'm curious. Which ideas would you steal from SiteBricks and JaxRS?
Juergen
On Tue, Nov 30, 2010 at 5:51 AM, Eelco Hillenius
<ee...@gmail.com> wrote:
> On Mon, Nov 29, 2010 at 7:45 PM, richard emberson
> <ri...@gmail.com> wrote:
>> If wicket was going to be coded over again, would you make
>> isEnabled and/or isVisible final methods?
>
> If *I* would do it, I'd probably write it for Scala and lean more
> heavily on functions rather than mutable state. I'd also steal a few
> ideas from other frameworks like SiteBricks and JaxRS, have better
> 'native' JavaScript support (but definitively not going the GWT
> route), make choosing between stateful and stateless more explicit and
> up-front, support client-side state (but not like JSF or ASP.NET do
> it), etc. I gathered plenty of ideas of how I'd do a new framework,
> but I'm not even going to attempt that anyway :-) After having done a
> few projects with alternatives (GWT, pure JS/ JaxRS), I'm totally sold
> on Wicket again. It's not a perfect framework, but sure beats the hell
> out of most other frameworks when it comes to productivity and
> maintainability. :-)
>
> Eelco
>
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
On Mon, Nov 29, 2010 at 7:45 PM, richard emberson
<ri...@gmail.com> wrote:
> If wicket was going to be coded over again, would you make
> isEnabled and/or isVisible final methods?
If *I* would do it, I'd probably write it for Scala and lean more
heavily on functions rather than mutable state. I'd also steal a few
ideas from other frameworks like SiteBricks and JaxRS, have better
'native' JavaScript support (but definitively not going the GWT
route), make choosing between stateful and stateless more explicit and
up-front, support client-side state (but not like JSF or ASP.NET do
it), etc. I gathered plenty of ideas of how I'd do a new framework,
but I'm not even going to attempt that anyway :-) After having done a
few projects with alternatives (GWT, pure JS/ JaxRS), I'm totally sold
on Wicket again. It's not a perfect framework, but sure beats the hell
out of most other frameworks when it comes to productivity and
maintainability. :-)
Eelco
Re: overriding isVisible bad?
Posted by richard emberson <ri...@gmail.com>.
If wicket was going to be coded over again, would you make
isEnabled and/or isVisible final methods?
Might the non-finality of the methods be deprecated
in some future release?
The majority of the isXXX methods in Component are final.
Richard
--
Quis custodiet ipsos custodes
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
it has nothing to do with requiring a function to be set. the problem
is that the function is free to change its mind at any moment, but we
rely on it returning the same value during some fixed periods of time.
if we truly want to support isvisible() we would need to cache/memoize
the value for the period we assume it to be constant. but, defining
these caching regions is very difficult, and maintaining them as we
work on core we be even more difficult still.
-igor
On Mon, Nov 29, 2010 at 6:44 PM, Eelco Hillenius
<ee...@gmail.com> wrote:
>> ive run into plenty of weird problems with overrides, but maybe
>> because this was in a high concurrency app where data changed
>> frequently. the problems arise from the fact that the value returned
>> from isvisible() can change while we are doing traversals, etc.
>
> Hmmm, yeah, I can see how that could be a problem. Still, in general I
> find overrides to be easier to backtrack because it is a simple
> function. Wicket's mutable programming model is both a boon and a bane
> :-) Maybe a good solution would be to require a function to be set
> (would obviously work better in say Scala than Java at this point)
> that is called during the rendering preparation and which result is
> put in a temporary storage as the sole way of doing this. I'd
> personally like that better than supporting setVisible.
>
> Eelco
>
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
> ive run into plenty of weird problems with overrides, but maybe
> because this was in a high concurrency app where data changed
> frequently. the problems arise from the fact that the value returned
> from isvisible() can change while we are doing traversals, etc.
Hmmm, yeah, I can see how that could be a problem. Still, in general I
find overrides to be easier to backtrack because it is a simple
function. Wicket's mutable programming model is both a boon and a bane
:-) Maybe a good solution would be to require a function to be set
(would obviously work better in say Scala than Java at this point)
that is called during the rendering preparation and which result is
put in a temporary storage as the sole way of doing this. I'd
personally like that better than supporting setVisible.
Eelco
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
ive run into plenty of weird problems with overrides, but maybe
because this was in a high concurrency app where data changed
frequently. the problems arise from the fact that the value returned
from isvisible() can change while we are doing traversals, etc.
eg we run a traversal for all visible components and put them in a
list. later we iterate over the list and try to render these
components. the render function also checks their visibility and if
they are no longer visible it throws an exception.
if isvisible() override depends on some external factor like the
database there is a small window where the value can change and now
you can have a weird exception: such as "tried to invoke a listener on
a component that is not visible or not enabled". these are very
intermittent and damn near impossible to reproduce.
another problem is performance. isvisible() is called multiple times
during the request and if it depends on the database it can be a
performance problem. in fact a couple of users have complained about
this on the list in the past. at least now we have an easy solution
for them - use onconfigure().
so as of right now the developers have two choices: override
isvisible() and potentially suffer the consequences. or, override
onconfigure() and set visibility there in a more deterministic
fashion.
-igor
On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
<ee...@gmail.com> wrote:
> To expand, unless I'm missing something (new?), things are really only
> problematic when both the mutable value and the override are mixed. In
> a way, I think that using the override is 'more pure', as it's a
> simple function that is executed when needed, whereas mutable state
> can be harder to deal with when trying to figure out how it got to be
> in that state. So, sorry Igor, but we disagree on this one.
>
> Eelco
>
>
> On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> <ee...@gmail.com> wrote:
>> Niether is evil. It has potential pitfalls, which you should just be
>> aware of. We use such overrides all over the place and never have
>> problems with them either. :-) Avoiding it is safer, but also more
>> verbose (in 1.3.x at least).
>>
>> Eelco
>>
>> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com> wrote:
>>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
>>>> Hi Douglas,
>>>>
>>>> WICKET-3171 describes a problematic case, where visibility of a
>>>> component changes while its form is being processed.
>>>> In our projects we're overriding isVisible() where appropriate and never
>>>> encountered a similar problem.
>>>>
>>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
>>>> isEnabled() going to be declared evil too? ;)
>>>
>>> yes
>>>
>>> -igor
>>>
>>>>
>>>> Sven
>>>>
>>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>>>>
>>>>> Can you explain why? We have done this all over the place.
>>>>>
>>>>> D/
>>>>>
>>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>>>>
>>>>> > The recommended way since a few 1.4 releases is to override onConfigure()
>>>>> > and call setVisible(true|false) depending on your conditions.
>>>>> >
>>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>>>> > douglas@douglasferguson.us> wrote:
>>>>> >
>>>>> >> Igor posted a comment to this bug saying that overriding isVisible() is
>>>>> >> "evil"
>>>>> >>
>>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>>>>> >>
>>>>> >> I was surprised by this and am curious to hear more.
>>>>> >>
>>>>> >> D/
>>>>>
>>>>
>>>>
>>>>
>>>
>>
>
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
To expand, unless I'm missing something (new?), things are really only
problematic when both the mutable value and the override are mixed. In
a way, I think that using the override is 'more pure', as it's a
simple function that is executed when needed, whereas mutable state
can be harder to deal with when trying to figure out how it got to be
in that state. So, sorry Igor, but we disagree on this one.
Eelco
On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
<ee...@gmail.com> wrote:
> Niether is evil. It has potential pitfalls, which you should just be
> aware of. We use such overrides all over the place and never have
> problems with them either. :-) Avoiding it is safer, but also more
> verbose (in 1.3.x at least).
>
> Eelco
>
> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com> wrote:
>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
>>> Hi Douglas,
>>>
>>> WICKET-3171 describes a problematic case, where visibility of a
>>> component changes while its form is being processed.
>>> In our projects we're overriding isVisible() where appropriate and never
>>> encountered a similar problem.
>>>
>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
>>> isEnabled() going to be declared evil too? ;)
>>
>> yes
>>
>> -igor
>>
>>>
>>> Sven
>>>
>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>>>
>>>> Can you explain why? We have done this all over the place.
>>>>
>>>> D/
>>>>
>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>>>
>>>> > The recommended way since a few 1.4 releases is to override onConfigure()
>>>> > and call setVisible(true|false) depending on your conditions.
>>>> >
>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>>> > douglas@douglasferguson.us> wrote:
>>>> >
>>>> >> Igor posted a comment to this bug saying that overriding isVisible() is
>>>> >> "evil"
>>>> >>
>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>>>> >>
>>>> >> I was surprised by this and am curious to hear more.
>>>> >>
>>>> >> D/
>>>>
>>>
>>>
>>>
>>
>
Re: overriding isVisible bad?
Posted by Eelco Hillenius <ee...@gmail.com>.
Niether is evil. It has potential pitfalls, which you should just be
aware of. We use such overrides all over the place and never have
problems with them either. :-) Avoiding it is safer, but also more
verbose (in 1.3.x at least).
Eelco
On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <ig...@gmail.com> wrote:
> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
>> Hi Douglas,
>>
>> WICKET-3171 describes a problematic case, where visibility of a
>> component changes while its form is being processed.
>> In our projects we're overriding isVisible() where appropriate and never
>> encountered a similar problem.
>>
>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
>> isEnabled() going to be declared evil too? ;)
>
> yes
>
> -igor
>
>>
>> Sven
>>
>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>>
>>> Can you explain why? We have done this all over the place.
>>>
>>> D/
>>>
>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>>
>>> > The recommended way since a few 1.4 releases is to override onConfigure()
>>> > and call setVisible(true|false) depending on your conditions.
>>> >
>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>>> > douglas@douglasferguson.us> wrote:
>>> >
>>> >> Igor posted a comment to this bug saying that overriding isVisible() is
>>> >> "evil"
>>> >>
>>> >> https://issues.apache.org/jira/browse/WICKET-3171
>>> >>
>>> >> I was surprised by this and am curious to hear more.
>>> >>
>>> >> D/
>>>
>>
>>
>>
>
Re: overriding isVisible bad?
Posted by Igor Vaynberg <ig...@gmail.com>.
On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <sv...@meiers.net> wrote:
> Hi Douglas,
>
> WICKET-3171 describes a problematic case, where visibility of a
> component changes while its form is being processed.
> In our projects we're overriding isVisible() where appropriate and never
> encountered a similar problem.
>
> I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
> isEnabled() going to be declared evil too? ;)
yes
-igor
>
> Sven
>
> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
>
>> Can you explain why? We have done this all over the place.
>>
>> D/
>>
>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>>
>> > The recommended way since a few 1.4 releases is to override onConfigure()
>> > and call setVisible(true|false) depending on your conditions.
>> >
>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
>> > douglas@douglasferguson.us> wrote:
>> >
>> >> Igor posted a comment to this bug saying that overriding isVisible() is
>> >> "evil"
>> >>
>> >> https://issues.apache.org/jira/browse/WICKET-3171
>> >>
>> >> I was surprised by this and am curious to hear more.
>> >>
>> >> D/
>>
>
>
>
Re: overriding isVisible bad?
Posted by Sven Meier <sv...@meiers.net>.
Hi Douglas,
WICKET-3171 describes a problematic case, where visibility of a
component changes while its form is being processed.
In our projects we're overriding isVisible() where appropriate and never
encountered a similar problem.
I'd say WICKET-3171 is the rare 5% usecase. What's next, is overriding
isEnabled() going to be declared evil too? ;)
Sven
On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> Can you explain why? We have done this all over the place.
>
> D/
>
> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
>
> > The recommended way since a few 1.4 releases is to override onConfigure()
> > and call setVisible(true|false) depending on your conditions.
> >
> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> > douglas@douglasferguson.us> wrote:
> >
> >> Igor posted a comment to this bug saying that overriding isVisible() is
> >> "evil"
> >>
> >> https://issues.apache.org/jira/browse/WICKET-3171
> >>
> >> I was surprised by this and am curious to hear more.
> >>
> >> D/
>
Re: overriding isVisible bad?
Posted by Douglas Ferguson <do...@douglasferguson.us>.
Can you explain why? We have done this all over the place.
D/
On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> The recommended way since a few 1.4 releases is to override onConfigure()
> and call setVisible(true|false) depending on your conditions.
>
> On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> douglas@douglasferguson.us> wrote:
>
>> Igor posted a comment to this bug saying that overriding isVisible() is
>> "evil"
>>
>> https://issues.apache.org/jira/browse/WICKET-3171
>>
>> I was surprised by this and am curious to hear more.
>>
>> D/
Re: overriding isVisible bad?
Posted by Martin Grigorov <mg...@apache.org>.
The recommended way since a few 1.4 releases is to override onConfigure()
and call setVisible(true|false) depending on your conditions.
On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
douglas@douglasferguson.us> wrote:
> Igor posted a comment to this bug saying that overriding isVisible() is
> "evil"
>
> https://issues.apache.org/jira/browse/WICKET-3171
>
> I was surprised by this and am curious to hear more.
>
> D/
overriding isVisible bad?
Posted by Douglas Ferguson <do...@douglasferguson.us>.
Igor posted a comment to this bug saying that overriding isVisible() is "evil"
https://issues.apache.org/jira/browse/WICKET-3171
I was surprised by this and am curious to hear more.
D/
Re: [vote] release Wicket 1.4.14
Posted by Jeremy Thomerson <je...@wickettraining.com>.
Okay, this is all done. It should be replicated (and ready for
announcement) within a day.
On Sat, Nov 27, 2010 at 11:05 AM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> The vote passes with three binding +1 votes and three non-binding +1 votes.
>
> I will try to update the changelog and rebuild this evening so that I
> can get it replicating to the mirrors. Expect an announcement on
> Monday if all goes well.
>
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> Need a CMS for Wicket? Use Brix! http://brixcms.org
>
>
>
> On Tue, Nov 23, 2010 at 10:22 PM, Jeremy Thomerson
> <je...@wickettraining.com> wrote:
>> This vote is to release wicket 1.4.14. This is a bugfix release on the
>> 1.4.x (stable) branch.
>>
>> Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
>> Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
>> Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
>> Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>>
>> This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
>>
>> Please test the release and offer your vote:
>> [ ] Yes, release
>> [ ] No, don't release it (and please tell us why)
>>
>>
>> --
>> Jeremy Thomerson
>> http://wickettraining.com
>> Need a CMS for Wicket? Use Brix! http://brixcms.org
>>
>
--
Jeremy Thomerson
http://wickettraining.com
Need a CMS for Wicket? Use Brix! http://brixcms.org
Re: [vote] release Wicket 1.4.14
Posted by Jeremy Thomerson <je...@wickettraining.com>.
The vote passes with three binding +1 votes and three non-binding +1 votes.
I will try to update the changelog and rebuild this evening so that I
can get it replicating to the mirrors. Expect an announcement on
Monday if all goes well.
--
Jeremy Thomerson
http://wickettraining.com
Need a CMS for Wicket? Use Brix! http://brixcms.org
On Tue, Nov 23, 2010 at 10:22 PM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
> This vote is to release wicket 1.4.14. This is a bugfix release on the
> 1.4.x (stable) branch.
>
> Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>
> This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
>
> Please test the release and offer your vote:
> [ ] Yes, release
> [ ] No, don't release it (and please tell us why)
>
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> Need a CMS for Wicket? Use Brix! http://brixcms.org
>
Re: [vote] release Wicket 1.4.14
Posted by Martin Makundi <ma...@koodaripalvelut.com>.
> [WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
> children whose parents are invisible?
> I'm +1 to revert the change at this ticket because we start to make
> unnecessary visibility checks due an recursion that always stack calls to
> every component parent. I know it is important to respect any parent
> visibility restriction, but if the component has an restriction by itself,
> than we don't need to code such parent test.
If child is not designed to be visible it may throw exception at
child.isVisible. This fix was originally to prevent child.isVisible
being called when parent is not visible.
Reverting this fix might break applications and there is a test case
attached that will demonstrate this issue.
**
Martin
>
>
>
> On Wed, Nov 24, 2010 at 6:25 AM, Martin Grigorov <mg...@apache.org>wrote:
>
>> On Wed, Nov 24, 2010 at 4:22 AM, Jeremy Thomerson <
>> jeremy@wickettraining.com
>> > wrote:
>>
>> > This vote is to release wicket 1.4.14. This is a bugfix release on the
>> > 1.4.x (stable) branch.
>> >
>> > Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
>> > Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist<http://people.apache.org/%7Ejrthomerson/wicket-1.4.14/dist>
>> > Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo<http://people.apache.org/%7Ejrthomerson/wicket-1.4.14/m2-repo>
>> > Changelog:
>> >
>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>>
>> The changelog is wrong.
>>
>> + * [WICKET-3135] - Improve JavaScriptPackageResource#toString() to show
>> filename instead of default Object#toString()
>> + * [WICKET-3136] - JVM 1.6 crash caused by WicketObjectOutputStream,
>> ClassStreamHandler, and Page.writePageObject
>>
>> These are not resolved yet.
>>
>> >
>> >
>> > This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5
>> hours)
>> >
>> > Please test the release and offer your vote:
>> > [ ] Yes, release
>> > [ ] No, don't release it (and please tell us why)
>> >
>> >
>> > --
>> > Jeremy Thomerson
>> > http://wickettraining.com
>> > Need a CMS for Wicket? Use Brix! http://brixcms.org
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>
Re: [vote] release Wicket 1.4.14
Posted by Pedro Santos <pe...@gmail.com>.
[WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
children whose parents are invisible?
I'm +1 to revert the change at this ticket because we start to make
unnecessary visibility checks due an recursion that always stack calls to
every component parent. I know it is important to respect any parent
visibility restriction, but if the component has an restriction by itself,
than we don't need to code such parent test.
On Wed, Nov 24, 2010 at 6:25 AM, Martin Grigorov <mg...@apache.org>wrote:
> On Wed, Nov 24, 2010 at 4:22 AM, Jeremy Thomerson <
> jeremy@wickettraining.com
> > wrote:
>
> > This vote is to release wicket 1.4.14. This is a bugfix release on the
> > 1.4.x (stable) branch.
> >
> > Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> > Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist<http://people.apache.org/%7Ejrthomerson/wicket-1.4.14/dist>
> > Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo<http://people.apache.org/%7Ejrthomerson/wicket-1.4.14/m2-repo>
> > Changelog:
> >
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>
> The changelog is wrong.
>
> + * [WICKET-3135] - Improve JavaScriptPackageResource#toString() to show
> filename instead of default Object#toString()
> + * [WICKET-3136] - JVM 1.6 crash caused by WicketObjectOutputStream,
> ClassStreamHandler, and Page.writePageObject
>
> These are not resolved yet.
>
> >
> >
> > This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5
> hours)
> >
> > Please test the release and offer your vote:
> > [ ] Yes, release
> > [ ] No, don't release it (and please tell us why)
> >
> >
> > --
> > Jeremy Thomerson
> > http://wickettraining.com
> > Need a CMS for Wicket? Use Brix! http://brixcms.org
> >
>
--
Pedro Henrique Oliveira dos Santos
Re: [vote] release Wicket 1.4.14
Posted by Martin Grigorov <mg...@apache.org>.
On Wed, Nov 24, 2010 at 4:22 AM, Jeremy Thomerson <jeremy@wickettraining.com
> wrote:
> This vote is to release wicket 1.4.14. This is a bugfix release on the
> 1.4.x (stable) branch.
>
> Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> Changelog:
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
The changelog is wrong.
+ * [WICKET-3135] - Improve JavaScriptPackageResource#toString() to show
filename instead of default Object#toString()
+ * [WICKET-3136] - JVM 1.6 crash caused by WicketObjectOutputStream,
ClassStreamHandler, and Page.writePageObject
These are not resolved yet.
>
>
> This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
>
> Please test the release and offer your vote:
> [ ] Yes, release
> [ ] No, don't release it (and please tell us why)
>
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> Need a CMS for Wicket? Use Brix! http://brixcms.org
>
Re: [vote] release Wicket 1.4.14
Posted by Benjamin Paige <bf...@gw.dec.state.ny.us>.
Can someone please help get my email off this listserv? Maybe I'm trying to unsubscribe to the wrong listserv, because says that I'm not in it.
>>> Martin Grigorov <mg...@apache.org> 11/26/2010 14:49 PM >>>
+1 but let's update CHANGELOG file before releasing. As I said two tickets
listed as bug fixes are not for this release
On Wed, Nov 24, 2010 at 2:32 PM, Steve Swinsburg
<st...@gmail.com>wrote:
> +1
>
> cheers,
> Steve
>
> On 24/11/2010, at 2:22 PM, Jeremy Thomerson wrote:
>
> > This vote is to release wicket 1.4.14. This is a bugfix release on the
> > 1.4.x (stable) branch.
> >
> > Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> > Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> > Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> > Changelog:
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
> >
> > This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5
> hours)
> >
> > Please test the release and offer your vote:
> > [ ] Yes, release
> > [ ] No, don't release it (and please tell us why)
> >
> >
> > --
> > Jeremy Thomerson
> > http://wickettraining.com
> > Need a CMS for Wicket? Use Brix! http://brixcms.org
>
>
Re: [vote] release Wicket 1.4.14
Posted by Martin Grigorov <mg...@apache.org>.
+1 but let's update CHANGELOG file before releasing. As I said two tickets
listed as bug fixes are not for this release
On Wed, Nov 24, 2010 at 2:32 PM, Steve Swinsburg
<st...@gmail.com>wrote:
> +1
>
> cheers,
> Steve
>
> On 24/11/2010, at 2:22 PM, Jeremy Thomerson wrote:
>
> > This vote is to release wicket 1.4.14. This is a bugfix release on the
> > 1.4.x (stable) branch.
> >
> > Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> > Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> > Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> > Changelog:
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
> >
> > This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5
> hours)
> >
> > Please test the release and offer your vote:
> > [ ] Yes, release
> > [ ] No, don't release it (and please tell us why)
> >
> >
> > --
> > Jeremy Thomerson
> > http://wickettraining.com
> > Need a CMS for Wicket? Use Brix! http://brixcms.org
>
>
Re: [vote] release Wicket 1.4.14
Posted by Steve Swinsburg <st...@gmail.com>.
+1
cheers,
Steve
On 24/11/2010, at 2:22 PM, Jeremy Thomerson wrote:
> This vote is to release wicket 1.4.14. This is a bugfix release on the
> 1.4.x (stable) branch.
>
> Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>
> This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
>
> Please test the release and offer your vote:
> [ ] Yes, release
> [ ] No, don't release it (and please tell us why)
>
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> Need a CMS for Wicket? Use Brix! http://brixcms.org
Re: [vote] release Wicket 1.4.14
Posted by Igor Vaynberg <ig...@gmail.com>.
+1
-igor
On Tue, Nov 23, 2010 at 7:23 PM, Jeremy Thomerson
<je...@wickettraining.com> wrote:
>> [ ] Yes, release
>
> +1 - release
>
> --
> Jeremy Thomerson
> http://wickettraining.com
> Need a CMS for Wicket? Use Brix! http://brixcms.org
>
Re: [vote] release Wicket 1.4.14
Posted by Jeremy Thomerson <je...@wickettraining.com>.
> [ ] Yes, release
+1 - release
--
Jeremy Thomerson
http://wickettraining.com
Need a CMS for Wicket? Use Brix! http://brixcms.org
Re: [vote] release Wicket 1.4.14
Posted by Andrea Del Bene <ad...@ciseonweb.it>.
+1 for release.
I can confirm that memory leak bug is solved.
bye!
> This vote is to release wicket 1.4.14. This is a bugfix release on the
> 1.4.x (stable) branch.
>
> Branch: http://svn.apache.org/repos/asf/wicket/branches/wicket-1.4.14/
> Artifacts: http://people.apache.org/~jrthomerson/wicket-1.4.14/dist
> Maven repo: http://people.apache.org/~jrthomerson/wicket-1.4.14/m2-repo
> Changelog: https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310561&styleName=Html&version=12315480
>
> This vote ends Friday, November 26 at 10:30pm (US/EST or UTC/GMT -5 hours)
>
> Please test the release and offer your vote:
> [ ] Yes, release
> [ ] No, don't release it (and please tell us why)
>
>
>