sorry: isEnabled CANT do that..
On 10/8/06, Johan Compagner <[EMAIL PROTECTED]> wrote:
isEnabled can do that..
Because then this code is screwed up:
final Component component = page.get
(pageRelativeComponentPath);
if (component == null || !component.isEnabled() ||
!component.isVisibleInHierarchy())
{
log
.info("component not enabled or visible,
redirecting to calling page, component: "
+ component);
return new RedirectPageRequestTarget(page);
}
if (!component.isEnableAllowed())
{
throw new UnauthorizedActionException(component,
Component.ENABLE);
}
See first the isEnabled and then the isEnabledAllowed() both doing 2
completely different actions when happening?
johan
On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
>
> Well, final isEnabled would be cleanest solution, though users could
> have hard time adapting to it, as well as we might need to do the same
> for isVisible, to be consistent.
>
> isReallyEnabled sounds really bad :)
> I can't think of a good name, maybe isEnableAllowed could do it,
> currently it just clutters api imho, but I'm not very sure with it.
>
> -Matej
>
> Igor Vaynberg wrote:
> > as i said, i would rather that little final method be called
> isEnabled()
> > and
> > have a new hook for the user - cant come up with a good name.
> >
> > i mean what do you propose for this helper? isReallyEnabled() { return
> > isEnabled()&&isAllowedEnabled(); } ???
> >
> > -Igor
> >
> >
> > On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
> >>
> >> Well, it's not a big deal to check if for call to isEnabled() whether
>
> >> isEnableAllowed is checked too.
> >>
> >> but if we have isEnableAllowed(), which basically does nothing else
> then
> >> calls isActionAllowed(ENABLE), then maybe we could have a nice small
> >> final method that would botm check isEnableAllowed() and isEnabled().
> >>
> >> WDYT?
> >>
> >> -Matje
> >>
> >> Igor Vaynberg wrote:
> >> > we shouls always be checking both.
> >> >
> >> > if we dont its a bug, its just so easy to forget to check for
> >> > isAllowedEnabled :)
> >> >
> >> > -Igor
> >> >
> >> >
> >> > On 10/8/06, Matej Knopp < [EMAIL PROTECTED]> wrote:
> >> >>
> >> >> Well, the thing is that we don't check isEnableAllowed on many
> places
> >> >> either. Take link as an example. Even if component is not allowed
> >> to be
> >> >> enabled, it's still not rendered as disabled.
> >> >>
> >> >> The question is: Should it be? I think so, but maybe there is a
> good
> >> >> reason why it ain't. Anyone?
> >> >>
> >> >> -Matej
> >> >>
> >> >> Igor Vaynberg wrote:
> >> >> > there was a long discussion on this. i first proposed what you
> >> propose,
> >> >> but
> >> >> > later i changed my mind, but still am on the fence :)
> >> >> >
> >> >> > the thing here is that isenabled() and isallowenabled() are
> >> orthogonal
> >> >> in
> >> >> > the sense that one is meant to be implemented by the user and
> the
> >> other
> >> >> by
> >> >> > the security policy.
> >> >> >
> >> >> > so if our isenabled() checks for isallowenabled() then anytime
> the
> >> user
> >> >> > overrides it they have to remember to check for isallowenabled()
> -
> >> and
> >> >> if
> >> >> > they forget they break security.
> >> >> >
> >> >> > on the other hand if the user is implementing a component that
> needs
> >> to
> >> >> > properly handle the disabled state they have to do the check
> >> >> > isEnabled()&&isAllowedEnabled() which also has a potential for
> >> breaking
> >> >> > security - but as a component _implementor_ they should be a bit
>
> >> more
> >> >> > aware.
> >> >> >
> >> >> > the first case happens a lot more often so we cater for that
> one.
> >> >> >
> >> >> > the perfect situation would be to have another hook for the user
> to
> >> >> > implement, and have isenabled() as final that performs both
> checks.
> >> the
> >> >> > problem is coming up with a good name for that new method.
> >> >> >
> >> >> > -Igor
> >> >> >
> >> >> >
> >> >> > On 10/8/06, Matej Knopp <[EMAIL PROTECTED]> wrote:
> >> >> >>
> >> >> >> We have two methods that determine whether component should be
> >> >> allowed.
> >> >> >> Do we always check both?
> >> >> >>
> >> >> >> I mean, look at Link#onComponentTag, we only check
> >> >> >> isEnabled. So even though enable is not allowed, we still
> render
> >> link
> >> >> as
> >> >> >> enabled. Is this right?
> >> >> >>
> >> >> >> Shouldn't e.g. isEnabled() return false if the ENABLE action is
> not
> >> >> >> permitted?
> >> >> >>
> >> >> >> -Matej
> >> >> >>
> >> >> >
> >> >>
> >> >>
> >> >
> >>
> >>
> >
>
>
