On Fri, Aug 21, 2009 at 4:06 PM, Andreas Veithen <andreas.veit...@gmail.com>wrote:
> Assume that a servlet extends AxisServlet and overrides the init > method with an implementation that doesn't call AxisServlet#init, but > that is a modified copy & paste of the code in AxisServlet (for an > example of this, see SynapseAxisServlet). Is that considered as an > extension that relies solely on public APIs? May be not in the usual case. But I see this as a rather special case. The init method actually comes from the high level interface javax.servlet.Servlet. Different servlet implementations may have drastically different requirements at initialization and hence that is something we need to take into consideration in AxisServlet. But anyway if we can provide a single protected method to handle port auto detection stuff we can keep the init method unchanged. The AxisServletListener attributes are required for the port auto detection only, so if the user is going to override that functionality, those attributes not getting initialized is not an issue. Just my 2 cents. Thanks, Hiranya > > Andreas > > On Fri, Aug 21, 2009 at 07:09, Hiranya Jayathilaka<hiranya...@gmail.com> > wrote: > > > > > > On Fri, Aug 21, 2009 at 5:54 AM, Deepal jayasinghe <deep...@gmail.com> > > wrote: > >> > >> > > >> > 2) I think there is no need to start yelling around about "merrily > >> > chang[ing] APIs", "things are going to become a big mess" and "engage > >> > the community before making such drastic changes". I did lots of fixes > >> > in Axiom and Axis2 since Synapse trunk switched from Axis2 SNAPSHOT to > >> > 1.5, and when Hiranya switched back to SNAPSHOT there were no > >> > particular issues related to them, except for the issue discussed > >> > here. I personally take the suggestion that my changes make of Axis2 a > >> > big mess as an offense. If that is what some people think, I can stop > >> > immediately to work on Axis2 and Axiom. With respect to engaging the > >> > community, as mentioned above, the description of AXIS2-4465 provides > >> > enough information about the reasons for the change and as Dims > >> > pointed out, we have a commit-and-review policy. Since the change is > >> > focused, not massive, can easily be reverted and contains an > >> > appropriate level of Javadoc, I think that the community has > >> > everything that is needed to review it. > >> > > >> You are doing a great work with Axis2 project and as I can see you are > >> so active in the project, so there is no need to stop what you are > doing. > > > > +1. > > > > Andreas, you have been a great contributor for Axis2 and AXIOM over the > past > > few months and I'm sure the community highly values and appreciates all > your > > contributions. Most of your changes were very good ones and required ones > > too. It is just this change needs a bit of reworking and cleanup to > maintain > > compatibility with the custom extensions - that's all :) > > > > Thanks, > > Hiranya > > > >> > >> > > >> > 3) The discussion in AXIS2-4465 pointed to the problem that even by > >> > preserving the public API of AxisServlet, we will probably not be able > >> > to avoid breaking subclasses. This is caused by the fact that > >> > AxisServlet doesn't have an extensible design. If somebody has a > >> > brilliant idea how to get around this problem (other than reverting > >> > the fix and reopening the 6 issues it is supposed to solve), please > >> > speak out. > >> > > >> I am not going to ask you to revert the patch, but we should not change > >> the public APIs. This is not the first time that happens, and I those > >> times I also point them out. Just because we have commit then review, we > >> do not have time to go though each and every commits (when I was having > >> time I did that). So as the commiters of the project let's not try to > >> break the features we have or let's not change public APIs. > >> > >> Thanks, > >> Deepal > >> > Andreas > >> > > >> > [1] http://markmail.org/message/62dixjx3qrqry3yr > >> > [2] http://svn.apache.org/viewvc?view=rev&revision=744900 > >> > > >> > On Thu, Aug 20, 2009 at 18:08, Senaka Fernando<sen...@wso2.com> > wrote: > >> > > >> >> Hi Andreas, > >> >> > >> >> Just wondering what you are trying to achieve here. Is this related > to > >> >> auto > >> >> detection of ports as Hiranya pointed out? While I appreciate the > >> >> effort > >> >> you've put into doing something worthwhile, I believe that getting > rid > >> >> of a > >> >> public method in a class is not the correct thing to do. I believe > that > >> >> what > >> >> you have done here is the addition of a new portion of code. Can we > >> >> make the > >> >> new portion of code optional? And leave the existing logic as it was? > >> >> Also, > >> >> are you planning further changes to this class? if so, it would > perhaps > >> >> be > >> >> better to figure out a more elaborate solution, which safeguards both > >> >> the > >> >> existing level of extensibility of this class and also its public > API. > >> >> > >> >> Thanks, > >> >> Senaka > >> >> > >> >> On Thu, Aug 20, 2009 at 6:14 PM, Hiranya Jayathilaka > >> >> <hiranya...@gmail.com> > >> >> wrote: > >> >> > >> >>> Hi Andreas, > >> >>> > >> >>> By looking at the code I got the impression that HTTP transport > >> >>> receivers > >> >>> should extend the AxisServletListener class for your logic of port > >> >>> auto > >> >>> detection to work. Is that correct? What happens if the transport > >> >>> receivers > >> >>> used do not extend this class? All request handler methods call the > >> >>> preprocessRequest method which in turns run port auto detection. If > >> >>> the > >> >>> transport receivers do not extend AxisServlerListener how is that > >> >>> handled? > >> >>> > >> >>> Thanks, > >> >>> Hiranya > >> >>> > >> >>> > >> >>> On Thu, Aug 20, 2009 at 6:05 PM, Andreas Veithen > >> >>> <andreas.veit...@gmail.com> wrote: > >> >>> > >> >>>> Afkham, > >> >>>> > >> >>>> The only change I see in the public APIs is the disappearance of > the > >> >>>> initContextRoot method. We can easily fix this be restoring the > >> >>>> original initContextRoot method and let the preprocessRequest > method > >> >>>> call initContextRoot. Do you see any other things to change? > >> >>>> > >> >>>> Andreas > >> >>>> > >> >>>> On Thu, Aug 20, 2009 at 13:45, Afkham Azeez<afk...@gmail.com> > wrote: > >> >>>> > >> >>>>> Yes Dims. However, if everybody continues to merrily change APIs, > >> >>>>> making public methods private & so on, things are going to become > a > >> >>>>> big mess. Axis2 provides public APIs, and those may be having > >> >>>>> problems, but still they are public APIs. This is why you have to > be > >> >>>>> very careful when defining APIs; if you get them wrong, you may > have > >> >>>>> to live with it for a long time. > >> >>>>> > >> >>>>> Azeez > >> >>>>> > >> >>>>> On Thu, Aug 20, 2009 at 11:38 AM, Davanum > >> >>>>> Srinivas<dava...@gmail.com> > >> >>>>> wrote: > >> >>>>> > >> >>>>>> Azeez, > >> >>>>>> > >> >>>>>> We are still following, commit-then-review right? > >> >>>>>> > >> >>>>>> thanks, > >> >>>>>> dims > >> >>>>>> > >> >>>>>> On 08/20/2009 07:33 AM, Afkham Azeez wrote: > >> >>>>>> > >> >>>>>>> Hi Andreas, > >> >>>>>>> The changes you've done to the APIs as per > >> >>>>>>> https://issues.apache.org/jira/browse/AXIS2-4465 badly breaks > some > >> >>>>>>> of > >> >>>>>>> the projects that depend on Axis2. Please revert this, and > please > >> >>>>>>> engage the community before making such drastic changes in the > >> >>>>>>> future. > >> >>>>>>> > >> >>>>>>> > >> >>>>> > >> >>>>> -- > >> >>>>> Thanks > >> >>>>> Afkham Azeez > >> >>>>> > >> >>>>> Blog: http://afkham.org > >> >>>>> Developer Portal: http://www.wso2.org > >> >>>>> WSAS Blog: http://wso2wsas.blogspot.com > >> >>>>> Company: http://wso2.com > >> >>>>> GPG Fingerprint: 643F C2AF EB78 F886 40C9 B2A2 4AE2 C887 665E > 0760 > >> >>>>> > >> >>>>> > >> >>> > >> >>> -- > >> >>> Hiranya Jayathilaka > >> >>> Software Engineer; > >> >>> WSO2 Inc.; http://wso2.org > >> >>> E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 > >> >>> Blog: http://techfeast-hiranya.blogspot.com > >> >>> > >> >> > >> > > >> > > >> > >> > >> -- > >> Thank you! > >> > >> > >> http://blogs.deepal.org > >> http://deepal.org > >> > > > > > > > > -- > > Hiranya Jayathilaka > > Software Engineer; > > WSO2 Inc.; http://wso2.org > > E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 > > Blog: http://techfeast-hiranya.blogspot.com > > > -- Hiranya Jayathilaka Software Engineer; WSO2 Inc.; http://wso2.org E-mail: hira...@wso2.com; Mobile: +94 77 633 3491 Blog: http://techfeast-hiranya.blogspot.com
