You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by an...@apache.org on 2012/03/14 19:18:27 UTC

svn commit: r1300671 - in /myfaces/trinidad/branches/andys-skin-pregen: trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/ trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/ trinidad-impl/src/main/java/org/apache/myfaces/trinida...

Author: andys
Date: Wed Mar 14 18:18:26 2012
New Revision: 1300671

URL: http://svn.apache.org/viewvc?rev=1300671&view=rev
Log:
Checkpoint: error handling overhaul.

Added:
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/FileUtils.java
Modified:
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/AllVariantsSkinPregenerator.java
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationService.java
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationUtils.java
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerator.java
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/config/PregenConfig.java
    myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts

Added: myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/FileUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/FileUtils.java?rev=1300671&view=auto
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/FileUtils.java (added)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/FileUtils.java Wed Mar 14 18:18:26 2012
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.trinidad.util;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.myfaces.trinidad.logging.TrinidadLogger;
+
+/**
+ * File-related utilities.
+ */
+public final class FileUtils
+{
+  /**
+   * Converts the specified directory path to a File
+   * instance representing an existing, writable directory.
+   * 
+   * This method will attempt to create a directory on the
+   * file system if it does not already exist.
+   * 
+   * It does not, however, attempt to modify write permissions.
+   * 
+   * This method is thread safe.  However, there is no way to
+   * prevent other threads from modifying the file system, which
+   * can lead to unexpected results.  For example, toWritableDirectory
+   * may return a File that it thinks is writable, but which has
+   * actually been removed from the file system by another thread.
+   * 
+   * @param path the path to convert to a File
+   * @return a File corresponding to the specified path
+   * @throws IOException if directory at the specified path
+   *   does not exist/cannnot be created, or is not writable.
+   */
+  public static File toWritableDirectory(String path)
+    throws IOException
+  {
+    File directory = new File(path);
+    
+    synchronized (FileUtils.class)
+    {
+      if (!directory.exists()  && !directory.mkdirs())
+      {
+        _fail("CANNOT_CREATE_DIRECTORY", path);
+      }
+    }
+
+    if (!directory.canWrite())
+    {
+      _fail("DIRECTORY_NOT_WRITABLE", path);
+    }
+
+    return directory;
+  }
+  
+  private static void _fail(String messageKey, String path)
+    throws IOException
+  {
+    String message = _LOG.getMessage(messageKey, path);
+    throw new IOException(message);
+  }
+
+  private FileUtils()
+  {
+  }
+  
+  private static final TrinidadLogger _LOG =
+    TrinidadLogger.createTrinidadLogger(FileUtils.class);  
+}

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts Wed Mar 14 18:18:26 2012
@@ -529,4 +529,8 @@
 
 <resource key="UNEXPECTED_VERSION_VALUE">Unable to parse version segment {0} of version string {1}.</resource>
 
