Parties that decorate FaceletCacheFactory must use reflection: not good

[eclipse-faces-bot]

Consider this simple case.

public class MDSFaceletCacheFactory extends FaceletCacheFactory
{
  public MDSFaceletCacheFactory(FaceletCacheFactory wrapped)
  {
    _wrapped = wrapped;
  }

  @Override
  public FaceletCache getFaceletCache()
  {
    FaceletCache defaultCache = _wrapped.getFaceletCache();
    return new MDSFaceletCache(defaultCache);
  }

  private final FaceletCacheFactory _wrapped;
}

and this one

public final class MDSFaceletCache
  extends FaceletCache
{
  public MDSFaceletCache(FaceletCache defaultFaceletCache)
  {
    _defaultFaceletCache = defaultFaceletCache;
  }
...

This is the natural way to use this API, but yet it wall cause a NullPointerException

java.lang.NullPointerException
	at com.sun.faces.facelets.impl.DefaultFaceletCache$2.newInstance(DefaultFaceletCache.java:107)
	at com.sun.faces.facelets.impl.DefaultFaceletCache$2.newInstance(DefaultFaceletCache.java:102)
	at com.sun.faces.util.ExpiringConcurrentCache$1.call(ExpiringConcurrentCache.java:99)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	at com.sun.faces.util.ExpiringConcurrentCache.get(ExpiringConcurrentCache.java:114)
	at com.sun.faces.facelets.impl.DefaultFaceletCache.getViewMetadataFacelet(DefaultFaceletCache.java:156)
	at com.sun.faces.facelets.impl.DefaultFaceletCache.getViewMetadataFacelet(DefaultFaceletCache.java:64)
	at com.sun.faces.test.servlet30.facelets.faceletCacheFactory.MDSFaceletCache.getViewMetadataFacelet(MDSFaceletCache.java:119)
	at com.sun.faces.facelets.impl.DefaultFaceletFactory.getMetadataFacelet(DefaultFaceletFactory.java:323)
	at com.sun.faces.facelets.impl.DefaultFaceletFactory.getMetadataFacelet(DefaultFaceletFactory.java:253)
	at com.sun.faces.application.view.ViewMetadataImpl.createMetadataView(ViewMetadataImpl.java:138)
	at com.sun.faces.lifecycle.RestoreViewPhase.execute(RestoreViewPhase.java:241)
	at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
	at com.sun.faces.lifecycle.RestoreViewPhase.doPhase(RestoreViewPhase.java:121)
	at com.sun.faces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:198)
	at javax.faces.webapp.FacesServlet.service(FacesServlet.java:646)

Affected Versions

[2.1, 2.2]

[eclipse-faces-bot]

@glassfishrobot Commented
Reported by @edburns

[eclipse-faces-bot]

@glassfishrobot Commented
Issue-Links:
is related to
JAVASERVERFACES_SPEC_PUBLIC-1177
JAVASERVERFACES-1592
JAVASERVERFACES_SPEC_PUBLIC-777
is related to
JAVASERVERFACES-3755

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
The reason for the NPE is that while the MDSFaceletCache does have its setMemberFactories() method called, there is no way for the container to ensure that initialization step is passed on to the decorated instance such as in

FaceletCache defaultCache = _wrapped.getFaceletCache();

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
The original design of this API was covered in #777, which was introduced in JSF 2.1.

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
It would seem we should pass the MemberFactory instances into FaceletCacheFactory.getFaceletCache() rather than out of band using the protected FaceletCache.setMemberFactories() method.

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
One reason this hasn't cropped up before is that ADF has been using the context param "com.sun.faces.faceletCache" introduced in JAVASERVERFACES-1592 and only now has to use the FaceletCacheFactory API.

[eclipse-faces-bot]

@glassfishrobot Commented
prakashudupa said:
ADF does have FaceletCacheFactory implementation for quite some time now. This issue did not surface earlier because because our FacletCache implementation always served the facelet well for all cases where our ResourceResolver implementation provided the URL. Composite component usecase is pretty new for us, and we now find it failing. The underlying issue is that JSF RI does not call our ResourceResolver for composite component case (which makes it a JSF bug). We tried to do a workaround based on observation by Mojarra team that the DefaultFaceletChange works fine for composite component case. Workaround is to delegate to DefaultFaceletChange for composite component case, upon doing so we hit this NPE.

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
While it is true that this issue didn't surface until MDS started using composite components, that is not the problem being tracked by this issue. This issue is tracking the fact that the initialization contract for the FaceletCacheFactory must include the runtime supplying the factory with the means to produce Facelet and metadata Facelet instances. Currently this is done with an additional step aside from calling FaceletCacheFactory.getFaceletCache().

[eclipse-faces-bot]

@glassfishrobot Commented
File: i_spec_1346-test-app.zip
Attached By: @edburns

[eclipse-faces-bot]

@glassfishrobot Commented
@edburns said:
1346-FaceletCacheFactoryInitializationContract PROPOSAL

The existing contract for creating the FaceletCache is the following:

FaceletCacheFactory cacheFactory = (FaceletCacheFactory)
FactoryFinder.getFactory(FactoryFinder.FACELET_CACHE_FACTORY);
FaceletCache cache = cacheFactory.getFaceletCache();

followed by a reflective invocation of the setMemberFactories() on the cache.

This is wrong in two ways.

1. It requires reflection

2. It is impossible to decorate the cache so that the setMemberFactories
call happens.

I propose modify the contract to remove the need for the reflective call.

The following steps must be taken.

• Deprecate FaceletCache method

protected void setMemberFactories()

• Add FaceletCache method

public void FaceletCache.setMemberFactoriesPublic().

With a default implementation that calls setMemberFactories(). This
is necessary because we can't make an existing protected method public
without breaking existing classes that extend FaceletCacheFactory.

[eclipse-faces-bot]

@glassfishrobot Commented
Marked as fixed on Wednesday, February 4th 2015, 1:10:25 pm

[eclipse-faces-bot]

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

[eclipse-faces-bot]

• Issue Imported From: Parties that decorate FaceletCacheFactory must use reflection: not good javaee/javaserverfaces-spec#1346
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @edburns
• Original Issue Closed By:@glassfishrobot