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
<eelco.hillen...@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 <igor.vaynb...@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 <checke...@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.maku...@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 <igor.vaynb...@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 <pedros...@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.vaynb...@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 <pedros...@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.vaynb...@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 <pedros...@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.vaynb...@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 <
>>>> pedros...@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.vaynb...@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 <
>>>> >>> pedros...@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.vaynb...@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
>>>> >>> >> >> >> >> <eelco.hillen...@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
>>>> >>> >> >> >> >> > <eelco.hillen...@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.vaynb...@gmail.com>
>>>> >>> >> >> >> >> wrote:
>>>> >>> >> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <
>>>> >>> s...@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 <
>>>> >>> >> >> >> >> >>>>> > doug...@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
>>>> >>
>>>> >
>>>>
>>>
>>
>
