Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-27 Thread Jacques Le Roux
Le 27/03/2018 à 11:38, Taher Alkhateeb a écrit : On Tue, Mar 27, 2018 at 11:13 AM, Jacques Le Roux wrote: Hi Taher, That's good questions and I'll try to build a technical perspective here answering them one by one and as possible in chronology. 1. Changes to cookies was introduced with OFB

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-27 Thread Taher Alkhateeb
On Tue, Mar 27, 2018 at 11:13 AM, Jacques Le Roux wrote: > Hi Taher, > > That's good questions and I'll try to build a technical perspective here > answering them one by one and as possible in chronology. > > 1. Changes to cookies was introduced with OFBIZ-4959. Actually it was not > really a bug

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-27 Thread Jacques Le Roux
Hi Taher, That's good questions and I'll try to build a technical perspective here answering them one by one and as possible in chronology. 1. Changes to cookies was introduced with OFBIZ-4959. Actually it was not really a bug rather a clean-up. The autoLogin cookies were only used by the e

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
BTW Paul, I forgot to thank you for your effort Yesterday I thought about explaining the whole picture with this problem and especially how I came there. It's now obvious to me but I understand that a summary would help everybody. I remember Taher was expressing his frustration about that. Ja

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
I don't exactly find it, but I'll take your word. Anyway it makes absolutely sense, not a point to discuss more. At least I'll not. Jacques Le 27/03/2018 à 02:07, Scott Gray a écrit : Yeah, I said exactly that elsewhere in the same email. Regards Scott On 26 March 2018 at 21:17, Jacques Le

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
Hi Paul, Inline... Le 27/03/2018 à 03:42, Paul Foxworthy a écrit : I hadn't read through OFBIZ-9833 until this morning. My understanding is now: - Tomcat SSO is a red herring. It can be implemented with HttpServletRequest. As you say, it doesn't need Servlet 4 or the servlet4preview package.

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
Le 25/03/2018 à 03:11, Scott Gray a écrit : That would solve the problem for now until the service engine is improved and tested or the tomcat SSO login design is improved. How is the tomcat SSO login design related here? I see only Tomcat 8.5. Of course the tomcat SSO login design may be improve

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Paul Foxworthy
Hi Jacques, On 27 March 2018 at 08:16, Jacques Le Roux wrote: > What makes you think that Tomcat SSO depends on servlet4preview? > Only your words "So when James introduced Tomcat SSO and optionally passed a javax.servlet.http.HttpServletRequest to the userLogin service it did not break. But w

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Scott Gray
Yeah, I said exactly that elsewhere in the same email. Regards Scott On 26 March 2018 at 21:17, Jacques Le Roux wrote: > Le 24/03/2018 à 18:49, Scott Gray a écrit : > >> I'm also not even sure if our custom ObjectType methods for checking this >> type of thing are necessary any more with so man

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
Michael, AFAIK, Tomcat SSO does not directly relates to this issue. For me it was just a catalyseur which allowed me to find the servlet4preview issue due to Tomcat 8.5 usage. And I can say it was not easy. This said all reviews of https://issues.apache.org/jira/browse/OFBIZ-10047 are welcome.

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
Le 24/03/2018 à 18:49, Scott Gray a écrit : 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. We would need some profiling before making any

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
That's indeed another option, but I'd prefer to update to Tomcat 9, to definitely get ride of servlet4preview and problems it brings Jacques Le 25/03/2018 à 03:11, Scott Gray a écrit : Another option if we don't want to have to care about this service engine limitation right now is the type-v

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-26 Thread Jacques Le Roux
Hi Paul, All, inline... Le 25/03/2018 à 03:02, Paul Foxworthy a écrit : 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 ).

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-25 Thread Michael Brohl
Hi Paul, I agree, that's what I tried to express in my comments on https://issues.apache.org/jira/browse/OFBIZ-10047: "7. Someone should check this solution from an architectural view. I appreciate the efforts but I am also in doubt if we should put this feature into the new release. It's ve

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-24 Thread Scott Gray
Another option if we don't want to have to care about this service engine limitation right now is the type-validation child element of attribute. Just specify a class/method that takes the HttpServletRequest as an Object parameter and then perform the "getAllInterfaces()" test inside there. e.g. U

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-24 Thread Paul Foxworthy
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

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-24 Thread Scott Gray
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

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-24 Thread Jacques Le Roux
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

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Scott Gray
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. Regards Scott

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Taher Alkhateeb
I have issues with multiple decisions all around that same topic that never got community consensus. Changes to cookies, http redirects, authentication, and other commits that did not get a proper review from the community. Such major design decisions need proper review IMO On Fri, Mar 23, 2018 at

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Jacques Le Roux
Le 23/03/2018 à 09:33, Jacques Le Roux a écrit : Le 23/03/2018 à 09:21, Jacopo Cappellato a écrit : On Fri, Mar 23, 2018 at 8:36 AM, Jacques Le Roux < jacques.le.r...@les7arts.com> wrote: Did you try what I said? You can easily check by svn updating to r1819133 and removing the wrapper in Con

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Jacques Le Roux
Le 23/03/2018 à 09:21, Jacopo Cappellato a écrit : On Fri, Mar 23, 2018 at 8:36 AM, Jacques Le Roux < jacques.le.r...@les7arts.com> 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 S

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Jacopo Cappellato
On Fri, Mar 23, 2018 at 8:36 AM, Jacques Le Roux < jacques.le.r...@les7arts.com> 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? A thorough review of that feature

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Jacques Le Roux
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 i

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-23 Thread Jacques Le Roux
See my answer to Scott Jacques Le 23/03/2018 à 07:00, Taher Alkhateeb a écrit : Not only is this commit unnecessary, but also breaks one of the best features in Object Oriented Programming -- polymorphism -- for no value that I can see anywhere at all. Also by saying we depend fully on Tomcat

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-22 Thread Taher Alkhateeb
Not only is this commit unnecessary, but also breaks one of the best features in Object Oriented Programming -- polymorphism -- for no value that I can see anywhere at all. Also by saying we depend fully on Tomcat and we should write code that further enforces that idea instead of making our archi

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-22 Thread Scott Gray
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 i

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-22 Thread Jacques Le Roux
Michael, I commited http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java?r1=1813679&r2=1813678&pathrev=1813679 Where I added a wrapper. Then for "Tomcat SSO" (OFBIZ-10047) James committed http://svn.apache.

Re: svn commit: r1827439 - /ofbiz/ofbiz-framework/trunk/framework/common/servicedef/services.xml

2018-03-22 Thread Michael Brohl
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