You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by "Christopher Schultz (JIRA)" <de...@velocity.apache.org> on 2007/10/30 15:09:51 UTC
[jira] Created: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
LinkTool could easily be modified to support current-request parameter duplication
----------------------------------------------------------------------------------
Key: VELTOOLS-88
URL: https://issues.apache.org/jira/browse/VELTOOLS-88
Project: Velocity Tools
Issue Type: New Feature
Reporter: Christopher Schultz
Priority: Trivial
When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Updated: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz updated VELTOOLS-88:
----------------------------------------
Attachment: VELTOOLS-88.diff
Replaced deleted patch for historical pusposes.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Commented: (VELTOOLS-88) LinkTool could easily be modified
to support current-request parameter duplication
Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12538881 ]
Nathan Bubna commented on VELTOOLS-88:
--------------------------------------
Nice feature. It sorta overlaps the self() method when selfIncludeParameters is changed to true, but as the default for that is false, this is a cleaner way to add them than doing $link.params($request.parameterMap), especially with the ability to ignore certain params. That said, here's some critiques before we put this in:
- The self() method will need to respect the paramsToIgnore when it is configured to include params.
- I'm not thrilled about using addQueryData(name,value) for each individual parameter. If there's a lot of params, that will needlessly create and discard a lot of LinkTool clones. I think it would be better if it copied the request.getParameterMap(), removed any params to be ignored from that and then passed the result to params(Map).
- The javadoc references ignore(String) which is not in the patch.
- addIgnore() should return a copy, not this. Which means you'll probably need to add a new copyWithIgnore(String) method and make sure that the paramsToIgnore are always copied to new copies.
Sorry if that seems like a lot, but i'd like to avoid introducing inconsistencies in how LinkTool behaves. speaking of which, i just noticed a few things in it that need attention. :)
let me know if you have questions about my feedback. i'd be happy to help, as i think this is a nice new feature.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Updated: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz updated VELTOOLS-88:
----------------------------------------
Attachment: VELTOOLS-88.diff
Patch to implement this feature. It implements the ignore/add capability rather than the auto-ignore as described in the initial description.
Any and all comments are appreciated.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Closed: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz closed VELTOOLS-88.
---------------------------------------
Fix verified.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Fix For: 1.4, 1.x, 2.0
>
> Attachments: LinkToolTests.java, VELTOOLS-88.diff, VELTOOLS-88a.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Updated: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz updated VELTOOLS-88:
----------------------------------------
Attachment: VELTOOLS-88a.diff
Updated patch addresses Nathan's concerns.
Take a look.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: LinkToolTests.java, VELTOOLS-88.diff, VELTOOLS-88a.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Resolved: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Nathan Bubna (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nathan Bubna resolved VELTOOLS-88.
----------------------------------
Resolution: Fixed
Fix Version/s: 2.0
1.x
1.4
Ok, i've committed the patch and the tests with a few modifications (mostly stylistic, but also a few simplifications) to the trunk (1.x branch) and ported them over to the 2.x branch with a few more tweaks, including a refactor of addAllParameters() that i'll port back to 1.x momentarily...
Regarding the overzealous backup code in LinkTool.duplicate()... yeah, it's probably unnecessary. feel free to post a patch to excise it, if you feel so inclined.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Fix For: 1.4, 1.x, 2.0
>
> Attachments: LinkToolTests.java, VELTOOLS-88.diff, VELTOOLS-88a.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Commented: (VELTOOLS-88) LinkTool could easily be modified
to support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12538905 ]
Christopher Schultz commented on VELTOOLS-88:
---------------------------------------------
The use of addQueryData for each of the parameters was a carryover from the implementation of StrutsLinkTool, which was intended to use its superclass as a black box. I will look at the use of the params() method to streamline this code.
I will make the other changes and re-submit.
I have a unit test working without too much funny business. I'll include that unit test along with the patch.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Updated: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz updated VELTOOLS-88:
----------------------------------------
Attachment: (was: VELTOOLS-88.diff)
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Commented: (VELTOOLS-88) LinkTool could easily be modified
to support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12538911 ]
Christopher Schultz commented on VELTOOLS-88:
---------------------------------------------
Okay, I've made all those changes, and I think I fixed a bug along the way. The existing implementation of getSelf() had this at the bottom:
if (this.selfParams)
{
dupe.params(request.getParameterMap());
}
return dupe;
Note that the parameters copied in the dupe.params() call will be entirely lost, since dupe.params() returns a copy. The code is useless.
My new implementation properly does:
dupe = dupe.copyWith(params);
...which should result in the proper behavior.
Another odd thing I noticed in the class... the duplicate() method makes arrangements for non-cloneable implementations of LinkTool, yet LinkTool itself implements Cloneable. Why should we have to have any backup code in case LinkTool suddenly isn't cloneable?
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: VELTOOLS-88.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
[jira] Updated: (VELTOOLS-88) LinkTool could easily be modified to
support current-request parameter duplication
Posted by "Christopher Schultz (JIRA)" <de...@velocity.apache.org>.
[ https://issues.apache.org/jira/browse/VELTOOLS-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Schultz updated VELTOOLS-88:
----------------------------------------
Attachment: LinkToolTests.java
Unit test to exercise new "ignore" and "add all parameters" capability.
> LinkTool could easily be modified to support current-request parameter duplication
> ----------------------------------------------------------------------------------
>
> Key: VELTOOLS-88
> URL: https://issues.apache.org/jira/browse/VELTOOLS-88
> Project: Velocity Tools
> Issue Type: New Feature
> Reporter: Christopher Schultz
> Priority: Trivial
> Attachments: LinkToolTests.java, VELTOOLS-88.diff, VELTOOLS-88a.diff
>
>
> When implementing a pager a long time ago, I created a subclass of the StrutsLinkTool that added two methods: ignore(String paramName) and addAllParameters(). Together, they could be used to copy all the parameters from the current request into the link currently being built, and you could ignore a parameter from the current request using the ignore method.
> I hadn't seen any demand for such a feature out in the wild, so I kept it to myself.
> Now that I'm looking into Struts 2, I can see that their link builder tool has the same capability. Since that team decided to add this feature, I'm offering to add it to the LinkTool, which will provide the same capability to all subclasses such as StrutsLinkTool and SecureLinkTool.
> The change would not affect any existing code out in the wild: this would merely be a new method which supports the new feature. Its use is entirely optional and should not disturb any other code.
> The behavior of the Struts 2 link builder is to replace any current-request parameter explicitly set in the link builder, while my implementation requires that you explicitly ignore those parameters. My enhancement can follow either strategy (and I'd appreciate feedback on this item).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org