You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/11/07 20:24:17 UTC

svn commit: r1539771 - /tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java

Author: markt
Date: Thu Nov  7 19:24:17 2013
New Revision: 1539771

URL: http://svn.apache.org/r1539771
Log:
Add a workaround for a Jira bug (to be removed later) as Jira is being used for testing

Modified:
    tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java

Modified: tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java?rev=1539771&r1=1539770&r2=1539771&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java (original)
+++ tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java Thu Nov  7 19:24:17 2013
@@ -24,6 +24,7 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Locale;
 import java.util.Set;
 
@@ -111,7 +112,10 @@ public class StandardRoot extends Lifecy
         }
 
         // Set because we don't want duplicates
-        HashSet<String> result = new HashSet<>();
+        // LinkedHashSet to retain the order (shouldn't matter but Jira is
+        // sensitive to the order the JARs are returned in).
+        // TODO - Revert to HashSet
+        HashSet<String> result = new LinkedHashSet<>();
         for (ArrayList<WebResourceSet> list : allResources) {
             for (WebResourceSet webResourceSet : list) {
                 if (!webResourceSet.getClassLoaderOnly()) {



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1539771 - /tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java

Posted by Mark Thomas <ma...@apache.org>.
On 08/11/2013 11:01, Konstantin Kolinko wrote:
> 2013/11/7  <ma...@apache.org>:
>> Author: markt
>> Date: Thu Nov  7 19:24:17 2013
>> New Revision: 1539771
>>
>> URL: http://svn.apache.org/r1539771
>> Log:
>> Add a workaround for a Jira bug (to be removed later) as Jira is being used for testing
>>
> 
> Ordering of jars is important. The ones in <PreResources> should be
> listed first.

The ordering of the groups is important but - and this was the Jira bug
- apps should not be depending on a particular priority order for JARs
in WEB-INF/lib.

Retaining the order in listResources is probably the simplest way of
implementing this.

> (VirtualWebappLoader of Tomcat 7 can inject jars to be searched before
> ones in WEB-INF/lib or after them, depending on configuration.  One
> should be able to configure the same in Tomcat 8).

Agreed.

> The same s/HashSet/LinkedHashSet/ is likely true for the
> implementation of #listWebAppPaths() as well, though I am not sure
> about an use case there.

I don't think so as it is only the paths in String form that are
returned. The required ordering would be enforced when one of those
paths was looked up.

> One more note on #listWebAppPaths(), looking at a method that uses it:
> 
> Javadoc of method ApplicationContext.getResourcePaths(String) says
> that the returned set is immutable. This statement is not true in
> Tomcat 8, as far as I see it returns the created mutable HashSet as
> is,
> 
> Actually, I do not see such immutability requirement in Javadoc of
> ServletContext (neither in ours nor in the one at Oracle site),
> http://docs.oracle.com/javaee/7/api/javax/servlet/ServletContext.html

I fixed the Javadoc.

Thanks for the review.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1539771 - /tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/11/7  <ma...@apache.org>:
> Author: markt
> Date: Thu Nov  7 19:24:17 2013
> New Revision: 1539771
>
> URL: http://svn.apache.org/r1539771
> Log:
> Add a workaround for a Jira bug (to be removed later) as Jira is being used for testing
>

Ordering of jars is important. The ones in <PreResources> should be
listed first.

(VirtualWebappLoader of Tomcat 7 can inject jars to be searched before
ones in WEB-INF/lib or after them, depending on configuration.  One
should be able to configure the same in Tomcat 8).


The same s/HashSet/LinkedHashSet/ is likely true for the
implementation of #listWebAppPaths() as well, though I am not sure
about an use case there.


One more note on #listWebAppPaths(), looking at a method that uses it:

Javadoc of method ApplicationContext.getResourcePaths(String) says
that the returned set is immutable. This statement is not true in
Tomcat 8, as far as I see it returns the created mutable HashSet as
is,

Actually, I do not see such immutability requirement in Javadoc of
ServletContext (neither in ours nor in the one at Oracle site),
http://docs.oracle.com/javaee/7/api/javax/servlet/ServletContext.html


> Modified:
>     tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java?rev=1539771&r1=1539770&r2=1539771&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/webresources/StandardRoot.java Thu Nov  7 19:24:17 2013
> @@ -24,6 +24,7 @@ import java.net.URISyntaxException;
>  import java.net.URL;
>  import java.util.ArrayList;
>  import java.util.HashSet;
> +import java.util.LinkedHashSet;
>  import java.util.Locale;
>  import java.util.Set;
>
> @@ -111,7 +112,10 @@ public class StandardRoot extends Lifecy
>          }
>
>          // Set because we don't want duplicates
> -        HashSet<String> result = new HashSet<>();
> +        // LinkedHashSet to retain the order (shouldn't matter but Jira is
> +        // sensitive to the order the JARs are returned in).
> +        // TODO - Revert to HashSet
> +        HashSet<String> result = new LinkedHashSet<>();
>          for (ArrayList<WebResourceSet> list : allResources) {
>              for (WebResourceSet webResourceSet : list) {
>                  if (!webResourceSet.getClassLoaderOnly()) {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org