You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Matej Knopp <ma...@gmail.com> on 2007/09/16 15:38:32 UTC
FLAG_IS_RENDER_ALLOWED
Hi,
is there a good reason why we traverse entire hierarchy and call
setRenderAllowed on every component, when we could just change
isRenderAllowed() to ask authorization strategy?
-Matej
Re: FLAG_IS_RENDER_ALLOWED
Posted by Maurice Marrink <ma...@gmail.com>.
Seems like a good idea.
I remember a while back when there was a problem with setRenderAllowed
not being set on new items in listviews etc, which was fixed i
believe, but this change would insure something like that could simply
not happen anymore.
It would be just like isEnableAllowed.
Maurice
On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> Hi,
>
> is there a good reason why we traverse entire hierarchy and call
> setRenderAllowed on every component, when we could just change
> isRenderAllowed() to ask authorization strategy?
>
> -Matej
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Maurice Marrink <ma...@gmail.com>.
I really should read this stuff more careful :)
On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> the value is cached per request not per session
>
> also since the security policy is more global then the request it is harder
> for it to cache on that level because it is a more granular level.
Not necessarily you can just as easily have a permission for an entire
page as for a single component.
>you would
> need something like a threadlocal with a map:component->boolean, and since
> strategy is not aware of request lifecycle it would be hard for it to clean
> the threadlocal.
Maybe i'm focusing to much on wasp/swarm but i don't see why see why
the security should be aware of the request lifecycle provided any and
all changes to the permissions of a user go through the security
layer, which i think is not a lot to ask for. This means the security
layer can implements its caching in the session (i know the session is
a threadlocal) or in a private store somewhere else it would not
matter.
Anyway to get back to what you first said. I agree that wicket caching
the results from the security layer for the duration of the request
should not be a problem but wonder if it is necessary.
Maurice
>
> -igor
>
>
> On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
> >
> > Should it not be the responsibility of the security layer to cache
> > that value if it is indeed such an expensive lookup?
> >
> > This same logic / problem would apply to all security inquiries wicket
> > makes.
> >
> > Also it is not unthinkable that this value changes during a user
> > session, just think of the su command in linux. wicket caching this
> > value would complicate matters in those situations. the security layer
> > oth will probably know / support this kind of changes to the
> > authorization of a person and will be able to keep its internal
> > bookkeeping / caching up to date.
> >
> > just my 2 cents.
> >
> > Maurice
> >
> > On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > wasnt the idea to do a pass one time and cache the value for the
> > request?
> > > because the lookup might be expensive.
> > >
> > > i wish we did that with isvisible, etc too :)
> > >
> > > -igor
> > >
> > >
> > > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > is there a good reason why we traverse entire hierarchy and call
> > > > setRenderAllowed on every component, when we could just change
> > > > isRenderAllowed() to ask authorization strategy?
> > > >
> > > > -Matej
> > > >
> > >
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Matej Knopp <ma...@gmail.com>.
But isRenderAllowed() is called during component rendering. And that
would not be invoked if the parent is not allowed. So normally the
isRenderAllowed() shouldn't be called on child that has parent with
isRenderAllowed() false.
Also right now, we traverse _entire_ hierarchy and call
component.isActionAuthorized(RENDER) on _every_ component, even on
invisible components or components with invisible parents. This
doesn't look very optimal to me.
-Matej
On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> well what about caching the cascade though?
>
> i guess if the parent is not enabled then none of the children can be as
> well.... so anytime you check for isenabled and its not cached yet you would
> have to do an upwords traversal to the root. with a single-pass we dont need
> to do that.
>
> -igor
>
>
> On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > I don't say not to cache it. But we could cache it after first
> > invocation. Just add a map<component,boolean> as request cycle
> > metadata. Would be lot cleaner and would save us one ugly traversal.
> >
> > -Matej
> >
> > On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > the value is cached per request not per session
> > >
> > > also since the security policy is more global then the request it is
> > harder
> > > for it to cache on that level because it is a more granular level. you
> > would
> > > need something like a threadlocal with a map:component->boolean, and
> > since
> > > strategy is not aware of request lifecycle it would be hard for it to
> > clean
> > > the threadlocal.
> > >
> > > -igor
> > >
> > >
> > > On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
> > > >
> > > > Should it not be the responsibility of the security layer to cache
> > > > that value if it is indeed such an expensive lookup?
> > > >
> > > > This same logic / problem would apply to all security inquiries wicket
> > > > makes.
> > > >
> > > > Also it is not unthinkable that this value changes during a user
> > > > session, just think of the su command in linux. wicket caching this
> > > > value would complicate matters in those situations. the security layer
> > > > oth will probably know / support this kind of changes to the
> > > > authorization of a person and will be able to keep its internal
> > > > bookkeeping / caching up to date.
> > > >
> > > > just my 2 cents.
> > > >
> > > > Maurice
> > > >
> > > > On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > > > wasnt the idea to do a pass one time and cache the value for the
> > > > request?
> > > > > because the lookup might be expensive.
> > > > >
> > > > > i wish we did that with isvisible, etc too :)
> > > > >
> > > > > -igor
> > > > >
> > > > >
> > > > > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > is there a good reason why we traverse entire hierarchy and call
> > > > > > setRenderAllowed on every component, when we could just change
> > > > > > isRenderAllowed() to ask authorization strategy?
> > > > > >
> > > > > > -Matej
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Igor Vaynberg <ig...@gmail.com>.
well what about caching the cascade though?
i guess if the parent is not enabled then none of the children can be as
well.... so anytime you check for isenabled and its not cached yet you would
have to do an upwords traversal to the root. with a single-pass we dont need
to do that.
-igor
On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
>
> I don't say not to cache it. But we could cache it after first
> invocation. Just add a map<component,boolean> as request cycle
> metadata. Would be lot cleaner and would save us one ugly traversal.
>
> -Matej
>
> On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > the value is cached per request not per session
> >
> > also since the security policy is more global then the request it is
> harder
> > for it to cache on that level because it is a more granular level. you
> would
> > need something like a threadlocal with a map:component->boolean, and
> since
> > strategy is not aware of request lifecycle it would be hard for it to
> clean
> > the threadlocal.
> >
> > -igor
> >
> >
> > On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
> > >
> > > Should it not be the responsibility of the security layer to cache
> > > that value if it is indeed such an expensive lookup?
> > >
> > > This same logic / problem would apply to all security inquiries wicket
> > > makes.
> > >
> > > Also it is not unthinkable that this value changes during a user
> > > session, just think of the su command in linux. wicket caching this
> > > value would complicate matters in those situations. the security layer
> > > oth will probably know / support this kind of changes to the
> > > authorization of a person and will be able to keep its internal
> > > bookkeeping / caching up to date.
> > >
> > > just my 2 cents.
> > >
> > > Maurice
> > >
> > > On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > > wasnt the idea to do a pass one time and cache the value for the
> > > request?
> > > > because the lookup might be expensive.
> > > >
> > > > i wish we did that with isvisible, etc too :)
> > > >
> > > > -igor
> > > >
> > > >
> > > > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > is there a good reason why we traverse entire hierarchy and call
> > > > > setRenderAllowed on every component, when we could just change
> > > > > isRenderAllowed() to ask authorization strategy?
> > > > >
> > > > > -Matej
> > > > >
> > > >
> > >
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Matej Knopp <ma...@gmail.com>.
I don't say not to cache it. But we could cache it after first
invocation. Just add a map<component,boolean> as request cycle
metadata. Would be lot cleaner and would save us one ugly traversal.
-Matej
On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> the value is cached per request not per session
>
> also since the security policy is more global then the request it is harder
> for it to cache on that level because it is a more granular level. you would
> need something like a threadlocal with a map:component->boolean, and since
> strategy is not aware of request lifecycle it would be hard for it to clean
> the threadlocal.
>
> -igor
>
>
> On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
> >
> > Should it not be the responsibility of the security layer to cache
> > that value if it is indeed such an expensive lookup?
> >
> > This same logic / problem would apply to all security inquiries wicket
> > makes.
> >
> > Also it is not unthinkable that this value changes during a user
> > session, just think of the su command in linux. wicket caching this
> > value would complicate matters in those situations. the security layer
> > oth will probably know / support this kind of changes to the
> > authorization of a person and will be able to keep its internal
> > bookkeeping / caching up to date.
> >
> > just my 2 cents.
> >
> > Maurice
> >
> > On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > > wasnt the idea to do a pass one time and cache the value for the
> > request?
> > > because the lookup might be expensive.
> > >
> > > i wish we did that with isvisible, etc too :)
> > >
> > > -igor
> > >
> > >
> > > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > is there a good reason why we traverse entire hierarchy and call
> > > > setRenderAllowed on every component, when we could just change
> > > > isRenderAllowed() to ask authorization strategy?
> > > >
> > > > -Matej
> > > >
> > >
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Igor Vaynberg <ig...@gmail.com>.
the value is cached per request not per session
also since the security policy is more global then the request it is harder
for it to cache on that level because it is a more granular level. you would
need something like a threadlocal with a map:component->boolean, and since
strategy is not aware of request lifecycle it would be hard for it to clean
the threadlocal.
-igor
On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
>
> Should it not be the responsibility of the security layer to cache
> that value if it is indeed such an expensive lookup?
>
> This same logic / problem would apply to all security inquiries wicket
> makes.
>
> Also it is not unthinkable that this value changes during a user
> session, just think of the su command in linux. wicket caching this
> value would complicate matters in those situations. the security layer
> oth will probably know / support this kind of changes to the
> authorization of a person and will be able to keep its internal
> bookkeeping / caching up to date.
>
> just my 2 cents.
>
> Maurice
>
> On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > wasnt the idea to do a pass one time and cache the value for the
> request?
> > because the lookup might be expensive.
> >
> > i wish we did that with isvisible, etc too :)
> >
> > -igor
> >
> >
> > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > is there a good reason why we traverse entire hierarchy and call
> > > setRenderAllowed on every component, when we could just change
> > > isRenderAllowed() to ask authorization strategy?
> > >
> > > -Matej
> > >
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Martijn Dashorst <ma...@gmail.com>.
I think Igor meant: during/for the duration of the request.
Martijn
On 9/16/07, Maurice Marrink <ma...@gmail.com> wrote:
> Should it not be the responsibility of the security layer to cache
> that value if it is indeed such an expensive lookup?
>
> This same logic / problem would apply to all security inquiries wicket makes.
>
> Also it is not unthinkable that this value changes during a user
> session, just think of the su command in linux. wicket caching this
> value would complicate matters in those situations. the security layer
> oth will probably know / support this kind of changes to the
> authorization of a person and will be able to keep its internal
> bookkeeping / caching up to date.
>
> just my 2 cents.
>
> Maurice
>
> On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > wasnt the idea to do a pass one time and cache the value for the request?
> > because the lookup might be expensive.
> >
> > i wish we did that with isvisible, etc too :)
> >
> > -igor
> >
> >
> > On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > is there a good reason why we traverse entire hierarchy and call
> > > setRenderAllowed on every component, when we could just change
> > > isRenderAllowed() to ask authorization strategy?
> > >
> > > -Matej
> > >
> >
>
--
Buy Wicket in Action: http://manning.com/dashorst
Apache Wicket 1.3.0-beta3 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta3/
Re: FLAG_IS_RENDER_ALLOWED
Posted by Maurice Marrink <ma...@gmail.com>.
Should it not be the responsibility of the security layer to cache
that value if it is indeed such an expensive lookup?
This same logic / problem would apply to all security inquiries wicket makes.
Also it is not unthinkable that this value changes during a user
session, just think of the su command in linux. wicket caching this
value would complicate matters in those situations. the security layer
oth will probably know / support this kind of changes to the
authorization of a person and will be able to keep its internal
bookkeeping / caching up to date.
just my 2 cents.
Maurice
On 9/16/07, Igor Vaynberg <ig...@gmail.com> wrote:
> wasnt the idea to do a pass one time and cache the value for the request?
> because the lookup might be expensive.
>
> i wish we did that with isvisible, etc too :)
>
> -igor
>
>
> On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > Hi,
> >
> > is there a good reason why we traverse entire hierarchy and call
> > setRenderAllowed on every component, when we could just change
> > isRenderAllowed() to ask authorization strategy?
> >
> > -Matej
> >
>
Re: FLAG_IS_RENDER_ALLOWED
Posted by Igor Vaynberg <ig...@gmail.com>.
wasnt the idea to do a pass one time and cache the value for the request?
because the lookup might be expensive.
i wish we did that with isvisible, etc too :)
-igor
On 9/16/07, Matej Knopp <ma...@gmail.com> wrote:
>
> Hi,
>
> is there a good reason why we traverse entire hierarchy and call
> setRenderAllowed on every component, when we could just change
> isRenderAllowed() to ask authorization strategy?
>
> -Matej
>