On May 10, 2008, at 1:46 AM, Sergiu Dumitriu wrote:

vmassol (SVN) wrote:

...

Author: vmassol Date: 2008-05-07 20:29:05 +0200 (Wed, 07 May 2008) New Revision: 9684

Modified: xwiki-platform/core/branches/xwiki-core-1.4/xwiki-core/src/main/ java/com/xpn/xwiki/render/XWikiVelocityRenderer.java Log: XWIKI-2369: Error when custom skin defined in wiki page has a macros.vm property

Modified: xwiki-platform/core/branches/xwiki-core-1.4/xwiki-core/ src/main/java/com/xpn/xwiki/render/XWikiVelocityRenderer.java =================================================================== --- xwiki-platform/core/branches/xwiki-core-1.4/xwiki-core/src/main/ java/com/xpn/xwiki/render/XWikiVelocityRenderer.java 2008-05-07 16:53:26 UTC (rev 9683) +++ xwiki-platform/core/branches/xwiki-core-1.4/xwiki-core/src/main/ java/com/xpn/xwiki/render/XWikiVelocityRenderer.java 2008-05-07 18:29:05 UTC (rev 9684) @@ -104,6 +104,40 @@ }

/** + * @return the key used to cache the Velocity Engines. We have one Velocity Engine + * per skin which has a macros.vm file on the filesystem. Right now we don't + * support macros.vm defined in custom skins in wiki pages. + */ + private static String getVelocityEngineCacheKey(String skin, XWikiContext context) + { + // We need the path relative to the webapp's home folder so we need to remove all path before + // the skins/ directory. This is a bit of a hack and should be improved with a proper api. + String skinMacros = context.getWiki().getSkinFile("macros.vm", skin, context); + String cacheKey; + if (skinMacros != null) { + // We're only using the path starting with the skin name since sometimes we'll + // get ".../skins/skins/<skinname>/...", sometimes we get ".../skins/<skinname>/...", + // sometimes we get "skins/<skinname>/..." and if the skin is done in wiki pages + // we get ".../skin/...". + int pos = skinMacros.indexOf("skins/"); + if (pos > -1) { + cacheKey = skinMacros.substring(pos); + } else { + // If the macros.vm file is stored in a wiki page (in a macros.vm property in + // a XWikiSkins object) then we use the parent skin's macros.vm since we + // currently don't support having global velocimacros defined in wiki pages. + String baseSkin = context.getWiki().getBaseSkin(context);

This can lead to an infinite loop if someone uses the skin document as its own base skin... I'd add a check here, just to be sure.

Yep. However this code is not correct and I'm working on a proper solution so that we can support both macros.vm defined in skins in wiki page and Global velocimacros defined in wiki pages. -Vincent

...

+ cacheKey = getVelocityEngineCacheKey(baseSkin, context); + } + } else { + // If no skin macros.vm file exists then use a "default" cache id + cacheKey = "default"; + } + + return cacheKey; + } + + /** * @todo Move this initialization code to a Skin Manager component. */ public static VelocityEngine getVelocityEngine(XWikiContext context) throws XWikiVelocityException @@ -118,19 +152,7 @@

// Get the location of the skin's macros.vm file String skin = context.getWiki().getSkin(context); - // We need the path relative to the webapp's home folder so we need to remove all path before - // the skins/ directory. This is a bit of a hack and should be improved with a proper api. - String skinMacros = context.getWiki().getSkinFile("macros.vm", skin, context); - String cacheKey; - if (skinMacros != null) { - // We're only using the path starting with the skin name since sometimes we'll - // get /skins/skins/<skinname>/..., sometimes we get "/ skins/<skinname>/..." - // and sometimes we get "skins/<skinname>/... - cacheKey = skinMacros.substring(skinMacros.indexOf("skins/")); - } else { - // If no skin macros.vm file exists then use a "default" cache id - cacheKey = "default"; - } + String cacheKey = getVelocityEngineCacheKey(skin, context);

// Get the Velocity Engine to use VelocityFactory velocityFactory = @@ -141,7 +163,7 @@ } else { // Gather the global Velocity macros that we want to have. These are skin dependent. Properties properties = new Properties(); - String macroList = "/templates/macros.vm" + ((skinMacros == null) ? "" : "," + cacheKey); + String macroList = "/templates/macros.vm" + (cacheKey.equals("default") ? "" : "," + cacheKey); properties.put(RuntimeConstants.VM_LIBRARY, macroList); velocityEngine = velocityFactory.createVelocityEngine(cacheKey, properties); }