Rework Environment implementation so that it is part of the ExecManager service. This makes it less coupled to bad practices and also merges all the native execution code into a central place.

Detected by JDepend


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@271062 13f79535-47bb-0310-9956-ffa450edef68

proposal/myrmidon/src/java/org/apache/antlib/nativelib/LoadEnvironment.java

@@ -7,15 +7,16 @@
  */
 package org.apache.antlib.nativelib;
 
-import java.io.IOException;
 import java.util.Iterator;
 import java.util.Properties;
+import org.apache.aut.nativelib.ExecException;
+import org.apache.aut.nativelib.ExecManager;
 import org.apache.avalon.excalibur.i18n.ResourceManager;
 import org.apache.avalon.excalibur.i18n.Resources;
 import org.apache.myrmidon.api.AbstractTask;
 import org.apache.myrmidon.api.TaskException;
-import org.apache.aut.nativelib.impl.Environment;
-import org.apache.aut.nativelib.ExecException;
+import org.apache.myrmidon.framework.factorys.ExecManagerFactory;
+import org.apache.myrmidon.services.ServiceException;
 
 /**
  * This task is responsible for loading that OS-specific environment
@@ -88,15 +89,18 @@ public class LoadEnvironment
     {
         try
         {
-            return Environment.getNativeEnvironment();
+            final ExecManagerFactory factory = new ExecManagerFactory();
+            final ExecManager manager = (ExecManager)factory.createService();
+
+            return manager.getNativeEnvironment();
         }
-        catch( final ExecException ee )
+        catch( final ServiceException se )
         {
-            throw new TaskException( ee.getMessage(), ee );
+            throw new TaskException( se.getMessage(), se );
         }
-        catch( final IOException ioe )
+        catch( final ExecException ee )
         {
-            throw new TaskException( ioe.getMessage(), ioe );
+            throw new TaskException( ee.getMessage(), ee );
         }
     }
 }

proposal/myrmidon/src/java/org/apache/aut/nativelib/ExecManager.java

@@ -10,6 +10,7 @@ package org.apache.aut.nativelib;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.Properties;
 
 /**
  * Interface via which clients can request that a native
@@ -30,6 +31,13 @@ import java.io.OutputStream;
  */
 public interface ExecManager
 {
+    /**
+     * Retrieve a properties object that contains a list of
+     * all the native environment variables.
+     */
+    Properties getNativeEnvironment()
+        throws ExecException;
+
     /**
      * Execute a process and wait for it to finish before
      * returning.

proposal/myrmidon/src/java/org/apache/aut/nativelib/impl/DefaultExecManager.java

@@ -12,16 +12,17 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Locale;
+import java.util.Properties;
 import org.apache.aut.nativelib.ExecException;
 import org.apache.aut.nativelib.ExecManager;
 import org.apache.aut.nativelib.ExecMetaData;
 import org.apache.aut.nativelib.ExecOutputHandler;
 import org.apache.aut.nativelib.Os;
+import org.apache.aut.nativelib.impl.launchers.CommandLauncher;
 import org.apache.aut.nativelib.impl.launchers.DefaultCommandLauncher;
 import org.apache.aut.nativelib.impl.launchers.MacCommandLauncher;
 import org.apache.aut.nativelib.impl.launchers.ScriptCommandLauncher;
 import org.apache.aut.nativelib.impl.launchers.WinNTCommandLauncher;
-import org.apache.aut.nativelib.impl.launchers.CommandLauncher;
 import org.apache.avalon.excalibur.io.FileUtil;
 import org.apache.avalon.excalibur.io.IOUtil;
 
@@ -34,6 +35,7 @@ import org.apache.avalon.excalibur.io.IOUtil;
  * @version $Revision$ $Date$
  * @see ExecManager
  * @see ExecMetaData
+ * @see Environment
  */
 public class DefaultExecManager
     implements ExecManager
@@ -46,11 +48,35 @@ public class DefaultExecManager
     private final CommandLauncher m_launcher;
     private final CommandLauncher m_shellLauncher;
 
+    /**
+     * Utility class that is used to load and parse the native
+     * environment variables.
+     */
+    private final Environment m_environment;
+
     public DefaultExecManager( final File homeDir )
         throws ExecException
     {
         m_launcher = new DefaultCommandLauncher();
         m_shellLauncher = createShellLauncher( homeDir );
+        m_environment = new Environment( this );
+    }
+
+    /**
+     * Retrieve a properties object that contains a list of
+     * all the native environment variables.
+     */
+    public Properties getNativeEnvironment()
+        throws ExecException
+    {
+        try
+        {
+            return m_environment.getNativeEnvironment();
+        }
+        catch( final IOException ioe )
+        {
+            throw new ExecException( ioe.getMessage(), ioe );
+        }
     }
 
     /**
@@ -79,13 +105,14 @@ public class DefaultExecManager
      * Execute a process and wait for it to finish before
      * returning.
      */
-    public int execute( final ExecMetaData metaData,
+    public int execute( final ExecMetaData command,
                         final InputStream input,
                         final OutputStream output,
                         final OutputStream error,
                         final long timeout )
         throws IOException, ExecException
     {
+        final ExecMetaData metaData = prepareExecMetaData( command );
         final CommandLauncher launcher = getLauncher( metaData );
         final Process process = launcher.exec( metaData );
         final ProcessMonitor monitor =
@@ -133,6 +160,30 @@ public class DefaultExecManager
         }
     }
 
+    /**
+     * Utility method to preapre a metaData object.
+     * This involves adding the native environment to the metaData if the
+     * metaData is specified as being additive.
+     */
+    private ExecMetaData prepareExecMetaData( final ExecMetaData metaData )
+        throws ExecException
+    {
+        if( !metaData.isEnvironmentAdditive() )
+        {
+            return metaData;
+        }
+        else
+        {
+            final Properties newEnvironment = new Properties();
+            newEnvironment.putAll( getNativeEnvironment() );
+            newEnvironment.putAll( metaData.getEnvironment() );
+            return new ExecMetaData( metaData.getCommand(),
+                                     newEnvironment,
+                                     metaData.getWorkingDirectory(),
+                                     false );
+        }
+    }
+
     private CommandLauncher getLauncher( final ExecMetaData metaData )
     {
         CommandLauncher launcher = m_launcher;

proposal/myrmidon/src/java/org/apache/aut/nativelib/impl/Environment.java

@@ -9,16 +9,16 @@ package org.apache.aut.nativelib.impl;
 
 import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
-import java.util.Iterator;
 import java.util.Locale;
 import java.util.Properties;
-import org.apache.avalon.excalibur.util.StringUtil;
-import org.apache.myrmidon.api.TaskException;
-import org.apache.tools.ant.taskdefs.exec.Execute;
 import org.apache.aut.nativelib.ExecException;
+import org.apache.aut.nativelib.ExecManager;
+import org.apache.aut.nativelib.ExecMetaData;
 import org.apache.aut.nativelib.Os;
+import org.apache.avalon.excalibur.util.StringUtil;
 
 /**
  * This is the class that can be used to retrieve the environment
@@ -28,7 +28,7 @@ import org.apache.aut.nativelib.Os;
  * @author <a href="mailto:thomas.haas@softwired-inc.com">Thomas Haas</a>
  * @version $Revision$ $Date$
  */
-public final class Environment
+final class Environment
 {
     private final static String[] COMMAND_COM = new String[]{"command.com", "/c", "set"};
     private final static String[] CMD_EXE = new String[]{"cmd", "/c", "set"};
@@ -37,39 +37,27 @@ public final class Environment
     private final static String[] ENV_CMD = new String[]{"/usr/bin/env"};
     private final static String[] ENV_RAW = new String[]{"env"};
 
-    private static Properties c_procEnvironment;
+    /**
+     * This is a cached version of the native environment variables.
+     */
+    private Properties m_procEnvironment;
 
     /**
-     * Private constructor to block instantiation.
+     * This is the class that is used to invoke the native process
+     * to retrieve then environment variables.
      */
-    private Environment()
-    {
-    }
+    private final ExecManager m_execManager;
 
-    public static void addNativeEnvironment( final Properties environment )
-        throws ExecException, IOException
+    public Environment( final ExecManager execManager )
     {
-        final Properties nativeEnvironment = getEnvironmentVariables();
-        final Iterator nativeKeys = nativeEnvironment.keySet().iterator();
-        while( nativeKeys.hasNext() )
-        {
-            final String key = (String)nativeKeys.next();
-            if( environment.contains( key ) )
-            {
-                //Skip environment variables that are overidden
-                continue;
-            }
-
-            final String value = nativeEnvironment.getProperty( key );
-            environment.setProperty( key, value );
-        }
+        m_execManager = execManager;
     }
 
     /**
      * Retrieve a Properties object that contains the list of all
      * native EnvironmentData Variables for the current process.
      */
-    public static Properties getNativeEnvironment()
+    public Properties getNativeEnvironment()
         throws IOException, ExecException
     {
         final Properties properties = new Properties();
@@ -81,21 +69,21 @@ public final class Environment
      * Get the Property object with all environment variables and
      * attempt to load it if it has not already been loaded.
      */
-    private static synchronized Properties getEnvironmentVariables()
+    private synchronized Properties getEnvironmentVariables()
         throws IOException, ExecException
     {
-        if( null == c_procEnvironment )
+        if( null == m_procEnvironment )
         {
-            c_procEnvironment = retrieveEnvironmentVariables();
+            m_procEnvironment = retrieveEnvironmentVariables();
         }
 
-        return c_procEnvironment;
+        return m_procEnvironment;
     }
 
     /**
      * Retrieve a last of environment variables from the native OS.
      */
-    private static synchronized Properties retrieveEnvironmentVariables()
+    private synchronized Properties retrieveEnvironmentVariables()
         throws IOException, ExecException
     {
         final String data = getEnvironmentText();
@@ -137,8 +125,8 @@ public final class Environment
      * '=' character then generate an exception. After parsed data place
      * the key-value pair into the specified Properties object.
      */
-    private static void addProperty( final Properties properties,
-                                     final String data )
+    private void addProperty( final Properties properties,
+                              final String data )
         throws ExecException
     {
         final int index = data.indexOf( '=' );
@@ -161,30 +149,18 @@ public final class Environment
      * Retrieve the text of data that is the result of
      * running the environment command.
      */
-    private static String getEnvironmentText()
+    private String getEnvironmentText()
         throws IOException, ExecException
     {
-        final ByteArrayOutputStream output = new ByteArrayOutputStream();
-        final Execute exe = new Execute();
-        exe.setOutput( output );
-        exe.setError( output );
-
-        exe.setCommandline( getEnvCommand() );
+        final String[] command = getEnvCommand();
+        final File workingDirectory = new File( "." );
+        final ExecMetaData metaData = new ExecMetaData( command, null, workingDirectory, false );
 
-        // Make sure we do not recurse forever
-        exe.setNewenvironment( true );
-
-        try
-        {
-            final int retval = exe.execute();
-            if( retval != 0 )
-            {
-                // Just try to use what we got
-            }
-        }
-        catch( final TaskException te )
+        final ByteArrayOutputStream output = new ByteArrayOutputStream();
+        final int retval = m_execManager.execute( metaData, null, output, output, 0 );
+        if( retval != 0 )
         {
-            throw new ExecException( te.getMessage(), te );
+            // Just try to use what we got
         }
 
         return output.toString();

proposal/myrmidon/src/java/org/apache/aut/nativelib/impl/launchers/ExecUtil.java

@@ -12,7 +12,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.Properties;
-import org.apache.aut.nativelib.impl.Environment;
 import org.apache.aut.nativelib.ExecException;
 import org.apache.aut.nativelib.ExecMetaData;
 
@@ -121,23 +120,13 @@ class ExecUtil
         throws ExecException, IOException
     {
         final Properties environment = metaData.getEnvironment();
-        if( 0 == environment.size() )
+        if( null == environment || 0 == environment.size() )
         {
             return null;
         }
         else
         {
-            if( metaData.isEnvironmentAdditive() )
-            {
-                final Properties newEnvironment = new Properties();
-                newEnvironment.putAll( Environment.getNativeEnvironment() );
-                newEnvironment.putAll( environment );
-                return toNativeEnvironment( newEnvironment );
-            }
-            else
-            {
-                return toNativeEnvironment( environment );
-            }
+            return toNativeEnvironment( environment );
         }
     }
 }