Hi all, The servlet4preview package in Tomcat 8.5 "provides early access to some of the features of Servlet 4.0" ( https://tomcat.apache.org/tomcat-8.5-doc/api/org/apache/catalina/servlet4preview/package-summary.html ).
Does use of this package make OFBiz dependent on a preview version, and if so is that a good idea? Should features that use some preview (like support for Tomcat SSO, it appears) be contained in a branch or plugin until the API has been stabilised? Tomcat 9 was released in January, with support for Servlet 4.0. If we now say OFBiz requires Servlet 4.0 and move to Tomcat 9, could we then use the standard HttpServletRequest? Thanks Paul Foxworthy On 25 March 2018 at 04:49, Scott Gray <[email protected]> wrote: > So to summarize your long email ;-) > > "The service engine has a limitation in that it only checks the interfaces > directly implemented by the object being tested. Tomcat's RequestFacade > doesn't directly implement javax.servlet.http.HttpServletRequest so it > fails to pass the type validation." > > On the surface your suggested fix looks fine to me, my only concern being > that we may need to do some performance testing. For every service that > specifies an interface we'll now be checking the full type hierarchy which > I imagine will be more expensive but I'm not sure how expensive. > > I'm also not even sure if our custom ObjectType methods for checking this > type of thing are necessary any more with so many new java versions since > it was decided these were needed for performance reasons. > > Regards > Scott > > On 24 March 2018 at 10:59, Jacques Le Roux <[email protected]> > wrote: > > > Le 23/03/2018 à 17:09, Scott Gray a écrit : > > > >> I don't need to try anything, I *know* that the service engine is > supposed > >> to accept a concrete class of an interface if the interface is specified > >> as > >> the attribute type. > >> > >> Either the service engine is broken by not accepting concrete > >> implementations, or the bug report is incorrect. > >> > > Neither, it's "unfortunate", and a bit complicated. > > > > Tomcat servlet4preview was introduced with Tomcat 8.5 > > https://tomcat.apache.org/tomcat-8.5-doc/api/org/apache/cata > > lina/servlet4preview/package-summary.html > > > > Before James introduced Tomcat SSO, we had only one service passing a > > javax.servlet.http.HttpServletRequest to a service: > payPalCheckoutUpdate. > > > > AFAIK, we have actually always passed a org.apache.catalina.connector. > RequestFacade > > to services when asking for javax.servlet.http.HttpServletRequest in > > services definition. > > Since Tomcat 8.5 RequestFacade implements javax.servlet.http. > HttpServletRequest > > indirectly through org.apache.catalina.servlet4pr > > eview.http.HttpServletRequest > > https://tomcat.apache.org/tomcat-8.5-doc/api/org/apache/cata > > lina/connector/RequestFacade.html > > > > Since we have no tests on the payPalCheckoutUpdate service we did not > spot > > that Tomcat servlet4preview was introduced with Tomcat 8.5 > > > > The classes check is done by the deeper interfaceOf() method of the > > ObjectType class using Class.getInterfaces() > > https://docs.oracle.com/javase/8/docs/api/java/lang/Class. > > html#getInterfaces-- > > Class.getInterfaces() does not recurse and stops at one level up. So in > > case of RequestFacade it returns only servlet4preview.http. > HttpServletRequest > > and not javax.servlet.http.HttpServletRequest > > So when interfaceOf() compares the Classes it fails. > > > > What happened with my introduction of the HttpServletRequestWrapper in > > ContextFilter is it hid the RequestFacade because > HttpServletRequestWrapper > > implements javax.servlet.http.HttpServletRequest > > https://docs.oracle.com/javaee/7/api/javax/servlet/http/ > > HttpServletRequest.html > > > > So when James introduced Tomcat SSO and optionally passed a > > javax.servlet.http.HttpServletRequest to the userLogin service it did > not > > break. > > But when I removed HttpServletRequestWrapper from ContextFilter it popped > > up > > > > Summary: it's unfortunate because we have no tests on the > > payPalCheckoutUpdate service. > > Because I temporarily introduced HttpServletRequestWrapper James was able > > to pass a javax.servlet.http.HttpServletRequest, like in > > payPalCheckoutUpdate. > > When I reverted (removed HttpServletRequestWrapper from ContextFilter) I > > discovered that we had a problem with Tomcat 8.5. > > > > I propose this fix which uses org.apache.commons.lang3. > ClassUtils.getAllInterfaces() > > and works > > http://commons.apache.org/proper/commons-lang/javadocs/api- > > release/src-html/org/apache/commons/lang3/ClassUtils.html > > > > Index: framework/base/src/main/java/org/apache/ofbiz/base/util/Obje > > ctType.java > > =================================================================== > > --- framework/base/src/main/java/org/apache/ofbiz/base/util/ > ObjectType.java > > (révision 1827594) > > +++ framework/base/src/main/java/org/apache/ofbiz/base/util/ > ObjectType.java > > (copie de travail) > > @@ -263,7 +263,7 @@ > > */ > > public static boolean interfaceOf(Class<?> objectClass, Class<?> > > interfaceClass) { > > while (objectClass != null) { > > - Class<?>[] ifaces = objectClass.getInterfaces(); > > + List<Class<?>> ifaces = org.apache.commons.lang3.Class > > Utils.getAllInterfaces(objectClass); > > > > for (Class<?> iface: ifaces) { > > if (iface == interfaceClass) { > > Index: framework/common/servicedef/services.xml > > =================================================================== > > --- framework/common/servicedef/services.xml (révision 1827594) > > +++ framework/common/servicedef/services.xml (copie de travail) > > @@ -379,7 +379,7 @@ > > <service name="userLogin" engine="java" location=" > > org.apache.ofbiz.common.login.LoginServices" invoke="userLogin"> > > <description>Used to Automatically Authenticate a > > username/password; create a UserLogin object</description> > > <implements service="authenticationInterface"/> > > - <attribute name="request" mode="IN" type="org.apache.catalina. > connector.RequestFacade" > > optional="true"/> > > + <attribute name="request" mode="IN" type="javax.servlet.http. > HttpServletRequest" > > optional="true"/> > > </service> > > <service name="createUserLogin" engine="java" auth="false" > > location="org.apache.ofbiz.common.login.LoginServices" > > invoke="createUserLogin"> > > > > Jacques > > > > > > > > > >> Regards > >> Scott > >> > >> On 23 March 2018 at 07:36, Jacques Le Roux < > [email protected]> > >> wrote: > >> > >> Did you try what I said? > >>> > >>> You can easily check by svn updating to r1819133 and removing the > wrapper > >>> in ContextFilter.java. > >>> > >>> Maybe we need to revert Tomcat SSO then? > >>> > >>> Jacques > >>> > >>> > >>> > >>> Le 23/03/2018 à 03:39, Scott Gray a écrit : > >>> > >>> Something else must be wrong Jacques, I can't understand what you're > >>>> saying > >>>> in the ticket description or in your reply above but I do know the > >>>> following: > >>>> OFBiz is perfectly capable of knowing that > >>>> org.apache.catalina.connector.RequestFacade > >>>> implements the javax.servlet.http.HttpServletRequest and it should > pass > >>>> type validation without errors. > >>>> > >>>> Since we know that, this commit becomes unnecessary. > >>>> > >>>> Regards > >>>> Scott > >>>> > >>>> On 22 March 2018 at 16:06, Jacques Le Roux < > >>>> [email protected]> > >>>> wrote: > >>>> > >>>> Michael, > >>>> > >>>>> I commited http://svn.apache.org/viewvc/o > >>>>> fbiz/ofbiz-framework/trunk/fra > >>>>> mework/webapp/src/main/java/org/apache/ofbiz/webapp/contro > >>>>> l/ContextFilter.java?r1=1813679&r2=1813678&pathrev=1813679 > >>>>> > >>>>> Where I added a wrapper. Then for "Tomcat SSO" (OFBIZ-10047) James > >>>>> committed > >>>>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra > >>>>> mework/common/servicedef/services.xml?r1=1819133&r2=1819132& > >>>>> pathrev=1819133 > >>>>> > >>>>> If I had not committed the wrapper in ContextFilter.java beforehand, > >>>>> James > >>>>> would not have been allowed to commit > >>>>> > >>>>> <attribute name="request" mode="IN" type="javax.servlet.http.HttpS > >>>>> ervletRequest" > >>>>> optional="true"/> > >>>>> > >>>>> Because it's then rejected, because then the userLogin service > actually > >>>>> receive an org.apache.catalina.connector.RequestFacade > (RequestFacade > >>>>> implements HttpServletRequest.) > >>>>> > >>>>> You can easily check by svn updating to r1819133 and removing the > >>>>> wrapper > >>>>> in ContextFilter.java. > >>>>> > >>>>> You are right that it ties OFBiz to Tomcat. We took this decision > >>>>> sometimes ago and we no longer support other webapp servers OOTB, nor > >>>>> tools > >>>>> like Jetty. > >>>>> > >>>>> So OFBiz OOTB totally depends on Tomcat as the build.gradle file > shows. > >>>>> So > >>>>> I think it's safe to use a RequestFacade there. > >>>>> > >>>>> If an user feels the need to use another webapp servers or Jetty, I > >>>>> think > >>>>> changing that would not be the most of the worries (the rest being > much > >>>>> more frightening, I know I did it once with Geronimo). > >>>>> > >>>>> Since the check is done at the service level, it's hard to do > otherwise > >>>>> But I must say I did not dig too much and it's maybe possible, we can > >>>>> discuss that... > >>>>> > >>>>> Jacques > >>>>> > >>>>> > >>>>> > >>>>> > >>>>> Le 22/03/2018 à 16:22, Michael Brohl a écrit : > >>>>> > >>>>> Mmhhh, Jacques, > >>>>> > >>>>>> I think this is problematic because it ties a special implementation > >>>>>> for > >>>>>> Tomcat to the service. I didn't see this anywhere else. > >>>>>> > >>>>>> The issue (https://issues.apache.org/jira/browse/OFBIZ-10304) is a > >>>>>> bit > >>>>>> unclear and I don't get the purpose of this change. > >>>>>> > >>>>>> Can you please explain more clearly which problem this changes > solves > >>>>>> and > >>>>>> why we'll need org.apache.catalina.connector.RequestFacade as the > >>>>>> type? > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> Michael > >>>>>> > >>>>>> > >>>>>> Am 21.03.18 um 21:53 schrieb [email protected]: > >>>>>> > >>>>>> Author: jleroux > >>>>>> > >>>>>>> Date: Wed Mar 21 20:53:41 2018 > >>>>>>> New Revision: 1827439 > >>>>>>> > >>>>>>> URL: http://svn.apache.org/viewvc?rev=1827439&view=rev > >>>>>>> Log: > >>>>>>> Fixed: The "request" attribute type of the userLogin service is > wrong > >>>>>>> (OFBIZ-10304) > >>>>>>> > >>>>>>> It should have been org.apache.catalina.connector.RequestFacade" > >>>>>>> instead of javax.servlet.http.HttpServletRequest see the Jira for > >>>>>>> details > >>>>>>> > >>>>>>> Modified: > >>>>>>> ofbiz/ofbiz-framework/trunk/framework/common/servicedef/ > services.xml > >>>>>>> > >>>>>>> Modified: ofbiz/ofbiz-framework/trunk/fr > >>>>>>> amework/common/servicedef/serv > >>>>>>> ices.xml > >>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/fra > >>>>>>> mework/common/servicedef/services.xml?rev=1827439&r1=1827438 > >>>>>>> &r2=1827439&view=diff > >>>>>>> ============================================================ > >>>>>>> ================== > >>>>>>> --- ofbiz/ofbiz-framework/trunk/framework/common/servicedef/serv > >>>>>>> ices.xml > >>>>>>> (original) > >>>>>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/servicedef/serv > >>>>>>> ices.xml > >>>>>>> Wed Mar 21 20:53:41 2018 > >>>>>>> @@ -379,7 +379,7 @@ under the License. > >>>>>>> <service name="userLogin" engine="java" location=" > >>>>>>> org.apache.ofbiz.common.login.LoginServices" invoke="userLogin"> > >>>>>>> <description>Used to Automatically Authenticate a > >>>>>>> username/password; create a UserLogin object</description> > >>>>>>> <implements service="authenticationInterface"/> > >>>>>>> - <attribute name="request" mode="IN" > >>>>>>> type="javax.servlet.http.HttpServletRequest" optional="true"/> > >>>>>>> + <attribute name="request" mode="IN" > >>>>>>> type="org.apache.catalina.connector.RequestFacade" > optional="true"/> > >>>>>>> </service> > >>>>>>> <service name="createUserLogin" engine="java" auth="false" > >>>>>>> location="org.apache.ofbiz.common.login.LoginServices" > >>>>>>> invoke="createUserLogin"> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > > > -- Coherent Software Australia Pty Ltd PO Box 2773 Cheltenham Vic 3192 Australia Phone: +61 3 9585 6788 Web: http://www.coherentsoftware.com.au/ Email: [email protected]
