Mailing-List: contact dev-help@curator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@curator.apache.org
Received-SPF: pass (nike.apache.org: local policy)
From: Simon Kitching <Simon.Kitching@smartstream-stp.com>
To: "dev@curator.apache.org" <dev@curator.apache.org>
Subject: RE: Proposal : remove references to guava library from public APIs
Thread-Topic: Proposal : remove references to guava library from public APIs
Thread-Index: AdBrqV071uMrJy19RCGnfxooZSREVAAD6q0AACZ58U4=
Date: Wed, 1 Apr 2015 08:08:07 +0000
Message-ID: <E0A869BF0B7A1647B6CFD99DDAB0D373DA242C6E@VIEEXCH0002.sst.stp>
References: 
 <E0A869BF0B7A1647B6CFD99DDAB0D373DA241B69@VIEEXCH0002.sst.stp>,<etPan.551ac13c.15014acb.146@Jordans-MacBook-Pro-2.local>
In-Reply-To: <etPan.551ac13c.15014acb.146@Jordans-MacBook-Pro-2.local>
Accept-Language: en-US, de-AT
Content-Language: en-US
Content-Type: multipart/alternative;
	boundary="_000_E0A869BF0B7A1647B6CFD99DDAB0D373DA242C6EVIEEXCH0002ssts_"
MIME-Version: 1.0

--_000_E0A869BF0B7A1647B6CFD99DDAB0D373DA242C6EVIEEXCH0002ssts_
Content-Type: text/plain; charset="Windows-1252"
Content-Transfer-Encoding: quoted-printable

Thanks Jordan.

The root cause of the problem isn't really anything osgi-specific; it's the=
 fact that curator uses another library (guava) as part of its _public_ API=
.

Imagine you wanted to change from using guava to some other collections lib=
rary - it wouldn't be possible without breaking the public API of curator. =
The question is whether guava really should be part of the curator API, or =
should be just an implementation detail. I would suggest that the use of gu=
ava is really an implementation detail that should be private/hidden - unli=
ke use of jaxws for example, which really is an externally-defined abstract=
 API and is reasonable to include as part of the public API of curator.

This difference between used-in-the-impl and used-in-the-api doesn't matter=
 so much in a java application with one big classloader that has every sing=
le jarfile in it; if you need the guava library in the classpath for intern=
al use by curator, then it will automatically also be visible to all other =
classes and so it is impossible to have a different version of the library =
also present. Using OSGi (which creates multiple classloaders) can allow mu=
ltiple versions of the same lib - but only when the lib is only used-in-the=
-impl (ie is for "internal" usage by a jarfile).

Re keeping a compatible API: possibly all classes in package "org.apache.cu=
rator.framework.listen" could be copied into a new package, and then the Li=
stenerContainer class updated to not expose guava. All classes in "org.apac=
he.curator.framework.listen" could then be deprecated. As long as OSGi code=
 avoids using any code from the old package, there would be no binding to t=
he guava library used by curator. I don't know if that would be better than=
 simply changing the API for a couple of methods or not.

I will create a JIRA issue to update the maven-build-plugin version; that's=
 trivial and is not a binary incompatibility.

Unless somebody objects within the next few days, I will also create a JIRA=
 issue regarding the APIs that expose guava. I might have time to work on t=
his myself next month (may).

By the way, if you're interested in how OSGi classloading works, this may b=
e helpful: http://moi.vonos.net/java/osgi-classloaders/

Thanks & Regards,
Simon (aka skitching at apache.org)

________________________________
From: Jordan Zimmerman [jordan@jordanzimmerman.com]
Sent: Tuesday, March 31, 2015 17:46
To: dev@curator.apache.org; Simon Kitching
Subject: Re: Proposal : remove references to guava library from public APIs

I don=92t have an objection in general. The biggest problem for me is that =
I know very little about OSGI. All of the OSGI work has been contributed so=
 it=92s hard to make sure that we keep it working. That said, changing exis=
