[
https://issues.apache.org/struts/browse/STR-3151?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=44025#action_44025
]
T.J. Hill commented on STR-3151:
--------------------------------
A scenario was described in my earlier comment. I came across this while
tracking down a bug that turned out to be in my code, but at first, thought
this issue was the lone culprit. Sure - changing ActionMessages.isAccessed()
from mutating to purely interrogative could break existing usages. Plenty of
ways around it when dealing with the scenario described earlier. I've re-up'd
and moved on. So in hindsight, priority should probably be lowered. And if
this hasn't come up before, then maybe precedence wins out and the issue be
canceled.
Although I still regard ActionMessages.isAccessed() as troubled, I yield to the
status quo.
Thanks for your time and quick feedback - TH
> messagesPresent should not alter ActionMessages.accessed flag
> -------------------------------------------------------------
>
> Key: STR-3151
> URL: https://issues.apache.org/struts/browse/STR-3151
> Project: Struts 1
> Issue Type: Bug
> Components: Taglibs
> Affects Versions: 1.3.8
> Environment: n/a
> Reporter: T.J. Hill
> Assignee: Paul Benedict
> Fix For: 1.4.0
>
>
> 20080603 12:53
> The html|nested:messagesPresent tags set ActionMessages.accessed flag to true
> when interrogating.
> Currently, MessagesPresentTag calls the methods get() and get(String) of
> ActionMessages to determine if any messages exist. However, both get(...)
> methods in ActionMessages set the accessed field to true. So any subsequent
> call to ActionMessages.isAccessed() by external code will get true,
> regardless if the messages were truly consumed.
> Below is the current condition method in MessagesPresentTag:
> protected boolean condition(boolean desired)
> throws JspException {
> ActionMessages am = null;
> String key = name;
> if ((message != null) && "true".equalsIgnoreCase(message)) {
> key = Globals.MESSAGE_KEY;
> }
> try {
> am = TagUtils.getInstance().getActionMessages(pageContext, key);
> } catch (JspException e) {
> TagUtils.getInstance().saveException(pageContext, e);
> throw e;
> }
> Iterator iterator = (property == null) ? am.get() : am.get(property);
> // HERE is the issue -- using get(...) methods will cause the accessed
> property of ActionMessages to be set to true.
> return (iterator.hasNext() == desired);
> }
> Perhaps the following change would be appropriate to resolve the issue.
> Modify the condition(boolean) method in MesssagesPresentTag to call the
> existing ActionMessages.size() [when property not specified] and the exsiting
> ActionMessages.size(String) method above [when property specified]:
>
> protected boolean condition(boolean desired)
> throws JspException {
> ActionMessages am = null;
>
> String key = name;
>
> if ((message != null) && "true".equalsIgnoreCase(message)) {
> key = Globals.MESSAGE_KEY;
> }
>
> try {
> am = TagUtils.getInstance().getActionMessages(pageContext, key);
> } catch (JspException e) {
> TagUtils.getInstance().saveException(pageContext, e);
> throw e;
> }
>
> // --- CURRENT ---
> //Iterator iterator = (property == null) ? am.get() :
> am.get(property);
> //return (iterator.hasNext() == desired);
>
> // +++ SUGGESTED +++
> int present = (property == null) ? am.size() : am.size(property);
> return ((present > 0) == desired);
> }
>
> Thanks - TH
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
