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 11:06:45 GMT
+1 Sounds good to me.

Thanks,
Keith.

On Mon, May 11, 2009 at 3:58 PM, Andreas Veithen
<andreas.veithen@gmail.com>wrote:

> I forgot about the refactoring of the clustering API, which is in
> trunk but didn't go into 1.5. That indeed means that that we can't do
> a 1.5.1 release from trunk.
>
> To summarize, the roadmap for the months after the 1.5 release (when
> is this actually going to happen?) would look like this:
>
> Release 1.5.1 (from 1.5 branch):
> - Critical bug fixes (if required)
> - Changes that didn't make it into 1.5 (e.g. the changes in
> axis2-saaj) and that can easily be merged from the trunk (if there is
> a user demand for this)
>
> Release 1.6 (current trunk):
> - API cleanup (concrete classes -> interfaces)
> - Refactoring of the clustering API (already in trunk)
> - Improvements to message builder/formatter API and implementations,
> as discussed in December
> - Clarifications and improvements in the transport API (if I'm not
> mistaken, there was a discussion around this some time ago)
> - ...
>
> If everybody is fine with this, then we should go ahead.
>
> Andreas
>
> On Mon, May 11, 2009 at 11:51, keith chapman <keithgchapman@gmail.com>
> wrote:
> > 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
> >
>



-- 
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