Re: Passing Component or Class to IAuthorizationStrategy#isInstantiationAuthorized()
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 igor.vaynb...@gmail.comwrote: 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 mgrigo...@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.vaynb...@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 mgrigo...@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()
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 mgrigo...@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()
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 s...@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 mgrigo...@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()
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 s...@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 s...@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 mgrigo...@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()
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-e...@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 mgrigo...@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 cmen...@wicketbuch.dewrote: 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 s...@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 mgrigo...@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()
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 mgrigo...@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()
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 igor.vaynb...@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 mgrigo...@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()
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 mgrigo...@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.vaynb...@gmail.comwrote: 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 mgrigo...@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
