Re: svn commit: r646349 - in /ofbiz/trunk/framework: appserver/ appserver/src/org/ofbiz/appservers/ appserver/templates/wasce2/ base/src/base/org/ofbiz/base/util/string/ webapp/src/org/ofbiz/webapp/control/ webtools/webapp/webtools/WEB-INF/

[Jacopo Cappellato-4]

Hi all,

this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you agree?
Jacques, before I start my review please let me know if you want to do a first pass on it.

Thanks,

Jacopo

On Apr 9, 2008, at 3:51 PM, [hidden email] wrote:

> Author: jleroux
> Date: Wed Apr  9 06:51:39 2008
> New Revision: 646349
>
> URL: http://svn.apache.org/viewvc?rev=646349&view=rev
> Log:
> New version which automatically deploys in application server see http://docs.ofbiz.org/x/Ah for more informations
> One base, one webapp, and the webtools web.xml files are slightly modified, nothing harmful anyway
>
> Modified:
>    ofbiz/trunk/framework/appserver/ofbiz-component.xml
>    ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
>    ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
>    ofbiz/trunk/framework/appserver/templates/wasce2/README
>    ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
>    ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
>    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
>    ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
>    ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
>
> Modified: ofbiz/trunk/framework/appserver/ofbiz-component.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/ofbiz-component.xml?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/ofbiz-component.xml (original)
> +++ ofbiz/trunk/framework/appserver/ofbiz-component.xml Wed Apr  9 06:51:39 2008
> @@ -23,4 +23,5 @@
>         xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/ofbiz-component.xsd">
>     <resource-loader name="main" type="component"/>
>     <classpath type="jar" location="build/lib/*"/>
> +    <classpath type="dir" location="config"/>
> </ofbiz-component>
>
> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java (original)
> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java Wed Apr  9 06:51:39 2008
> @@ -19,23 +19,50 @@
>
> package org.ofbiz.appservers;
>
> -import java.util.Map;
> +import java.io.File;
> +import java.io.FileInputStream;
> +import java.io.FileNotFoundException;
> +import java.io.FileWriter;
> +import java.io.IOException;
> +import java.io.InputStreamReader;
> +import java.io.Reader;
> +import java.io.StringWriter;
> +import java.io.Writer;
> import java.util.List;
> -import java.util.ArrayList;
> -import java.util.HashMap;
> -import java.util.Iterator;
> -import java.io.*;
> +import java.util.Map;
> +
> +import javax.xml.parsers.DocumentBuilder;
> +import javax.xml.parsers.DocumentBuilderFactory;
> +import javax.xml.parsers.ParserConfigurationException;
> +import javax.xml.transform.OutputKeys;
> +import javax.xml.transform.Result;
> +import javax.xml.transform.Source;
> +import javax.xml.transform.Transformer;
> +import javax.xml.transform.TransformerConfigurationException;
> +import javax.xml.transform.TransformerException;
> +import javax.xml.transform.TransformerFactory;
> +import javax.xml.transform.TransformerFactoryConfigurationError;
> +import javax.xml.transform.dom.DOMSource;
> +import javax.xml.transform.stream.StreamResult;
>
> import javolution.util.FastList;
> import javolution.util.FastMap;
>
> +import org.ofbiz.base.component.ComponentConfig;
> +import org.ofbiz.base.component.ComponentConfig.WebappInfo;
> import org.ofbiz.base.container.Container;
> import org.ofbiz.base.container.ContainerException;
> +import org.ofbiz.base.start.Classpath;
> import org.ofbiz.base.util.Debug;
> import org.ofbiz.base.util.UtilURL;
> +import org.ofbiz.base.util.UtilProperties;
> +import org.ofbiz.base.util.UtilValidate;
> import org.ofbiz.base.util.template.FreeMarkerWorker;
> -import org.ofbiz.base.start.Classpath;
> -import org.ofbiz.base.component.ComponentConfig;
> +import org.w3c.dom.Attr;
> +import org.w3c.dom.Document;
> +import org.w3c.dom.NamedNodeMap;
> +import org.w3c.dom.Node;
> +import org.xml.sax.SAXException;
>
> /**
>  * GenerateContainer - Generates Configuration Files For Application Servers
> @@ -47,14 +74,15 @@
>
>     public static final String module = GenerateContainer.class.getName();
>     public static final String source = "/framework/appserver/templates/";
> -    public static final String target = "/setup/";
> -    private boolean isGeronimo = false;
> -
> +    public static String target = "/setup/";
>
>     protected String configFile = null;
>     protected String ofbizHome = null;
>     protected String args[] = null;
>
> +    private boolean isGeronimo = false;
> +    private String geronimoHome = null;
> +
>     /**
>      * @see org.ofbiz.base.container.Container#init(java.lang.String[], java.lang.String)
>      */
> @@ -63,6 +91,10 @@
>         this.configFile = configFile;
>         this.args = args;
>         this.isGeronimo = args[0].toLowerCase().contains("geronimo") || args[0].toLowerCase().contains("wasce");
> +        if (this.isGeronimo) {
> +            this.target="/META-INF/";
> +            this.geronimoHome = UtilProperties.getPropertyValue("appserver", "geronimoHome", null);
> +        }
>     }
>
>     /**
> @@ -86,30 +118,216 @@
>     private void generateFiles() throws ContainerException {
>         File files[] = getTemplates();
>         Map<String, Object> dataMap = buildDataMap();
> +
> +        String user = UtilProperties.getPropertyValue("appserver", "user", "system");
> +        String password = UtilProperties.getPropertyValue("appserver", "password", "manager");
> +        boolean offline = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "offline", "true");
> +        boolean pauseInGeronimoScript = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "pauseInGeronimoScript", "true");
> +        int instancesNumber = (int) UtilProperties.getPropertyNumber("appserver", "instancesNumber");
> +        String instanceNumber = "";
> +
>         if (isGeronimo) {
> -            String serverType = args[0];
> -            String geronimoHome = null;
> -            if (args.length > 2) {
> -                geronimoHome = args[2];
> -            }
> -            GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
> -            List classpathJars = geronimoDeployment.generate(serverType, geronimoHome);
> -            if (classpathJars == null) {
> -                throw new ContainerException("Error in Geronimo deployment, please check the log");
> +            if (geronimoHome == null) {
> +                geronimoHome = System.getenv("GERONIMO_HOME");
> +                if (geronimoHome == null) {
> +                    Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
> +                }
> +                File geronimoHomeDir = new File (geronimoHome);
> +                if (! (geronimoHomeDir.isDirectory())) {
> +                    Debug.logFatal(geronimoHome + " does not exist or is not a directoy. Please set the location of Geronimo into a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
> +                }
>             }
> -            dataMap.put("classpathJars", classpathJars);
> -        }
>
> -        //Debug.log("Using Data : " + dataMap, module);
> -        String applicationPrefix = "";
> -        if (args.length > 3 && args[3].length() > 0) {
> -            applicationPrefix = args[3] + "-";
> -        }
> -        dataMap.put("applicationPrefix", applicationPrefix);
> -        dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
> -        for (File file: files) {
> -            if (isGeronimo && !(file.isDirectory() || file.isHidden() || file.getName().equalsIgnoreCase("geronimo-web.xml"))) {
> -                parseTemplate(file, dataMap);
> +            for(int inst = 0; inst <= instancesNumber; inst++) {
> +                instanceNumber = (inst == 0 ? "" : inst).toString();
> +                GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
> +                List classpathJars = geronimoDeployment.generate(args[0], geronimoHome, instanceNumber);
> +                if (classpathJars == null) {
> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
> +                }
> +                dataMap.put("classpathJars", classpathJars);
> +                dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
> +                dataMap.put("instanceNumber", instanceNumber);
> +                //                if (UtilValidate.isNotEmpty(instanceNumber)) {
> +                //                    List webApps = (List) dataMap.get("webApps");
> +                //                    for (Object webAppObject: webApps) {
> +                //                        WebappInfo webAppInfo = (ComponentConfig.WebappInfo) webAppObject;
> +                //                        String webAppLocation = webAppInfo.getLocation();
> +                //                        String webXmlLocation = webAppLocation + "/WEB-INF/web.xml";
> +                //                        if (isFileExistsAndCanWrite(webXmlLocation)) {
> +                //                            DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
> +                //                            DocumentBuilder docBuilder = null;
> +                //                            try {
> +                //                                docBuilder = docFactory.newDocumentBuilder();
> +                //                            } catch (ParserConfigurationException e) {
> +                //                                throw new ContainerException(e);
> +                //                            }
> +                //                            Document doc = null;
> +                //                            try {
> +                //                                doc = docBuilder.parse(webXmlLocation);
> +                //                            } catch (SAXException e) {
> +                //                                throw new ContainerException(e);
> +                //                            } catch (IOException e) {
> +                //                                throw new ContainerException(e);
> +                //                            }
> +                //                            Node webApp = doc.getFirstChild();
> +                //                            Node contextParam = doc.createElement("context-param");
> +                //                            NamedNodeMap contextParamAttributes = contextParam.getAttributes();
> +                //
> +                //                            Attr paramName = doc.createAttribute("param-name");
> +                //                            paramName.setValue("instanceNumber");
> +                //                            contextParamAttributes.setNamedItem(paramName);
> +                //
> +                //                            Attr paramValue = doc.createAttribute("param-value");
> +                //                            paramValue.setValue(instanceNumber);
> +                //                            contextParamAttributes.setNamedItem(paramValue);
> +                //        //                    Node nodeToAppend = doc.importNode(contextParam, true); this should not be needed
> +                //        //                    webApp.appendChild(nodeToAppend);
> +                //
> +                //        //                    webApp.appendChild(contextParam); this is the line needed but commented for now
> +                //
> +                //                            Transformer transformer;
> +                //                            try {
> +                //                                transformer = TransformerFactory.newInstance().newTransformer();
> +                //                            } catch (TransformerConfigurationException e) {
> +                //                                throw new ContainerException(e);
> +                //                            } catch (TransformerFactoryConfigurationError e) {
> +                //                                throw new ContainerException(e);
> +                //                            }
> +                //                            transformer.setOutputProperty(OutputKeys.INDENT, "yes");
> +                //
> +                //                            StreamResult result = new StreamResult(new StringWriter());
> +                //                            DOMSource source = new DOMSource(doc);
> +                //                            try {
> +                //                                transformer.transform(source, result);
> +                //                            } catch (TransformerException e) {
> +                //                                throw new ContainerException(e);
> +                //                            }
> +                //                            String xmlString = result.getWriter().toString();
> +                //                            System.out.println(xmlString); //TODO write to file using writeToXmlFile
> +                //                            break; // Only the 1st web.xml file need to be modified
> +                //                        } else {
> +                //                            Debug.logInfo("Unable to change the deployment descriptor : " + webXmlLocation + ". Maybe it does not exist, or is in read only mode ?", module);
> +                //                        }
> +                //                    }
> +                //                }
> +
> +                //Debug.log("Using Data : " + dataMap, module);
> +                for (int i = 0; i < files.length; i++) {
> +                    if (!(files[i].isDirectory() || files[i].isHidden() || files[i].getName().equalsIgnoreCase("geronimo-web.xml"))) {
> +                        parseTemplate(files[i], dataMap);
> +                    }
> +                }
> +
> +                String ofbizName = "ofbiz" + instanceNumber;
> +                String separator = File.separator;
> +                String geronimoBin = geronimoHome + separator + "bin";
> +                File workingDir = new File(geronimoBin);
> +                ProcessBuilder pb = null;
> +                String command = null;
> +
> +                if ("\\".equals(separator)) { //Windows
> +                    if (offline) {
> +                        command = "deploy --user " + user +  " --password " +  password + " --offline undeploy " + ofbizName;
> +                    } else {
> +                        command = "deploy --user " +  user +  " --password " +  password + " undeploy " + ofbizName;
> +                    }
> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
> +                } else {                        // Linux
> +                    if (offline) {
> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline undeploy " + ofbizName;
> +                    } else {
> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " undeploy " + ofbizName;
> +                    }
> +                    pb = new ProcessBuilder("sh", "-c", command);
> +                }
> +
> +                if (pauseInGeronimoScript) {
> +                    Map<String, String> env = pb.environment();
> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
> +                }
> +                pb.directory(workingDir);
> +                
> +                try {
> +                    System.out.println("Currently undeploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
> +                    pb.redirectErrorStream(true);
> +                    Process p = pb.start();
> +                    java.io.InputStream is = p.getInputStream();
> +                    byte[] buf = new byte[2024];
> +                    int readLen = 0;
> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
> +                        if ("\\".equals(separator)) {   //Windows
> +                            System.out.print(new String(buf,0,readLen));
> +                        } else {
> +                            System.out.println(new String(buf,0,readLen));                                
> +                        }
> +                    }
> +                    is.close();
> +                    p.waitFor();
> +                    //                    System.out.println(p.waitFor());
> +                    //                    System.out.println("exit value" + p.exitValue());
> +                    Debug.logInfo(ofbizName + " undeployment ended" , module);
> +                } catch (IOException e) {
> +                    throw new ContainerException(e);
> +                } catch (InterruptedException e) {
> +                    throw new ContainerException(e);
> +                }
> +
> +                if ("\\".equals(separator)) { //Windows
> +                    if (offline) {
> +                        command = "deploy --user " + user +  " --password " +  password + " --offline deploy --inPlace " + ofbizHome;
> +                    } else {
> +                        command = "deploy --user " + user +  " --password " +  password + " deploy --inPlace " + ofbizHome;
> +                    }
> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
> +                } else {                      // Linux
> +                    if (offline) {
> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline deploy --inPlace " + ofbizHome;
> +                    } else {
> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " deploy --inPlace " + ofbizHome;
> +                    }
> +                    pb = new ProcessBuilder("sh", "-c", command);
> +                }
> +
> +                if (pauseInGeronimoScript) {
> +                    Map<String, String> env = pb.environment();
> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
> +                }
> +                pb.directory(workingDir);
> +                
> +                try {
> +                    System.out.println("Currently deploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
> +                    pb.redirectErrorStream(true);
> +                    Process p = pb.start();
> +                    java.io.InputStream is = p.getInputStream();
> +                    byte[] buf = new byte[2024];
> +                    int readLen = 0;
> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
> +                        if ("\\".equals(separator)) {   //Windows
> +                            System.out.print(new String(buf,0,readLen));
> +                        } else {
> +                            System.out.println(new String(buf,0,readLen));                                
> +                        }
> +                    }
> +                    is.close();
> +                    p.waitFor();
> +                    //                    System.out.println(p.waitFor());
> +                    //                    System.out.println("exit value" + p.exitValue());
> +                    Debug.logInfo(ofbizName + " deployment ended" , module);
> +                } catch (IOException e) {
> +                    throw new ContainerException(e);
> +                } catch (InterruptedException e) {
> +                    throw new ContainerException(e);
> +                }
> +            }
> +        } else {
> +            //Debug.log("Using Data : " + dataMap, module);
> +            for (int i = 0; i < files.length; i++) {
> +                if (!files[i].isDirectory() && !files[i].isHidden()) {
> +                    parseTemplate(files[i], dataMap);
> +                }
>             }
>         }
>     }
> @@ -158,6 +376,7 @@
>                 }
>             }
>         }
> +
>         List[] lists = { jar, dir };
>         return lists;
>     }
> @@ -213,5 +432,28 @@
>         } catch (IOException e) {
>             throw new ContainerException(e);
>         }
> +    }
> +
> +    // This method writes a DOM document to a file
> +    public static void writeToXmlFile(Document doc, String filename) {
> +        try {
> +            // Prepare the DOM document for writing
> +            Source source = new DOMSource(doc);
> +
> +            // Prepare the output file
> +            File file = new File(filename);
> +            Result result = new StreamResult(file);
> +
> +            // Write the DOM document to the file
> +            Transformer xformer = TransformerFactory.newInstance().newTransformer();
> +            xformer.transform(source, result);
> +        } catch (TransformerConfigurationException e) {
> +        } catch (TransformerException e) {
> +        }
> +    }
> +
> +    public boolean isFileExistsAndCanWrite(String fileName) {
> +        File f = new File(fileName);
> +        return f.exists() && f.canWrite();
>     }
> }
>
> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java (original)
> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java Wed Apr  9 06:51:39 2008
> @@ -52,18 +52,8 @@
>     public static final String source = "/framework/appserver/templates/";
>
>     protected  String geronimoRepository = null;
> -    protected String geronimoHome = null;
>
> -    @SuppressWarnings("unchecked")
> -    public List<String> generate(String geronimoVersion, String geronimoHome) {
> -        // Check environment for Geronimo Home
> -        if (geronimoHome == null) {
> -            geronimoHome = System.getenv("GERONIMO_HOME");
> -            if (geronimoHome == null) {
> -                Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into GERONIMO_HOME.", module);
> -                return null;
> -            }
> -        }
> +    public List<String> generate(String geronimoVersion, String geronimoHome, String instanceNumber) {
>         geronimoRepository = geronimoHome + "/repository";
>         Debug.logInfo("The WASCE or Geronimo Repository is " + geronimoRepository, module);
>         Classpath classPath = new Classpath(System.getProperty("java.class.path"));
>
> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/README
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/README?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/templates/wasce2/README (original)
> +++ ofbiz/trunk/framework/appserver/templates/wasce2/README Wed Apr  9 06:51:39 2008
> @@ -17,9 +17,10 @@
>     under the License.
> -->
>
> -Websphere Application Server Community Edition 2 Setup - 2008-03-31 - Jacques Le Roux
> +Websphere Application Server Community Edition (WASCE) 2
> +or Geronimo 2.0.3 Setup - 2008-04-08 - Jacques Le Roux
>
> -To use WASCE 2 with OFBiz 4.0 follow the following steps (all steps assume Orion is already installed and working)
> +To use OFBiz with WASCE 2 or Geronimo 2.0.3 follow the following steps (all steps assume WASCE 2 or Geronimo 2.0.3  is already installed and working)
>
> =======================================================================================================================================================
>     If you use WASCE, copy the 2 lines below in the setJavaOpts section (Windows) or somewhere in the setEnv script you will find in GERONIMO_HOME/bin
> @@ -33,13 +34,44 @@
>     <#assign classpath = classpath + dir + pathSeparatorChar/>
> </#list>
> <#assign classpath = classpath?substring(0, classpath?length - 1)/>
> +
> +Windows
> +-------
> set JAVA_OPTS=-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en
> set CLASSPATH=${classpath}
>
> +Linux
> +-----
> +Add this 2 lines after the
> +    elif [ "$1" = "run" ]; then
> +line
> +  JAVA_OPTS="-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en"
> +  CLASSPATH="${classpath}"
> +Don't forget to escape the inside quotes in ${ofbizHome}.
> +
> =======================================================================================================================================================
> -    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the Geronimo script.
> +    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the geronimo script
> =======================================================================================================================================================
> -
> +Windows
> +-------
> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS% %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed" -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -jar %_JARFILE% %_LONG_OPT% %CMD_LINE_ARGS%
>     to
> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS% %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed" -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -cp .;%_JARFILE%;%CLASSPATH% %_LONG_OPT% %CMD_LINE_ARGS% %MAINCLASS%
> +
> +Linux
> +-----
> +In the <<elif [ "$1" = "run" ]>> block
> +Replace the line
> +    -jar "$GERONIMO_HOME"/bin/server.jar $LONG_OPT "$@"
> +by
> +    -classpath ".":"$GERONIMO_HOME"/bin/server.jar:"$CLASSPATH" org.apache.geronimo.cli.daemon.DaemonCLI $LONG_OPT "$@"
> +
> +=======================================================================================================================================================
> +    Windows only
> +    ------------
> +    If you want to use OFBiz multi-instances in Geronimo or WASCE using you might put "exit" as last line in the deploy script.
> +    Else, of course you may also type exit by hand....
> +
> +    Alternatively you may not put exit at the end of the deploy script,
> +    choose to not have pause in script and type "exit" yourself in the command windows (actually I found later that it's an easier way)
> +=======================================================================================================================================================
>
> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/application.xml?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/templates/wasce2/application.xml (original)
> +++ ofbiz/trunk/framework/appserver/templates/wasce2/application.xml Wed Apr  9 06:51:39 2008
> @@ -29,36 +29,31 @@
>   id="Application_ID"
>   version="5">
>
> -  <description>The Apache OFBiz Project</description>
> -  <display-name>OFBiz</display-name>
> +  <description>The Apache OFBiz Project  this is instance ${instanceNumber}</description>
> +  <display-name>OFBiz instance ${instanceNumber}</display-name>
>
>   <#list webApps as webapp>
>     <#assign location = webapp.getLocation()?if_exists/>
> -    <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
> -      || location.contains("applications")
> -      || location.contains("specialpurpose")
> -      || location.contains("hot-deploy")>
> -      <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
> +    <#assign origin = webapp.getLocation()?if_exists/>
> +      <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>         <#assign location = location.substring(location.lastIndexOf("framework"))/>
>       </#if>
> -      <#if location.contains("applications")>
> +    <#if location.contains("/applications/")>
>         <#assign location = location.substring(location.lastIndexOf("applications"))/>
>       </#if>
> -      <#if location.contains("specialpurpose")>
> +    <#if location.contains("/specialpurpose/")>
>         <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>       </#if>
> -      <#if location.contains("hot-deploy") && !location.contains("images")>
> +      <#if location.contains("/hot-deploy/")>
>         <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>       </#if>
> -      <#if !(location.contains("neogia") && location.contains("shipment"))
> -        && !(location.contains("hot-deploy") && location.contains("images"))>
> -  <module id="${applicationPrefix}${webapp.getName()}">
> +    <#if location != origin>
> +  <module id="${webapp.getName()}${instanceNumber}">
>     <web>
>       <web-uri>${location}/</web-uri>
> -      <context-root>${webapp.getContextRoot()}</context-root>
> +      <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>     </web>
>   </module>
> -      </#if>
>     </#if>
>   </#list>
> </application>
>
> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml (original)
> +++ ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml Wed Apr  9 06:51:39 2008
> @@ -28,7 +28,7 @@
>     <dep:environment>
>         <dep:moduleId>
>             <dep:groupId>org.ofbiz</dep:groupId>
> -            <dep:artifactId>${applicationPrefix}ofbiz</dep:artifactId>
> +            <dep:artifactId>ofbiz${instanceNumber}</dep:artifactId>
>             <dep:version>1.0</dep:version>
>             <dep:type>ear</dep:type>
>         </dep:moduleId>
> @@ -51,31 +51,26 @@
>
>     <#list webApps as webapp>
>         <#assign location = webapp.getLocation()?if_exists/>
> -        <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
> -            || location.contains("applications")
> -            || location.contains("specialpurpose")
> -            || location.contains("hot-deploy")>
> -            <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
> +        <#assign origin = webapp.getLocation()?if_exists/>
> +        <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>                 <#assign location = location.substring(location.lastIndexOf("framework"))/>
> -            </#if>
> -            <#if location.contains("applications")>
> -                <#assign location = location.substring(location.lastIndexOf("applications"))/>
> -            </#if>
> -            <#if location.contains("specialpurpose")>
> -                <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
> -            </#if>
> -            <#if location.contains("hot-deploy") && !location.contains("images")>
> -                <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
> -            </#if>
> -            <#if !(location.contains("neogia") && location.contains("shipment"))
> -                && !(location.contains("hot-deploy") && location.contains("images"))>
> +        </#if>
> +        <#if location.contains("/applications/")>
> +            <#assign location = location.substring(location.lastIndexOf("applications"))/>
> +        </#if>
> +        <#if location.contains("/specialpurpose/")>
> +            <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
> +        </#if>
> +        <#if location.contains("/hot-deploy/")>
> +            <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
> +        </#if>
> +        <#if location != origin>
>     <module>
>         <web>${location}/</web>
>         <web-app xmlns="http://geronimo.apache.org/xml/ns/j2ee/web-1.2">
> -            <context-root>${webapp.getContextRoot()}</context-root>
> +            <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>         </web-app>
>     </module>
> -            </#if>
>         </#if>
>     </#list>
>
>
> Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java (original)
> +++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java Wed Apr  9 06:51:39 2008
> @@ -30,6 +30,7 @@
> import org.ofbiz.base.util.BshUtil;
> import org.ofbiz.base.util.Debug;
> import org.ofbiz.base.util.UtilDateTime;
> +import org.ofbiz.base.util.UtilValidate;
> import org.ofbiz.base.util.collections.FlexibleMapAccessor;
> import org.ofbiz.base.util.UtilFormatOut;
> import org.ofbiz.base.util.UtilMisc;
> @@ -395,6 +396,11 @@
>                     targetBuffer.append(UtilFormatOut.formatCurrency(new Double(envVal.toString()), currencyCode, locale));
>                 } else {
>                     targetBuffer.append(envVal.toString());
> +                }
> +            } else if (envName.equals("ofbiz.home")) { // This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env
> +                String ofbizHome = System.getProperty("ofbiz.home");
> +                if (UtilValidate.isNotEmpty(ofbizHome)) {
> +                    targetBuffer.append(ofbizHome);
>                 }
>             } else {
>                 Debug.logWarning("Could not find value in environment for the name [" + envName + "], inserting nothing.", module);
>
> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java (original)
> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java Wed Apr  9 06:51:39 2008
> @@ -346,6 +346,15 @@
>             if (Debug.infoOn()) Debug.logInfo("Adding web.xml context-param to application attribute with name [" + initParamName + "] and value [" + initParamValue + "]", module);
>             config.getServletContext().setAttribute(initParamName, initParamValue);
>         }
> +        String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
> +        if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
> +            String ofbizHome = System.getProperty("ofbiz.home");
> +            if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
> +                ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env
> +                System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
> +                System.setProperty("ofbiz.home", ofbizHome);
> +            }
> +        }
>     }
>
>     protected String getServerId() {
>
> Modified: ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml?rev=646349&r1=646348&r2=646349&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml (original)
> +++ ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml Wed Apr  9 06:51:39 2008
> @@ -149,4 +149,11 @@
>     </user-data-constraint>
>   </security-constraint>
> -->
> +
> +  <context-param>
> +    <param-name>GeronimoMultiOfbizInstances</param-name>
> +    <param-value>true</param-value>
> +    <description>This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env</description>
> +  </context-param>
> +
> </web-app>
>
>

[Jacopo Cappellato-4]

Another commit (somewhat related) that will need to be reviewed is

Author: jleroux
Date: Tue Apr 29 15:35:32 2008
New Revision: 652177

URL: http://svn.apache.org/viewvc?rev=652177&view=rev
Log:
This add the possiblity to run RMI services under WASCE or Geronimo.
There is only one small drawback : so far if you restart the OFBiz EAR from the console you loose the RMI Dispatcher. However stopping and starting (even after a restart) is OK. I guess it's the same for the commande redeploy, I will test tomorrow.

Modified:
   ofbiz/trunk/framework/appserver/config/appserver.properties
   ofbiz/trunk/framework/appserver/templates/wasce2/README
   ofbiz/trunk/framework/base/src/base/org/ofbiz/base/container/ContainerLoader.java
   ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java

Regards,

Jacopo

On Jun 25, 2012, at 5:40 PM, Jacopo Cappellato wrote:

> Hi all,
>
> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you agree?
> Jacques, before I start my review please let me know if you want to do a first pass on it.
>
> Thanks,
>
> Jacopo
>
> On Apr 9, 2008, at 3:51 PM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Wed Apr  9 06:51:39 2008
>> New Revision: 646349
>>
>> URL: http://svn.apache.org/viewvc?rev=646349&view=rev
>> Log:
>> New version which automatically deploys in application server see http://docs.ofbiz.org/x/Ah for more informations
>> One base, one webapp, and the webtools web.xml files are slightly modified, nothing harmful anyway
>>
>> Modified:
>>   ofbiz/trunk/framework/appserver/ofbiz-component.xml
>>   ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
>>   ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
>>   ofbiz/trunk/framework/appserver/templates/wasce2/README
>>   ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
>>   ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
>>   ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>   ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
>>   ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
>>
>> Modified: ofbiz/trunk/framework/appserver/ofbiz-component.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/ofbiz-component.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/ofbiz-component.xml (original)
>> +++ ofbiz/trunk/framework/appserver/ofbiz-component.xml Wed Apr  9 06:51:39 2008
>> @@ -23,4 +23,5 @@
>>        xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/ofbiz-component.xsd">
>>    <resource-loader name="main" type="component"/>
>>    <classpath type="jar" location="build/lib/*"/>
>> +    <classpath type="dir" location="config"/>
>> </ofbiz-component>
>>
>> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java (original)
>> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java Wed Apr  9 06:51:39 2008
>> @@ -19,23 +19,50 @@
>>
>> package org.ofbiz.appservers;
>>
>> -import java.util.Map;
>> +import java.io.File;
>> +import java.io.FileInputStream;
>> +import java.io.FileNotFoundException;
>> +import java.io.FileWriter;
>> +import java.io.IOException;
>> +import java.io.InputStreamReader;
>> +import java.io.Reader;
>> +import java.io.StringWriter;
>> +import java.io.Writer;
>> import java.util.List;
>> -import java.util.ArrayList;
>> -import java.util.HashMap;
>> -import java.util.Iterator;
>> -import java.io.*;
>> +import java.util.Map;
>> +
>> +import javax.xml.parsers.DocumentBuilder;
>> +import javax.xml.parsers.DocumentBuilderFactory;
>> +import javax.xml.parsers.ParserConfigurationException;
>> +import javax.xml.transform.OutputKeys;
>> +import javax.xml.transform.Result;
>> +import javax.xml.transform.Source;
>> +import javax.xml.transform.Transformer;
>> +import javax.xml.transform.TransformerConfigurationException;
>> +import javax.xml.transform.TransformerException;
>> +import javax.xml.transform.TransformerFactory;
>> +import javax.xml.transform.TransformerFactoryConfigurationError;
>> +import javax.xml.transform.dom.DOMSource;
>> +import javax.xml.transform.stream.StreamResult;
>>
>> import javolution.util.FastList;
>> import javolution.util.FastMap;
>>
>> +import org.ofbiz.base.component.ComponentConfig;
>> +import org.ofbiz.base.component.ComponentConfig.WebappInfo;
>> import org.ofbiz.base.container.Container;
>> import org.ofbiz.base.container.ContainerException;
>> +import org.ofbiz.base.start.Classpath;
>> import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilURL;
>> +import org.ofbiz.base.util.UtilProperties;
>> +import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.template.FreeMarkerWorker;
>> -import org.ofbiz.base.start.Classpath;
>> -import org.ofbiz.base.component.ComponentConfig;
>> +import org.w3c.dom.Attr;
>> +import org.w3c.dom.Document;
>> +import org.w3c.dom.NamedNodeMap;
>> +import org.w3c.dom.Node;
>> +import org.xml.sax.SAXException;
>>
>> /**
>> * GenerateContainer - Generates Configuration Files For Application Servers
>> @@ -47,14 +74,15 @@
>>
>>    public static final String module = GenerateContainer.class.getName();
>>    public static final String source = "/framework/appserver/templates/";
>> -    public static final String target = "/setup/";
>> -    private boolean isGeronimo = false;
>> -
>> +    public static String target = "/setup/";
>>
>>    protected String configFile = null;
>>    protected String ofbizHome = null;
>>    protected String args[] = null;
>>
>> +    private boolean isGeronimo = false;
>> +    private String geronimoHome = null;
>> +
>>    /**
>>     * @see org.ofbiz.base.container.Container#init(java.lang.String[], java.lang.String)
>>     */
>> @@ -63,6 +91,10 @@
>>        this.configFile = configFile;
>>        this.args = args;
>>        this.isGeronimo = args[0].toLowerCase().contains("geronimo") || args[0].toLowerCase().contains("wasce");
>> +        if (this.isGeronimo) {
>> +            this.target="/META-INF/";
>> +            this.geronimoHome = UtilProperties.getPropertyValue("appserver", "geronimoHome", null);
>> +        }
>>    }
>>
>>    /**
>> @@ -86,30 +118,216 @@
>>    private void generateFiles() throws ContainerException {
>>        File files[] = getTemplates();
>>        Map<String, Object> dataMap = buildDataMap();
>> +
>> +        String user = UtilProperties.getPropertyValue("appserver", "user", "system");
>> +        String password = UtilProperties.getPropertyValue("appserver", "password", "manager");
>> +        boolean offline = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "offline", "true");
>> +        boolean pauseInGeronimoScript = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "pauseInGeronimoScript", "true");
>> +        int instancesNumber = (int) UtilProperties.getPropertyNumber("appserver", "instancesNumber");
>> +        String instanceNumber = "";
>> +
>>        if (isGeronimo) {
>> -            String serverType = args[0];
>> -            String geronimoHome = null;
>> -            if (args.length > 2) {
>> -                geronimoHome = args[2];
>> -            }
>> -            GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
>> -            List classpathJars = geronimoDeployment.generate(serverType, geronimoHome);
>> -            if (classpathJars == null) {
>> -                throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +            if (geronimoHome == null) {
>> +                geronimoHome = System.getenv("GERONIMO_HOME");
>> +                if (geronimoHome == null) {
>> +                    Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>> +                File geronimoHomeDir = new File (geronimoHome);
>> +                if (! (geronimoHomeDir.isDirectory())) {
>> +                    Debug.logFatal(geronimoHome + " does not exist or is not a directoy. Please set the location of Geronimo into a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>>            }
>> -            dataMap.put("classpathJars", classpathJars);
>> -        }
>>
>> -        //Debug.log("Using Data : " + dataMap, module);
>> -        String applicationPrefix = "";
>> -        if (args.length > 3 && args[3].length() > 0) {
>> -            applicationPrefix = args[3] + "-";
>> -        }
>> -        dataMap.put("applicationPrefix", applicationPrefix);
>> -        dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
>> -        for (File file: files) {
>> -            if (isGeronimo && !(file.isDirectory() || file.isHidden() || file.getName().equalsIgnoreCase("geronimo-web.xml"))) {
>> -                parseTemplate(file, dataMap);
>> +            for(int inst = 0; inst <= instancesNumber; inst++) {
>> +                instanceNumber = (inst == 0 ? "" : inst).toString();
>> +                GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
>> +                List classpathJars = geronimoDeployment.generate(args[0], geronimoHome, instanceNumber);
>> +                if (classpathJars == null) {
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>> +                dataMap.put("classpathJars", classpathJars);
>> +                dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
>> +                dataMap.put("instanceNumber", instanceNumber);
>> +                //                if (UtilValidate.isNotEmpty(instanceNumber)) {
>> +                //                    List webApps = (List) dataMap.get("webApps");
>> +                //                    for (Object webAppObject: webApps) {
>> +                //                        WebappInfo webAppInfo = (ComponentConfig.WebappInfo) webAppObject;
>> +                //                        String webAppLocation = webAppInfo.getLocation();
>> +                //                        String webXmlLocation = webAppLocation + "/WEB-INF/web.xml";
>> +                //                        if (isFileExistsAndCanWrite(webXmlLocation)) {
>> +                //                            DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
>> +                //                            DocumentBuilder docBuilder = null;
>> +                //                            try {
>> +                //                                docBuilder = docFactory.newDocumentBuilder();
>> +                //                            } catch (ParserConfigurationException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            Document doc = null;
>> +                //                            try {
>> +                //                                doc = docBuilder.parse(webXmlLocation);
>> +                //                            } catch (SAXException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            } catch (IOException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            Node webApp = doc.getFirstChild();
>> +                //                            Node contextParam = doc.createElement("context-param");
>> +                //                            NamedNodeMap contextParamAttributes = contextParam.getAttributes();
>> +                //
>> +                //                            Attr paramName = doc.createAttribute("param-name");
>> +                //                            paramName.setValue("instanceNumber");
>> +                //                            contextParamAttributes.setNamedItem(paramName);
>> +                //
>> +                //                            Attr paramValue = doc.createAttribute("param-value");
>> +                //                            paramValue.setValue(instanceNumber);
>> +                //                            contextParamAttributes.setNamedItem(paramValue);
>> +                //        //                    Node nodeToAppend = doc.importNode(contextParam, true); this should not be needed
>> +                //        //                    webApp.appendChild(nodeToAppend);
>> +                //
>> +                //        //                    webApp.appendChild(contextParam); this is the line needed but commented for now
>> +                //
>> +                //                            Transformer transformer;
>> +                //                            try {
>> +                //                                transformer = TransformerFactory.newInstance().newTransformer();
>> +                //                            } catch (TransformerConfigurationException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            } catch (TransformerFactoryConfigurationError e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            transformer.setOutputProperty(OutputKeys.INDENT, "yes");
>> +                //
>> +                //                            StreamResult result = new StreamResult(new StringWriter());
>> +                //                            DOMSource source = new DOMSource(doc);
>> +                //                            try {
>> +                //                                transformer.transform(source, result);
>> +                //                            } catch (TransformerException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            String xmlString = result.getWriter().toString();
>> +                //                            System.out.println(xmlString); //TODO write to file using writeToXmlFile
>> +                //                            break; // Only the 1st web.xml file need to be modified
>> +                //                        } else {
>> +                //                            Debug.logInfo("Unable to change the deployment descriptor : " + webXmlLocation + ". Maybe it does not exist, or is in read only mode ?", module);
>> +                //                        }
>> +                //                    }
>> +                //                }
>> +
>> +                //Debug.log("Using Data : " + dataMap, module);
>> +                for (int i = 0; i < files.length; i++) {
>> +                    if (!(files[i].isDirectory() || files[i].isHidden() || files[i].getName().equalsIgnoreCase("geronimo-web.xml"))) {
>> +                        parseTemplate(files[i], dataMap);
>> +                    }
>> +                }
>> +
>> +                String ofbizName = "ofbiz" + instanceNumber;
>> +                String separator = File.separator;
>> +                String geronimoBin = geronimoHome + separator + "bin";
>> +                File workingDir = new File(geronimoBin);
>> +                ProcessBuilder pb = null;
>> +                String command = null;
>> +
>> +                if ("\\".equals(separator)) { //Windows
>> +                    if (offline) {
>> +                        command = "deploy --user " + user +  " --password " +  password + " --offline undeploy " + ofbizName;
>> +                    } else {
>> +                        command = "deploy --user " +  user +  " --password " +  password + " undeploy " + ofbizName;
>> +                    }
>> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
>> +                } else {                        // Linux
>> +                    if (offline) {
>> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline undeploy " + ofbizName;
>> +                    } else {
>> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " undeploy " + ofbizName;
>> +                    }
>> +                    pb = new ProcessBuilder("sh", "-c", command);
>> +                }
>> +
>> +                if (pauseInGeronimoScript) {
>> +                    Map<String, String> env = pb.environment();
>> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
>> +                }
>> +                pb.directory(workingDir);
>> +                
>> +                try {
>> +                    System.out.println("Currently undeploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
>> +                    pb.redirectErrorStream(true);
>> +                    Process p = pb.start();
>> +                    java.io.InputStream is = p.getInputStream();
>> +                    byte[] buf = new byte[2024];
>> +                    int readLen = 0;
>> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
>> +                        if ("\\".equals(separator)) {   //Windows
>> +                            System.out.print(new String(buf,0,readLen));
>> +                        } else {
>> +                            System.out.println(new String(buf,0,readLen));                                
>> +                        }
>> +                    }
>> +                    is.close();
>> +                    p.waitFor();
>> +                    //                    System.out.println(p.waitFor());
>> +                    //                    System.out.println("exit value" + p.exitValue());
>> +                    Debug.logInfo(ofbizName + " undeployment ended" , module);
>> +                } catch (IOException e) {
>> +                    throw new ContainerException(e);
>> +                } catch (InterruptedException e) {
>> +                    throw new ContainerException(e);
>> +                }
>> +
>> +                if ("\\".equals(separator)) { //Windows
>> +                    if (offline) {
>> +                        command = "deploy --user " + user +  " --password " +  password + " --offline deploy --inPlace " + ofbizHome;
>> +                    } else {
>> +                        command = "deploy --user " + user +  " --password " +  password + " deploy --inPlace " + ofbizHome;
>> +                    }
>> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
>> +                } else {                      // Linux
>> +                    if (offline) {
>> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline deploy --inPlace " + ofbizHome;
>> +                    } else {
>> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " deploy --inPlace " + ofbizHome;
>> +                    }
>> +                    pb = new ProcessBuilder("sh", "-c", command);
>> +                }
>> +
>> +                if (pauseInGeronimoScript) {
>> +                    Map<String, String> env = pb.environment();
>> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
>> +                }
>> +                pb.directory(workingDir);
>> +                
>> +                try {
>> +                    System.out.println("Currently deploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
>> +                    pb.redirectErrorStream(true);
>> +                    Process p = pb.start();
>> +                    java.io.InputStream is = p.getInputStream();
>> +                    byte[] buf = new byte[2024];
>> +                    int readLen = 0;
>> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
>> +                        if ("\\".equals(separator)) {   //Windows
>> +                            System.out.print(new String(buf,0,readLen));
>> +                        } else {
>> +                            System.out.println(new String(buf,0,readLen));                                
>> +                        }
>> +                    }
>> +                    is.close();
>> +                    p.waitFor();
>> +                    //                    System.out.println(p.waitFor());
>> +                    //                    System.out.println("exit value" + p.exitValue());
>> +                    Debug.logInfo(ofbizName + " deployment ended" , module);
>> +                } catch (IOException e) {
>> +                    throw new ContainerException(e);
>> +                } catch (InterruptedException e) {
>> +                    throw new ContainerException(e);
>> +                }
>> +            }
>> +        } else {
>> +            //Debug.log("Using Data : " + dataMap, module);
>> +            for (int i = 0; i < files.length; i++) {
>> +                if (!files[i].isDirectory() && !files[i].isHidden()) {
>> +                    parseTemplate(files[i], dataMap);
>> +                }
>>            }
>>        }
>>    }
>> @@ -158,6 +376,7 @@
>>                }
>>            }
>>        }
>> +
>>        List[] lists = { jar, dir };
>>        return lists;
>>    }
>> @@ -213,5 +432,28 @@
>>        } catch (IOException e) {
>>            throw new ContainerException(e);
>>        }
>> +    }
>> +
>> +    // This method writes a DOM document to a file
>> +    public static void writeToXmlFile(Document doc, String filename) {
>> +        try {
>> +            // Prepare the DOM document for writing
>> +            Source source = new DOMSource(doc);
>> +
>> +            // Prepare the output file
>> +            File file = new File(filename);
>> +            Result result = new StreamResult(file);
>> +
>> +            // Write the DOM document to the file
>> +            Transformer xformer = TransformerFactory.newInstance().newTransformer();
>> +            xformer.transform(source, result);
>> +        } catch (TransformerConfigurationException e) {
>> +        } catch (TransformerException e) {
>> +        }
>> +    }
>> +
>> +    public boolean isFileExistsAndCanWrite(String fileName) {
>> +        File f = new File(fileName);
>> +        return f.exists() && f.canWrite();
>>    }
>> }
>>
>> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java (original)
>> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java Wed Apr  9 06:51:39 2008
>> @@ -52,18 +52,8 @@
>>    public static final String source = "/framework/appserver/templates/";
>>
>>    protected  String geronimoRepository = null;
>> -    protected String geronimoHome = null;
>>
>> -    @SuppressWarnings("unchecked")
>> -    public List<String> generate(String geronimoVersion, String geronimoHome) {
>> -        // Check environment for Geronimo Home
>> -        if (geronimoHome == null) {
>> -            geronimoHome = System.getenv("GERONIMO_HOME");
>> -            if (geronimoHome == null) {
>> -                Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into GERONIMO_HOME.", module);
>> -                return null;
>> -            }
>> -        }
>> +    public List<String> generate(String geronimoVersion, String geronimoHome, String instanceNumber) {
>>        geronimoRepository = geronimoHome + "/repository";
>>        Debug.logInfo("The WASCE or Geronimo Repository is " + geronimoRepository, module);
>>        Classpath classPath = new Classpath(System.getProperty("java.class.path"));
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/README
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/README?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/README (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/README Wed Apr  9 06:51:39 2008
>> @@ -17,9 +17,10 @@
>>    under the License.
>> -->
>>
>> -Websphere Application Server Community Edition 2 Setup - 2008-03-31 - Jacques Le Roux
>> +Websphere Application Server Community Edition (WASCE) 2
>> +or Geronimo 2.0.3 Setup - 2008-04-08 - Jacques Le Roux
>>
>> -To use WASCE 2 with OFBiz 4.0 follow the following steps (all steps assume Orion is already installed and working)
>> +To use OFBiz with WASCE 2 or Geronimo 2.0.3 follow the following steps (all steps assume WASCE 2 or Geronimo 2.0.3  is already installed and working)
>>
>> =======================================================================================================================================================
>>    If you use WASCE, copy the 2 lines below in the setJavaOpts section (Windows) or somewhere in the setEnv script you will find in GERONIMO_HOME/bin
>> @@ -33,13 +34,44 @@
>>    <#assign classpath = classpath + dir + pathSeparatorChar/>
>> </#list>
>> <#assign classpath = classpath?substring(0, classpath?length - 1)/>
>> +
>> +Windows
>> +-------
>> set JAVA_OPTS=-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en
>> set CLASSPATH=${classpath}
>>
>> +Linux
>> +-----
>> +Add this 2 lines after the
>> +    elif [ "$1" = "run" ]; then
>> +line
>> +  JAVA_OPTS="-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en"
>> +  CLASSPATH="${classpath}"
>> +Don't forget to escape the inside quotes in ${ofbizHome}.
>> +
>> =======================================================================================================================================================
>> -    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the Geronimo script.
>> +    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the geronimo script
>> =======================================================================================================================================================
>> -
>> +Windows
>> +-------
>> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS% %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed" -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -jar %_JARFILE% %_LONG_OPT% %CMD_LINE_ARGS%
>>    to
>> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS% %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed" -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -cp .;%_JARFILE%;%CLASSPATH% %_LONG_OPT% %CMD_LINE_ARGS% %MAINCLASS%
>> +
>> +Linux
>> +-----
>> +In the <<elif [ "$1" = "run" ]>> block
>> +Replace the line
>> +    -jar "$GERONIMO_HOME"/bin/server.jar $LONG_OPT "$@"
>> +by
>> +    -classpath ".":"$GERONIMO_HOME"/bin/server.jar:"$CLASSPATH" org.apache.geronimo.cli.daemon.DaemonCLI $LONG_OPT "$@"
>> +
>> +=======================================================================================================================================================
>> +    Windows only
>> +    ------------
>> +    If you want to use OFBiz multi-instances in Geronimo or WASCE using you might put "exit" as last line in the deploy script.
>> +    Else, of course you may also type exit by hand....
>> +
>> +    Alternatively you may not put exit at the end of the deploy script,
>> +    choose to not have pause in script and type "exit" yourself in the command windows (actually I found later that it's an easier way)
>> +=======================================================================================================================================================
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/application.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/application.xml (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/application.xml Wed Apr  9 06:51:39 2008
>> @@ -29,36 +29,31 @@
>>  id="Application_ID"
>>  version="5">
>>
>> -  <description>The Apache OFBiz Project</description>
>> -  <display-name>OFBiz</display-name>
>> +  <description>The Apache OFBiz Project  this is instance ${instanceNumber}</description>
>> +  <display-name>OFBiz instance ${instanceNumber}</display-name>
>>
>>  <#list webApps as webapp>
>>    <#assign location = webapp.getLocation()?if_exists/>
>> -    <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
>> -      || location.contains("applications")
>> -      || location.contains("specialpurpose")
>> -      || location.contains("hot-deploy")>
>> -      <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
>> +    <#assign origin = webapp.getLocation()?if_exists/>
>> +      <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>>        <#assign location = location.substring(location.lastIndexOf("framework"))/>
>>      </#if>
>> -      <#if location.contains("applications")>
>> +    <#if location.contains("/applications/")>
>>        <#assign location = location.substring(location.lastIndexOf("applications"))/>
>>      </#if>
>> -      <#if location.contains("specialpurpose")>
>> +    <#if location.contains("/specialpurpose/")>
>>        <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>>      </#if>
>> -      <#if location.contains("hot-deploy") && !location.contains("images")>
>> +      <#if location.contains("/hot-deploy/")>
>>        <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>>      </#if>
>> -      <#if !(location.contains("neogia") && location.contains("shipment"))
>> -        && !(location.contains("hot-deploy") && location.contains("images"))>
>> -  <module id="${applicationPrefix}${webapp.getName()}">
>> +    <#if location != origin>
>> +  <module id="${webapp.getName()}${instanceNumber}">
>>    <web>
>>      <web-uri>${location}/</web-uri>
>> -      <context-root>${webapp.getContextRoot()}</context-root>
>> +      <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>>    </web>
>>  </module>
>> -      </#if>
>>    </#if>
>>  </#list>
>> </application>
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml Wed Apr  9 06:51:39 2008
>> @@ -28,7 +28,7 @@
>>    <dep:environment>
>>        <dep:moduleId>
>>            <dep:groupId>org.ofbiz</dep:groupId>
>> -            <dep:artifactId>${applicationPrefix}ofbiz</dep:artifactId>
>> +            <dep:artifactId>ofbiz${instanceNumber}</dep:artifactId>
>>            <dep:version>1.0</dep:version>
>>            <dep:type>ear</dep:type>
>>        </dep:moduleId>
>> @@ -51,31 +51,26 @@
>>
>>    <#list webApps as webapp>
>>        <#assign location = webapp.getLocation()?if_exists/>
>> -        <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
>> -            || location.contains("applications")
>> -            || location.contains("specialpurpose")
>> -            || location.contains("hot-deploy")>
>> -            <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
>> +        <#assign origin = webapp.getLocation()?if_exists/>
>> +        <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>>                <#assign location = location.substring(location.lastIndexOf("framework"))/>
>> -            </#if>
>> -            <#if location.contains("applications")>
>> -                <#assign location = location.substring(location.lastIndexOf("applications"))/>
>> -            </#if>
>> -            <#if location.contains("specialpurpose")>
>> -                <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>> -            </#if>
>> -            <#if location.contains("hot-deploy") && !location.contains("images")>
>> -                <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>> -            </#if>
>> -            <#if !(location.contains("neogia") && location.contains("shipment"))
>> -                && !(location.contains("hot-deploy") && location.contains("images"))>
>> +        </#if>
>> +        <#if location.contains("/applications/")>
>> +            <#assign location = location.substring(location.lastIndexOf("applications"))/>
>> +        </#if>
>> +        <#if location.contains("/specialpurpose/")>
>> +            <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>> +        </#if>
>> +        <#if location.contains("/hot-deploy/")>
>> +            <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>> +        </#if>
>> +        <#if location != origin>
>>    <module>
>>        <web>${location}/</web>
>>        <web-app xmlns="http://geronimo.apache.org/xml/ns/j2ee/web-1.2">
>> -            <context-root>${webapp.getContextRoot()}</context-root>
>> +            <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>>        </web-app>
>>    </module>
>> -            </#if>
>>        </#if>
>>    </#list>
>>
>>
>> Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java (original)
>> +++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java Wed Apr  9 06:51:39 2008
>> @@ -30,6 +30,7 @@
>> import org.ofbiz.base.util.BshUtil;
>> import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilDateTime;
>> +import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.collections.FlexibleMapAccessor;
>> import org.ofbiz.base.util.UtilFormatOut;
>> import org.ofbiz.base.util.UtilMisc;
>> @@ -395,6 +396,11 @@
>>                    targetBuffer.append(UtilFormatOut.formatCurrency(new Double(envVal.toString()), currencyCode, locale));
>>                } else {
>>                    targetBuffer.append(envVal.toString());
>> +                }
>> +            } else if (envName.equals("ofbiz.home")) { // This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>> +                String ofbizHome = System.getProperty("ofbiz.home");
>> +                if (UtilValidate.isNotEmpty(ofbizHome)) {
>> +                    targetBuffer.append(ofbizHome);
>>                }
>>            } else {
>>                Debug.logWarning("Could not find value in environment for the name [" + envName + "], inserting nothing.", module);
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java Wed Apr  9 06:51:39 2008
>> @@ -346,6 +346,15 @@
>>            if (Debug.infoOn()) Debug.logInfo("Adding web.xml context-param to application attribute with name [" + initParamName + "] and value [" + initParamValue + "]", module);
>>            config.getServletContext().setAttribute(initParamName, initParamValue);
>>        }
>> +        String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
>> +        if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
>> +            String ofbizHome = System.getProperty("ofbiz.home");
>> +            if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
>> +                ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>> +                System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
>> +                System.setProperty("ofbiz.home", ofbizHome);
>> +            }
>> +        }
>>    }
>>
>>    protected String getServerId() {
>>
>> Modified: ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml (original)
>> +++ ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml Wed Apr  9 06:51:39 2008
>> @@ -149,4 +149,11 @@
>>    </user-data-constraint>
>>  </security-constraint>
>> -->
>> +
>> +  <context-param>
>> +    <param-name>GeronimoMultiOfbizInstances</param-name>
>> +    <param-value>true</param-value>
>> +    <description>This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env</description>
>> +  </context-param>
>> +
>> </web-app>
>>
>>
>

[Jacques Le Roux]

From: "Jacopo Cappellato" <[hidden email]>
> Hi all,
>
> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you
> agree?
> Jacques, before I start my review please let me know if you want to do a first pass on it.

I can certainly have a fresh look, depends on timing though...
What did you saw at 1st glance which makes you want to improve, is it related to GeronimoMultiOfbizInstances?

Jacques

> Thanks,
>
> Jacopo
>
> On Apr 9, 2008, at 3:51 PM, [hidden email] wrote:
>
>> Author: jleroux
>> Date: Wed Apr  9 06:51:39 2008
>> New Revision: 646349
>>
>> URL: http://svn.apache.org/viewvc?rev=646349&view=rev
>> Log:
>> New version which automatically deploys in application server see http://docs.ofbiz.org/x/Ah for more informations
>> One base, one webapp, and the webtools web.xml files are slightly modified, nothing harmful anyway
>>
>> Modified:
>>    ofbiz/trunk/framework/appserver/ofbiz-component.xml
>>    ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
>>    ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
>>    ofbiz/trunk/framework/appserver/templates/wasce2/README
>>    ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
>>    ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
>>    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>    ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
>>    ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
>>
>> Modified: ofbiz/trunk/framework/appserver/ofbiz-component.xml
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/ofbiz-component.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/ofbiz-component.xml (original)
>> +++ ofbiz/trunk/framework/appserver/ofbiz-component.xml Wed Apr  9 06:51:39 2008
>> @@ -23,4 +23,5 @@
>>         xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/ofbiz-component.xsd">
>>     <resource-loader name="main" type="component"/>
>>     <classpath type="jar" location="build/lib/*"/>
>> +    <classpath type="dir" location="config"/>
>> </ofbiz-component>
>>
>> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java (original)
>> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java Wed Apr  9 06:51:39 2008
>> @@ -19,23 +19,50 @@
>>
>> package org.ofbiz.appservers;
>>
>> -import java.util.Map;
>> +import java.io.File;
>> +import java.io.FileInputStream;
>> +import java.io.FileNotFoundException;
>> +import java.io.FileWriter;
>> +import java.io.IOException;
>> +import java.io.InputStreamReader;
>> +import java.io.Reader;
>> +import java.io.StringWriter;
>> +import java.io.Writer;
>> import java.util.List;
>> -import java.util.ArrayList;
>> -import java.util.HashMap;
>> -import java.util.Iterator;
>> -import java.io.*;
>> +import java.util.Map;
>> +
>> +import javax.xml.parsers.DocumentBuilder;
>> +import javax.xml.parsers.DocumentBuilderFactory;
>> +import javax.xml.parsers.ParserConfigurationException;
>> +import javax.xml.transform.OutputKeys;
>> +import javax.xml.transform.Result;
>> +import javax.xml.transform.Source;
>> +import javax.xml.transform.Transformer;
>> +import javax.xml.transform.TransformerConfigurationException;
>> +import javax.xml.transform.TransformerException;
>> +import javax.xml.transform.TransformerFactory;
>> +import javax.xml.transform.TransformerFactoryConfigurationError;
>> +import javax.xml.transform.dom.DOMSource;
>> +import javax.xml.transform.stream.StreamResult;
>>
>> import javolution.util.FastList;
>> import javolution.util.FastMap;
>>
>> +import org.ofbiz.base.component.ComponentConfig;
>> +import org.ofbiz.base.component.ComponentConfig.WebappInfo;
>> import org.ofbiz.base.container.Container;
>> import org.ofbiz.base.container.ContainerException;
>> +import org.ofbiz.base.start.Classpath;
>> import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilURL;
>> +import org.ofbiz.base.util.UtilProperties;
>> +import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.template.FreeMarkerWorker;
>> -import org.ofbiz.base.start.Classpath;
>> -import org.ofbiz.base.component.ComponentConfig;
>> +import org.w3c.dom.Attr;
>> +import org.w3c.dom.Document;
>> +import org.w3c.dom.NamedNodeMap;
>> +import org.w3c.dom.Node;
>> +import org.xml.sax.SAXException;
>>
>> /**
>>  * GenerateContainer - Generates Configuration Files For Application Servers
>> @@ -47,14 +74,15 @@
>>
>>     public static final String module = GenerateContainer.class.getName();
>>     public static final String source = "/framework/appserver/templates/";
>> -    public static final String target = "/setup/";
>> -    private boolean isGeronimo = false;
>> -
>> +    public static String target = "/setup/";
>>
>>     protected String configFile = null;
>>     protected String ofbizHome = null;
>>     protected String args[] = null;
>>
>> +    private boolean isGeronimo = false;
>> +    private String geronimoHome = null;
>> +
>>     /**
>>      * @see org.ofbiz.base.container.Container#init(java.lang.String[], java.lang.String)
>>      */
>> @@ -63,6 +91,10 @@
>>         this.configFile = configFile;
>>         this.args = args;
>>         this.isGeronimo = args[0].toLowerCase().contains("geronimo") || args[0].toLowerCase().contains("wasce");
>> +        if (this.isGeronimo) {
>> +            this.target="/META-INF/";
>> +            this.geronimoHome = UtilProperties.getPropertyValue("appserver", "geronimoHome", null);
>> +        }
>>     }
>>
>>     /**
>> @@ -86,30 +118,216 @@
>>     private void generateFiles() throws ContainerException {
>>         File files[] = getTemplates();
>>         Map<String, Object> dataMap = buildDataMap();
>> +
>> +        String user = UtilProperties.getPropertyValue("appserver", "user", "system");
>> +        String password = UtilProperties.getPropertyValue("appserver", "password", "manager");
>> +        boolean offline = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "offline", "true");
>> +        boolean pauseInGeronimoScript = UtilProperties.propertyValueEqualsIgnoreCase("appserver", "pauseInGeronimoScript",
>> "true");
>> +        int instancesNumber = (int) UtilProperties.getPropertyNumber("appserver", "instancesNumber");
>> +        String instanceNumber = "";
>> +
>>         if (isGeronimo) {
>> -            String serverType = args[0];
>> -            String geronimoHome = null;
>> -            if (args.length > 2) {
>> -                geronimoHome = args[2];
>> -            }
>> -            GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
>> -            List classpathJars = geronimoDeployment.generate(serverType, geronimoHome);
>> -            if (classpathJars == null) {
>> -                throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +            if (geronimoHome == null) {
>> +                geronimoHome = System.getenv("GERONIMO_HOME");
>> +                if (geronimoHome == null) {
>> +                    Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into
>> a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>> +                File geronimoHomeDir = new File (geronimoHome);
>> +                if (! (geronimoHomeDir.isDirectory())) {
>> +                    Debug.logFatal(geronimoHome + " does not exist or is not a directoy. Please set the location of Geronimo
>> into a GERONIMO_HOME env var or pass it as geronimoHome property in setup.properties file.", module);
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>>             }
>> -            dataMap.put("classpathJars", classpathJars);
>> -        }
>>
>> -        //Debug.log("Using Data : " + dataMap, module);
>> -        String applicationPrefix = "";
>> -        if (args.length > 3 && args[3].length() > 0) {
>> -            applicationPrefix = args[3] + "-";
>> -        }
>> -        dataMap.put("applicationPrefix", applicationPrefix);
>> -        dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
>> -        for (File file: files) {
>> -            if (isGeronimo && !(file.isDirectory() || file.isHidden() || file.getName().equalsIgnoreCase("geronimo-web.xml"))) {
>> -                parseTemplate(file, dataMap);
>> +            for(int inst = 0; inst <= instancesNumber; inst++) {
>> +                instanceNumber = (inst == 0 ? "" : inst).toString();
>> +                GenerateGeronimoDeployment geronimoDeployment = new GenerateGeronimoDeployment();
>> +                List classpathJars = geronimoDeployment.generate(args[0], geronimoHome, instanceNumber);
>> +                if (classpathJars == null) {
>> +                    throw new ContainerException("Error in Geronimo deployment, please check the log");
>> +                }
>> +                dataMap.put("classpathJars", classpathJars);
>> +                dataMap.put("pathSeparatorChar", File.pathSeparatorChar);
>> +                dataMap.put("instanceNumber", instanceNumber);
>> +                //                if (UtilValidate.isNotEmpty(instanceNumber)) {
>> +                //                    List webApps = (List) dataMap.get("webApps");
>> +                //                    for (Object webAppObject: webApps) {
>> +                //                        WebappInfo webAppInfo = (ComponentConfig.WebappInfo) webAppObject;
>> +                //                        String webAppLocation = webAppInfo.getLocation();
>> +                //                        String webXmlLocation = webAppLocation + "/WEB-INF/web.xml";
>> +                //                        if (isFileExistsAndCanWrite(webXmlLocation)) {
>> +                //                            DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance();
>> +                //                            DocumentBuilder docBuilder = null;
>> +                //                            try {
>> +                //                                docBuilder = docFactory.newDocumentBuilder();
>> +                //                            } catch (ParserConfigurationException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            Document doc = null;
>> +                //                            try {
>> +                //                                doc = docBuilder.parse(webXmlLocation);
>> +                //                            } catch (SAXException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            } catch (IOException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            Node webApp = doc.getFirstChild();
>> +                //                            Node contextParam = doc.createElement("context-param");
>> +                //                            NamedNodeMap contextParamAttributes = contextParam.getAttributes();
>> +                //
>> +                //                            Attr paramName = doc.createAttribute("param-name");
>> +                //                            paramName.setValue("instanceNumber");
>> +                //                            contextParamAttributes.setNamedItem(paramName);
>> +                //
>> +                //                            Attr paramValue = doc.createAttribute("param-value");
>> +                //                            paramValue.setValue(instanceNumber);
>> +                //                            contextParamAttributes.setNamedItem(paramValue);
>> +                //        //                    Node nodeToAppend = doc.importNode(contextParam, true); this should not be
>> needed
>> +                //        //                    webApp.appendChild(nodeToAppend);
>> +                //
>> +                //        //                    webApp.appendChild(contextParam); this is the line needed but commented for now
>> +                //
>> +                //                            Transformer transformer;
>> +                //                            try {
>> +                //                                transformer = TransformerFactory.newInstance().newTransformer();
>> +                //                            } catch (TransformerConfigurationException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            } catch (TransformerFactoryConfigurationError e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            transformer.setOutputProperty(OutputKeys.INDENT, "yes");
>> +                //
>> +                //                            StreamResult result = new StreamResult(new StringWriter());
>> +                //                            DOMSource source = new DOMSource(doc);
>> +                //                            try {
>> +                //                                transformer.transform(source, result);
>> +                //                            } catch (TransformerException e) {
>> +                //                                throw new ContainerException(e);
>> +                //                            }
>> +                //                            String xmlString = result.getWriter().toString();
>> +                //                            System.out.println(xmlString); //TODO write to file using writeToXmlFile
>> +                //                            break; // Only the 1st web.xml file need to be modified
>> +                //                        } else {
>> +                //                            Debug.logInfo("Unable to change the deployment descriptor : " + webXmlLocation +
>> ". Maybe it does not exist, or is in read only mode ?", module);
>> +                //                        }
>> +                //                    }
>> +                //                }
>> +
>> +                //Debug.log("Using Data : " + dataMap, module);
>> +                for (int i = 0; i < files.length; i++) {
>> +                    if (!(files[i].isDirectory() || files[i].isHidden() ||
>> files[i].getName().equalsIgnoreCase("geronimo-web.xml"))) {
>> +                        parseTemplate(files[i], dataMap);
>> +                    }
>> +                }
>> +
>> +                String ofbizName = "ofbiz" + instanceNumber;
>> +                String separator = File.separator;
>> +                String geronimoBin = geronimoHome + separator + "bin";
>> +                File workingDir = new File(geronimoBin);
>> +                ProcessBuilder pb = null;
>> +                String command = null;
>> +
>> +                if ("\\".equals(separator)) { //Windows
>> +                    if (offline) {
>> +                        command = "deploy --user " + user +  " --password " +  password + " --offline undeploy " + ofbizName;
>> +                    } else {
>> +                        command = "deploy --user " +  user +  " --password " +  password + " undeploy " + ofbizName;
>> +                    }
>> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
>> +                } else {                        // Linux
>> +                    if (offline) {
>> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline undeploy
>> " + ofbizName;
>> +                    } else {
>> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " undeploy " +
>> ofbizName;
>> +                    }
>> +                    pb = new ProcessBuilder("sh", "-c", command);
>> +                }
>> +
>> +                if (pauseInGeronimoScript) {
>> +                    Map<String, String> env = pb.environment();
>> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
>> +                }
>> +                pb.directory(workingDir);
>> +
>> +                try {
>> +                    System.out.println("Currently undeploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
>> +                    pb.redirectErrorStream(true);
>> +                    Process p = pb.start();
>> +                    java.io.InputStream is = p.getInputStream();
>> +                    byte[] buf = new byte[2024];
>> +                    int readLen = 0;
>> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
>> +                        if ("\\".equals(separator)) {   //Windows
>> +                            System.out.print(new String(buf,0,readLen));
>> +                        } else {
>> +                            System.out.println(new String(buf,0,readLen));
>> +                        }
>> +                    }
>> +                    is.close();
>> +                    p.waitFor();
>> +                    //                    System.out.println(p.waitFor());
>> +                    //                    System.out.println("exit value" + p.exitValue());
>> +                    Debug.logInfo(ofbizName + " undeployment ended" , module);
>> +                } catch (IOException e) {
>> +                    throw new ContainerException(e);
>> +                } catch (InterruptedException e) {
>> +                    throw new ContainerException(e);
>> +                }
>> +
>> +                if ("\\".equals(separator)) { //Windows
>> +                    if (offline) {
>> +                        command = "deploy --user " + user +  " --password " +  password + " --offline deploy --inPlace " +
>> ofbizHome;
>> +                    } else {
>> +                        command = "deploy --user " + user +  " --password " +  password + " deploy --inPlace " + ofbizHome;
>> +                    }
>> +                    pb = new ProcessBuilder("cmd.exe", "/c", command);
>> +                } else {                      // Linux
>> +                    if (offline) {
>> +                        command = workingDir + "/deploy.sh --user " + user +  " --password " +  password + " --offline
>> deploy --inPlace " + ofbizHome;
>> +                    } else {
>> +                        command = workingDir + "/deploy.sh --user " +  user +  " --password " +  password + " deploy --inPlace "
>> + ofbizHome;
>> +                    }
>> +                    pb = new ProcessBuilder("sh", "-c", command);
>> +                }
>> +
>> +                if (pauseInGeronimoScript) {
>> +                    Map<String, String> env = pb.environment();
>> +                    env.put("GERONIMO_BATCH_PAUSE", "on");
>> +                }
>> +                pb.directory(workingDir);
>> +
>> +                try {
>> +                    System.out.println("Currently deploying " + ofbizName + ", using : <<" + command + ">>, please wait ...");
>> +                    pb.redirectErrorStream(true);
>> +                    Process p = pb.start();
>> +                    java.io.InputStream is = p.getInputStream();
>> +                    byte[] buf = new byte[2024];
>> +                    int readLen = 0;
>> +                    while((readLen = is.read(buf,0,buf.length)) != -1) {
>> +                        if ("\\".equals(separator)) {   //Windows
>> +                            System.out.print(new String(buf,0,readLen));
>> +                        } else {
>> +                            System.out.println(new String(buf,0,readLen));
>> +                        }
>> +                    }
>> +                    is.close();
>> +                    p.waitFor();
>> +                    //                    System.out.println(p.waitFor());
>> +                    //                    System.out.println("exit value" + p.exitValue());
>> +                    Debug.logInfo(ofbizName + " deployment ended" , module);
>> +                } catch (IOException e) {
>> +                    throw new ContainerException(e);
>> +                } catch (InterruptedException e) {
>> +                    throw new ContainerException(e);
>> +                }
>> +            }
>> +        } else {
>> +            //Debug.log("Using Data : " + dataMap, module);
>> +            for (int i = 0; i < files.length; i++) {
>> +                if (!files[i].isDirectory() && !files[i].isHidden()) {
>> +                    parseTemplate(files[i], dataMap);
>> +                }
>>             }
>>         }
>>     }
>> @@ -158,6 +376,7 @@
>>                 }
>>             }
>>         }
>> +
>>         List[] lists = { jar, dir };
>>         return lists;
>>     }
>> @@ -213,5 +432,28 @@
>>         } catch (IOException e) {
>>             throw new ContainerException(e);
>>         }
>> +    }
>> +
>> +    // This method writes a DOM document to a file
>> +    public static void writeToXmlFile(Document doc, String filename) {
>> +        try {
>> +            // Prepare the DOM document for writing
>> +            Source source = new DOMSource(doc);
>> +
>> +            // Prepare the output file
>> +            File file = new File(filename);
>> +            Result result = new StreamResult(file);
>> +
>> +            // Write the DOM document to the file
>> +            Transformer xformer = TransformerFactory.newInstance().newTransformer();
>> +            xformer.transform(source, result);
>> +        } catch (TransformerConfigurationException e) {
>> +        } catch (TransformerException e) {
>> +        }
>> +    }
>> +
>> +    public boolean isFileExistsAndCanWrite(String fileName) {
>> +        File f = new File(fileName);
>> +        return f.exists() && f.canWrite();
>>     }
>> }
>>
>> Modified: ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java (original)
>> +++ ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateGeronimoDeployment.java Wed Apr  9 06:51:39 2008
>> @@ -52,18 +52,8 @@
>>     public static final String source = "/framework/appserver/templates/";
>>
>>     protected  String geronimoRepository = null;
>> -    protected String geronimoHome = null;
>>
>> -    @SuppressWarnings("unchecked")
>> -    public List<String> generate(String geronimoVersion, String geronimoHome) {
>> -        // Check environment for Geronimo Home
>> -        if (geronimoHome == null) {
>> -            geronimoHome = System.getenv("GERONIMO_HOME");
>> -            if (geronimoHome == null) {
>> -                Debug.logFatal("'GERONIMO_HOME' was not found in your environment. Please set the location of Geronimo into
>> GERONIMO_HOME.", module);
>> -                return null;
>> -            }
>> -        }
>> +    public List<String> generate(String geronimoVersion, String geronimoHome, String instanceNumber) {
>>         geronimoRepository = geronimoHome + "/repository";
>>         Debug.logInfo("The WASCE or Geronimo Repository is " + geronimoRepository, module);
>>         Classpath classPath = new Classpath(System.getProperty("java.class.path"));
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/README
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/README?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/README (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/README Wed Apr  9 06:51:39 2008
>> @@ -17,9 +17,10 @@
>>     under the License.
>> -->
>>
>> -Websphere Application Server Community Edition 2 Setup - 2008-03-31 - Jacques Le Roux
>> +Websphere Application Server Community Edition (WASCE) 2
>> +or Geronimo 2.0.3 Setup - 2008-04-08 - Jacques Le Roux
>>
>> -To use WASCE 2 with OFBiz 4.0 follow the following steps (all steps assume Orion is already installed and working)
>> +To use OFBiz with WASCE 2 or Geronimo 2.0.3 follow the following steps (all steps assume WASCE 2 or Geronimo 2.0.3  is already
>> installed and working)
>>
>> =======================================================================================================================================================
>>     If you use WASCE, copy the 2 lines below in the setJavaOpts section (Windows) or somewhere in the setEnv script you will find
>> in GERONIMO_HOME/bin
>> @@ -33,13 +34,44 @@
>>     <#assign classpath = classpath + dir + pathSeparatorChar/>
>> </#list>
>> <#assign classpath = classpath?substring(0, classpath?length - 1)/>
>> +
>> +Windows
>> +-------
>> set JAVA_OPTS=-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en
>> set CLASSPATH=${classpath}
>>
>> +Linux
>> +-----
>> +Add this 2 lines after the
>> +    elif [ "$1" = "run" ]; then
>> +line
>> +  JAVA_OPTS="-Dofbiz.home="${ofbizHome}" -Xms256M -Xmx512M -XX:MaxPermSize=128M -Duser.language=en"
>> +  CLASSPATH="${classpath}"
>> +Don't forget to escape the inside quotes in ${ofbizHome}.
>> +
>> =======================================================================================================================================================
>> -    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the Geronimo script.
>> +    Change also this line of the geronimo script in the doneSetArgs section (Windows) or somewhere in the geronimo script
>> =======================================================================================================================================================
>> -
>> +Windows
>> +-------
>> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS%
>> %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed"
>>  -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -jar %_JARFILE% %_LONG_OPT%
>> %CMD_LINE_ARGS%
>>     to
>> %_EXECJAVA% %JAVA_OPTS% %GERONIMO_OPTS%
>> %JAVA_AGENT_OPTS% -Djava.ext.dirs="%GERONIMO_BASE%\lib\ext;%JRE_HOME%\lib\ext" -Djava.endorsed.dirs="%GERONIMO_BASE%\lib\endorsed;%JRE_HOME%\lib\endorsed"
>>  -Dorg.apache.geronimo.base.dir="%GERONIMO_BASE%" -Djava.io.tmpdir="%GERONIMO_TMPDIR%" -cp .;%_JARFILE%;%CLASSPATH% %_LONG_OPT%
>> %CMD_LINE_ARGS% %MAINCLASS%
>> +
>> +Linux
>> +-----
>> +In the <<elif [ "$1" = "run" ]>> block
>> +Replace the line
>> +    -jar "$GERONIMO_HOME"/bin/server.jar $LONG_OPT "$@"
>> +by
>> +    -classpath ".":"$GERONIMO_HOME"/bin/server.jar:"$CLASSPATH" org.apache.geronimo.cli.daemon.DaemonCLI $LONG_OPT "$@"
>> +
>> +=======================================================================================================================================================
>> +    Windows only
>> +    ------------
>> +    If you want to use OFBiz multi-instances in Geronimo or WASCE using you might put "exit" as last line in the deploy script.
>> +    Else, of course you may also type exit by hand....
>> +
>> +    Alternatively you may not put exit at the end of the deploy script,
>> +    choose to not have pause in script and type "exit" yourself in the command windows (actually I found later that it's an
>> easier way)
>> +=======================================================================================================================================================
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/application.xml
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/application.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/application.xml (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/application.xml Wed Apr  9 06:51:39 2008
>> @@ -29,36 +29,31 @@
>>   id="Application_ID"
>>   version="5">
>>
>> -  <description>The Apache OFBiz Project</description>
>> -  <display-name>OFBiz</display-name>
>> +  <description>The Apache OFBiz Project  this is instance ${instanceNumber}</description>
>> +  <display-name>OFBiz instance ${instanceNumber}</display-name>
>>
>>   <#list webApps as webapp>
>>     <#assign location = webapp.getLocation()?if_exists/>
>> -    <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
>> -      || location.contains("applications")
>> -      || location.contains("specialpurpose")
>> -      || location.contains("hot-deploy")>
>> -      <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
>> +    <#assign origin = webapp.getLocation()?if_exists/>
>> +      <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>>         <#assign location = location.substring(location.lastIndexOf("framework"))/>
>>       </#if>
>> -      <#if location.contains("applications")>
>> +    <#if location.contains("/applications/")>
>>         <#assign location = location.substring(location.lastIndexOf("applications"))/>
>>       </#if>
>> -      <#if location.contains("specialpurpose")>
>> +    <#if location.contains("/specialpurpose/")>
>>         <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>>       </#if>
>> -      <#if location.contains("hot-deploy") && !location.contains("images")>
>> +      <#if location.contains("/hot-deploy/")>
>>         <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>>       </#if>
>> -      <#if !(location.contains("neogia") && location.contains("shipment"))
>> -        && !(location.contains("hot-deploy") && location.contains("images"))>
>> -  <module id="${applicationPrefix}${webapp.getName()}">
>> +    <#if location != origin>
>> +  <module id="${webapp.getName()}${instanceNumber}">
>>     <web>
>>       <web-uri>${location}/</web-uri>
>> -      <context-root>${webapp.getContextRoot()}</context-root>
>> +      <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>>     </web>
>>   </module>
>> -      </#if>
>>     </#if>
>>   </#list>
>> </application>
>>
>> Modified: ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml (original)
>> +++ ofbiz/trunk/framework/appserver/templates/wasce2/geronimo-application.xml Wed Apr  9 06:51:39 2008
>> @@ -28,7 +28,7 @@
>>     <dep:environment>
>>         <dep:moduleId>
>>             <dep:groupId>org.ofbiz</dep:groupId>
>> -            <dep:artifactId>${applicationPrefix}ofbiz</dep:artifactId>
>> +            <dep:artifactId>ofbiz${instanceNumber}</dep:artifactId>
>>             <dep:version>1.0</dep:version>
>>             <dep:type>ear</dep:type>
>>         </dep:moduleId>
>> @@ -51,31 +51,26 @@
>>
>>     <#list webApps as webapp>
>>         <#assign location = webapp.getLocation()?if_exists/>
>> -        <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))
>> -            || location.contains("applications")
>> -            || location.contains("specialpurpose")
>> -            || location.contains("hot-deploy")>
>> -            <#if location.contains("framework") && (location.contains("webtools") || location.contains("images"))>
>> +        <#assign origin = webapp.getLocation()?if_exists/>
>> +        <#if location.contains("/framework/") && (location.contains("/webtools/") || location.contains("/images/"))>
>>                 <#assign location = location.substring(location.lastIndexOf("framework"))/>
>> -            </#if>
>> -            <#if location.contains("applications")>
>> -                <#assign location = location.substring(location.lastIndexOf("applications"))/>
>> -            </#if>
>> -            <#if location.contains("specialpurpose")>
>> -                <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>> -            </#if>
>> -            <#if location.contains("hot-deploy") && !location.contains("images")>
>> -                <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>> -            </#if>
>> -            <#if !(location.contains("neogia") && location.contains("shipment"))
>> -                && !(location.contains("hot-deploy") && location.contains("images"))>
>> +        </#if>
>> +        <#if location.contains("/applications/")>
>> +            <#assign location = location.substring(location.lastIndexOf("applications"))/>
>> +        </#if>
>> +        <#if location.contains("/specialpurpose/")>
>> +            <#assign location = location.substring(location.lastIndexOf("specialpurpose"))/>
>> +        </#if>
>> +        <#if location.contains("/hot-deploy/")>
>> +            <#assign location = location.substring(location.lastIndexOf("hot-deploy"))/>
>> +        </#if>
>> +        <#if location != origin>
>>     <module>
>>         <web>${location}/</web>
>>         <web-app xmlns="http://geronimo.apache.org/xml/ns/j2ee/web-1.2">
>> -            <context-root>${webapp.getContextRoot()}</context-root>
>> +            <context-root>${webapp.getContextRoot()}${instanceNumber}</context-root>
>>         </web-app>
>>     </module>
>> -            </#if>
>>         </#if>
>>     </#list>
>>
>>
>> Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java (original)
>> +++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/string/FlexibleStringExpander.java Wed Apr  9 06:51:39 2008
>> @@ -30,6 +30,7 @@
>> import org.ofbiz.base.util.BshUtil;
>> import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilDateTime;
>> +import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.collections.FlexibleMapAccessor;
>> import org.ofbiz.base.util.UtilFormatOut;
>> import org.ofbiz.base.util.UtilMisc;
>> @@ -395,6 +396,11 @@
>>                     targetBuffer.append(UtilFormatOut.formatCurrency(new Double(envVal.toString()), currencyCode, locale));
>>                 } else {
>>                     targetBuffer.append(envVal.toString());
>> +                }
>> +            } else if (envName.equals("ofbiz.home")) { // This is only used in case of Geronimo or WASCE using OFBiz
>> multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>> +                String ofbizHome = System.getProperty("ofbiz.home");
>> +                if (UtilValidate.isNotEmpty(ofbizHome)) {
>> +                    targetBuffer.append(ofbizHome);
>>                 }
>>             } else {
>>                 Debug.logWarning("Could not find value in environment for the name [" + envName + "], inserting nothing.",
>> module);
>>
>> Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java (original)
>> +++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ContextFilter.java Wed Apr  9 06:51:39 2008
>> @@ -346,6 +346,15 @@
>>             if (Debug.infoOn()) Debug.logInfo("Adding web.xml context-param to application attribute with name [" + initParamName
>> + "] and value [" + initParamValue + "]", module);
>>             config.getServletContext().setAttribute(initParamName, initParamValue);
>>         }
>> +        String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
>> +        if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
>> +            String ofbizHome = System.getProperty("ofbiz.home");
>> +            if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
>> +                ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz
>> multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>> +                System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
>> +                System.setProperty("ofbiz.home", ofbizHome);
>> +            }
>> +        }
>>     }
>>
>>     protected String getServerId() {
>>
>> Modified: ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml?rev=646349&r1=646348&r2=646349&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml (original)
>> +++ ofbiz/trunk/framework/webtools/webapp/webtools/WEB-INF/web.xml Wed Apr  9 06:51:39 2008
>> @@ -149,4 +149,11 @@
>>     </user-data-constraint>
>>   </security-constraint>
>> -->
>> +
>> +  <context-param>
>> +    <param-name>GeronimoMultiOfbizInstances</param-name>
>> +    <param-value>true</param-value>
>> +    <description>This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home
>> value set in JVM env</description>
>> +  </context-param>
>> +
>> </web-app>
>>
>>
>
>

[Jacopo Cappellato-4]

On Jun 25, 2012, at 7:12 PM, Jacques Le Roux wrote:

> From: "Jacopo Cappellato" <[hidden email]>
>> Hi all,
>>
>> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you
>> agree?
>> Jacques, before I start my review please let me know if you want to do a first pass on it.
>
> I can certainly have a fresh look, depends on timing though...
> What did you saw at 1st glance which makes you want to improve, is it related to GeronimoMultiOfbizInstances?
>
> Jacques

Mostly all the code in the commit seems questionable and I would like to see it reverted or completely refactored.
In particular, the changes to ContextFilter are awful:

        String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
        if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
            String ofbizHome = System.getProperty("ofbiz.home");
            if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
                ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz multi-instances. It allows to retrieve ofbiz.home value set in JVM env
                System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
                System.setProperty("ofbiz.home", ofbizHome);
            }
        }

In just a few lines added to the class I see:

* bad variable name ("GeronimoMultiOfbizInstances")
* two if blocks that should be one
* a bad practice for logging
* all the code seems like a bad tweak
* the code doesn't make any sense to me... to the point that I may be misunderstanding it: why do you get the property and then set it? how this is going to fix the problem? what was the problem?

Why did you commit code like this? Did you really think it was good for OFBiz?

Regards,

Jacopo

[Jacques Le Roux]

Hi Jacopo,

If it's only that part (all related to GeronimoMultiOfbizInstances) which worries you then no problem for me to revert it (I will
put a note into Attic though).
It was a try (not in requirements I had, but made sense) but I never really got a chance to finish and nobody was interested it
seems.
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=7045153#Geronimo&IBMWebsphereApplicationServerCommunityEdition-Multiinstances

We could even put all Geronimo/WASCE  in Attic
I think nobody is really using it nowadays.  Anyway a mention and link to Attic from the wiki page would do it.
BTW this list of commit should help since removing framework\appserver\templates\wasce2 will not be enough
http://svn.apache.org/viewvc?view=revision&revision=643173
http://svn.apache.org/viewvc?view=revision&revision=646349
http://svn.apache.org/viewvc?view=revision&revision=652177

I can do it myself if you want

BTW I wonder if we should keep all appserver implementations in trunk? Some begin to be really old. I must say that they are much
simple, the challenge in the requirements was to deploy automatically (r646349)

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> On Jun 25, 2012, at 7:12 PM, Jacques Le Roux wrote:
>
>> From: "Jacopo Cappellato" <[hidden email]>
>>> Hi all,
>>>
>>> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you
>>> agree?
>>> Jacques, before I start my review please let me know if you want to do a first pass on it.
>>
>> I can certainly have a fresh look, depends on timing though...
>> What did you saw at 1st glance which makes you want to improve, is it related to GeronimoMultiOfbizInstances?
>>
>> Jacques
>
> Mostly all the code in the commit seems questionable and I would like to see it reverted or completely refactored.
> In particular, the changes to ContextFilter are awful:
>
>        String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
>        if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
>            String ofbizHome = System.getProperty("ofbiz.home");
>            if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
>                ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz
> multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>                System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
>                System.setProperty("ofbiz.home", ofbizHome);
>            }
>        }
>
> In just a few lines added to the class I see:
>
> * bad variable name ("GeronimoMultiOfbizInstances")
> * two if blocks that should be one
> * a bad practice for logging
> * all the code seems like a bad tweak
> * the code doesn't make any sense to me... to the point that I may be misunderstanding it: why do you get the property and then
> set it? how this is going to fix the problem? what was the problem?
>
> Why did you commit code like this? Did you really think it was good for OFBiz?
>
> Regards,
>
> Jacopo
>
>

[Jacopo Cappellato-4]

Hi Jacques,

if you could help to remove all code that is specific to Geronimo/WASCE that would be great in my opinion; some of the code I have seen is really a kind of tweak and if it is not finished it would be better to clean everything and then, when someone will really have to work with Geronimo again, and if the issues you have faced are still there in the new versions, then we could see if we can improve the framework without requiring a specific handling/tweak for Geronimo.
As regards the appserver component... in my opinion it could go to specialpurpose or to attic (all or a part of it).

Regards,

Jacopo

On Jul 14, 2012, at 11:49 AM, Jacques Le Roux wrote:

> Hi Jacopo,
>
> If it's only that part (all related to GeronimoMultiOfbizInstances) which worries you then no problem for me to revert it (I will put a note into Attic though).
> It was a try (not in requirements I had, but made sense) but I never really got a chance to finish and nobody was interested it seems.
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=7045153#Geronimo&IBMWebsphereApplicationServerCommunityEdition-Multiinstances
>
> We could even put all Geronimo/WASCE  in Attic
> I think nobody is really using it nowadays.  Anyway a mention and link to Attic from the wiki page would do it.
> BTW this list of commit should help since removing framework\appserver\templates\wasce2 will not be enough
> http://svn.apache.org/viewvc?view=revision&revision=643173
> http://svn.apache.org/viewvc?view=revision&revision=646349
> http://svn.apache.org/viewvc?view=revision&revision=652177
>
> I can do it myself if you want
>
> BTW I wonder if we should keep all appserver implementations in trunk? Some begin to be really old. I must say that they are much simple, the challenge in the requirements was to deploy automatically (r646349)
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> On Jun 25, 2012, at 7:12 PM, Jacques Le Roux wrote:
>>
>>> From: "Jacopo Cappellato" <[hidden email]>
>>>> Hi all,
>>>>
>>>> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you
>>>> agree?
>>>> Jacques, before I start my review please let me know if you want to do a first pass on it.
>>>
>>> I can certainly have a fresh look, depends on timing though...
>>> What did you saw at 1st glance which makes you want to improve, is it related to GeronimoMultiOfbizInstances?
>>>
>>> Jacques
>>
>> Mostly all the code in the commit seems questionable and I would like to see it reverted or completely refactored.
>> In particular, the changes to ContextFilter are awful:
>>
>>       String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
>>       if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
>>           String ofbizHome = System.getProperty("ofbiz.home");
>>           if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
>>               ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz
>> multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>>               System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
>>               System.setProperty("ofbiz.home", ofbizHome);
>>           }
>>       }
>>
>> In just a few lines added to the class I see:
>>
>> * bad variable name ("GeronimoMultiOfbizInstances")
>> * two if blocks that should be one
>> * a bad practice for logging
>> * all the code seems like a bad tweak
>> * the code doesn't make any sense to me... to the point that I may be misunderstanding it: why do you get the property and then
>> set it? how this is going to fix the problem? what was the problem?
>>
>> Why did you commit code like this? Did you really think it was good for OFBiz?
>>
>> Regards,
>>
>> Jacopo
>>
>>

[Jacques Le Roux]

It's actually finished and has been used by a company during 2 years. Initially I did it like the other appservers has been done
precedently.
But I then got a requirement to automatically deploy. This works began at http://svn.apache.org/viewvc?view=revision&revision=646349
and was completed, see
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=7045153#Geronimo&IBMWebsphereApplicationServerCommunityEdition-Setupanddeploy(orredeploy)

The only part which was not completed is the 2n point
"2. Instances are differents or mixed (some instances may be the same)" at the link I gave below.
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=7045153#Geronimo&IBMWebsphereApplicationServerCommunityEdition-Multiinstances
where it's explained.

Note that it's rare to have a complete documentation in wiki of a such feature (a bit exotic in OFBiz world).
From comment I got from users and even consultants, with it's aging UI, documentation is what OFBiz misses more...

Anyway, as I said, I think nobody use it anymore. The company which payed for it has been sold and restructured since. So it's no pb
for me
to put in Attic, from where it can be resurrected at any moment (mayber with some pain later though, since part of code used whiil
change)
Then this must be done carefully to allow its possible use later. Though it seems also that Geronimo did not have a big momentum
lately, so I don't expect much about that...

For the appserver component, we would need a larger consensus. If ever people are interested to simply  give their opinions :/
Else, then Attic, people will have to help themself more if they are not supporting the team enough! Most of the time they come,
pick what they need and go.

Jacques

From: "Jacopo Cappellato" <[hidden email]>

> Hi Jacques,
>
> if you could help to remove all code that is specific to Geronimo/WASCE that would be great in my opinion; some of the code I have
> seen is really a kind of tweak and if it is not finished it would be better to clean everything and then, when someone will really
> have to work with Geronimo again, and if the issues you have faced are still there in the new versions, then we could see if we
> can improve the framework without requiring a specific handling/tweak for Geronimo.
> As regards the appserver component... in my opinion it could go to specialpurpose or to attic (all or a part of it).
>
> Regards,
>
> Jacopo
>
> On Jul 14, 2012, at 11:49 AM, Jacques Le Roux wrote:
>
>> Hi Jacopo,
>>
>> If it's only that part (all related to GeronimoMultiOfbizInstances) which worries you then no problem for me to revert it (I will
>> put a note into Attic though).
>> It was a try (not in requirements I had, but made sense) but I never really got a chance to finish and nobody was interested it
>> seems.
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=7045153#Geronimo&IBMWebsphereApplicationServerCommunityEdition-Multiinstances
>>
>> We could even put all Geronimo/WASCE  in Attic
>> I think nobody is really using it nowadays.  Anyway a mention and link to Attic from the wiki page would do it.
>> BTW this list of commit should help since removing framework\appserver\templates\wasce2 will not be enough
>> http://svn.apache.org/viewvc?view=revision&revision=643173
>> http://svn.apache.org/viewvc?view=revision&revision=646349
>> http://svn.apache.org/viewvc?view=revision&revision=652177
>>
>> I can do it myself if you want
>>
>> BTW I wonder if we should keep all appserver implementations in trunk? Some begin to be really old. I must say that they are much
>> simple, the challenge in the requirements was to deploy automatically (r646349)
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <[hidden email]>
>>> On Jun 25, 2012, at 7:12 PM, Jacques Le Roux wrote:
>>>
>>>> From: "Jacopo Cappellato" <[hidden email]>
>>>>> Hi all,
>>>>>
>>>>> this is part of my review of framework code to make it cleaner. I see a series of questionable changes in this commit, do you
>>>>> agree?
>>>>> Jacques, before I start my review please let me know if you want to do a first pass on it.
>>>>
>>>> I can certainly have a fresh look, depends on timing though...
>>>> What did you saw at 1st glance which makes you want to improve, is it related to GeronimoMultiOfbizInstances?
>>>>
>>>> Jacques
>>>
>>> Mostly all the code in the commit seems questionable and I would like to see it reverted or completely refactored.
>>> In particular, the changes to ContextFilter are awful:
>>>
>>>       String GeronimoMultiOfbizInstances = (String) config.getServletContext().getAttribute("GeronimoMultiOfbizInstances");
>>>       if (UtilValidate.isNotEmpty(GeronimoMultiOfbizInstances)) {
>>>           String ofbizHome = System.getProperty("ofbiz.home");
>>>           if (GeronimoMultiOfbizInstances.equalsIgnoreCase("true") && UtilValidate.isEmpty(ofbizHome)) {
>>>               ofbizHome = System.getProperty("ofbiz.home"); // This is only used in case of Geronimo or WASCE using OFBiz
>>> multi-instances. It allows to retrieve ofbiz.home value set in JVM env
>>>               System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
>>>               System.setProperty("ofbiz.home", ofbizHome);
>>>           }
>>>       }
>>>
>>> In just a few lines added to the class I see:
>>>
>>> * bad variable name ("GeronimoMultiOfbizInstances")
>>> * two if blocks that should be one
>>> * a bad practice for logging
>>> * all the code seems like a bad tweak
>>> * the code doesn't make any sense to me... to the point that I may be misunderstanding it: why do you get the property and then
>>> set it? how this is going to fix the problem? what was the problem?
>>>
>>> Why did you commit code like this? Did you really think it was good for OFBiz?
>>>
>>> Regards,
>>>
>>> Jacopo
>>>
>>>
>
>