myfaces-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jwald...@apache.org
Subject svn commit: r1607709 - in /myfaces/trinidad/trunk: trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/ trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/ trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin...
Date Thu, 03 Jul 2014 18:25:03 GMT
Author: jwaldman
Date: Thu Jul  3 18:25:03 2014
New Revision: 1607709

URL: http://svn.apache.org/r1607709
Log:
TRINIDAD-2490 skin additions are not loaded for simple, minimal and casablanca skins
reviewed by Prakash Udupa
thanks to Anand Nath for the patch.

Modified:
    myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/SkinAddition.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinImpl.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinUtils.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/BaseSkinProvider.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/ExternalSkinProvider.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/SkinProviderRegistry.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/TrinidadSkinProvider.java

Modified: myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/SkinAddition.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/SkinAddition.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/SkinAddition.java
(original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/skin/SkinAddition.java
Thu Jul  3 18:25:03 2014
@@ -335,7 +335,7 @@ public class SkinAddition implements Com
   }
 
   /**
-   * convinience builder for SkinAddition
+   * convenience builder for SkinAddition
    * does not support the deprecated ValueBinding for translationSource
    */
   public static class Builder

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
Thu Jul  3 18:25:03 2014
@@ -427,30 +427,39 @@ public final class GlobalConfiguratorImp
           _setupRequestContextFactory();
 
           // Create a new SkinFactory if needed.
+          // SkinFactory is now deprecated. For backward compatibility, SkinFactory internally
+          // calls SkinProvider. So when we init SkinFactory we need to init SkinProvider
as well.
+          // SkinProviderRegistry is the implementation of SkinProvider. This in turn needs
other
+          // internal SkinProviders such as ExternalSkinProvider, TrinidadSkinProvider to
be
+          // available. So we init all of that one by one.
           if (SkinFactory.getFactory() == null)
           {
             SkinFactory.setFactory(new SkinFactoryImpl());
           }
 
-          // init external skin provider
-          // this has to be done before SkinProviderRegistry, because SkinProviderRegistry
uses this
-          Object externalSkinProvider = ec.getApplicationMap().get(ExternalSkinProvider.EXTERNAL_SKIN_PROVIDER_KEY);
+          // init external skin provider before init of SkinProviderRegistry
+          Object externalSkinProvider = ec.getApplicationMap()
+                                          .get(ExternalSkinProvider.EXTERNAL_SKIN_PROVIDER_KEY);
 
           if (externalSkinProvider == null)
-            ec.getApplicationMap().put(ExternalSkinProvider.EXTERNAL_SKIN_PROVIDER_KEY, new
ExternalSkinProvider());
+            ec.getApplicationMap().put(ExternalSkinProvider.EXTERNAL_SKIN_PROVIDER_KEY,
+                                       new ExternalSkinProvider());
 
-          // init trinidad skin provider
-          // this has to be done before SkinProviderRegistry, because SkinProviderRegistry
uses this
-          Object trinidadSkinProvider = ec.getApplicationMap().get(TrinidadSkinProvider.TRINDIAD_SKIN_PROVIDER_KEY);
+          // init trinidad skin provider before init of SkinProviderRegistry
+          Object trinidadSkinProvider = ec.getApplicationMap()
+                                          .get(TrinidadSkinProvider.TRINDIAD_SKIN_PROVIDER_KEY);
 
           if (trinidadSkinProvider == null)
-            ec.getApplicationMap().put(TrinidadSkinProvider.TRINDIAD_SKIN_PROVIDER_KEY, new
TrinidadSkinProvider());
+            ec.getApplicationMap().put(TrinidadSkinProvider.TRINDIAD_SKIN_PROVIDER_KEY,
+                                       new TrinidadSkinProvider());
 
           // init skin provider
-          Object provider = ec.getApplicationMap().get(SkinProvider.SKIN_PROVIDER_INSTANCE_KEY);
+          Object provider = ec.getApplicationMap()
+                              .get(SkinProvider.SKIN_PROVIDER_INSTANCE_KEY);
 
           if (provider == null)
-            ec.getApplicationMap().put(SkinProvider.SKIN_PROVIDER_INSTANCE_KEY, new SkinProviderRegistry());
+            ec.getApplicationMap().put(SkinProvider.SKIN_PROVIDER_INSTANCE_KEY,
+                                       new SkinProviderRegistry());
 
           // init the config property service
           ConfigPropertyServiceImpl.initialize(ec);

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinImpl.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinImpl.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinImpl.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinImpl.java
Thu Jul  3 18:25:03 2014
@@ -28,6 +28,7 @@ import java.util.MissingResourceExceptio
 import java.util.ResourceBundle;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.el.ELContext;
@@ -257,9 +258,7 @@ abstract public class SkinImpl extends S
   /**
    * Adds a SkinAddition on this Skin. You can call this method as many times
    * as you like for the Skin, and it will add the SkinAddition to the list of
-   * SkinAdditions.
-   * However, it does not make sense to call this method more than once
-   * with the same SkinAddition object.
+   * SkinAdditions, if it is unique.
    * This is meant for the skin-addition use-cases, where a custom component
    * developer has a style sheet and/or resource bundle for their custom
    * components, and they want the style sheet and/or resource bundle
@@ -274,23 +273,19 @@ abstract public class SkinImpl extends S
    * @throws NullPointerException if SkinAddition is null.
    */
   @Override
-  public void addSkinAddition (
-    SkinAddition skinAddition
-    )
+  public void addSkinAddition(SkinAddition skinAddition)
   {
-     if (skinAddition == null)
-       throw new NullPointerException("NULL_SKINADDITION");
+    if (skinAddition == null)
+      throw new NullPointerException("NULL_SKINADDITION");
 
-     if (_skinAdditions == null)
-     {
-       _skinAdditions = new ArrayList<SkinAddition>();
-     }
-   
-    //The following code will insert SkinAddition objects in order according to
-    //comparable.  This yields log(n) performance for ArrayList which is as good
-    //as it gets for this type of insertion.
-    int insertionPoint = Collections.binarySearch(_skinAdditions, skinAddition, null);
-    _skinAdditions.add((insertionPoint > -1) ? insertionPoint : (-insertionPoint) - 1,
skinAddition);
+    // _skinAdditions is set as ConcurrentSkipListSet.
+    // will insert SkinAddition objects in order according to
+    // comparable.  This yields log(n) performance which is as good
+    // as it gets for this type of insertion.
+    if (_skinAdditions.add(skinAddition) && _LOG.isInfo())
+    {
+      _LOG.info("ADDED_SKIN_ADDITION", new Object[]{skinAddition, this});
+    }
   }
 
   /**
@@ -308,7 +303,7 @@ abstract public class SkinImpl extends S
       return Collections.emptyList();
     }
     else
-      return Collections.unmodifiableList(_skinAdditions);
+      return Collections.unmodifiableList(new ArrayList<SkinAddition>(_skinAdditions));
   }
 
    /**
@@ -1361,8 +1356,9 @@ abstract public class SkinImpl extends S
   // plus all the SkinAdditions translation sources.
   private List<TranslationSource> _translationSourceList;
 
-  // List of skin-additions for this Skin
-  private List<SkinAddition> _skinAdditions;
+  // Set of skin-additions for this Skin
+  // creating the set here in order to make creation of this set thread-safe
+  private Set<SkinAddition> _skinAdditions = new ConcurrentSkipListSet<SkinAddition>();
 
   // Optional features for rendering
   protected Map<String, String> _skinFeatures;
@@ -1372,9 +1368,9 @@ abstract public class SkinImpl extends S
   private ConcurrentHashMap<Object, Object> _properties= new ConcurrentHashMap<Object,
Object>();
 
   private boolean _dirty;
-  
+
   private static final TrinidadLogger _LOG = TrinidadLogger.createTrinidadLogger(SkinImpl.class);
-  
+
   private static final String _FORCE_DISABLE_CONTENT_COMPRESSION_PARAM="org.apache.myfaces.trinidad.skin.disableStyleCompression";
 
 }

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinUtils.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinUtils.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/SkinUtils.java
Thu Jul  3 18:25:03 2014
@@ -122,32 +122,6 @@ public class SkinUtils
   }
 
   /**
-   * Adds skinAddition passed into the skin object passed, if the skin object does not have
the same
-   * skin addition already.
-   *
-   * @param skin
-   * @param skinAddition
-   * @return true if the SkinAddition was added into Skin and false if it was not.
-   */
-  static public boolean addSkinAdditionToSkinIfAbsent(Skin skin, SkinAddition skinAddition)
-  {
-    if (skin == null || skinAddition == null)
-      throw new NullPointerException("Skin or SkinAddition passed is null");
-
-    List<SkinAddition> additions = skin.getSkinAdditions();
-
-    if (skinAddition != null)
-    {
-      for (SkinAddition addn : additions)
-        if (addn != null && addn.equals(skinAddition))
-          return false;
-    }
-
-    skin.addSkinAddition(skinAddition);
-    return true;
-  }
-
-  /**
    * @param provider    skin provider
    * @param context     faces context
    * @param renderKitId renderKit Id for default skin

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/BaseSkinProvider.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/BaseSkinProvider.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/BaseSkinProvider.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/BaseSkinProvider.java
Thu Jul  3 18:25:03 2014
@@ -42,7 +42,7 @@ import org.apache.myfaces.trinidad.skin.
  * @See TrinidadSkinProvider
  * @See ExternalSkinProvider
  */
-public abstract class BaseSkinProvider extends SkinProvider
+abstract class BaseSkinProvider extends SkinProvider
 {
   public BaseSkinProvider()
   {
@@ -192,8 +192,8 @@ public abstract class BaseSkinProvider e
       _LOG.fine("SP_FINDING_SKIN_FOR", new Object[]{searchMetadata.toString()});
 
     // first check if a skin matching the requirement is present in this provider
-    SkinMetadata availableMetadata = _findSkinMetadata(context, searchMetadata);
 
+    SkinMetadata availableMetadata = _findSkinMetadata(context, searchMetadata);
     if (availableMetadata == null)
     {
       if (_LOG.isFine())

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/ExternalSkinProvider.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/ExternalSkinProvider.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/ExternalSkinProvider.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/ExternalSkinProvider.java
Thu Jul  3 18:25:03 2014
@@ -43,23 +43,18 @@ import org.apache.myfaces.trinidadintern
 public class ExternalSkinProvider extends BaseSkinProvider
 {
   /**
+   * Key for the ExternalSkinProvider stored in ExternalContext
+   */
+  public static final String EXTERNAL_SKIN_PROVIDER_KEY =
+    "org.apache.myfaces.trinidad.skin.EXTERNAL_SKIN_PROVIDER_INSTANCE";
+
+  /**
    * {@inheritDoc}
    */
   @Override
   public Skin getSkin(ExternalContext context, SkinMetadata skinMetadata)
   {
-    synchronized (this)
-    {
-      Skin skin = super.getSkin(context, skinMetadata);
-
-      // ensure that the skin's and its parent's skin additions are added before we return.
-      // see _ensureSkinAdditions method documentation for more information on why we need
to do
-      // this.
-      if (skin != null)
-        _ensureSkinAdditions(context, skin);
-
-      return skin;
-    }
+    return super.getSkin(context, skinMetadata);
   }
 
   /**
@@ -130,39 +125,6 @@ public class ExternalSkinProvider extend
     return esp;
   }
 
-  /**
-   * {@link org.apache.myfaces.trinidad.config.Configurator} allows access to SkinFactory
in its
-   * init() and reloadSkin() API. These are now deprecated. But for existing use cases where
user
-   * registers Skins during init() of Configurator, they obtains their base skin by doing
something
-   * like SkinFactory.getSkin(null, "simple.desktop"); Then custom skin is created using
the simple
-   * skin as base and put it into SkinFactory using SkinFactory.addSkin method. All skins
registered
-   * using SkinFactory.addSkin method lands up here in ExternalSkinProvider. Such skins will
not
-   * have the skin additions registered in trinidad-skins.xml because TrinidadSkinProvider
was not
-   * able to kick in and provide this information. So we need to ensure that skins returned
from
-   * ExternalSkinProvider the skin additions for that skin and its base skins are added,
before we
-   * return them to the caller.
-   *
-   * @param context
-   * @param skin
-   * @return
-   */
-  private Skin _ensureSkinAdditions(ExternalContext context, Skin skin)
-  {
-    // It is possible to optimize here by keeping track of skins which already went through
-    // this process. Such optimization will avoid repeating this operation for skins which
already
-    // went through this. However, this is a corner case as there are not many skins are
registered
-    // like this. Moreover we are deprecating SkinFactory.addSkin and it may be removed
-    // in a later release, so there is no point in adding premature optimization.
-    TrinidadSkinProvider trinidadSkinProvider = TrinidadSkinProvider.getCurrentInstance(context);
-    trinidadSkinProvider.ensureSkinAdditions(skin);
-    return skin;
-  }
-
-  /**
-   * Key for the ExternalSkinProvider stored in ExternalContext
-   */
-  public static final  String         EXTERNAL_SKIN_PROVIDER_KEY =
-    "org.apache.myfaces.trinidad.skin.EXTERNAL_SKIN_PROVIDER_INSTANCE";
-  private static final TrinidadLogger _LOG                       =
+  private static final TrinidadLogger _LOG =
     TrinidadLogger.createTrinidadLogger(ExternalSkinProvider.class);
 }
\ No newline at end of file

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/SkinProviderRegistry.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/SkinProviderRegistry.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/SkinProviderRegistry.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/SkinProviderRegistry.java
Thu Jul  3 18:25:03 2014
@@ -50,7 +50,7 @@ public class SkinProviderRegistry extend
 
   public SkinProviderRegistry()
   {
-    ExternalContext context = FacesContext.getCurrentInstance().getExternalContext();
+    ExternalContext externalContext = FacesContext.getCurrentInstance().getExternalContext();
     List<SkinProvider> services = ClassLoaderUtils.getServices(SkinProvider.class.getName());
 
     if (_LOG.isFine())
@@ -64,34 +64,35 @@ public class SkinProviderRegistry extend
     if (_LOG.isFine())
       _LOG.fine("Adding TrinidadSkinProvider... ");
 
-    providers.add(TrinidadSkinProvider.getCurrentInstance(context));
+    providers.add(TrinidadSkinProvider.getCurrentInstance(externalContext));
 
     if (_LOG.isFine())
       _LOG.fine("Adding TrinidadBaseSkinProvider... ");
 
+    // this provider serves static base skins, so we can create a new instance here
     providers.add(new TrinidadBaseSkinProvider());
 
     if (_LOG.isFine())
       _LOG.fine("Adding ExternalSkinProvider... ");
 
-    providers.add(ExternalSkinProvider.getCurrentInstance(context));
+    providers.add(ExternalSkinProvider.getCurrentInstance(externalContext));
 
     _providers = Collections.unmodifiableList(providers);
   }
 
   @Override
-  public Collection<SkinMetadata> getSkinMetadata(ExternalContext context)
+  public Collection<SkinMetadata> getSkinMetadata(ExternalContext externalContext)
   {
     Collection<SkinMetadata> metadata = new ArrayList<SkinMetadata>();
 
     for (SkinProvider provider : _providers)
-      metadata.addAll(provider.getSkinMetadata(context));
+      metadata.addAll(provider.getSkinMetadata(externalContext));
 
     return Collections.unmodifiableCollection(metadata);
   }
 
   /**
-   * @param context
+   * @param externalContext
    * @param skinMetadata search criteria object containing the information of skin to be
queried id,
    *                     family, renderKit, version are the information used from skin metadata
to
    *                     perform search for the skin requested. Other fields do not participate
in
@@ -101,7 +102,7 @@ public class SkinProviderRegistry extend
    * automatically assumes it as SkinMetadata.RenderKitId.DESKTOP
    */
   @Override
-  public Skin getSkin(ExternalContext context, SkinMetadata skinMetadata)
+  public Skin getSkin(ExternalContext externalContext, SkinMetadata skinMetadata)
   {
     _handleCircularDependency(skinMetadata);
 
@@ -113,7 +114,7 @@ public class SkinProviderRegistry extend
 
     for (SkinProvider provider : _providers)
     {
-      matchingSkin = provider.getSkin(context, skinMetadata);
+      matchingSkin = provider.getSkin(externalContext, skinMetadata);
 
       if (matchingSkin != null)
       {
@@ -143,12 +144,14 @@ public class SkinProviderRegistry extend
         _LOG.fine("NO MATCH. Will return a simple skin or null for skin metadata: " + skinMetadata);
 
       assert (skinMetadata.getRenderKitId() != null);
+
       // if renderKit is available return the simple skin for that renderKit
       // skinMetadata.getRenderKitId() can never be null, default renderKit will always be
DESKTOP
-      return _returnSkin(skinMetadata,
-                         SkinUtils.getDefaultSkinForRenderKitId(this,
-                                                                context,
-                                                                skinMetadata.getRenderKitId()));
+      matchingSkin = SkinUtils.getDefaultSkinForRenderKitId(this,
+                                                            externalContext,
+                                                            skinMetadata.getRenderKitId());
+      _processSkinForReturn(skinMetadata, matchingSkin);
+      return matchingSkin;
     }
 
 
@@ -160,12 +163,15 @@ public class SkinProviderRegistry extend
       if (_LOG.isFine())
         _LOG.fine("returning ONLY match for skin metadata: " + skinMetadata);
 
-      return _returnSkin(skinMetadata, matchingSkins.get(0));
+
+      matchingSkin = matchingSkins.get(0);
+      _processSkinForReturn(skinMetadata, matchingSkin);
+      return matchingSkin;
     }
 
     // at this point we have more than one matches for the search criteria passed
     // so, extract the best match based on version
-    return _versionFilter(context, skinMetadata, matchingSkins);
+    return _versionFilter(skinMetadata, matchingSkins);
   }
 
   /**
@@ -276,13 +282,11 @@ public class SkinProviderRegistry extend
    * not mentioned or version is default try to return default skin else try to return leaf
skin
    * else return first match
    *
-   * @param context
    * @param searchCriteria
    * @param matchingSkins
    * @return
    */
-  private Skin _versionFilter(ExternalContext context,
-                              SkinMetadata searchCriteria,
+  private Skin _versionFilter(SkinMetadata searchCriteria,
                               List<Skin> matchingSkins)
   {
     SkinVersion version = searchCriteria.getVersion();
@@ -298,7 +302,8 @@ public class SkinProviderRegistry extend
       if (_LOG.isFine())
         _LOG.fine("returning exact version match.");
 
-      return _returnSkin(searchCriteria, matchingSkin);
+      _processSkinForReturn(searchCriteria, matchingSkin);
+      return matchingSkin;
     }
 
     // either user is looking for default version or did not find the version requested
@@ -310,7 +315,8 @@ public class SkinProviderRegistry extend
     {
       if (_LOG.isFine())
         _LOG.fine("returning DEFAULT version match.");
-      return _returnSkin(searchCriteria, matchingSkin);
+      _processSkinForReturn(searchCriteria, matchingSkin);
+      return matchingSkin;
     }
 
     // the version that user asked for is not available
@@ -323,20 +329,45 @@ public class SkinProviderRegistry extend
       if (_LOG.isFine())
         _LOG.fine("return LEAF skin or one of the matches.");
 
-      return _returnSkin(searchCriteria, matchingSkin);
+      _processSkinForReturn(searchCriteria, matchingSkin);
+      return matchingSkin;
     }
 
     // we failed to find any better result for the given version, so return first match
     if (_LOG.isFine())
       _LOG.fine("nothing worked so return first match.");
 
-    return _returnSkin(searchCriteria, matchingSkins.get(0));
+    matchingSkin = matchingSkins.get(0);
+    _processSkinForReturn(searchCriteria, matchingSkins.get(0));
+    return matchingSkin;
   }
 
-  private Skin _returnSkin(SkinMetadata skinMetadata, Skin skin)
+  /**
+   * This method processes a skin that is being returned from the provider.
+   * 1. remove the skin from the context tracking
+   * 2. add any missing skin additions to the skin
+   * @param skinMetadata
+   * @return
+   */
+  private void _processSkinForReturn(SkinMetadata skinMetadata,
+                                     Skin skin)
   {
     FacesContext context = FacesContext.getCurrentInstance();
+    _clearReturningSkinContext(context, skinMetadata);
+
+    // before returning the skin, ensure that all skin additions defined in trinidad-skins.xml
+    // are added.
+    TrinidadSkinProvider.getCurrentInstance(context.getExternalContext()).ensureSkinAdditions(skin);
+  }
 
+  /**
+   * We track the skin being requested to build the hierarchy so that we can identify circular
+   * dependencies. This method clears the information for the present Skin that is being
returned.
+   * @param context
+   * @param skinMetadata
+   */
+  private void _clearReturningSkinContext(FacesContext context, SkinMetadata skinMetadata)
+  {
     Map attrMap = context.getAttributes();
     Object o = attrMap.get(_SKIN_PROVIDER_CONTEXT);
     List<SkinMetadata> requesters;
@@ -353,8 +384,6 @@ public class SkinProviderRegistry extend
         _LOG.finer("Context now is " + requesters);
       }
     }
-
-    return skin;
   }
 
   /**
@@ -458,6 +487,13 @@ public class SkinProviderRegistry extend
     return null;
   }
 
+  /**
+   * We track the skin being requested to build the hierarchy so that we can identify circular
+   * dependencies. This method adds the currently requested skin into the context. If a skin
is
+   * re-requested, then an exception will be thrown
+   * @param skinMetadata
+   * @throws IllegalStateException if a circular dependency is detected
+   */
   private void _handleCircularDependency(SkinMetadata skinMetadata)
   {
     FacesContext context = FacesContext.getCurrentInstance();
@@ -475,7 +511,7 @@ public class SkinProviderRegistry extend
 
     if (requesters.contains(skinMetadata))
     {
-      String message = "Circlular dependency detected whille loading skin: " + skinMetadata
+      String message = "Circular dependency detected while loading skin: " + skinMetadata
                          + ". Requesters are: " + requesters;
       if (_LOG.isSevere())
         _LOG.severe(message);

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/TrinidadSkinProvider.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/TrinidadSkinProvider.java?rev=1607709&r1=1607708&r2=1607709&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/TrinidadSkinProvider.java
(original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/provider/TrinidadSkinProvider.java
Thu Jul  3 18:25:03 2014
@@ -46,7 +46,7 @@ public final class TrinidadSkinProvider 
    * Key for the TrinidadSkinProvider stored in ExternalContext
    */
   public static final String TRINDIAD_SKIN_PROVIDER_KEY =
-    "org.apache.myfaces.trinidad.skin.TRINDIAD_SKIN_PROVIDER_INSTANCE";
+    "org.apache.myfaces.trinidad.skin.TRINIDAD_SKIN_PROVIDER_INSTANCE";
 
   /**
    * static factory method to get hold of a TrinidadSkinProvider object This can be used
for easy
@@ -66,24 +66,6 @@ public final class TrinidadSkinProvider 
   }
 
   /**
-   * used by ExternalSkinProvider to ensure that the skin and its base skin additions are
added
-   * correctly
-   *
-   * @param skin
-   */
-  public void ensureSkinAdditions(Skin skin)
-  {
-    if (_skinAdditionNodes == null || _skinAdditionNodes.isEmpty())
-      return;
-
-    for (SkinAddition addition : _skinAdditionNodes)
-    {
-      // skin additions in _skinAdditionNodes will not be null
-      _checkAndAddInHierarchy(skin, addition);
-    }
-  }
-
-  /**
    * {@inheritDoc}
    */
   @Override
@@ -148,31 +130,7 @@ public final class TrinidadSkinProvider 
       _LOG.fine("Creating skin extension for : " + skinMetadata);
 
     // features object itself cannot be null
-    Skin loadedSkin = new SkinExtension(baseSkin, matchingNode);
-
-    if (_skinAdditionNodes != null)
-      for (SkinAddition addition : _skinAdditionNodes)
-      {
-        String additionSkinId = addition.getSkinId();
-
-        if (id.equals(additionSkinId))
-        {
-          if (_LOG.isFine())
-            _LOG.fine("Adding skin addition : " + addition);
-
-          loadedSkin.addSkinAddition(addition);
-        }
-
-        if (baseSkinId.equals(additionSkinId))
-        {
-          boolean added = SkinUtils.addSkinAdditionToSkinIfAbsent(baseSkin, addition);
-
-          if (added && _LOG.isFine())
-            _LOG.fine("Adding parent skin addition: " + addition);
-        }
-      }
-
-    return loadedSkin;
+    return new SkinExtension(baseSkin, matchingNode);
   }
 
   /**
@@ -223,6 +181,23 @@ public final class TrinidadSkinProvider 
     }
   }
 
+  /**
+   * used to ensure that the skin and its base skin additions are added correctly
+   * This is called at the exit point of SkinProviderRegistry
+   * @param skin
+   */
+  void ensureSkinAdditions(Skin skin)
+  {
+    if (_skinAdditionNodes == null || _skinAdditionNodes.isEmpty())
+      return;
+
+    for (SkinAddition addition : _skinAdditionNodes)
+    {
+      // skin additions in _skinAdditionNodes will not be null
+      _checkAndAddInHierarchy(skin, addition);
+    }
+  }
+
   private void _checkAndAddInHierarchy(Skin skin, SkinAddition addition)
   {
     // exit condition for the recursive call
@@ -233,10 +208,7 @@ public final class TrinidadSkinProvider 
 
     if (skinId != null && skinId.equals(skin.getId()))
     {
-      boolean added = SkinUtils.addSkinAdditionToSkinIfAbsent(skin, addition);
-
-      if (added && _LOG.isFine())
-        _LOG.fine("Adding skin addition : " + addition);
+      skin.addSkinAddition(addition);
     }
 
     _checkAndAddInHierarchy(skin.getBaseSkin(), addition);



Mime
View raw message