ting APIs is very disruptive to the Curator community. I=92d like to see so=
meone (Simon?) commit to maintaining the OSGi compatibility of Curator and =
make sure releases don=92t introduce issues. Also, can the existing APIs re=
main and new, OSGi compatible parallel APIs be added?

-JZ


On March 31, 2015 at 7:39:08 AM, Simon Kitching (simon.kitching@smartstream=
-stp.com<mailto:simon.kitching@smartstream-stp.com>) wrote:

Hi,

I've noticed that several curator classes expose the use of classes from go=
ogle's guava library [1] as part of their *public* api.

[1] maven artifact "com.google.guava:guava" which contains java packages co=
m.google.common.*

In an OSGi environment, it is possible to load multiple different versions =
of the same library, as long as that library is a purely internal implement=
ation detail. Unfortunately, as curator exposes its use of guava, this make=
s it impossible for code that uses curator to also use a different version =
of Guava for its own purposes. Unfortunately, this has just bitten me : I n=
eed to write code that uses both curator (requires guava 16.0 or later) and=
 a third-party library that requires an earlier version of guava.

Are there any objections to me raising an enhancement issue in JIRA for thi=
s? Note that this change would be a binary incompatibility (though fairly l=
imited).

The problem classes that I have found are:
* curator-framework: org.apache.curator.framework.listen.ListenerContainer =
: method forEach takes a parameter of type com.google.common.base.Function
* curator-framework: org.apache.curator.framework.api.transaction.CuratorTr=
ansactionResult : method ofTypeAndPath returns com.google.common.base.Predi=
cate
* curator-x-discovery-server: org.apache.curator.x.discovery.server.context=
s.GenericDiscoveryContext : constructor takes param of type com.google.comm=
on.reflect.TypeToken
* curator-x-discovery: org.apache.curator.x.discovery.InstanceFilter : inhe=
rits from com.google.common.base.Predicate

And by the way, I noticed that org.codehaus.jackson types are also used in =
public APIs (at least, GenericDiscoveryContext). It may also be worth looki=
ng into whether it is really necessary to expose this dependency.

The goal of the change would be to ensure that in the MANIFEST.MF file for =
each curator bundle (jarfile), the Export-Packages line minimises the "uses=
:=3D" entries which refer to non-curator packages. A uses-constraint on a p=
ackage should only be needed when something in the package being exported u=
ses an external type in its public API.

As a separate problem, I have noticed that with the 2.7.1 release (at least=
), the "bnd" tool (via maven-bundle-plugin) is adding entries to the "uses"=
 entries even when the referenced library is purely used internally. I have=
 found a reference (https://github.com/emlun/bnd-uses-strange) that suggest=
s this is a bug which is fixed in later bnd releases. Unfortunately I can f=
ind no release-notes for bnd, nor any source-code repository so cannot conf=
irm this. However updating curator/pom.xml to specify the following fixes t=
he "uses" clauses:
<maven-bundle-plugin-version>2.5.3</maven-bundle-plugin-version>

Thanks & Regards,
Simon

________________________________
The information in this email is confidential and may be legally privileged=
. It is intended solely for the addressee. Access to this email by anyone e=
lse is unauthorised. If you are not the intended recipient, any disclosure,=
 copying, distribution or any action taken or omitted to be taken in relian=
ce on it, is prohibited and may be unlawful. SmartStream Technologies GmbH,=
 Vienna Twin Tower, Wienerbergstrasse 11, 1100 Vienna, Austria, FN 194340w,=
 HG Wien
________________________________
The information in this email is confidential and may be legally privileged=
. It is intended solely for the addressee. Access to this email by anyone e=
lse is unauthorised. If you are not the intended recipient, any disclosure,=
 copying, distribution or any action taken or omitted to be taken in relian=
ce on it, is prohibited and may be unlawful. SmartStream Technologies GmbH,=
 Vienna Twin Tower, Wienerbergstrasse 11, 1100 Vienna, Austria, FN 194340w,=
 HG Wien

--_000_E0A869BF0B7A1647B6CFD99DDAB0D373DA242C6EVIEEXCH0002ssts_--