[jira] [Comment Edited] (TRINIDAD-2515) Avoid caching skins from external SkinProviders and make LRU cache concurrent

["Anand V Nath (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/TRINIDAD-2515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14172070#comment-14172070 ] 

Anand V Nath edited comment on TRINIDAD-2515 at 10/15/14 6:48 AM:
------------------------------------------------------------------

This patch contains images so please follow the following instructions for applying the patch:

1. Extract test-skin-and-images.zip to trinidad check out folder (the folder that contains trinidad-impl folder)
2. Apply the patch. Run {{svn patch jira-2515.patch}}
3. Run the following command to add the images from trinidad check out folder:
{code}
svn add trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/purple trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/suede trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/beach
{code}


was (Author: anand.v.nath@oracle.com):
This patch contains images so please follow the following instructions for applying the patch:

1. Extract test-skin-and-images.zip to trinidad check out folder (the folder that contains trinidad-impl folder)
2. Apply the patch. Run {{svn patch jira-2515.patch}}
3. Run the following command to add the images from trinidad check out folder:
{{svn add trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/purple trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/suede trinidad-impl/src/test/resources/org/apache/myfaces/trinidadinternal/skin/beach}}

> Avoid caching skins from external SkinProviders and make LRU cache concurrent
> -----------------------------------------------------------------------------
>
>                 Key: TRINIDAD-2515
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-2515
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>          Components: Skinning
>    Affects Versions: 2.1.0-core
>            Reporter: Anand V Nath
>         Attachments: jira-2515.patch, test-skin-and-images.zip
>
>
> Problem statement:
> SkinProvider SPI allows creation of external skin repositories and plug it into trinidad skin framework. Skin framework should allow such skins to be managed (created, cached, destroyed) by the SkinProvider. Currently this is not happening.
> Internal problem description:
> When user requests a Skin, skin framework creates a StyleProvider for it. This StyleProvider is cached at application level and reused for subsequent requests for the same Skin. Since the StyleProvider contains the Skin, it effectively caches the Skin also.
> Solution:
> Don't cache StyleProvider associated with Skins served from external SkinProviders. For achieveing this introduce isCacheable API on SkinImpl and implement it in SkinExtension:
> {code}
>   /**
>    * Used by SkinStyleProvider to decide whether to cache the StyleProvider or not.
>    * @return true if skin is internal to the framework.
>    */
>   public abstract boolean isCacheable();
> {code}
> In SkinExtension introduce overloaded constructor for handling internal Skin creation and external Skin creation (through SkinFactory.createSkin)
> {code}
>   /**
>    * Creates SkinExtension from SkinMetadata and base Skin
>    * This constructor is used for creating skins for external SkinProvider implementations.
>    * We keep skins thus created as not-cacheable.
>    * @see org.apache.myfaces.trinidadinternal.skin.SkinFactoryImpl
>    * @param baseSkin
>    * @param skinMetadata
>    */
>   public SkinExtension(Skin baseSkin, SkinMetadata skinMetadata)
>   {
>     this(baseSkin, skinMetadata, false);
>   }
>   /**
>    * Creates SkinExtension from SkinMetadata, base Skin and cacheability
>    * This constructor is used only for INTERNAL skins
>    * @see org.apache.myfaces.trinidadinternal.skin.provider.TrinidadSkinProvider
>    * @param baseSkin
>    * @param skinMetadata
>    */
>   public SkinExtension(Skin baseSkin, SkinMetadata skinMetadata, boolean isCacheable)
>   {
>     // existing code:
>     // read values from skinMetadata and set
>     _isCacheable = isCacheable;
>   }
> {code}
> Check SkinImpl.isCacheable() before caching the StyleProvider.
> For solving the concurrency issue in LRU cache used for caching StyleProviders, introduce new CopyOnWriteArrayMap and use it.
> For testing add SkinProvider test cases, CopyOnWriteArrayMap test cases.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)