Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Portlet bridge unable to wrap Flash implementation #1071

Closed
eclipse-faces-bot opened this issue Feb 14, 2012 · 20 comments
Closed

Portlet bridge unable to wrap Flash implementation #1071

eclipse-faces-bot opened this issue Feb 14, 2012 · 20 comments

Comments

@eclipse-faces-bot
Copy link

Unless Java reflection is used, it is not possible for a portlet bridge to use the JSF 2.0/2.1 API to wrap the Flash implementation provided by Mojarra or MyFaces.

In order to solve this problem, this issue serves as a proposal to add javax.faces.context.FlashWrapper and javax.faces.context.FlashFactory to the JSF API.

Background:

The JSF 2.0/2.1 API does not currently provide a factory-style way of obtaining Flash scope instances. Instead, the ExternalContext#getFlash() method inside the JSF runtime is responsible for acting as
a pseudo-factory for creating instances.

One solution for a portlet bridge would be to create its own implementation of the Flash interface. However it is much more desirable to simply wrap the Mojarra/MyFaces flash implementation and override methods where necessary.

  1. Add a FlashWrapper class:

package javax.faces.context;
public abstract class FlashWrapper extends Flash implements FacesWrapper

Precedent/Example:
http://javaserverfaces.java.net/nonav/docs/2.0/javadocs/javax/faces/application/ApplicationWrapper.html

  1. Also to add a FlashFactory:

package javax.faces.context;
public abstract class FlashFactory
extends Object implements FacesWrapper
public abstract Flash getFlash();

The wrappable FlashFactory instance would take a one-arg constructor and look something like this:

public class FlashFactoryImpl extends FlashFactory {

private FlashFactory wrappedFactory;

public FlashFactoryImpl(FlashFactory flashFactory)

{ wrappedFactory = flashFactory; }

@OverRide
public Flash getFlash()

{ return new FlashImpl(wrappedFactory.getFlash()); }

@OverRide
public FlashFactory getWrapped()

{ return wrappedFactory; }

}

Precedent/Example:
http://javaserverfaces.java.net/nonav/docs/2.0/javadocs/javax/faces/application/ApplicationFactory.html

  1. Add constant FLASH_FACTORY to FactoryFinder and require JSF implementations to return a wrappable FlashFactory instance.

http://javaserverfaces.java.net/nonav/docs/2.0/javadocs/javax/faces/FactoryFinder.html

Affected Versions

[2.0, 2.1]

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
Reported by ngriffin7a

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
Issue-Links:
is related to
JAVASERVERFACES_SPEC_PUBLIC-1070

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
Neil, I'll meet you half way. Here's the first part of the work already done.

I need you to fill out the API javadoc for the two new classes in jsf-api. The real kicker is my insistence on the color coded changebars.

I'm about to write a FAQ entry on how to do those. When I have done so, I'll point to it from this issue.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
File: 20120214-1203-i_spec_1071_snapshot.patch
Attached By: @edburns

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
ngriffin7a said:
Thanks so much for this – I'd be happy to fill out the API JavaDoc.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
As promised, here's the FAQ entry.

https://wikis.oracle.com/display/GlassFish/JavaServerFacesRI#JavaServerFacesRI-HowdoImakesurethatchangestothespechavethecorrectversionchangebarattributions%3F

Neil, please download, apply, edit (following the recommendations of the FAQ entry), re-diff, and attach a new patch.

Let me know when you've done it and I'll go to the next base.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
File: 20120214-1203-i_spec_1071_snapshot-javadoc.patch
Attached By: ngriffin7a

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
ngriffin7a said:
JavaDoc changes incorporated into original patch

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
Looks great! I'll take it to the next base. I'll be rounding third toward home tomorrow.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
svncom
Sending jsf-api/doc/web-facesconfig_2_2.xsd
Sending jsf-api/src/main/java/javax/faces/FactoryFinder.java
Adding jsf-api/src/main/java/javax/faces/context/FlashFactory.java
Adding jsf-api/src/main/java/javax/faces/context/FlashWrapper.java
Sending jsf-api/src/main/resources/overview.html
Sending jsf-ri/resources/jsf-ri-config.xml
Sending jsf-ri/src/main/java/com/sun/faces/config/processor/FactoryConfigProcessor.java
Sending jsf-ri/src/main/java/com/sun/faces/context/ExternalContextImpl.java
Sending jsf-ri/src/main/java/com/sun/faces/context/FacesContextFactoryImpl.java
Sending jsf-ri/src/main/java/com/sun/faces/context/flash/ELFlash.java
Sending jsf-ri/src/main/java/com/sun/faces/context/flash/FlashELResolver.java
Adding jsf-ri/src/main/java/com/sun/faces/context/flash/FlashFactoryImpl.java
Adding jsf-test/#1071
Adding jsf-test/#1071/build.xml
Adding jsf-test/#1071/i_spec_1071_war
Adding jsf-test/#1071/i_spec_1071_war/pom.xml
Adding jsf-test/#1071/i_spec_1071_war/src
Adding jsf-test/#1071/i_spec_1071_war/src/main
Adding jsf-test/#1071/i_spec_1071_war/src/main/java
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces/test
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces/test/i_spec_1071_war
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces/test/i_spec_1071_war/EchoFlash.java
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces/test/i_spec_1071_war/MyFlashFactory.java
Adding jsf-test/#1071/i_spec_1071_war/src/main/java/com/sun/faces/test/i_spec_1071_war/MyFlashImpl.java
Adding jsf-test/#1071/i_spec_1071_war/src/main/resources
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp/WEB-INF
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp/WEB-INF/beans.xml
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp/WEB-INF/faces-config.xml
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp/WEB-INF/web.xml
Adding jsf-test/#1071/i_spec_1071_war/src/main/webapp/i_spec_1071_war.xhtml
Sending jsf-test/build.xml
Transmitting file data ......................
Committed revision 9683.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
ngriffin7a said:
Thanks Ed!!!

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
Thank goodness for thorough tests.

One testcase that only failed in virtual server mode started failing after this commit and it found this problem:

FactoryConfigProcessor#
private static final String[] FACTORY_NAMES = {

doesn't have FlashFactory.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
svncom
Sending jsf-ri/src/main/java/com/sun/faces/config/processor/FactoryConfigProcessor.java
Transmitting file data .
Committed revision 9735.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:
Re-open at Neil's request to add wrapper.

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@edburns said:

On Sat, 24 Nov 2012 15:11:44 -0500, Neil Griffin said:

NG> Regarding #1071 [1], the signature for
NG> the FlashFactory.getFlash [2] method currently looks like this:

NG> public abstract Flash getFlash(ExternalContext context, boolean create);

NG> But I think that the signature should look like the following:

NG> public abstract Flash getFlash(boolean create);

NG> The ExternalContext parameter is used by Mojarra as an internal
NG> implementation detail, and should not be passed as a
NG> parameter. Instead, the Mojarra FlashImpl constructor should call
NG> FacesContext.getCurrentInstance().getExternalContext().

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
Marked as fixed on Monday, December 3rd 2012, 2:43:51 pm

@eclipse-faces-bot
Copy link
Author

@glassfishrobot Commented
@manfredriem said:
Closing resolved issue out

@eclipse-faces-bot
Copy link
Author

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

@eclipse-faces-bot
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant