You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2012/04/14 09:30:31 UTC

svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Author: jleroux
Date: Sat Apr 14 07:30:30 2012
New Revision: 1326064

URL: http://svn.apache.org/viewvc?rev=1326064&view=rev
Log:
Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect <distributable/> Node for web.xml files." https://issues.apache.org/jira/browse/OFBIZ-4242

Per the servlet specification, the web.xml file should allow an optional <distributable/> node to indicate that the application is clusterable. Currently, OFBiz does not respect this setting and assumes all applications should be distributed if any cluster configuration is provided in the ofbiz-containers.xml file. As a result, if, for example, the DeltaManager is enable, all applications attempt to cluster and communicate via DeltaManager.

The expected and proper functionality is to check the individual application's web.xml file for the <distributable/> node and only use the DeltaManager if found; otherwise the StandardManager should be used for sessions.

jleroux: replaced some tabs, reformatted, added a comment about <distributable/> in the *-containers.file

Modified:
    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
    ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java (original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java Sat Apr 14 07:30:30 2012
@@ -29,6 +29,7 @@ import java.io.FileWriter;
 import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.Reader;
 import java.io.Writer;
 import java.net.MalformedURLException;
 import java.util.List;
@@ -67,7 +68,7 @@ public class FileUtil {
 
     /**
      * Converts a file path to one that is compatible with the host operating system.
-     * 
+     *
      * @param path The file path to convert.
      * @return The converted file path.
      */
@@ -341,4 +342,57 @@ public class FileUtil {
             return false;
         }
     }
+
+    /**
+    *
+    *
+    * Search for the specified <code>searchString</code> in the given
+    * {@link Reader}.
+    *
+    * @param reader A Reader in which the String will be searched.
+    * @param searchString The String to search for
+    * @return <code>TRUE</code> if the <code>searchString</code> is found;
+    *         <code>FALSE</code> otherwise.
+    * @throws IOException
+    */
+   public static boolean containsString(Reader reader, final String searchString) throws IOException {
+       char[] buffer = new char[1024];
+       int numCharsRead;
+       int count = 0;
+       while((numCharsRead = reader.read(buffer)) > 0) {
+           for (int c = 0; c < numCharsRead; ++c) {
+               if (buffer[c] == searchString.charAt(count)) count++;
+               else count = 0;
+               if (count == searchString.length()) return true;
+           }
+       }
+       return false;
+   }
+
+   /**
+    *
+    *
+    * Search for the specified <code>searchString</code> in the given
+    * filename. If the specified file doesn't exist, <code>FALSE</code>
+    * returns.
+    *
+    * @param fileName A full path to a file in which the String will be searched.
+    * @param searchString The String to search for
+    * @return <code>TRUE</code> if the <code>searchString</code> is found;
+    *         <code>FALSE</code> otherwise.
+    * @throws IOException
+    */
+   public static boolean containsString(final String fileName, final String searchString) throws IOException {
+       File inFile = new File(fileName);
+       if (inFile.exists()) {
+           BufferedReader in = new BufferedReader(new FileReader(inFile));
+           try {
+               return containsString(in, searchString);
+           } finally {
+               if (in != null)in.close();
+           }
+       } else {
+           return false;
+       }
+   }
 }

Modified: ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff
==============================================================================
--- ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java (original)
+++ ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java Sat Apr 14 07:30:30 2012
@@ -75,9 +75,10 @@ import org.ofbiz.base.concurrent.Executi
 import org.ofbiz.base.container.ClassLoaderContainer;
 import org.ofbiz.base.container.Container;
 import org.ofbiz.base.container.ContainerConfig;
-import org.ofbiz.base.container.ContainerException;
 import org.ofbiz.base.container.ContainerConfig.Container.Property;
+import org.ofbiz.base.container.ContainerException;
 import org.ofbiz.base.util.Debug;
+import org.ofbiz.base.util.FileUtil;
 import org.ofbiz.base.util.SSLUtil;
 import org.ofbiz.base.util.UtilURL;
 import org.ofbiz.base.util.UtilValidate;
@@ -537,7 +538,7 @@ public class CatalinaContainer implement
             } catch (Exception e) {
                 Debug.logError(e, "Couldn't create connector.", module);
             }
-            
+
             try {
                 for (ContainerConfig.Container.Property prop: connectorProp.properties.values()) {
                     connector.setProperty(prop.name, prop.value);
@@ -583,7 +584,7 @@ public class CatalinaContainer implement
                 engine.addChild(host);
             }
         }
-        
+
         if (host instanceof StandardHost) {
             // set the catalina's work directory to the host
             StandardHost standardHost = (StandardHost) host;
@@ -618,11 +619,24 @@ public class CatalinaContainer implement
             mount = mount.substring(0, mount.length() - 2);
         }
 
+
+        final String webXmlFilePath = new StringBuilder().append(location)
+            .append(File.separatorChar).append("WEB-INF")
+            .append(File.separatorChar).append("web.xml").toString();
+        boolean appIsDistributable = false;
+        try {
+            appIsDistributable = FileUtil.containsString(webXmlFilePath, "<distributable/>");
+        } catch (IOException e) {
+            Debug.logWarning(String.format("Failed to read web.xml [%s].", webXmlFilePath), module);
+            appIsDistributable = false;
+        }
+        final boolean contextIsDistributable = distribute && appIsDistributable;
+
         // configure persistent sessions
         Property clusterProp = clusterConfig.get(engine.getName());
 
         Manager sessionMgr = null;