+<resource key="CANNOT_CREATE_DIRECTORY">The directory {0} does not exist and cannot be created.</resource>
+
+<resource key="DIRECTORY_NOT_WRITABLE">The directory {0} exists, but is not writable.</resource>
+
 </resources>

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/AllVariantsSkinPregenerator.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/AllVariantsSkinPregenerator.java?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/AllVariantsSkinPregenerator.java (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/AllVariantsSkinPregenerator.java Wed Mar 14 18:18:26 2012
@@ -29,12 +29,9 @@ import org.apache.myfaces.trinidad.loggi
 import org.apache.myfaces.trinidad.skin.Skin;
 
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent.Application;
-import org.apache.myfaces.trinidadinternal.renderkit.core.CoreRenderingContext;
-import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.TrinidadRenderingConstants;
 import org.apache.myfaces.trinidadinternal.skin.DocumentProviderSkin;
 import org.apache.myfaces.trinidadinternal.skin.pregen.config.PregenConfig;
 import org.apache.myfaces.trinidadinternal.skin.pregen.context.PregenStyleContext;
-import org.apache.myfaces.trinidadinternal.skin.pregen.context.PregenStyleProvider;
 import org.apache.myfaces.trinidadinternal.skin.pregen.variant.SkinVariant;
 import org.apache.myfaces.trinidadinternal.skin.pregen.variant.SkinVariants;
 import org.apache.myfaces.trinidadinternal.style.StyleContext;
@@ -51,26 +48,22 @@ import org.apache.myfaces.trinidadintern
  */
 class AllVariantsSkinPregenerator implements SkinPregenerator
 {
-  public AllVariantsSkinPregenerator(FacesContext context)
-  {
-    _generatedFilesPath = _getGeneratedFilesPath(context);
-    _generatedSkinFilesPath = _getGeneratedSkinFilesPath(_generatedFilesPath);
-  }
-
   @Override
   public void pregenerate(
     FacesContext    context, 
     Skin            skin,
-    PregenConfig    config
+    PregenConfig    config,
+    StyleProvider   provider
     )
   {
-    _pregenerateWithStats(context, skin, config);
+    _pregenerateWithStats(context, skin, config, provider);
   }
  
   private void _pregenerateWithStats(
     FacesContext   context,
     Skin           skin,
-    PregenConfig   config
+    PregenConfig   config,
+    StyleProvider  provider
     )
   {
     Stats stats = new Stats(skin);
@@ -78,7 +71,7 @@ class AllVariantsSkinPregenerator implem
     
     try
     {
-      _pregenerate(context, skin, config, stats); 
+      _pregenerate(context, skin, config, provider, stats); 
     }
     finally
     {
@@ -90,22 +83,18 @@ class AllVariantsSkinPregenerator implem
     FacesContext    context,
     Skin            skin,
     PregenConfig    config,
+    StyleProvider   provider,
     Stats           stats
     )
   {
-    StyleProvider provider = _getStyleProviderForSkin(skin);
-    StyleSheetDocument document = _getDocumentForSkin(context, skin, provider);
-    _pregenerateAllVariants(context, provider, document, config, stats);     
-  }
-  
-  private StyleProvider _getStyleProviderForSkin(Skin skin)
-  {
-    return new PregenStyleProvider(skin, _generatedSkinFilesPath);
+    StyleSheetDocument document = _getDocumentForSkin(context, skin, config, provider);
+    _pregenerateAllVariants(context, config, provider, document, stats);     
   }
   
   private StyleSheetDocument _getDocumentForSkin(
     FacesContext  context,
     Skin          skin,
+    PregenConfig  config,
     StyleProvider provider
     )
   {
@@ -113,7 +102,8 @@ class AllVariantsSkinPregenerator implem
     
     if (skin instanceof DocumentProviderSkin)
     {
-      StyleContext styleContext = PregenStyleContext.documentContext(context, provider, _generatedFilesPath);
+      StyleContext styleContext =
+        PregenStyleContext.documentContext(context, provider, config.getTargetDirectoryPath());
       document = ((DocumentProviderSkin) skin).getStyleSheetDocument(styleContext);
     }
     
@@ -128,9 +118,9 @@ class AllVariantsSkinPregenerator implem
   
   private void _pregenerateAllVariants(
     FacesContext       context,
+    PregenConfig       config,    
     StyleProvider      provider,
     StyleSheetDocument document,
-    PregenConfig       config,
     Stats              stats
     )
   {
@@ -138,18 +128,20 @@ class AllVariantsSkinPregenerator implem
     
     for (SkinVariant variant : variants)
     {
-      _pregenerateVariant(context, provider, variant, stats);
+      _pregenerateVariant(context, config, provider, variant, stats);
     }
   }
 
   private void _pregenerateVariant(
     FacesContext           context,
+    PregenConfig           config,
     StyleProvider          provider,
     SkinVariant            variant,
     Stats                  stats
     )
   {
     StyleContext styleContext = _createStyleContext(context,
+                                                    config,
                                                     provider,
                                                     variant);
 
@@ -162,6 +154,7 @@ class AllVariantsSkinPregenerator implem
 
   private StyleContext _createStyleContext(
     FacesContext           context,
+    PregenConfig           config,
     StyleProvider          provider,
     SkinVariant            variant
     )
@@ -170,7 +163,7 @@ class AllVariantsSkinPregenerator implem
     
     return new PregenStyleContext(context, 
                                   provider,
-                                  _generatedFilesPath,
+                                  config.getTargetDirectoryPath(),
                                   variant,
                                   _isDirty(variant));
   }
@@ -189,30 +182,6 @@ class AllVariantsSkinPregenerator implem
     return ((_variant != null) && (_variant.getPlatform() != variant.getPlatform()));
   }
 
-  // Returns the root directory for generated files.
-  private static String _getGeneratedFilesPath(FacesContext context)
-  {
-    String targetDirectory = System.getProperty(_TARGET_DIRECTORY_PROPERTY);
-    
-    if ((targetDirectory == null) || targetDirectory.isEmpty())
-    {
-      // Refactor to avoid dependency on core renderkit-specific API?
-      targetDirectory = CoreRenderingContext.getTemporaryDirectory(context, false);
-    }
-    
-    return targetDirectory;
-  }
-
-  // Returns the subdirectory for skin-specific generated files.
-  private static String _getGeneratedSkinFilesPath(String generatedFilesPath)
-  {
-    assert(generatedFilesPath != null);
-    assert(!generatedFilesPath.isEmpty());
-    assert(generatedFilesPath.charAt(generatedFilesPath.length() - 1) != '/');
-    
-    return generatedFilesPath + TrinidadRenderingConstants.STYLES_CACHE_DIRECTORY;
-  }
-
   // Little utility class for tracking statistics (eg.
   // # of generated style sheets, duration, etc...)
   private class Stats
@@ -295,21 +264,12 @@ class AllVariantsSkinPregenerator implem
     private long              _startTime = -1;    
   }
 
-  // Root directory for generated files
-  private final String _generatedFilesPath;
-  
-  // Subdirectory for skin-specific generated files.
-  private final String _generatedSkinFilesPath;
-  
   // The variant that we are currently processing
   private SkinVariant _variant;
   
   private static final StyleSheetDocument _EMPTY_DOCUMENT = 
     new StyleSheetDocument(new StyleSheetNode[0], null, StyleSheetDocument.UNKNOWN_TIMESTAMP);
 
-  private static final String _TARGET_DIRECTORY_PROPERTY =
-    "org.apache.myfaces.trinidad.SKIN_PREGENERATION_SERVICE_TARGET_DIRECTORY";
-
   private static final TrinidadLogger _LOG =
     TrinidadLogger.createTrinidadLogger(AllVariantsSkinPregenerator.class);
 }

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationService.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationService.java?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationService.java (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationService.java Wed Mar 14 18:18:26 2012
@@ -157,35 +157,27 @@ public class SkinPregenerationService ex
   // request parameters and returns the duration.
   private static long _pregenerateSkin(FacesContext context)
   {
-    PregenConfig config = _parsePregenConfig(context);
-    Skin skin = _parseSkin(context);
-    
     long startTime = System.currentTimeMillis();
 
-    if (skin != null)
-    {
-      SkinPregenerationUtils.pregenerate(context, skin, config);
-    }
-
-    return (System.currentTimeMillis() - startTime);
-  }
-
-  private static PregenConfig _parsePregenConfig(FacesContext context)
-  {
     try
     {
-      return PregenConfig.parse(context.getExternalContext());      
+      PregenConfig config = PregenConfig.parse(context);
+      Skin skin = _parseSkin(context);
+      SkinPregenerationUtils.pregenerate(context, skin, config);      
     }
-    catch (InvalidConfigException e)
+    catch (Exception e)
     {
-      _LOG.severe(e);
-      _sendError(context, e.getMessage());
+      _pregenFailed(context, e);
     }
-    
-    return PregenConfig.nullInstance();
-  }
 
+    return (System.currentTimeMillis() - startTime);
+  }
+  
+  // Returns a non-null Skin corresponding based on the "id"
+  // request parameter.  Throws InvalidConfigException if no
+  // skin is found.
   private static Skin _parseSkin(FacesContext context)
+    throws InvalidConfigException
   {
     String skinId = _getSkinId(context);
     if (skinId != null)
@@ -197,10 +189,10 @@ public class SkinPregenerationService ex
       }
     }
 
