You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Mohammad Nour El-Din <no...@gmail.com> on 2008/07/22 14:26:28 UTC

Re: svn commit: r678330 - /openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java

Hi Karan...

  I really like the way you add comments to commits specially the "How
it was fixed" section, thank you very much for teaching me a new good
practice :)

On Mon, Jul 21, 2008 at 3:04 AM,  <km...@apache.org> wrote:
> Author: kmalhi
> Date: Sun Jul 20 17:04:56 2008
> New Revision: 678330
>
> URL: http://svn.apache.org/viewvc?rev=678330&view=rev
> Log:
> OPENEJB-861 Add support to add JSF impl jars to WEB-INF/lib
> FIXED this issue
>
> How it was fixed:-
> Made the org.apache.openejb.core.TempClassLoader also load classes belong to packages starting with javax.faces
> This means that the TempClassLoader will be able to load javax.faces.FacesServlet, which is configured
> in web.xml of every JSF application
>
>
> Modified:
>    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
>
> Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
> URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java?rev=678330&r1=678329&r2=678330&view=diff
> ==============================================================================
> --- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java (original)
> +++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java Sun Jul 20 17:04:56 2008
> @@ -60,12 +60,21 @@
>         // bug #283. defer to system if the name is a protected name.
>         // "sun." is required for JDK 1.4, which has an access check for
>         // sun.reflect.GeneratedSerializationConstructorAccessor1
> -        if (name.startsWith("java.") ||
> -                name.startsWith("javax.") ||
> -                name.startsWith("sun.")) {
> +        /*
> +         * FIX for openejb-tomcat JSF support . Added the following to the if statement below: !name.startsWith("javax.faces")
> +         *We want to use this TempClassLoader to also load the classes in the javax.faces package.
> +         *If not, then our AnnotationDeployer will not be able to load the javax.faces.FacesServlet class if this class is in a jar which
> +         *is in the WEB-INF/lib directory of a web app.
> +         * see AnnotationDeployer$ProcessAnnotatedBeans.deploy(WebModule webModule)
> +         * Here is what happened  before this fix was applied:
> +         * 1. The AnnotationDeployer tries to load the javax.faces.FacesServlet using this classloader (TempClassLoader)
> +         * 2. Since this class loader uses Class.forName to load classes starting with java, javax or sun, it cannot load javax.faces.FacesServlet
> +         * 3. Result is , AnnotationDeployer throws a ClassNotFoundException
> +         */
> +        if ( !name.startsWith("javax.faces.") && ( name.startsWith("java.") || name.startsWith("javax.") || name.startsWith("sun."))) {
>             return Class.forName(name, resolve, getClass().getClassLoader());
>         }
> -
> +//        ( && !name.startsWith("javax.faces.") )||
>         String resourceName = name.replace('.', '/') + ".class";
>         InputStream in = getResourceAsStream(resourceName);
>         if (in == null) {
>
>
>



-- 
Thanks
- Mohammad Nour

Re: svn commit: r678330 - /openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java

Posted by Mohammad Nour El-Din <no...@gmail.com>.
Sure, you can extract the tips Dain mentioned in the mail you'd sent
to me, and then I can add on it if I found something new with my task
:)

On Tue, Jul 22, 2008 at 3:33 PM, Karan Malhi <ka...@gmail.com> wrote:
> Thanks,
>
> I just followed the tips and suggestions at this wiki page.
> http://openejb.apache.org/tips-and-suggestions.html
>
> This is the procedure which DBlevins recommends.
> I also tried to follow this time to add the revision number to the JIRA,
> (see Vamsi's suggestion)
>
> We should also add some tips for Jaxb in here.
>
> On Tue, Jul 22, 2008 at 8:26 AM, Mohammad Nour El-Din <
> nour.mohammad@gmail.com> wrote:
>
>> Hi Karan...
>>
>>  I really like the way you add comments to commits specially the "How
>> it was fixed" section, thank you very much for teaching me a new good
>> practice :)
>>
>> On Mon, Jul 21, 2008 at 3:04 AM,  <km...@apache.org> wrote:
>> > Author: kmalhi
>> > Date: Sun Jul 20 17:04:56 2008
>> > New Revision: 678330
>> >
>> > URL: http://svn.apache.org/viewvc?rev=678330&view=rev
>> > Log:
>> > OPENEJB-861 Add support to add JSF impl jars to WEB-INF/lib
>> > FIXED this issue
>> >
>> > How it was fixed:-
>> > Made the org.apache.openejb.core.TempClassLoader also load classes belong
>> to packages starting with javax.faces
>> > This means that the TempClassLoader will be able to load
>> javax.faces.FacesServlet, which is configured
>> > in web.xml of every JSF application
>> >
>> >
>> > Modified:
>> >
>>  openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
>> >
>> > Modified:
>> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
>> > URL:
>> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java?rev=678330&r1=678329&r2=678330&view=diff
>> >
>> ==============================================================================
>> > ---
>> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
>> (original)
>> > +++
>> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
>> Sun Jul 20 17:04:56 2008
>> > @@ -60,12 +60,21 @@
>> >         // bug #283. defer to system if the name is a protected name.
>> >         // "sun." is required for JDK 1.4, which has an access check for
>> >         // sun.reflect.GeneratedSerializationConstructorAccessor1
>> > -        if (name.startsWith("java.") ||
>> > -                name.startsWith("javax.") ||
>> > -                name.startsWith("sun.")) {
>> > +        /*
>> > +         * FIX for openejb-tomcat JSF support . Added the following to
>> the if statement below: !name.startsWith("javax.faces")
>> > +         *We want to use this TempClassLoader to also load the classes
>> in the javax.faces package.
>> > +         *If not, then our AnnotationDeployer will not be able to load
>> the javax.faces.FacesServlet class if this class is in a jar which
>> > +         *is in the WEB-INF/lib directory of a web app.
>> > +         * see AnnotationDeployer$ProcessAnnotatedBeans.deploy(WebModule
>> webModule)
>> > +         * Here is what happened  before this fix was applied:
>> > +         * 1. The AnnotationDeployer tries to load the
>> javax.faces.FacesServlet using this classloader (TempClassLoader)
>> > +         * 2. Since this class loader uses Class.forName to load classes
>> starting with java, javax or sun, it cannot load javax.faces.FacesServlet
>> > +         * 3. Result is , AnnotationDeployer throws a
>> ClassNotFoundException
>> > +         */
>> > +        if ( !name.startsWith("javax.faces.") && (
>> name.startsWith("java.") || name.startsWith("javax.") ||
>> name.startsWith("sun."))) {
>> >             return Class.forName(name, resolve,
>> getClass().getClassLoader());
>> >         }
>> > -
>> > +//        ( && !name.startsWith("javax.faces.") )||
>> >         String resourceName = name.replace('.', '/') + ".class";
>> >         InputStream in = getResourceAsStream(resourceName);
>> >         if (in == null) {
>> >
>> >
>> >
>>
>>
>>
>> --
>> Thanks
>> - Mohammad Nour
>>
>
>
>
> --
> Karan Singh Malhi
>



-- 
Thanks
- Mohammad Nour

Re: svn commit: r678330 - /openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java

Posted by Karan Malhi <ka...@gmail.com>.
Thanks,

I just followed the tips and suggestions at this wiki page.
http://openejb.apache.org/tips-and-suggestions.html

This is the procedure which DBlevins recommends.
I also tried to follow this time to add the revision number to the JIRA,
(see Vamsi's suggestion)

We should also add some tips for Jaxb in here.

On Tue, Jul 22, 2008 at 8:26 AM, Mohammad Nour El-Din <
nour.mohammad@gmail.com> wrote:

> Hi Karan...
>
>  I really like the way you add comments to commits specially the "How
> it was fixed" section, thank you very much for teaching me a new good
> practice :)
>
> On Mon, Jul 21, 2008 at 3:04 AM,  <km...@apache.org> wrote:
> > Author: kmalhi
> > Date: Sun Jul 20 17:04:56 2008
> > New Revision: 678330
> >
> > URL: http://svn.apache.org/viewvc?rev=678330&view=rev
> > Log:
> > OPENEJB-861 Add support to add JSF impl jars to WEB-INF/lib
> > FIXED this issue
> >
> > How it was fixed:-
> > Made the org.apache.openejb.core.TempClassLoader also load classes belong
> to packages starting with javax.faces
> > This means that the TempClassLoader will be able to load
> javax.faces.FacesServlet, which is configured
> > in web.xml of every JSF application
> >
> >
> > Modified:
> >
>  openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
> >
> > Modified:
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
> > URL:
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java?rev=678330&r1=678329&r2=678330&view=diff
> >
> ==============================================================================
> > ---
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
> (original)
> > +++
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/TempClassLoader.java
> Sun Jul 20 17:04:56 2008
> > @@ -60,12 +60,21 @@
> >         // bug #283. defer to system if the name is a protected name.
> >         // "sun." is required for JDK 1.4, which has an access check for
> >         // sun.reflect.GeneratedSerializationConstructorAccessor1
> > -        if (name.startsWith("java.") ||
> > -                name.startsWith("javax.") ||
> > -                name.startsWith("sun.")) {
> > +        /*
> > +         * FIX for openejb-tomcat JSF support . Added the following to
> the if statement below: !name.startsWith("javax.faces")
> > +         *We want to use this TempClassLoader to also load the classes
> in the javax.faces package.
> > +         *If not, then our AnnotationDeployer will not be able to load
> the javax.faces.FacesServlet class if this class is in a jar which
> > +         *is in the WEB-INF/lib directory of a web app.
> > +         * see AnnotationDeployer$ProcessAnnotatedBeans.deploy(WebModule
> webModule)
> > +         * Here is what happened  before this fix was applied:
> > +         * 1. The AnnotationDeployer tries to load the
> javax.faces.FacesServlet using this classloader (TempClassLoader)
> > +         * 2. Since this class loader uses Class.forName to load classes
> starting with java, javax or sun, it cannot load javax.faces.FacesServlet
> > +         * 3. Result is , AnnotationDeployer throws a
> ClassNotFoundException
> > +         */
> > +        if ( !name.startsWith("javax.faces.") && (
> name.startsWith("java.") || name.startsWith("javax.") ||
> name.startsWith("sun."))) {
> >             return Class.forName(name, resolve,
> getClass().getClassLoader());
> >         }
> > -
> > +//        ( && !name.startsWith("javax.faces.") )||
> >         String resourceName = name.replace('.', '/') + ".class";
> >         InputStream in = getResourceAsStream(resourceName);
> >         if (in == null) {
> >
> >
> >
>
>
>
> --
> Thanks
> - Mohammad Nour
>



-- 
Karan Singh Malhi