You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Dominik Drzewiecki (JIRA)" <ji...@apache.org> on 2009/02/13 11:13:59 UTC

[jira] Created: (WICKET-2101) PageLink is inherently non extendable

PageLink is inherently non extendable
-------------------------------------

                 Key: WICKET-2101
                 URL: https://issues.apache.org/jira/browse/WICKET-2101
             Project: Wicket
          Issue Type: Bug
          Components: wicket
    Affects Versions: 1.4-RC2
            Reporter: Dominik Drzewiecki
             Fix For: 1.4-RC3


Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().

I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.

	public Class<? extends Page> getPageClass()
	{
		return pageLink.getPageIdentity();
	}

Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WICKET-2101) PageLink is inherently non extendable

Posted by "Dominik Drzewiecki (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Dominik Drzewiecki updated WICKET-2101:
---------------------------------------

    Attachment: link.patch

This patch ensures the extensibility of both PageLink and BookmarkablePageLink. Moreover, it makes the BookmarkablePageLink a descendant of the PageLink, thus stripping some common code. Be aware that the class resolution as seen in original BookmarkablePageLink has been scraped. All tests seem to pass. I'll also provide a revisited patch for https://issues.apache.org/jira/browse/WICKET-2089

> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2101) PageLink is inherently non extendable

Posted by "Dominik Drzewiecki (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673448#action_12673448 ] 

Dominik Drzewiecki commented on WICKET-2101:
--------------------------------------------

Hmm. what do you suggest then? IMHO, it was only the (String,Page) constructor that was evil. For many cases PageLink (used wisely) will suffice. 
Right now many applications make use of PageLink. Should they be rewritten (for the sake of being future-proof) to use BookmarkablePageLink instead? It seems to me that if for some reason PageLink  has been left in 1.4, the rest of the framework cannot act as if it had been scraped. Therefore it would be nice if the PageLink provided the very same interface (or its subset) as the class that is to supersede it. And the PageLink is lacking getPageClass() method. Please provide it as specified above, as without it there's no reasonable way (i.e. not offending OOP principles) of achieving what is an aim of WICKET-2089. 

> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2101) PageLink is inherently non extendable

Posted by "Dominik Drzewiecki (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673373#action_12673373 ] 

drzewo edited comment on WICKET-2101 at 2/13/09 12:47 PM:
----------------------------------------------------------------------

This patch ensures the extensibility of both PageLink and BookmarkablePageLink. Moreover, it makes the BookmarkablePageLink a descendant of the PageLink, thus stripping some common code. Be aware that the class resolution as seen in original BookmarkablePageLink has been scraped. All tests seem to pass. I'll also provide a revised patch for https://issues.apache.org/jira/browse/WICKET-2089

      was (Author: drzewo):
    This patch ensures the extensibility of both PageLink and BookmarkablePageLink. Moreover, it makes the BookmarkablePageLink a descendant of the PageLink, thus stripping some common code. Be aware that the class resolution as seen in original BookmarkablePageLink has been scraped. All tests seem to pass. I'll also provide a revisited patch for https://issues.apache.org/jira/browse/WICKET-2089
  
> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (WICKET-2101) PageLink is inherently non extendable

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg updated WICKET-2101:
----------------------------------

      Priority: Minor  (was: Major)
    Issue Type: Improvement  (was: Bug)

> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>            Assignee: Igor Vaynberg
>            Priority: Minor
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (WICKET-2101) PageLink is inherently non extendable

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673387#action_12673387 ] 

Igor Vaynberg commented on WICKET-2101:
---------------------------------------

as far as i remember we had plans to deprecate and remove pagelink from core. im not sure why we havent done this in 1.4 but its too late now. in 1.5 we will remove it i think. there is nothing you cant do with pagelink that you cant do with a regular Link or regular BookmarkablePageLink, especially since we deprecated (String,Page) constructor.

> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (WICKET-2101) PageLink is inherently non extendable

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Igor Vaynberg resolved WICKET-2101.
-----------------------------------

    Resolution: Fixed
      Assignee: Igor Vaynberg

what do i propose? well...since it made it into the RC we cannot remove it. so i have deprecated it and we will remove it in 1.5. i have added the accessor you need so you can do whatever it is you need to. i did not change bookmarkablepagelink, since pagelink is going away there is no sense to do that.

> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>            Assignee: Igor Vaynberg
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (WICKET-2101) PageLink is inherently non extendable

Posted by "Igor Vaynberg (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/WICKET-2101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673387#action_12673387 ] 

ivaynberg edited comment on WICKET-2101 at 2/13/09 1:55 PM:
----------------------------------------------------------------

as far as i remember we had plans to deprecate and remove pagelink from core. im not sure why we havent done this in 1.4 but its too late now. in 1.5 we will remove it i think. there is nothing you can do with pagelink that you cant do with a regular Link or regular BookmarkablePageLink, especially since we deprecated (String,Page) constructor.

      was (Author: ivaynberg):
    as far as i remember we had plans to deprecate and remove pagelink from core. im not sure why we havent done this in 1.4 but its too late now. in 1.5 we will remove it i think. there is nothing you cant do with pagelink that you cant do with a regular Link or regular BookmarkablePageLink, especially since we deprecated (String,Page) constructor.
  
> PageLink is inherently non extendable
> -------------------------------------
>
>                 Key: WICKET-2101
>                 URL: https://issues.apache.org/jira/browse/WICKET-2101
>             Project: Wicket
>          Issue Type: Bug
>          Components: wicket
>    Affects Versions: 1.4-RC2
>            Reporter: Dominik Drzewiecki
>             Fix For: 1.4-RC3
>
>         Attachments: link.patch
>
>
> Due to the fact that there is no accessor provided for the private final IPageLink pageLink, there's no way of writing subclasses that would be aware of the pages they are linking to. My particular case is closely related to the annotation based authorization and an enhancement proposal http://issues.apache.org/jira/browse/WICKET-2089. Now, provided that this enhancement (that otherwise needs more look into) won't make it into wicket/wicket-auth-roles (though I am more than convinced that it should), and I would have to put the proposed IAuthorisationStrategy implementation outside core wicket, I would also have to write (copy&paste) my own PageLink that exposes its pageLink member in order to retrieve the page class it is referring to and substitute all occurernces of org.apache.wicket.markup.html.link.PageLink with it in my projects, so that my IAuthorisationStrategy implementation can retrieve the page class the the link is pointing to. Please note, that BokmarkablePageLink, which also is constructed with Page class *does* expose page class identity via public final Class<? extends Page> getPageClass().
> I suggest that for the sake of keeping the API clean and coherent the method with the same signature as in BookmarkablePageLink should be added to the PageLink.
> 	public Class<? extends Page> getPageClass()
> 	{
> 		return pageLink.getPageIdentity();
> 	}
> Another question arises - why BookmarkablePageLink is not a subclass of PageLink and why class resolution for those classes differs so much. Wouldn't it be easier to keep the class reference in the BookmarkablePageClass rather than its textual representation and skip all this mumbo-jumbo class resolution. But this is quite another story and OT for this issue.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.