-    _handleInvalidSkin(context, skinId);
-    return null;
+    String message = _LOG.getMessage("SKIN_PREGEN_REQUESTED_SKIN_INVALID", skinId);
+    throw new InvalidConfigException(message);
   }
-
+  
   private static String _getSkinId(FacesContext context)
   {
     ExternalContext external = context.getExternalContext();
@@ -213,13 +205,10 @@ public class SkinPregenerationService ex
     return factory.getSkin(context, skinId);
   }
 
-  private static void _handleInvalidSkin(
-    FacesContext context,
-    String       skinId
-    )
+  private static void _pregenFailed(FacesContext context, Exception e)
   {
-    String message = _LOG.getMessage("SKIN_PREGEN_REQUESTED_SKIN_INVALID", skinId);
-    _LOG.severe(message);
+    String message = _LOG.getMessage("SKIN_PREGEN_FAILED", e.getMessage());
+    _LOG.severe(message, e);
     _sendError(context, message);
   }
 
@@ -241,6 +230,11 @@ public class SkinPregenerationService ex
 
   private static void _sendError(FacesContext context, String message)
   {
+    if (context.getResponseComplete())
+    {
+      return;
+    }
+
     ExternalContext external = context.getExternalContext();
     
     try

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationUtils.java?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationUtils.java (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerationUtils.java Wed Mar 14 18:18:26 2012
@@ -20,10 +20,18 @@ package org.apache.myfaces.trinidadinter
 
 import java.beans.Beans;
 
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
 import javax.faces.context.FacesContext;
 
 import org.apache.myfaces.trinidad.skin.Skin;
+import org.apache.myfaces.trinidad.util.FileUtils;
+import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.TrinidadRenderingConstants;
 import org.apache.myfaces.trinidadinternal.skin.pregen.config.PregenConfig;
+import org.apache.myfaces.trinidadinternal.skin.pregen.context.PregenStyleProvider;
+import org.apache.myfaces.trinidadinternal.style.StyleProvider;
 
 /**
  * Skin pregeneration utilities.
@@ -42,23 +50,42 @@ public final class SkinPregenerationUtil
     FacesContext    context,
     Skin            skin,
     PregenConfig    config
-    )
+    ) throws IOException
   {
     assert(skin != null);
     assert(config != null);
 
-    SkinPregenerator pregenerator = _getSkinPregenerator(context);
-    pregenerator.pregenerate(context, skin, config);
+    SkinPregenerator pregenerator = _getSkinPregenerator();
+    String stylesCacheDirectoryPath = _getStylesCacheDirectoryPath(config);
+    StyleProvider provider = new PregenStyleProvider(skin, stylesCacheDirectoryPath);
+
+    pregenerator.pregenerate(context, skin, config, provider);
   }
   
-  private static SkinPregenerator _getSkinPregenerator(FacesContext context)
+  private static SkinPregenerator _getSkinPregenerator()
   {
     if (Beans.isDesignTime())
     {
       return _NOOP_PREGENERATOR;
     }
 
-    return new AllVariantsSkinPregenerator(context);
+    return new AllVariantsSkinPregenerator();
+  }
+
+  // Returns the subdirectory for skin-specific generated files.
+  // Throws an IOException if the directory does not exist/cannot be
+  // created or is not writable.
+  private static String _getStylesCacheDirectoryPath(PregenConfig config)
+    throws IOException
+  {
+    String targetDirPath = config.getTargetDirectoryPath();
+    String subDirPath = TrinidadRenderingConstants.STYLES_CACHE_DIRECTORY;
+    String stylesCacheDirPath = targetDirPath + subDirPath;
+    
+    // Verify that the directory exists and is writable.
+    FileUtils.toWritableDirectory(stylesCacheDirPath);
+    
+    return stylesCacheDirPath;
   }
 
   private SkinPregenerationUtils()
@@ -72,7 +99,8 @@ public final class SkinPregenerationUtil
       public void pregenerate(
         FacesContext    context,
         Skin            skin, 
-        PregenConfig    config)
+        PregenConfig    config,
+        StyleProvider   provider)
       {
       }
     };

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerator.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerator.java?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerator.java (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/SkinPregenerator.java Wed Mar 14 18:18:26 2012
@@ -22,6 +22,7 @@ import javax.faces.context.FacesContext;
 
 import org.apache.myfaces.trinidad.skin.Skin;
 import org.apache.myfaces.trinidadinternal.skin.pregen.config.PregenConfig;
+import org.apache.myfaces.trinidadinternal.style.StyleProvider;
 
 /**
  * Interface for objects that perform skin pregeneration.
@@ -36,8 +37,9 @@ interface SkinPregenerator
    * @param config configuration info that influences pregeneration
    */
   public void pregenerate(
-    FacesContext context,
-    Skin         skin,
-    PregenConfig config
+    FacesContext  context,
+    Skin          skin,
+    PregenConfig  config,
+    StyleProvider provider
     );
 }

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/config/PregenConfig.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/config/PregenConfig.java?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/config/PregenConfig.java (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/skin/pregen/config/PregenConfig.java Wed Mar 14 18:18:26 2012
@@ -18,6 +18,9 @@
  */
 package org.apache.myfaces.trinidadinternal.skin.pregen.config;
 
+import java.io.File;
+import java.io.IOException;
+
 import java.util.AbstractCollection;
 import java.util.Arrays;
 import java.util.Collection;
@@ -29,13 +32,17 @@ import java.util.Map;
 
 import javax.faces.context.ExternalContext;
 
+import javax.faces.context.FacesContext;
+
 import org.apache.myfaces.trinidad.context.AccessibilityProfile;
 import org.apache.myfaces.trinidad.context.LocaleContext;
 import org.apache.myfaces.trinidad.logging.TrinidadLogger;
 import org.apache.myfaces.trinidad.util.EnumParseException;
 import org.apache.myfaces.trinidad.util.Enums;
+import org.apache.myfaces.trinidad.util.FileUtils;
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent;
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent.Application;
+import org.apache.myfaces.trinidadinternal.renderkit.core.CoreRenderingContext;
 import org.apache.myfaces.trinidadinternal.share.nls.NullLocaleContext;
 import org.apache.myfaces.trinidadinternal.util.nls.LocaleUtils;
 
@@ -97,16 +104,18 @@ abstract public class PregenConfig
    * 
    * Request parameters are documented in SkinPregenerationService class doc.
    * 
-   * @param external the ExternalContext to use for retreving request parameter
+   * @param context the FacesContext to use for retreving request parameter
    *   values.
    * @return
    */
-  public static PregenConfig parse(ExternalContext external)
+  public static PregenConfig parse(FacesContext context)
     throws InvalidConfigException
   {
+    ExternalContext external = context.getExternalContext();
+
     return _isAllVariantsRequest(external) ?
-      new AllPregenConfig(external) :
-      new CommonPregenConfig(external);
+      new AllPregenConfig(context) :
+      new CommonPregenConfig(context);
   }
 
   private static boolean _isAllVariantsRequest(ExternalContext external)
@@ -217,6 +226,16 @@ abstract public class PregenConfig
    */
   abstract public Collection<StyleClassType> getStyleClassTypes();
   
+  /**
+   * Returns the path of the target output directory.
+   * 
+   * This returns a non-null path to an existing, writable directory.
+   * 
+   * The returned path is guaranteed to not have a trailing file
+   * separator.
+   */
+  abstract public String getTargetDirectoryPath();
+  
   // Utility enum class used to identify whether PregenConfig.parse() should
   // return a PregenConfig instance that is configured for generating
   // "common" or "all" variants.
@@ -350,9 +369,11 @@ abstract public class PregenConfig
   // from request parameters.
   private static abstract class ParamPregenConfig extends PregenConfig
   {
-    public ParamPregenConfig(ExternalContext external)
+    public ParamPregenConfig(FacesContext context)
       throws InvalidConfigException
     {
+      ExternalContext external = context.getExternalContext();
+
       _containerTypes = _parseDisplayNameParam(external,
                                                "containerType",
                                                ContainerType.class,
@@ -366,7 +387,9 @@ abstract public class PregenConfig
       _styleClassTypes = _parseDisplayNameParam(external,
                                                 "styleClassType",
                                                 StyleClassType.class,
-                                                StyleClassType.COMPRESSED);      
+                                                StyleClassType.COMPRESSED);
+      
+      _targetDirectoryPath = _getTargetDirectoryPath(context);
     }
     
     @Override
@@ -387,18 +410,88 @@ abstract public class PregenConfig
       return _styleClassTypes;
     }
 
+    @Override
+    public String getTargetDirectoryPath()
+    {
+      return _targetDirectoryPath;
+    }
+
+    // Returns the target directory path.  Throws an InvalidConfigException
+    // if the target directory does not exist/cannot be created or is not
+    // writable.
+    private static String _getTargetDirectoryPath(FacesContext context)
+      throws InvalidConfigException
+    {
+      String targetDirectoryPath = _getNormalizedTargetDirectoryPath(context);
+      
+      try
+      {
+        FileUtils.toWritableDirectory(targetDirectoryPath);
+      }
+      catch (IOException e)
+      {
+        // The directory either does not exist/cannot be created or
+        // exists but is not writable.  Propagate this failure out.
+        throw new InvalidConfigException(e.getMessage());
+      }
+      
+      return targetDirectoryPath;
+    }
+
+    // Returns the target directory path in its normalized form.
+    private static String _getNormalizedTargetDirectoryPath(FacesContext context)
+    {
+      String targetDirectoryPath = System.getProperty(_TARGET_DIRECTORY_PROPERTY);
+    
+      if ((targetDirectoryPath == null) || targetDirectoryPath.isEmpty())
+      {
+        // Refactor to avoid dependency on core renderkit-specific API?
+        targetDirectoryPath = CoreRenderingContext.getTemporaryDirectory(context, false);
+      }
+    
+      return _normalizeDirectoryPath(targetDirectoryPath);
+    }
+    
+    // Normalizes the specified directory path by stripping off the trailing
+    // file separator, if present.
+    private static String _normalizeDirectoryPath(String path)
+      throws InvalidConfigException
+    {
+      if ((path == null) || path.isEmpty())
+      {
+        String message = _LOG.getMessage("SKIN_PREGEN_NO_TARGET_DIRECTORY", 
+                                         _TARGET_DIRECTORY_PROPERTY);
+        throw new InvalidConfigException(message);
+      }
+
+      return _hasTrailingSeparator(path) ? 
+        path.substring(0, path.length() - 1) :
+        path;
+    }
+    
+    private static boolean _hasTrailingSeparator(String path)
+    {
+      char lastChar = path.charAt(path.length() - 1);
+      
+      // Explicitly check '/' just in case someone happens to be using
+      // this as a separator in their sys property value (and we're on
+      // a platform that uses a different separator, eg. Windows).
+      return ((lastChar == File.separatorChar) || (lastChar == '/'));
+    }
+
     private final Collection<ContainerType>  _containerTypes;
     private final Collection<RequestType>    _requestTypes;
     private final Collection<StyleClassType> _styleClassTypes;
+    private final String                     _targetDirectoryPath;
   }
   
   // PregenConfig implementation that is used for pregeneration of
   // all possible variant values.
   private static class AllPregenConfig extends ParamPregenConfig
   {
-    public AllPregenConfig(ExternalContext external)
+    public AllPregenConfig(FacesContext context)
     {
-      super(external);
+      super(context);
     }
 
     @Override
@@ -436,9 +529,9 @@ abstract public class PregenConfig
   // only the most common variant values.
   private static class CommonPregenConfig extends ParamPregenConfig
   {
-    public CommonPregenConfig(ExternalContext external)
+    public CommonPregenConfig(FacesContext context)
     {
-      super(external);
+      super(context);
     }
 
     @Override
@@ -532,10 +625,19 @@ abstract public class PregenConfig
     {
       return Collections.emptySet();
     }
+    
+    @Override
+    public String getTargetDirectoryPath()
+    {
+      return null;
+    }
   }
 
   private static final PregenConfig _NULL_CONFIG = new NullPregenConfig();
-  
+
+  private static final String _TARGET_DIRECTORY_PROPERTY =
+    "org.apache.myfaces.trinidad.SKIN_PREGENERATION_SERVICE_TARGET_DIRECTORY";
+
   private static final TrinidadLogger _LOG =
     TrinidadLogger.createTrinidadLogger(PregenConfig.class);  
 }

Modified: myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts?rev=1300671&r1=1300670&r2=1300671&view=diff
==============================================================================
--- myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts (original)
+++ myfaces/trinidad/branches/andys-skin-pregen/trinidad-impl/src/main/xrts/org/apache/myfaces/trinidadinternal/resource/LoggerBundle.xrts Wed Mar 14 18:18:26 2012
@@ -1172,11 +1172,15 @@ The skin {0} specified on the requestMap
 
 <resource key="SKIN_PREGEN_DISABLED">Skin pregeneration failed: the skin pregeneration service is disabled.  To enable skin pregeneration, set the {0} system property to one of: {1}</resource>
 
-<resource key="SKIN_PREGEN_REQUESTED_SKIN_INVALID">Skin pregeneration failed: {0} is not valid skin id.</resource>
+<resource key="SKIN_PREGEN_FAILED">Skin pregeneration failed: {0}</resource>
+
+<resource key="SKIN_PREGEN_REQUESTED_SKIN_INVALID">{0} is not valid skin id.</resource>
+
+<resource key="SKIN_PREGEN_NO_TARGET_DIRECTORY">No target directory found.  Please specify a target directory via the {0} system property.</resource>
 
 <resource key="SKIN_PREGEN_RESPONSE">Skin pregeneration completed in {0}ms.</resource>
 
-<resource key="SKIN_PREGEN_NO_DOCUMENT">Pregeneration of skin {0} failed.  No StyleSheetDocument found for skin.</resource>
+<resource key="SKIN_PREGEN_NO_DOCUMENT">No StyleSheetDocument found for skin {0}.</resource>
 
 <resource key="SKIN_PREGEN_STARTING">Starting pregeneration of skin {0}</resource>