-        if (clusterProp != null) {
+        if (clusterProp != null && contextIsDistributable) {
             String mgrClassName = ContainerConfig.getPropertyValue(clusterProp, "manager-class", "org.apache.catalina.ha.session.DeltaManager");
             try {
                 sessionMgr = (Manager)Class.forName(mgrClassName).newInstance();
@@ -639,16 +653,16 @@ public class CatalinaContainer implement
         context.setDocBase(location);
         context.setPath(mount);
         context.addLifecycleListener(new ContextConfig());
-        
+
         JarScanner jarScanner = context.getJarScanner();
         if (jarScanner instanceof StandardJarScanner) {
             StandardJarScanner standardJarScanner = (StandardJarScanner) jarScanner;
             standardJarScanner.setScanClassPath(false);
         }
-        
+
         Engine egn = (Engine) context.getParent().getParent();
         egn.setService(tomcat.getService());
-        
+
         Debug.logInfo("host[" + host + "].addChild(" + context + ")", module);
         //context.setDeployOnStartup(false);
         //context.setBackgroundProcessorDelay(5);
@@ -664,7 +678,9 @@ public class CatalinaContainer implement
         context.setAllowLinking(true);
 
         context.setReloadable(contextReloadable);
-        context.setDistributable(distribute);
+
+        context.setDistributable(contextIsDistributable);
+
         context.setCrossContext(crossContext);
         context.setPrivileged(appInfo.privileged);
         context.setManager(sessionMgr);



Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Ha yes, I already put a comment there and it's actually clear: you need to set both to have a webapp distributable
ie
<property name="apps-distributable" value="true"/>
in ofbiz-container.catalina-container
AND
<distributable/>
in each webapp you want to distribute

Jacques

From: "Pierre Smits" <pi...@gmail.com>
> Looking for 'distributable' I found no reference in any web.xml. However I
> found in  *-containers.xml the reference:
>
> <property name="apps-distributable" value="false"/><!-- you must also set
> all the webapps you want distributable, by adding <distributable/> in their
> web.xml file -->
>
> If I understand it correctly than this overrides any <distributable/> or
> <distributable /> in web.xml files. So it could be safe to have it in any
> web.xml.
>
> Shall I provide a patch for the code used by the  <ant create-component>
> process to establish new components in the hot-deploy folder?
>
> Regards,
>
> Pierre
>
> 2012/4/30 Scott Gray <sc...@hotwaxmedia.com>
>
>> This is not a good implementation, simply searching the web.xml file for a
>> string containing "<distributable/>" is not good enough.  It'll find the
>> tag even if commented out and and won't find the tag "<distributable />"
>> (space before closing, perfectly valid).  I'm surprised this got past you
>> Jacques.
>>
>> Regards
>> Scott
>>
>> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>>
>> > Pierre,
>> >
>> > I did not test, but I believe it's only used in a clustered environment
>> and should have any impacts in other cases.
>> > It makes only a webapp distributable. Before we added this, all were
>> distributable by default, which could annoying in certain circumstances.
>> > So adding it in web.xml files should not changes from previous
>> behaviour. Which makes me believe it's safe... (see commit for more)
>> >
>> > Jacque
>> >
>> > From: "Pierre Smits" <pi...@gmail.com>
>> >> Jacques,
>> >>
>> >> If I have this tag in the web.xml, but the change is tested in an
>> >> unclustered environment what do you thing the result would be? Is it of
>> no
>> >> effect, would it fail?
>> >>
>> >> Regards,
>> >>
>> >> Pierre
>> >>
>> >> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>> >>
>> >>> Hi Sam,
>> >>>
>> >>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>> >>> html#Cluster_Basics<
>> http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>> >>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<
>> http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>> >>>
>> >>> Also consider
>> >>> <property name="apps-cross-context" value="true"/>
>> >>> <property name="apps-context-reloadable" value="true"/>
>> >>> (false by default)
>> >>>
>> >>> If you use sticky sessions all above is not an issue...
>> >>>
>> >>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>> >>> provide one ;o)
>> >>>
>> >>> Jacques
>> >>>
>> >>>
>> >>> Sam Hamilton wrote:
>> >>>
>> >>>> Hi Jacques,
>> >>>>
>> >>>> Quick question - does this setting mean that now if you uncomment
>> >>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>> >>>> cluster and that you should also add a <distributable/> tag to all the
>> >>>> web.xml files?
>> >>>>
>> >>>> Thanks
>> >>>> Sam
>> >>>>
>> >>>>
>> >>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>> >>>>
>> >>>> Author: jleroux
>> >>>>> Date: Sat Apr 14 07:30:30 2012
>> >>>>> New Revision: 1326064
>> >>>>>
>> >>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<
>> http://svn.apache.org/viewvc?rev=1326064&view=rev>
>> >>>>> Log:
>> >>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>> >>>>> <distributable/> Node for web.xml files."
>> >>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<
>> https://issues.apache.org/jira/browse/OFBIZ-4242>
>> >>>>>
>> >>>>> Per the servlet specification, the web.xml file should allow an
>> optional
>> >>>>> <distributable/> node to indicate that the application
>> >>>>> is clusterable. Currently, OFBiz does not respect this setting and
>> >>>>> assumes all applications should be distributed if any cluster
>> >>>>> configuration is provided in the ofbiz-containers.xml file. As a
>> result,
>> >>>>> if, for example, the DeltaManager is enable, all
>> >>>>> applications attempt to cluster and communicate via DeltaManager.
>> >>>>>
>> >>>>> The expected and proper functionality is to check the individual
>> >>>>> application's web.xml file for the <distributable/> node and
>> >>>>> only use the DeltaManager if found; otherwise the StandardManager
>> should
>> >>>>> be used for sessions.
>> >>>>>
>> >>>>> jleroux: replaced some tabs, reformatted, added a comment about
>> >>>>> <distributable/> in the *-containers.file
>> >>>>>
>> >>>>> Modified:
>> >>>>>  ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>> >>>>>
>>  ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>> >>>>> CatalinaContainer.java
>> >>>>>
>> >>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>> >>>>> FileUtil.java
>> >>>>> URL:
>> >>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>> >>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>> >>>>> 1326063&r2=1326064&view=diff<
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff
>> >
>> >>>>>
>> ==============================**==============================**==================
>> >>>>> ---
>> >>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>> >>>>> (original) +++
>> >>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>> >>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>> >>>>> java.io.FileWriter;
>> >>>>> import java.io.FilenameFilter;
>> >>>>> import java.io.IOException;
>> >>>>> import java.io.OutputStream;
>> >>>>> +import java.io.Reader;
>> >>>>> import java.io.Writer;
>> >>>>> import java.net.**MalformedURLException;
>> >>>>> import java.util.List;
>> >>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>> >>>>>
>> >>>>>   /**
>> >>>>>    * Converts a file path to one that is compatible with the host
>> >>>>> operating system.
>> >>>>> -     *
>> >>>>> +     *
>> >>>>>    * @param path The file path to convert.
>> >>>>>    * @return The converted file path.
>> >>>>>    */
>> >>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>> >>>>>           return false;
>> >>>>>       }
>> >>>>>   }
>> >>>>> +
>> >>>>> +    /**
>> >>>>> +    *
>> >>>>> +    *
>> >>>>> +    * Search for the specified <code>searchString</code> in the
>> given
>> >>>>> +    * {@link Reader}.
>> >>>>> +    *
>> >>>>> +    * @param reader A Reader in which the String will be searched.
>> >>>>> +    * @param searchString The String to search for
>> >>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>> >>>>> found;
>> >>>>> +    *         <code>FALSE</code> otherwise.
>> >>>>> +    * @throws IOException
>> >>>>> +    */
>> >>>>> +   public static boolean containsString(Reader reader, final String
>> >>>>> searchString) throws IOException {
>> >>>>> +       char[] buffer = new char[1024];
>> >>>>> +       int numCharsRead;
>> >>>>> +       int count = 0;
>> >>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>> >>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>> >>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>> >>>>> +               else count = 0;
>> >>>>> +               if (count == searchString.length()) return true;
>> >>>>> +           }
>> >>>>> +       }
>> >>>>> +       return false;
>> >>>>> +   }
>> >>>>> +
>> >>>>> +   /**
>> >>>>> +    *
>> >>>>> +    *
>> >>>>> +    * Search for the specified <code>searchString</code> in the
>> given
>> >>>>> +    * filename. If the specified file doesn't exist,
>> <code>FALSE</code>
>> >>>>> +    * returns.
>> >>>>> +    *
>> >>>>> +    * @param fileName A full path to a file in which the String
>> will be
>> >>>>> searched.
>> >>>>> +    * @param searchString The String to search for
>> >>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>> >>>>> found;
>> >>>>> +    *         <code>FALSE</code> otherwise.
>> >>>>> +    * @throws IOException
>> >>>>> +    */
>> >>>>> +   public static boolean containsString(final String fileName, final
>> >>>>> String searchString) throws IOException {
>> >>>>> +       File inFile = new File(fileName);
>> >>>>> +       if (inFile.exists()) {
>> >>>>> +           BufferedReader in = new BufferedReader(new
>> >>>>> FileReader(inFile));
>> >>>>> +           try {
>> >>>>> +               return containsString(in, searchString);
>> >>>>> +           } finally {
>> >>>>> +               if (in != null)in.close();
>> >>>>> +           }
>> >>>>> +       } else {
>> >>>>> +           return false;
>> >>>>> +       }
>> >>>>> +   }
>> >>>>> }
>> >>>>>
>> >>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>> >>>>> catalina/container/**CatalinaContainer.java
>> >>>>> URL:
>> >>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>> >>>>> catalina/src/org/ofbiz/**catalina/container/**
>> >>>>>
>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff
>> >
>> >>>>>
>> ==============================**==============================**==================
>> >>>>> ---
>> >>>>>
>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>> >>>>> (original) +++
>> >>>>>
>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>> >>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>> >>>>> @@ import org.ofbiz.base.concurrent.**Executi
>> >>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>> >>>>> import org.ofbiz.base.container.**Container;
>> >>>>> import org.ofbiz.base.container.**ContainerConfig;
>> >>>>> -import org.ofbiz.base.container.**ContainerException;
>> >>>>> import
>> org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>> >>>>> +import org.ofbiz.base.container.**ContainerException;
>> >>>>> import org.ofbiz.base.util.Debug;
>> >>>>> +import org.ofbiz.base.util.FileUtil;
>> >>>>> import org.ofbiz.base.util.SSLUtil;
>> >>>>> import org.ofbiz.base.util.UtilURL;
>> >>>>> import org.ofbiz.base.util.**UtilValidate;
>> >>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>> >>>>>           } catch (Exception e) {
>> >>>>>               Debug.logError(e, "Couldn't create connector.",
>> module);
>> >>>>>           }
>> >>>>> -
>> >>>>> +
>> >>>>>           try {
>> >>>>>               for (ContainerConfig.Container.**Property prop:
>> >>>>> connectorProp.properties.**values()) {
>> >>>>>                   connector.setProperty(prop.**name <
>> http://prop.name>,
>> >>>>> prop.value);
>> >>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>> >>>>>               engine.addChild(host);
>> >>>>>           }
>> >>>>>       }
>> >>>>> -
>> >>>>> +
>> >>>>>       if (host instanceof StandardHost) {
>> >>>>>           // set the catalina's work directory to the host
>> >>>>>           StandardHost standardHost = (StandardHost) host;
>> >>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>> >>>>>           mount = mount.substring(0, mount.length() - 2);
>> >>>>>       }
>> >>>>>
>> >>>>> +
>> >>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>> >>>>> location)
>> >>>>> +            .append(File.separatorChar).**append("WEB-INF")
>> >>>>> +
>>  .append(File.separatorChar).**append("web.xml").toString();
>> >>>>> +        boolean appIsDistributable = false;
>> >>>>> +        try {
>> >>>>> +            appIsDistributable =
>> FileUtil.containsString(**webXmlFilePath,
>> >>>>> "<distributable/>");
>> >>>>> +        } catch (IOException e) {
>> >>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>> >>>>> [%s].", webXmlFilePath), module);
>> >>>>> +            appIsDistributable = false;
>> >>>>> +        }
>> >>>>> +        final boolean contextIsDistributable = distribute &&
>> >>>>> appIsDistributable;
>> >>>>> +
>> >>>>>       // configure persistent sessions
>> >>>>>       Property clusterProp = clusterConfig.get(engine.**getName());
>> >>>>>
>> >>>>>       Manager sessionMgr = null;
>> >>>>> -        if (clusterProp != null) {
>> >>>>> +        if (clusterProp != null && contextIsDistributable) {
>> >>>>>           String mgrClassName =
>> ContainerConfig.**getPropertyValue(clusterProp,
>> >>>>> "manager-class",
>> >>>>>           "org.apache.catalina.ha.**session.DeltaManager"); try {
>> >>>>>               sessionMgr = (Manager)Class.forName(**
>> >>>>> mgrClassName).newInstance();
>> >>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>> >>>>>       context.setDocBase(location);
>> >>>>>       context.setPath(mount);
>> >>>>>       context.addLifecycleListener(**new ContextConfig());
>> >>>>> -
>> >>>>> +
>> >>>>>       JarScanner jarScanner = context.getJarScanner();
>> >>>>>       if (jarScanner instanceof StandardJarScanner) {
>> >>>>>           StandardJarScanner standardJarScanner =
>> (StandardJarScanner)
>> >>>>> jarScanner;
>> >>>>>           standardJarScanner.**setScanClassPath(false);
>> >>>>>       }
>> >>>>> -
>> >>>>> +
>> >>>>>       Engine egn = (Engine) context.getParent().getParent(**);
>> >>>>>       egn.setService(tomcat.**getService());
>> >>>>> -
>> >>>>> +
>> >>>>>       Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>> >>>>> module);
>> >>>>>       //context.setDeployOnStartup(**false);
>> >>>>>       //context.**setBackgroundProcessorDelay(5)**;
>> >>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>> >>>>>       context.setAllowLinking(true);
>> >>>>>
>> >>>>>       context.setReloadable(**contextReloadable);
>> >>>>> -        context.setDistributable(**distribute);
>> >>>>> +
>> >>>>> +        context.setDistributable(**contextIsDistributable);
>> >>>>> +
>> >>>>>       context.setCrossContext(**crossContext);
>> >>>>>       context.setPrivileged(appInfo.**privileged);
>> >>>>>       context.setManager(sessionMgr)**;
>> >>>>>
>> >>>>
>>
>>
> 

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Pierre Smits <pi...@gmail.com>.
Looking for 'distributable' I found no reference in any web.xml. However I
found in  *-containers.xml the reference:

<property name="apps-distributable" value="false"/><!-- you must also set
all the webapps you want distributable, by adding <distributable/> in their
web.xml file -->

If I understand it correctly than this overrides any <distributable/> or
<distributable /> in web.xml files. So it could be safe to have it in any
web.xml.

Shall I provide a patch for the code used by the  <ant create-component>
process to establish new components in the hot-deploy folder?

Regards,

Pierre

2012/4/30 Scott Gray <sc...@hotwaxmedia.com>

> This is not a good implementation, simply searching the web.xml file for a
> string containing "<distributable/>" is not good enough.  It'll find the
> tag even if commented out and and won't find the tag "<distributable />"
> (space before closing, perfectly valid).  I'm surprised this got past you
> Jacques.
>
> Regards
> Scott
>
> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>
> > Pierre,
> >
> > I did not test, but I believe it's only used in a clustered environment
> and should have any impacts in other cases.
> > It makes only a webapp distributable. Before we added this, all were
> distributable by default, which could annoying in certain circumstances.
> > So adding it in web.xml files should not changes from previous
> behaviour. Which makes me believe it's safe... (see commit for more)
> >
> > Jacque
> >
> > From: "Pierre Smits" <pi...@gmail.com>
> >> Jacques,
> >>
> >> If I have this tag in the web.xml, but the change is tested in an
> >> unclustered environment what do you thing the result would be? Is it of
> no
> >> effect, would it fail?
> >>
> >> Regards,
> >>
> >> Pierre
> >>
> >> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
> >>
> >>> Hi Sam,
> >>>
> >>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
> >>> html#Cluster_Basics<
> http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
> >>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<
> http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
> >>>
> >>> Also consider
> >>> <property name="apps-cross-context" value="true"/>
> >>> <property name="apps-context-reloadable" value="true"/>
> >>> (false by default)
> >>>
> >>> If you use sticky sessions all above is not an issue...
> >>>
> >>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
> >>> provide one ;o)
> >>>
> >>> Jacques
> >>>
> >>>
> >>> Sam Hamilton wrote:
> >>>
> >>>> Hi Jacques,
> >>>>
> >>>> Quick question - does this setting mean that now if you uncomment
> >>>> framework/config/ofbiz-**containers.xml cluster settings it wont
> >>>> cluster and that you should also add a <distributable/> tag to all the
> >>>> web.xml files?
> >>>>
> >>>> Thanks
> >>>> Sam
> >>>>
> >>>>
> >>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
> >>>>
> >>>> Author: jleroux
> >>>>> Date: Sat Apr 14 07:30:30 2012
> >>>>> New Revision: 1326064
> >>>>>
> >>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<
> http://svn.apache.org/viewvc?rev=1326064&view=rev>
> >>>>> Log:
> >>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
> >>>>> <distributable/> Node for web.xml files."
> >>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<
> https://issues.apache.org/jira/browse/OFBIZ-4242>
> >>>>>
> >>>>> Per the servlet specification, the web.xml file should allow an
> optional
> >>>>> <distributable/> node to indicate that the application
> >>>>> is clusterable. Currently, OFBiz does not respect this setting and
> >>>>> assumes all applications should be distributed if any cluster
> >>>>> configuration is provided in the ofbiz-containers.xml file. As a
> result,
> >>>>> if, for example, the DeltaManager is enable, all
> >>>>> applications attempt to cluster and communicate via DeltaManager.
> >>>>>
> >>>>> The expected and proper functionality is to check the individual
> >>>>> application's web.xml file for the <distributable/> node and
> >>>>> only use the DeltaManager if found; otherwise the StandardManager
> should
> >>>>> be used for sessions.
> >>>>>
> >>>>> jleroux: replaced some tabs, reformatted, added a comment about
> >>>>> <distributable/> in the *-containers.file
> >>>>>
> >>>>> Modified:
> >>>>>  ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
> >>>>>
>  ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
> >>>>> CatalinaContainer.java
> >>>>>
> >>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
> >>>>> FileUtil.java
> >>>>> URL:
> >>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
> >>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
> >>>>> 1326063&r2=1326064&view=diff<
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff
> >
> >>>>>
> ==============================**==============================**==================
> >>>>> ---
> >>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
> >>>>> (original) +++
> >>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
> >>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
> >>>>> java.io.FileWriter;
> >>>>> import java.io.FilenameFilter;
> >>>>> import java.io.IOException;
> >>>>> import java.io.OutputStream;
> >>>>> +import java.io.Reader;
> >>>>> import java.io.Writer;
> >>>>> import java.net.**MalformedURLException;
> >>>>> import java.util.List;
> >>>>> @@ -67,7 +68,7 @@ public class FileUtil {
> >>>>>
> >>>>>   /**
> >>>>>    * Converts a file path to one that is compatible with the host
> >>>>> operating system.
> >>>>> -     *
> >>>>> +     *
> >>>>>    * @param path The file path to convert.
> >>>>>    * @return The converted file path.
> >>>>>    */
> >>>>> @@ -341,4 +342,57 @@ public class FileUtil {
> >>>>>           return false;
> >>>>>       }
> >>>>>   }
> >>>>> +
> >>>>> +    /**
> >>>>> +    *
> >>>>> +    *
> >>>>> +    * Search for the specified <code>searchString</code> in the
> given
> >>>>> +    * {@link Reader}.
> >>>>> +    *
> >>>>> +    * @param reader A Reader in which the String will be searched.
> >>>>> +    * @param searchString The String to search for
> >>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
> >>>>> found;
> >>>>> +    *         <code>FALSE</code> otherwise.
> >>>>> +    * @throws IOException
> >>>>> +    */
> >>>>> +   public static boolean containsString(Reader reader, final String
> >>>>> searchString) throws IOException {
> >>>>> +       char[] buffer = new char[1024];
> >>>>> +       int numCharsRead;
> >>>>> +       int count = 0;
> >>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
> >>>>> +           for (int c = 0; c < numCharsRead; ++c) {
> >>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
> >>>>> +               else count = 0;
> >>>>> +               if (count == searchString.length()) return true;
> >>>>> +           }
> >>>>> +       }
> >>>>> +       return false;
> >>>>> +   }
> >>>>> +
> >>>>> +   /**
> >>>>> +    *
> >>>>> +    *
> >>>>> +    * Search for the specified <code>searchString</code> in the
> given
> >>>>> +    * filename. If the specified file doesn't exist,
> <code>FALSE</code>
> >>>>> +    * returns.
> >>>>> +    *
> >>>>> +    * @param fileName A full path to a file in which the String
> will be
> >>>>> searched.
> >>>>> +    * @param searchString The String to search for
> >>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
> >>>>> found;
> >>>>> +    *         <code>FALSE</code> otherwise.
> >>>>> +    * @throws IOException
> >>>>> +    */
> >>>>> +   public static boolean containsString(final String fileName, final
> >>>>> String searchString) throws IOException {
> >>>>> +       File inFile = new File(fileName);
> >>>>> +       if (inFile.exists()) {
> >>>>> +           BufferedReader in = new BufferedReader(new
> >>>>> FileReader(inFile));
> >>>>> +           try {
> >>>>> +               return containsString(in, searchString);
> >>>>> +           } finally {
> >>>>> +               if (in != null)in.close();
> >>>>> +           }
> >>>>> +       } else {
> >>>>> +           return false;
> >>>>> +       }
> >>>>> +   }
> >>>>> }
> >>>>>
> >>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
> >>>>> catalina/container/**CatalinaContainer.java
> >>>>> URL:
> >>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
> >>>>> catalina/src/org/ofbiz/**catalina/container/**
> >>>>>
> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff
> >
> >>>>>
> ==============================**==============================**==================
> >>>>> ---
> >>>>>
> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
> >>>>> (original) +++
> >>>>>
> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
> >>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
> >>>>> @@ import org.ofbiz.base.concurrent.**Executi
> >>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
> >>>>> import org.ofbiz.base.container.**Container;
> >>>>> import org.ofbiz.base.container.**ContainerConfig;
> >>>>> -import org.ofbiz.base.container.**ContainerException;
> >>>>> import
> org.ofbiz.base.container.**ContainerConfig.Container.**Property;
> >>>>> +import org.ofbiz.base.container.**ContainerException;
> >>>>> import org.ofbiz.base.util.Debug;
> >>>>> +import org.ofbiz.base.util.FileUtil;
> >>>>> import org.ofbiz.base.util.SSLUtil;
> >>>>> import org.ofbiz.base.util.UtilURL;
> >>>>> import org.ofbiz.base.util.**UtilValidate;
> >>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
> >>>>>           } catch (Exception e) {
> >>>>>               Debug.logError(e, "Couldn't create connector.",
> module);
> >>>>>           }
> >>>>> -
> >>>>> +
> >>>>>           try {
> >>>>>               for (ContainerConfig.Container.**Property prop:
> >>>>> connectorProp.properties.**values()) {
> >>>>>                   connector.setProperty(prop.**name <
> http://prop.name>,
> >>>>> prop.value);
> >>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
> >>>>>               engine.addChild(host);
> >>>>>           }
> >>>>>       }
> >>>>> -
> >>>>> +
> >>>>>       if (host instanceof StandardHost) {
> >>>>>           // set the catalina's work directory to the host
> >>>>>           StandardHost standardHost = (StandardHost) host;
> >>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
> >>>>>           mount = mount.substring(0, mount.length() - 2);
> >>>>>       }
> >>>>>
> >>>>> +
> >>>>> +        final String webXmlFilePath = new StringBuilder().append(**
> >>>>> location)
> >>>>> +            .append(File.separatorChar).**append("WEB-INF")
> >>>>> +
>  .append(File.separatorChar).**append("web.xml").toString();
> >>>>> +        boolean appIsDistributable = false;
> >>>>> +        try {
> >>>>> +            appIsDistributable =
> FileUtil.containsString(**webXmlFilePath,
> >>>>> "<distributable/>");
> >>>>> +        } catch (IOException e) {
> >>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
> >>>>> [%s].", webXmlFilePath), module);
> >>>>> +            appIsDistributable = false;
> >>>>> +        }
> >>>>> +        final boolean contextIsDistributable = distribute &&
> >>>>> appIsDistributable;
> >>>>> +
> >>>>>       // configure persistent sessions
> >>>>>       Property clusterProp = clusterConfig.get(engine.**getName());
> >>>>>
> >>>>>       Manager sessionMgr = null;
> >>>>> -        if (clusterProp != null) {
> >>>>> +        if (clusterProp != null && contextIsDistributable) {
> >>>>>           String mgrClassName =
> ContainerConfig.**getPropertyValue(clusterProp,
> >>>>> "manager-class",
> >>>>>           "org.apache.catalina.ha.**session.DeltaManager"); try {
> >>>>>               sessionMgr = (Manager)Class.forName(**
> >>>>> mgrClassName).newInstance();
> >>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
> >>>>>       context.setDocBase(location);
> >>>>>       context.setPath(mount);
> >>>>>       context.addLifecycleListener(**new ContextConfig());
> >>>>> -
> >>>>> +
> >>>>>       JarScanner jarScanner = context.getJarScanner();
> >>>>>       if (jarScanner instanceof StandardJarScanner) {
> >>>>>           StandardJarScanner standardJarScanner =
> (StandardJarScanner)
> >>>>> jarScanner;
> >>>>>           standardJarScanner.**setScanClassPath(false);
> >>>>>       }
> >>>>> -
> >>>>> +
> >>>>>       Engine egn = (Engine) context.getParent().getParent(**);
> >>>>>       egn.setService(tomcat.**getService());
> >>>>> -
> >>>>> +
> >>>>>       Debug.logInfo("host[" + host + "].addChild(" + context + ")",
> >>>>> module);
> >>>>>       //context.setDeployOnStartup(**false);
> >>>>>       //context.**setBackgroundProcessorDelay(5)**;
> >>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
> >>>>>       context.setAllowLinking(true);
> >>>>>
> >>>>>       context.setReloadable(**contextReloadable);
> >>>>> -        context.setDistributable(**distribute);
> >>>>> +
> >>>>> +        context.setDistributable(**contextIsDistributable);
> >>>>> +
> >>>>>       context.setCrossContext(**crossContext);
> >>>>>       context.setPrivileged(appInfo.**privileged);
> >>>>>       context.setManager(sessionMgr)**;
> >>>>>
> >>>>
>
>

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Done at r1334336, backported in R12.04

Jacques

From: "Jacques Le Roux" <ja...@les7arts.com>
> From: "Scott Gray" <sc...@hotwaxmedia.com>
>> That's an understatement and could actually be said of every patch in jira, shall we just go ahead and commit them all?  We can 
>> improve them later right?
>>
>> Reading an xml file character by character to find something is like driving in a nail with the claw side of a hammer.  It sort 
>> of works, but I wouldn't put the end result in my display cabinet.  Did you not review this before committing or is treating an 
>> xml file like plain text an acceptable approach to you?
>
> I thought it was enough for that case since we don't need to read any values. So I simply put a comment in ofbiz-containier. I 
> will improve.
>
> Jacques
>
>> Thanks
>> Scott
>>
>> On 1/05/2012, at 5:52 AM, Jacques Le Roux wrote:
>>
>>> You are right Scott, this can be improved...
>>>
>>> Jacques
>>>
>>> From: "Scott Gray" <sc...@hotwaxmedia.com>
>>>> This is not a good implementation, simply searching the web.xml file for a string containing "<distributable/>" is not good
>>>> enough.  It'll find the tag even if commented out and and won't find the tag "<distributable />" (space before closing, 
>>>> perfectly
>>>> valid).  I'm surprised this got past you Jacques.
>>>>
>>>> Regards
>>>> Scott
>>>>
>>>> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>>>>
>>>>> Pierre,
>>>>>
>>>>> I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
>>>>> It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain
>>>>> circumstances.
>>>>> So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for
>>>>> more)
>>>>>
>>>>> Jacque
>>>>>
>>>>> From: "Pierre Smits" <pi...@gmail.com>
>>>>>> Jacques,
>>>>>>
>>>>>> If I have this tag in the web.xml, but the change is tested in an
>>>>>> unclustered environment what do you thing the result would be? Is it of no
>>>>>> effect, would it fail?
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Pierre
>>>>>>
>>>>>> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>>>>>>
>>>>>>> Hi Sam,
>>>>>>>
>>>>>>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>>>>>>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>>>>>>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>>>>>>
>>>>>>> Also consider
>>>>>>> <property name="apps-cross-context" value="true"/>
>>>>>>> <property name="apps-context-reloadable" value="true"/>
>>>>>>> (false by default)
>>>>>>>
>>>>>>> If you use sticky sessions all above is not an issue...
>>>>>>>
>>>>>>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>>>>>>> provide one ;o)
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>> Sam Hamilton wrote:
>>>>>>>
>>>>>>>> Hi Jacques,
>>>>>>>>
>>>>>>>> Quick question - does this setting mean that now if you uncomment
>>>>>>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>>>>>>> cluster and that you should also add a <distributable/> tag to all the
>>>>>>>> web.xml files?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Sam
>>>>>>>>
>>>>>>>>
>>>>>>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>>>>>>
>>>>>>>> Author: jleroux
>>>>>>>>> Date: Sat Apr 14 07:30:30 2012
>>>>>>>>> New Revision: 1326064
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>>>>>>> Log:
>>>>>>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>>>>>>> <distributable/> Node for web.xml files."
>>>>>>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>>>>>>
>>>>>>>>> Per the servlet specification, the web.xml file should allow an optional
>>>>>>>>> <distributable/> node to indicate that the application
>>>>>>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>>>>>>> assumes all applications should be distributed if any cluster
>>>>>>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>>>>>>> if, for example, the DeltaManager is enable, all
>>>>>>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>>>>>>
>>>>>>>>> The expected and proper functionality is to check the individual
>>>>>>>>> application's web.xml file for the <distributable/> node and
>>>>>>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>>>>>>> be used for sessions.
>>>>>>>>>
>>>>>>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>>>>>>> <distributable/> in the *-containers.file
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>>>>>>> CatalinaContainer.java
>>>>>>>>>
>>>>>>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>>>>>>> FileUtil.java
>>>>>>>>> URL:
>>>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>>>>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>>>>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>>>> ==============================**==============================**==================
>>>>>>>>> ---
>>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>>> (original) +++
>>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>>>>>>> java.io.FileWriter;
>>>>>>>>> import java.io.FilenameFilter;
>>>>>>>>> import java.io.IOException;
>>>>>>>>> import java.io.OutputStream;
>>>>>>>>> +import java.io.Reader;
>>>>>>>>> import java.io.Writer;
>>>>>>>>> import java.net.**MalformedURLException;
>>>>>>>>> import java.util.List;
>>>>>>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>>>>>>
>>>>>>>>>  /**
>>>>>>>>>   * Converts a file path to one that is compatible with the host
>>>>>>>>> operating system.
>>>>>>>>> -     *
>>>>>>>>> +     *
>>>>>>>>>   * @param path The file path to convert.
>>>>>>>>>   * @return The converted file path.
>>>>>>>>>   */
>>>>>>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>>>>>>          return false;
>>>>>>>>>      }
>>>>>>>>>  }
>>>>>>>>> +
>>>>>>>>> +    /**
>>>>>>>>> +    *
>>>>>>>>> +    *
>>>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>>>> +    * {@link Reader}.
>>>>>>>>> +    *
>>>>>>>>> +    * @param reader A Reader in which the String will be searched.
>>>>>>>>> +    * @param searchString The String to search for
>>>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>>>> found;
>>>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>>>> +    * @throws IOException
>>>>>>>>> +    */
>>>>>>>>> +   public static boolean containsString(Reader reader, final String
>>>>>>>>> searchString) throws IOException {
>>>>>>>>> +       char[] buffer = new char[1024];
>>>>>>>>> +       int numCharsRead;
>>>>>>>>> +       int count = 0;
>>>>>>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>>>>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>>>>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>>>>>>> +               else count = 0;
>>>>>>>>> +               if (count == searchString.length()) return true;
>>>>>>>>> +           }
>>>>>>>>> +       }
>>>>>>>>> +       return false;
>>>>>>>>> +   }
>>>>>>>>> +
>>>>>>>>> +   /**
>>>>>>>>> +    *
>>>>>>>>> +    *
>>>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>>>>>>> +    * returns.
>>>>>>>>> +    *
>>>>>>>>> +    * @param fileName A full path to a file in which the String will be
>>>>>>>>> searched.
>>>>>>>>> +    * @param searchString The String to search for
>>>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>>>> found;
>>>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>>>> +    * @throws IOException
>>>>>>>>> +    */
>>>>>>>>> +   public static boolean containsString(final String fileName, final
>>>>>>>>> String searchString) throws IOException {
>>>>>>>>> +       File inFile = new File(fileName);
>>>>>>>>> +       if (inFile.exists()) {
>>>>>>>>> +           BufferedReader in = new BufferedReader(new
>>>>>>>>> FileReader(inFile));
>>>>>>>>> +           try {
>>>>>>>>> +               return containsString(in, searchString);
>>>>>>>>> +           } finally {
>>>>>>>>> +               if (in != null)in.close();
>>>>>>>>> +           }
>>>>>>>>> +       } else {
>>>>>>>>> +           return false;
>>>>>>>>> +       }
>>>>>>>>> +   }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>>>>>>> catalina/container/**CatalinaContainer.java
>>>>>>>>> URL:
>>>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>>>>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>>>>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>>>> ==============================**==============================**==================
>>>>>>>>> ---
>>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>>>> (original) +++
>>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>>>>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>>>>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>>>>>>> import org.ofbiz.base.container.**Container;
>>>>>>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>>>>>>> -import org.ofbiz.base.container.**ContainerException;
>>>>>>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>>>>>>> +import org.ofbiz.base.container.**ContainerException;
>>>>>>>>> import org.ofbiz.base.util.Debug;
>>>>>>>>> +import org.ofbiz.base.util.FileUtil;
>>>>>>>>> import org.ofbiz.base.util.SSLUtil;
>>>>>>>>> import org.ofbiz.base.util.UtilURL;
>>>>>>>>> import org.ofbiz.base.util.**UtilValidate;
>>>>>>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>>>>>>          } catch (Exception e) {
>>>>>>>>>              Debug.logError(e, "Couldn't create connector.", module);
>>>>>>>>>          }
>>>>>>>>> -
>>>>>>>>> +
>>>>>>>>>          try {
>>>>>>>>>              for (ContainerConfig.Container.**Property prop:
>>>>>>>>> connectorProp.properties.**values()) {
>>>>>>>>>                  connector.setProperty(prop.**name <http://prop.name>,
>>>>>>>>> prop.value);
>>>>>>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>>>>>>              engine.addChild(host);
>>>>>>>>>          }
>>>>>>>>>      }
>>>>>>>>> -
>>>>>>>>> +
>>>>>>>>>      if (host instanceof StandardHost) {
>>>>>>>>>          // set the catalina's work directory to the host
>>>>>>>>>          StandardHost standardHost = (StandardHost) host;
>>>>>>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>>>>>>          mount = mount.substring(0, mount.length() - 2);
>>>>>>>>>      }
>>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>>>>>>> location)
>>>>>>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>>>>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>>>>>>> +        boolean appIsDistributable = false;
>>>>>>>>> +        try {
>>>>>>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>>>>>>> "<distributable/>");
>>>>>>>>> +        } catch (IOException e) {
>>>>>>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>>>>>>> [%s].", webXmlFilePath), module);
>>>>>>>>> +            appIsDistributable = false;
>>>>>>>>> +        }
>>>>>>>>> +        final boolean contextIsDistributable = distribute &&
>>>>>>>>> appIsDistributable;
>>>>>>>>> +
>>>>>>>>>      // configure persistent sessions
>>>>>>>>>      Property clusterProp = clusterConfig.get(engine.**getName());
>>>>>>>>>
>>>>>>>>>      Manager sessionMgr = null;
>>>>>>>>> -        if (clusterProp != null) {
>>>>>>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>>>>>>          String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>>>>>>> "manager-class",
>>>>>>>>>          "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>>>>>>              sessionMgr = (Manager)Class.forName(**
>>>>>>>>> mgrClassName).newInstance();
>>>>>>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>>>>>>      context.setDocBase(location);
>>>>>>>>>      context.setPath(mount);
>>>>>>>>>      context.addLifecycleListener(**new ContextConfig());
>>>>>>>>> -
>>>>>>>>> +
>>>>>>>>>      JarScanner jarScanner = context.getJarScanner();
>>>>>>>>>      if (jarScanner instanceof StandardJarScanner) {
>>>>>>>>>          StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>>>>>>> jarScanner;
>>>>>>>>>          standardJarScanner.**setScanClassPath(false);
>>>>>>>>>      }
>>>>>>>>> -
>>>>>>>>> +
>>>>>>>>>      Engine egn = (Engine) context.getParent().getParent(**);
>>>>>>>>>      egn.setService(tomcat.**getService());
>>>>>>>>> -
>>>>>>>>> +
>>>>>>>>>      Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>>>>>>> module);
>>>>>>>>>      //context.setDeployOnStartup(**false);
>>>>>>>>>      //context.**setBackgroundProcessorDelay(5)**;
>>>>>>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>>>>>>      context.setAllowLinking(true);
>>>>>>>>>
>>>>>>>>>      context.setReloadable(**contextReloadable);
>>>>>>>>> -        context.setDistributable(**distribute);
>>>>>>>>> +
>>>>>>>>> +        context.setDistributable(**contextIsDistributable);
>>>>>>>>> +
>>>>>>>>>      context.setCrossContext(**crossContext);
>>>>>>>>>      context.setPrivileged(appInfo.**privileged);
>>>>>>>>>      context.setManager(sessionMgr)**;
>>>>>>>>>
>>>>>>>>
>>>>
>>>>
>>
>> 

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Scott Gray" <sc...@hotwaxmedia.com>
> That's an understatement and could actually be said of every patch in jira, shall we just go ahead and commit them all?  We can 
> improve them later right?
>
> Reading an xml file character by character to find something is like driving in a nail with the claw side of a hammer.  It sort of 
> works, but I wouldn't put the end result in my display cabinet.  Did you not review this before committing or is treating an xml 
> file like plain text an acceptable approach to you?

I thought it was enough for that case since we don't need to read any values. So I simply put a comment in ofbiz-containier. I will 
improve.

Jacques

> Thanks
> Scott
>
> On 1/05/2012, at 5:52 AM, Jacques Le Roux wrote:
>
>> You are right Scott, this can be improved...
>>
>> Jacques
>>
>> From: "Scott Gray" <sc...@hotwaxmedia.com>
>>> This is not a good implementation, simply searching the web.xml file for a string containing "<distributable/>" is not good
>>> enough.  It'll find the tag even if commented out and and won't find the tag "<distributable />" (space before closing, 
>>> perfectly
>>> valid).  I'm surprised this got past you Jacques.
>>>
>>> Regards
>>> Scott
>>>
>>> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>>>
>>>> Pierre,
>>>>
>>>> I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
>>>> It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain
>>>> circumstances.
>>>> So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for
>>>> more)
>>>>
>>>> Jacque
>>>>
>>>> From: "Pierre Smits" <pi...@gmail.com>
>>>>> Jacques,
>>>>>
>>>>> If I have this tag in the web.xml, but the change is tested in an
>>>>> unclustered environment what do you thing the result would be? Is it of no
>>>>> effect, would it fail?
>>>>>
>>>>> Regards,
>>>>>
>>>>> Pierre
>>>>>
>>>>> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>>>>>
>>>>>> Hi Sam,
>>>>>>
>>>>>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>>>>>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>>>>>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>>>>>
>>>>>> Also consider
>>>>>> <property name="apps-cross-context" value="true"/>
>>>>>> <property name="apps-context-reloadable" value="true"/>
>>>>>> (false by default)
>>>>>>
>>>>>> If you use sticky sessions all above is not an issue...
>>>>>>
>>>>>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>>>>>> provide one ;o)
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>> Sam Hamilton wrote:
>>>>>>
>>>>>>> Hi Jacques,
>>>>>>>
>>>>>>> Quick question - does this setting mean that now if you uncomment
>>>>>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>>>>>> cluster and that you should also add a <distributable/> tag to all the
>>>>>>> web.xml files?
>>>>>>>
>>>>>>> Thanks
>>>>>>> Sam
>>>>>>>
>>>>>>>
>>>>>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>>>>>
>>>>>>> Author: jleroux
>>>>>>>> Date: Sat Apr 14 07:30:30 2012
>>>>>>>> New Revision: 1326064
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>>>>>> Log:
>>>>>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>>>>>> <distributable/> Node for web.xml files."
>>>>>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>>>>>
>>>>>>>> Per the servlet specification, the web.xml file should allow an optional
>>>>>>>> <distributable/> node to indicate that the application
>>>>>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>>>>>> assumes all applications should be distributed if any cluster
>>>>>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>>>>>> if, for example, the DeltaManager is enable, all
>>>>>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>>>>>
>>>>>>>> The expected and proper functionality is to check the individual
>>>>>>>> application's web.xml file for the <distributable/> node and
>>>>>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>>>>>> be used for sessions.
>>>>>>>>
>>>>>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>>>>>> <distributable/> in the *-containers.file
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>>>>>> CatalinaContainer.java
>>>>>>>>
>>>>>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>>>>>> FileUtil.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>>>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>>>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>>> ==============================**==============================**==================
>>>>>>>> ---
>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>> (original) +++
>>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>>>>>> java.io.FileWriter;
>>>>>>>> import java.io.FilenameFilter;
>>>>>>>> import java.io.IOException;
>>>>>>>> import java.io.OutputStream;
>>>>>>>> +import java.io.Reader;
>>>>>>>> import java.io.Writer;
>>>>>>>> import java.net.**MalformedURLException;
>>>>>>>> import java.util.List;
>>>>>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>>>>>
>>>>>>>>  /**
>>>>>>>>   * Converts a file path to one that is compatible with the host
>>>>>>>> operating system.
>>>>>>>> -     *
>>>>>>>> +     *
>>>>>>>>   * @param path The file path to convert.
>>>>>>>>   * @return The converted file path.
>>>>>>>>   */
>>>>>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>>>>>          return false;
>>>>>>>>      }
>>>>>>>>  }
>>>>>>>> +
>>>>>>>> +    /**
>>>>>>>> +    *
>>>>>>>> +    *
>>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>>> +    * {@link Reader}.
>>>>>>>> +    *
>>>>>>>> +    * @param reader A Reader in which the String will be searched.
>>>>>>>> +    * @param searchString The String to search for
>>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>>> found;
>>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>>> +    * @throws IOException
>>>>>>>> +    */
>>>>>>>> +   public static boolean containsString(Reader reader, final String
>>>>>>>> searchString) throws IOException {
>>>>>>>> +       char[] buffer = new char[1024];
>>>>>>>> +       int numCharsRead;
>>>>>>>> +       int count = 0;
>>>>>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>>>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>>>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>>>>>> +               else count = 0;
>>>>>>>> +               if (count == searchString.length()) return true;
>>>>>>>> +           }
>>>>>>>> +       }
>>>>>>>> +       return false;
>>>>>>>> +   }
>>>>>>>> +
>>>>>>>> +   /**
>>>>>>>> +    *
>>>>>>>> +    *
>>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>>>>>> +    * returns.
>>>>>>>> +    *
>>>>>>>> +    * @param fileName A full path to a file in which the String will be
>>>>>>>> searched.
>>>>>>>> +    * @param searchString The String to search for
>>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>>> found;
>>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>>> +    * @throws IOException
>>>>>>>> +    */
>>>>>>>> +   public static boolean containsString(final String fileName, final
>>>>>>>> String searchString) throws IOException {
>>>>>>>> +       File inFile = new File(fileName);
>>>>>>>> +       if (inFile.exists()) {
>>>>>>>> +           BufferedReader in = new BufferedReader(new
>>>>>>>> FileReader(inFile));
>>>>>>>> +           try {
>>>>>>>> +               return containsString(in, searchString);
>>>>>>>> +           } finally {
>>>>>>>> +               if (in != null)in.close();
>>>>>>>> +           }
>>>>>>>> +       } else {
>>>>>>>> +           return false;
>>>>>>>> +       }
>>>>>>>> +   }
>>>>>>>> }
>>>>>>>>
>>>>>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>>>>>> catalina/container/**CatalinaContainer.java
>>>>>>>> URL:
>>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>>>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>>>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>>> ==============================**==============================**==================
>>>>>>>> ---
>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>>> (original) +++
>>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>>>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>>>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>>>>>> import org.ofbiz.base.container.**Container;
>>>>>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>>>>>> -import org.ofbiz.base.container.**ContainerException;
>>>>>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>>>>>> +import org.ofbiz.base.container.**ContainerException;
>>>>>>>> import org.ofbiz.base.util.Debug;
>>>>>>>> +import org.ofbiz.base.util.FileUtil;
>>>>>>>> import org.ofbiz.base.util.SSLUtil;
>>>>>>>> import org.ofbiz.base.util.UtilURL;
>>>>>>>> import org.ofbiz.base.util.**UtilValidate;
>>>>>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>>>>>          } catch (Exception e) {
>>>>>>>>              Debug.logError(e, "Couldn't create connector.", module);
>>>>>>>>          }
>>>>>>>> -
>>>>>>>> +
>>>>>>>>          try {
>>>>>>>>              for (ContainerConfig.Container.**Property prop:
>>>>>>>> connectorProp.properties.**values()) {
>>>>>>>>                  connector.setProperty(prop.**name <http://prop.name>,
>>>>>>>> prop.value);
>>>>>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>>>>>              engine.addChild(host);
>>>>>>>>          }
>>>>>>>>      }
>>>>>>>> -
>>>>>>>> +
>>>>>>>>      if (host instanceof StandardHost) {
>>>>>>>>          // set the catalina's work directory to the host
>>>>>>>>          StandardHost standardHost = (StandardHost) host;
>>>>>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>>>>>          mount = mount.substring(0, mount.length() - 2);
>>>>>>>>      }
>>>>>>>>
>>>>>>>> +
>>>>>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>>>>>> location)
>>>>>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>>>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>>>>>> +        boolean appIsDistributable = false;
>>>>>>>> +        try {
>>>>>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>>>>>> "<distributable/>");
>>>>>>>> +        } catch (IOException e) {
>>>>>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>>>>>> [%s].", webXmlFilePath), module);
>>>>>>>> +            appIsDistributable = false;
>>>>>>>> +        }
>>>>>>>> +        final boolean contextIsDistributable = distribute &&
>>>>>>>> appIsDistributable;
>>>>>>>> +
>>>>>>>>      // configure persistent sessions
>>>>>>>>      Property clusterProp = clusterConfig.get(engine.**getName());
>>>>>>>>
>>>>>>>>      Manager sessionMgr = null;
>>>>>>>> -        if (clusterProp != null) {
>>>>>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>>>>>          String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>>>>>> "manager-class",
>>>>>>>>          "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>>>>>              sessionMgr = (Manager)Class.forName(**
>>>>>>>> mgrClassName).newInstance();
>>>>>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>>>>>      context.setDocBase(location);
>>>>>>>>      context.setPath(mount);
>>>>>>>>      context.addLifecycleListener(**new ContextConfig());
>>>>>>>> -
>>>>>>>> +
>>>>>>>>      JarScanner jarScanner = context.getJarScanner();
>>>>>>>>      if (jarScanner instanceof StandardJarScanner) {
>>>>>>>>          StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>>>>>> jarScanner;
>>>>>>>>          standardJarScanner.**setScanClassPath(false);
>>>>>>>>      }
>>>>>>>> -
>>>>>>>> +
>>>>>>>>      Engine egn = (Engine) context.getParent().getParent(**);
>>>>>>>>      egn.setService(tomcat.**getService());
>>>>>>>> -
>>>>>>>> +
>>>>>>>>      Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>>>>>> module);
>>>>>>>>      //context.setDeployOnStartup(**false);
>>>>>>>>      //context.**setBackgroundProcessorDelay(5)**;
>>>>>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>>>>>      context.setAllowLinking(true);
>>>>>>>>
>>>>>>>>      context.setReloadable(**contextReloadable);
>>>>>>>> -        context.setDistributable(**distribute);
>>>>>>>> +
>>>>>>>> +        context.setDistributable(**contextIsDistributable);
>>>>>>>> +
>>>>>>>>      context.setCrossContext(**crossContext);
>>>>>>>>      context.setPrivileged(appInfo.**privileged);
>>>>>>>>      context.setManager(sessionMgr)**;
>>>>>>>>
>>>>>>>
>>>
>>>
>
> 

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
That's an understatement and could actually be said of every patch in jira, shall we just go ahead and commit them all?  We can improve them later right? 

Reading an xml file character by character to find something is like driving in a nail with the claw side of a hammer.  It sort of works, but I wouldn't put the end result in my display cabinet.  Did you not review this before committing or is treating an xml file like plain text an acceptable approach to you?

Thanks
Scott

On 1/05/2012, at 5:52 AM, Jacques Le Roux wrote:

> You are right Scott, this can be improved...
> 
> Jacques
> 
> From: "Scott Gray" <sc...@hotwaxmedia.com>
>> This is not a good implementation, simply searching the web.xml file for a string containing "<distributable/>" is not good
>> enough.  It'll find the tag even if commented out and and won't find the tag "<distributable />" (space before closing, perfectly
>> valid).  I'm surprised this got past you Jacques.
>> 
>> Regards
>> Scott
>> 
>> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>> 
>>> Pierre,
>>> 
>>> I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
>>> It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain
>>> circumstances.
>>> So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for
>>> more)
>>> 
>>> Jacque
>>> 
>>> From: "Pierre Smits" <pi...@gmail.com>
>>>> Jacques,
>>>> 
>>>> If I have this tag in the web.xml, but the change is tested in an
>>>> unclustered environment what do you thing the result would be? Is it of no
>>>> effect, would it fail?
>>>> 
>>>> Regards,
>>>> 
>>>> Pierre
>>>> 
>>>> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>>>> 
>>>>> Hi Sam,
>>>>> 
>>>>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>>>>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>>>>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>>>> 
>>>>> Also consider
>>>>> <property name="apps-cross-context" value="true"/>
>>>>> <property name="apps-context-reloadable" value="true"/>
>>>>> (false by default)
>>>>> 
>>>>> If you use sticky sessions all above is not an issue...
>>>>> 
>>>>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>>>>> provide one ;o)
>>>>> 
>>>>> Jacques
>>>>> 
>>>>> 
>>>>> Sam Hamilton wrote:
>>>>> 
>>>>>> Hi Jacques,
>>>>>> 
>>>>>> Quick question - does this setting mean that now if you uncomment
>>>>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>>>>> cluster and that you should also add a <distributable/> tag to all the
>>>>>> web.xml files?
>>>>>> 
>>>>>> Thanks
>>>>>> Sam
>>>>>> 
>>>>>> 
>>>>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>>>> 
>>>>>> Author: jleroux
>>>>>>> Date: Sat Apr 14 07:30:30 2012
>>>>>>> New Revision: 1326064
>>>>>>> 
>>>>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>>>>> Log:
>>>>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>>>>> <distributable/> Node for web.xml files."
>>>>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>>>> 
>>>>>>> Per the servlet specification, the web.xml file should allow an optional
>>>>>>> <distributable/> node to indicate that the application
>>>>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>>>>> assumes all applications should be distributed if any cluster
>>>>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>>>>> if, for example, the DeltaManager is enable, all
>>>>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>>>> 
>>>>>>> The expected and proper functionality is to check the individual
>>>>>>> application's web.xml file for the <distributable/> node and
>>>>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>>>>> be used for sessions.
>>>>>>> 
>>>>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>>>>> <distributable/> in the *-containers.file
>>>>>>> 
>>>>>>> Modified:
>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>>>>> CatalinaContainer.java
>>>>>>> 
>>>>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>>>>> FileUtil.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>> ==============================**==============================**==================
>>>>>>> ---
>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>> (original) +++
>>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>>>>> java.io.FileWriter;
>>>>>>> import java.io.FilenameFilter;
>>>>>>> import java.io.IOException;
>>>>>>> import java.io.OutputStream;
>>>>>>> +import java.io.Reader;
>>>>>>> import java.io.Writer;
>>>>>>> import java.net.**MalformedURLException;
>>>>>>> import java.util.List;
>>>>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>>>> 
>>>>>>>  /**
>>>>>>>   * Converts a file path to one that is compatible with the host
>>>>>>> operating system.
>>>>>>> -     *
>>>>>>> +     *
>>>>>>>   * @param path The file path to convert.
>>>>>>>   * @return The converted file path.
>>>>>>>   */
>>>>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>>>>          return false;
>>>>>>>      }
>>>>>>>  }
>>>>>>> +
>>>>>>> +    /**
>>>>>>> +    *
>>>>>>> +    *
>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>> +    * {@link Reader}.
>>>>>>> +    *
>>>>>>> +    * @param reader A Reader in which the String will be searched.
>>>>>>> +    * @param searchString The String to search for
>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>> found;
>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>> +    * @throws IOException
>>>>>>> +    */
>>>>>>> +   public static boolean containsString(Reader reader, final String
>>>>>>> searchString) throws IOException {
>>>>>>> +       char[] buffer = new char[1024];
>>>>>>> +       int numCharsRead;
>>>>>>> +       int count = 0;
>>>>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>>>>> +               else count = 0;
>>>>>>> +               if (count == searchString.length()) return true;
>>>>>>> +           }
>>>>>>> +       }
>>>>>>> +       return false;
>>>>>>> +   }
>>>>>>> +
>>>>>>> +   /**
>>>>>>> +    *
>>>>>>> +    *
>>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>>>>> +    * returns.
>>>>>>> +    *
>>>>>>> +    * @param fileName A full path to a file in which the String will be
>>>>>>> searched.
>>>>>>> +    * @param searchString The String to search for
>>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>>> found;
>>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>>> +    * @throws IOException
>>>>>>> +    */
>>>>>>> +   public static boolean containsString(final String fileName, final
>>>>>>> String searchString) throws IOException {
>>>>>>> +       File inFile = new File(fileName);
>>>>>>> +       if (inFile.exists()) {
>>>>>>> +           BufferedReader in = new BufferedReader(new
>>>>>>> FileReader(inFile));
>>>>>>> +           try {
>>>>>>> +               return containsString(in, searchString);
>>>>>>> +           } finally {
>>>>>>> +               if (in != null)in.close();
>>>>>>> +           }
>>>>>>> +       } else {
>>>>>>> +           return false;
>>>>>>> +       }
>>>>>>> +   }
>>>>>>> }
>>>>>>> 
>>>>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>>>>> catalina/container/**CatalinaContainer.java
>>>>>>> URL:
>>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>>> ==============================**==============================**==================
>>>>>>> ---
>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>> (original) +++
>>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>>>>> import org.ofbiz.base.container.**Container;
>>>>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>>>>> -import org.ofbiz.base.container.**ContainerException;
>>>>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>>>>> +import org.ofbiz.base.container.**ContainerException;
>>>>>>> import org.ofbiz.base.util.Debug;
>>>>>>> +import org.ofbiz.base.util.FileUtil;
>>>>>>> import org.ofbiz.base.util.SSLUtil;
>>>>>>> import org.ofbiz.base.util.UtilURL;
>>>>>>> import org.ofbiz.base.util.**UtilValidate;
>>>>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>>>>          } catch (Exception e) {
>>>>>>>              Debug.logError(e, "Couldn't create connector.", module);
>>>>>>>          }
>>>>>>> -
>>>>>>> +
>>>>>>>          try {
>>>>>>>              for (ContainerConfig.Container.**Property prop:
>>>>>>> connectorProp.properties.**values()) {
>>>>>>>                  connector.setProperty(prop.**name <http://prop.name>,
>>>>>>> prop.value);
>>>>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>>>>              engine.addChild(host);
>>>>>>>          }
>>>>>>>      }
>>>>>>> -
>>>>>>> +
>>>>>>>      if (host instanceof StandardHost) {
>>>>>>>          // set the catalina's work directory to the host
>>>>>>>          StandardHost standardHost = (StandardHost) host;
>>>>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>>>>          mount = mount.substring(0, mount.length() - 2);
>>>>>>>      }
>>>>>>> 
>>>>>>> +
>>>>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>>>>> location)
>>>>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>>>>> +        boolean appIsDistributable = false;
>>>>>>> +        try {
>>>>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>>>>> "<distributable/>");
>>>>>>> +        } catch (IOException e) {
>>>>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>>>>> [%s].", webXmlFilePath), module);
>>>>>>> +            appIsDistributable = false;
>>>>>>> +        }
>>>>>>> +        final boolean contextIsDistributable = distribute &&
>>>>>>> appIsDistributable;
>>>>>>> +
>>>>>>>      // configure persistent sessions
>>>>>>>      Property clusterProp = clusterConfig.get(engine.**getName());
>>>>>>> 
>>>>>>>      Manager sessionMgr = null;
>>>>>>> -        if (clusterProp != null) {
>>>>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>>>>          String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>>>>> "manager-class",
>>>>>>>          "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>>>>              sessionMgr = (Manager)Class.forName(**
>>>>>>> mgrClassName).newInstance();
>>>>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>>>>      context.setDocBase(location);
>>>>>>>      context.setPath(mount);
>>>>>>>      context.addLifecycleListener(**new ContextConfig());
>>>>>>> -
>>>>>>> +
>>>>>>>      JarScanner jarScanner = context.getJarScanner();
>>>>>>>      if (jarScanner instanceof StandardJarScanner) {
>>>>>>>          StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>>>>> jarScanner;
>>>>>>>          standardJarScanner.**setScanClassPath(false);
>>>>>>>      }
>>>>>>> -
>>>>>>> +
>>>>>>>      Engine egn = (Engine) context.getParent().getParent(**);
>>>>>>>      egn.setService(tomcat.**getService());
>>>>>>> -
>>>>>>> +
>>>>>>>      Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>>>>> module);
>>>>>>>      //context.setDeployOnStartup(**false);
>>>>>>>      //context.**setBackgroundProcessorDelay(5)**;
>>>>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>>>>      context.setAllowLinking(true);
>>>>>>> 
>>>>>>>      context.setReloadable(**contextReloadable);
>>>>>>> -        context.setDistributable(**distribute);
>>>>>>> +
>>>>>>> +        context.setDistributable(**contextIsDistributable);
>>>>>>> +
>>>>>>>      context.setCrossContext(**crossContext);
>>>>>>>      context.setPrivileged(appInfo.**privileged);
>>>>>>>      context.setManager(sessionMgr)**;
>>>>>>> 
>>>>>> 
>> 
>> 


Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
You are right Scott, this can be improved...

Jacques

From: "Scott Gray" <sc...@hotwaxmedia.com>
> This is not a good implementation, simply searching the web.xml file for a string containing "<distributable/>" is not good
> enough.  It'll find the tag even if commented out and and won't find the tag "<distributable />" (space before closing, perfectly
> valid).  I'm surprised this got past you Jacques.
>
> Regards
> Scott
>
> On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:
>
>> Pierre,
>>
>> I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
>> It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain
>> circumstances.
>> So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for
>> more)
>>
>> Jacque
>>
>> From: "Pierre Smits" <pi...@gmail.com>
>>> Jacques,
>>>
>>> If I have this tag in the web.xml, but the change is tested in an
>>> unclustered environment what do you thing the result would be? Is it of no
>>> effect, would it fail?
>>>
>>> Regards,
>>>
>>> Pierre
>>>
>>> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>>>
>>>> Hi Sam,
>>>>
>>>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>>>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>>>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>>>
>>>> Also consider
>>>> <property name="apps-cross-context" value="true"/>
>>>> <property name="apps-context-reloadable" value="true"/>
>>>> (false by default)
>>>>
>>>> If you use sticky sessions all above is not an issue...
>>>>
>>>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>>>> provide one ;o)
>>>>
>>>> Jacques
>>>>
>>>>
>>>> Sam Hamilton wrote:
>>>>
>>>>> Hi Jacques,
>>>>>
>>>>> Quick question - does this setting mean that now if you uncomment
>>>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>>>> cluster and that you should also add a <distributable/> tag to all the
>>>>> web.xml files?
>>>>>
>>>>> Thanks
>>>>> Sam
>>>>>
>>>>>
>>>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>>>
>>>>> Author: jleroux
>>>>>> Date: Sat Apr 14 07:30:30 2012
>>>>>> New Revision: 1326064
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>>>> Log:
>>>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>>>> <distributable/> Node for web.xml files."
>>>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>>>
>>>>>> Per the servlet specification, the web.xml file should allow an optional
>>>>>> <distributable/> node to indicate that the application
>>>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>>>> assumes all applications should be distributed if any cluster
>>>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>>>> if, for example, the DeltaManager is enable, all
>>>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>>>
>>>>>> The expected and proper functionality is to check the individual
>>>>>> application's web.xml file for the <distributable/> node and
>>>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>>>> be used for sessions.
>>>>>>
>>>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>>>> <distributable/> in the *-containers.file
>>>>>>
>>>>>> Modified:
>>>>>>  ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>>  ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>>>> CatalinaContainer.java
>>>>>>
>>>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>>>> FileUtil.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>> ==============================**==============================**==================
>>>>>> ---
>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>> (original) +++
>>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>>>> java.io.FileWriter;
>>>>>> import java.io.FilenameFilter;
>>>>>> import java.io.IOException;
>>>>>> import java.io.OutputStream;
>>>>>> +import java.io.Reader;
>>>>>> import java.io.Writer;
>>>>>> import java.net.**MalformedURLException;
>>>>>> import java.util.List;
>>>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>>>
>>>>>>   /**
>>>>>>    * Converts a file path to one that is compatible with the host
>>>>>> operating system.
>>>>>> -     *
>>>>>> +     *
>>>>>>    * @param path The file path to convert.
>>>>>>    * @return The converted file path.
>>>>>>    */
>>>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>>>           return false;
>>>>>>       }
>>>>>>   }
>>>>>> +
>>>>>> +    /**
>>>>>> +    *
>>>>>> +    *
>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>> +    * {@link Reader}.
>>>>>> +    *
>>>>>> +    * @param reader A Reader in which the String will be searched.
>>>>>> +    * @param searchString The String to search for
>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>> found;
>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>> +    * @throws IOException
>>>>>> +    */
>>>>>> +   public static boolean containsString(Reader reader, final String
>>>>>> searchString) throws IOException {
>>>>>> +       char[] buffer = new char[1024];
>>>>>> +       int numCharsRead;
>>>>>> +       int count = 0;
>>>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>>>> +               else count = 0;
>>>>>> +               if (count == searchString.length()) return true;
>>>>>> +           }
>>>>>> +       }
>>>>>> +       return false;
>>>>>> +   }
>>>>>> +
>>>>>> +   /**
>>>>>> +    *
>>>>>> +    *
>>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>>>> +    * returns.
>>>>>> +    *
>>>>>> +    * @param fileName A full path to a file in which the String will be
>>>>>> searched.
>>>>>> +    * @param searchString The String to search for
>>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>>> found;
>>>>>> +    *         <code>FALSE</code> otherwise.
>>>>>> +    * @throws IOException
>>>>>> +    */
>>>>>> +   public static boolean containsString(final String fileName, final
>>>>>> String searchString) throws IOException {
>>>>>> +       File inFile = new File(fileName);
>>>>>> +       if (inFile.exists()) {
>>>>>> +           BufferedReader in = new BufferedReader(new
>>>>>> FileReader(inFile));
>>>>>> +           try {
>>>>>> +               return containsString(in, searchString);
>>>>>> +           } finally {
>>>>>> +               if (in != null)in.close();
>>>>>> +           }
>>>>>> +       } else {
>>>>>> +           return false;
>>>>>> +       }
>>>>>> +   }
>>>>>> }
>>>>>>
>>>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>>>> catalina/container/**CatalinaContainer.java
>>>>>> URL:
>>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>>> ==============================**==============================**==================
>>>>>> ---
>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>> (original) +++
>>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>>>> import org.ofbiz.base.container.**Container;
>>>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>>>> -import org.ofbiz.base.container.**ContainerException;
>>>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>>>> +import org.ofbiz.base.container.**ContainerException;
>>>>>> import org.ofbiz.base.util.Debug;
>>>>>> +import org.ofbiz.base.util.FileUtil;
>>>>>> import org.ofbiz.base.util.SSLUtil;
>>>>>> import org.ofbiz.base.util.UtilURL;
>>>>>> import org.ofbiz.base.util.**UtilValidate;
>>>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>>>           } catch (Exception e) {
>>>>>>               Debug.logError(e, "Couldn't create connector.", module);
>>>>>>           }
>>>>>> -
>>>>>> +
>>>>>>           try {
>>>>>>               for (ContainerConfig.Container.**Property prop:
>>>>>> connectorProp.properties.**values()) {
>>>>>>                   connector.setProperty(prop.**name <http://prop.name>,
>>>>>> prop.value);
>>>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>>>               engine.addChild(host);
>>>>>>           }
>>>>>>       }
>>>>>> -
>>>>>> +
>>>>>>       if (host instanceof StandardHost) {
>>>>>>           // set the catalina's work directory to the host
>>>>>>           StandardHost standardHost = (StandardHost) host;
>>>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>>>           mount = mount.substring(0, mount.length() - 2);
>>>>>>       }
>>>>>>
>>>>>> +
>>>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>>>> location)
>>>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>>>> +        boolean appIsDistributable = false;
>>>>>> +        try {
>>>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>>>> "<distributable/>");
>>>>>> +        } catch (IOException e) {
>>>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>>>> [%s].", webXmlFilePath), module);
>>>>>> +            appIsDistributable = false;
>>>>>> +        }
>>>>>> +        final boolean contextIsDistributable = distribute &&
>>>>>> appIsDistributable;
>>>>>> +
>>>>>>       // configure persistent sessions
>>>>>>       Property clusterProp = clusterConfig.get(engine.**getName());
>>>>>>
>>>>>>       Manager sessionMgr = null;
>>>>>> -        if (clusterProp != null) {
>>>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>>>           String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>>>> "manager-class",
>>>>>>           "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>>>               sessionMgr = (Manager)Class.forName(**
>>>>>> mgrClassName).newInstance();
>>>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>>>       context.setDocBase(location);
>>>>>>       context.setPath(mount);
>>>>>>       context.addLifecycleListener(**new ContextConfig());
>>>>>> -
>>>>>> +
>>>>>>       JarScanner jarScanner = context.getJarScanner();
>>>>>>       if (jarScanner instanceof StandardJarScanner) {
>>>>>>           StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>>>> jarScanner;
>>>>>>           standardJarScanner.**setScanClassPath(false);
>>>>>>       }
>>>>>> -
>>>>>> +
>>>>>>       Engine egn = (Engine) context.getParent().getParent(**);
>>>>>>       egn.setService(tomcat.**getService());
>>>>>> -
>>>>>> +
>>>>>>       Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>>>> module);
>>>>>>       //context.setDeployOnStartup(**false);
>>>>>>       //context.**setBackgroundProcessorDelay(5)**;
>>>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>>>       context.setAllowLinking(true);
>>>>>>
>>>>>>       context.setReloadable(**contextReloadable);
>>>>>> -        context.setDistributable(**distribute);
>>>>>> +
>>>>>> +        context.setDistributable(**contextIsDistributable);
>>>>>> +
>>>>>>       context.setCrossContext(**crossContext);
>>>>>>       context.setPrivileged(appInfo.**privileged);
>>>>>>       context.setManager(sessionMgr)**;
>>>>>>
>>>>>
>
>

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
This is not a good implementation, simply searching the web.xml file for a string containing "<distributable/>" is not good enough.  It'll find the tag even if commented out and and won't find the tag "<distributable />" (space before closing, perfectly valid).  I'm surprised this got past you Jacques.

Regards
Scott

On 1/05/2012, at 1:31 AM, Jacques Le Roux wrote:

> Pierre,
> 
> I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
> It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain circumstances.
> So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for more)
> 
> Jacque
> 
> From: "Pierre Smits" <pi...@gmail.com>
>> Jacques,
>> 
>> If I have this tag in the web.xml, but the change is tested in an
>> unclustered environment what do you thing the result would be? Is it of no
>> effect, would it fail?
>> 
>> Regards,
>> 
>> Pierre
>> 
>> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>> 
>>> Hi Sam,
>>> 
>>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>> 
>>> Also consider
>>> <property name="apps-cross-context" value="true"/>
>>> <property name="apps-context-reloadable" value="true"/>
>>> (false by default)
>>> 
>>> If you use sticky sessions all above is not an issue...
>>> 
>>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>>> provide one ;o)
>>> 
>>> Jacques
>>> 
>>> 
>>> Sam Hamilton wrote:
>>> 
>>>> Hi Jacques,
>>>> 
>>>> Quick question - does this setting mean that now if you uncomment
>>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>>> cluster and that you should also add a <distributable/> tag to all the
>>>> web.xml files?
>>>> 
>>>> Thanks
>>>> Sam
>>>> 
>>>> 
>>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>> 
>>>> Author: jleroux
>>>>> Date: Sat Apr 14 07:30:30 2012
>>>>> New Revision: 1326064
>>>>> 
>>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>>> Log:
>>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>>> <distributable/> Node for web.xml files."
>>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>> 
>>>>> Per the servlet specification, the web.xml file should allow an optional
>>>>> <distributable/> node to indicate that the application
>>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>>> assumes all applications should be distributed if any cluster
>>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>>> if, for example, the DeltaManager is enable, all
>>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>> 
>>>>> The expected and proper functionality is to check the individual
>>>>> application's web.xml file for the <distributable/> node and
>>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>>> be used for sessions.
>>>>> 
>>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>>> <distributable/> in the *-containers.file
>>>>> 
>>>>> Modified:
>>>>>  ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>>  ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>>> CatalinaContainer.java
>>>>> 
>>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>>> FileUtil.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>> ==============================**==============================**==================
>>>>> ---
>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>> (original) +++
>>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>>> java.io.FileWriter;
>>>>> import java.io.FilenameFilter;
>>>>> import java.io.IOException;
>>>>> import java.io.OutputStream;
>>>>> +import java.io.Reader;
>>>>> import java.io.Writer;
>>>>> import java.net.**MalformedURLException;
>>>>> import java.util.List;
>>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>> 
>>>>>   /**
>>>>>    * Converts a file path to one that is compatible with the host
>>>>> operating system.
>>>>> -     *
>>>>> +     *
>>>>>    * @param path The file path to convert.
>>>>>    * @return The converted file path.
>>>>>    */
>>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>>           return false;
>>>>>       }
>>>>>   }
>>>>> +
>>>>> +    /**
>>>>> +    *
>>>>> +    *
>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>> +    * {@link Reader}.
>>>>> +    *
>>>>> +    * @param reader A Reader in which the String will be searched.
>>>>> +    * @param searchString The String to search for
>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>> found;
>>>>> +    *         <code>FALSE</code> otherwise.
>>>>> +    * @throws IOException
>>>>> +    */
>>>>> +   public static boolean containsString(Reader reader, final String
>>>>> searchString) throws IOException {
>>>>> +       char[] buffer = new char[1024];
>>>>> +       int numCharsRead;
>>>>> +       int count = 0;
>>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>>> +               else count = 0;
>>>>> +               if (count == searchString.length()) return true;
>>>>> +           }
>>>>> +       }
>>>>> +       return false;
>>>>> +   }
>>>>> +
>>>>> +   /**
>>>>> +    *
>>>>> +    *
>>>>> +    * Search for the specified <code>searchString</code> in the given
>>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>>> +    * returns.
>>>>> +    *
>>>>> +    * @param fileName A full path to a file in which the String will be
>>>>> searched.
>>>>> +    * @param searchString The String to search for
>>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>>> found;
>>>>> +    *         <code>FALSE</code> otherwise.
>>>>> +    * @throws IOException
>>>>> +    */
>>>>> +   public static boolean containsString(final String fileName, final
>>>>> String searchString) throws IOException {
>>>>> +       File inFile = new File(fileName);
>>>>> +       if (inFile.exists()) {
>>>>> +           BufferedReader in = new BufferedReader(new
>>>>> FileReader(inFile));
>>>>> +           try {
>>>>> +               return containsString(in, searchString);
>>>>> +           } finally {
>>>>> +               if (in != null)in.close();
>>>>> +           }
>>>>> +       } else {
>>>>> +           return false;
>>>>> +       }
>>>>> +   }
>>>>> }
>>>>> 
>>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>>> catalina/container/**CatalinaContainer.java
>>>>> URL:
>>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>>> ==============================**==============================**==================
>>>>> ---
>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>> (original) +++
>>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>>> import org.ofbiz.base.container.**Container;
>>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>>> -import org.ofbiz.base.container.**ContainerException;
>>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>>> +import org.ofbiz.base.container.**ContainerException;
>>>>> import org.ofbiz.base.util.Debug;
>>>>> +import org.ofbiz.base.util.FileUtil;
>>>>> import org.ofbiz.base.util.SSLUtil;
>>>>> import org.ofbiz.base.util.UtilURL;
>>>>> import org.ofbiz.base.util.**UtilValidate;
>>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>>           } catch (Exception e) {
>>>>>               Debug.logError(e, "Couldn't create connector.", module);
>>>>>           }
>>>>> -
>>>>> +
>>>>>           try {
>>>>>               for (ContainerConfig.Container.**Property prop:
>>>>> connectorProp.properties.**values()) {
>>>>>                   connector.setProperty(prop.**name <http://prop.name>,
>>>>> prop.value);
>>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>>               engine.addChild(host);
>>>>>           }
>>>>>       }
>>>>> -
>>>>> +
>>>>>       if (host instanceof StandardHost) {
>>>>>           // set the catalina's work directory to the host
>>>>>           StandardHost standardHost = (StandardHost) host;
>>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>>           mount = mount.substring(0, mount.length() - 2);
>>>>>       }
>>>>> 
>>>>> +
>>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>>> location)
>>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>>> +        boolean appIsDistributable = false;
>>>>> +        try {
>>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>>> "<distributable/>");
>>>>> +        } catch (IOException e) {
>>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>>> [%s].", webXmlFilePath), module);
>>>>> +            appIsDistributable = false;
>>>>> +        }
>>>>> +        final boolean contextIsDistributable = distribute &&
>>>>> appIsDistributable;
>>>>> +
>>>>>       // configure persistent sessions
>>>>>       Property clusterProp = clusterConfig.get(engine.**getName());
>>>>> 
>>>>>       Manager sessionMgr = null;
>>>>> -        if (clusterProp != null) {
>>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>>           String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>>> "manager-class",
>>>>>           "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>>               sessionMgr = (Manager)Class.forName(**
>>>>> mgrClassName).newInstance();
>>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>>       context.setDocBase(location);
>>>>>       context.setPath(mount);
>>>>>       context.addLifecycleListener(**new ContextConfig());
>>>>> -
>>>>> +
>>>>>       JarScanner jarScanner = context.getJarScanner();
>>>>>       if (jarScanner instanceof StandardJarScanner) {
>>>>>           StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>>> jarScanner;
>>>>>           standardJarScanner.**setScanClassPath(false);
>>>>>       }
>>>>> -
>>>>> +
>>>>>       Engine egn = (Engine) context.getParent().getParent(**);
>>>>>       egn.setService(tomcat.**getService());
>>>>> -
>>>>> +
>>>>>       Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>>> module);
>>>>>       //context.setDeployOnStartup(**false);
>>>>>       //context.**setBackgroundProcessorDelay(5)**;
>>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>>       context.setAllowLinking(true);
>>>>> 
>>>>>       context.setReloadable(**contextReloadable);
>>>>> -        context.setDistributable(**distribute);
>>>>> +
>>>>> +        context.setDistributable(**contextIsDistributable);
>>>>> +
>>>>>       context.setCrossContext(**crossContext);
>>>>>       context.setPrivileged(appInfo.**privileged);
>>>>>       context.setManager(sessionMgr)**;
>>>>> 
>>>> 


Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Pierre,

I did not test, but I believe it's only used in a clustered environment and should have any impacts in other cases.
It makes only a webapp distributable. Before we added this, all were distributable by default, which could annoying in certain 
circumstances.
So adding it in web.xml files should not changes from previous behaviour. Which makes me believe it's safe... (see commit for more)

Jacque

From: "Pierre Smits" <pi...@gmail.com>
> Jacques,
>
> If I have this tag in the web.xml, but the change is tested in an
> unclustered environment what do you thing the result would be? Is it of no
> effect, would it fail?
>
> Regards,
>
> Pierre
>
> 2012/4/30 Jacques Le Roux <ja...@les7arts.com>
>
>> Hi Sam,
>>
>> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
>> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
>> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>>
>> Also consider
>> <property name="apps-cross-context" value="true"/>
>> <property name="apps-context-reloadable" value="true"/>
>> (false by default)
>>
>> If you use sticky sessions all above is not an issue...
>>
>> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
>> provide one ;o)
>>
>> Jacques
>>
>>
>> Sam Hamilton wrote:
>>
>>> Hi Jacques,
>>>
>>> Quick question - does this setting mean that now if you uncomment
>>> framework/config/ofbiz-**containers.xml cluster settings it wont
>>> cluster and that you should also add a <distributable/> tag to all the
>>> web.xml files?
>>>
>>> Thanks
>>> Sam
>>>
>>>
>>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>>
>>>  Author: jleroux
>>>> Date: Sat Apr 14 07:30:30 2012
>>>> New Revision: 1326064
>>>>
>>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>>> Log:
>>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>>> <distributable/> Node for web.xml files."
>>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>>
>>>> Per the servlet specification, the web.xml file should allow an optional
>>>> <distributable/> node to indicate that the application
>>>> is clusterable. Currently, OFBiz does not respect this setting and
>>>> assumes all applications should be distributed if any cluster
>>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>>> if, for example, the DeltaManager is enable, all
>>>> applications attempt to cluster and communicate via DeltaManager.
>>>>
>>>> The expected and proper functionality is to check the individual
>>>> application's web.xml file for the <distributable/> node and
>>>> only use the DeltaManager if found; otherwise the StandardManager should
>>>> be used for sessions.
>>>>
>>>> jleroux: replaced some tabs, reformatted, added a comment about
>>>> <distributable/> in the *-containers.file
>>>>
>>>> Modified:
>>>>   ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>>   ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>>> CatalinaContainer.java
>>>>
>>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>>> FileUtil.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>> ==============================**==============================**==================
>>>> ---
>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>> (original) +++
>>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>>> java.io.FileWriter;
>>>> import java.io.FilenameFilter;
>>>> import java.io.IOException;
>>>> import java.io.OutputStream;
>>>> +import java.io.Reader;
>>>> import java.io.Writer;
>>>> import java.net.**MalformedURLException;
>>>> import java.util.List;
>>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>>
>>>>    /**
>>>>     * Converts a file path to one that is compatible with the host
>>>> operating system.
>>>> -     *
>>>> +     *
>>>>     * @param path The file path to convert.
>>>>     * @return The converted file path.
>>>>     */
>>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>>            return false;
>>>>        }
>>>>    }
>>>> +
>>>> +    /**
>>>> +    *
>>>> +    *
>>>> +    * Search for the specified <code>searchString</code> in the given
>>>> +    * {@link Reader}.
>>>> +    *
>>>> +    * @param reader A Reader in which the String will be searched.
>>>> +    * @param searchString The String to search for
>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>> found;
>>>> +    *         <code>FALSE</code> otherwise.
>>>> +    * @throws IOException
>>>> +    */
>>>> +   public static boolean containsString(Reader reader, final String
>>>> searchString) throws IOException {
>>>> +       char[] buffer = new char[1024];
>>>> +       int numCharsRead;
>>>> +       int count = 0;
>>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>>> +               else count = 0;
>>>> +               if (count == searchString.length()) return true;
>>>> +           }
>>>> +       }
>>>> +       return false;
>>>> +   }
>>>> +
>>>> +   /**
>>>> +    *
>>>> +    *
>>>> +    * Search for the specified <code>searchString</code> in the given
>>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>>> +    * returns.
>>>> +    *
>>>> +    * @param fileName A full path to a file in which the String will be
>>>> searched.
>>>> +    * @param searchString The String to search for
>>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>>> found;
>>>> +    *         <code>FALSE</code> otherwise.
>>>> +    * @throws IOException
>>>> +    */
>>>> +   public static boolean containsString(final String fileName, final
>>>> String searchString) throws IOException {
>>>> +       File inFile = new File(fileName);
>>>> +       if (inFile.exists()) {
>>>> +           BufferedReader in = new BufferedReader(new
>>>> FileReader(inFile));
>>>> +           try {
>>>> +               return containsString(in, searchString);
>>>> +           } finally {
>>>> +               if (in != null)in.close();
>>>> +           }
>>>> +       } else {
>>>> +           return false;
>>>> +       }
>>>> +   }
>>>> }
>>>>
>>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>>> catalina/container/**CatalinaContainer.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>>> catalina/src/org/ofbiz/**catalina/container/**
>>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>>> ==============================**==============================**==================
>>>> ---
>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>> (original) +++
>>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>>> @@ import org.ofbiz.base.concurrent.**Executi
>>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>>> import org.ofbiz.base.container.**Container;
>>>> import org.ofbiz.base.container.**ContainerConfig;
>>>> -import org.ofbiz.base.container.**ContainerException;
>>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>>> +import org.ofbiz.base.container.**ContainerException;
>>>> import org.ofbiz.base.util.Debug;
>>>> +import org.ofbiz.base.util.FileUtil;
>>>> import org.ofbiz.base.util.SSLUtil;
>>>> import org.ofbiz.base.util.UtilURL;
>>>> import org.ofbiz.base.util.**UtilValidate;
>>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>>            } catch (Exception e) {
>>>>                Debug.logError(e, "Couldn't create connector.", module);
>>>>            }
>>>> -
>>>> +
>>>>            try {
>>>>                for (ContainerConfig.Container.**Property prop:
>>>> connectorProp.properties.**values()) {
>>>>                    connector.setProperty(prop.**name <http://prop.name>,
>>>> prop.value);
>>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>>                engine.addChild(host);
>>>>            }
>>>>        }
>>>> -
>>>> +
>>>>        if (host instanceof StandardHost) {
>>>>            // set the catalina's work directory to the host
>>>>            StandardHost standardHost = (StandardHost) host;
>>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>>            mount = mount.substring(0, mount.length() - 2);
>>>>        }
>>>>
>>>> +
>>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>>> location)
>>>> +            .append(File.separatorChar).**append("WEB-INF")
>>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>>> +        boolean appIsDistributable = false;
>>>> +        try {
>>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>>> "<distributable/>");
>>>> +        } catch (IOException e) {
>>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>>> [%s].", webXmlFilePath), module);
>>>> +            appIsDistributable = false;
>>>> +        }
>>>> +        final boolean contextIsDistributable = distribute &&
>>>> appIsDistributable;
>>>> +
>>>>        // configure persistent sessions
>>>>        Property clusterProp = clusterConfig.get(engine.**getName());
>>>>
>>>>        Manager sessionMgr = null;
>>>> -        if (clusterProp != null) {
>>>> +        if (clusterProp != null && contextIsDistributable) {
>>>>            String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>>> "manager-class",
>>>>            "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>>                sessionMgr = (Manager)Class.forName(**
>>>> mgrClassName).newInstance();
>>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>>        context.setDocBase(location);
>>>>        context.setPath(mount);
>>>>        context.addLifecycleListener(**new ContextConfig());
>>>> -
>>>> +
>>>>        JarScanner jarScanner = context.getJarScanner();
>>>>        if (jarScanner instanceof StandardJarScanner) {
>>>>            StandardJarScanner standardJarScanner = (StandardJarScanner)
>>>> jarScanner;
>>>>            standardJarScanner.**setScanClassPath(false);
>>>>        }
>>>> -
>>>> +
>>>>        Engine egn = (Engine) context.getParent().getParent(**);
>>>>        egn.setService(tomcat.**getService());
>>>> -
>>>> +
>>>>        Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>>> module);
>>>>        //context.setDeployOnStartup(**false);
>>>>        //context.**setBackgroundProcessorDelay(5)**;
>>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>>        context.setAllowLinking(true);
>>>>
>>>>        context.setReloadable(**contextReloadable);
>>>> -        context.setDistributable(**distribute);
>>>> +
>>>> +        context.setDistributable(**contextIsDistributable);
>>>> +
>>>>        context.setCrossContext(**crossContext);
>>>>        context.setPrivileged(appInfo.**privileged);
>>>>        context.setManager(sessionMgr)**;
>>>>
>>>
> 

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Pierre Smits <pi...@gmail.com>.
Jacques,

If I have this tag in the web.xml, but the change is tested in an
unclustered environment what do you thing the result would be? Is it of no
effect, would it fail?

Regards,

Pierre

2012/4/30 Jacques Le Roux <ja...@les7arts.com>

> Hi Sam,
>
> Yes: http://tomcat.apache.org/**tomcat-6.0-doc/cluster-howto.**
> html#Cluster_Basics<http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics>
> http://wiki.metawerx.net/Wiki.**jsp?page=web.xml.Distributable<http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable>
>
> Also consider
> <property name="apps-cross-context" value="true"/>
> <property name="apps-context-reloadable" value="true"/>
> (false by default)
>
> If you use sticky sessions all above is not an issue...
>
> Maybe we should ask a comment in *-containers.xml BTW... Feel free to
> provide one ;o)
>
> Jacques
>
>
> Sam Hamilton wrote:
>
>> Hi Jacques,
>>
>> Quick question - does this setting mean that now if you uncomment
>> framework/config/ofbiz-**containers.xml cluster settings it wont
>> cluster and that you should also add a <distributable/> tag to all the
>> web.xml files?
>>
>> Thanks
>> Sam
>>
>>
>> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>>
>>  Author: jleroux
>>> Date: Sat Apr 14 07:30:30 2012
>>> New Revision: 1326064
>>>
>>> URL: http://svn.apache.org/viewvc?**rev=1326064&view=rev<http://svn.apache.org/viewvc?rev=1326064&view=rev>
>>> Log:
>>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect
>>> <distributable/> Node for web.xml files."
>>> https://issues.apache.org/**jira/browse/OFBIZ-4242<https://issues.apache.org/jira/browse/OFBIZ-4242>
>>>
>>> Per the servlet specification, the web.xml file should allow an optional
>>> <distributable/> node to indicate that the application
>>> is clusterable. Currently, OFBiz does not respect this setting and
>>> assumes all applications should be distributed if any cluster
>>> configuration is provided in the ofbiz-containers.xml file. As a result,
>>> if, for example, the DeltaManager is enable, all
>>> applications attempt to cluster and communicate via DeltaManager.
>>>
>>> The expected and proper functionality is to check the individual
>>> application's web.xml file for the <distributable/> node and
>>> only use the DeltaManager if found; otherwise the StandardManager should
>>> be used for sessions.
>>>
>>> jleroux: replaced some tabs, reformatted, added a comment about
>>> <distributable/> in the *-containers.file
>>>
>>> Modified:
>>>   ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>>   ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**
>>> CatalinaContainer.java
>>>
>>> Modified: ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**
>>> FileUtil.java
>>> URL:
>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/base/**
>>> src/org/ofbiz/base/util/**FileUtil.java?rev=1326064&r1=**
>>> 1326063&r2=1326064&view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>> ==============================**==============================**==================
>>> ---
>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>> (original) +++
>>> ofbiz/trunk/framework/base/**src/org/ofbiz/base/util/**FileUtil.java
>>> Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>>> java.io.FileWriter;
>>> import java.io.FilenameFilter;
>>> import java.io.IOException;
>>> import java.io.OutputStream;
>>> +import java.io.Reader;
>>> import java.io.Writer;
>>> import java.net.**MalformedURLException;
>>> import java.util.List;
>>> @@ -67,7 +68,7 @@ public class FileUtil {
>>>
>>>    /**
>>>     * Converts a file path to one that is compatible with the host
>>> operating system.
>>> -     *
>>> +     *
>>>     * @param path The file path to convert.
>>>     * @return The converted file path.
>>>     */
>>> @@ -341,4 +342,57 @@ public class FileUtil {
>>>            return false;
>>>        }
>>>    }
>>> +
>>> +    /**
>>> +    *
>>> +    *
>>> +    * Search for the specified <code>searchString</code> in the given
>>> +    * {@link Reader}.
>>> +    *
>>> +    * @param reader A Reader in which the String will be searched.
>>> +    * @param searchString The String to search for
>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>> found;
>>> +    *         <code>FALSE</code> otherwise.
>>> +    * @throws IOException
>>> +    */
>>> +   public static boolean containsString(Reader reader, final String
>>> searchString) throws IOException {
>>> +       char[] buffer = new char[1024];
>>> +       int numCharsRead;
>>> +       int count = 0;
>>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>>> +           for (int c = 0; c < numCharsRead; ++c) {
>>> +               if (buffer[c] == searchString.charAt(count)) count++;
>>> +               else count = 0;
>>> +               if (count == searchString.length()) return true;
>>> +           }
>>> +       }
>>> +       return false;
>>> +   }
>>> +
>>> +   /**
>>> +    *
>>> +    *
>>> +    * Search for the specified <code>searchString</code> in the given
>>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>>> +    * returns.
>>> +    *
>>> +    * @param fileName A full path to a file in which the String will be
>>> searched.
>>> +    * @param searchString The String to search for
>>> +    * @return <code>TRUE</code> if the <code>searchString</code> is
>>> found;
>>> +    *         <code>FALSE</code> otherwise.
>>> +    * @throws IOException
>>> +    */
>>> +   public static boolean containsString(final String fileName, final
>>> String searchString) throws IOException {
>>> +       File inFile = new File(fileName);
>>> +       if (inFile.exists()) {
>>> +           BufferedReader in = new BufferedReader(new
>>> FileReader(inFile));
>>> +           try {
>>> +               return containsString(in, searchString);
>>> +           } finally {
>>> +               if (in != null)in.close();
>>> +           }
>>> +       } else {
>>> +           return false;
>>> +       }
>>> +   }
>>> }
>>>
>>> Modified: ofbiz/trunk/framework/**catalina/src/org/ofbiz/**
>>> catalina/container/**CatalinaContainer.java
>>> URL:
>>> http://svn.apache.org/viewvc/**ofbiz/trunk/framework/**
>>> catalina/src/org/ofbiz/**catalina/container/**
>>> CatalinaContainer.java?rev=**1326064&r1=1326063&r2=1326064&**view=diff<http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff>
>>> ==============================**==============================**==================
>>> ---
>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>> (original) +++
>>> ofbiz/trunk/framework/**catalina/src/org/ofbiz/**catalina/container/**CatalinaContainer.java
>>> Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>>> @@ import org.ofbiz.base.concurrent.**Executi
>>> import org.ofbiz.base.container.**ClassLoaderContainer;
>>> import org.ofbiz.base.container.**Container;
>>> import org.ofbiz.base.container.**ContainerConfig;
>>> -import org.ofbiz.base.container.**ContainerException;
>>> import org.ofbiz.base.container.**ContainerConfig.Container.**Property;
>>> +import org.ofbiz.base.container.**ContainerException;
>>> import org.ofbiz.base.util.Debug;
>>> +import org.ofbiz.base.util.FileUtil;
>>> import org.ofbiz.base.util.SSLUtil;
>>> import org.ofbiz.base.util.UtilURL;
>>> import org.ofbiz.base.util.**UtilValidate;
>>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>>            } catch (Exception e) {
>>>                Debug.logError(e, "Couldn't create connector.", module);
>>>            }
>>> -
>>> +
>>>            try {
>>>                for (ContainerConfig.Container.**Property prop:
>>> connectorProp.properties.**values()) {
>>>                    connector.setProperty(prop.**name <http://prop.name>,
>>> prop.value);
>>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>>                engine.addChild(host);
>>>            }
>>>        }
>>> -
>>> +
>>>        if (host instanceof StandardHost) {
>>>            // set the catalina's work directory to the host
>>>            StandardHost standardHost = (StandardHost) host;
>>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>>            mount = mount.substring(0, mount.length() - 2);
>>>        }
>>>
>>> +
>>> +        final String webXmlFilePath = new StringBuilder().append(**
>>> location)
>>> +            .append(File.separatorChar).**append("WEB-INF")
>>> +            .append(File.separatorChar).**append("web.xml").toString();
>>> +        boolean appIsDistributable = false;
>>> +        try {
>>> +            appIsDistributable = FileUtil.containsString(**webXmlFilePath,
>>> "<distributable/>");
>>> +        } catch (IOException e) {
>>> +            Debug.logWarning(String.**format("Failed to read web.xml
>>> [%s].", webXmlFilePath), module);
>>> +            appIsDistributable = false;
>>> +        }
>>> +        final boolean contextIsDistributable = distribute &&
>>> appIsDistributable;
>>> +
>>>        // configure persistent sessions
>>>        Property clusterProp = clusterConfig.get(engine.**getName());
>>>
>>>        Manager sessionMgr = null;
>>> -        if (clusterProp != null) {
>>> +        if (clusterProp != null && contextIsDistributable) {
>>>            String mgrClassName = ContainerConfig.**getPropertyValue(clusterProp,
>>> "manager-class",
>>>            "org.apache.catalina.ha.**session.DeltaManager"); try {
>>>                sessionMgr = (Manager)Class.forName(**
>>> mgrClassName).newInstance();
>>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>>        context.setDocBase(location);
>>>        context.setPath(mount);
>>>        context.addLifecycleListener(**new ContextConfig());
>>> -
>>> +
>>>        JarScanner jarScanner = context.getJarScanner();
>>>        if (jarScanner instanceof StandardJarScanner) {
>>>            StandardJarScanner standardJarScanner = (StandardJarScanner)
>>> jarScanner;
>>>            standardJarScanner.**setScanClassPath(false);
>>>        }
>>> -
>>> +
>>>        Engine egn = (Engine) context.getParent().getParent(**);
>>>        egn.setService(tomcat.**getService());
>>> -
>>> +
>>>        Debug.logInfo("host[" + host + "].addChild(" + context + ")",
>>> module);
>>>        //context.setDeployOnStartup(**false);
>>>        //context.**setBackgroundProcessorDelay(5)**;
>>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>>        context.setAllowLinking(true);
>>>
>>>        context.setReloadable(**contextReloadable);
>>> -        context.setDistributable(**distribute);
>>> +
>>> +        context.setDistributable(**contextIsDistributable);
>>> +
>>>        context.setCrossContext(**crossContext);
>>>        context.setPrivileged(appInfo.**privileged);
>>>        context.setManager(sessionMgr)**;
>>>
>>

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Sam,

Yes: http://tomcat.apache.org/tomcat-6.0-doc/cluster-howto.html#Cluster_Basics
http://wiki.metawerx.net/Wiki.jsp?page=web.xml.Distributable

Also consider
<property name="apps-cross-context" value="true"/>
<property name="apps-context-reloadable" value="true"/>
(false by default)

If you use sticky sessions all above is not an issue...

Maybe we should ask a comment in *-containers.xml BTW... Feel free to provide one ;o)

Jacques

Sam Hamilton wrote:
> Hi Jacques,
>
> Quick question - does this setting mean that now if you uncomment framework/config/ofbiz-containers.xml cluster settings it wont
> cluster and that you should also add a <distributable/> tag to all the web.xml files?
>
> Thanks
> Sam
>
>
> On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:
>
>> Author: jleroux
>> Date: Sat Apr 14 07:30:30 2012
>> New Revision: 1326064
>>
>> URL: http://svn.apache.org/viewvc?rev=1326064&view=rev
>> Log:
>> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect <distributable/> Node for web.xml files."
>> https://issues.apache.org/jira/browse/OFBIZ-4242
>>
>> Per the servlet specification, the web.xml file should allow an optional <distributable/> node to indicate that the application
>> is clusterable. Currently, OFBiz does not respect this setting and assumes all applications should be distributed if any cluster
>> configuration is provided in the ofbiz-containers.xml file. As a result, if, for example, the DeltaManager is enable, all
>> applications attempt to cluster and communicate via DeltaManager.
>>
>> The expected and proper functionality is to check the individual application's web.xml file for the <distributable/> node and
>> only use the DeltaManager if found; otherwise the StandardManager should be used for sessions.
>>
>> jleroux: replaced some tabs, reformatted, added a comment about <distributable/> in the *-containers.file
>>
>> Modified:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
>>    ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java
>>
>> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java (original) +++
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java Sat Apr 14 07:30:30 2012 @@ -29,6 +29,7 @@ import
>> java.io.FileWriter;
>> import java.io.FilenameFilter;
>> import java.io.IOException;
>> import java.io.OutputStream;
>> +import java.io.Reader;
>> import java.io.Writer;
>> import java.net.MalformedURLException;
>> import java.util.List;
>> @@ -67,7 +68,7 @@ public class FileUtil {
>>
>>     /**
>>      * Converts a file path to one that is compatible with the host operating system.
>> -     *
>> +     *
>>      * @param path The file path to convert.
>>      * @return The converted file path.
>>      */
>> @@ -341,4 +342,57 @@ public class FileUtil {
>>             return false;
>>         }
>>     }
>> +
>> +    /**
>> +    *
>> +    *
>> +    * Search for the specified <code>searchString</code> in the given
>> +    * {@link Reader}.
>> +    *
>> +    * @param reader A Reader in which the String will be searched.
>> +    * @param searchString The String to search for
>> +    * @return <code>TRUE</code> if the <code>searchString</code> is found;
>> +    *         <code>FALSE</code> otherwise.
>> +    * @throws IOException
>> +    */
>> +   public static boolean containsString(Reader reader, final String searchString) throws IOException {
>> +       char[] buffer = new char[1024];
>> +       int numCharsRead;
>> +       int count = 0;
>> +       while((numCharsRead = reader.read(buffer)) > 0) {
>> +           for (int c = 0; c < numCharsRead; ++c) {
>> +               if (buffer[c] == searchString.charAt(count)) count++;
>> +               else count = 0;
>> +               if (count == searchString.length()) return true;
>> +           }
>> +       }
>> +       return false;
>> +   }
>> +
>> +   /**
>> +    *
>> +    *
>> +    * Search for the specified <code>searchString</code> in the given
>> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
>> +    * returns.
>> +    *
>> +    * @param fileName A full path to a file in which the String will be searched.
>> +    * @param searchString The String to search for
>> +    * @return <code>TRUE</code> if the <code>searchString</code> is found;
>> +    *         <code>FALSE</code> otherwise.
>> +    * @throws IOException
>> +    */
>> +   public static boolean containsString(final String fileName, final String searchString) throws IOException {
>> +       File inFile = new File(fileName);
>> +       if (inFile.exists()) {
>> +           BufferedReader in = new BufferedReader(new FileReader(inFile));
>> +           try {
>> +               return containsString(in, searchString);
>> +           } finally {
>> +               if (in != null)in.close();
>> +           }
>> +       } else {
>> +           return false;
>> +       }
>> +   }
>> }
>>
>> Modified: ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java (original) +++
>> ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java Sat Apr 14 07:30:30 2012 @@ -75,9 +75,10
>> @@ import org.ofbiz.base.concurrent.Executi
>> import org.ofbiz.base.container.ClassLoaderContainer;
>> import org.ofbiz.base.container.Container;
>> import org.ofbiz.base.container.ContainerConfig;
>> -import org.ofbiz.base.container.ContainerException;
>> import org.ofbiz.base.container.ContainerConfig.Container.Property;
>> +import org.ofbiz.base.container.ContainerException;
>> import org.ofbiz.base.util.Debug;
>> +import org.ofbiz.base.util.FileUtil;
>> import org.ofbiz.base.util.SSLUtil;
>> import org.ofbiz.base.util.UtilURL;
>> import org.ofbiz.base.util.UtilValidate;
>> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>>             } catch (Exception e) {
>>                 Debug.logError(e, "Couldn't create connector.", module);
>>             }
>> -
>> +
>>             try {
>>                 for (ContainerConfig.Container.Property prop: connectorProp.properties.values()) {
>>                     connector.setProperty(prop.name, prop.value);
>> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>>                 engine.addChild(host);
>>             }
>>         }
>> -
>> +
>>         if (host instanceof StandardHost) {
>>             // set the catalina's work directory to the host
>>             StandardHost standardHost = (StandardHost) host;
>> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>>             mount = mount.substring(0, mount.length() - 2);
>>         }
>>
>> +
>> +        final String webXmlFilePath = new StringBuilder().append(location)
>> +            .append(File.separatorChar).append("WEB-INF")
>> +            .append(File.separatorChar).append("web.xml").toString();
>> +        boolean appIsDistributable = false;
>> +        try {
>> +            appIsDistributable = FileUtil.containsString(webXmlFilePath, "<distributable/>");
>> +        } catch (IOException e) {
>> +            Debug.logWarning(String.format("Failed to read web.xml [%s].", webXmlFilePath), module);
>> +            appIsDistributable = false;
>> +        }
>> +        final boolean contextIsDistributable = distribute && appIsDistributable;
>> +
>>         // configure persistent sessions
>>         Property clusterProp = clusterConfig.get(engine.getName());
>>
>>         Manager sessionMgr = null;
>> -        if (clusterProp != null) {
>> +        if (clusterProp != null && contextIsDistributable) {
>>             String mgrClassName = ContainerConfig.getPropertyValue(clusterProp, "manager-class",
>>             "org.apache.catalina.ha.session.DeltaManager"); try {
>>                 sessionMgr = (Manager)Class.forName(mgrClassName).newInstance();
>> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>>         context.setDocBase(location);
>>         context.setPath(mount);
>>         context.addLifecycleListener(new ContextConfig());
>> -
>> +
>>         JarScanner jarScanner = context.getJarScanner();
>>         if (jarScanner instanceof StandardJarScanner) {
>>             StandardJarScanner standardJarScanner = (StandardJarScanner) jarScanner;
>>             standardJarScanner.setScanClassPath(false);
>>         }
>> -
>> +
>>         Engine egn = (Engine) context.getParent().getParent();
>>         egn.setService(tomcat.getService());
>> -
>> +
>>         Debug.logInfo("host[" + host + "].addChild(" + context + ")", module);
>>         //context.setDeployOnStartup(false);
>>         //context.setBackgroundProcessorDelay(5);
>> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>>         context.setAllowLinking(true);
>>
>>         context.setReloadable(contextReloadable);
>> -        context.setDistributable(distribute);
>> +
>> +        context.setDistributable(contextIsDistributable);
>> +
>>         context.setCrossContext(crossContext);
>>         context.setPrivileged(appInfo.privileged);
>>         context.setManager(sessionMgr);

Re: svn commit: r1326064 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/FileUtil.java catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java

Posted by Sam Hamilton <sa...@sh81.com>.
Hi Jacques, 

Quick question - does this setting mean that now if you uncomment framework/config/ofbiz-containers.xml cluster settings it wont cluster and that you should also add a <distributable/> tag to all the web.xml files? 

Thanks
Sam


On 14 Apr 2012, at 15:30, jleroux@apache.org wrote:

> Author: jleroux
> Date: Sat Apr 14 07:30:30 2012
> New Revision: 1326064
> 
> URL: http://svn.apache.org/viewvc?rev=1326064&view=rev
> Log:
> Adapted by hand from Lon F. Binder "CatalinaContainer Doesn't Respect <distributable/> Node for web.xml files." https://issues.apache.org/jira/browse/OFBIZ-4242
> 
> Per the servlet specification, the web.xml file should allow an optional <distributable/> node to indicate that the application is clusterable. Currently, OFBiz does not respect this setting and assumes all applications should be distributed if any cluster configuration is provided in the ofbiz-containers.xml file. As a result, if, for example, the DeltaManager is enable, all applications attempt to cluster and communicate via DeltaManager.
> 
> The expected and proper functionality is to check the individual application's web.xml file for the <distributable/> node and only use the DeltaManager if found; otherwise the StandardManager should be used for sessions.
> 
> jleroux: replaced some tabs, reformatted, added a comment about <distributable/> in the *-containers.file
> 
> Modified:
>    ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
>    ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java
> 
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java?rev=1326064&r1=1326063&r2=1326064&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/FileUtil.java Sat Apr 14 07:30:30 2012
> @@ -29,6 +29,7 @@ import java.io.FileWriter;
> import java.io.FilenameFilter;
> import java.io.IOException;
> import java.io.OutputStream;
> +import java.io.Reader;
> import java.io.Writer;
> import java.net.MalformedURLException;
> import java.util.List;
> @@ -67,7 +68,7 @@ public class FileUtil {
> 
>     /**
>      * Converts a file path to one that is compatible with the host operating system.
> -     * 
> +     *
>      * @param path The file path to convert.
>      * @return The converted file path.
>      */
> @@ -341,4 +342,57 @@ public class FileUtil {
>             return false;
>         }
>     }
> +
> +    /**
> +    *
> +    *
> +    * Search for the specified <code>searchString</code> in the given
> +    * {@link Reader}.
> +    *
> +    * @param reader A Reader in which the String will be searched.
> +    * @param searchString The String to search for
> +    * @return <code>TRUE</code> if the <code>searchString</code> is found;
> +    *         <code>FALSE</code> otherwise.
> +    * @throws IOException
> +    */
> +   public static boolean containsString(Reader reader, final String searchString) throws IOException {
> +       char[] buffer = new char[1024];
> +       int numCharsRead;
> +       int count = 0;
> +       while((numCharsRead = reader.read(buffer)) > 0) {
> +           for (int c = 0; c < numCharsRead; ++c) {
> +               if (buffer[c] == searchString.charAt(count)) count++;
> +               else count = 0;
> +               if (count == searchString.length()) return true;
> +           }
> +       }
> +       return false;
> +   }
> +
> +   /**
> +    *
> +    *
> +    * Search for the specified <code>searchString</code> in the given
> +    * filename. If the specified file doesn't exist, <code>FALSE</code>
> +    * returns.
> +    *
> +    * @param fileName A full path to a file in which the String will be searched.
> +    * @param searchString The String to search for
> +    * @return <code>TRUE</code> if the <code>searchString</code> is found;
> +    *         <code>FALSE</code> otherwise.
> +    * @throws IOException
> +    */
> +   public static boolean containsString(final String fileName, final String searchString) throws IOException {
> +       File inFile = new File(fileName);
> +       if (inFile.exists()) {
> +           BufferedReader in = new BufferedReader(new FileReader(inFile));
> +           try {
> +               return containsString(in, searchString);
> +           } finally {
> +               if (in != null)in.close();
> +           }
> +       } else {
> +           return false;
> +       }
> +   }
> }
> 
> Modified: ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java?rev=1326064&r1=1326063&r2=1326064&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java (original)
> +++ ofbiz/trunk/framework/catalina/src/org/ofbiz/catalina/container/CatalinaContainer.java Sat Apr 14 07:30:30 2012
> @@ -75,9 +75,10 @@ import org.ofbiz.base.concurrent.Executi
> import org.ofbiz.base.container.ClassLoaderContainer;
> import org.ofbiz.base.container.Container;
> import org.ofbiz.base.container.ContainerConfig;
> -import org.ofbiz.base.container.ContainerException;
> import org.ofbiz.base.container.ContainerConfig.Container.Property;
> +import org.ofbiz.base.container.ContainerException;
> import org.ofbiz.base.util.Debug;
> +import org.ofbiz.base.util.FileUtil;
> import org.ofbiz.base.util.SSLUtil;
> import org.ofbiz.base.util.UtilURL;
> import org.ofbiz.base.util.UtilValidate;
> @@ -537,7 +538,7 @@ public class CatalinaContainer implement
>             } catch (Exception e) {
>                 Debug.logError(e, "Couldn't create connector.", module);
>             }
> -            
> +
>             try {
>                 for (ContainerConfig.Container.Property prop: connectorProp.properties.values()) {
>                     connector.setProperty(prop.name, prop.value);
> @@ -583,7 +584,7 @@ public class CatalinaContainer implement
>                 engine.addChild(host);
>             }
>         }
> -        
> +
>         if (host instanceof StandardHost) {
>             // set the catalina's work directory to the host
>             StandardHost standardHost = (StandardHost) host;
> @@ -618,11 +619,24 @@ public class CatalinaContainer implement
>             mount = mount.substring(0, mount.length() - 2);
>         }
> 
> +
> +        final String webXmlFilePath = new StringBuilder().append(location)
> +            .append(File.separatorChar).append("WEB-INF")
> +            .append(File.separatorChar).append("web.xml").toString();
> +        boolean appIsDistributable = false;
> +        try {
> +            appIsDistributable = FileUtil.containsString(webXmlFilePath, "<distributable/>");
> +        } catch (IOException e) {
> +            Debug.logWarning(String.format("Failed to read web.xml [%s].", webXmlFilePath), module);
> +            appIsDistributable = false;
> +        }
> +        final boolean contextIsDistributable = distribute && appIsDistributable;
> +
>         // configure persistent sessions
>         Property clusterProp = clusterConfig.get(engine.getName());
> 
>         Manager sessionMgr = null;
> -        if (clusterProp != null) {
> +        if (clusterProp != null && contextIsDistributable) {
>             String mgrClassName = ContainerConfig.getPropertyValue(clusterProp, "manager-class", "org.apache.catalina.ha.session.DeltaManager");
>             try {
>                 sessionMgr = (Manager)Class.forName(mgrClassName).newInstance();
> @@ -639,16 +653,16 @@ public class CatalinaContainer implement
>         context.setDocBase(location);
>         context.setPath(mount);
>         context.addLifecycleListener(new ContextConfig());
> -        
> +
>         JarScanner jarScanner = context.getJarScanner();
>         if (jarScanner instanceof StandardJarScanner) {
>             StandardJarScanner standardJarScanner = (StandardJarScanner) jarScanner;
>             standardJarScanner.setScanClassPath(false);
>         }
> -        
> +
>         Engine egn = (Engine) context.getParent().getParent();
>         egn.setService(tomcat.getService());
> -        
> +
>         Debug.logInfo("host[" + host + "].addChild(" + context + ")", module);
>         //context.setDeployOnStartup(false);
>         //context.setBackgroundProcessorDelay(5);
> @@ -664,7 +678,9 @@ public class CatalinaContainer implement
>         context.setAllowLinking(true);
> 
>         context.setReloadable(contextReloadable);
> -        context.setDistributable(distribute);
> +
> +        context.setDistributable(contextIsDistributable);
> +
>         context.setCrossContext(crossContext);
>         context.setPrivileged(appInfo.privileged);
>         context.setManager(sessionMgr);
> 
>