You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2013/12/20 09:53:40 UTC

Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Hi,

The reporter of https://issues.apache.org/jira/browse/WICKET-5454 asked to
pass the Component instance
to  IAuthorizationStrategy#isInstantiationAuthorized() instead of just its
class.
I have no idea why the API has been designed this way but Carl-Eric gave a
good explanation - the component is not yet fully constructed.

The thing that bothers me is why it is OK to use the instance in my custom
IComponentInstantiationListener and it is not OK to do the same in
IAuthorizationStrategy#isInstantiationAuthorized() ?
If there is a javadoc explaining the possible problem (as for
IComponentInstantiationListener#onInstantiation()) then it is OK.

Even more - at
https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
can see that right ater rejecting the *Class* we pass the *instance*
to
the UnauthorizedComponentInstantiationListener!


Martin Grigorov
Wicket Training and Consulting

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Martin Grigorov <mg...@apache.org>.
Done.
https://issues.apache.org/jira/browse/WICKET-5490

Martin Grigorov
Wicket Training and Consulting


On Fri, Dec 20, 2013 at 8:48 PM, Igor Vaynberg <ig...@gmail.com>wrote:

> i am guessing that the id of the component would be useful for logging
> in some cases, but i think it should just be passed in as an extra
> argument if thats the case. something to fix in 7.0...
>
> -igor
>
>
> On Fri, Dec 20, 2013 at 11:44 AM, Martin Grigorov <mg...@apache.org>
> wrote:
> > and what about IUnauthorizedComponentInstantiationListener ?
> > it receives the partially constructed object in case of rejection
> > its javadoc states: The partially constructed component (only the id is
> > guaranteed to be valid)
> > but even Wicket sources use it (partially) wrong later:
> >
> org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedInstantiation
> > casts the instance to a Page and passes it to
> >
>  org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedPage(Page)
> > Here we use just "page.getClass()" but specialization of this class may
> try
> > to use the page instance for anything
> >
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> >
> >
> > On Fri, Dec 20, 2013 at 6:14 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> this is a security check, so the whole idea is that it is ran before
> >> any of the user's code in the constructor which may have side-effects.
> >> eg a constructor marking a record as ready to be deleted because a
> >> delete panel was instantiated. the class itself should be enough. even
> >> if you get an instance you cant use anything in it because its
> >> partially constructed. the question is if we do pass an instance how
> >> many users will bother reading javadoc? and out of those how many
> >> really understand how objects are constructed? i think we should close
> >> the issue as wont-fix, reading it "It would be easier to decide if
> >> instantiation is authorized if one could access some properties of the
> >> component being constructed." which is exactly what you cannot/must
> >> not do because the object is only partially initialized, thus proving
> >> my point above.
> >>
> >> the ComponentInstantiationListener is a very special case where we
> >> make an exception. the entire point of this interface is to work with
> >> a partially constructed object and most users will never implement
> >> their own as opposed to the authorization strategy...
> >>
> >> -igor
> >>
> >>
> >> On Fri, Dec 20, 2013 at 12:53 AM, Martin Grigorov <mgrigorov@apache.org
> >
> >> wrote:
> >> > Hi,
> >> >
> >> > The reporter of https://issues.apache.org/jira/browse/WICKET-5454asked
> >> to
> >> > pass the Component instance
> >> > to  IAuthorizationStrategy#isInstantiationAuthorized() instead of just
> >> its
> >> > class.
> >> > I have no idea why the API has been designed this way but Carl-Eric
> gave
> >> a
> >> > good explanation - the component is not yet fully constructed.
> >> >
> >> > The thing that bothers me is why it is OK to use the instance in my
> >> custom
> >> > IComponentInstantiationListener and it is not OK to do the same in
> >> > IAuthorizationStrategy#isInstantiationAuthorized() ?
> >> > If there is a javadoc explaining the possible problem (as for
> >> > IComponentInstantiationListener#onInstantiation()) then it is OK.
> >> >
> >> > Even more - at
> >> >
> >>
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> >> > can see that right ater rejecting the *Class* we pass the *instance*
> >> > to
> >> > the UnauthorizedComponentInstantiationListener!
> >> >
> >> >
> >> > Martin Grigorov
> >> > Wicket Training and Consulting
> >>
>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Igor Vaynberg <ig...@gmail.com>.
i am guessing that the id of the component would be useful for logging
in some cases, but i think it should just be passed in as an extra
argument if thats the case. something to fix in 7.0...

-igor


On Fri, Dec 20, 2013 at 11:44 AM, Martin Grigorov <mg...@apache.org> wrote:
> and what about IUnauthorizedComponentInstantiationListener ?
> it receives the partially constructed object in case of rejection
> its javadoc states: The partially constructed component (only the id is
> guaranteed to be valid)
> but even Wicket sources use it (partially) wrong later:
> org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedInstantiation
> casts the instance to a Page and passes it to
>  org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedPage(Page)
> Here we use just "page.getClass()" but specialization of this class may try
> to use the page instance for anything
>
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Fri, Dec 20, 2013 at 6:14 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> this is a security check, so the whole idea is that it is ran before
>> any of the user's code in the constructor which may have side-effects.
>> eg a constructor marking a record as ready to be deleted because a
>> delete panel was instantiated. the class itself should be enough. even
>> if you get an instance you cant use anything in it because its
>> partially constructed. the question is if we do pass an instance how
>> many users will bother reading javadoc? and out of those how many
>> really understand how objects are constructed? i think we should close
>> the issue as wont-fix, reading it "It would be easier to decide if
>> instantiation is authorized if one could access some properties of the
>> component being constructed." which is exactly what you cannot/must
>> not do because the object is only partially initialized, thus proving
>> my point above.
>>
>> the ComponentInstantiationListener is a very special case where we
>> make an exception. the entire point of this interface is to work with
>> a partially constructed object and most users will never implement
>> their own as opposed to the authorization strategy...
>>
>> -igor
>>
>>
>> On Fri, Dec 20, 2013 at 12:53 AM, Martin Grigorov <mg...@apache.org>
>> wrote:
>> > Hi,
>> >
>> > The reporter of https://issues.apache.org/jira/browse/WICKET-5454 asked
>> to
>> > pass the Component instance
>> > to  IAuthorizationStrategy#isInstantiationAuthorized() instead of just
>> its
>> > class.
>> > I have no idea why the API has been designed this way but Carl-Eric gave
>> a
>> > good explanation - the component is not yet fully constructed.
>> >
>> > The thing that bothers me is why it is OK to use the instance in my
>> custom
>> > IComponentInstantiationListener and it is not OK to do the same in
>> > IAuthorizationStrategy#isInstantiationAuthorized() ?
>> > If there is a javadoc explaining the possible problem (as for
>> > IComponentInstantiationListener#onInstantiation()) then it is OK.
>> >
>> > Even more - at
>> >
>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
>> > can see that right ater rejecting the *Class* we pass the *instance*
>> > to
>> > the UnauthorizedComponentInstantiationListener!
>> >
>> >
>> > Martin Grigorov
>> > Wicket Training and Consulting
>>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Martin Grigorov <mg...@apache.org>.
and what about IUnauthorizedComponentInstantiationListener ?
it receives the partially constructed object in case of rejection
its javadoc states: The partially constructed component (only the id is
guaranteed to be valid)
but even Wicket sources use it (partially) wrong later:
org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedInstantiation
casts the instance to a Page and passes it to
 org.apache.wicket.authroles.authentication.AuthenticatedWebApplication#onUnauthorizedPage(Page)
Here we use just "page.getClass()" but specialization of this class may try
to use the page instance for anything


Martin Grigorov
Wicket Training and Consulting


On Fri, Dec 20, 2013 at 6:14 PM, Igor Vaynberg <ig...@gmail.com>wrote:

> this is a security check, so the whole idea is that it is ran before
> any of the user's code in the constructor which may have side-effects.
> eg a constructor marking a record as ready to be deleted because a
> delete panel was instantiated. the class itself should be enough. even
> if you get an instance you cant use anything in it because its
> partially constructed. the question is if we do pass an instance how
> many users will bother reading javadoc? and out of those how many
> really understand how objects are constructed? i think we should close
> the issue as wont-fix, reading it "It would be easier to decide if
> instantiation is authorized if one could access some properties of the
> component being constructed." which is exactly what you cannot/must
> not do because the object is only partially initialized, thus proving
> my point above.
>
> the ComponentInstantiationListener is a very special case where we
> make an exception. the entire point of this interface is to work with
> a partially constructed object and most users will never implement
> their own as opposed to the authorization strategy...
>
> -igor
>
>
> On Fri, Dec 20, 2013 at 12:53 AM, Martin Grigorov <mg...@apache.org>
> wrote:
> > Hi,
> >
> > The reporter of https://issues.apache.org/jira/browse/WICKET-5454 asked
> to
> > pass the Component instance
> > to  IAuthorizationStrategy#isInstantiationAuthorized() instead of just
> its
> > class.
> > I have no idea why the API has been designed this way but Carl-Eric gave
> a
> > good explanation - the component is not yet fully constructed.
> >
> > The thing that bothers me is why it is OK to use the instance in my
> custom
> > IComponentInstantiationListener and it is not OK to do the same in
> > IAuthorizationStrategy#isInstantiationAuthorized() ?
> > If there is a javadoc explaining the possible problem (as for
> > IComponentInstantiationListener#onInstantiation()) then it is OK.
> >
> > Even more - at
> >
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> > can see that right ater rejecting the *Class* we pass the *instance*
> > to
> > the UnauthorizedComponentInstantiationListener!
> >
> >
> > Martin Grigorov
> > Wicket Training and Consulting
>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Thanks Igor for making my point much better than I did. I agree 100%.

Carl-Eric

On Fri, 20 Dec 2013 08:14:27 -0800
Igor Vaynberg <ig...@gmail.com> wrote:

> this is a security check, so the whole idea is that it is ran before
> any of the user's code in the constructor which may have side-effects.
> eg a constructor marking a record as ready to be deleted because a
> delete panel was instantiated. the class itself should be enough. even
> if you get an instance you cant use anything in it because its
> partially constructed. the question is if we do pass an instance how
> many users will bother reading javadoc? and out of those how many
> really understand how objects are constructed? i think we should close
> the issue as wont-fix, reading it "It would be easier to decide if
> instantiation is authorized if one could access some properties of the
> component being constructed." which is exactly what you cannot/must
> not do because the object is only partially initialized, thus proving
> my point above.
> 
> the ComponentInstantiationListener is a very special case where we
> make an exception. the entire point of this interface is to work with
> a partially constructed object and most users will never implement
> their own as opposed to the authorization strategy...
> 
> -igor
> 
> 
> On Fri, Dec 20, 2013 at 12:53 AM, Martin Grigorov
> <mg...@apache.org> wrote:
> > Hi,
> >
> > The reporter of https://issues.apache.org/jira/browse/WICKET-5454
> > asked to pass the Component instance
> > to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
> > just its class.
> > I have no idea why the API has been designed this way but Carl-Eric
> > gave a good explanation - the component is not yet fully
> > constructed.
> >
> > The thing that bothers me is why it is OK to use the instance in my
> > custom IComponentInstantiationListener and it is not OK to do the
> > same in IAuthorizationStrategy#isInstantiationAuthorized() ?
> > If there is a javadoc explaining the possible problem (as for
> > IComponentInstantiationListener#onInstantiation()) then it is OK.
> >
> > Even more - at
> > https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> > can see that right ater rejecting the *Class* we pass the *instance*
> > to
> > the UnauthorizedComponentInstantiationListener!
> >
> >
> > Martin Grigorov
> > Wicket Training and Consulting


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Igor Vaynberg <ig...@gmail.com>.
this is a security check, so the whole idea is that it is ran before
any of the user's code in the constructor which may have side-effects.
eg a constructor marking a record as ready to be deleted because a
delete panel was instantiated. the class itself should be enough. even
if you get an instance you cant use anything in it because its
partially constructed. the question is if we do pass an instance how
many users will bother reading javadoc? and out of those how many
really understand how objects are constructed? i think we should close
the issue as wont-fix, reading it "It would be easier to decide if
instantiation is authorized if one could access some properties of the
component being constructed." which is exactly what you cannot/must
not do because the object is only partially initialized, thus proving
my point above.

the ComponentInstantiationListener is a very special case where we
make an exception. the entire point of this interface is to work with
a partially constructed object and most users will never implement
their own as opposed to the authorization strategy...

-igor


On Fri, Dec 20, 2013 at 12:53 AM, Martin Grigorov <mg...@apache.org> wrote:
> Hi,
>
> The reporter of https://issues.apache.org/jira/browse/WICKET-5454 asked to
> pass the Component instance
> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of just its
> class.
> I have no idea why the API has been designed this way but Carl-Eric gave a
> good explanation - the component is not yet fully constructed.
>
> The thing that bothers me is why it is OK to use the instance in my custom
> IComponentInstantiationListener and it is not OK to do the same in
> IAuthorizationStrategy#isInstantiationAuthorized() ?
> If there is a javadoc explaining the possible problem (as for
> IComponentInstantiationListener#onInstantiation()) then it is OK.
>
> Even more - at
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> can see that right ater rejecting the *Class* we pass the *instance*
> to
> the UnauthorizedComponentInstantiationListener!
>
>
> Martin Grigorov
> Wicket Training and Consulting

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Martin Grigorov <mg...@apache.org>.
Oh.
I was thinking only about the read part.
One more +1 for the component! :)

Martin Grigorov
Wicket Training and Consulting


On Fri, Dec 20, 2013 at 11:44 AM, Carl-Eric Menzel <
carl-eric@duesenklipper.de> wrote:

> How can you inject dependencies into an instance's fields without
> actually having that instance?
>
> Carl-Eric
>
> On Fri, 20 Dec 2013 11:27:43 +0200
> Martin Grigorov <mg...@apache.org> wrote:
>
> > All IOC listeners (Spring, Guice, CDI) can do their job by using only
> > the class.
> > But I still find the instance more useful :-)
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> >
> >
> > On Fri, Dec 20, 2013 at 11:19 AM, Carl-Eric Menzel
> > <cm...@wicketbuch.de>wrote:
> >
> > > Duh. I knew I missed an obvious one! You are of course right.
> > >
> > > So we can't get rid of that one easily. That said, I'm still wary of
> > > spreading unfinished objects even further around the API.
> > >
> > > Carl-Eric
> > >
> > > On Fri, 20 Dec 2013 10:12:32 +0100
> > > Sven Meier <sv...@meiers.net> wrote:
> > >
> > > > SpringComponentInjector?
> > > >
> > > > Sven
> > > >
> > > > On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> > > > > I agree that something should be cleaned up. But like I said in
> > > > > the comment to that Jira ticket, I think it should in fact move
> > > > > more toward passing the class rather than the uninitialized
> > > > > instance. I like my objects to be predictable, and I like it
> > > > > even more when I don't have to think about any traps, even if
> > > > > they are well-documented.
> > > > >
> > > > > I think (I also wrote that in the comment) that perhaps
> > > > > IComponentInstantiationListener should be deprecated and
> > > > > eventually removed, since IComponentInitializationListener
> > > > > should be able to cover all the same use cases without having
> > > > > the unfinished constructor problem.
> > > > >
> > > > > Is there a use case that would not be handled by the
> > > > > initialization listener?
> > > > >
> > > > > Carl-Eric
> > > > >
> > > > > On Fri, 20 Dec 2013 10:53:40 +0200
> > > > > Martin Grigorov <mg...@apache.org> wrote:
> > > > >
> > > > >> Hi,
> > > > >>
> > > > >> The reporter of
> > > > >> https://issues.apache.org/jira/browse/WICKET-5454 asked to
> > > > >> pass the Component instance to
> > > > >> IAuthorizationStrategy#isInstantiationAuthorized() instead of
> > > > >> just its class. I have no idea why the API has been designed
> > > > >> this way but Carl-Eric gave a good explanation - the component
> > > > >> is not yet fully constructed.
> > > > >>
> > > > >> The thing that bothers me is why it is OK to use the instance
> > > > >> in my custom IComponentInstantiationListener and it is not OK
> > > > >> to do the same in
> > > > >> IAuthorizationStrategy#isInstantiationAuthorized() ? If there
> > > > >> is a javadoc explaining the possible problem (as for
> > > > >> IComponentInstantiationListener#onInstantiation()) then it is
> > > > >> OK.
> > > > >>
> > > > >> Even more - at
> > > > >>
> > >
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> > > > >> can see that right ater rejecting the *Class* we pass the
> > > > >> *instance* to
> > > > >> the UnauthorizedComponentInstantiationListener!
> > > > >>
> > > > >>
> > > > >> Martin Grigorov
> > > > >> Wicket Training and Consulting
> > > >
> > >
> > >
>
>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Carl-Eric Menzel <ca...@duesenklipper.de>.
How can you inject dependencies into an instance's fields without
actually having that instance?

Carl-Eric

On Fri, 20 Dec 2013 11:27:43 +0200
Martin Grigorov <mg...@apache.org> wrote:

> All IOC listeners (Spring, Guice, CDI) can do their job by using only
> the class.
> But I still find the instance more useful :-)
> 
> Martin Grigorov
> Wicket Training and Consulting
> 
> 
> On Fri, Dec 20, 2013 at 11:19 AM, Carl-Eric Menzel
> <cm...@wicketbuch.de>wrote:
> 
> > Duh. I knew I missed an obvious one! You are of course right.
> >
> > So we can't get rid of that one easily. That said, I'm still wary of
> > spreading unfinished objects even further around the API.
> >
> > Carl-Eric
> >
> > On Fri, 20 Dec 2013 10:12:32 +0100
> > Sven Meier <sv...@meiers.net> wrote:
> >
> > > SpringComponentInjector?
> > >
> > > Sven
> > >
> > > On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> > > > I agree that something should be cleaned up. But like I said in
> > > > the comment to that Jira ticket, I think it should in fact move
> > > > more toward passing the class rather than the uninitialized
> > > > instance. I like my objects to be predictable, and I like it
> > > > even more when I don't have to think about any traps, even if
> > > > they are well-documented.
> > > >
> > > > I think (I also wrote that in the comment) that perhaps
> > > > IComponentInstantiationListener should be deprecated and
> > > > eventually removed, since IComponentInitializationListener
> > > > should be able to cover all the same use cases without having
> > > > the unfinished constructor problem.
> > > >
> > > > Is there a use case that would not be handled by the
> > > > initialization listener?
> > > >
> > > > Carl-Eric
> > > >
> > > > On Fri, 20 Dec 2013 10:53:40 +0200
> > > > Martin Grigorov <mg...@apache.org> wrote:
> > > >
> > > >> Hi,
> > > >>
> > > >> The reporter of
> > > >> https://issues.apache.org/jira/browse/WICKET-5454 asked to
> > > >> pass the Component instance to
> > > >> IAuthorizationStrategy#isInstantiationAuthorized() instead of
> > > >> just its class. I have no idea why the API has been designed
> > > >> this way but Carl-Eric gave a good explanation - the component
> > > >> is not yet fully constructed.
> > > >>
> > > >> The thing that bothers me is why it is OK to use the instance
> > > >> in my custom IComponentInstantiationListener and it is not OK
> > > >> to do the same in
> > > >> IAuthorizationStrategy#isInstantiationAuthorized() ? If there
> > > >> is a javadoc explaining the possible problem (as for
> > > >> IComponentInstantiationListener#onInstantiation()) then it is
> > > >> OK.
> > > >>
> > > >> Even more - at
> > > >>
> > https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> > > >> can see that right ater rejecting the *Class* we pass the
> > > >> *instance* to
> > > >> the UnauthorizedComponentInstantiationListener!
> > > >>
> > > >>
> > > >> Martin Grigorov
> > > >> Wicket Training and Consulting
> > >
> >
> >


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Martin Grigorov <mg...@apache.org>.
All IOC listeners (Spring, Guice, CDI) can do their job by using only the
class.
But I still find the instance more useful :-)

Martin Grigorov
Wicket Training and Consulting


On Fri, Dec 20, 2013 at 11:19 AM, Carl-Eric Menzel <cm...@wicketbuch.de>wrote:

> Duh. I knew I missed an obvious one! You are of course right.
>
> So we can't get rid of that one easily. That said, I'm still wary of
> spreading unfinished objects even further around the API.
>
> Carl-Eric
>
> On Fri, 20 Dec 2013 10:12:32 +0100
> Sven Meier <sv...@meiers.net> wrote:
>
> > SpringComponentInjector?
> >
> > Sven
> >
> > On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> > > I agree that something should be cleaned up. But like I said in the
> > > comment to that Jira ticket, I think it should in fact move more
> > > toward passing the class rather than the uninitialized instance. I
> > > like my objects to be predictable, and I like it even more when I
> > > don't have to think about any traps, even if they are
> > > well-documented.
> > >
> > > I think (I also wrote that in the comment) that perhaps
> > > IComponentInstantiationListener should be deprecated and eventually
> > > removed, since IComponentInitializationListener should be able to
> > > cover all the same use cases without having the unfinished
> > > constructor problem.
> > >
> > > Is there a use case that would not be handled by the initialization
> > > listener?
> > >
> > > Carl-Eric
> > >
> > > On Fri, 20 Dec 2013 10:53:40 +0200
> > > Martin Grigorov <mg...@apache.org> wrote:
> > >
> > >> Hi,
> > >>
> > >> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
> > >> asked to pass the Component instance
> > >> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
> > >> just its class.
> > >> I have no idea why the API has been designed this way but Carl-Eric
> > >> gave a good explanation - the component is not yet fully
> > >> constructed.
> > >>
> > >> The thing that bothers me is why it is OK to use the instance in my
> > >> custom IComponentInstantiationListener and it is not OK to do the
> > >> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
> > >> If there is a javadoc explaining the possible problem (as for
> > >> IComponentInstantiationListener#onInstantiation()) then it is OK.
> > >>
> > >> Even more - at
> > >>
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> > >> can see that right ater rejecting the *Class* we pass the
> > >> *instance* to
> > >> the UnauthorizedComponentInstantiationListener!
> > >>
> > >>
> > >> Martin Grigorov
> > >> Wicket Training and Consulting
> >
>
>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Tom Götz <to...@decoded.de>.
Well, how can you tell that a page is an error page if you only have it’s class in your hands? AFAIK you need to check wether:

a) it’s a descendant of org.apache.wicket.markup.html.pages.AbstractErrorPage
b) it’s a descendant of your own error page hierarchy, if any
c) it is one of the classes (or subclasses) returned by:
Application.get().getApplicationSettings().get[AccessDeniedPage|InternalErrorPage|PageExpiredErrorPage]()

Or do you see an easier solution to this? In that usecase isErrorPage() always returns static values, so IMHO it would be safe to call it. Yes, but I also see your concerns of passing around not-yet-fully-initialized instances. Personally I’d prefer to pass them and document it accordingly as it means more freedom and flexibility.

   -Tom


On 20.12.2013, at 10:32, Sven Meier <sv...@meiers.net> wrote:

> The reporter of the issue states that a "call to isErrorPage() would be more elegant in that situation"
> 
> If the passed instance is not yet fully constructed, how can #isErrorPage() return anything useful other than static information? Isn't the class or any of its annotations sufficient in this case?
> 
> Sven
> 
> On 12/20/2013 10:19 AM, Carl-Eric Menzel wrote:
>> Duh. I knew I missed an obvious one! You are of course right.
>> 
>> So we can't get rid of that one easily. That said, I'm still wary of
>> spreading unfinished objects even further around the API.
>> 
>> Carl-Eric
>> 
>> On Fri, 20 Dec 2013 10:12:32 +0100
>> Sven Meier <sv...@meiers.net> wrote:
>> 
>>> SpringComponentInjector?
>>> 
>>> Sven
>>> 
>>> On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
>>>> I agree that something should be cleaned up. But like I said in the
>>>> comment to that Jira ticket, I think it should in fact move more
>>>> toward passing the class rather than the uninitialized instance. I
>>>> like my objects to be predictable, and I like it even more when I
>>>> don't have to think about any traps, even if they are
>>>> well-documented.
>>>> 
>>>> I think (I also wrote that in the comment) that perhaps
>>>> IComponentInstantiationListener should be deprecated and eventually
>>>> removed, since IComponentInitializationListener should be able to
>>>> cover all the same use cases without having the unfinished
>>>> constructor problem.
>>>> 
>>>> Is there a use case that would not be handled by the initialization
>>>> listener?
>>>> 
>>>> Carl-Eric
>>>> 
>>>> On Fri, 20 Dec 2013 10:53:40 +0200
>>>> Martin Grigorov <mg...@apache.org> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
>>>>> asked to pass the Component instance
>>>>> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
>>>>> just its class.
>>>>> I have no idea why the API has been designed this way but Carl-Eric
>>>>> gave a good explanation - the component is not yet fully
>>>>> constructed.
>>>>> 
>>>>> The thing that bothers me is why it is OK to use the instance in my
>>>>> custom IComponentInstantiationListener and it is not OK to do the
>>>>> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
>>>>> If there is a javadoc explaining the possible problem (as for
>>>>> IComponentInstantiationListener#onInstantiation()) then it is OK.
>>>>> 
>>>>> Even more - at
>>>>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
>>>>> can see that right ater rejecting the *Class* we pass the
>>>>> *instance* to
>>>>> the UnauthorizedComponentInstantiationListener!
>>>>> 
>>>>> 
>>>>> Martin Grigorov
>>>>> Wicket Training and Consulting
> 


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
That was basically my point there. Handing out instances here invites
exactly that kind of bug, where the developer expects to be able to get
the instance to do anything sensible, only to have that (mysteriously)
fail.

Carl-Eric

On Fri, 20 Dec 2013 10:32:45 +0100
Sven Meier <sv...@meiers.net> wrote:

> The reporter of the issue states that a "call to isErrorPage() would
> be more elegant in that situation"
> 
> If the passed instance is not yet fully constructed, how can 
> #isErrorPage() return anything useful other than static information? 
> Isn't the class or any of its annotations sufficient in this case?
> 
> Sven
> 
> On 12/20/2013 10:19 AM, Carl-Eric Menzel wrote:
> > Duh. I knew I missed an obvious one! You are of course right.
> >
> > So we can't get rid of that one easily. That said, I'm still wary of
> > spreading unfinished objects even further around the API.
> >
> > Carl-Eric
> >
> > On Fri, 20 Dec 2013 10:12:32 +0100
> > Sven Meier <sv...@meiers.net> wrote:
> >
> >> SpringComponentInjector?
> >>
> >> Sven
> >>
> >> On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> >>> I agree that something should be cleaned up. But like I said in
> >>> the comment to that Jira ticket, I think it should in fact move
> >>> more toward passing the class rather than the uninitialized
> >>> instance. I like my objects to be predictable, and I like it even
> >>> more when I don't have to think about any traps, even if they are
> >>> well-documented.
> >>>
> >>> I think (I also wrote that in the comment) that perhaps
> >>> IComponentInstantiationListener should be deprecated and
> >>> eventually removed, since IComponentInitializationListener should
> >>> be able to cover all the same use cases without having the
> >>> unfinished constructor problem.
> >>>
> >>> Is there a use case that would not be handled by the
> >>> initialization listener?
> >>>
> >>> Carl-Eric
> >>>
> >>> On Fri, 20 Dec 2013 10:53:40 +0200
> >>> Martin Grigorov <mg...@apache.org> wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
> >>>> asked to pass the Component instance
> >>>> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
> >>>> just its class.
> >>>> I have no idea why the API has been designed this way but
> >>>> Carl-Eric gave a good explanation - the component is not yet
> >>>> fully constructed.
> >>>>
> >>>> The thing that bothers me is why it is OK to use the instance in
> >>>> my custom IComponentInstantiationListener and it is not OK to do
> >>>> the same in IAuthorizationStrategy#isInstantiationAuthorized() ?
> >>>> If there is a javadoc explaining the possible problem (as for
> >>>> IComponentInstantiationListener#onInstantiation()) then it is OK.
> >>>>
> >>>> Even more - at
> >>>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> >>>> can see that right ater rejecting the *Class* we pass the
> >>>> *instance* to
> >>>> the UnauthorizedComponentInstantiationListener!
> >>>>
> >>>>
> >>>> Martin Grigorov
> >>>> Wicket Training and Consulting
> 


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Martin Grigorov <mg...@apache.org>.
On Fri, Dec 20, 2013 at 11:32 AM, Sven Meier <sv...@meiers.net> wrote:

> The reporter of the issue states that a "call to isErrorPage() would be
> more elegant in that situation"
>
> If the passed instance is not yet fully constructed, how can
> #isErrorPage() return anything useful other than static information? Isn't
> the class or any of its annotations sufficient in this case?


https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/markup/html/pages/AbstractErrorPage.java?source=cc#L49
will
work fine.
anything that requires more state may not work.

And yes, using the class type or annotations will do the job for his use
case.

My question is more about the overall consistency.
It is strange that the unauthorized listener receives the instance right
after the rejected class.


>
>
> Sven
>
>
> On 12/20/2013 10:19 AM, Carl-Eric Menzel wrote:
>
>> Duh. I knew I missed an obvious one! You are of course right.
>>
>> So we can't get rid of that one easily. That said, I'm still wary of
>> spreading unfinished objects even further around the API.
>>
>> Carl-Eric
>>
>> On Fri, 20 Dec 2013 10:12:32 +0100
>> Sven Meier <sv...@meiers.net> wrote:
>>
>>  SpringComponentInjector?
>>>
>>> Sven
>>>
>>> On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
>>>
>>>> I agree that something should be cleaned up. But like I said in the
>>>> comment to that Jira ticket, I think it should in fact move more
>>>> toward passing the class rather than the uninitialized instance. I
>>>> like my objects to be predictable, and I like it even more when I
>>>> don't have to think about any traps, even if they are
>>>> well-documented.
>>>>
>>>> I think (I also wrote that in the comment) that perhaps
>>>> IComponentInstantiationListener should be deprecated and eventually
>>>> removed, since IComponentInitializationListener should be able to
>>>> cover all the same use cases without having the unfinished
>>>> constructor problem.
>>>>
>>>> Is there a use case that would not be handled by the initialization
>>>> listener?
>>>>
>>>> Carl-Eric
>>>>
>>>> On Fri, 20 Dec 2013 10:53:40 +0200
>>>> Martin Grigorov <mg...@apache.org> wrote:
>>>>
>>>>  Hi,
>>>>>
>>>>> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
>>>>> asked to pass the Component instance
>>>>> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
>>>>> just its class.
>>>>> I have no idea why the API has been designed this way but Carl-Eric
>>>>> gave a good explanation - the component is not yet fully
>>>>> constructed.
>>>>>
>>>>> The thing that bothers me is why it is OK to use the instance in my
>>>>> custom IComponentInstantiationListener and it is not OK to do the
>>>>> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
>>>>> If there is a javadoc explaining the possible problem (as for
>>>>> IComponentInstantiationListener#onInstantiation()) then it is OK.
>>>>>
>>>>> Even more - at
>>>>> https://github.com/apache/wicket/blob/master/wicket-
>>>>> core/src/main/java/org/apache/wicket/Application.java#L276you
>>>>> can see that right ater rejecting the *Class* we pass the
>>>>> *instance* to
>>>>> the UnauthorizedComponentInstantiationListener!
>>>>>
>>>>>
>>>>> Martin Grigorov
>>>>> Wicket Training and Consulting
>>>>>
>>>>
>

Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Sven Meier <sv...@meiers.net>.
The reporter of the issue states that a "call to isErrorPage() would be 
more elegant in that situation"

If the passed instance is not yet fully constructed, how can 
#isErrorPage() return anything useful other than static information? 
Isn't the class or any of its annotations sufficient in this case?

Sven

On 12/20/2013 10:19 AM, Carl-Eric Menzel wrote:
> Duh. I knew I missed an obvious one! You are of course right.
>
> So we can't get rid of that one easily. That said, I'm still wary of
> spreading unfinished objects even further around the API.
>
> Carl-Eric
>
> On Fri, 20 Dec 2013 10:12:32 +0100
> Sven Meier <sv...@meiers.net> wrote:
>
>> SpringComponentInjector?
>>
>> Sven
>>
>> On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
>>> I agree that something should be cleaned up. But like I said in the
>>> comment to that Jira ticket, I think it should in fact move more
>>> toward passing the class rather than the uninitialized instance. I
>>> like my objects to be predictable, and I like it even more when I
>>> don't have to think about any traps, even if they are
>>> well-documented.
>>>
>>> I think (I also wrote that in the comment) that perhaps
>>> IComponentInstantiationListener should be deprecated and eventually
>>> removed, since IComponentInitializationListener should be able to
>>> cover all the same use cases without having the unfinished
>>> constructor problem.
>>>
>>> Is there a use case that would not be handled by the initialization
>>> listener?
>>>
>>> Carl-Eric
>>>
>>> On Fri, 20 Dec 2013 10:53:40 +0200
>>> Martin Grigorov <mg...@apache.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
>>>> asked to pass the Component instance
>>>> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
>>>> just its class.
>>>> I have no idea why the API has been designed this way but Carl-Eric
>>>> gave a good explanation - the component is not yet fully
>>>> constructed.
>>>>
>>>> The thing that bothers me is why it is OK to use the instance in my
>>>> custom IComponentInstantiationListener and it is not OK to do the
>>>> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
>>>> If there is a javadoc explaining the possible problem (as for
>>>> IComponentInstantiationListener#onInstantiation()) then it is OK.
>>>>
>>>> Even more - at
>>>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
>>>> can see that right ater rejecting the *Class* we pass the
>>>> *instance* to
>>>> the UnauthorizedComponentInstantiationListener!
>>>>
>>>>
>>>> Martin Grigorov
>>>> Wicket Training and Consulting


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
Duh. I knew I missed an obvious one! You are of course right.

So we can't get rid of that one easily. That said, I'm still wary of
spreading unfinished objects even further around the API.

Carl-Eric

On Fri, 20 Dec 2013 10:12:32 +0100
Sven Meier <sv...@meiers.net> wrote:

> SpringComponentInjector?
> 
> Sven
> 
> On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> > I agree that something should be cleaned up. But like I said in the
> > comment to that Jira ticket, I think it should in fact move more
> > toward passing the class rather than the uninitialized instance. I
> > like my objects to be predictable, and I like it even more when I
> > don't have to think about any traps, even if they are
> > well-documented.
> >
> > I think (I also wrote that in the comment) that perhaps
> > IComponentInstantiationListener should be deprecated and eventually
> > removed, since IComponentInitializationListener should be able to
> > cover all the same use cases without having the unfinished
> > constructor problem.
> >
> > Is there a use case that would not be handled by the initialization
> > listener?
> >
> > Carl-Eric
> >
> > On Fri, 20 Dec 2013 10:53:40 +0200
> > Martin Grigorov <mg...@apache.org> wrote:
> >
> >> Hi,
> >>
> >> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
> >> asked to pass the Component instance
> >> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
> >> just its class.
> >> I have no idea why the API has been designed this way but Carl-Eric
> >> gave a good explanation - the component is not yet fully
> >> constructed.
> >>
> >> The thing that bothers me is why it is OK to use the instance in my
> >> custom IComponentInstantiationListener and it is not OK to do the
> >> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
> >> If there is a javadoc explaining the possible problem (as for
> >> IComponentInstantiationListener#onInstantiation()) then it is OK.
> >>
> >> Even more - at
> >> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> >> can see that right ater rejecting the *Class* we pass the
> >> *instance* to
> >> the UnauthorizedComponentInstantiationListener!
> >>
> >>
> >> Martin Grigorov
> >> Wicket Training and Consulting
> 


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Sven Meier <sv...@meiers.net>.
SpringComponentInjector?

Sven

On 12/20/2013 10:09 AM, Carl-Eric Menzel wrote:
> I agree that something should be cleaned up. But like I said in the
> comment to that Jira ticket, I think it should in fact move more toward
> passing the class rather than the uninitialized instance. I like my
> objects to be predictable, and I like it even more when I don't have to
> think about any traps, even if they are well-documented.
>
> I think (I also wrote that in the comment) that perhaps
> IComponentInstantiationListener should be deprecated and eventually
> removed, since IComponentInitializationListener should be able to cover
> all the same use cases without having the unfinished constructor
> problem.
>
> Is there a use case that would not be handled by the initialization
> listener?
>
> Carl-Eric
>
> On Fri, 20 Dec 2013 10:53:40 +0200
> Martin Grigorov <mg...@apache.org> wrote:
>
>> Hi,
>>
>> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
>> asked to pass the Component instance
>> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
>> just its class.
>> I have no idea why the API has been designed this way but Carl-Eric
>> gave a good explanation - the component is not yet fully constructed.
>>
>> The thing that bothers me is why it is OK to use the instance in my
>> custom IComponentInstantiationListener and it is not OK to do the
>> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
>> If there is a javadoc explaining the possible problem (as for
>> IComponentInstantiationListener#onInstantiation()) then it is OK.
>>
>> Even more - at
>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
>> can see that right ater rejecting the *Class* we pass the *instance*
>> to
>> the UnauthorizedComponentInstantiationListener!
>>
>>
>> Martin Grigorov
>> Wicket Training and Consulting


Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()

Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
I agree that something should be cleaned up. But like I said in the
comment to that Jira ticket, I think it should in fact move more toward
passing the class rather than the uninitialized instance. I like my
objects to be predictable, and I like it even more when I don't have to
think about any traps, even if they are well-documented.

I think (I also wrote that in the comment) that perhaps
IComponentInstantiationListener should be deprecated and eventually
removed, since IComponentInitializationListener should be able to cover
all the same use cases without having the unfinished constructor
problem.

Is there a use case that would not be handled by the initialization
listener?

Carl-Eric

On Fri, 20 Dec 2013 10:53:40 +0200
Martin Grigorov <mg...@apache.org> wrote:

> Hi,
> 
> The reporter of https://issues.apache.org/jira/browse/WICKET-5454
> asked to pass the Component instance
> to  IAuthorizationStrategy#isInstantiationAuthorized() instead of
> just its class.
> I have no idea why the API has been designed this way but Carl-Eric
> gave a good explanation - the component is not yet fully constructed.
> 
> The thing that bothers me is why it is OK to use the instance in my
> custom IComponentInstantiationListener and it is not OK to do the
> same in IAuthorizationStrategy#isInstantiationAuthorized() ?
> If there is a javadoc explaining the possible problem (as for
> IComponentInstantiationListener#onInstantiation()) then it is OK.
> 
> Even more - at
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/Application.java#L276you
> can see that right ater rejecting the *Class* we pass the *instance*
> to
> the UnauthorizedComponentInstantiationListener!
> 
> 
> Martin Grigorov
> Wicket Training and Consulting