AFAIK there have been quite a bit of development on the trunk since the Axis2 1.5 branch was cut hence if we are to do a 1.5.1 release it will have to be done off the 1.5 branch and not the trunk. Hence I don't see an issue with doing this change on the trunk.
Thanks, Keith. On Mon, May 11, 2009 at 3:09 PM, Andreas Veithen <[email protected]>wrote: > Please note that changing the return types from concrete > implementations to interfaces will break binary compatibility. This > means that if we do the change on trunk now, the next release from > trunk should be a major release, i.e. 1.6. How are we going to handle > the case where we need to produce a new release quickly after 1.5 to > fix serious issues (don't forget that 1.4.1 was produces by merging > only selected changes from trunk to 1.4 and that therefore 1.5 > contains probably lots of changes)? If we don't do the API changes > immediately, then we keep the option of doing a 1.5.1 maintenance > release from the trunk. > > Andreas > > On Mon, May 11, 2009 at 10:27, keith chapman <[email protected]> > wrote: > > I don't think we can put it into 1.5. Lets make the change in the trunk > so > > that it will be available in the next release. > > > > Thanks, > > Keith. > > > > On Mon, May 11, 2009 at 1:45 PM, Andreas Veithen < > [email protected]> > > wrote: > >> > >> +1, but we need to agree on the target release for this change. > >> > >> Andreas > >> > >> On Mon, May 11, 2009 at 09:57, keith chapman <[email protected]> > >> wrote: > >> > Shall we go ahead with this change then? > >> > > >> > Thanks, > >> > Keith. > >> > > >> > On Wed, May 6, 2009 at 7:11 PM, Amila Suriarachchi > >> > <[email protected]> wrote: > >> >> > >> >> > >> >> On Wed, May 6, 2009 at 4:33 PM, Glen Daniels <[email protected]> > >> >> wrote: > >> >>> > >> >>> Amila Suriarachchi wrote: > >> >>> > Hm - it seems like you didn't actually get my point. We CAN'T > >> >>> > return the > >> >>> > actual allServices map because that would be breaking the > >> >>> > abstraction > >> >>> > boundary provided by the class. > >> >>> > > >> >>> > As I remember this change was done to avoid concurrent > modifications > >> >>> > to > >> >>> > service map[1]. > >> >>> > >> >>> Right - before this change we were doing something actively > bad/wrong, > >> >>> i.e. > >> >>> returning the internal map. After the change we were returning a > >> >>> cloned > >> >>> copy > >> >>> of the map (though not using clone() for some reason), which is good > >> >>> in > >> >>> that > >> >>> it prevented people from misusing it. > >> >>> > >> >>> > At that time services map was a HashMap and could not fix the > issue > >> >>> > by > >> >>> > changing it to a ConcurrentHashMap since API method returns a > >> >>> > HashMap. > >> >>> > > >> >>> > Currently anyone can get a copy of internal map (I think we can > not > >> >>> > use > >> >>> > clone() method since internal implementation is ConcurrentHashMap > >> >>> > and > >> >>> > we > >> >>> > need to return a HashMap). And if they want to add or remove > service > >> >>> > they have to use addService and removeService respectively. > >> >>> > > >> >>> > Keith, if you really need the internal map IMHO best way is to add > a > >> >>> > new > >> >>> > API method. > >> >>> > >> >>> Ah, no. The "best way" is NOT TO OFFER ACCESS TO THE INTERNAL MAP. > >> >>> > >> >>> Keith's suggestion is to change the API so that it returns a Map - > >> >>> this > >> >>> would > >> >>> be much more correct since it increases the level of abstraction for > >> >>> the > >> >>> method, and would therefore let us, the implementors, freely decide > >> >>> how > >> >>> this > >> >>> should work internally. Right now we have problems because someone > >> >>> made > >> >>> this > >> >>> method overly specific, and this is what we should fix. (I was > >> >>> incorrect > >> >>> earlier when I said this wouldn't break people, btw - I was thinking > >> >>> about > >> >>> stuff like getServices().get("MyService"), but of course "HashMap > foo > >> >>> = > >> >>> getServices()" would fail. I still think we should fix it.) > >> >>> > >> >>> My comment is that we should not only return a Map, we should change > >> >>> the > >> >>> implementation to make sure the Map is immutable, and make sure the > >> >>> JavaDoc > >> >>> explains what is going on. > >> >> > >> >> +1. Here the real problem is this API contains a hashMap instead of > >> >> Map. > >> >> > >> >> What I got from the Keiths' first mail is that he need the API change > >> >> to > >> >> return the internal map without copying. I suggested to add a new > >> >> method if > >> >> he really need it so that only he use that method. I agree with you > >> >> that > >> >> this is not a good change. > >> >> > >> >> thanks, > >> >> Amila. > >> >>> > >> >>> > >> >>> Make sense? > >> >>> > >> >>> --Glen > >> >> > >> >> > >> >> > >> >> -- > >> >> Amila Suriarachchi > >> >> WSO2 Inc. > >> >> blog: http://amilachinthaka.blogspot.com/ > >> > > >> > > >> > > >> > -- > >> > Keith Chapman > >> > Senior Software Engineer > >> > WSO2 Inc. > >> > Oxygenating the Web Service Platform. > >> > http://wso2.org/ > >> > > >> > blog: http://www.keith-chapman.org > >> > > > > > > > > > -- > > Keith Chapman > > Senior Software Engineer > > WSO2 Inc. > > Oxygenating the Web Service Platform. > > http://wso2.org/ > > > > blog: http://www.keith-chapman.org > > > -- Keith Chapman Senior Software Engineer WSO2 Inc. Oxygenating the Web Service Platform. http://wso2.org/ blog: http://www.keith-chapman.org
