Yeah, I don't have a problem with doing this for Proton 0.4 if people think
the API change caused by our constructor visibility reduction is too
abrupt.

I don't think this is strictly necessary for Hiram's code to continue
working, since Keith and I are modifying it to use factories.  This is
already done on the JNI branch - take a look at
https://svn.apache.org/repos/asf/qpid/proton/branches/jni-binding/proton-j/contrib.
However, I understand there may be other projects already calling these
constructors so accept we need to be polite to our users.

We'd put @SuppressWarnings("deprecation") in EngineFactoryImpl so that its
(valid) use of the constructors wouldn't cause compiler warnings.

We would then make the constructors package-private in the followong
release - Proton 0.5.

Any objections?

Phil


On 28 January 2013 11:40, Rafael Schloming <r...@alum.mit.edu> wrote:

> Would it be better if we were to deprecate the constructors and remove them
> in a future release?
>
> --Rafael
>
> On Fri, Jan 25, 2013 at 6:25 PM, Hiram Chirino <hi...@hiramchirino.com
> >wrote:
>
> > I was hoping I could upgrade to newer proton versions without having to
> > change my client code, since I was not planning on using the JNI impl
> > anyways.
> >
> >
> > On Fri, Jan 25, 2013 at 4:24 PM, Rajith Attapattu <rajit...@gmail.com
> > >wrote:
> >
> > > This is also my impression about Hirams work.
> > > So I'm not sure why there is resistance to the changes being proposed
> > > as it's only going to benefit in the long run should the impl changes.
> > > (I do understand that there will be some initial work in changing the
> > > code to use the factories and interfaces).
> > >
> > > I also thought part of Phil's (and Rob's) work was to come up with a
> > > set of interfaces for the API.
> > > If I'm not mistaken this work is already sitting in a branch ? (I
> > > stand to be corrected though).
> > >
> > > Rajith
> > >
> > > On Fri, Jan 25, 2013 at 3:44 PM, Rafael Schloming <r...@alum.mit.edu>
> > > wrote:
> > > > FWIW, I don't think Hiram's usage fundamentally needs to depend on
> the
> > > pure
> > > > Java impl. I may be out of date on this one, but I believe his access
> > of
> > > > the implementation was done for expediency. I know at least in a
> number
> > > of
> > > > cases we discussed how the C implementation could accommodate his
> > > > requirements, and we plan do to so at some point. Perhaps our efforts
> > > here
> > > > would be better spent in building out the interface so that he
> doesn't
> > > need
> > > > direct access to the implementation.
> > > >
> > > > --Rafael
> > > >
> > > > On Fri, Jan 25, 2013 at 10:31 AM, Phil Harvey <
> > p...@philharveyonline.com
> > > >wrote:
> > > >
> > > >> This has implications for API versioning,
> > > >>
> > > >> I think developers of third party code can assume that the API of
> > public
> > > >> methods, including constructors, will remain stable across minor
> > > revisions
> > > >> of a library.  If a constructor is public then we, the Proton
> > > developers,
> > > >> have an obligation to respect this assumption (or at least to add
> "for
> > > >> internal use only" to its Javadoc).
> > > >>
> > > >> On reflection, I'm coming round to the idea that keeping these
> > > constructors
> > > >> public will place unreasonable restrictions on our ability to evolve
> > the
> > > >> implementation whilst keeping the API stable.
> > > >>
> > > >> Phil
> > > >>
> > > >>
> > > >> On 25 January 2013 14:51, Rob Godfrey <rob.j.godf...@gmail.com>
> > wrote:
> > > >>
> > > >> > What's your need for the direct constructor?
> > > >> >
> > > >> >
> > > >> > -- Rob
> > > >> >
> > > >> > On 25 January 2013 15:49, Hiram Chirino <hi...@hiramchirino.com>
> > > wrote:
> > > >> > > Let me clarify, I have no problem with adding Factories and
> using
> > > them
> > > >> > > everywhere possible.  Just don't take my access away from direct
> > > >> > > constructors.
> > > >> > >
> > > >> > >
> > > >> > > On Fri, Jan 25, 2013 at 9:39 AM, Rob Godfrey <
> > > rob.j.godf...@gmail.com
> > > >> > >wrote:
> > > >> > >
> > > >> > >> In general dependency on implementation is bad, dependency on
> > > >> interface
> > > >> > is
> > > >> > >> better.  One of the problems we've had with Qpid historically
> is
> > it
> > > >> > become
> > > >> > >> hard to test as we have dependencies on implementation
> > everywhere.
> > > >> > >>
> > > >> > >> From an end user perspective the difference is only replacing
> > > >> > >>
> > > >> > >> X x  = new X();
> > > >> > >>
> > > >> > >> with
> > > >> > >>
> > > >> > >> X x = XFactory.newInstance();
> > > >> > >>
> > > >> > >> But it makes it a lot easier to properly describe the
> > functionality
> > > >> > being
> > > >> > >> returned. For implementation reasons, the concrete class may
> have
> > > >> public
> > > >> > >> methods that are not intended to be exposed to the application
> > > >> > programmer
> > > >> > >> but only to other cooperating classes.  To avoid confusion, and
> > to
> > > >> make
> > > >> > it
> > > >> > >> clear to us the delta between the "shared" API and the API
> > > extensions
> > > >> > that
> > > >> > >> we are supporting for the pure Java implementation the
> interfaces
> > > seem
> > > >> > like
> > > >> > >> the right way to go for me.
> > > >> > >>
> > > >> > >> The idea is that those who need the extension features will
> still
> > > >> have a
> > > >> > >> very clear way to get the implementation that provides these,
> > > without
> > > >> > ever
> > > >> > >> having to cast.
> > > >> > >>
> > > >> > >> -- Rob
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >>
> > > >> > >> On 24 January 2013 20:03, Rafael Schloming <r...@alum.mit.edu>
> > > wrote:
> > > >> > >>
> > > >> > >> > On Thu, Jan 24, 2013 at 5:06 AM, Rob Godfrey <
> > > >> rob.j.godf...@gmail.com
> > > >> > >> > >wrote:
> > > >> > >> >
> > > >> > >> > > On 23 January 2013 17:36, Phil Harvey <
> > > p...@philharveyonline.com>
> > > >> > >> wrote:
> > > >> > >> > > >
> > > >> > >> > > > As part of the Proton JNI work, I would like to remove
> all
> > > calls
> > > >> > to
> > > >> > >> > > > proton-j implementation constructors from "client code".
>  I
> > > >> intend
> > > >> > >> that
> > > >> > >> > > > factories will be used instead [1], thereby abstracting
> > away
> > > >> > whether
> > > >> > >> > the
> > > >> > >> > > > implementation is pure Java or proton-c-via-JNI.
> > > >> > >> > > >
> > > >> > >> > > > I'd like to check that folks are happy with me making
> this
> > > >> change,
> > > >> > >> and
> > > >> > >> > to
> > > >> > >> > > > mention a couple of problems I've had.
> > > >> > >> > > >
> > > >> > >> > > > In this context, "client code" is anything outside the
> > > current
> > > >> > >> > > > sub-component, where our sub-components are Engine,
> Codec,
> > > >> Driver,
> > > >> > >> > > Message
> > > >> > >> > > > and Messenger, plus each of the contrib modules, and of
> > > course
> > > >> > third
> > > >> > >> > > party
> > > >> > >> > > > code.
> > > >> > >> > > >
> > > >> > >> > > > To enforce this abstraction, I am planning to make the
> > > >> > constructors
> > > >> > >> of
> > > >> > >> > > the
> > > >> > >> > > > affected classes package-private where possible.  I
> believe
> > > >> that,
> > > >> > >> > > although
> > > >> > >> > > > third party projects might already be calling these
> > > >> constructors,
> > > >> > it
> > > >> > >> is
> > > >> > >> > > > acceptable for us to change its public API in this manner
> > > while
> > > >> > >> Proton
> > > >> > >> > is
> > > >> > >> > > > such a young project.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > +1 to all of the above
> > > >> > >> > >
> > > >> > >> > > >
> > > >> > >> > > > Please shout if you disagree with any of the above.
> > > >> > >> > > >
> > > >> > >> > > > Now, onto my problem.  I started off with the
> > > >> > >> > > org.apache.qpid.proton.engine.
> > > >> > >> > > > impl package, and found that
> > > >> >  o.a.q.p.hawtdispatch.impl.AmqpTransport
> > > >> > >> > > calls
> > > >> > >> > > > various methods on ConnectionImpl and TransportImpl, so
> > > simply
> > > >> > using
> > > >> > >> a
> > > >> > >> > > > Connection and Transport will not work.  I don't know
> what
> > > to do
> > > >> > >> about
> > > >> > >> > > > this, and would welcome people's opinions.
> > > >> > >> > > >
> > > >> > >> > >
> > > >> > >> > > So, the simplest change would be to change the factories to
> > use
> > > >> > >> > > covariant return types
> > > >> > >> > >
> > > >> > >> > > e.g. EngingFactoryImpl becomes:
> > > >> > >> > >
> > > >> > >> > >     @Override
> > > >> > >> > >     public ConnectionImpl createConnection()
> > > >> > >> > >     {
> > > >> > >> > >         return new ConnectionImpl();
> > > >> > >> > >     }
> > > >> > >> > >
> > > >> > >> > >     @Override
> > > >> > >> > >     public TransportImpl createTransport()
> > > >> > >> > >     {
> > > >> > >> > >         return new TransportImpl();
> > > >> > >> > >     }
> > > >> > >> > >
> > > >> > >> > >
> > > >> > >> > > ... etc
> > > >> > >> > >
> > > >> > >> > > Code that requires the extended functionality offered by
> the
> > > pure
> > > >> > Java
> > > >> > >> > > implementation can thus instantiate the desired Factory
> > > directly.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > What's the point of going through the factory in this
> scenario
> > > >> rather
> > > >> > >> than
> > > >> > >> > directly instantiating the classes as Hiram suggests? Is
> there
> > > some
> > > >> > class
> > > >> > >> > of thing the factory would/could do that the constructor
> > > >> > can't/shouldn't?
> > > >> > >> >
> > > >> > >> > A second refinement might be to actually separate out the
> > > interface
> > > >> > >> > > and implementation within the pure Java implementation so
> > that
> > > we
> > > >> > have
> > > >> > >> > > a well defined "extended" Java API.  This interface could
> > then
> > > be
> > > >> > the
> > > >> > >> > > return type of the factory.
> > > >> > >> > >
> > > >> > >> >
> > > >> > >> > Maybe I'm misunderstanding, but what's the point of using an
> > > >> interface
> > > >> > >> here
> > > >> > >> > if you're still locked into the pure Java impl? Are you
> > > expecting to
> > > >> > swap
> > > >> > >> > out that impl under some circumstances?
> > > >> > >> >
> > > >> > >> > --Rafael
> > > >> > >> >
> > > >> > >>
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > **
> > > >> > >
> > > >> > > *Hiram Chirino*
> > > >> > >
> > > >> > > *Engineering | Red Hat, Inc.*
> > > >> > >
> > > >> > > *hchir...@redhat.com <hchir...@redhat.com> | fusesource.com |
> > > >> redhat.com
> > > >> > *
> > > >> > >
> > > >> > > *skype: hiramchirino | twitter: @hiramchirino<
> > > >> > http://twitter.com/hiramchirino>
> > > >> > > *
> > > >> > >
> > > >> > > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/
> >*
> > > >> >
> > > >>
> > >
> >
> >
> >
> > --
> >
> > **
> >
> > *Hiram Chirino*
> >
> > *Engineering | Red Hat, Inc.*
> >
> > *hchir...@redhat.com <hchir...@redhat.com> | fusesource.com | redhat.com
> *
> >
> > *skype: hiramchirino | twitter: @hiramchirino<
> > http://twitter.com/hiramchirino>
> > *
> >
> > *blog: Hiram Chirino's Bit Mojo <http://hiramchirino.com/blog/>*
> >
>

Reply via email to