Bug 49548 - Refactoring SetSupport
Summary: Refactoring SetSupport
Status: RESOLVED FIXED
Alias: None
Product: Taglibs
Classification: Unclassified
Component: Standard Taglib (show other bugs)
Version: 1.2.0
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-04 12:22 UTC by Jeremy Boynes
Modified: 2010-07-06 22:24 UTC (History)
0 users



Attachments
Refactors SetSupport (22.09 KB, patch)
2010-07-04 12:23 UTC, Jeremy Boynes
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Boynes 2010-07-04 12:22:01 UTC
This started with "add some more test cases for the setting code" and got more momentum.

Patch primarily adds test cases and refactors the large "if" clause that sets the result into smaller methods. It also clears up IDE warnings (at least from IDEA).

One side effect was I18N of the exception message for "SET_BAD_DEFERRED_SCOPE" This also required changing the POM so that the resources were included (that perhaps could have been a separate patch). We could also move the resource bundles to Maven's default of src/main/resources.

I18N also needed access to the original value (which was not being retained) and in conjunction with that I replaced the "scopeSpecified" indicator with a check on whether the "scope" attribute is null.

I also changed the comment where the exception is thrown when target is null. This can happen as target is a request-time attribute and the spec does define this behaviour.

When setting a bean property, the for loop now exits after setting its property rather than continuing to iterate through all of them. This should have no impact unless a bean can have two PropertyDescriptors with the same name and I don't believe it can.

Finally, all throws of JspException were replaced with JspTagException as these are errors coming from a Tag Handler rather than the JSP Engine itself.
Comment 1 Jeremy Boynes 2010-07-04 12:23:01 UTC
Created attachment 25694 [details]
Refactors SetSupport
Comment 2 Henri Yandell 2010-07-06 02:35:45 UTC
pom.xml changes applied (r960812).
Comment 3 Henri Yandell 2010-07-06 02:54:53 UTC
SetSupport and .properties changes also committed (r960817).

On the tests - what are Syntax1 and Syntax3?
Comment 4 Jeremy Boynes 2010-07-06 09:20:16 UTC
(In reply to comment #3)
> On the tests - what are Syntax1 and Syntax3?

They are references to the "Syntax" section in the spec. Syntax one is where a variable is set from the value attribute, syntax 3 is where a property on some target option is set. The description in the spec refers to them like that, which I will confess assumes you're looking at the spec doc :-)
Comment 5 Henri Yandell 2010-07-06 22:24:07 UTC
Makes sense. I think it's a fair bit of tribal knowledge to have to pick up and I like the methodical spec based approach it leads to.

svn ci -m "Adding more tests for SetSupport from Jeremy Boynes' patch to #49548. The Syntax1/Syntax3 notations refers to the specification. "
Sending        impl/src/test/java/org/apache/taglibs/standard/tag/common/core/TestSetSupport.java
Transmitting file data .
Committed revision 961058.