axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From keith chapman <keithgchap...@gmail.com>
Subject Re: Axis2 API design issue, Should we fix it?
Date Mon, 11 May 2009 09:51:52 GMT
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
<andreas.veithen@gmail.com>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 <keithgchapman@gmail.com>
> 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 <
> andreas.veithen@gmail.com>
> > 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 <keithgchapman@gmail.com>
> >> wrote:
> >> > Shall we go ahead with this change then?
> >> >
> >> > Thanks,
> >> > Keith.
> >> >
> >> > On Wed, May 6, 2009 at 7:11 PM, Amila Suriarachchi
> >> > <amilasuriarachchi@gmail.com> wrote:
> >> >>
> >> >>
> >> >> On Wed, May 6, 2009 at 4:33 PM, Glen Daniels <glen@thoughtcraft.com>
> >> >> 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

Mime
View raw message