add a JUnit test to make sure that classes implementing FacesWrapper do wrap all public and protected methods of the wrapped class

[eclipse-faces-bot]

Often it gets forgotten to change the wrapper classes whe a method is changed or added to the wrapped class. This can result in unexpected behavior.

Therfore I developed the attached JUnit test to scan all classes implementing FacesWrapper and make sure the wrappers do wrap all public and protected methods of the wrapped class.

On the current 2.1.0 development stage as of 23. dec 2010, the following missing wrapper methods where detected:

ResourceWrapper:

• getContentType()
• getLibraryName()
• getResourceName()
• setContentType(String)
• setLibraryName(String)
• setResourceName(String)

ExternalContextWrapper:

• getSessionMaxInactiveInterval()
• isSecure()
• setSessionMaxInactiveInterval()

PartialViewContextWrapper

• setPartialRequest(boolean)

Since the above methods would cause API change and 2.1 closed, this has to be planned for 2.2.

See also attached patch file providing the FacesWrapperTestCase and the minimal change for the wrapper classes.

Affected Versions

[2.1]

[eclipse-faces-bot]

@glassfishrobot Commented
Reported by dueni

[eclipse-faces-bot]

@glassfishrobot Commented
Issue-Links:
depends on
JAVASERVERFACES_SPEC_PUBLIC-914
is related to
JAVASERVERFACES_SPEC_PUBLIC-938

[eclipse-faces-bot]

@glassfishrobot Commented
Was assigned to dueni

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
914 is related, but this one goes the full way to make sure wrappers do wrap all methods.

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
Make this a task.

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
Hi Ed,

I'd like to contribute that - please review patches and let me commit these changes.
While updating on this issue I also fixed some missing/wrong javadocs on ExteralContextWrapper - see latest attachement.

regards
Hanspeter

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
Hi Ed,

just updated the patch to also include the build.xml changes to run the FacesWrapperTestCase.

I was able to run "ant clean main test.with.container.refresh" with no error. Please review the patch or let someone test it so I can commit the changes.

Since these changes also affect the TCK for JSF 2.2, how are the changes sent to the TCK developers?

regards
Hanspeter

[eclipse-faces-bot]

@glassfishrobot Commented
File: spec-917-FacesWrapper-testcase.patch
Attached By: dueni

[eclipse-faces-bot]

@glassfishrobot Commented
i_oss said:
Hi Hanspeter,
nice solution!!!

Ed: maybe you want to add this one: http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-938 as a duplicate or "going to be fixed by this" .

Thank you,
Imre

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
I like this. r=edburns.

This is the first change to any class in package javax.faces.application in JSF 2.2. Therefore, make sure to modify javax/faces/application/package.html to include changed_modified_2_2 in the span there where the rest of the changes are flagged.

Please commit to trunk when the extenal hudson jobs for trunk are all clean <http://hudson.glassfish.org/view/JSF%20Mojarra/>.

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
updated patch with changed_added_2_2/changed_modified_2_2 markers

[eclipse-faces-bot]

@glassfishrobot Commented
File: spec-917-FacesWrapper-testcase-with-docs.patch
Attached By: dueni

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
Added FacesWrapperTestCase to ensure wraper classes do wrap all the public/protected methods of the wrapped class.
By adding this test missing wrapper methods on 3 existing wrappers where detected:

javax.faces.application.ResourceWrapper:

• getContentType()
• getLibraryName()
• getResourceName()
• setContentType(String)
• setLibraryName(String)
• setResourceName(String)
  javax.faces.context.ExternalContextWrapper:
• getSessionMaxInactiveInterval()
• isSecure()
• setSessionMaxInactiveInterval()
  javax.faces.context.PartialViewContextWrapper
• setPartialRequest(boolean)

r=edburns

SECTION: Modified files

M jsf-api/build.xml
M jsf-api/build-source.xml
M jsf-api/src/main/java/javax/faces/application/package.html
M jsf-api/src/main/java/javax/faces/application/ResourceWrapper.java
M jsf-api/src/main/java/javax/faces/context/ExternalContextWrapper.java
M jsf-api/src/main/java/javax/faces/context/package.html
M jsf-api/src/main/java/javax/faces/context/PartialViewContextWrapper.java
A jsf-api/src/test/java/javax/faces/context/FacesWrapperTestCase.java

[eclipse-faces-bot]

@glassfishrobot Commented
Marked as fixed on Wednesday, May 18th 2011, 11:32:26 pm

[eclipse-faces-bot]

@glassfishrobot Commented
File: spec-917-FacesWrapper-testcase-ready-for-commit.patch
Attached By: dueni

[eclipse-faces-bot]

@glassfishrobot Commented
dueni said:
Patch created immediately before commit.

[eclipse-faces-bot]

@glassfishrobot Commented
@manfredriem said:
Closing resolved issue out

[eclipse-faces-bot]

@glassfishrobot Commented
This issue was imported from java.net JIRA JAVASERVERFACES_SPEC_PUBLIC-917

[eclipse-faces-bot]

• Issue Imported From: add a JUnit test to make sure that classes implementing FacesWrapper do wrap all public and protected methods of the wrapped class javaee/javaserverfaces-spec#917
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @glassfishrobot
• Original Issue Closed By:@glassfishrobot